xiangfu0 commented on code in PR #19894:
URL: https://github.com/apache/pinot/pull/19894#discussion_r4296507985
##########
.github/workflows/pinot_compatibility_checks.yml:
##########
@@ -32,8 +32,6 @@ on:
- "licenses-binary/**"
- "**.md"
pull_request:
- branches:
- - master
paths-ignore:
Review Comment:
Dropping the `pull_request: branches: [master]` filter here (and in the
other 5 workflows) makes these run on PRs against *any* base branch. That's
needed for this stacked PR, but if it merges to master as-is every future PR
targeting a non-master branch will trigger full CI. Please revert these filter
removals before merge, or confirm it's intended.
##########
compatibility-verifier/checkoutAndBuild.sh:
##########
@@ -55,8 +55,12 @@ function checkOut() {
git pull origin master 1>&2 || exit 1
# Pull the tag list so that we can check out by tag name
git fetch --tags 1>&2 || exit 1
- # Return different error code if old version is not on master
- git checkout $commitHash 1>&2 || exit 22 # invalid argument error code
+ # Unmerged PR baselines are not necessarily reachable from master or a tag.
+ if ! git rev-parse --verify "${commitHash}^{commit}" >/dev/null 2>&1; then
+ git fetch origin "$commitHash" 1>&2 || exit 22
+ commitHash=FETCH_HEAD
+ fi
+ git checkout --detach "$commitHash" 1>&2 || exit 22 # invalid argument error
code
Review Comment:
The rev-parse-verify + fetch fallback + `--detach` is a clean fix for
unmerged baselines. Nice — this avoids the old `git checkout` failing on
commits not reachable from master/tags.
##########
.github/workflows/scripts/.pinot_compatibility_verifier.sh:
##########
@@ -75,7 +75,12 @@ NEW_COMMIT_HASH=$(commitHash HEAD)
if [ ! -z "${NEW_COMMIT}" ]; then
NEW_COMMIT_HASH=$(commitHash "${NEW_COMMIT}")
fi
-OLD_COMMIT_HASH=$(commitHash "${OLD_COMMIT}")
+OLD_COMMIT_HASH=$(commitHash "${OLD_COMMIT}") || OLD_COMMIT_HASH=""
+# A pinned rollback baseline may be an unmerged PR commit, absent from master
and tags.
+if [ -z "${OLD_COMMIT_HASH}" ]; then
Review Comment:
`git fetch origin "${OLD_COMMIT}"` relies on the pinned #19900 recovery SHA
still being reachable from apache/pinot. It's fetchable now, but if #19900 is
rebased/force-pushed before this lands the SHA disappears and CI hard-fails
here. Fine while stacked; just flagging the coupling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
