fix(crc32c): escape the version dot and anchor the pyenv check regex - #17555
fix(crc32c): escape the version dot and anchor the pyenv check regex#17555parthea merged 6 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request improves Python version matching in macOS build and publish scripts by escaping dots and using anchors/word boundaries in grep patterns. The feedback suggests refactoring the bash conditional checks from [ -z "$(pipeline | grep ...)" ] to a more idiomatic and efficient if ! pipeline | grep -q ...; then pattern to avoid unnecessary subshells and string comparisons.
Sorry, something went wrong.
|
Re: the failing cover test: When google-crc32c gets a change to core code, it triggers as a diff in CI/CD and attempts to run unittests in the unit job in workflows/unittest.yml. However... in the crc32c noxfile.py it uses session.skip for unit tests because they are not implemented. This means NO coverage results are produced. Note running coverage is entirely managed in the unittest.yml file. OPTIONS under consideration:
|
Sorry, something went wrong.
…nt matching sub-versions like 3.13.14
…ure as it skips unit tests
…n packages skip tests
An alternate approach was taken and merged into main that solves the majority of the issues but still had a minor problem with google-crc32c. this PR addresses that last edge case. |
Sorry, something went wrong.
Problem
The OS X python wheel build script (build_python_wheel.sh) checks if a Python version is installed in pyenv before building:
When $version is 3.14, this expands to grep 3.14. Because the dot (.) is not escaped, it is treated as a wildcard and will also match the substring 3.13.14 (which contains 3.14 at the end). This results in a false positive, causing the script to skip compiling Python 3.14. The script then fails when executing pyenv shell 3.14 with a "version not installed" error.
Solution
Escape the version dots in bash: ${version//./\.}.
Anchor the grep match with line start (^) and word boundaries (\b) to ensure it only matches the exact version line (e.g. ^3\.14\b instead of matching 3.13.14).
Applied this fix to both build_python_wheel.sh and publish_python_wheel.sh.
Blocks: #17642