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

[Mac Build] Add stdlib build #843

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Steelskin
Copy link
Collaborator

@Steelskin Steelskin commented Nov 7, 2024

  • Adapt stdlib job to build for both Windows and Mac.

@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch 6 times, most recently from e744c95 to 5a2d842 Compare November 12, 2024 03:12
@Steelskin Steelskin force-pushed the fabrice/mac-build-compilers branch 4 times, most recently from f5f0bd7 to cee6e49 Compare November 13, 2024 18:00
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch from 5a2d842 to 6ea346c Compare November 13, 2024 18:01
@Steelskin Steelskin force-pushed the fabrice/mac-build-compilers branch from 93fad08 to d65f3fa Compare December 2, 2024 21:00
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch from 6ea346c to fb11672 Compare December 4, 2024 18:50
@Steelskin Steelskin force-pushed the fabrice/mac-build-compilers branch 2 times, most recently from 34b2bed to 1d9ced6 Compare December 14, 2024 00:22
Base automatically changed from fabrice/mac-build-compilers to main December 16, 2024 16:08
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch 4 times, most recently from 425fe14 to 4643a5a Compare December 19, 2024 22:51
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch from 4643a5a to fe272e4 Compare January 6, 2025 23:12
@Steelskin Steelskin marked this pull request as ready for review January 14, 2025 00:58
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch from fe272e4 to 718402d Compare January 14, 2025 00:58
@Steelskin Steelskin requested a review from compnerd January 14, 2025 00:59
Copy link
Owner

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Any chance we could re-use or try to upstream the cmake caches?

# TODO: Build this on macOS or make an equivalent Mac-only job
if: inputs.build_os == 'Windows'
needs: [compilers, cmark_gfm]
needs: [compilers]
Copy link
Owner

Choose a reason for hiding this comment

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

Compilers will need the CMark GFM to execute, why is the dependency removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed it because it is not a direct dependency of the stdlib job.

- uses: actions/checkout@v4
if: matrix.os == 'Darwin'
with:
path: ${{ github.workspace }}/SourceCache/swift-build
Copy link
Owner

Choose a reason for hiding this comment

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

Can you use the explicit revision to checkout at?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I intend to upstream the cache files before landing this and this will then go away.

.github/workflows/swift-toolchain.yml Outdated Show resolved Hide resolved
$StdlibCxx = "${{ github.workspace }}/BinaryCache/Library/Developer/Toolchains/unknown-Asserts-development.xctoolchain/usr/bin/clang++"
$StdlibFlags = ""
$StdlibCacheFile = "${{ github.workspace }}/SourceCache/swift-build/Runtime-${{ matrix.platform }}-${{ matrix.arch }}.cmake"
}
Copy link
Owner

Choose a reason for hiding this comment

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

Can we push this down into the workflow through the matrix and simplify this somehow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved these to the matrices. It's a bit simpler now.

@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch from 718402d to fb2523c Compare January 15, 2025 23:09
@Steelskin Steelskin requested a review from compnerd January 17, 2025 21:54
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch 2 times, most recently from 22f85d3 to 71128d5 Compare January 27, 2025 21:01
Steelskin added a commit that referenced this pull request Jan 30, 2025
In the stdlib job, the toolchain is now extracted under
`BinaryCache/Library`, rather then under `BuildRoot/Library`. This
allows to change the SDK artifact root path to `BuildRoot/Library`,
simplifying download in subsequent jobs and making room for the macOS
toolchain paths.

Extracted from #843
Steelskin added a commit that referenced this pull request Jan 31, 2025
Currently, in the `stdlib` job, the toolchain is extracted to
`BuildRoot/Library`, and the Swift Standard Library is also installed to
`BuildRoot/Library`. Other jobs typically extract the toolchain to
`BinaryCache/Library`. This changes the extraction path to
`BinaryCache/Library` in the `stdlib` job, bringing it in-line with the
rest of the build.

In addition, this changes the Swift Standard Library artifact root to be
`BuildRoot/Library`, rather than the full path to the platform SDK,
`BuildRoot/Library/Developer/Platforms/${{ matrix.os }}.platform`. This
is in preparation for adapting the job to build for macOS, where the
Swift Standard Library will be installed under
`Library/Toolchains/unknown-Asserts-development.xctoolchain/usr`.

Extracted from #843
* Add cache files for the stdlib build
* Adapt stdlib job to build for both Windows and Mac.
@Steelskin Steelskin force-pushed the fabrice/mac-build-stdlib branch from 71128d5 to 116e75d Compare February 3, 2025 18:00
Comment on lines +471 to +472
"build_cc": "${HostToolchain}/usr/bin/clang-cl.exe",
"build_cxx": "${HostToolchain}/usr/bin/clang-cl.exe",
Copy link
Owner

Choose a reason for hiding this comment

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

These are confusing. The build C/C++ compilers change between phases.

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.

2 participants