Build: pass through Embedded flag to link jobs
#7304
Conversation
MaxDesiatov
force-pushed
the
maxd/fix-embedded-linking
branch
2 times, most recently
from
6840d35 to
8fc6189
Compare
|
@swift-ci test |
|
I think we should vend the used experimental/upcoming flags as a separate API at the model level instead of parsing the generated flags after the fact. Whenever we resorted to parsing flags like this in the past it only lead to future issues. It also seems like we shouldn't limit this to one particular feature flag, adding special handling like that is the opposite of what the feature flags are supposed to accomplish. Can we just pass each of them to the link invocation? |
|
Would it not be more prudent to pass the flags to the integrated driver? The swift-driver should be the source of truth. I would love to see the concept of "unsafe flags" removed entirely |
I'm not sure we use the integrated driver in all cases, probably we don't use it by default at all, at least that was my understanding. Maybe @neonichu can clarify. |
|
Yes, we're not using the integrated driver by default, IIRC the current integration is more of a prototype. |
|
@swift-ci test |
@neonichu this is addressed now by storing a new property of type Ready for review. |
Without passing this flag, Swift Driver links such products as if they're not built for embedded platforms at all, passing incorrect flags to the linker. This is not reproducible when using plain `swiftc`, as that combines both compile and link jobs by default, but SwiftPM prefers to run those separately, which requires this special handling. ### Modifications: Directly checking in `ProductBuildDescription/linkArguments` whether all of the product's targets are built in the embedded mode. If so, we're passing the embedded mode flag to Swift Driver. Unfortunately, `BuildSettings.AssignmentTable` is formed too early in the build process right in `PackageModel`, while we still need to run checks specific build settings quite late in the build stage. Because of that, there's no clean way to check if `Embedded` flag is passed other than directly rendering `BuildSettings.Scope` to a string and running a substring check on that. In the future we should move `BuildSettings` out of `PackageModel`, ensuring that SwiftPM clients don't rely on this behavior anymore. ### Result: Products that have targets using Embedded Swift can be built with SwiftPM, assuming that Swift Driver handles other linker flags correctly.
MaxDesiatov
force-pushed
the
maxd/fix-embedded-linking
branch
from
b47c2c7 to
4c2d55a
Compare
|
@swift-ci test |
|
@swift-ci test windows |
…xd/fix-embedded-linking
|
@swift-ci test |
|
@swift-ci test windows |
|
@swift-ci test |
|
@swift-ci test windows |
1 similar comment
|
@swift-ci test windows |
| // Pass experimental features to link jobs in addition to compile jobs. Preserve ordering while eliminating | ||
| // duplicates with `OrderedSet`. | ||
| var experimentalFeatures = OrderedSet<String>() | ||
| for target in self.product.targets { |
There was a problem hiding this comment.
I'm not sure what is the semantically correct way, but should we collect features from transitive dependencies too? Or it's it fine to collect from only top-level targets as is now?
Either way, it would be better to cover transitive dependencies in test cases.
There was a problem hiding this comment.
This is tricky, in my testing the embedded mode doesn't even support multiple modules in the same product right now, or at least SwiftPM is not ready for that yet. When we unblock that in a future PR, we can look into transitive dependencies support too.
### Motivation: Without passing this flag, Swift Driver links such products as if they're not built for embedded platforms at all, passing incorrect flags to the linker. This is not reproducible when using plain `swiftc`, as that combines both compile and link jobs by default, but SwiftPM prefers to run those separately, which requires this special handling. ### Modifications: Directly checking in `ProductBuildDescription/linkArguments` whether all of the product's targets are built in the embedded mode. If so, we're passing the embedded mode flag to Swift Driver. Unfortunately, `BuildSettings.AssignmentTable` is formed too early in the build process right in `PackageModel`, while we still need to run checks specific build settings quite late in the build stage. Because of that, there's no clean way to check if `Embedded` flag is passed other than directly rendering `BuildSettings.Scope` to a string and running a substring check on that. In the future we should move `BuildSettings` out of `PackageModel`, ensuring that SwiftPM clients don't rely on this behavior anymore. ### Result: Products that have targets using Embedded Swift can be built with SwiftPM, assuming that Swift Driver handles other linker flags correctly.
### Motivation: Without passing this flag, Swift Driver links such products as if they're not built for embedded platforms at all, passing incorrect flags to the linker. This is not reproducible when using plain `swiftc`, as that combines both compile and link jobs by default, but SwiftPM prefers to run those separately, which requires this special handling. ### Modifications: Directly checking in `ProductBuildDescription/linkArguments` whether all of the product's targets are built in the embedded mode. If so, we're passing the embedded mode flag to Swift Driver. Unfortunately, `BuildSettings.AssignmentTable` is formed too early in the build process right in `PackageModel`, while we still need to run checks specific build settings quite late in the build stage. Because of that, there's no clean way to check if `Embedded` flag is passed other than directly rendering `BuildSettings.Scope` to a string and running a substring check on that. In the future we should move `BuildSettings` out of `PackageModel`, ensuring that SwiftPM clients don't rely on this behavior anymore. ### Result: Products that have targets using Embedded Swift can be built with SwiftPM, assuming that Swift Driver handles other linker flags correctly.
Motivation:
Without passing this flag, Swift Driver links such products as if they're not built for embedded platforms at all, passing incorrect flags to the linker. This is not reproducible when using plain
swiftc, as that combines both compile and link jobs by default, but SwiftPM prefers to run those separately, which requires this special handling.Modifications:
Directly checking in
ProductBuildDescription/linkArgumentswhether all of the product's targets are built in the embedded mode. If so, we're passing the embedded mode flag to Swift Driver.Unfortunately,
BuildSettings.AssignmentTableis formed too early in the build process right inPackageModel, while we still need to run checks specific build settings quite late in the build stage. Because of that, there's no clean way to check ifEmbeddedflag is passed other than directly renderingBuildSettings.Scopeto a string and running a substring check on that. In the future we should moveBuildSettingsout ofPackageModel, ensuring that SwiftPM clients don't rely on this behavior anymore.Result:
Products that have targets using Embedded Swift can be built with SwiftPM, assuming that Swift Driver handles other linker flags correctly.