fix(ci): preserve target branch history for import profiler baseline generation - #18105
Merged
Merged
Conversation
…generation
Avoid shallow-fetching origin/${TARGET_BRANCH} when TEST_TYPE is import_profile
so git merge-base can successfully find baseline commits for baseline profiling comparison.
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the CI scripts to handle shallow clones and fetch target branches more reliably. The review feedback correctly points out that the unshallow fetch logic in 'ci/run_single_test.sh' should also support forks by attempting to fetch from the 'upstream' remote before falling back to 'origin'.
…er baseline Attempt fetching from upstream before falling back to origin to properly support fork repositories.
parthea
reviewed
Aug 13, 2026
Comment on lines
+57
to
+61
| if [[ "${TEST_TYPE}" == "import_profile" ]]; then | ||
| git fetch upstream "${TARGET_BRANCH}" 2>/dev/null || git fetch origin "${TARGET_BRANCH}" || true | ||
| else | ||
| git fetch upstream "${TARGET_BRANCH}" --depth=200 2>/dev/null || git fetch origin "${TARGET_BRANCH}" --depth=200 || true | ||
| fi |
Contributor
There was a problem hiding this comment.
Suggested change
| if [[ "${TEST_TYPE}" == "import_profile" ]]; then | |
| git fetch upstream "${TARGET_BRANCH}" 2>/dev/null || git fetch origin "${TARGET_BRANCH}" || true | |
| else | |
| git fetch upstream "${TARGET_BRANCH}" --depth=200 2>/dev/null || git fetch origin "${TARGET_BRANCH}" --depth=200 || true | |
| fi | |
| if [[ "${TEST_TYPE}" == "import_profile" ]]; then | |
| git fetch origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" || true | |
| else | |
| git fetch origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" --depth=200 || true | |
| fi |
Since we do not need to support external forks upstream, we can target origin directly. Gemini also suggested this edit
Comment on lines
+141
to
+147
| # Fetch history for the target branch without --depth=1 in case it was shallowly fetched | ||
| if [ -f "$(git rev-parse --git-dir)/shallow" ]; then | ||
| git fetch upstream "${TARGET_BRANCH}" --unshallow 2>/dev/null || \ | ||
| git fetch origin "${TARGET_BRANCH}" --unshallow 2>/dev/null || \ | ||
| git fetch upstream "${TARGET_BRANCH}" 2>/dev/null || \ | ||
| git fetch origin "${TARGET_BRANCH}" 2>/dev/null || true | ||
| fi |
Contributor
There was a problem hiding this comment.
Similar feedback as my other comment
Suggested change
| # Fetch history for the target branch without --depth=1 in case it was shallowly fetched | |
| if [ -f "$(git rev-parse --git-dir)/shallow" ]; then | |
| git fetch upstream "${TARGET_BRANCH}" --unshallow 2>/dev/null || \ | |
| git fetch origin "${TARGET_BRANCH}" --unshallow 2>/dev/null || \ | |
| git fetch upstream "${TARGET_BRANCH}" 2>/dev/null || \ | |
| git fetch origin "${TARGET_BRANCH}" 2>/dev/null || true | |
| fi | |
| # Fetch history for the target branch without --depth=1 in case it was shallowly fetched | |
| if [ -f "$(git rev-parse --git-dir)/shallow" ]; then | |
| git fetch origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" --unshallow 2>/dev/null || \ | |
| git fetch origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" 2>/dev/null || true | |
| fi |
Contributor
Author
There was a problem hiding this comment.
Simplified the unshallow fetch to target origin directly with the explicit refspec.
Contributor
There was a problem hiding this comment.
Since we don't need to support upstream, we can use
# Try origin first, then fallback to HEAD if everything else fails
BASELINE_COMMIT=$(git merge-base HEAD "origin/${TARGET_BRANCH}" 2>/dev/null || \
git rev-parse HEAD)
Contributor
Author
There was a problem hiding this comment.
Done! Updated to check origin/${TARGET_BRANCH} directly and fallback to git rev-parse HEAD.
…ne fetching
Address PR review feedback:
- Explicitly pass ${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH} to git fetch to update remote tracking branch refs.
- Target origin directly and fall back to HEAD if merge-base fails.
parthea
approved these changes
Aug 13, 2026
parthea
marked this pull request as ready for review
August 13, 2026 18:37
hebaalazzeh
enabled auto-merge (squash)
August 13, 2026 18:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Avoid shallow-fetching
origin/${TARGET_BRANCH}whenTEST_TYPEis import_profile so git merge-base can successfully find baseline commits for baseline profiling comparison.