-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Use the correct module name for executable targets combined with an executable product with a different name #9480
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
Conversation
|
Closes #9468 |
|
@swift-ci test |
…xecutable product with a different name
20a0818 to
36cda8e
Compare
|
@swift-ci test |
|
@swift-ci test self hosted |
|
@swift-ci test windows |
|
@swift-ci test self hosted |
| buildSystem: BuildSystemProvider.Kind, | ||
| configuration: BuildConfiguration, | ||
| ) async throws { | ||
| try await withKnownIssue(isIntermittent: true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (blocking): can we add a .bug() trait referencing the GitHub issue that track the Windows intermittent failure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what IssueWindowsLongPath is for, no? It's marked intermittent so the ongoing work to resolve those issues doesn't trigger "known issue was not recorded" failures
| settings[.PRODUCT_NAME] = "$(TARGET_NAME)" | ||
| settings[.PRODUCT_MODULE_NAME] = product.c99name | ||
| // We must use the main module name here instead of the product name, because they're not guranteed to be the same, and the users may have authored e.g. tests which rely on an executable's module name. | ||
| settings[.PRODUCT_MODULE_NAME] = mainModule.c99name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: can we add a PIFBuilder Test that will validate we don't regress this behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's really important in this case that we ensure packages compile instead of just producing valid PIF, which is why I added the full text fixture above. Otherwise we might generate valid PIF which then fails to compile the user's sources.
If an executable target is combined with an executable product at the PIF layer, it must retain the target's module name, so that tests which consume it can import the right module name.