Cross compilation improvements #7593

xedin · 2024-05-25T00:42:17Z

A collection of fixes and improvements related to cross-compilation.

Motivation:

An attempt to make cross-compilation support more robust, consistent, error-prone and deal with performance regressions related to plugins.

Modifications:

BuildParameters are now associated with a destination (host or target) to make sure that they are always used in accordance with their original purpose.
Buildset gains a default for: parameter to indicate the intended destination of a product/target
LLBuild names for products and modules are computed based on the build parameters instead of relying on buildTriple of an product/target
Adds a way to get a command and llbuild name from a build description instead of having to reach for an underlying resolved module/product and pass parameters to it
Refactors most of the uses of allTargets and allProducts to use the graph and supply an intended destination
SwiftTargetBuildDescription no longer needs to reference toolsBuildParameters by reworking the way requires macros
are referenced
Removes ModulesGraph.updateBuildTripleRecursively
Plugins:
- Fixes a bug where graph doesn't get a "tools" product synthesized for an executable target referenced by a plugin
- Fixes a bug where plugin outputs weren't added to test targets that a rebuilt for "tools"
Tests:
- Introduces a new convenient way to mock build plans that respects tools/destination separation of parameters
- Adds a "destination" parameter to check{Target, Product} default to .destination

Result:

swift build with --target and --product now behave consistently
performance regression related to plugins has been fixed
It should be harder to request product/target without a destination

xedin · 2024-05-25T01:08:29Z

Sources/SPMBuildCore/BuildParameters/BuildParameters.swift

@@ -26,6 +26,18 @@ public struct BuildParameters: Encodable {
        case auto
    }

+    /// The destination for which code should be compiled for.
+    public enum Destination: Encodable {


I think this is an intermediate type that currently connects to buildTriple for each resolved product/module. As a follow up to these changes I'm planning to remove buildTriple, this could only be accomplished if a build plan could be formed earlier than today and all of the "destination" handling would be moved there. Modules graph would have destination-less products/modules and build plan would be responsible for determining destinations and building complete set of host/target build descriptions that other types can query if they need a product/module for a particular destination. I major obstacle to this "plan" is how plugins are built/invoked - they create a separate build command and subsequently a new build plan etc. I'm hoping to change that and make plugins/ directory obsolete.

xedin · 2024-05-27T03:52:17Z

@swift-ci please build toolchain macOS platform

…nation The change is introduced by swiftlang/swift-package-manager#7593

Sources/Build/BuildManifest/LLBuildManifestBuilder+Product.swift

MaxDesiatov

This makes sense, but needs conflicts resolved for CI to run. Also unsure if newly declared symbols need to be public instead of package, otherwise why not hide them with @_spi.

…hrough the graph First step on the way to hide and remove `all{Products, Targets}` from modules graph because with cross-compilation attempting to fetch data from them directly could produce surprising results since they don't always account for build triples.

… modules graph Build plan data structures refer to the underling module/product in their descriptions so some of the uses of module graph's `allTargets` and `allProducts` could be removed in favor of getting the same information directly from descriptions in the build plan.

… through product/target descriptions Instead of looking through the build description and passing data from it down to underlying module/product, let's provided convenient APIs directly on build descriptions.

`BuildParameters` could either be used for `host` to `target` builds, add an ability to indicate that and switch `suffix` to use that information instead of requiring triple to be passed in. This helps avoid mismatches and makes it harder to mix us parameter's intended use.

xedin · 2024-05-29T22:36:35Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test

xedin · 2024-05-29T22:36:56Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test Windows platform

Instead of using unified `buildParameters:` let's switch over to a more fine-grained control over build plan construction and allow setting common driver, linker and other flags but handle destinations internally.

…t targets Pass macros associated with a particular swift target as a list of build descriptions. That was the only users of `tools` build parameters that are now obsolete.

…rs` to `buildParameters`

… that plugin depends on Create a new executable product if plugin depends on an executable target. This is necessary, even though PackageBuilder creates one already, because that product is going to be built for `destination`, and this one has to be built for `tools`.

…ecursively` Plugins don't actually need whole graph to be switched to use `.tools` triple because `PluginTarget.processAccessibleTools` is able to find targets/products that have to be built for the host, it just needs a way to signal that to the build system. `Buildset` has been extended to support a destination which is defaulted to `.target` because really only plugin (command) tools have to set it to `.host`, this helps to propagate the information about expected destination down through `BuildOperation.build(subset:)` and find correct llbuild names and build products.

xedin · 2024-05-29T22:52:24Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test

xedin · 2024-05-29T22:52:38Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test Windows platform

rauhul · 2024-05-29T23:31:54Z

I confirmed I am able to remove the following workarounds from swift-mmio with this change.

#if compiler(>=6.0)
#warning("Skipping SVD2SwiftTests, see apple/swift-package-manager#7596.")
package.targets = package.targets.filter { $0.name != "SVD2SwiftTests" }
#warning("Skipping SVD2SwiftPluginTests, see apple/swift-package-manager#7597.")
package.targets = package.targets.filter { $0.name != "SVD2SwiftPluginTests" }
#endif

I used a toolchain built from this PR and running in xcrun --toolchain org.swift.pr.73908.1274 swift test with those lines deleted from swift-mmio's Package.swift.

…stination modules Remove incorrect assumption that the outputs of the plugin are always connected to a destination build of a particular module and add a build triple into the path to separate the outputs of a plugin. This comes into play when i.e. a test is built for both `tools` and `destination`, we need two plugin invocations to handle that.

… a destination The products and targets are currently indentified by their package + name + build triple and they should be looked up in the same fashion to avoid situations when results of `product(for:)` and `target(for:)` return target that has an unexpected destination.

…nd plugins `Buildset` provided by `SwiftBuildCommand` cannot have right destination because it doesn't know what kind of product/target it's attempting to build so we need to fix that up in `computeLLBuildTargetName` until there is a way to properly express that via command line options.

…arameters

…ethods into computed properties

xedin · 2024-05-30T16:55:42Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test Windows platform

xedin · 2024-05-30T16:56:01Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test

xedin · 2024-05-30T16:56:22Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test Windows platform

xedin · 2024-05-30T17:24:40Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test Windows platform

…nation The change is introduced by swiftlang/swift-package-manager#7593

xedin · 2024-05-30T20:01:22Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test macOS platform

xedin · 2024-05-30T20:01:28Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test Linux platform

xedin · 2024-05-30T20:27:42Z

swiftlang/sourcekit-lsp#1361
@swift-ci please test macOS platform

…nation The change is introduced by swiftlang/swift-package-manager#7593

…nation The change is introduced by swiftlang/swift-package-manager#7593 (cherry picked from commit 12b848e)

Reenables SVD2SwiftTests and SVD2SwiftPluginTests when building with SwiftPM 6.0 or newer. The bug which required disabling these tests was resolved in swiftlang/swift-package-manager#7593.

xedin requested review from bnbarham, MaxDesiatov, jakepetroules and francescomikulis as code owners May 25, 2024 00:42

xedin mentioned this pull request May 25, 2024

[DNM][main] Source compat testing/toolchain building SwiftPM changes swiftlang/swift#73908

Closed

xedin commented May 25, 2024

View reviewed changes

rauhul mentioned this pull request May 25, 2024

Unconditionally build and test MMIOInterposable apple/swift-mmio#99

Merged

xedin mentioned this pull request May 27, 2024

TestTarget unable to use build tool plugin to generate files #7597

Closed

1 task

swiftlang deleted a comment from domciiiiix May 28, 2024

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request May 28, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

620a67a

…nation The change is introduced by swiftlang/swift-package-manager#7593

xedin mentioned this pull request May 28, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti… swiftlang/sourcekit-lsp#1361

Merged

MaxDesiatov reviewed May 29, 2024

View reviewed changes

Sources/Build/BuildManifest/LLBuildManifestBuilder+Product.swift Outdated Show resolved Hide resolved

MaxDesiatov reviewed May 29, 2024

View reviewed changes

xedin added 4 commits May 29, 2024 11:09

[BuildManifest] NFC: Route uses of getCommandName and getLLBuild*…

8949a04

… through product/target descriptions Instead of looking through the build description and passing data from it down to underlying module/product, let's provided convenient APIs directly on build descriptions.

xedin force-pushed the cross-compilation-improvements branch from 91155bc to aad559f Compare May 29, 2024 22:35

xedin added 5 commits May 29, 2024 15:51

[Tests] NFC: Add a way to mock BuildPlan construction

25d1cae

Instead of using unified `buildParameters:` let's switch over to a more fine-grained control over build plan construction and allow setting common driver, linker and other flags but handle destinations internally.

[Build] Remove the need to reference tools build parameters in swif…

0f64b2e

…t targets Pass macros associated with a particular swift target as a list of build descriptions. That was the only users of `tools` build parameters that are now obsolete.

[Build] SwiftTargetBuildDescription: Rename `destinationBuildParamete…

f958833

…rs` to `buildParameters`

xedin force-pushed the cross-compilation-improvements branch from aad559f to 6aaf53d Compare May 29, 2024 22:52

xedin added 5 commits May 30, 2024 09:49

[Build/Commands] NFC: Replace more uses of buildTriple with build p…

6c816fe

…arameters

[Build] LLBuildManifest: Mark all new APIs as package and convert m…

799c0dd

…ethods into computed properties

xedin force-pushed the cross-compilation-improvements branch from 6aaf53d to 799c0dd Compare May 30, 2024 16:49

MaxDesiatov approved these changes May 30, 2024

View reviewed changes

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request May 30, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

9153f8b

…nation The change is introduced by swiftlang/swift-package-manager#7593

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request May 30, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

12b848e

…nation The change is introduced by swiftlang/swift-package-manager#7593

xedin merged commit 3a2d24b into swiftlang:main May 31, 2024
5 checks passed

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request Jun 7, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

c96df8e

…nation The change is introduced by swiftlang/swift-package-manager#7593 (cherry picked from commit 12b848e)

xedin mentioned this pull request Jun 7, 2024

[6.0] Support macros when cross-compiling #7640

Merged

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request Jun 7, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

bc3b4e9

…nation The change is introduced by swiftlang/swift-package-manager#7593 (cherry picked from commit 12b848e)

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request Jun 7, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

0151c26

…nation The change is introduced by swiftlang/swift-package-manager#7593 (cherry picked from commit 12b848e)

xedin added a commit to xedin/sourcekit-lsp that referenced this pull request Jun 8, 2024

[SwiftPMBuildSystem] Adjust BuildParameters use to indicate a desti…

6a6bc4d

…nation The change is introduced by swiftlang/swift-package-manager#7593 (cherry picked from commit 12b848e)

rauhul mentioned this pull request Jun 22, 2024

Reenable disabled tests apple/swift-mmio#105

Merged

This was linked to issues Jun 26, 2024

Cannot @testable import executable target #7596

Closed

TestTarget unable to use build tool plugin to generate files #7597

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross compilation improvements #7593

Cross compilation improvements #7593

xedin commented May 25, 2024

xedin May 25, 2024

xedin commented May 27, 2024

MaxDesiatov left a comment

xedin commented May 29, 2024

xedin commented May 29, 2024

xedin commented May 29, 2024

xedin commented May 29, 2024

rauhul commented May 29, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

Cross compilation improvements #7593

Cross compilation improvements #7593

Conversation

xedin commented May 25, 2024

Motivation:

Modifications:

Result:

xedin May 25, 2024

Choose a reason for hiding this comment

xedin commented May 27, 2024

MaxDesiatov left a comment

Choose a reason for hiding this comment

xedin commented May 29, 2024

xedin commented May 29, 2024

xedin commented May 29, 2024

xedin commented May 29, 2024

rauhul commented May 29, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024

xedin commented May 30, 2024