LWN: Comments on "Handling messy pull-request diffstats"

Handling messy pull-request diffstats

geert — Thu, 05 May 2022 08:28:26 +0000

Because Linus does not like receiving pull requests for branches that have been rebased recently, as that invalidates any testing done on the pre-rebased branch.

It's still a good idea to maintain your own separate rebased branch for cross-checking: merging in a branch, and rebasing your own work on top of that branch should yield the exact same tree.

Handling messy pull-request diffstats

jepsis — Thu, 05 May 2022 07:50:34 +0000

Why not just maintain a separate rebased branch and send a pull request from there?

Handling messy pull-request diffstats

k3ninho — Mon, 25 Apr 2022 11:06:51 +0000

The tooling isn't matched to the 'send a patch' workflow of the mailing list -- a merge request wants patch-equivalent state that would apply cleanly like a good happy patch should. That said, it's weird this is a 'trick' given it's how I've come to use git for pull-request mediated development.

Maybe there should be a more-automated way to avoid suggesting things are in conflict for files you didn't edit -- the ones that change due to the repository changing around you -- but that doesn't give a hint as to the importance of changes when you merge a second branch to your non-trunk working set.

K3n.

Handling messy pull-request diffstats

olof — Sat, 23 Apr 2022 21:11:30 +0000

We've done this for the (ARM) SoC trees almost since the start, due to the complex merge history.

The normal flow I used was to remove any rerere cache, merge each branch one by one into current HEAD of Linus' tree, writing down any conflict resolution, and then generate the pull requests with a local script that did the diffstat (and log) manually. Worked well for 10+ years so far, with a few manual hiccups when I diff the wrong merge vs git tag.

Handling messy pull-request diffstats

pm215 — Sat, 23 Apr 2022 16:40:47 +0000

I tend to feel like if this complicated explanation and tricks like "do a merge purely to create a diffstat, then throw it away" are necessary, then there's a problem with either the workflow or with the tooling's ability to support that workflow...

Handling messy pull-request diffstats

mathstuf — Sat, 23 Apr 2022 11:22:52 +0000

`rerere` doesn't use the object store (for better or worse). My issue tends to be messing up a resolution and having to remove it. But it's just easier to nuke them all than to figure out which one it is once you have a collection amassed. If they were stored with the other objects, they'd need some more tooling to clean them out more effectively.

Handling messy pull-request diffstats

NYKevin — Sat, 23 Apr 2022 00:27:39 +0000

> since the work of conflict resolution has been done once and should not be wasted.

Strictly speaking, you don't need to keep the temp branch around even for the purpose of saving yourself work. You can just use git rerere for that. I would tend to assume there is a reasonable way of exporting the rerere metadata so that you can share it with others, but cursory Googling only turns up weird ideas like "make a dedicated branch," and I have no idea why that should be necessary or useful for pulling already-existing data out of .git.

Handling messy pull-request diffstats

jgg — Fri, 22 Apr 2022 22:52:24 +0000

I've been doing this for years.. The other twist Linus shared with me is to actually keep the merged branch in some cases and send it as a 'FYI here is my merge resolution'. I do this when there are conflicts, I'll send the conflict resolution in the pull request and also mention that I have done it in the for-linus-merged tag. Linus says he will pull and compare the for-linus-merged as a sanity check (or perhaps use it sometimes)

This is an example:

https://lore.kernel.org/linux-rdma/20210901234159.GA24219...

(and look at the bottom my script that sorts this out puts a note about the diffstat)

Handling messy pull-request diffstats

pbonzini — Fri, 22 Apr 2022 22:40:29 +0000

> resolving conflicts is the judgement of the maintainer who accepts the merge request:

Resolving conflicts in Linux is done earlier, using common topic branches that are included by multiple maintainers (or that the maintainers includes in both the for-rcN and the for-next branches).

If the conflicts are not caught earlier, Linus absolutely doesn't care if he has conflicts to solve, in fact he wants to see them because it's his way of detecting places that are not collaborating well or that could be messy during the -rc phase.

Looks like material for a second article... :)

Handling messy pull-request diffstats

mathstuf — Fri, 22 Apr 2022 21:43:14 +0000

> And yet, merging master (or whatever upstream) into your development branch seems to be frowned on by Linus and other guardians of git workflow.

Yes, I agree. It puts your target on the non-first parent path which "twists" the DAG a bit in most renderings. What you can do instead is make a new branch off of the target rc and merge your proposal into it and then submit that with conflicts resolved. This makes it look like `c2` in the later build graphs in the article (which is fine).

Handling messy pull-request diffstats

epa — Fri, 22 Apr 2022 21:19:27 +0000

If there are merge conflicts then when the pull request is finally accepted they will have to be resolved too. That suggests submitting this ‘temp’ branch as the branch to be merged, since the work of conflict resolution has been done once and should not be wasted. That also seems cleaner than using one branch just to fake up a diffstat while what’s actually merged is something else.

And yet, merging master (or whatever upstream) into your development branch seems to be frowned on by Linus and other guardians of git workflow. It does make the final graph more tangled. And you could argue that resolving conflicts is the judgement of the maintainer who accepts the merge request: you don’t necessarily trust some contributor to handle conflicts correctly even if you would be prepared to review and merge code they wrote.

So is there a way git could deal with this? Like a ‘merge but not merge’? If there are any conflicts you have to resolve them, and the way you did it is recorded somehow, but the final content remains unchanged. This ‘prepared to merge’ branch could then be submitted as a merge request, and the maintainer could choose to stick with the conflict resolution you did, or ignore it and resolve from scratch.

I guess in some muddled way I am analogizing to ‘prepare to commit’ in database systems, where any data conflicts are checked now, so if the next operation is a commit it is guaranteed to succeed.

Handling messy pull-request diffstats

pbonzini — Fri, 22 Apr 2022 19:46:42 +0000

Mostly because there can be conflicts and in that case it is not possible to do it automatically.

And since all maintainers should be doing a test merge (to check for conflicts, especially the rare semantic ones that cause the build to fail and Linus to shout...) before sending a pull request, you already have the test branch ready and can use it to create the diffstat.

It's true though that, assuming you have the test branch ready, it may make sense for git-request-pull to have an option that uses it.

(FWIW this was one of the most confusing things to learn when I started maintaining KVM...)

Handling messy pull-request diffstats

khim — Fri, 22 Apr 2022 18:39:42 +0000

This sounds like a suspiciously simple algorithm. Why can't Git do that automatically?