Handling messy pull-request diffstats

Posted Apr 22, 2022 19:46 UTC (Fri) by pbonzini (subscriber, #60935)
In reply to: Handling messy pull-request diffstats by khim
Parent article: Handling messy pull-request diffstats

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

Posted Apr 22, 2022 21:19 UTC (Fri) by epa (subscriber, #39769) [Link] (4 responses)

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

Posted Apr 22, 2022 21:43 UTC (Fri) by mathstuf (subscriber, #69389) [Link]

> 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

Posted Apr 22, 2022 22:40 UTC (Fri) by pbonzini (subscriber, #60935) [Link]

> 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

Posted Apr 23, 2022 0:27 UTC (Sat) by NYKevin (subscriber, #129325) [Link] (1 responses)

> 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

Posted Apr 23, 2022 11:22 UTC (Sat) by mathstuf (subscriber, #69389) [Link]

`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.