Support macros when cross-compiling #7118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci test |
MaxDesiatov
commented
Nov 23, 2023
|
@swift-ci test |
|
@swift-ci test windows |
|
What happens if we need to build a target for both triples? Separately, it feels like eventually we have to resolve the idea that there is a singular destination triple since that doesn't actually match the reality on Apple platforms. |
|
I don't have answers to these questions. This is a draft PR that doesn't fully work yet. |
MaxDesiatov
changed the base branch from
main
to
maxd/packagegraph-value-types
MaxDesiatov
force-pushed
the
maxd/packagegraph-value-types
branch
from
9ba50be
to
27d4f68
Compare
MaxDesiatov
force-pushed
the
maxd/cross-compile-macros
branch
from
624ff10
to
a9904dd
Compare
This was referenced Dec 3, 2023
MaxDesiatov
changed the base branch from
maxd/packagegraph-value-types
to
main
MaxDesiatov
force-pushed
the
maxd/cross-compile-macros
branch
from
a9904dd
to
887d02d
Compare
MaxDesiatov
added a commit
that referenced
this pull request
Dec 5, 2023
Unblocks `PackageGraph` value types refactoring in #7160 and macros cross-compilation fix in #7118. Multiple places in our codebase pass around and store `[any PackageConditionProtocol]` arrays of existentials with `FIXME` notes that those should be sets and not arrays. We also can't convert types containing these arrays to value types with auto-generated `Hashable` conformance, since neither elements of these arrays nor arrays themselves are `Hashable`. Adding `Hashable` requirement to `PackageConditionProtocol` doesn't fix the issue, since existentials of this protocol still wouldn't conform to `Hashable`. A simple alternative is to create `enum PackageCondition` with cases for each condition type. We only have two of those types and they're always declared in the same module. There's no use case for externally defined types for this protocol. That allows us to convert uses of `[any PackageConditionProtocol]` to `[PackageCondition]`. Existing protocol is kept around until clients of SwiftPM can migrate to the new `PackageCondition` enum. This also allows us to remove a custom conformance to `Hashable` on `ResolvedTarget`, unblocking a conversion of `ResolvedTarget` to a value type and making it `Sendable` in the future.
`BuildParameters` to date explicitly mentioned both host and target triples, but assumed you can use the same toolchain for both, which is not true. Additionally, it would assume that all other build parameters can be shared between the host and target, like debug info format, build configuration, sanitizers etc. That's not true either, one would use CodeView for build tools on Windows when compiling targets with DWARF for Linux, or build macros in release mode for performance when cross-compiling code that uses debug configuration itself.
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test windows |
1 similar comment
|
@swift-ci test windows |
|
@swift-ci test |
|
@swift-ci test windows |
DougGregor
added a commit
that referenced
this pull request
Feb 19, 2024
This reverts commit a0b25d3.
MaxDesiatov
added a commit
that referenced
this pull request
Feb 20, 2024
This reverts commit a0b25d3.
MaxDesiatov
added a commit
that referenced
this pull request
Feb 20, 2024
MaxDesiatov
added a commit
that referenced
this pull request
Feb 20, 2024
MaxDesiatov
added a commit
that referenced
this pull request
Apr 17, 2024
1 task
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
### Motivation: Not supporting macros in cross-compilation is a major limitation, especially for libraries like https://github.com/apple/swift-mmio. ### Modifications: Added `enum BuildTriple { case tools, destination }` and `var buildTriple: BuildTriple` on `ResolvedTarget` and `ResolvedProduct`. We're not using "host" and "target" triple terminology in this enum, as that clashes with build system "targets" and can lead to confusion in this context. Corresponding value is assigned to this property depending on target and product type: `tools` for macros, plugins, and their dependencies, `destination` for everything else (the default). Based on this property we can choose between `buildParameters.hostTriple` and `buildParameters.targetTriple` during build plan construction. Additionally, the resolved products and resolved targets graph is now constructed in a way that allows certain targets and products to be built twice: once for host triple, once for target triple if needed. This required modifying build description and build manifest generation to distinguish these products and targets from each other that are built twice. Artifacts built for the host now have `-tools` suffix appended to their names. This cascaded into making changes throughout the code base for build tool plugins and package plugins handling, which constructed their paths in an ad-hoc manner without accounting for possible changes to these names. Also added `CrossCompilationPackageGraphTests` and `CrossCompilationBuildPlanTests` to verify the changes made are applied correctly. ### Result: Resolves swiftlang#6950 Resolves rdar://105991372
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
…swiftlang#7352)" (swiftlang#7353) Reverts swiftlang#7352. Modified to build tests for the host triple when any macros are added to test modules directly as dependencies.
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
### Motivation: Not supporting macros in cross-compilation is a major limitation, especially for libraries like https://github.com/apple/swift-mmio. ### Modifications: Added `enum BuildTriple { case tools, destination }` and `var buildTriple: BuildTriple` on `ResolvedTarget` and `ResolvedProduct`. We're not using "host" and "target" triple terminology in this enum, as that clashes with build system "targets" and can lead to confusion in this context. Corresponding value is assigned to this property depending on target and product type: `tools` for macros, plugins, and their dependencies, `destination` for everything else (the default). Based on this property we can choose between `buildParameters.hostTriple` and `buildParameters.targetTriple` during build plan construction. Additionally, the resolved products and resolved targets graph is now constructed in a way that allows certain targets and products to be built twice: once for host triple, once for target triple if needed. This required modifying build description and build manifest generation to distinguish these products and targets from each other that are built twice. Artifacts built for the host now have `-tools` suffix appended to their names. This cascaded into making changes throughout the code base for build tool plugins and package plugins handling, which constructed their paths in an ad-hoc manner without accounting for possible changes to these names. Also added `CrossCompilationPackageGraphTests` and `CrossCompilationBuildPlanTests` to verify the changes made are applied correctly. ### Result: Resolves swiftlang#6950 Resolves rdar://105991372
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
…swiftlang#7352)" (swiftlang#7353) Reverts swiftlang#7352. Modified to build tests for the host triple when any macros are added to test modules directly as dependencies.
MaxDesiatov
added a commit
that referenced
this pull request
Jun 6, 2024
…#7353) Reverts #7352. Modified to build tests for the host triple when any macros are added to test modules directly as dependencies. (cherry picked from commit cb3b085) # Conflicts: # CHANGELOG.md # Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift # Sources/Build/BuildManifest/LLBuildManifestBuilder.swift # Sources/Build/BuildPlan/BuildPlan.swift # Sources/Commands/SwiftTestCommand.swift # Sources/Commands/Utilities/PluginDelegate.swift # Sources/Commands/Utilities/TestingSupport.swift # Sources/PackageGraph/ModulesGraph+Loading.swift # Sources/PackageGraph/ModulesGraph.swift # Sources/SPMTestSupport/MockBuildTestHelper.swift # Sources/SPMTestSupport/MockPackageGraphs.swift # Sources/SPMTestSupport/PackageGraphTester.swift # Sources/SPMTestSupport/ResolvedTarget+Mock.swift # Tests/BuildTests/ClangTargetBuildDescriptionTests.swift # Tests/BuildTests/CrossCompilationBuildPlanTests.swift # Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
Not supporting macros in cross-compilation is a major limitation, especially for libraries like https://github.com/apple/swift-mmio, but even more crucially for https://github.com/apple/swift-foundation and https://github.com/apple/swift-testing, all of those increasingly use macros for their core functionality.
Modifications:
Added
enum BuildTriple { case tools, destination }andvar buildTriple: BuildTripleonResolvedTargetandResolvedProduct. We're not using "host" and "target" triple terminology in this enum, as that clashes with build system "targets" and can lead to confusion in this context.Corresponding value is assigned to this property depending on target and product type:
toolsfor macros, plugins, and their dependencies,destinationfor everything else (the default). Based on this property we can choose betweenbuildParameters.hostTripleandbuildParameters.targetTripleduring build plan construction.Additionally, the resolved products and resolved targets graph is now constructed in a way that allows certain targets and products to be built twice: once for host triple, once for target triple if needed. This required modifying build description and build manifest generation to distinguish these products and targets from each other that are built twice.
Artifacts built for the host now have
-toolssuffix appended to their names. This cascaded into making changes throughout the code base for build tool plugins and package plugins handling, which constructed their paths in an ad-hoc manner without accounting for possible changes to these names.Also added
CrossCompilationPackageGraphTestsandCrossCompilationBuildPlanTeststo verify the changes made are applied correctly.Result:
Resolves #6950
Resolves rdar://105991372