Skip to content

utils: extract Build-SDK helper #80157

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

Merged
merged 1 commit into from
Apr 7, 2025
Merged

utils: extract Build-SDK helper #80157

merged 1 commit into from
Apr 7, 2025

Conversation

compnerd
Copy link
Member

This creates a helper for building the SDK for a given OS/Architecture. The building of the SDK should be uniform and this ensures that we can maintain that uniformity.

This also highlights any structural changes that are being adjusted manually. The desire is to bring this to zero by gaining control over the install rules.

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

@swift-ci please test Windows platform

@compnerd
Copy link
Member Author

CC: @jeffdav

@xavgru12
Copy link

I was excited until I read build.ps1.. nothing for me on Ubuntu

@compnerd
Copy link
Member Author

I was excited until I read build.ps1.. nothing for me on Ubuntu

Shouldn't be too terrible to port build.ps1 to run on Ubuntu. It will require just going through and adjusting the host expectation of Windows.

@compnerd
Copy link
Member Author

@swift-ci please test Windows platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

@swift-ci please test Windows platform

@xavgru12
Copy link

Why are you maintaining a windows only file? I figured this script‘s features would be more or less the same as build-script.

Android {
$SDKSettings.SupportedTargets.Android.LLVMTargetVendor = "unknown"
$SDKSettings.SupportedTargets.Android.LLVMTargetSys = "linux"
$SDKSettings.SupportedTargets.Android.LLVMTargetTripleEnvironment = "android${AndroidAPILevel}"
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, SwiftBuild has android and windows as lowercase under "SupportedTargets".

Environment also should be androideabi for armv7, but we have no way to vary that per architecture right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. I thought it was androideabi as well. However, when we recently updated the install rules to use the output of swiftc -print-target-info, we end up with armv7-unknown-linux-android. I'm happy to change this to androideabi for armv7 though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happy to change this to androideabi for armv7 though.

You can't at present, because the current schema only allows specifying the triple components per logical platform not per architecture. We'd need to extend the SDKSettings schema (the "authoritative" implementation lives in Swift Build) to allow that.

@compnerd
Copy link
Member Author

compnerd commented Mar 29, 2025

Why are you maintaining a windows only file? I figured this script‘s features would be more or less the same as build-script.

build-script is not portable; it does not support the necessary setup that Windows requires (integration with VS), support for cross-compilation, doesn't do the optimized build tricks that we do for minimizing the build, nor does it handle paths properly (path limits are a serious concern on Windows).

This creates a helper for building the SDK for a given OS/Architecture.
The building of the SDK should be uniform and this ensures that we can
maintain that uniformity.

This also highlights any structural changes that are being adjusted
manually. The desire is to bring this to zero by gaining control over
the install rules.
@compnerd
Copy link
Member Author

@swift-ci please test Windows platform

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-installer-scripts#407

@swift-ci please test Windows platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test macOS platform

@bkhouri bkhouri self-requested a review March 31, 2025 19:22
@compnerd
Copy link
Member Author

compnerd commented Apr 1, 2025

@swift-ci please smoke test macOS platform

@bkhouri
Copy link
Contributor

bkhouri commented Apr 4, 2025

According to #80356 (comment), this might fix #80356. Is there an ETA on when this will land?

@compnerd
Copy link
Member Author

compnerd commented Apr 7, 2025

@bkhouri this needs @shahmishal or @etcwilde to approve swiftlang/swift-installer-scripts#407, but this is ready to go otherwise.

@compnerd compnerd merged commit a20b568 into swiftlang:main Apr 7, 2025
3 checks passed
@compnerd compnerd deleted the helper branch April 7, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants