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

Revert "Revert "Support macros when cross-compiling (#7118)" (#7352)" #7353

Merged
merged 23 commits into from Apr 17, 2024

Conversation

MaxDesiatov
Copy link
Member

@MaxDesiatov MaxDesiatov commented Feb 20, 2024

Reverts #7352.

Modified to build tests for the host triple when any macros are added to test modules directly as dependencies.

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

Currently blocked by CI failures in apple/swift-syntax#2504

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

…vert-7352-maxd/revert-cross-compilation

# Conflicts:
#	Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift
#	Sources/Build/BuildManifest/LLBuildManifestBuilder.swift
#	Sources/Build/BuildPlan/BuildPlan+Test.swift
#	Sources/Build/BuildPlan/BuildPlan.swift
#	Sources/Commands/PackageCommands/PluginCommand.swift
#	Sources/Commands/SwiftTestCommand.swift
#	Sources/Commands/Utilities/PluginDelegate.swift
#	Sources/Commands/Utilities/TestingSupport.swift
#	Sources/SPMTestSupport/MockBuildTestHelper.swift
#	Sources/SPMTestSupport/MockPackageGraphs.swift
#	Sources/SPMTestSupport/PackageGraphTester.swift
#	Tests/BuildTests/BuildPlanTests.swift
#	Tests/PackageGraphTests/ModulesGraphTests.swift
#	Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift
@MaxDesiatov MaxDesiatov force-pushed the revert-7352-maxd/revert-cross-compilation branch from 3b0105b to 14482e0 Compare March 8, 2024 12:13
@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov MaxDesiatov changed the title Revert "Revert "Support macros when cross-compiling (#7118)"" Introduce --experimental-macros-cross-compilation flag Mar 8, 2024
@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov
Copy link
Member Author

@swift-ci test

@MaxDesiatov MaxDesiatov changed the title Introduce --experimental-macros-cross-compilation flag Revert "Revert "Support macros when cross-compiling (#7118)" (#7352)" Mar 25, 2024
@MaxDesiatov
Copy link
Member Author

@swift-ci test

@grynspan
Copy link
Contributor

@swift-ci please test Windows

@MaxDesiatov
Copy link
Member Author

This needs performance testing before it can be merged.

…vert-7352-maxd/revert-cross-compilation

# Conflicts:
#	Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift
#	Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift
#	Sources/Build/BuildManifest/LLBuildManifestBuilder.swift
#	Sources/Build/BuildPlan/BuildPlan+Test.swift
#	Sources/Build/BuildPlan/BuildPlan.swift
#	Sources/Commands/Utilities/SymbolGraphExtract.swift
#	Sources/PackageGraph/ModulesGraph.swift
#	Sources/PackageGraph/Resolution/ResolvedProduct.swift
#	Sources/PackageGraph/Resolution/ResolvedTarget.swift
@MaxDesiatov
Copy link
Member Author

@swift-ci test

MaxDesiatov added a commit that referenced this pull request Apr 17, 2024
Benchmarking modules graph is now generalized with `syntheticModulesGraph` function. It also uncovered a bug in the previous `SyntheticModulesGraph`, which didn't pass generated `TargetDescription`s array to `loadModulesGraph`, which is fixed now.

New `SyntheticModulesGraphWithMacros` calls `syntheticModulesGraph` with `includeMacros: true` argument, which splits all modules in three parts: library modules, macros modules that library modules depend on, and macro dependencies that macros depend on. This allows us to track potential performance regressions in #7353.
@bnbarham bnbarham requested a review from neonichu April 17, 2024 17:44
@MaxDesiatov
Copy link
Member Author

@swift-ci test windows

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) April 17, 2024 17:48
@MaxDesiatov MaxDesiatov merged commit cb3b085 into main Apr 17, 2024
5 checks passed
@MaxDesiatov MaxDesiatov deleted the revert-7352-maxd/revert-cross-compilation branch April 17, 2024 20:27
@rauhul
Copy link
Contributor

rauhul commented Apr 17, 2024

EXCITING :D

MaxDesiatov added a commit that referenced this pull request Apr 18, 2024
Benchmarking modules graph is now generalized with
`syntheticModulesGraph` function. It also uncovered a bug in the
previous `SyntheticModulesGraph`, which didn't pass generated
`TargetDescription`s array to `loadModulesGraph`, which is fixed now.

New `SyntheticModulesGraphWithMacros` calls `syntheticModulesGraph` with
`includeMacros: true` argument, which splits all modules in three parts:
library modules, macros modules that library modules depend on, and
macro dependencies that macros depend on. This allows us to track
potential performance regressions in
#7353.
@finagolfin
Copy link
Contributor

@MaxDesiatov, thanks for working on this, looking forward to using it.

I tried using the official April 22 snapshot for ubi9 x86_64 to cross-compile the swift-syntax macro examples, after patching swift-build to apply your fix from #7493, but I'm seeing errors like this:

> cd swift-syntax/Examples/
> ~/swift-DEVELOPMENT-SNAPSHOT-2024-04-22-a-ubi9/usr/bin/swift build --build-tests --destination ~/swift-android-sdk/android-aarch64.json -Xlinker -rpath -Xlinker \$ORIGIN/swift-trunk-android-aarch64-2024-04-22-24-sdk/usr/lib/swift/android
Building for debugging...
error: Failed opening '/home/fina/swift-syntax/Examples/.build/aarch64-unknown-linux-android28/debug/index/store/v5/units/AddAsyncMacroTests.swift.o-1VZ0WNZLXJYVM': No such file or directory

Swift-foundation also doesn't build with similar errors, but other macro-free packages cross-compile fine. Are you seeing build errors like this when cross-compiling packages that have macros?

grynspan added a commit that referenced this pull request Apr 26, 2024
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 apple/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.
MaxDesiatov added a commit that referenced this pull request Apr 26, 2024
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
apple/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>
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
…ple#7352)" (apple#7353)

Reverts apple#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
Benchmarking modules graph is now generalized with
`syntheticModulesGraph` function. It also uncovered a bug in the
previous `SyntheticModulesGraph`, which didn't pass generated
`TargetDescription`s array to `loadModulesGraph`, which is fixed now.

New `SyntheticModulesGraphWithMacros` calls `syntheticModulesGraph` with
`includeMacros: true` argument, which splits all modules in three parts:
library modules, macros modules that library modules depend on, and
macro dependencies that macros depend on. This allows us to track
potential performance regressions in
apple#7353.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
After apple#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
apple/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>
grynspan added a commit that referenced this pull request May 15, 2024
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
apple/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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cross-compiling packages which contain macros doesn't work
5 participants