Skip to content
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

Auto cherry-picker always errors with merge conflicts #19082

Closed
benjyw opened this issue May 21, 2023 · 1 comment · Fixed by #19125 or #19127
Closed

Auto cherry-picker always errors with merge conflicts #19082

benjyw opened this issue May 21, 2023 · 1 comment · Fixed by #19125 or #19127
Assignees
Labels

Comments

@benjyw
Copy link
Contributor

benjyw commented May 21, 2023

It failed on #19079 with a zillion merge conflicts:

https://github.com/pantsbuild/pants/actions/runs/5039766268/jobs/9038117307

But actually is a super-trivial merge that a manual run of the cherry-pick.sh script
did with no trouble.

@thejcannon thejcannon changed the title Issue with auto cherry-picker Auto cherry-picker always errors with merge conflicts May 22, 2023
stuhood pushed a commit that referenced this issue May 24, 2023
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.
stuhood pushed a commit that referenced this issue May 24, 2023
What an unexciting end to this saga.
And I don't even know _why_ this works.

But alas, `act` has shown me this does, so... Fixes #19082
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants