ci: modernize and fix GitHub Actions workflow#2717
ci: modernize and fix GitHub Actions workflow#2717kevmoo wants to merge 5 commits intoWebAssembly:mainfrom
Conversation
- Bump actions/checkout from v1/v3 to v4 to avoid Node.js deprecation warnings. - Bump actions/setup-python from v1 to v5 for the same reason. - Add `-y` to all `apt-get install` and `apt-get update` commands to prevent process hangs. - Modernize CMake configuration by using `-S . -B out` instead of manually creating directories and using `working-directory: out`. - Fix the Windows build step missing the `-G Ninja` flag, ensuring it actually uses the Ninja binary installed in the previous step (issue present since initial GH actions migration in a08df5c). - Fix minor typo ('Temporatily' -> 'Temporarily'). - Unified some install commands
| - uses: actions/checkout@v1 | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
Do we need fetch-depth: 0 for some reason? IIRC this is slower since it fetches the whole history
There was a problem hiding this comment.
The fetch-depth: 0 setting was necessary because the lint job relies on the
./scripts/clang-format-diff.sh script (called on line 27 of build.yml).
By default, the actions/checkout step only performs a shallow fetch of the repository (fetching just a single commit, i.e., depth=1).
However, the clang-format-diff.sh script uses Git commands that need to inspect your repository's history to figure out what changed so it only formats modified code:
# Finds the common ancestor commit
MERGE_BASE=$(git merge-base $BRANCH HEAD)
# Runs the formatter only on the diff against the merge base
git clang-format $MERGE_BASE -q --diff -- src/Without the full Git history fetched (fetch-depth: 0), git merge-base and git clang-format wouldn't be able to find the common ancestor commit locally, causing the CI lint step to fail.
There was a problem hiding this comment.
I totally wrote that 😛
There was a problem hiding this comment.
Ah yes, of course, this is only the lint step, that makes sense.
Why did the v1 version not need this? Did it fetch all the history by default maybe?
There was a problem hiding this comment.
v1 did the full checkout. v6 is being efficient. But too efficient in this case.
| - name: mkdir | ||
| run: mkdir -p out | ||
| - uses: ilammy/msvc-dev-cmd@v1 | ||
| if: matrix.os == 'windows-latest' |
There was a problem hiding this comment.
What is this new external dependency doing?
There was a problem hiding this comment.
That step (the ilammy/msvc-dev-cmd@v1 action) activates the Microsoft Visual C++ (MSVC) Developer Command Prompt environment on the windows-latest GitHub Actions runner.
It was introduced in the very same commit (ea995cd92) you asked about previously, which fixed Windows CI failures.
Here's why it was needed:
The WABT build uses the Ninja build system across all platforms (cmake -G Ninja). On Windows runners, GitHub Actions has multiple compilers installed (both MSVC and MinGW/GCC). If you run CMake with Ninja without explicitly setting up the MSVC environment, CMake might default to using the MinGW gcc compiler instead of Microsoft's cl.exe, or it might fail entirely to find the C/C++ compiler.
By calling - uses: ilammy/msvc-dev-cmd@v1, it injects all the environment variables (like PATH, LIB, and INCLUDE) required for CMake and Ninja to locate and use cl.exe + link.exe correctly for the native Windows build.
There was a problem hiding this comment.
We could also revert the bits here that move the Windows build to use Ninja, but it seemed nice to have everything using ninja 🤷
There was a problem hiding this comment.
I'm happy to add more notes so there are bread crumbs if that's helpful – in the actual source
There was a problem hiding this comment.
Can you split out the windows->ninja parts? We can discuss those more in a separate PR. The other changes here seem trivially good.
|
Yep! They changed the default to be more efficient.
…On Sun, Mar 15, 2026, 10:16 Sam Clegg ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .github/workflows/build.yml
<#2717 (comment)>:
> with:
python-version: '3.x'
- - uses: ***@***.***
+ - uses: ***@***.***
+ with:
+ fetch-depth: 0
Ah yes, of course, this is only the lint step, that makes sense.
Why did the v1 version not need this? Did it fetch all the history by
default maybe?
—
Reply to this email directly, view it on GitHub
<#2717 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEFCWAMYJLCBYY6C2SM2T4Q3QN3AVCNFSM6AAAAACWSDMTS6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSNJQGQYTANRUGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
2 participants
-yto allapt-get installandapt-get updatecommands to prevent process hangs.-S . -B outinstead of manually creating directories and usingworking-directory: out.-G Ninjaflag, ensuring it actually uses the Ninja binary installed in the previous step (issue present since initial GH actions migration in a08df5c).