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.

Sign up for GitHub

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

Jump to bottom

Use MSVC for the Windows toolchain instead of MinGW #456

Merged
merged 5 commits into from
Jul 23, 2024
Merged

Use MSVC for the Windows toolchain instead of MinGW #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7f4416e
Use MSVC for the Windows toolchain instead of MinGW
alexcrichton Jul 22, 2024
0d91270
Update .github/workflows/main.yml
alexcrichton Jul 22, 2024
d9a25b5
Specify MinSizeRel in `build.sh`
alexcrichton Jul 22, 2024
c4006db
Don't persist ccache stats across runs
alexcrichton Jul 23, 2024
0d894e1
Move zeroing
alexcrichton Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ jobs:
run: sudo apt install ccache
if: runner.os == 'Linux'

- name: Clear ccache statistics
run: ccache --zero-stats

- name: Build and test (macOS)
run: ./ci/build.sh
if: runner.os == 'macOS'
Expand All @@ -133,11 +136,23 @@ jobs:
run: ./ci/docker-build.sh ${{ matrix.artifact }}
if: runner.os == 'Linux'

# Use a shorter build directory than the default on Windows to avoid
# hitting path length and command line length limits. See
# WebAssembly/wasi-libc#514
# Setup the VS Developoer Prompt environment variables to explicitly use
# MSVC to compile LLVM as that avoids extra runtime dependencies
# msys/mingw might bring.
#
# As of 2024-07-22 this sha is the "v1.13.0" tag.
- uses: ilammy/msvc-dev-cmd@0b201ec74fa43914dc39ae48a89fd1d8cb592756
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was debating between the need for this action versus the risk: it looks like ~17k other projects depend on this, it has more than a few contributors, and the action is relatively simple — if it solves the problem then let's go for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I was wrestling with the same. I spend 30 min or so googling around to see if there were other common solutions to this but everything pointed to a github action one way or another so I opted to do that with a specific sha to at least be insulated against changes.

Copy link

Choose a reason for hiding this comment

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

personally, I would fork any dep like this (in case the repo gets deleted, repurposed, ...)

if: runner.os == 'Windows'
- name: Build and test (Windows)
run: |
# Delete a troublesome binary as recommended here
# https://github.com/ilammy/msvc-dev-cmd?tab=readme-ov-file#name-conflicts-with-shell-bash
rm /usr/bin/link
# Use a shorter build directory than the default on Windows to avoid
# hitting path length and command line length limits. See
# WebAssembly/wasi-libc#514. Despite using a different build directory
# though still move the `dist` folder to `build/dist` so the upload
# step below doesn't need a windows-specific hook.
./ci/build.sh C:/wasi-sdk
mkdir build
cp -r C:/wasi-sdk/dist build
Expand Down
1 change: 1 addition & 0 deletions ci/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ fi

cmake -G Ninja -B $build_dir/toolchain -S . \
-DWASI_SDK_BUILD_TOOLCHAIN=ON \
-DCMAKE_BUILD_TYPE=MinSizeRel \
"-DCMAKE_INSTALL_PREFIX=$build_dir/install" \
$WASI_SDK_CI_TOOLCHAIN_CMAKE_ARGS \
"-DLLVM_CMAKE_FLAGS=$WASI_SDK_CI_TOOLCHAIN_LLVM_CMAKE_ARGS"
Expand Down
Loading