[PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Johannes Schindelin]

If dir2 already exists, git-mv should move dir1 _into_dir2/.
Noticed by Jon Smirl.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	Jon, this was your problem, right?

 t/t7001-mv.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index d78c56a..322eaad 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -59,4 +59,19 @@ test_expect_success \
      git-diff-tree -r -M --name-status  HEAD^ HEAD | \
      grep -E "^R100.+path0/README.+path2/README"'
 
+test_expect_success \
+    'moving whole subdirectory into subdirectory' \
+    'git-mv path2 path1'
+
+test_expect_success \
+    'commiting the change' \
+    'git-commit -m dir-move -a'
+
+test_expect_success \
+    'checking the commit' \
+    'git-diff-tree -r -M --name-status  HEAD^ HEAD | \
+     grep -E "^R100.+path2/COPYING.+path1/path2/COPYING" &&
+     git-diff-tree -r -M --name-status  HEAD^ HEAD | \
+     grep -E "^R100.+path2/README.+path1/path2/README"'
+
 test_done
-- 
1.4.2.rc2.g96f2-dirty

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Jon Smirl]

An exact test case:

git clone git foo
git clone git foo1
cd foo
mkdir zzz
git mv gitweb zzz
cg diff >patch
cd ../foo1
cg patch <../foo/patch

This patch won't apply because zzz does not exist in foo1

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Junio C Hamano]

"Jon Smirl" <jonsmirl@gmail.com> writes:

> An exact test case:
>
> git clone git foo
> git clone git foo1
> cd foo
> mkdir zzz
> git mv gitweb zzz
> cg diff >patch
> cd ../foo1
> cg patch <../foo/patch
>
> This patch won't apply because zzz does not exist in foo1

A short and sweet reproduction recipe is essential for a problem
report like this, and I appreciate it very much.  "git mv" in
"master" should do the right thing and I see Johannes fixing (or
fixed) the problem of "git mv" failing in his version.

We should correctly handle cases that fit your general
description (the test t/t4112-apply-renames.sh has a file in
"klibc/arch/x86_64" which is renamed and copied to two different
locations under "include/arch").  The above does not reproduce
for me if I used "git diff HEAD >patch" in place of "cg diff" (I
cannot make cg behave on my machine).

$ mkdir zzz
$ ~/git-master/bin/git-mv gitweb zzz/
$ LANG=C LC_ALL=C find gitweb zzz -type f -print
find: gitweb: No such file or directory
zzz/gitweb/README
zzz/gitweb/gitweb.cgi
zzz/gitweb/gitweb.css
zzz/gitweb/test/M??rchen
zzz/gitweb/test/file with spaces
zzz/gitweb/test/file+plus+sign
$ git diff HEAD >patch
$ git checkout -f HEAD
$ LANG=C LC_ALL=C find gitweb zzz -type f -print
gitweb/README
gitweb/gitweb.cgi
gitweb/gitweb.css
gitweb/test/M??rchen
gitweb/test/file with spaces
gitweb/test/file+plus+sign
find: zzz: No such file or directory
$ git apply --index <patch
$ LANG=C LC_ALL=C find gitweb zzz -type f -print
zzz/gitweb/README
zzz/gitweb/gitweb.cgi
zzz/gitweb/gitweb.css
zzz/gitweb/test/M??rchen
zzz/gitweb/test/file with spaces
zzz/gitweb/test/file+plus+sign

A couple of weeks ago test t/t4114-apply-typechange.sh was added
by Eric Wong, and it caught cases where git-apply was assuming
that leading directories of a new file already exists or was
complaining when you create file "foo" and remove a file
"foo/bar" (that is, you used to have directory at "foo").  The
problems that test found were all fixed as far as I recall.

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Jon Smirl]

On 7/26/06, Junio C Hamano <junkio@cox.net> wrote:
> We should correctly handle cases that fit your general
> description (the test t/t4112-apply-renames.sh has a file in
> "klibc/arch/x86_64" which is renamed and copied to two different
> locations under "include/arch").  The above does not reproduce
> for me if I used "git diff HEAD >patch" in place of "cg diff" (I
> cannot make cg behave on my machine).

cg diff makes a patch this looks like this

diff --git a/gitweb/README b/zzz/gitweb/README
similarity index 100%
rename from gitweb/README
rename to zzz/gitweb/README
diff --git a/gitweb/gitweb.cgi b/zzz/gitweb/gitweb.cgi
similarity index 100%
rename from gitweb/gitweb.cgi
rename to zzz/gitweb/gitweb.cgi
diff --git a/gitweb/gitweb.css b/zzz/gitweb/gitweb.css
similarity index 100%
rename from gitweb/gitweb.css
rename to zzz/gitweb/gitweb.css
diff --git "a/gitweb/test/M\303\244rchen" "b/zzz/gitweb/test/M\303\244rchen"
similarity index 100%
rename from "gitweb/test/M\303\244rchen"
rename to "zzz/gitweb/test/M\303\244rchen"
diff --git a/gitweb/test/file with spaces b/zzz/gitweb/test/file with spaces
similarity index 100%
rename from gitweb/test/file with spaces
rename to zzz/gitweb/test/file with spaces
diff --git a/gitweb/test/file+plus+sign b/zzz/gitweb/test/file+plus+sign
similarity index 100%
rename from gitweb/test/file+plus+sign
rename to zzz/gitweb/test/file+plus+sign

git diff HEAD makes a much longer patch that deltas out the existing
files and delta in the new file.

It's applying patches in the extended git format that fails.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Josef Weidendorfer]

On Wednesday 26 July 2006 19:41, Johannes Schindelin wrote:
> 
> If dir2 already exists, git-mv should move dir1 _into_dir2/.
> Noticed by Jon Smirl.

Thanks for adding this test.
BTW, the original PERL script passes it quite fine.

I just looked at Jon's problem. Doesn't seem to be related to
git-mv or git at all, but more a cogito problem.
I have some cogito-0.18pre installed, and cg-patch is patching
the stuff all itself, not using git for this. Pasky?

Doing the same with git, i.e. in a rep with existing dir/

 mkdir new
 git mv dir new
 git diff --cached -M -C >patch
 git reset --hard
 git apply <patch

However, "git status" shows the "new/" directory totally
untracked afterwards. Is this expected?

Josef

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Johannes Schindelin]

Hi,

On Wed, 26 Jul 2006, Jon Smirl wrote:

> An exact test case:
> 
> git clone git foo
> git clone git foo1
> cd foo
> mkdir zzz
> git mv gitweb zzz
> cg diff >patch
> cd ../foo1
> cg patch <../foo/patch
> 
> This patch won't apply because zzz does not exist in foo1

Okay, I got it wrong, then. Thanks for the clarification!

Ciao,
Dscho

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Junio C Hamano]

"Jon Smirl" <jonsmirl@gmail.com> writes:

> git diff HEAD makes a much longer patch that deltas out the existing
> files and delta in the new file.
>
> It's applying patches in the extended git format that fails.

Thanks.

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Junio C Hamano]

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> On Wednesday 26 July 2006 19:41, Johannes Schindelin wrote:
>> 
>> If dir2 already exists, git-mv should move dir1 _into_dir2/.
>> Noticed by Jon Smirl.
>
> Thanks for adding this test.
> BTW, the original PERL script passes it quite fine.
>
> I just looked at Jon's problem. Doesn't seem to be related to
> git-mv or git at all, but more a cogito problem.
> I have some cogito-0.18pre installed, and cg-patch is patching
> the stuff all itself, not using git for this. Pasky?

"git apply" seems to grok this just fine.

> Doing the same with git, i.e. in a rep with existing dir/
>
>  mkdir new
>  git mv dir new
>  git diff --cached -M -C >patch
>  git reset --hard
>  git apply <patch
>
> However, "git status" shows the "new/" directory totally
> untracked afterwards. Is this expected?

Running "git apply --index <patch" I see the "renamed: " in
there and zzz (or your "new") is tracked.

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> "Jon Smirl" <jonsmirl@gmail.com> writes:
>
>> git diff HEAD makes a much longer patch that deltas out the existing
>> files and delta in the new file.
>>
>> It's applying patches in the extended git format that fails.
>
> Thanks.

... and it turns out that "git apply" (with or without --index)
groks it just fine.  I suspect the fix is quite recent (not in
v1.4.2-rc1 but in v1.4.2-rc2).

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Jon Smirl]

On 7/26/06, Junio C Hamano <junkio@cox.net> wrote:
> Junio C Hamano <junkio@cox.net> writes:
>
> > "Jon Smirl" <jonsmirl@gmail.com> writes:
> >
> >> git diff HEAD makes a much longer patch that deltas out the existing
> >> files and delta in the new file.
> >>
> >> It's applying patches in the extended git format that fails.
> >
> > Thanks.
>
> ... and it turns out that "git apply" (with or without --index)
> groks it just fine.  I suspect the fix is quite recent (not in
> v1.4.2-rc1 but in v1.4.2-rc2).

I can confirm that git apply is handling this correctly with code from
the current git tree. cogito is failing this case.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Petr Baudis]

Dear diary, on Wed, Jul 26, 2006 at 08:39:24PM CEST, I got a letter
where Josef Weidendorfer <Josef.Weidendorfer@gmx.de> said that...
> On Wednesday 26 July 2006 19:41, Johannes Schindelin wrote:
> > 
> > If dir2 already exists, git-mv should move dir1 _into_dir2/.
> > Noticed by Jon Smirl.
> 
> Thanks for adding this test.
> BTW, the original PERL script passes it quite fine.
> 
> I just looked at Jon's problem. Doesn't seem to be related to
> git-mv or git at all, but more a cogito problem.
> I have some cogito-0.18pre installed, and cg-patch is patching
> the stuff all itself, not using git for this. Pasky?

Unfortunately, git-apply is still quite unusable for Cogito. It can do
fuzzy merging now, but here's some random list of more issues I still
have with it (I'm leaving for some two weeks or so of holiday soon so I
won't have to fix them soon personally; it'd be nice if someone did,
though ;) :

  (i) No git-apply -R - well, it seems to me that I revert patches all
the time, don't you?

  (ii) I'd like git-apply to be as verbose as patch is, that is list
the files it touches as it goes

  (iii) There's no reject handling besides "panic" right now - it should
be able to create .rej files so that the user can fix things up

  (iv) I need git-apply to add/remove to/from index new/gone files,
while at the same time...

  (v) I want to allow applying of patches to working copy that is not
completely clean, even on top of modified files

But yes, I'd like cg-patch to move to use git-apply. It's currently
_way_ too scary.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>   (i) No git-apply -R - well, it seems to me that I revert patches all
> the time, don't you?
>
>   (ii) I'd like git-apply to be as verbose as patch is, that is list
> the files it touches as it goes
>
>   (iii) There's no reject handling besides "panic" right now - it should
> be able to create .rej files so that the user can fix things up
>
>   (iv) I need git-apply to add/remove to/from index new/gone files,
> while at the same time...
>
>   (v) I want to allow applying of patches to working copy that is not
> completely clean, even on top of modified files

You probably should be able to talk me into doing these, but
doesn't it already do (iv) and (v)?

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Petr Baudis]

Dear diary, on Fri, Jul 28, 2006 at 04:41:04AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> >   (iv) I need git-apply to add/remove to/from index new/gone files,
> > while at the same time...
> >
> >   (v) I want to allow applying of patches to working copy that is not
> > completely clean, even on top of modified files
> 
> You probably should be able to talk me into doing these, but
> doesn't it already do (iv) and (v)?

Well, at once? I can do (iv) by adding --index but that contradicts (v).
But maybe I'm missing something.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Fri, Jul 28, 2006 at 04:41:04AM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> Petr Baudis <pasky@suse.cz> writes:
>> >   (iv) I need git-apply to add/remove to/from index new/gone files,
>> > while at the same time...
>> >
>> >   (v) I want to allow applying of patches to working copy that is not
>> > completely clean, even on top of modified files
>> 
>> You probably should be able to talk me into doing these, but
>> doesn't it already do (iv) and (v)?
>
> Well, at once? I can do (iv) by adding --index but that contradicts (v).
> But maybe I'm missing something.

What should the semantics of such operation be?  Apply to index
on paths that are clean while leave the index entries untouched
for paths that are dirty?  What should happen on renamed paths
that are dirty?

[PATCH] Teach git-apply about '-R'

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

On Fri, 28 Jul 2006, Petr Baudis wrote:

>   (i) No git-apply -R - well, it seems to me that I revert patches all
> the time, don't you?

 builtin-apply.c         |   68 ++++++++++++++++++++++++++++++++++++++++-------
 t/t4102-apply-rename.sh |   24 +++++++++++++++--
 2 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index d924ac3..eb3eda1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -120,7 +120,7 @@ struct fragment {
 struct patch {
 	char *new_name, *old_name, *def_name;
 	unsigned int old_mode, new_mode;
-	int is_rename, is_copy, is_new, is_delete, is_binary;
+	int is_rename, is_copy, is_new, is_delete, is_binary, is_reverse;
 #define BINARY_DELTA_DEFLATED 1
 #define BINARY_LITERAL_DEFLATED 2
 	unsigned long deflate_origlen;
@@ -1119,6 +1119,37 @@ static int parse_chunk(char *buffer, uns
 	return offset + hdrsize + patchsize;
 }
 
+/* a and b may not overlap! */
+static void memswap(void *a, void *b, unsigned int len)
+{
+	char *ap = a, *bp = b;
+	int i;
+	char c;
+
+	for (i = 0; i < len; i++) {
+		c = ap[i]; ap[i] = bp[i]; bp[i] = c;
+	}
+}
+
+static void reverse_patches(struct patch *p)
+{
+	for (; p; p = p->next) {
+		struct fragment *frag = p->fragments;
+
+		memswap(p->new_name, p->old_name, sizeof(char *));
+		memswap(&p->new_mode, &p->old_mode, sizeof(unsigned int));
+		memswap(&p->is_new, &p->is_delete, sizeof(int));
+		memswap(&p->lines_added, &p->lines_deleted, sizeof(int));
+		memswap(p->old_sha1_prefix, p->new_sha1_prefix, 41);
+
+		for (; frag; frag = frag->next) {
+			memswap(&frag->newpos, &frag->oldpos, sizeof(int));
+			memswap(&frag->newlines, &frag->oldlines, sizeof(int));
+		}
+		p->is_reverse = !p->is_reverse;
+	}
+}
+
 static const char pluses[] = "++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++";
 static const char minuses[]= "----------------------------------------------------------------------";
 
@@ -1336,7 +1367,7 @@ static int apply_line(char *output, cons
 }
 
 static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag,
-	int inaccurate_eof)
+	int reverse, int inaccurate_eof)
 {
 	int match_beginning, match_end;
 	char *buf = desc->buffer;
@@ -1350,6 +1381,7 @@ static int apply_one_fragment(struct buf
 	int pos, lines;
 
 	while (size > 0) {
+		char first;
 		int len = linelen(patch, size);
 		int plen;
 
@@ -1366,16 +1398,23 @@ static int apply_one_fragment(struct buf
 		plen = len-1;
 		if (len < size && patch[len] == '\\')
 			plen--;
-		switch (*patch) {
+		first = *patch;
+		if (reverse) {
+			if (first == '-')
+				first = '+';
+			else if (first == '+')
+				first = '-';
+		}
+		switch (first) {
 		case ' ':
 		case '-':
 			memcpy(old + oldsize, patch + 1, plen);
 			oldsize += plen;
-			if (*patch == '-')
+			if (first == '-')
 				break;
 		/* Fall-through for ' ' */
 		case '+':
-			if (*patch != '+' || !no_add)
+			if (first != '+' || !no_add)
 				newsize += apply_line(new + newsize, patch,
 						      plen);
 			break;
@@ -1615,7 +1654,8 @@ static int apply_fragments(struct buffer
 		return apply_binary(desc, patch);
 
 	while (frag) {
-		if (apply_one_fragment(desc, frag, patch->inaccurate_eof) < 0)
+		if (apply_one_fragment(desc, frag, patch->is_reverse,
+					patch->inaccurate_eof) < 0)
 			return error("patch failed: %s:%ld",
 				     name, frag->oldpos);
 		frag = frag->next;
@@ -2142,7 +2182,8 @@ static int use_patch(struct patch *p)
 	return 1;
 }
 
-static int apply_patch(int fd, const char *filename, int inaccurate_eof)
+static int apply_patch(int fd, const char *filename,
+		int reverse, int inaccurate_eof)
 {
 	unsigned long offset, size;
 	char *buffer = read_patch_file(fd, &size);
@@ -2162,6 +2203,8 @@ static int apply_patch(int fd, const cha
 		nr = parse_chunk(buffer + offset, size, patch);
 		if (nr < 0)
 			break;
+		if (reverse)
+			reverse_patches(patch);
 		if (use_patch(patch)) {
 			patch_stats(patch);
 			*listp = patch;
@@ -2226,6 +2269,7 @@ int cmd_apply(int argc, const char **arg
 {
 	int i;
 	int read_stdin = 1;
+	int reverse = 0;
 	int inaccurate_eof = 0;
 
 	const char *whitespace_option = NULL;
@@ -2236,7 +2280,7 @@ int cmd_apply(int argc, const char **arg
 		int fd;
 
 		if (!strcmp(arg, "-")) {
-			apply_patch(0, "<stdin>", inaccurate_eof);
+			apply_patch(0, "<stdin>", reverse, inaccurate_eof);
 			read_stdin = 0;
 			continue;
 		}
@@ -2313,6 +2357,10 @@ int cmd_apply(int argc, const char **arg
 			parse_whitespace_option(arg + 13);
 			continue;
 		}
+		if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
+			reverse = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--inaccurate-eof")) {
 			inaccurate_eof = 1;
 			continue;
@@ -2333,12 +2381,12 @@ int cmd_apply(int argc, const char **arg
 			usage(apply_usage);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		apply_patch(fd, arg, inaccurate_eof);
+		apply_patch(fd, arg, reverse, inaccurate_eof);
 		close(fd);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		apply_patch(0, "<stdin>", inaccurate_eof);
+		apply_patch(0, "<stdin>", reverse, inaccurate_eof);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {
diff --git a/t/t4102-apply-rename.sh b/t/t4102-apply-rename.sh
index fbb508d..22da6a0 100755
--- a/t/t4102-apply-rename.sh
+++ b/t/t4102-apply-rename.sh
@@ -13,8 +13,8 @@ # setup
 cat >test-patch <<\EOF
 diff --git a/foo b/bar
 similarity index 47%
-copy from foo
-copy to bar
+rename from foo
+rename to bar
 --- a/foo
 +++ b/bar
 @@ -1 +1 @@
@@ -39,4 +39,24 @@ else
 	    'test -f bar && ls -l bar | grep "^-..x......"'
 fi
 
+test_expect_success 'apply reverse' \
+    'git-apply -R --index --stat --summary --apply test-patch &&
+     test "$(cat foo)" = "This is foo"'
+
+cat >test-patch <<\EOF
+diff --git a/foo b/bar
+similarity index 47%
+copy from foo
+copy to bar
+--- a/foo
++++ b/bar
+@@ -1 +1 @@
+-This is foo
++This is bar
+EOF
+
+test_expect_success 'apply copy' \
+    'git-apply --index --stat --summary --apply test-patch &&
+     test "$(cat bar)" = "This is bar" -a "$(cat foo)" = "This is foo"'
+
 test_done
-- 
1.4.2.rc2.g813d5-dirty

Re: [PATCH] Teach git-apply about '-R'

[Linus Torvalds]

On Fri, 28 Jul 2006, Johannes Schindelin wrote:
>
> +/* a and b may not overlap! */
> +static void memswap(void *a, void *b, unsigned int len)

This is disgusting.

Especially since it's also slow as hell.

> +		memswap(p->new_name, p->old_name, sizeof(char *));
> +		memswap(&p->new_mode, &p->old_mode, sizeof(unsigned int));
> +		memswap(&p->is_new, &p->is_delete, sizeof(int));
> +		memswap(&p->lines_added, &p->lines_deleted, sizeof(int));
> +		memswap(p->old_sha1_prefix, p->new_sha1_prefix, 41);
> +
> +		for (; frag; frag = frag->next) {
> +			memswap(&frag->newpos, &frag->oldpos, sizeof(int));
> +			memswap(&frag->newlines, &frag->oldlines, sizeof(int));

All but one of those are register sizes, so doing a horribly ugly 
"memswap()"to do them is truly nasty, when you could have done

	#define swap(a,b) myswap((a),(b),sizeof(a))

	#define myswap(a,b,size) do {		\
		unsigned char mytmp[size];	\
		memcpy(tmp, &a, size);		\
		memcpy(&a, &b, size);		\
		memcpy(&b, mytmp, size);	\
	} while (0)

and it would have worked MUCH more efficiently, since any sane compiler 
would immediately have noticed that you're doing word-sized copies, and 
optimized the hell out of it.

(Untested, of course).

		Linus

Re: [PATCH] Teach git-apply about '-R'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

A quick comment without looking much at the code.  Do you sanely
bail out when asked to reverse-apply a binary patch?

Also what was the reason to change an existing test vector in
4102?  That does not look like related to -R flag.

[PATCH v2] Teach git-apply about '-R'

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

On Fri, 28 Jul 2006, Linus Torvalds wrote:

> On Fri, 28 Jul 2006, Johannes Schindelin wrote:
> >
> > +/* a and b may not overlap! */
> > +static void memswap(void *a, void *b, unsigned int len)
> 
> This is disgusting.

Yes, it is. Sorry, was just meant for prototyping. I had in mind to 
replace it with temporary variables, but your solution seems way 
nicer.

 builtin-apply.c         |   65 ++++++++++++++++++++++++++++++++++++++++-------
 t/t4102-apply-rename.sh |   24 ++++++++++++++++-
 2 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index d924ac3..6b38a8a 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -120,7 +120,7 @@ struct fragment {
 struct patch {
 	char *new_name, *old_name, *def_name;
 	unsigned int old_mode, new_mode;
-	int is_rename, is_copy, is_new, is_delete, is_binary;
+	int is_rename, is_copy, is_new, is_delete, is_binary, is_reverse;
 #define BINARY_DELTA_DEFLATED 1
 #define BINARY_LITERAL_DEFLATED 2
 	unsigned long deflate_origlen;
@@ -1119,6 +1119,34 @@ static int parse_chunk(char *buffer, uns
 	return offset + hdrsize + patchsize;
 }
 
+#define swap(a,b) myswap((a),(b),sizeof(a))
+
+#define myswap(a, b, size) do {		\
+	unsigned char mytmp[size];	\
+	memcpy(mytmp, &a, size);		\
+	memcpy(&a, &b, size);		\
+	memcpy(&b, mytmp, size);		\
+} while (0)
+
+static void reverse_patches(struct patch *p)
+{
+	for (; p; p = p->next) {
+		struct fragment *frag = p->fragments;
+
+		swap(p->new_name, p->old_name);
+		swap(p->new_mode, p->old_mode);
+		swap(p->is_new, p->is_delete);
+		swap(p->lines_added, p->lines_deleted);
+		swap(p->old_sha1_prefix, p->new_sha1_prefix);
+
+		for (; frag; frag = frag->next) {
+			swap(frag->newpos, frag->oldpos);
+			swap(frag->newlines, frag->oldlines);
+		}
+		p->is_reverse = !p->is_reverse;
+	}
+}
+
 static const char pluses[] = "++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++";
 static const char minuses[]= "----------------------------------------------------------------------";
 
@@ -1336,7 +1364,7 @@ static int apply_line(char *output, cons
 }
 
 static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag,
-	int inaccurate_eof)
+	int reverse, int inaccurate_eof)
 {
 	int match_beginning, match_end;
 	char *buf = desc->buffer;
@@ -1350,6 +1378,7 @@ static int apply_one_fragment(struct buf
 	int pos, lines;
 
 	while (size > 0) {
+		char first;
 		int len = linelen(patch, size);
 		int plen;
 
@@ -1366,16 +1395,23 @@ static int apply_one_fragment(struct buf
 		plen = len-1;
 		if (len < size && patch[len] == '\\')
 			plen--;
-		switch (*patch) {
+		first = *patch;
+		if (reverse) {
+			if (first == '-')
+				first = '+';
+			else if (first == '+')
+				first = '-';
+		}
+		switch (first) {
 		case ' ':
 		case '-':
 			memcpy(old + oldsize, patch + 1, plen);
 			oldsize += plen;
-			if (*patch == '-')
+			if (first == '-')
 				break;
 		/* Fall-through for ' ' */
 		case '+':
-			if (*patch != '+' || !no_add)
+			if (first != '+' || !no_add)
 				newsize += apply_line(new + newsize, patch,
 						      plen);
 			break;
@@ -1615,7 +1651,8 @@ static int apply_fragments(struct buffer
 		return apply_binary(desc, patch);
 
 	while (frag) {
-		if (apply_one_fragment(desc, frag, patch->inaccurate_eof) < 0)
+		if (apply_one_fragment(desc, frag, patch->is_reverse,
+					patch->inaccurate_eof) < 0)
 			return error("patch failed: %s:%ld",
 				     name, frag->oldpos);
 		frag = frag->next;
@@ -2142,7 +2179,8 @@ static int use_patch(struct patch *p)
 	return 1;
 }
 
-static int apply_patch(int fd, const char *filename, int inaccurate_eof)
+static int apply_patch(int fd, const char *filename,
+		int reverse, int inaccurate_eof)
 {
 	unsigned long offset, size;
 	char *buffer = read_patch_file(fd, &size);
@@ -2162,6 +2200,8 @@ static int apply_patch(int fd, const cha
 		nr = parse_chunk(buffer + offset, size, patch);
 		if (nr < 0)
 			break;
+		if (reverse)
+			reverse_patches(patch);
 		if (use_patch(patch)) {
 			patch_stats(patch);
 			*listp = patch;
@@ -2226,6 +2266,7 @@ int cmd_apply(int argc, const char **arg
 {
 	int i;
 	int read_stdin = 1;
+	int reverse = 0;
 	int inaccurate_eof = 0;
 
 	const char *whitespace_option = NULL;
@@ -2236,7 +2277,7 @@ int cmd_apply(int argc, const char **arg
 		int fd;
 
 		if (!strcmp(arg, "-")) {
-			apply_patch(0, "<stdin>", inaccurate_eof);
+			apply_patch(0, "<stdin>", reverse, inaccurate_eof);
 			read_stdin = 0;
 			continue;
 		}
@@ -2313,6 +2354,10 @@ int cmd_apply(int argc, const char **arg
 			parse_whitespace_option(arg + 13);
 			continue;
 		}
+		if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
+			reverse = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--inaccurate-eof")) {
 			inaccurate_eof = 1;
 			continue;
@@ -2333,12 +2378,12 @@ int cmd_apply(int argc, const char **arg
 			usage(apply_usage);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		apply_patch(fd, arg, inaccurate_eof);
+		apply_patch(fd, arg, reverse, inaccurate_eof);
 		close(fd);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		apply_patch(0, "<stdin>", inaccurate_eof);
+		apply_patch(0, "<stdin>", reverse, inaccurate_eof);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {
diff --git a/t/t4102-apply-rename.sh b/t/t4102-apply-rename.sh
index fbb508d..22da6a0 100755
--- a/t/t4102-apply-rename.sh
+++ b/t/t4102-apply-rename.sh
@@ -13,8 +13,8 @@ # setup
 cat >test-patch <<\EOF
 diff --git a/foo b/bar
 similarity index 47%
-copy from foo
-copy to bar
+rename from foo
+rename to bar
 --- a/foo
 +++ b/bar
 @@ -1 +1 @@
@@ -39,4 +39,24 @@ else
 	    'test -f bar && ls -l bar | grep "^-..x......"'
 fi
 
+test_expect_success 'apply reverse' \
+    'git-apply -R --index --stat --summary --apply test-patch &&
+     test "$(cat foo)" = "This is foo"'
+
+cat >test-patch <<\EOF
+diff --git a/foo b/bar
+similarity index 47%
+copy from foo
+copy to bar
+--- a/foo
++++ b/bar
+@@ -1 +1 @@
+-This is foo
++This is bar
+EOF
+
+test_expect_success 'apply copy' \
+    'git-apply --index --stat --summary --apply test-patch &&
+     test "$(cat bar)" = "This is bar" -a "$(cat foo)" = "This is foo"'
+
 test_done
-- 
1.4.2.rc2.g8b063-dirty

Re: [PATCH 1/2] t7001: add test for git-mv dir1 dir2/

[Petr Baudis]

Dear diary, on Fri, Jul 28, 2006 at 06:48:49AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> > Well, at once? I can do (iv) by adding --index but that contradicts (v).
> > But maybe I'm missing something.
> 
> What should the semantics of such operation be?  Apply to index
> on paths that are clean while leave the index entries untouched
> for paths that are dirty?  What should happen on renamed paths
> that are dirty?

Keep the original sha1 but change the name. If an entry with the new
name already exists, we might just leave the index alone and create a
.rej file. (Alternatively we might create two stages in the index file
but we can't do that in case of regular rejects so I'd rather stay
consistent.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam

Re: [PATCH] Teach git-apply about '-R'

[Johannes Schindelin]

Hi,

On Fri, 28 Jul 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> A quick comment without looking much at the code.  Do you sanely
> bail out when asked to reverse-apply a binary patch?

Nope. I swap old_sha1_prefix and new_sha1_prefix in that case, I hoped 
that is enough?

> Also what was the reason to change an existing test vector in
> 4102?  That does not look like related to -R flag.

I changed "copy" to "rename", so that I could reuse that patch to test -R, 
too. Note that in the end, the original test vector is written, and 
tested.

Ciao,
Dscho

Re: [PATCH] Teach git-apply about '-R'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 28 Jul 2006, Junio C Hamano wrote:
>> 
>> A quick comment without looking much at the code.  Do you sanely
>> bail out when asked to reverse-apply a binary patch?
>
> Nope. I swap old_sha1_prefix and new_sha1_prefix in that case, I hoped 
> that is enough?

You would need something like this, at least for now, since both
deflated literal and deflated delta methods are irreversible.
I'll cook up another binary diff output that can go
bidirectional.

Note that --allow-binary-replacement uses the blob object name
recorded on index lines of binary patch, and uses it to cheat
(iow, when it knows your version is the old version recorded on
index line and your repository happens to have the resulting
blob, it just uses the blob without looking at the binary
contents recorded in the patch), so you have to work a bit hard
to cause it to fail in t/trash repository after you run t4103
test.  Resetting to "master", apply BF.diff, and immediately
reverse apply BF.diff would _work_, only because "master" and
"binary" branch keep both preimage and postimage.

-- >8 --
diff --git a/builtin-apply.c b/builtin-apply.c
index 6b38a8a..d4381d9 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1535,6 +1535,12 @@ static int apply_binary_fragment(struct 
 	void *data;
 	void *result;
 
+	/* Binary patch is irreversible */
+	if (patch->is_reverse)
+		return error("cannot reverse-apply a binary patch to '%s'",
+			     patch->new_name
+			     ? patch->new_name : patch->old_name);
+
 	data = inflate_it(fragment->patch, fragment->size,
 			  patch->deflate_origlen);
 	if (!data)

Re: [PATCH] Teach git-apply about '-R'

[Johannes Schindelin]

Hi,

On Fri, 28 Jul 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Fri, 28 Jul 2006, Junio C Hamano wrote:
> >> 
> >> A quick comment without looking much at the code.  Do you sanely
> >> bail out when asked to reverse-apply a binary patch?
> >
> > Nope. I swap old_sha1_prefix and new_sha1_prefix in that case, I hoped 
> > that is enough?
> 
> You would need something like this, at least for now, since both
> deflated literal and deflated delta methods are irreversible.

Somehow I had the impression that binary diff meant that you needed both 
objects in the object database. I was wrong.

Ciao,
Dscho