Swift 6 Support #86
Swift 6 Support #86
Conversation
tothambrus11
force-pushed
the
swift-601
branch
2 times, most recently
from
ba0a314 to
338e248
Compare
.devcontainer/Dockerfile
Outdated
| # Get a recent cmake (https://www.kitware.com//cmake-python-wheels/) | ||
| RUN if $(/usr/bin/which cmake) ; then apt purge --auto-remove cmake ; fi | ||
| RUN pip3 install --upgrade cmake | ||
| # Remove system cmake if installed |
There was a problem hiding this comment.
Why are we replacing a simple method of upgrading cmake with a more complex one?
The original was an officially supported method as referenced in the URL.
There was a problem hiding this comment.
A month ago when I started this, pip was complaining that installing stuff without a virtual envirnment was dangerous and I should use pipx. It seems, since then they reverted this warning, and now I could install cmake globally without an issue.
I replaced the cmake installation with an even simpler one, just using apt-get. Since the Swift 6.1 image is using Ubuntu 24, it is a quite recent version (28), so I think it should be okay. Should I revert back to the pip method? (that may come with some extra runtime cost)
There was a problem hiding this comment.
Hylo's CMakeLists.txt says: cmake_minimum_required(VERSION 3.30). I think we should use Swifty-LLVM as a testbed for Hylo CI/configuration, and try to replicate as much as possible in both places. So I think pip is the better choice.
| "userUid": "1000", | ||
| "userGid": "1000", | ||
| "userUid": "1001", | ||
| "userGid": "1001", |
There was a problem hiding this comment.
Good idea. I know why you did this; it's usually prudent to add a comment for subtle things like this. // userid 1001 becayse https://something… could be sufficient.
.github/workflows/test.yml
Outdated
| @@ -168,7 +170,7 @@ jobs: | |||
| -DCMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} | |||
| -DBUILD_TESTING=YES | |||
| -DLLVM_DIR=${{ github.workspace }}/${{ env.llvm_package_basename }}/lib/cmake/llvm | |||
| ${{ runner.os == 'macOS' && '-D CMAKE_Swift_COMPILER=swiftc' || '' }} | |||
| ${{ matrix.os == 'macos-13' && '-DCMAKE_Swift_COMPILER=swiftc -DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path)' || '' }} | |||
There was a problem hiding this comment.
The use runner was correct and intentional. We don't care which version of macOS is being used and anyway, the matrix is a per-project configuration thing.
There was a problem hiding this comment.
Oh true, I didn't catch this when pasting something from the other project. I'll fix this.
There was a problem hiding this comment.
If by the other project you mean Hylo, we probably ought to make that one match this one. I wish there was a simpler way to share GH actions configuration.
There was a problem hiding this comment.
If you're feeling ambitious, explore this
.github/workflows/test.yml
Outdated
| @@ -197,15 +199,14 @@ jobs: | |||
| -DCMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} | |||
| -DBUILD_TESTING=YES | |||
| -DLLVM_DIR=${{ github.workspace }}/${{ env.llvm_package_basename }}/lib/cmake/llvm | |||
| ${{ runner.os == 'macOS' && '-D CMAKE_Swift_COMPILER=swiftc' || '' }} | |||
| ${{ matrix.os == 'macos-13' && '-DCMAKE_Swift_COMPILER=swiftc -DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path)' || '' }} | |||
Upgraded Swift 6 by
Name.llvmandAddressSpace.default) to constants.String(contentsOfFile:)to explicitly pass character encoding and praying that it's utf8. (I checked the llvm.pc, it was indeed utf8, but idk if this is a guarantee)CI fixes:
DCMAKE_OSX_SYSROOTvariable:${{ matrix.os == 'macos-13' && '-DCMAKE_Swift_COMPILER=swiftc -DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path)' || '' }}