Use merge-base in find_commit.dart #138033
Conversation
Closes #97595 The prior approach of manually diffing the entire log chain is less efficient, and only found the original branch point ignoring subsequent merges. The limitation forced PR workflows into rebasing and force pushing new history to get the branch point far enough for CI to pass. Use `git merge-base` to find the latest common commit with the main branch. Add an `allowFailure` argument to the `git` utility to use a more specific failure in the case of no shared history when this command will fail with a generic error. Use `^branch` with the `git log` commands to exclude shared history and more easily count the unique commits on each branch.
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
The `hash` field is now unused, drop the abstraction instead of wrap a single time. Count lines directly for the commit counting use case.
This is more direct than log and forcing a single line format, rev-list uses a single line format by default.
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Looks reasonable to me, but @Hixie should review since he's more familiar with the script and the needs of customer_test.
natebosch
added
the
autosubmit
|
The git parts of this are all arcane magic to me, unfortunately. How did you test this? Does it correctly pick the right branch in common cases? What cases would it fail in? Are there cases where this will find a commit that isn't the one that indicates what the branch is up to date to? |
Hixie
left a comment
Hixie
left a comment
There was a problem hiding this comment.
...modulo answers to the questions asked above
Manual testing locally with some repos set up in various states of orphaned or branched and merged commits.
Yes, it seems to pick the correct commit in common cases, and a better commit in merge cases.
I don't think I can imagine any, AFAICT
I cannot think of any reason this would find a worse commit than the prior implementation. If there were some workflows where people where explicitly using merge commits and avoiding the rebase because they specifically didn't want the newer commit chosen then those workflows will be broken. I wouldn't expect such a workflow to exist, but I'm more confident on my knowledge of the git side than the flutter CI infrastructure side of this change. |
3 participants
Closes #97595
The prior approach of manually diffing the entire log chain is less
efficient, and only found the original branch point ignoring subsequent
merges. The limitation forced PR workflows into rebasing and force
pushing new history to get the branch point far enough for CI to pass.
Use
git merge-baseto find the latest common commit with the mainbranch.
Add an
allowFailureargument to thegitutility to use a morespecific failure in the case of no shared history when this command will
fail with a generic error.
Use
^branchwith thegit logcommands to exclude shared history andmore easily count the unique commits on each branch.
Drop the
Commitabstraction. Parse directly to timestamp or line counts.