Skip to content

Remove variant name from explicit precompile rule info to allow deduping #291

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
Mar 12, 2025

Conversation

owenv
Copy link
Collaborator

@owenv owenv commented Mar 12, 2025

Not all variants depend on incompatible modules. Remove the variant name from the rule info so that identical tasks are deduplicated properly and cannot race.

@owenv
Copy link
Collaborator Author

owenv commented Mar 12, 2025

@swift-ci test

@owenv owenv requested a review from aciidgh March 12, 2025 02:00
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.

Change LGTM.

The test seems a bit fragile since where that will be one or two task scheduled is at the mercy of the compiler implementation to produce the same module task or not. Fine with me for now but I am not sure if we decide to stick with the current module flags for swift asan.

@owenv
Copy link
Collaborator Author

owenv commented Mar 12, 2025

@cachemeifyoucan in this test instead of using ASAN I'm building a variant with identical compiler args, which should ensure that even if the compiler changes the test is still sound

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.

I see. It is testing other variants. LGTM.

@owenv
Copy link
Collaborator Author

owenv commented Mar 12, 2025

@swift-ci test

Not all variants depend on incompatible modules. Remove the variant name from
the rule info so that identical tasks are deduplicated properly and cannot race.
@owenv owenv force-pushed the owenv/ebm-variant branch from 8d875e6 to eef2dc6 Compare March 12, 2025 23:14
@owenv
Copy link
Collaborator Author

owenv commented Mar 12, 2025

@swift-ci test

@owenv
Copy link
Collaborator Author

owenv commented Mar 12, 2025

@swift-ci test Linux

@owenv
Copy link
Collaborator Author

owenv commented Mar 12, 2025

@swift-ci test macOS

@owenv owenv merged commit 123ceb0 into main Mar 12, 2025
3 checks passed
@owenv owenv deleted the owenv/ebm-variant branch March 12, 2025 23:51
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.

5 participants