Skip to content

Ensure swift jobs with the same command line but different dependenci… #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 22, 2025

Conversation

owenv
Copy link
Collaborator

@owenv owenv commented May 22, 2025

…es aren't deduplicated

This can happen if the Swift Driver plans the same job in two different builds, but specifies a differtent set of input dependencies which either remove up to date jobs or add out of date ones. Without this change, this could rarely lead to looking up the wrong set of or nonexistent input jobs in an incremental build following a failed or cancelled one

Fix this by incorporating the job dependencies in the key which uniquely identifies a job's dynamic task

…es aren't deduplicated

This can happen if the Swift Driver plans the same job in two different builds, but specifies
a differtent set of input dependencies which either remove up to date jobs or add out of date ones.
Without this change, this could rarely lead to looking up the wrong set of or nonexistent input jobs
in an incremental build following a failed or cancelled one
@owenv
Copy link
Collaborator Author

owenv commented May 22, 2025

@swift-ci test

Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@owenv owenv merged commit bd0d3ee into main May 22, 2025
21 of 24 checks passed
@owenv owenv deleted the owenv/dep-dedupe branch May 22, 2025 18:28
@@ -196,6 +197,7 @@ extension LibSwiftDriver {
try driverJob = deserializer.deserialize()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owenv This missed changing 4 to 5 in deserialization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants