Git development
 help / color / mirror / Atom feed
* Re: log/show: relative pathnames do not work in rev:path
From: Alex Riesen @ 2007-12-18 20:46 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Johannes Schindelin, Linus Torvalds
In-Reply-To: <m3d4t3q4e5.fsf@roke.D-201>

Jakub Narebski, Tue, Dec 18, 2007 18:50:20 +0100:
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> > Noticed by a collegue of mine. Consider:
> > 
> >     $ cd $GIT/t
> >     $ git show 570f32266:t/test-lib.sh    # works
> >     $ git show 570f32266:test-lib.sh      # does not work
> >     $ git show 570f32266:./test-lib.sh    # does not work
> >     $ git show 570f32266:/t/test-lib.sh   # does not work
> > 
> > Considering that the relative path names work as filters (and many
> > agreed on that being useful), it would be nice to allow relative
> > pathnames in blob specifications for git-show and git-cat-file.
> > 
> > (besides the colon is a good delimiter, even tab-completion works with it)
> 
> If you think about it a bit, relative path names nor absolute
> path names does and should not work.  570f32266:t/test-lib.sh
> means path t/test-lib.sh staring from 570f32266^{tree}.  Where
> you are in the filesystem is not important and matters not for
> this syntax.  Besides if you access other branch file might be
> not in filesystem (deleted file, or disjoint branch with separate
> contents like 'todo' or 'html' branch in git.git repository).

Not convinced. It is *not* the plumbing problem I was trying to
describe. They discussion, metaphorically, should not have left the
command-line parser.

I think that we have parsing of the blob locators at the wrong level:
so that git-show, git-log and git-diff can handle its pathnames as
they handle path filters (relative to cwd), and git-cat-file,
git-diff-tree, git-rev-list, etc can handle theirs always relative to
the project root.

I actually do not see any problem for git-show (being porcelain-level
program) to treat *each and every* path anywhere relatively to the
current directory. It is just more comfortable.

Please consider the following patches.

^ permalink raw reply

* Re: git with custom diff for commits
From: Junio C Hamano @ 2007-12-18 20:40 UTC (permalink / raw)
  To: Gerald Gutierrez
  Cc: 'Junio C Hamano', 'Matthieu Moy',
	'Johannes Schindelin', git
In-Reply-To: <000001c841b5$89fcef00$762a14ac@na.acco.com>

"Gerald Gutierrez" <ggmlfs@gmail.com> writes:

> I ended up doing the following, which sounds similar to the above. Instead
> of doing mysqldump into data.sql, it goes into data.sql.2 which I compare
> with the checked in data.sql using "diff -I <timestamp RE>". If there are no
> differences, I delete data.sql.2. If there are differences, I move
> data.sql.2 into data.sql and check in. Perhaps not as elegant but certainly
> works.

Heh, that's essentially how automated html/man branches are managed ;-)

^ permalink raw reply

* Re: [PATCH] Authentication support for pserver
From: Martin Langhoff @ 2007-12-18 20:39 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Junio C Hamano, git, martyn, martin
In-Reply-To: <46a038f90712180141x2f27e6cei5ef53339fd3f90dc@mail.gmail.com>

On Dec 18, 2007 10:41 PM, Martin Langhoff <martin.langhoff@gmail.com> wrote:
>  - git/config is very likely to be readable if the site is served via
> other means, like dumb http protocol, or git+ssh. So even if the
> password scrambling is mickey-mouse. it might make sense to force the
> password data to live elsewhere.

On this aspect, I see no reason why we wouldn't have the passwords
crypt()ed or SHA1'd. Perl includes crypt() in the default
distribution, so it wouldn't add any dependency.

The protocol may be ROT13'ing the passwords, but we don't have to
double-ROT13 them in our storage format ;-)

cheers,


martin

^ permalink raw reply

* RE: git with custom diff for commits
From: Gerald Gutierrez @ 2007-12-18 20:35 UTC (permalink / raw)
  To: 'Junio C Hamano', 'Matthieu Moy'
  Cc: 'Johannes Schindelin', git
In-Reply-To: <7vodco1him.fsf@gitster.siamese.dyndns.org>


> 
> So my updated suggestion would be:
> 
>  - Keep a checkout of that mysql dump;
> 
>  - Make the dump procedure to dump to that checkout area;
> 
>  - Run "git diff HEAD", perhaps with the external diff that 
> filters out
>    the cruft as suggested in the thread, and run "git checkout $path"
>    for paths that had only meaningless changes;
> 
>  - Run "git commit" if the above step did not nuke all changes.
> 

I ended up doing the following, which sounds similar to the above. Instead
of doing mysqldump into data.sql, it goes into data.sql.2 which I compare
with the checked in data.sql using "diff -I <timestamp RE>". If there are no
differences, I delete data.sql.2. If there are differences, I move
data.sql.2 into data.sql and check in. Perhaps not as elegant but certainly
works.

Gerald.

^ permalink raw reply

* Re: [PATCH] provide advance warning of some future pack default changes
From: Junio C Hamano @ 2007-12-18 20:24 UTC (permalink / raw)
  To: Jeff King
  Cc: Martin Langhoff, Nicolas Pitre, Joel Becker, Jakub Narebski, git
In-Reply-To: <20071218111136.GA6266@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I can think of two other user-visible changes which have been discussed
> that might warrant such a version bump:
>
>   - option parsing tweaks (hopefully these should be minor, but it is
>     clear that we cannot be 100% consistent while retaining the
>     identical previous behavior)

This could have a fallout, like *-default disambiguation which scripts
did not have to implement.

>   - moving dashed forms out of paths

This is already planned for 1.5.5 and it is not among "other
user-visible changes".  Technically the use of git-foo form without
preparing the environment has not been supported for quite some time,
but people have come to rely on it and I'd agree this warrants a 1.6.0.

^ permalink raw reply

* Re: [PATCH w/ test] rebase -p -i: handle "no changes" gracefully
From: Junio C Hamano @ 2007-12-18 20:24 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Pieter de Bie, Johannes Sixt, git
In-Reply-To: <Pine.LNX.4.64.0712172100450.9446@racer.site>

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

> Since commit 376ccb8cbb453343998e734d8a1ce79f57a4e092, unchanged
> SHA-1s are no longer mapped via $REWRITTEN.  But the updating
> phase was not prepared for the old head not being rewritten.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	On Mon, 17 Dec 2007, Junio C Hamano wrote:
>
> 	> Hmph, care to add a test to t3404?
>
> 	How about this?

Looks good.  Thanks.

^ permalink raw reply

* Re: [PATCH] Improved submodule merge support
From: Johannes Schindelin @ 2007-12-18 20:21 UTC (permalink / raw)
  To: Finn Arne Gangstad; +Cc: gitster, git
In-Reply-To: <20071218195028.GA25510@pvv.org>

Hi,

On Tue, 18 Dec 2007, Finn Arne Gangstad wrote:

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 2a58dad..33ccc40 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1463,10 +1467,13 @@ static int process_entry(const char *path, struct stage_data *entry,
>  		mfi = merge_file(&o, &a, &b,
>  				 branch1, branch2);
>  
> +		clean_merge = mfi.clean;
>  		if (mfi.clean)
>  			update_file(1, mfi.sha, mfi.mode, path);
> +		else if (S_ISGITLINK(mfi.mode))
> +			output(1, "CONFLICT (submodule): Merge conflict in %s "
> +			       "- needs %s", path, sha1_to_hex(b.sha1));
>  		else {
> -			clean_merge = 0;
>  			output(1, "CONFLICT (%s): Merge conflict in %s",
>  					reason, path);
>  

It took me a little while to find that the unilateral assignment to 
clean_merge does not break things.  But as it was only set to 1 at the 
beginning of the function, and no loops are involved, this change is 
correct.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] provide advance warning of some future pack default changes
From: Nicolas Pitre @ 2007-12-18 20:12 UTC (permalink / raw)
  To: Jeff King
  Cc: Johannes Schindelin, Junio C Hamano, Martin Langhoff, Joel Becker,
	Jakub Narebski, git
In-Reply-To: <20071218193035.GA4583@sigill.intra.peff.net>

On Tue, 18 Dec 2007, Jeff King wrote:

> So there will be user-visible changes (though I don't expect them to be
> huge...there simply aren't that many variables with optional arguments).

OTOH, there are quite a bunch of changes affecting the user experience.  
Many of the feedback messages printed by Git were completely revamped, 
starting with the progress display to the fetch summary.


Nicolas

^ permalink raw reply

* Re: [PATCH] git show <tag>: show the tagger
From: Junio C Hamano @ 2007-12-18 20:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0712181800250.23902@racer.site>

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

> For commit objects, the Author is shown, so do the equivalent for
> tag objects, too.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	I know, it's feature freeze period.  But this is arguably a 
> 	usability bug.

I'll wait for people to argue this fixes a usability bug, then.

^ permalink raw reply

* Re: [PATCH] Add format-patch option --no-name-prefix.
From: Junio C Hamano @ 2007-12-18 20:07 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Pascal Obry, Johannes Sixt, Pascal Obry, git
In-Reply-To: <alpine.LFD.0.9999.0712180840060.21557@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Tue, 18 Dec 2007, Pascal Obry wrote:
>
>> Johannes Sixt a écrit :
>> > that we need another diff option for it. Ok, on my keyboard 0 is typed
>> > with the right hand, and 1 with the left hand, but... ??
>> 
>> Because you just did not read my follow-up message :)
>> 
>> I need this has I do not have the way to change the server applying the
>> patch. So nothing wrong with my hands or fingers :)
> ....
> So I think you'd need separate arguments for the from/to prefixes, and not 
> try to shoehorn it into one argument. With possibly some simple form to 
> say "no prefix". So maybe something like
>
>   --src-prefix=<string>		// default "a/"
>   --dst-prefix=<string>		// default "b/"
>   --no-prefix			// shorthand for --src-prefix="" --dst-prefix=""
>
> would work for everybody?

One worry I have is that "diff --git" is validated more strictly than
other diffs by "git-apply", and patches generated with arbitrary prefix
would break it.  It might make sense to drop " --git" from the patch
header if we allow a/ and b/ to be changed inconsistently.

^ permalink raw reply

* Re: [PATCH v4] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Linus Torvalds @ 2007-12-18 19:55 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, Pascal Obry, Johannes Sixt, Pascal Obry, git
In-Reply-To: <Pine.LNX.4.64.0712181930130.23902@racer.site>



On Tue, 18 Dec 2007, Johannes Schindelin wrote:
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Acked-by: Linus Torvalds <torvalds@linux-foundation.org>

Thanks,

		Linus

^ permalink raw reply

* [PATCH] Improved submodule merge support
From: Finn Arne Gangstad @ 2007-12-18 19:50 UTC (permalink / raw)
  To: gitster, git

Currently merging submodules from a super module does not work at
all, here is an example:

$ git merge change1
Updating 41dee71..6dbd2d9
Fast forward
 sub-module |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

$ git merge change2
fatal: cannot read object 4a0b570e9b7c6fd36f964eb6ef55263834462235 'sub-module'
Merge with strategy recursive failed.

$ git status
# On branch mergetest
nothing to commit (working directory clean)

$ git submodule status
 4a0b570e9b7c6fd36f964eb6ef55263834462235 sub-module (undefined)

So - the merge simply failed, and git status has no clue what happened.


With the following patch, this happens instead:

$ git merge change1
Updating 41dee71..6dbd2d9
Fast forward
 sub-module |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

$ git merge change2
Auto-merged sub-module
CONFLICT (submodule): Merge conflict in sub-module - needs b65e6131a2705620a0b08a4ecc44427b9059e4e3
Automatic merge failed; fix conflicts and then commit the result.

$ git status
sub-module: needs merge
# On branch mergetest
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       unmerged:   sub-module
#
no changes added to commit (use "git add" and/or "git commit -a")

$ git submodule status
 11253a619a520fa4b2e2c2f83cb5c897170faafd sub-module (undefined)
 4a0b570e9b7c6fd36f964eb6ef55263834462235 sub-module (undefined)
 b65e6131a2705620a0b08a4ecc44427b9059e4e3 sub-module (undefined)


To resolve this:

$ cd sub-module
$ git merge b65e6131a27
$ cd ..
$ git add sub-module
$ git commit -m 'merged sub-module blablabla'
$ git submodule status
 eb66300641185297648b2a71e41a66b2053fdae0 sub-module (undefined)


So it actually works in some sense, and you see what you need to merge
aftwards in the submodule.

- Finn Arne


-- >8 --
Improved submodule merge support

When merging conflicting submodule changes from a supermodule, generate
a conflict message saying what went wrong. Also leave the tree in a state
where git status shows the conflict, and git submodule status gives the user
enough information to do the merge manally. Previously this would just fail.

Signed-off-by: Finn Arne Gangstad <finnag@pvv.org>
---
 merge-recursive.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 2a58dad..33ccc40 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -549,6 +549,10 @@ static void update_file_flags(const unsigned char *sha,
 		void *buf;
 		unsigned long size;
 
+		if (S_ISGITLINK(mode))
+			die("cannot read object %s '%s': It is a submodule!",
+			    sha1_to_hex(sha), path);
+
 		buf = read_sha1_file(sha, &type, &size);
 		if (!buf)
 			die("cannot read object %s '%s'", sha1_to_hex(sha), path);
@@ -1463,10 +1467,13 @@ static int process_entry(const char *path, struct stage_data *entry,
 		mfi = merge_file(&o, &a, &b,
 				 branch1, branch2);
 
+		clean_merge = mfi.clean;
 		if (mfi.clean)
 			update_file(1, mfi.sha, mfi.mode, path);
+		else if (S_ISGITLINK(mfi.mode))
+			output(1, "CONFLICT (submodule): Merge conflict in %s "
+			       "- needs %s", path, sha1_to_hex(b.sha1));
 		else {
-			clean_merge = 0;
 			output(1, "CONFLICT (%s): Merge conflict in %s",
 					reason, path);
 
-- 
1.5.4.rc0.67.gf9c5

^ permalink raw reply related

* [PATCH] Fix tests for broken sed on Leopard
From: Wincent Colaiuta @ 2007-12-18 19:45 UTC (permalink / raw)
  To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <7vzlw7zwj8.fsf@gitster.siamese.dyndns.org>

El 18/12/2007, a las 19:29, Junio C Hamano escribió:

> Could you work it around not by changing what is tested, but by changing
> the way the result (actual and expect) are compared, please?  I actually
> wanted to enhance this to test with values 1023, 2047, and 4095, and
> your change will close the door for such changes.

How about the following? This swaps in perl in place of sed, which we can
hopefully rely upon to work across platforms.

Cheers,
Wincent

-------- 8< --------
Fix tests for broken sed on Leopard

The newly-added common-tail-optimization test fails on Leopard because
the broken sed implementation bails with a spurious "unterminated
substitute pattern" error because of the length of one of the
arguments.

So use perl instead of sed, and at the same time add test cases for
1024 - 1 and 4096 - 1 as suggested by Junio.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 t/t4024-diff-optimize-common.sh |  104 ++++++++++++++++++++++++++++++---------
 1 files changed, 80 insertions(+), 24 deletions(-)

diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index 20fe87b..84dfb05 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -7,28 +7,12 @@ test_description='common tail optimization'
 z=zzzzzzzz ;# 8
 z="$z$z$z$z$z$z$z$z" ;# 64
 z="$z$z$z$z$z$z$z$z" ;# 512
-z="$z$z$z$z" ;# 2048
-z2047=$(expr "$z" : '.\(.*\)') ; #2047
-
-test_expect_success setup '
-
-	echo "a$z2047" >file-a &&
-	echo "b" >file-b &&
-	echo "$z2047" >>file-b &&
-	echo "c$z2047" | tr -d "\012" >file-c &&
-	echo "d" >file-d &&
-	echo "$z2047" | tr -d "\012" >>file-d &&
-
-	git add file-a file-b file-c file-d &&
-
-	echo "A$z2047" >file-a &&
-	echo "B" >file-b &&
-	echo "$z2047" >>file-b &&
-	echo "C$z2047" | tr -d "\012" >file-c &&
-	echo "D" >file-d &&
-	echo "$z2047" | tr -d "\012" >>file-d
-
-'
+z1024="$z$z"
+z1023=$(expr "$z1024" : '.\(.*\)')
+z2048="$z1024$z1024"
+z2047=$(expr "$z2048" : '.\(.*\)')
+z4096="$z2048$z2048"
+z4095=$(expr "$z4096" : '.\(.*\)')
 
 cat >expect <<\EOF
 diff --git a/file-a b/file-a
@@ -59,11 +43,83 @@ diff --git a/file-d b/file-d
 +D
 EOF
 
-test_expect_success 'diff -U0' '
+test_expect_success 'setup (1023-char sequence)' '
+
+	echo "a$z1023" >file-a &&
+	echo "b" >file-b &&
+	echo "$z1023" >>file-b &&
+	echo "c$z1023" | tr -d "\012" >file-c &&
+	echo "d" >file-d &&
+	echo "$z1023" | tr -d "\012" >>file-d &&
+
+	git add file-a file-b file-c file-d &&
+
+	echo "A$z1023" >file-a &&
+	echo "B" >file-b &&
+	echo "$z1023" >>file-b &&
+	echo "C$z1023" | tr -d "\012" >file-c &&
+	echo "D" >file-d &&
+	echo "$z1023" | tr -d "\012" >>file-d
+
+'
+
+test_expect_success 'diff -U0 (1023-char sequence)' '
+	git diff -U0 | perl -pe "s/^index.+\n//g; s/$z1023/Z/g" >actual &&
+	diff -u expect actual
+
+'
+
+test_expect_success 'setup (2047-char sequence)' '
 
-	git diff -U0 | sed -e "/^index/d" -e "s/$z2047/Z/g" >actual &&
+	echo "a$z2047" >file-a &&
+	echo "b" >file-b &&
+	echo "$z2047" >>file-b &&
+	echo "c$z2047" | tr -d "\012" >file-c &&
+	echo "d" >file-d &&
+	echo "$z2047" | tr -d "\012" >>file-d &&
+
+	git add file-a file-b file-c file-d &&
+
+	echo "A$z2047" >file-a &&
+	echo "B" >file-b &&
+	echo "$z2047" >>file-b &&
+	echo "C$z2047" | tr -d "\012" >file-c &&
+	echo "D" >file-d &&
+	echo "$z2047" | tr -d "\012" >>file-d
+
+'
+
+test_expect_success 'diff -U0 (2047-char sequence)' '
+
+	git diff -U0 | perl -pe "s/^index.+\n//g; s/$z2047/Z/g" >actual &&
 	diff -u expect actual
 
 '
 
+test_expect_success 'setup (4095-char sequence)' '
+
+	echo "a$z4095" >file-a &&
+	echo "b" >file-b &&
+	echo "$z4095" >>file-b &&
+	echo "c$z4095" | tr -d "\012" >file-c &&
+	echo "d" >file-d &&
+	echo "$z4095" | tr -d "\012" >>file-d &&
+
+	git add file-a file-b file-c file-d &&
+
+	echo "A$z4095" >file-a &&
+	echo "B" >file-b &&
+	echo "$z4095" >>file-b &&
+	echo "C$z4095" | tr -d "\012" >file-c &&
+	echo "D" >file-d &&
+	echo "$z4095" | tr -d "\012" >>file-d
+
+'
+
+test_expect_success 'diff -U0 (4095-char sequence)' '
+
+	git diff -U0 | perl -pe "s/^index.+\n//g; s/$z4095/Z/g" >actual &&
+	diff -u expect actual
+
+'
 test_done
-- 
1.5.4.rc0.68.g15eb8-dirty

^ permalink raw reply related

* [PATCH v4] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Johannes Schindelin @ 2007-12-18 19:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: gitster, Pascal Obry, Johannes Sixt, Pascal Obry, git
In-Reply-To: <alpine.LFD.0.9999.0712181059220.21557@woody.linux-foundation.org>


With the new options "--src-prefix=<prefix>", "--dst-prefix=<prefix>"
and "--no-prefix", you can now control the path prefixes of the diff
machinery.  These used to by hardwired to "a/" for the source prefix
and "b/" for the destination prefix.

Initial patch by Pascal Obry.  Sane option names suggested by Linus.

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

	On Tue, 18 Dec 2007, Linus Torvalds wrote:

	> On Tue, 18 Dec 2007, Johannes Schindelin wrote:
	> > 
	> > With the new options "--src-prefix <prefix>", "--dst-prefix 
	> > <prefix>"
	> 
	> I really would prefer "--[src|dst]-prefix=<prefix>" as a single 
	> argument.
	> 
	> I think that's the more common form for long arguments, isn't it 
	> (with any short-format arguments usually using the "-L <prefix>" 
	> kind of form)?

	Incidentally, this fixes src-prefix and dst-prefix (I wrote av[1] 
	instead of arg + 1, but forgot to commit).

	So now, it is tested at least once per option.

 Documentation/diff-options.txt |    9 +++++++++
 diff.c                         |   25 +++++++++++++++++--------
 diff.h                         |    1 +
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9ecc1d7..1a78635 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -211,5 +211,14 @@ endif::git-format-patch[]
 --no-ext-diff::
 	Disallow external diff drivers.
 
+--src-prefix=<prefix>::
+	Show the given source prefix instead of "a/".
+
+--dst-prefix=<prefix>::
+	Show the given destination prefix instead of "b/".
+
+--no-prefix::
+	Do not show any source or destination prefix.
+
 For more detailed explanation on these common options, see also
 link:diffcore.html[diffcore documentation].
diff --git a/diff.c b/diff.c
index e26584c..61fd492 100644
--- a/diff.c
+++ b/diff.c
@@ -290,9 +290,10 @@ static void emit_rewrite_diff(const char *name_a,
 			      const char *name_b,
 			      struct diff_filespec *one,
 			      struct diff_filespec *two,
-			      int color_diff)
+			      struct diff_options *o)
 {
 	int lc_a, lc_b;
+	int color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 	const char *name_a_tab, *name_b_tab;
 	const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO);
 	const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO);
@@ -309,9 +310,9 @@ static void emit_rewrite_diff(const char *name_a,
 	diff_populate_filespec(two, 0);
 	lc_a = count_lines(one->data, one->size);
 	lc_b = count_lines(two->data, two->size);
-	printf("%s--- a/%s%s%s\n%s+++ b/%s%s%s\n%s@@ -",
-	       metainfo, name_a, name_a_tab, reset,
-	       metainfo, name_b, name_b_tab, reset, fraginfo);
+	printf("%s--- %s%s%s%s\n%s+++ %s%s%s%s\n%s@@ -",
+	       metainfo, o->a_prefix, name_a, name_a_tab, reset,
+	       metainfo, o->b_prefix, name_b, name_b_tab, reset, fraginfo);
 	print_line_count(lc_a);
 	printf(" +");
 	print_line_count(lc_b);
@@ -1212,8 +1213,8 @@ static void builtin_diff(const char *name_a,
 	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
 	const char *reset = diff_get_color_opt(o, DIFF_RESET);
 
-	a_one = quote_two("a/", name_a + (*name_a == '/'));
-	b_two = quote_two("b/", name_b + (*name_b == '/'));
+	a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
+	b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
 	printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
@@ -1242,8 +1243,7 @@ static void builtin_diff(const char *name_a,
 		if ((one->mode ^ two->mode) & S_IFMT)
 			goto free_ab_and_return;
 		if (complete_rewrite) {
-			emit_rewrite_diff(name_a, name_b, one, two,
-					DIFF_OPT_TST(o, COLOR_DIFF));
+			emit_rewrite_diff(name_a, name_b, one, two, o);
 			o->found_changes = 1;
 			goto free_ab_and_return;
 		}
@@ -2020,6 +2020,9 @@ void diff_setup(struct diff_options *options)
 	else
 		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
+
+	options->a_prefix = "a/";
+	options->b_prefix = "b/";
 }
 
 int diff_setup_done(struct diff_options *options)
@@ -2291,6 +2294,12 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		else if (40 < options->abbrev)
 			options->abbrev = 40;
 	}
+	else if (!prefixcmp(arg, "--src-prefix="))
+		options->a_prefix = arg + 13;
+	else if (!prefixcmp(arg, "--dst-prefix="))
+		options->b_prefix = arg + 13;
+	else if (!strcmp(arg, "--no-prefix"))
+		options->a_prefix = options->b_prefix = "";
 	else
 		return 0;
 	return 1;
diff --git a/diff.h b/diff.h
index 7e8000a..beccf85 100644
--- a/diff.h
+++ b/diff.h
@@ -69,6 +69,7 @@ struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
 	const char *single_follow;
+	const char *a_prefix, *b_prefix;
 	unsigned flags;
 	int context;
 	int break_opt;
-- 
1.5.4.rc0.70.g30f7

^ permalink raw reply related

* Re: [PATCH] provide advance warning of some future pack default changes
From: Jeff King @ 2007-12-18 19:30 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Martin Langhoff, Nicolas Pitre, Joel Becker,
	Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0712181329340.23902@racer.site>

On Tue, Dec 18, 2007 at 01:30:49PM +0000, Johannes Schindelin wrote:

> > Yes, it should be, but I think there will be a few user-visible fallouts
> > (like "--abbrev $foo" in scripts should now be "--abbrev-default $foo"
> > for safety).
> 
> But we are on our way to fix this, no?  IOW this warrants not a version 
> bump, but an extended feature freeze/bug fix period (like Junio suggested, 
> until January).

I think the resolution seems to be that we will now support "--abbrev
foo", though we didn't in the past. Because the "foo" here is optional,
the old "git log --abbrev HEAD" is ambiguous. In this case we'll see
that "HEAD" isn't a number and DWIM. But that means a script trying to
be unambiguous should use "git log --abbrev-default $foo" to make sure
that "$foo" doesn't accidentally match as a number.

So there will be user-visible changes (though I don't expect them to be
huge...there simply aren't that many variables with optional arguments).

-Peff

^ permalink raw reply

* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Linus Torvalds @ 2007-12-18 19:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, Pascal Obry, Johannes Sixt, Pascal Obry, git
In-Reply-To: <Pine.LNX.4.64.0712181855020.23902@racer.site>



On Tue, 18 Dec 2007, Johannes Schindelin wrote:
> 
> With the new options "--src-prefix <prefix>", "--dst-prefix <prefix>"

I really would prefer "--[src|dst]-prefix=<prefix>" as a single argument. 

I think that's the more common form for long arguments, isn't it (with any 
short-format arguments usually using the "-L <prefix>" kind of form)?

		Linus

^ permalink raw reply

* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Pascal Obry @ 2007-12-18 19:03 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: gitster, Linus Torvalds, Johannes Sixt, Pascal Obry, git
In-Reply-To: <Pine.LNX.4.64.0712181855020.23902@racer.site>

Johannes Schindelin a écrit :
> With the new options "--src-prefix <prefix>", "--dst-prefix <prefix>"
> and "--no-prefix", you can now control the path prefixes of the diff
> machinery.  These used to by hardwired to "a/" for the source prefix
> and "b/" for the destination prefix.
> 
> Initial patch by Pascal Obry.  Sane option names suggested by Linus.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thanks this looks good to me.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Johannes Schindelin @ 2007-12-18 18:56 UTC (permalink / raw)
  To: gitster; +Cc: Pascal Obry, Linus Torvalds, Johannes Sixt, Pascal Obry, git
In-Reply-To: <476809EA.6080608@obry.net>


With the new options "--src-prefix <prefix>", "--dst-prefix <prefix>"
and "--no-prefix", you can now control the path prefixes of the diff
machinery.  These used to by hardwired to "a/" for the source prefix
and "b/" for the destination prefix.

Initial patch by Pascal Obry.  Sane option names suggested by Linus.

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

	On Tue, 18 Dec 2007, Pascal Obry wrote:

	> Johannes Schindelin a ?crit :
	> > If this is preferred, please squash this:
	> 
	> Work fine for me.
	> 
	> We just need a consolidated patch with proper change log.

	How does this grab you?

 Documentation/diff-options.txt |    9 +++++++++
 diff.c                         |   33 +++++++++++++++++++++++++--------
 diff.h                         |    1 +
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9ecc1d7..0d3dccc 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -211,5 +211,14 @@ endif::git-format-patch[]
 --no-ext-diff::
 	Disallow external diff drivers.
 
+--src-prefix <prefix>::
+	Show the given source prefix instead of "a/".
+
+--dst-prefix <prefix>::
+	Show the given destination prefix instead of "b/".
+
+--no-prefix::
+	Do not show any source or destination prefix.
+
 For more detailed explanation on these common options, see also
 link:diffcore.html[diffcore documentation].
diff --git a/diff.c b/diff.c
index e26584c..43f62d8 100644
--- a/diff.c
+++ b/diff.c
@@ -290,9 +290,10 @@ static void emit_rewrite_diff(const char *name_a,
 			      const char *name_b,
 			      struct diff_filespec *one,
 			      struct diff_filespec *two,
-			      int color_diff)
+			      struct diff_options *o)
 {
 	int lc_a, lc_b;
+	int color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 	const char *name_a_tab, *name_b_tab;
 	const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO);
 	const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO);
@@ -309,9 +310,9 @@ static void emit_rewrite_diff(const char *name_a,
 	diff_populate_filespec(two, 0);
 	lc_a = count_lines(one->data, one->size);
 	lc_b = count_lines(two->data, two->size);
-	printf("%s--- a/%s%s%s\n%s+++ b/%s%s%s\n%s@@ -",
-	       metainfo, name_a, name_a_tab, reset,
-	       metainfo, name_b, name_b_tab, reset, fraginfo);
+	printf("%s--- %s%s%s%s\n%s+++ %s%s%s%s\n%s@@ -",
+	       metainfo, o->a_prefix, name_a, name_a_tab, reset,
+	       metainfo, o->b_prefix, name_b, name_b_tab, reset, fraginfo);
 	print_line_count(lc_a);
 	printf(" +");
 	print_line_count(lc_b);
@@ -1212,8 +1213,8 @@ static void builtin_diff(const char *name_a,
 	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
 	const char *reset = diff_get_color_opt(o, DIFF_RESET);
 
-	a_one = quote_two("a/", name_a + (*name_a == '/'));
-	b_two = quote_two("b/", name_b + (*name_b == '/'));
+	a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
+	b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
 	printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
@@ -1242,8 +1243,7 @@ static void builtin_diff(const char *name_a,
 		if ((one->mode ^ two->mode) & S_IFMT)
 			goto free_ab_and_return;
 		if (complete_rewrite) {
-			emit_rewrite_diff(name_a, name_b, one, two,
-					DIFF_OPT_TST(o, COLOR_DIFF));
+			emit_rewrite_diff(name_a, name_b, one, two, o);
 			o->found_changes = 1;
 			goto free_ab_and_return;
 		}
@@ -2020,6 +2020,9 @@ void diff_setup(struct diff_options *options)
 	else
 		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
+
+	options->a_prefix = "a/";
+	options->b_prefix = "b/";
 }
 
 int diff_setup_done(struct diff_options *options)
@@ -2291,6 +2294,20 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		else if (40 < options->abbrev)
 			options->abbrev = 40;
 	}
+	else if (!strcmp(arg, "--src-prefix")) {
+		if (ac < 2)
+			return error("--src-prefix needs a parameter");
+		options->a_prefix = arg + 1;
+		return 2;
+	}
+	else if (!strcmp(arg, "--dst-prefix")) {
+		if (ac < 2)
+			return error("--dst-prefix needs a parameter");
+		options->b_prefix = arg + 1;
+		return 2;
+	}
+	else if (!strcmp(arg, "--no-prefix"))
+		options->a_prefix = options->b_prefix = "";
 	else
 		return 0;
 	return 1;
diff --git a/diff.h b/diff.h
index 7e8000a..beccf85 100644
--- a/diff.h
+++ b/diff.h
@@ -69,6 +69,7 @@ struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
 	const char *single_follow;
+	const char *a_prefix, *b_prefix;
 	unsigned flags;
 	int context;
 	int break_opt;
-- 
1.5.4.rc0.70.g30f7

^ permalink raw reply related

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Johannes Schindelin @ 2007-12-18 18:03 UTC (permalink / raw)
  To: Sergei Organov
  Cc: Andreas Ericsson, Jakub Narebski, Sebastian Harl, Junio C Hamano,
	Benoit Sigoure, git
In-Reply-To: <871w9jrjdm.fsf@osv.gnss.ru>

Hi,

On Tue, 18 Dec 2007, Sergei Organov wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Tue, 18 Dec 2007, Andreas Ericsson wrote:
> >
> >> Jakub Narebski wrote:
> >>
> >> > Andreas Ericsson <ae@op5.se> writes:
> >> >
> >> > > My point is that it would be nice if all git commands that 
> >> > > actually manipulate objects (create/delete/modify) had a safe 
> >> > > default, and that experienced users such as yourself could endure 
> >> > > the insufferable agony of retraining your fingers to type five 
> >> > > more chars so that people won't have to get bitten by surprises.
> >> > 
> >> > Also for "git commit"?
> >> 
> >> git commit has a very safe default; It runs "git status" and exits.
> >
> > Not in my universe.  It starts an editor, and then commits what I 
> > staged.
> 
> ... allowing you to abort the operation by means of providing empty 
> commit message. On the other hand, "git stash" has immediate effect, so 
> it's somewhat more dangerous. Just to be picky, anyway.

Sorry, I have no time for this kind of discussions.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] git show <tag>: show the tagger
From: Johannes Schindelin @ 2007-12-18 18:01 UTC (permalink / raw)
  To: git, gitster


For commit objects, the Author is shown, so do the equivalent for
tag objects, too.

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

	I know, it's feature freeze period.  But this is arguably a 
	usability bug.

	Well, it's not _that_ serious, so it could wait for 2.0.0, too ;-)

 builtin-log.c |   39 ++++++++++++++++++++++++++++++++-------
 1 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index cc3cc90..c2ad863 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -244,7 +244,29 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 	return cmd_log_walk(&rev);
 }
 
-static int show_object(const unsigned char *sha1, int suppress_header)
+static void show_tagger(char *buf, int len, struct rev_info *rev)
+{
+	char *email_end, *p;
+	unsigned long date;
+	int tz;
+
+	email_end = memchr(buf, '>', len);
+	if (!email_end)
+		return;
+	p = ++email_end;
+	while (isspace(*p))
+		p++;
+	date = strtoul(p, &p, 10);
+	while (isspace(*p))
+		p++;
+	tz = (int)strtol(p, NULL, 10);
+	printf("Tagger: %.*s\nDate:   %s\n",
+			email_end - buf, buf,
+			show_date(date, tz, rev->date_mode));
+}
+
+static int show_object(const unsigned char *sha1, int show_tag_object,
+	struct rev_info *rev)
 {
 	unsigned long size;
 	enum object_type type;
@@ -254,11 +276,14 @@ static int show_object(const unsigned char *sha1, int suppress_header)
 	if (!buf)
 		return error("Could not read object %s", sha1_to_hex(sha1));
 
-	if (suppress_header)
-		while (offset < size && buf[offset++] != '\n') {
-			int new_offset = offset;
+	if (show_tag_object)
+		while (offset < size && buf[offset] != '\n') {
+			int new_offset = offset + 1;
 			while (new_offset < size && buf[new_offset++] != '\n')
 				; /* do nothing */
+			if (!prefixcmp(buf + offset, "tagger "))
+				show_tagger(buf + offset + 7,
+					    new_offset - offset - 7, rev);
 			offset = new_offset;
 		}
 
@@ -299,16 +324,16 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 		const char *name = objects[i].name;
 		switch (o->type) {
 		case OBJ_BLOB:
-			ret = show_object(o->sha1, 0);
+			ret = show_object(o->sha1, 0, NULL);
 			break;
 		case OBJ_TAG: {
 			struct tag *t = (struct tag *)o;
 
-			printf("%stag %s%s\n\n",
+			printf("%stag %s%s\n",
 					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					t->tag,
 					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
-			ret = show_object(o->sha1, 1);
+			ret = show_object(o->sha1, 1, &rev);
 			objects[i].item = (struct object *)t->tagged;
 			i--;
 			break;
-- 
1.5.4.rc0.70.g30f7

^ permalink raw reply related

* Re: [PATCH] Add format-patch option --no-name-prefix.
From: Pascal Obry @ 2007-12-18 17:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Linus Torvalds, Johannes Sixt, Pascal Obry, git
In-Reply-To: <Pine.LNX.4.64.0712181703560.23902@racer.site>

Johannes Schindelin a écrit :
> If this is preferred, please squash this:

Work fine for me.

We just need a consolidated patch with proper change log.

Thanks,
Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* Re: log/show: relative pathnames do not work in rev:path
From: Jakub Narebski @ 2007-12-18 17:50 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <20071218173321.GB2875@steel.home>

Alex Riesen <raa.lkml@gmail.com> writes:

> Noticed by a collegue of mine. Consider:
> 
>     $ cd $GIT/t
>     $ git show 570f32266:t/test-lib.sh    # works
>     $ git show 570f32266:test-lib.sh      # does not work
>     $ git show 570f32266:./test-lib.sh    # does not work
>     $ git show 570f32266:/t/test-lib.sh   # does not work
> 
> Considering that the relative path names work as filters (and many
> agreed on that being useful), it would be nice to allow relative
> pathnames in blob specifications for git-show and git-cat-file.
> 
> (besides the colon is a good delimiter, even tab-completion works with it)

If you think about it a bit, relative path names nor absolute
path names does and should not work.  570f32266:t/test-lib.sh
means path t/test-lib.sh staring from 570f32266^{tree}.  Where
you are in the filesystem is not important and matters not for
this syntax.  Besides if you access other branch file might be
not in filesystem (deleted file, or disjoint branch with separate
contents like 'todo' or 'html' branch in git.git repository).

Besides,

    $ git show 570f32266:t/test-lib.sh    # works
    $ git show cc5ac8b72:test-lib.sh      # also works

works... but 49d8bcd7a2df5 here is SHA-1 id of a 't/' tree
(shortened output of "git rev-parse 570f32266:t").

And I think that with bash-completion from contrib you complete
correct pathnames; don't rely on filesystem pathnames completion.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Sergei Organov @ 2007-12-18 17:40 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Andreas Ericsson, Jakub Narebski, Sebastian Harl, Junio C Hamano,
	Benoit Sigoure, git
In-Reply-To: <Pine.LNX.4.64.0712181610080.23902@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi,
>
> On Tue, 18 Dec 2007, Andreas Ericsson wrote:
>
>> Jakub Narebski wrote:
>>
>> > Andreas Ericsson <ae@op5.se> writes:
>> >
>> > > My point is that it would be nice if all git commands that actually 
>> > > manipulate objects (create/delete/modify) had a safe default, and 
>> > > that experienced users such as yourself could endure the 
>> > > insufferable agony of retraining your fingers to type five more 
>> > > chars so that people won't have to get bitten by surprises.
>> > 
>> > Also for "git commit"?
>> 
>> git commit has a very safe default; It runs "git status" and exits.
>
> Not in my universe.  It starts an editor, and then commits what I
> staged.

... allowing you to abort the operation by means of providing empty
commit message. On the other hand, "git stash" has immediate effect, so
it's somewhat more dangerous. Just to be picky, anyway.

-- 
Sergei.

^ permalink raw reply

* log/show: relative pathnames do not work in rev:path
From: Alex Riesen @ 2007-12-18 17:33 UTC (permalink / raw)
  To: git

Noticed by a collegue of mine. Consider:

    $ cd $GIT/t
    $ git show 570f32266:t/test-lib.sh    # works
    $ git show 570f32266:test-lib.sh      # does not work
    $ git show 570f32266:./test-lib.sh    # does not work
    $ git show 570f32266:/t/test-lib.sh   # does not work

Considering that the relative path names work as filters (and many
agreed on that being useful), it would be nice to allow relative
pathnames in blob specifications for git-show and git-cat-file.

(besides the colon is a good delimiter, even tab-completion works with it)

^ permalink raw reply

* Re: git-svn rebase issues (the commiter gets changed)
From: Kelvie Wong @ 2007-12-18 17:16 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20071216032523.GA17666@muzzle>

On Dec 15, 2007 7:27 PM, Eric Wong <normalperson@yhbt.net> wrote:
> Sorry for the late reply, I've been all over the place lately.
>
> Kelvie Wong <kelvie@ieee.org> wrote:
>
> > Just did it again this morning, with a clean test branch:
> >
> > kelvie@mudd (text-edit) qt $ git checkout -b test git-svn
> > Switched to a new branch "test"
> > kelvie@mudd (test) qt $ touch test
> > kelvie@mudd (test) qt $ git add test
> > kelvie@mudd (test) qt $ git commit -a -m 'Test!'
> > Created commit 05c4016: Test!
> >  0 files changed, 0 insertions(+), 0 deletions(-)
> >  create mode 100644 apps/qt/test
> > kelvie@mudd (test) qt $ git-cat-file commit HEAD
> > tree 867c0aa4c814542f0752b5d4c85fc96ba2279aac
> > parent 831ffbf25057ed30274d4216269c572cfce12184
> > author Kelvie Wong <Kelvie.Wong@safe.com> 1196352603 -0800
> > committer Kelvie Wong <Kelvie.Wong@safe.com> 1196352603 -0800
> >
> > Test!
> > kelvie@mudd (test) qt $ git svn rebase
> > <snip>
> > HEAD is now at 7319c2a... (svn commit message)
> > kelvie@mudd (test) qt $ git-cat-file commit HEAD
> > tree 4edacbd41af76ac243099467b33350887c0fb03d
> > parent 7319c2a810554aab25a688bcc2b16fc60529b59d
> > author Kelvie Wong <Kelvie.Wong@safe.com> 1196352603 -0800
> > committer ogibbins <ogibbins@e2d93294-a71b-0410-9dca-e2ea525a67c9>
> > 1196346907 +0000
> >
> > Test!
> > kelvie@mudd (test) qt $ git --version
> > git version 1.5.3.6.736.gb7f30
> >
> > And again, the committer of a local commit gets changed.
> >
> > Now, this part is more interesting:
> >
> > kelvie@mudd (test) qt $ git checkout working
> > Switched to branch "working"
> > kelvie@mudd (working) qt $ git svn rebase
> > <no fetch, just a checkout and rebase>
> >
> > And when I cat-file the commit, this time it's preserved.  Wild guess
> > here (this behaviour seems kind of inconsistent), but it has to do
> > with the transition between fetch and rebase?  Or is this a bug in
> > git-rebase somewhere?
>
> Ah, it looks like a bug in git-svn.  The following should fix it:
>
> From 84e99bffc72c10ec7db6d5ae7af6a795b75ef724 Mon Sep 17 00:00:00 2001
> From: Eric Wong <normalperson@yhbt.net>
> Date: Sat, 15 Dec 2007 19:08:22 -0800
> Subject: [PATCH] git-svn: avoid leaving leftover committer/author info in rebase
>
> We set the 6 environment variables for controlling
> committer/author email/name/time for every commit.
>
> We do this in the parent process to be passed to
> git-commit-tree, because open3() doesn't afford us the control
> of doing it only in the child process.  This means we leave them
> hanging around in the main process until the next revision comes
> around and all 6 environment variables are overwridden again.
>
> Unfortunately, for the last commit, leaving them hanging around
> means the git-rebase invocation will pick it up, rewriting the
> rebased commit with incorrect author information.  This should fix
> it.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>  git-svn.perl |   50 +++++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index d411a34..7cd62fc 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2052,18 +2052,16 @@ sub full_url {
>         $self->{url} . (length $self->{path} ? '/' . $self->{path} : '');
>  }
>
> -sub do_git_commit {
> -       my ($self, $log_entry) = @_;
> -       my $lr = $self->last_rev;
> -       if (defined $lr && $lr >= $log_entry->{revision}) {
> -               die "Last fetched revision of ", $self->refname,
> -                   " was r$lr, but we are about to fetch: ",
> -                   "r$log_entry->{revision}!\n";
> -       }
> -       if (my $c = $self->rev_map_get($log_entry->{revision})) {
> -               croak "$log_entry->{revision} = $c already exists! ",
> -                     "Why are we refetching it?\n";
> +
> +sub set_commit_header_env {
> +       my ($log_entry) = @_;
> +       my %env;
> +       foreach my $ned (qw/NAME EMAIL DATE/) {
> +               foreach my $ac (qw/AUTHOR COMMITTER/) {
> +                       $env{"GIT_${ac}_${ned}"} = $ENV{"GIT_${ac}_${ned}"};
> +               }
>         }
> +
>         $ENV{GIT_AUTHOR_NAME} = $log_entry->{name};
>         $ENV{GIT_AUTHOR_EMAIL} = $log_entry->{email};
>         $ENV{GIT_AUTHOR_DATE} = $ENV{GIT_COMMITTER_DATE} = $log_entry->{date};
> @@ -2074,7 +2072,36 @@ sub do_git_commit {
>         $ENV{GIT_COMMITTER_EMAIL} = (defined $log_entry->{commit_email})
>                                                 ? $log_entry->{commit_email}
>                                                 : $log_entry->{email};
> +       \%env;
> +}
>
> +sub restore_commit_header_env {
> +       my ($env) = @_;
> +       foreach my $ned (qw/NAME EMAIL DATE/) {
> +               foreach my $ac (qw/AUTHOR COMMITTER/) {
> +                       my $k = "GIT_${ac}_${ned}";
> +                       if (defined $env->{$k}) {
> +                               $ENV{$k} = $env->{$k};
> +                       } else {
> +                               delete $ENV{$k};
> +                       }
> +               }
> +       }
> +}
> +
> +sub do_git_commit {
> +       my ($self, $log_entry) = @_;
> +       my $lr = $self->last_rev;
> +       if (defined $lr && $lr >= $log_entry->{revision}) {
> +               die "Last fetched revision of ", $self->refname,
> +                   " was r$lr, but we are about to fetch: ",
> +                   "r$log_entry->{revision}!\n";
> +       }
> +       if (my $c = $self->rev_map_get($log_entry->{revision})) {
> +               croak "$log_entry->{revision} = $c already exists! ",
> +                     "Why are we refetching it?\n";
> +       }
> +       my $old_env = set_commit_header_env($log_entry);
>         my $tree = $log_entry->{tree};
>         if (!defined $tree) {
>                 $tree = $self->tmp_index_do(sub {
> @@ -2089,5 +2116,6 @@ sub do_git_commit {
>         defined(my $pid = open3(my $msg_fh, my $out_fh, '>&STDERR', @exec))
>                                                                    or croak $!;
>         print $msg_fh $log_entry->{log} or croak $!;
> +       restore_commit_header_env($old_env);
>         unless ($self->no_metadata) {
>                 print $msg_fh "\ngit-svn-id: $log_entry->{metadata}\n"
> \ No newline at end of file
> --
> Eric Wong
>

kelvie@mudd (test) qt $ git cat-file commit HEAD
tree 20adec5e5b186ea4360b5866586af7cfa5e3d88a
parent 15f3ab6b0c667b4900d5a7be0e80ef4e20b5bac2
author Kelvie Wong <Kelvie.Wong@safe.com> 1197990883 -0800
committer Kelvie Wong <Kelvie.Wong@safe.com> 1197990883 -0800

test
kelvie@mudd (test) qt $ git svn rebase
<snip>
HEAD is now at ebd86b2... SnappingFactory: Fixed bizarre omission from
previous commit for this bug. (PR#12838) <omg>
Applying test
kelvie@mudd (test) qt $ git cat-file commit HEAD
tree 2f242f83496f707758a863634a803f60a77c2786
parent ebd86b245505a18603fd114862eb97ea9609d7e1
author Kelvie Wong <Kelvie.Wong@safe.com> 1197990883 -0800
committer Kelvie Wong <Kelvie.Wong@safe.com> 1197998130 +0000

test
kelvie@mudd (test) qt $ git --version
git version 1.5.4.rc0.57.g4d99a
kelvie@mudd (test) qt $ git svn --version
Use of uninitialized value in pattern match (m//) at /usr/bin/git-svn line 202.
git-svn version 1.5.4.rc0.57.g4d99a (svn 1.4.4)
kelvie@mudd (test) qt $


Looks like it did it. Thanks for the fix.

-- 
Kelvie Wong

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox