"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: show original content in merge conflicts
To:
gameoftrees@openbsd.org
Date:
Fri, 11 Oct 2019 10:31:15 +0200

Download raw body.

Thread 
On Wed, Oct 09, 2019 at 12:25:23PM +0200, Stefan Sperling wrote:
> This diff makes conflict markers include the original group of lines
> which were changed in conflicting ways in two derived versions of the
> same file.

I have not yet heard any feedback regarding this change.
Is nobody interested in this? Should I drop this change or go ahead?

This is an updated diff which fixes a bug where the wrong original
content was shown for changes beyond the first line in a file (duh!)

I have adjusted the test_diff_shows_conflict test to check for this.

diff refs/heads/master refs/heads/3way-conflicts
blob - e7951e14f2548e423ecb820f33291e316d6819ee
blob + 5f130a29a313bcbbe7dccd873f075a8ef95cb3ba
--- lib/diff3.c
+++ lib/diff3.c
@@ -117,6 +117,7 @@ struct off_range {
 struct diff {
 	struct line_range old;
 	struct line_range new;
+	struct off_range oldo;
 	struct off_range newo;
 };
 
@@ -129,7 +130,8 @@ struct diff3_state {
 	/*
 	 * "de" is used to gather editing scripts.  These are later spewed out
 	 * in reverse order.  Its first element must be all zero, the "new"
-	 * component of "de" contains line positions and byte positions.
+	 * component of "de" contains line positions, and "oldo" and "newo"
+	 * components contain byte positions.
 	 * Array overlap indicates which sections in "de" correspond to lines
 	 * that are different in all three files.
 	 */
@@ -152,7 +154,7 @@ struct diff3_state {
 };
 
 
-static const struct got_error *duplicate(int *, struct line_range *,
+static const struct got_error *duplicate(int *, int, struct line_range *,
     struct line_range *, struct diff3_state *);
 static const struct got_error *edit(struct diff *, int, int *,
     struct diff3_state *);
@@ -758,7 +760,7 @@ merge(size_t m1, size_t m2, struct diff3_state *d3s)
 		}
 		/* stuff peculiar to third file or different in all */
 		if (d1->new.from == d2->new.from && d1->new.to == d2->new.to) {
-			err = duplicate(&dpl, &d1->old, &d2->old, d3s);
+			err = duplicate(&dpl, j, &d1->old, &d2->old, d3s);
 			if (err)
 				return err;
 
@@ -851,9 +853,12 @@ skip(size_t *nskipped, int i, int from, struct diff3_s
 /*
  * Set *dpl to 1 or 0 according as the old range (in file 1) contains exactly
  * the same data as the new range (in file 2).
+ *
+ * If this change could overlap, remember start/end offsets in file 2 so we
+ * can write out the original lines of text if a merge conflict occurs.
  */
 static const struct got_error *
-duplicate(int *dpl, struct line_range *r1, struct line_range *r2,
+duplicate(int *dpl, int j, struct line_range *r1, struct line_range *r2,
     struct diff3_state *d3s)
 {
 	const struct got_error *err = NULL;
@@ -861,6 +866,7 @@ duplicate(int *dpl, struct line_range *r1, struct line
 	int nchar;
 	int nline;
 	size_t nskipped;
+	off_t off;
 
 	*dpl = 0;
 
@@ -870,9 +876,16 @@ duplicate(int *dpl, struct line_range *r1, struct line
 	err = skip(&nskipped, 0, r1->from, d3s);
 	if (err)
 		return err;
+
 	err = skip(&nskipped, 1, r2->from, d3s);
 	if (err)
 		return err;
+
+	off = ftello(d3s->fp[1]);
+	if (off == -1)
+		return got_error_from_errno("ftello");
+	d3s->de[j + 1].oldo.from = off; /* original lines start here */
+
 	nchar = 0;
 	for (nline = 0; nline < r1->to - r1->from; nline++) {
 		do {
@@ -883,8 +896,20 @@ duplicate(int *dpl, struct line_range *r1, struct line
 			if (d == EOF)
 				return got_ferror(d3s->fp[1], GOT_ERR_EOF);
 			nchar++;
-			if (c != d)
-				return repos(nchar, d3s);
+			if (c != d) {
+				err = repos(nchar, d3s);
+				if (err)
+					return err;
+				err = skip(&nskipped, 1, r2->to, d3s);
+				if (err)
+					return err;
+				off = ftello(d3s->fp[1]);
+				if (off == -1)
+					return got_error_from_errno("ftello");
+				/* original lines end here */
+				d3s->de[j + 1].oldo.to = off;
+				return NULL;
+			}
 		} while (c != '\n');
 	}
 	err = repos(nchar, d3s);
@@ -952,7 +977,26 @@ edscript(int n, struct diff3_state *d3s)
 				return err;
 		} else {
 			err = diff_output(d3s->diffbuf, "%da\n%s\n",
-			    d3s->de[n].old.to -1, GOT_DIFF_CONFLICT_MARKER_SEP);
+			    d3s->de[n].old.to -1,
+			    GOT_DIFF_CONFLICT_MARKER_ORIG);
+			if (err)
+				return err;
+			if (fseeko(d3s->fp[1], d3s->de[n].oldo.from, SEEK_SET)
+			    == -1)
+				return got_error_from_errno("fseeko");
+			k = (size_t)(d3s->de[n].oldo.to - d3s->de[n].oldo.from);
+			for (; k > 0; k -= len) {
+				len = k > BUFSIZ ? BUFSIZ : k;
+				if (fread(block, 1, len, d3s->fp[1]) != len)
+					return got_ferror(d3s->fp[1],
+					    GOT_ERR_IO);
+				block[len] = '\0';
+				err = diff_output(d3s->diffbuf, "%s", block);
+				if (err)
+					return err;
+			}
+			err = diff_output(d3s->diffbuf, "%s\n",
+			    GOT_DIFF_CONFLICT_MARKER_SEP);
 			if (err)
 				return err;
 		}
blob - f3ddb3fb288bda4b679437664c0f0f403351969d
blob + 0d74b1bbdb78b195fcfd6b16c5042af2a781aa5f
--- lib/got_lib_diff.h
+++ lib/got_lib_diff.h
@@ -135,6 +135,7 @@ struct got_diff_args {
 };
 
 #define GOT_DIFF_CONFLICT_MARKER_BEGIN	"<<<<<<<"
+#define GOT_DIFF_CONFLICT_MARKER_ORIG	"|||||||"
 #define GOT_DIFF_CONFLICT_MARKER_SEP	"======="
 #define GOT_DIFF_CONFLICT_MARKER_END	">>>>>>>"
 
blob - d5083e047e683d5211b8352a65aeb41fea0facb2
blob + 0454b55efa3ced59507448a4ccee518a3a29f74e
--- regress/cmdline/diff.sh
+++ regress/cmdline/diff.sh
@@ -111,11 +111,13 @@ function test_diff_shows_conflict {
 	fi
 
 	sed -i 's/2/22/' $testroot/repo/numbers
+	sed -i 's/8/33/' $testroot/repo/numbers
 	git_commit $testroot/repo -m "modified line 2"
 	local head_rev=`git_show_head $testroot/repo`
 
-	# modify line 2 in a conflicting way
+	# modify lines 2 and 8 in conflicting ways
 	sed -i 's/2/77/' $testroot/wt/numbers
+	sed -i 's/8/88/' $testroot/wt/numbers
 
 	echo "C  numbers" > $testroot/stdout.expected
 	echo -n "Updated to commit $head_rev" >> $testroot/stdout.expected
@@ -138,16 +140,27 @@ function test_diff_shows_conflict {
 	echo 'file + numbers' >> $testroot/stdout.expected
 	echo '--- numbers' >> $testroot/stdout.expected
 	echo '+++ numbers' >> $testroot/stdout.expected
-	echo '@@ -1,5 +1,9 @@' >> $testroot/stdout.expected
+	echo '@@ -1,8 +1,20 @@' >> $testroot/stdout.expected
 	echo ' 1' >> $testroot/stdout.expected
 	echo "+<<<<<<< commit $head_rev" >> $testroot/stdout.expected
 	echo ' 22' >> $testroot/stdout.expected
+	echo '+|||||||' >> $testroot/stdout.expected
+	echo '+2' >> $testroot/stdout.expected
 	echo '+=======' >> $testroot/stdout.expected
 	echo '+77' >> $testroot/stdout.expected
 	echo '+>>>>>>> numbers' >> $testroot/stdout.expected
 	echo ' 3' >> $testroot/stdout.expected
 	echo ' 4' >> $testroot/stdout.expected
 	echo ' 5' >> $testroot/stdout.expected
+	echo ' 6' >> $testroot/stdout.expected
+	echo ' 7' >> $testroot/stdout.expected
+	echo "+<<<<<<< commit $head_rev" >> $testroot/stdout.expected
+	echo ' 33' >> $testroot/stdout.expected
+	echo '+|||||||' >> $testroot/stdout.expected
+	echo '+8' >> $testroot/stdout.expected
+	echo '+=======' >> $testroot/stdout.expected
+	echo '+88' >> $testroot/stdout.expected
+	echo '+>>>>>>> numbers' >> $testroot/stdout.expected
 
 	(cd $testroot/wt && got diff > $testroot/stdout)
 
blob - 0d64a54e87019fb4decc9861f1e2c2da67aba905
blob + c94727a0b05067fb3a4deadd1017588265245e68
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -218,6 +218,8 @@ function test_rebase_continue {
 
 	echo "<<<<<<< commit $orig_commit1" > $testroot/content.expected
 	echo "modified alpha on branch" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
+	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha on master" >> $testroot/content.expected
 	echo '>>>>>>> alpha' >> $testroot/content.expected
@@ -347,6 +349,8 @@ function test_rebase_abort {
 
 	echo "<<<<<<< commit $orig_commit1" > $testroot/content.expected
 	echo "modified alpha on branch" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
+	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha on master" >> $testroot/content.expected
 	echo '>>>>>>> alpha' >> $testroot/content.expected
@@ -455,6 +459,8 @@ function test_rebase_no_op_change {
 
 	echo "<<<<<<< commit $orig_commit1" > $testroot/content.expected
 	echo "modified alpha on branch" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
+	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha on master" >> $testroot/content.expected
 	echo '>>>>>>> alpha' >> $testroot/content.expected
@@ -560,6 +566,8 @@ function test_rebase_in_progress {
 
 	echo "<<<<<<< commit $orig_commit1" > $testroot/content.expected
 	echo "modified alpha on branch" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
+	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha on master" >> $testroot/content.expected
 	echo '>>>>>>> alpha' >> $testroot/content.expected
blob - 1cdd63392f3e865270de6ffdb9b10cb96f773a41
blob + a5e79ffbed7c31ef1cd484016eb9c214c3a673d5
--- regress/cmdline/update.sh
+++ regress/cmdline/update.sh
@@ -680,6 +680,8 @@ function test_update_merges_file_edits {
 	git_show_head $testroot/repo >> $testroot/content.expected
 	echo >> $testroot/content.expected
 	echo "modified alpha" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
+	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha, too" >> $testroot/content.expected
 	echo '>>>>>>> alpha' >> $testroot/content.expected
@@ -883,6 +885,7 @@ function test_update_conflict_wt_add_vs_repo_add {
 	git_show_head $testroot/repo >> $testroot/content.expected
 	echo >> $testroot/content.expected
 	echo "new" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "also new" >> $testroot/content.expected
 	echo '>>>>>>> gamma/new' >> $testroot/content.expected
@@ -1364,6 +1367,8 @@ function test_update_to_another_branch {
 	git_show_head $testroot/repo >> $testroot/content.expected
 	echo >> $testroot/content.expected
 	echo "modified alpha on new branch" >> $testroot/content.expected
+	echo "|||||||" >> $testroot/content.expected
+	echo "alpha" >> $testroot/content.expected
 	echo "=======" >> $testroot/content.expected
 	echo "modified alpha in work tree" >> $testroot/content.expected
 	echo '>>>>>>> alpha' >> $testroot/content.expected