-
Notifications
You must be signed in to change notification settings - Fork 52
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
[compilers] Use a pre-configure setup step #856
base: main
Are you sure you want to change the base?
Conversation
fde4ec2
to
3243bb7
Compare
3243bb7
to
824eea7
Compare
4d4d026
to
62027d9
Compare
* Add a preliminary step to export the context for the compiler configuration step. This makes it easier to debug the compiler configuration step by seeing the full command line invocation on the GHA page.
62027d9
to
b5f43a6
Compare
PTAL. This is how the compilers configure step shows up as before this change:
And after:
|
$CACHE="${{ github.workspace }}/SourceCache/swift/cmake/caches/Windows-aarch64.cmake" | ||
|
||
# FIXME(compnerd) re-enable runtimes after we sort out compiler-rt | ||
(Get-Content ${{ github.workspace }}/SourceCache/swift/cmake/caches/Windows-aarch64.cmake).Replace(' runtimes', '') | Set-Content ${{ github.workspace }}/SourceCache/swift/cmake/caches/Windows-aarch64.cmake | ||
} else { | ||
# FIXME(steelskin) Setting `CMAKE_SYSTEM_NAME and `CMAKE_SYSTEM_PROCESSOR` breaks the compiler-rt build | ||
$ExtraFlags = "" |
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.
I had the impression that this PR doesn't intend to change the behavior, but does this line 998 change the behavior?
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.
I slightly changed the way ExtraFlags
is populated, compared to the EXTRA_FLAGS
array. On line 983, ExtraFlags
is now initialized to ${{ matrix.extra_flags }}
. Before, it was initialized to an empty array and then ${{ matrix.extra_flags }}
was added in all cases, except for Windows x64. I am now adding ${{ matrix.extra_flags }}
unconditionally and then resetting it for Windows x64. In the long term, we'll want to fix the issue in CMake and remove this line, which will also make this script simpler.
While this is a change, it is functionally identical as the final command line invocation is the same as before.
Write-Output "cc=${CC}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "cxx=${CXX}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "swiftc=${SWIFTC}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "cxxflags=${CxxFlags}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "swiftflags=${SwiftFlags}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "extra_flags=${ExtraFlags}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "cache=${CACHE}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "clang_location=${CLANG_LOCATION}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "libpython_path=${LIBPYTHON_PATH}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "python_include_dir=${PYTHON_INCLUDE_DIR}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "python_binary=${PYTHON_BINARY}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append | ||
Write-Output "sdkroot=${SDKROOT}" | Out-File -FilePath $env:GITHUB_OUTPUT -Encoding utf8 -Append |
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.
Can you change this to a heredoc instead?
configuration step. This makes it easier to debug the compiler
configuration step by seeing the full command line invocation on the
GHA page.