Skip to content

Commit

Permalink
Use explicit depths on fetches when running cherry-picks in CI (#19127)
Browse files Browse the repository at this point in the history
This patch (hopefully!) fixes the cherry-picking script when running in
CI, in a shallow repository.

`actions/checkout`, by default, creates a shallow clone that contains
only the `HEAD` commit of the default branch, doing `git clone --depth=1
...`. This means git operations in that checkout don't have access to
the full history. For doing a cherry-pick, access to history is
critical: to pick `$COMMIT`, the contents of the parent `$COMMIT^` need
to be available, to compute the diff.

This PR resolves the situation (and makes the script faster) by making
the `git fetch`es explicitly about how much history they need:

- fetching the commit to be cherry-picked requires both the commit and
its parent, so `--depth=2` ensures that they're both available, no
matter what the original `HEAD` of the CI check-out was
- creating the cherry-picking branch from the milestone branch only
needs the `HEAD` of that branch, so `--depth=1` is fine, and saves time
by not cloning the full history

Without the explicit arg, `git fetch ... some_ref` has unhelpful
behaviour:

- if `some_ref` already exists in the local checkout (which can occur if
`$COMMIT` the latest commit on the default branch), it fetches too
little, by doing nothing at all
- if `some_ref` doesn't exist in the local checkout, if fetches too
much, by pulling down the whole history

Passing `--depth` to `git fetch` will truncate the log (setting the
"shallow boundary"), so I've conditionalised this to only use the
`--depth` args when actually running in CI. If someone runs it locally,
it'll just do full history fetches, which is fine: an interactive user
will typically have all the history available already anyway.

Fixes #19082 

This was inspired by @thejcannon's learnings in #19125.
  • Loading branch information
huonw authored May 24, 2023
1 parent e536740 commit 99c1669
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions build-support/bin/cherry_pick.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ function fail {
exit "${2-1}"
}

function git_fetch_with_depth {
DEPTH="$1"
REF="$2"

# setting --depth will forcibly truncate history, which is fine in the temporary checkout on CI,
# but would be annoying locally: people will usually already have the full history, so doing a
# full fetch is fine
if [[ $CI = 1 ]]; then
DEPTH_ARGS=("--depth=$DEPTH")
else
DEPTH_ARGS=()
fi

git fetch "${DEPTH_ARGS[@]}" https://github.com/pantsbuild/pants "$REF"
}

if [[ -z $(which gh 2> /dev/null) ]]; then
fail "Requires the GitHub CLI: https://cli.github.com/"
fi
Expand Down Expand Up @@ -51,7 +67,8 @@ COMMIT=$(gh pr view "$PR_NUM" --json mergeCommit --jq '.mergeCommit.oid')
if [[ -z $COMMIT ]]; then
fail "Wasn't able to retrieve merge commit for $PR_NUM."
fi
git fetch https://github.com/pantsbuild/pants "$COMMIT"
# Both the commit and its parent are required, to compute the diff to apply when cherry picking
git_fetch_with_depth 2 "$COMMIT"

TITLE=$(gh pr view "$PR_NUM" --json title --jq '.title')
CATEGORY_LABEL=$(gh pr view "$PR_NUM" --json labels --jq '.labels.[] | select(.name|test("category:.")).name')
Expand All @@ -68,7 +85,9 @@ BODY_FILE=$(mktemp "/tmp/github.cherrypick.$PR_NUM.XXXXXX")
gh pr view "$PR_NUM" --json body --jq '.body' > "$BODY_FILE"

for MILESTONE in $MILESTONES; do
git fetch https://github.com/pantsbuild/pants "$MILESTONE" || continue
# Only the HEAD is required to be able to create the cherry-pick branch, so there's no need to
# spend time cloning the whole history
git_fetch_with_depth 1 "$MILESTONE" || continue

PR_CREATE_CMD=(gh pr create --base "$MILESTONE" --title "$TITLE (Cherry-pick of #$PR_NUM)" --label "$CATEGORY_LABEL" --body-file "$BODY_FILE")
while IFS= read -r REVIEWER; do PR_CREATE_CMD+=(--reviewer "$REVIEWER"); done <<< "$REVIEWERS"
Expand Down

0 comments on commit 99c1669

Please sign in to comment.