Store proxy-verifier in .git directory and fix AUTEST_OPTIONS escaping #12664
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the proxy-verifier download mechanism to defer it from CMake configure time to first autest execution, and stores the downloaded binaries in the .git directory to enable sharing across multiple build directories. Additionally, it fixes a CMake argument escaping issue where spaces in AUTEST_OPTIONS were being incorrectly escaped.
- Deferred proxy-verifier download from configure time to runtime
- Relocated proxy-verifier storage from build directory to
.gitdirectory for cross-build sharing - Fixed space escaping in
AUTEST_OPTIONSby converting to list format
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmake/download-proxy-verifier.cmake | New script that handles deferred proxy-verifier download and extraction to .git directory on first autest execution |
| cmake/proxy-verifier.cmake | Removed download logic and updated paths to point to .git directory location |
| tests/CMakeLists.txt | Added proxy-verifier download step to all autest targets and switched to AUTEST_OPTIONS_LIST to fix space escaping |
| tests/autest.sh.in | Added proxy-verifier download check at script startup with error handling |
| CMakeLists.txt | Created AUTEST_OPTIONS_LIST from AUTEST_OPTIONS string using separate_arguments to prevent improper space escaping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bneradt
force-pushed
the
autest_cmake_updates
branch
from
03bc6a4 to
cebe77f
Compare
cmcfarlen
left a comment
cmcfarlen
left a comment
There was a problem hiding this comment.
I can understand downloading to the .git directory to save from multiple builds downloading the same file, but I'm not sure I understand why we wouldn't want to just download it at configure time if the AUTEST build option is specified. This adds quite a bit of complexity to the autest parts of the build (arguably the most complicated part already).
bneradt
changed the title
- Store proxy-verifier in .git directory to share across build directories. - Fix AUTEST_OPTIONS space escaping issue in cmake targets.
bneradt
force-pushed
the
autest_cmake_updates
branch
from
fa5f41c to
fb2a53a
Compare
Sounds good. I reverted that part so that proxy verifier is now fetched via the cmake config step again. Thanks for the review! |
PR apache#12664 changed proxy-verifier storage to be in ${CMAKE_SOURCE_DIR}/.git. Sadly, this fails in git worktrees where .git is a file pointing to the actual git directory. Use git rev-parse --git-common-dir to dynamically detect the correct git directory, which works for both regular repositories and worktrees. This ensures proxy-verifier binaries are stored in a shared location accessible to all worktrees.
PR apache#12664 changed proxy-verifier storage to be in ${CMAKE_SOURCE_DIR}/.git. Sadly, this fails in git worktrees where .git is a file pointing to the actual git directory. Use git rev-parse --git-common-dir to dynamically detect the correct git directory, which works for both regular repositories and worktrees. This ensures proxy-verifier binaries are stored in a shared location accessible to all worktrees.
PR #12664 changed proxy-verifier storage to be in ${CMAKE_SOURCE_DIR}/.git. Sadly, this fails in git worktrees where .git is a file pointing to the actual git directory. Use git rev-parse --git-common-dir to dynamically detect the correct git directory, which works for both regular repositories and worktrees. This ensures proxy-verifier binaries are stored in a shared location accessible to all worktrees.
3 participants
Uh oh!
There was an error while loading. Please reload this page.