Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix: backporting#62788

Merged
craigfurman merged 1 commit into
mainfrom
fix-backport
May 20, 2024
Merged

fix: backporting#62788
craigfurman merged 1 commit into
mainfrom
fix-backport

Conversation

@craigfurman

Copy link
Copy Markdown
Contributor

Revert line that tries to fetch a specific remote branch that has never actually been pushed, back to fetching from the default remote.

I'm not 100% sure this fixes the underlying problem in https://linear.app/sourcegraph/issue/REL-38/having-trouble-back-porting-panw-pr-to-540-need-help, so let's execute the test plan pre-merge.

Test plan

There don't appear to be many automated tests for backporting, which is fair enough as this is quite fiddly. We can manually test this by running a backport command pre-merge, and seeing if it fixes https://linear.app/sourcegraph/issue/REL-38/having-trouble-back-porting-panw-pr-to-540-need-help:

go run ./dev/sg backport <args>

Comment thread dev/sg/internal/backport/backport.go Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code ran git fetch -a, but I am not sure if the -a was load-bearing. From man git-fetch:

       -a, --append
           Append ref names and object names of fetched refs to the existing contents of .git/FETCH_HEAD.
           Without this option old data in .git/FETCH_HEAD will be overwritten.

I can't think of why this would matter, so I left it out. I wonder if the original code was trying to run git fetch --all. I thought -a was short for --all until reading that man page today.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent here is to update the latest changes so we can have access to the commit created by the PR. We could just run a fetch origin <PR branch> to fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this line works, I think. git fetch fetches all branches from the default remote (conventionally "origin")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Let's test then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, holding off while we wait for @RXminuS's approval to use his PR as a test case, on the slack thread.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change works! We do still get an error though when running go run ./dev/sg backport -p 62720 -r 5.4.0:

Unable to cherry-pick merge commit: "6a7666c42cf349a6b702b8c72c9cc2c4e8421ad7". This might be the result of a merge conflict. Manually run `git cherry-pick 6a7666c42cf349a6b702b8c72c9cc2c4e8421ad7` and fix on your machine.: exit status 1

But that's not an issue with the tool, @RXminuS will need to resolve conflicts. Let's merge this 🚀

Revert line that tries to fetch a specific remote branch that has never
actually been pushed, back to fetching from the default remote.
@craigfurman craigfurman marked this pull request as ready for review May 20, 2024 10:56
@craigfurman craigfurman enabled auto-merge (squash) May 20, 2024 10:58
@craigfurman craigfurman merged commit b414332 into main May 20, 2024
@craigfurman craigfurman deleted the fix-backport branch May 20, 2024 11:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants