Skip to content

Do not inherit host destination's extra flags for target destination when cross-compiling #3703

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

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Sep 1, 2021

Do not inherit host destination's extra flags for target destination

Motivation:

Host environment specific flags, such as XCTest module directory, were implicitly inherited by target destination. It caused unexpected reference to host module file when cross-compiling for WASI, so drop them explicitly

Modifications:

Create a new fresh Destination for WASI target and move some logic into Destination to be more generic for different targets.

CC: @MaxDesiatov

@MaxDesiatov
Copy link
Contributor

@swift-ci please smoke test

Copy link
Contributor

@neonichu neonichu left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@kateinoigakukun kateinoigakukun force-pushed the katei/drop-invalid-host-specific-args-upstream branch from a09680a to 497f846 Compare September 2, 2021 00:02
@@ -178,6 +178,24 @@ public struct Destination: Encodable, Equatable {
}
/// Cache storage for sdk platform path.
private static var _sdkPlatformFrameworkPath: (fwk: AbsolutePath, lib: AbsolutePath)? = nil

/// Returns a default destination of a given target environment
public static func defaultDestination(target: Triple, hostDestination: Destination) -> Destination? {
Copy link
Contributor

@tomerd tomerd Sep 2, 2021

Choose a reason for hiding this comment

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

target has a very specific meaning in SwiftPM, hence my suggestion to label it for. another alternative is to label it triple or targetTriple, tho I think the type already says that its a triple - which is also why labeling hostDestination: Destination is redundant as Destination is already communicated by the type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, sorry. I overlooked your comment around labels. I followed defaultDestination(for triple: Triple, host: Destination) -> Destination?

Host environment specific flags, such as XCTest module directory, were
implicitly inherited by target destination. It caused unexpected
reference to host module file, so drop them explicitly
@kateinoigakukun kateinoigakukun force-pushed the katei/drop-invalid-host-specific-args-upstream branch from 497f846 to 0ceb626 Compare September 2, 2021 03:21
@tomerd
Copy link
Contributor

tomerd commented Sep 2, 2021

@swift-ci please smoke test

@MaxDesiatov MaxDesiatov requested a review from tomerd September 2, 2021 09:46
@MaxDesiatov MaxDesiatov merged commit 603d7ee into swiftlang:main Sep 3, 2021
@kateinoigakukun kateinoigakukun deleted the katei/drop-invalid-host-specific-args-upstream branch September 3, 2021 09:38
mattt pushed a commit to mattt/swift-package-manager that referenced this pull request Sep 16, 2021
…wiftlang#3703)

Host environment specific flags, such as XCTest module directory, were
implicitly inherited by target destination. It caused unexpected
reference to host module file, so drop them explicitly
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