Skip to content
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

[6.0] Support macros when cross-compiling #7640

Merged
merged 14 commits into from
Jun 10, 2024

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Jun 6, 2024

Explanation: Macros cross-compiled by SwiftPM with Swift SDKs should be correctly built, loaded, and evaluated for the host triple.
Scope: isolated to modules dependency resolution and llbuild, does not impact code related to XCBuild.
Risk: medium, known issues were addressed on main and are cherry-picked here, with no new issues reported for a few weeks now.
Testing: added unit tests, manual end-to-end testing done with existing Swift SDKs.
Issue: rdar://105991372
Reviewers: @bnbarham @xedin @neonichu

(cherry picked from commit cb3b085, #7353)

# 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

(cherry picked from commit b9eb3c1, #7493)

# Conflicts:
#	Sources/PackageGraph/ModulesGraph+Loading.swift
#	Sources/PackageGraph/Resolution/ResolvedPackage.swift

(cherry picked from commit 5a4c024, #7508)

# Conflicts:
#	Sources/Commands/SwiftBuildCommand.swift
#	Sources/Commands/SwiftTestCommand.swift
#	Sources/Commands/Utilities/TestingSupport.swift

(cherry picked from commit 8db2401, #7519)

# Conflicts:
#	Tests/BuildTests/CrossCompilationBuildPlanTests.swift

…#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
@MaxDesiatov MaxDesiatov added cross-compilation build system Changes to interactions with build systems swift 6.0 Related to Swift 6.0 release branch macros Support for macros labels Jun 6, 2024
@MaxDesiatov MaxDesiatov requested a review from a team as a code owner June 6, 2024 10:47
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

MaxDesiatov and others added 4 commits June 6, 2024 11:52
Modified `ResolvedModule` build triples were not reflected in
`ResolvedPackage`, which excluded those modules from the build graph.

Verified manually with `swift-foundation` and `swift-testing` packages.
More comprehensive automated tests will be included in a future PR.

Resolves #7479.

(cherry picked from commit b9eb3c1)

# Conflicts:
#	Sources/PackageGraph/ModulesGraph+Loading.swift
#	Sources/PackageGraph/Resolution/ResolvedPackage.swift
After #7353 landed, I noticed that the build products for test targets
were not being emitted correctly. swift-testing and XCTest produce
separate build products (with distinct names) but this wasn't happening
as intended. It turns out that the changes to split `buildParameters`
into `productsBuildParameters` and `toolsBuildParameters` weren't fully
propagated to our testing infrastructure.

I also noticed `SWIFT_PM_SUPPORTS_SWIFT_TESTING` wasn't being set
correctly anymore (same root cause) although we've decided to ignore
that flag over in swift-testing anyway (see
swiftlang/swift-testing#376.)

This regression caused build failures in swift-testing (e.g.
[here](https://ci.swift.org/job/pr-swift-testing-macos/663/console))
with the telltale failure signature:

>
/Users/ec2-user/jenkins/workspace/pr-swift-testing-macos/branch-main/swift-testing/.build/x86_64-apple-macosx/debug/swift-testingPackageTests.xctest/Contents/MacOS/swift-testingPackageTests:
/Users/ec2-user/jenkins/workspace/pr-swift-testing-macos/branch-main/swift-testing/.build/x86_64-apple-macosx/debug/swift-testingPackageTests.xctest/Contents/MacOS/swift-testingPackageTests:
cannot execute binary file

Which indicates that it thinks the filename for the swift-testing build
product is the XCTest bundle's executable.

This PR plumbs through the two build parameters arguments to everywhere
in `swift test` and `swift build` that needs them and resolves the
issue.

---------

Co-authored-by: Max Desiatov <m_desiatov@apple.com>
(cherry picked from commit 5a4c024)

# Conflicts:
#	Sources/Commands/SwiftBuildCommand.swift
#	Sources/Commands/SwiftTestCommand.swift
#	Sources/Commands/Utilities/TestingSupport.swift
Library products declared with `.static` or `.dynamic` linkage didn't
have a `-tools` suffix in their file names when built as dependencies of
macros. This change resolves the inconsistency and adds corresponding
build plan tests.

The regression in question broke the compatibility test suite, which had
[an old version of
`swift-power-assert`](https://github.com/kishikawakatsumi/swift-power-assert/blob/a60cb50/Package.swift)
depending on an old version of `swift-syntax` with explicit `.static`
linkage for its products.

rdar://127170225
(cherry picked from commit 8db2401)

# Conflicts:
#	Tests/BuildTests/CrossCompilationBuildPlanTests.swift
@MaxDesiatov MaxDesiatov self-assigned this Jun 6, 2024
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

This test should be cherry-picked in #7641 and initially ended up in this cherry-pick PR by mistake.
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@bnbarham
Copy link
Contributor

bnbarham commented Jun 7, 2024

@swift-ci please test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test windows

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Thank you for bringing this into 6.0!

@rauhul
Copy link
Member

rauhul commented Jun 7, 2024

I think you might also want this change: #7629

@bnbarham
Copy link
Contributor

bnbarham commented Jun 7, 2024

There's still more to add here, this was the initial batch - @xedin has a few to add on top. And yes we should probably take #7629 as well if that broke because of these changes 😅

kateinoigakukun and others added 2 commits June 7, 2024 11:32
The build database should be shared between different triple builds to
re-generate the build manifest correctly.

With the current implementation, the following build command sequence
fails:
```
$ swift build --experimental-swift-sdk wasm32-unknown-wasi
$ swift build

$ swift build --experimental-swift-sdk wasm32-unknown-wasi
$ swift build

$ swift build --experimental-swift-sdk wasm32-unknown-wasi
```

This changes the llbuild build database to be placed under
`.build/build.db` instead of `.build/<product triple>/build.db`.
Also this change splits llbuild target names for each triple to avoid
cache invalidation when switching triple.

`build.db` will be shared across product target triples, and SwiftPM
will keep consistent cache state when switching triples.

(cherry picked from commit e9399c2)
A collection of fixes and improvements related to cross-compilation.

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

- `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`

- `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 added 3 commits June 7, 2024 11:36
#7619)

…on for test products

### Motivation:

Just like macros and plugins, tests products can also be built for the
host depending on their test target(s) destinations (which are inferred
based on dependencies). In order to properly support `--test-product`
option we need to fallback to `host` lookup for test products just like
we do for macros and plugins.

### Modifications:

- Added a fallback lookup to `computeLLBuildTargetName`

### Result:

`--test-product` now supports both `host` and `target` destination
products.
…ts and packages identification into `ModulesGraph`

This is a temporary fix until we can figure out a proper way to
handle situations were all we get is a name of a product or a
target.

Resolves: rdar://129400066
(cherry picked from commit 09b3ed1)
@xedin
Copy link
Contributor

xedin commented Jun 7, 2024

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

@xedin
Copy link
Contributor

xedin commented Jun 7, 2024

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

@xedin
Copy link
Contributor

xedin commented Jun 7, 2024

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

@xedin
Copy link
Contributor

xedin commented Jun 7, 2024

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

@xedin
Copy link
Contributor

xedin commented Jun 9, 2024

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

@xedin
Copy link
Contributor

xedin commented Jun 9, 2024

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

@xedin xedin merged commit bb7f039 into release/6.0 Jun 10, 2024
5 checks passed
@xedin xedin deleted the maxd/6.0-macros-xcompilation branch June 10, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system Changes to interactions with build systems cross-compilation macros Support for macros swift 6.0 Related to Swift 6.0 release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants