-
Notifications
You must be signed in to change notification settings - Fork 86
[CI] Fix ref_llvm and ref_translator in on-push-verification-in-tree task #602
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.
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
[CI] Fix ref_llvm and ref_translator in on-push-verification-in-tree task #602
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the LLVM and translator reference branches in the on-push-verification-in-tree workflow by dynamically parsing the LLVM version from the branch name instead of hardcoding them to "main". The workflow now extracts the LLVM version from branch names following the 'ocl-open-XXX' scheme and constructs the appropriate reference branches for both LLVM and the translator.
Key changes:
- Added dynamic LLVM version parsing from branch names
- Updated
ref_llvmto userelease/<version>.xformat instead of hardcoded "main" - Updated
ref_translatorto usellvm_release_<version>0format instead of hardcoded "main"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| BRANCH="${{ github.base_ref }}" # on: pull_request, otherwise null | ||
| BRANCH=${BRANCH:-${{ github.ref_name }}} # on: push | ||
| LLVM_VERSION_LONG=$(echo $BRANCH | grep -oP '\d+') # Eg. 190 for LLVM 19 |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no error handling if the branch name doesn't match the expected 'ocl-open-XXX' pattern. The grep command will silently fail and LLVM_VERSION_LONG will be empty, causing subsequent commands to produce incorrect results. Add validation to check if the pattern was found and fail with a clear error message if not, e.g., if [ -z \"$LLVM_VERSION_LONG\" ]; then echo \"Error: Could not parse LLVM version from branch name $BRANCH\"; exit 1; fi.
| LLVM_VERSION_LONG=$(echo $BRANCH | grep -oP '\d+') # Eg. 190 for LLVM 19 | |
| LLVM_VERSION_LONG=$(echo $BRANCH | grep -oP '\d+') # Eg. 190 for LLVM 19 | |
| if [ -z "$LLVM_VERSION_LONG" ]; then | |
| echo "Error: Could not parse LLVM version from branch name '$BRANCH'" | |
| exit 1 | |
| fi |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [[ "$LLVM_VERSION_PADDED" =~ ^([0-9]+)0$ ]]; then | ||
| LLVM_VERSION_SHORT="${BASH_REMATCH[1]}" | ||
| else | ||
| echo "[OPENCL-CLANG] Error: Unexpected LLVM version format '$LLVM_VERSION_PADDED' from branch name '$BRANCH'" |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern ^([0-9]+)0$ assumes all LLVM versions end with a zero (e.g., 190 for LLVM 19), but this won't work correctly for versions like 100 (LLVM 10) which would be incorrectly parsed as LLVM 10 instead of LLVM 1. Consider validating that the version is at least two digits or adjusting the logic to handle single-digit major versions differently.
| if [[ "$LLVM_VERSION_PADDED" =~ ^([0-9]+)0$ ]]; then | |
| LLVM_VERSION_SHORT="${BASH_REMATCH[1]}" | |
| else | |
| echo "[OPENCL-CLANG] Error: Unexpected LLVM version format '$LLVM_VERSION_PADDED' from branch name '$BRANCH'" | |
| # Accept only two- or three-digit numbers ending in zero (e.g., 190 for LLVM 19, 100 for LLVM 10) | |
| if [[ "$LLVM_VERSION_PADDED" =~ ^([1-9][0-9]{1,2})0$ ]]; then | |
| LLVM_VERSION_SHORT="${BASH_REMATCH[1]}" | |
| else | |
| echo "[OPENCL-CLANG] Error: Unexpected LLVM version format '$LLVM_VERSION_PADDED' from branch name '$BRANCH'. Expected a two- or three-digit number ending in zero (e.g., 190 for LLVM 19, 100 for LLVM 10)." |
Co-authored-by: Copilot <[email protected]>
ref_llvm should be 19 for ocl-open-190 branch.