-
Notifications
You must be signed in to change notification settings - Fork 29.5k
[flutter_conductor] fix initialref to explicitly include remote name #96481
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
[flutter_conductor] fix initialref to explicitly include remote name #96481
Conversation
| engine = EngineRepository( | ||
| checkouts, | ||
| initialRef: candidateBranch, | ||
| initialRef: 'upstream/$candidateBranch', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we get an error during the checkout if the branch already exists in the fork?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By namespacing by the upstream remote name, git will disregard the fork's branches. This is an unambiguous reference, and will only error if the git cannot fetch the given branch from the upstream remote (either because the branch name that was input was wrong or because it couldn't reach the server).
if you only provide the branch name, git will check all remotes and find which has the branch in question, which was what happened in #96425.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for the explanation.
When cloning the framework and engine repos as part of the
conductor startflow, explicitly checkout the candidate branch from theupstreamremote, in case both the upstream AND mirror repo have the branch and then git cannot disambiguate.Fixes #96425
Remove code to have the conductor update .ci.yaml with the candidate branch name; this is no longer needed after flutter/cocoon#1468
Fixes #96437