feature suggestion: improve rerere

feature suggestion: improve rerere

[Uwe Kleine-König]

Hello,

I think the following suggestion is sound. And it might even be easy to
implement but I don't know how rerere works, so I might be wrong here.

When applying a patch it's normal to hit a conflict. For me this just
happend:

	$ git diff
	diff --cc flash_otp_write.c
	index f360a3e,648e042..0000000
	--- a/flash_otp_write.c
	+++ b/flash_otp_write.c
	@@@ -15,8 -15,23 +15,29 @@@
	  
	  #include <mtd/mtd-user.h>
	  
	++<<<<<<< ours
	 +#include "common.h"
	 +
	++||||||| base
	++=======
	+ ssize_t xread(int fd, void *buf, size_t count)
	+ {
	+ 	ssize_t ret, done = 0;
	+ 
	+ retry:
	+ 	ret = read(fd, buf + done, count - done);
	+ 	if (ret < 0)
	+ 		return ret;
	+ 
	+ 	done += ret;
	+ 
	+ 	if (ret == 0 /* EOF */ || done == count)
	+ 		return done;
	+ 	else
	+ 		goto retry;
	+ }
	+ 
	++>>>>>>> theirs
	  int main(int argc,char *argv[])
	  {
		int fd, val, ret, size, wrote, len;

easy to resolve to:

	$ git diff
	diff --cc flash_otp_write.c
	index f360a3e,648e042..0000000
	--- a/flash_otp_write.c
	+++ b/flash_otp_write.c
	@@@ -15,8 -15,23 +15,25 @@@
	  
	  #include <mtd/mtd-user.h>
	  
	 +#include "common.h"
	 +
	+ ssize_t xread(int fd, void *buf, size_t count)
	+ {
	+ 	ssize_t ret, done = 0;
	+ 
	+ retry:
	+ 	ret = read(fd, buf + done, count - done);
	+ 	if (ret < 0)
	+ 		return ret;
	+ 
	+ 	done += ret;
	+ 
	+ 	if (ret == 0 /* EOF */ || done == count)
	+ 		return done;
	+ 	else
	+ 		goto retry;
	+ }
	+ 
	  int main(int argc,char *argv[])
	  {
		int fd, val, ret, size, wrote, len;

Now if I shuffle patches and put the new patch before the conflicting
patch, I have to resolve first:

	$ git diff
	diff --cc flash_otp_write.c
	index d407ebb,31b963e..0000000
	--- a/flash_otp_write.c
	+++ b/flash_otp_write.c
	@@@ -15,6 -15,25 +15,31 @@@
	  
	  #include <mtd/mtd-user.h>
	  
	++<<<<<<< ours
	++||||||| base
	++#include "common.h"
	++
	++=======
	+ #include "common.h"
	+ 
	+ ssize_t xread(int fd, void *buf, size_t count)
	+ {
	+ 	ssize_t ret, done = 0;
	+ 
	+ retry:
	+ 	ret = read(fd, buf + done, count - done);
	+ 	if (ret < 0)
	+ 		return ret;
	+ 
	+ 	done += ret;
	+ 
	+ 	if (ret == 0 /* EOF */ || done == count)
	+ 		return done;
	+ 	else
	+ 		goto retry;
	+ }
	+ 
	++>>>>>>> theirs
	  int main(int argc,char *argv[])
	  {
		int fd, val, ret, size, wrote, len;

which is resolved to just adding the function and dropping the #include.
But then readding the 2nd patch it conflicts again:

	$ git diff
	diff --cc flash_otp_write.c
	index 648e042,f360a3e..0000000
	--- a/flash_otp_write.c
	+++ b/flash_otp_write.c
	@@@ -15,23 -15,8 +15,29 @@@
	  
	  #include <mtd/mtd-user.h>
	  
	++<<<<<<< ours
	 +ssize_t xread(int fd, void *buf, size_t count)
	 +{
	 +	ssize_t ret, done = 0;
	 +
	 +retry:
	 +	ret = read(fd, buf + done, count - done);
	 +	if (ret < 0)
	 +		return ret;
	 +
	 +	done += ret;
	 +
	 +	if (ret == 0 /* EOF */ || done == count)
	 +		return done;
	 +	else
	 +		goto retry;
	 +}
	 +
	++||||||| base
	++=======
	+ #include "common.h"
	+ 
	++>>>>>>> theirs
	  int main(int argc,char *argv[])
	  {
		int fd, val, ret, size, wrote, len;

This is the same conflict as the first one, just with ours and theirs
exchanged. So my suggestion is to make rerere use the resolution
recorded for the first conflict here.

Sounds sensible?

Best regards
Uwe

PS: I'm using Debian's git 1.8.2~rc2-1 and hope I didn't miss this
feature already implemented while looking through v1.8.2..junio/next.

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: feature suggestion: improve rerere

[Johannes Sixt]

Am 3/6/2013 11:16, schrieb Uwe Kleine-König:
> 	++<<<<<<< ours
> 	 +ssize_t xread(int fd, void *buf, size_t count)
> 	 +{
> 	 +	ssize_t ret, done = 0;
> 	 +
> 	 +retry:
> 	 +	ret = read(fd, buf + done, count - done);
> 	 +	if (ret < 0)
> 	 +		return ret;
> 	 +
> 	 +	done += ret;
> 	 +
> 	 +	if (ret == 0 /* EOF */ || done == count)
> 	 +		return done;
> 	 +	else
> 	 +		goto retry;
> 	 +}
> 	 +
> 	++||||||| base
> 	++=======
> 	+ #include "common.h"
> 	+ 
> 	++>>>>>>> theirs
> 	  int main(int argc,char *argv[])
> 	  {
> 		int fd, val, ret, size, wrote, len;
> 
> This is the same conflict as the first one, just with ours and theirs
> exchanged. So my suggestion is to make rerere use the resolution
> recorded for the first conflict here.
> 
> Sounds sensible?

Of course, and rerere already does it. But only when you use git's default
conflict markers rather than diff3 style markers that have this extra
||||| line.

-- Hannes

Re: feature suggestion: improve rerere

[Uwe Kleine-König]

On Wed, Mar 06, 2013 at 02:24:18PM +0100, Johannes Sixt wrote:
> Am 3/6/2013 11:16, schrieb Uwe Kleine-König:
> > 	++<<<<<<< ours
> > 	 +ssize_t xread(int fd, void *buf, size_t count)
> > 	 +{
> > 	 +	ssize_t ret, done = 0;
> > 	 +
> > 	 +retry:
> > 	 +	ret = read(fd, buf + done, count - done);
> > 	 +	if (ret < 0)
> > 	 +		return ret;
> > 	 +
> > 	 +	done += ret;
> > 	 +
> > 	 +	if (ret == 0 /* EOF */ || done == count)
> > 	 +		return done;
> > 	 +	else
> > 	 +		goto retry;
> > 	 +}
> > 	 +
> > 	++||||||| base
> > 	++=======
> > 	+ #include "common.h"
> > 	+ 
> > 	++>>>>>>> theirs
> > 	  int main(int argc,char *argv[])
> > 	  {
> > 		int fd, val, ret, size, wrote, len;
> > 
> > This is the same conflict as the first one, just with ours and theirs
> > exchanged. So my suggestion is to make rerere use the resolution
> > recorded for the first conflict here.
> > 
> > Sounds sensible?
> 
> Of course, and rerere already does it. But only when you use git's default
> conflict markers rather than diff3 style markers that have this extra
> ||||| line.
I only did git checkout --conflict=diff3 after the merge conflict
happend. So I cannot confirm that git already does it.

So here is a reproduction receipe:

	git clone git://git.infradead.org/mtd-utils.git
	cd mtd-utils
	git checkout ca39eb1
	wget -O patch1 http://article.gmane.org/gmane.linux.drivers.mtd/45779/raw
	wget -O patch2 http://article.gmane.org/gmane.linux.drivers.mtd/45591/raw
	for p in patch1 patch2; do perl -p -i -e 'print "From tralala Mon Sep 17 00:00:00 2001\n" if $. == 1' $p; done
	git am patch1
	git am -3 patch2 # first merge conflict
	perl -n -i -e 's/=======//; print unless /^[<>]{7} /;' flash_otp_write.c # resolve
	git add flash_otp_write.c
	git am --resolved
	git rebase -i ca39eb1 # swap order of the two patches

results in

	$ git ls-files -u
	100644 f360a3e025deaf7acfb7b20c9fad90f498ae4430 1	flash_otp_write.c
	100644 d407ebbf400e630dc00ee004ecb44be8af51b25d 2	flash_otp_write.c
	100644 31b963e2d6cf0016ca542529886e1ee71a22664e 3	flash_otp_write.c

and resolving yields:

	$ git ls-files -s flash_otp_write.c
	100644 648e0422d21c0ffa7621f82b86c02a065e488293 0	flash_otp_write.c

Then
	git rebase --continue 

gives the 2nd rebase conflict:

	$ git ls-files -u
	100644 d407ebbf400e630dc00ee004ecb44be8af51b25d 1	flash_otp_write.c
	100644 648e0422d21c0ffa7621f82b86c02a065e488293 2	flash_otp_write.c
	100644 f360a3e025deaf7acfb7b20c9fad90f498ae4430 3	flash_otp_write.c

Now knowing from the previous resolution that with base=f360a3e0
(= origin + patch1) merging

	d407ebbf (= origin) and
	31b963e2 (= origin + patch1 + patch2)

gives 648e0422 (origin + patch2),
git could know that with base=d407ebbf (origin) merging 648e0422 (origin
+ patch1) and f360a3e0 (origin + patch1) gives 31b963e2 (origin + patch1
+ patch2) again.

And git doesn't prepare 31b963e2 in flash_otp_write.c for me.

@Johannes, do you have some non-standard setting, or can you reproduce?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: feature suggestion: improve rerere

[Johannes Sixt]

Am 3/6/2013 15:42, schrieb Uwe Kleine-König:
> On Wed, Mar 06, 2013 at 02:24:18PM +0100, Johannes Sixt wrote:
>> Am 3/6/2013 11:16, schrieb Uwe Kleine-König:
>>> 	++<<<<<<< ours
>>> 	 +ssize_t xread(int fd, void *buf, size_t count)
>>> 	 +{
>>> 	 +	ssize_t ret, done = 0;
>>> 	 +
>>> 	 +retry:
>>> 	 +	ret = read(fd, buf + done, count - done);
>>> 	 +	if (ret < 0)
>>> 	 +		return ret;
>>> 	 +
>>> 	 +	done += ret;
>>> 	 +
>>> 	 +	if (ret == 0 /* EOF */ || done == count)
>>> 	 +		return done;
>>> 	 +	else
>>> 	 +		goto retry;
>>> 	 +}
>>> 	 +
>>> 	++||||||| base
>>> 	++=======
>>> 	+ #include "common.h"
>>> 	+ 
>>> 	++>>>>>>> theirs
>>> 	  int main(int argc,char *argv[])
>>> 	  {
>>> 		int fd, val, ret, size, wrote, len;
>>>
>>> This is the same conflict as the first one, just with ours and theirs
>>> exchanged. So my suggestion is to make rerere use the resolution
>>> recorded for the first conflict here.
>>>
>>> Sounds sensible?
>>
>> Of course, and rerere already does it. But only when you use git's default
>> conflict markers rather than diff3 style markers that have this extra
>> ||||| line.
> I only did git checkout --conflict=diff3 after the merge conflict
> happend. So I cannot confirm that git already does it.
> 
> So here is a reproduction receipe:
> 
> 	git clone git://git.infradead.org/mtd-utils.git
> 	cd mtd-utils
> 	git checkout ca39eb1
> 	wget -O patch1 http://article.gmane.org/gmane.linux.drivers.mtd/45779/raw
> 	wget -O patch2 http://article.gmane.org/gmane.linux.drivers.mtd/45591/raw

At least patch2 has incorret index information; it does not apply with am
-3. I generated a working version with format-patch after applying it
directly on top of ca39eb1.

> 	for p in patch1 patch2; do perl -p -i -e 'print "From tralala Mon Sep 17 00:00:00 2001\n" if $. == 1' $p; done

Do you have rerere enabled at this point?

	git config rerere.enabled true

> 	git am patch1
> 	git am -3 patch2 # first merge conflict
> 	perl -n -i -e 's/=======//; print unless /^[<>]{7} /;' flash_otp_write.c # resolve
> 	git add flash_otp_write.c
> 	git am --resolved
> 	git rebase -i ca39eb1 # swap order of the two patches
> 
> results in
> 
> 	$ git ls-files -u
> 	100644 f360a3e025deaf7acfb7b20c9fad90f498ae4430 1	flash_otp_write.c
> 	100644 d407ebbf400e630dc00ee004ecb44be8af51b25d 2	flash_otp_write.c
> 	100644 31b963e2d6cf0016ca542529886e1ee71a22664e 3	flash_otp_write.c

Same here.

> 
> and resolving yields:
> 
> 	$ git ls-files -s flash_otp_write.c
> 	100644 648e0422d21c0ffa7621f82b86c02a065e488293 0	flash_otp_write.c

OK.

> 
> Then
> 	git rebase --continue 
> 
> gives the 2nd rebase conflict:
> 
> 	$ git ls-files -u
> 	100644 d407ebbf400e630dc00ee004ecb44be8af51b25d 1	flash_otp_write.c
> 	100644 648e0422d21c0ffa7621f82b86c02a065e488293 2	flash_otp_write.c
> 	100644 f360a3e025deaf7acfb7b20c9fad90f498ae4430 3	flash_otp_write.c

Same here.

> 
> Now knowing from the previous resolution that with base=f360a3e0
> (= origin + patch1) merging
> 
> 	d407ebbf (= origin) and
> 	31b963e2 (= origin + patch1 + patch2)
> 
> gives 648e0422 (origin + patch2),
> git could know that with base=d407ebbf (origin) merging 648e0422 (origin
> + patch1) and f360a3e0 (origin + patch1) gives 31b963e2 (origin + patch1
> + patch2) again.

For me, it works as expected, i.e., I get the conflict resolved by rerere.

> 
> And git doesn't prepare 31b963e2 in flash_otp_write.c for me.
> 
> @Johannes, do you have some non-standard setting, or can you reproduce?

Perhaps:

$ git config --global -l | grep rerere
rerere.enabled=true

-- Hannes