-
Notifications
You must be signed in to change notification settings - Fork 35
build: add support for installing the image #124
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
Conversation
This replicates much of the logic from the new runtimes build to support installing the Swift interface and the runtime components. Additionally, we are now able to generate the targets configuration to allow dependencies between projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMake Error at cmake/modules/PlatformInfo.cmake:30 (string):
string sub-command JSON member 'target platform' not found.
Call Stack (most recent call first):
CMakeLists.txt:25 (include)
CMake Error at cmake/modules/PlatformInfo.cmake:38 (string):
string sub-command JSON member 'target arch' not found.
Call Stack (most recent call first):
CMakeLists.txt:25 (include)
okay, so seems like we are going to need a backwards compatibility path for this as we need to target 6.1 |
761b56c
to
8cb2c8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chatted with @compnerd and I think we don't need the CMake build to be compatible with 6.1, since it's only used for the build of the toolchain itself. So switching the CMake CI to nightly seems like the least friction thing to do here.
Update to the nightly snapshots to make the CMake build easier to maintain.
8cb2c8c
to
4a9024d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need swift interfaces or are the binary swift modules sufficient? Where are we using the CMake build with a stable ABI? I've only used it as a static archive out of the build directory.
@@ -40,7 +40,7 @@ jobs: | |||
cmake_build: | |||
name: CMake Build | |||
runs-on: ubuntu-latest | |||
container: swift:6.1-noble | |||
container: swiftlang/swift:nightly-noble |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this bump intentional? I had used 6.1 to keep it in sync with the required version in Package.swift
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need the CMake build to be compatible with 6.1, since it's only used for the build of the toolchain itself
and it would require more effort to make this compatible with 6.1; no reason to add support for something we will never use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, this was for the platform
and arch
subdirectories wasn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since it's only used for the build of the toolchain itself.
It isn't though. I have projects that are non-SwiftPM that use this build today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could set Subprocess_INSTALL_NESTED_SUBDIR
to OFF
when using something earlier than 6.2 and the arch and platform subdirs aren't set. Then we wouldn't need to request that info from the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows will want that, so we will need to set that to true for CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since it's only used for the build of the toolchain itself.
It isn't though. I have projects that are non-SwiftPM that use this build today.
Ah, that's unfortunate...
This replicates much of the logic from the new runtimes build to support installing the Swift interface and the runtime components. Additionally, we are now able to generate the targets configuration to allow dependencies between projects.