On Mon, Oct 21, 2019 at 2:47 AM Ingo Molnar <mi...@kernel.org> wrote:
>
> I think at least once I ran into that and sent you a 'slightly wrong'
> diffstat - and maybe there's also been a few cases where you noticed
> diffstats that didn't match your merge result, double checked it yourself
> and didn't complain about it because you knew that this is a "git
> request-pull" artifact?
Right. If I see a diffstat that doesn't match, I just look at what a
non-merged diffstat would have looked like, and if that matches I know
what happened.
There are other reasons why diffstats won't match, of course. Like me
just having merged part of the same commits from another source (or
multiple trees applying the same patch). So it's not _just_ due to
multiple merge bases that the mis-match can happen.
> Most of the time I notice it like Will did because the diffstat is
> obviously weird and it's good to check pull requests a second (and a
> third :-) time as well, but it's possible to have relatively small
> distances between the merge bases where the diffstat doesn't look
> 'obviously' bogus and mistakes can slip through.
Yup.
> Anyway, a small Git feature request: it would be super useful if "git
> request-pull" output was a bit more dependable and at least warned about
> this and didn't include what is, from the viewpoint of the person doing
> the merge, a bogus diffstat.
Well, warning for it would be fairly simple. Giving the "right" result
isn't simple, though, since the merge might need manual fixup to
succeed.
The warning you can check yourself: just do
git merge-base --all upstream mybranch
and if you get more than one result, you know you are in the situation
where a diff from the merge base might not work (it *might* work, but
probably won't).
You can play around with it yourself, of course. Look at the
git-request-puill.sh script, it says something like this:
merge_base=$(git merge-base $baserev $headrev) ||
die "fatal: No commits in common between $base and $head"
and you could add something like
all_merge_bases="$(git merge-base --all $baserev $headrev)"
and then add a warning if "all_merge_bases" doesn't match "merge_base".
Linus
