From: Stefan Sperling <stsp@stsp.name>
Subject: Re: stop got diff from lying in mixed-commit work trees
To: Mark Jamsek <mark@jamsek.com>
Cc: gameoftrees@openbsd.org
Date: Sun, 24 Nov 2024 14:09:42 +0100

On Sun, Nov 24, 2024 at 04:55:01PM +1100, Mark Jamsek wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > stsp pointed out on IRC that got diff can lie in mixed-commit work trees
> > as only the work tree base commit is emitted at the top of the diff; as
> > such, users could reasonably infer that the same base commit id applies
> > to all files in the diff. This is not true in mixed-commit work trees.
> > 
> > After discussing this with op and stsp, we decided to emit each file's
> > base commit in its diff header, rather than just emit the work tree's
> > base commit once. This requires some regress churn but not nearly as
> > much as anticipated. The bonus is got patch already does the right
> > thing by parsing the new commit header so it requires no changes :)
> > 
> > Apart from the mechanical relocation and addition of the commit header
> > in the changes to the test harness, only one patch test needed changing
> > as we now show the file's true base commit in conflict markers. I've
> > also added a new test to got diff regress to cover this change.
> > 
> > This is what mixed-commit work tree diffs look like (observe the new
> > commit - $basecommit line above each blob - line in all file headers):
> > 
> > [[
> > 	diff /tmp/got-test-diff_mixed_worktree-xQKZURYe9K/wt
> > 	path + /tmp/got-test-diff_mixed_worktree-xQKZURYe9K/wt
> > 	commit - 400880623c67bcd21fc38e5ce5a39382f7249634
> > 	blob - 459809db67d164c738cac58282b2e3426b6aaa33
> > 	file + alpha
> > 	--- alpha
> > 	+++ alpha
> > 	@@ -1 +1 @@
> > 	-'alpha
> > 	+alpha
> > 	commit - b1efe2d25322d0c819fe57ec7b244feb4fd90569
> > 	blob - 65b2df87f7df3aeedef04be96703e55ac19c2cfb
> > 	file + beta
> > 	--- beta
> > 	+++ beta
> > 	@@ -1 +1 @@
> > 	-beta
> > 	+'beta
> > 	commit - 852030ab7b6d55b358767cd71b4be3cdb6ad6154
> > 	blob - /dev/null
> > 	file + new (mode 644)
> > 	--- /dev/null
> > 	+++ new
> > 	@@ -0,0 +1 @@
> > 	+new
> > ]]
> 
> Diff rebased on top of HEAD :)
> 
> If we're going ahead with this change, I'd like it to land before the
> gotwebd test harness to save some churn as this will likely change the
> diff output there too.

Yes, let's try this.  ok