Repository navigation
[test] Check LLVM and Binaryen versions when rebaselining codesize tests - #28053
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces revision verification for LLVM and Binaryen during rebaselining in test_codesize.py by extracting dependency-fetching logic into a reusable get_deps_revisions function in sync_deps.py. Feedback suggests refactoring the verification logic from __init__ to setUpClass to prevent issues during test discovery, wrapping the network call in a try-except block for offline resilience, and making the revision comparison more robust to support short SHAs.
2ad29f9 to
b1f0e13
Compare
|
Maybe I'm missing something, but this is just the official version, correct? But we update between versions and changes happen all the time. The commit that happens to update the official version is not special? |
Its not really an "official" version but just the current blessed version that all codesize tests are reported against. If you try to rebaseline against a different version then CI will for sure fail. This early error means that contributors won't get frustrated when trying to match the code size expectations with the CI machines (Which always pin to this version) |
|
Any commit that updates the official/blessed version will also need to be accompanied by the corresponding codesize changes. |
|
But the CI machines pin to a git revision, while this checks |
Here we are grabbing the |
When running `codesize` tests with `--rebaseline`, check that the active LLVM and Binaryen revisions match the revisions specified by `test/emsdk_version.txt` via `emscripten-releases`.
b1f0e13 to
710c160
Compare
Previously `git describe` was only executed when configuring CMake, so switching git revisions and running an incremental build left `PROJECT_VERSION` (and `--version` output) stale unless CMake happened to re-run. This change (or something like this) is needed to make the version check in emscripten-core/emscripten#28053 work. Right now `--version` lies which is not great. Inspired by LLVM's `GenerateVersionFromVCS.cmake` / `VCSRevision.h` setup, move `config.h` generation into `scripts/gen-version.cmake` and wire it up via `add_custom_command` depending on `.git/logs/HEAD`. Because `configure_file` only updates `config.h`'s timestamp when its contents actually change, and only `src/support/command-line.cpp` includes `config.h`, incremental rebuilds only recompile that single file when `HEAD` changes. `config.h` is also still generated at configure time so tools like `clang-tidy` that run before a build can find it.
When running
codesizetests with--rebaseline, check that the active LLVM and Binaryen revisions match the revisions specified bytest/emsdk_version.txtviaemscripten-releases.This should help prevent confusion when folks get a different results locally to the CI