Git development
 help / color / mirror / Atom feed
* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Andreas Ericsson @ 2007-12-18 15:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Sebastian Harl, Junio C Hamano, Benoit Sigoure, git
In-Reply-To: <Pine.LNX.4.64.0712181513060.23902@racer.site>

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 18 Dec 2007, Andreas Ericsson wrote:
> 
>> Johannes Schindelin wrote:
>>
>>> In the alternative, you could just scrap all those default actions, 
>>> showing synopses instead.  For all commands, including "git commit", 
>>> "git log", "git fetch", etc.
>> Like we do for the git wrapper, you mean? Yes, that would be one 
>> solution, although not a very good one for all commands.
> 
> Exactly.  Not a good one.
> 
>> It's probably not a bad idea for commands where the primary use is 
>> something else than producing visual output though, such as tag or 
>> branch, but those handle creation/deletion of stuff, so the default 
>> action for them is to list stuff of the kind they operate on. I fail to 
>> see why stash should be any different.
> 
> I also fail to see why stash should be any different.  And that's why I 
> expect it to have a default operation, which is -- you guessed it -- 
> "stash the changes!"
> 

Actually, I guessed "list the stashes".

> If I am not sure what I am about to do, there is -- wonder of wonders -- 
> the "-h" option!  And indeed:
> 
> 	$ git stash -h
> 	Usage: /home/gitte/bin/git-stash [  | save | list | show | apply | 
> 		clear | create ]
> 
> So what exactly was your point again?
> 

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.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Wincent Colaiuta @ 2007-12-18 15:28 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Johannes Schindelin, Sebastian Harl, Junio C Hamano,
	Benoit Sigoure, git
In-Reply-To: <4767D7A2.30703@op5.se>

El 18/12/2007, a las 15:22, Andreas Ericsson escribió:

> Johannes Schindelin wrote:
>> Hi,
>> On Tue, 18 Dec 2007, Sebastian Harl wrote:
>>> On Mon, Dec 17, 2007 at 04:31:12PM -0800, Junio C Hamano wrote:
>>>> But the original point by Sebastian hasn't been answered.  He  
>>>> wanted to make the command list the stash without arguments.
>>>>
>>>> This was discussed already in the early days of stash and there  
>>>> indeed was a suggestion to do so (I think I sided with that), but  
>>>> the users did not want it.  IIRC, the argument went like: "when I  
>>>> say 'stash', that is because I want a quick and immediate way to  
>>>> stash, and I do not want a list.  If I do not have to have a  
>>>> quick way, I would create a temporary commit on the current  
>>>> branch, or switch to a temporary branch and commit there."
>>> Well, "git stash save" is just five characters more - I really  
>>> don't see why this would be less comfortable (and for the really  
>>> lazy people there are still aliases...). On the other hand (if  
>>> "list" is the default), we'd get a more consistent interface which  
>>> imho is imho more important than typing five characters less.
>> It's more about what you're used to.  I had an alias named 'stash'  
>> long before it became a git command.  And now guess how _annoying_  
>> it would be to type "git stash<Return><Curse out loud at my  
>> mouse>git stash save<Return>".
>
> Not nearly as annoying as losing work because of it, and you obviously
> *know* what to do when you're done cursing, while clueless-newbie-X  
> just
> hops away and uses subversion.

There's not really any peril of losing work here, seeing as this  
should be a lossless round trip:

git stash
# oops! didn't mean to save
git stash apply

We could help clueless-newbie-X here by augmenting the save output:

   Saved "WIP on master: 8ed8a26... s"
   HEAD is now at 8ed8a26... s

As follows (or similar):

   Saved working directory and index state "WIP on master: 8ed8a26... s"
   (To restore them type "git stash apply")
   HEAD is now at 8ed8a26... s

ie. we explicitly tell them what was saved (their working directory  
and index state), and also how to get it back immediately if that's  
not what they meant to do. Something like this (no doubt will be  
whitespace-mangled because I'm pasting this into my email client, but  
it's just to demo the idea):

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..a2f3723 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -99,7 +99,8 @@ save_stash () {

         git update-ref -m "$stash_msg" $ref_stash $w_commit ||
                 die "Cannot save the current status"
-       printf >&2 'Saved "%s"\n' "$stash_msg"
+       printf >&2 'Saved working directory and index state "%s"\n'  
"$stash_msg"
+       echo >&2 '(To restore them type "git stash apply")'
  }

  have_stash () {

Cheers,
Wincent

^ permalink raw reply related

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Jakub Narebski @ 2007-12-18 15:40 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Johannes Schindelin, Sebastian Harl, Junio C Hamano,
	Benoit Sigoure, git
In-Reply-To: <4767E717.2060902@op5.se>

Andreas Ericsson <ae@op5.se> writes:
> Johannes Schindelin wrote:
>> On Tue, 18 Dec 2007, Andreas Ericsson wrote:
>>> Johannes Schindelin wrote:
>>>
>>>> In the alternative, you could just scrap all those default
>>>> actions, showing synopses instead.  For all commands, including
>>>> "git commit", "git log", "git fetch", etc.
>>>
>>> Like we do for the git wrapper, you mean? Yes, that would be one
>>> solution, although not a very good one for all commands.
>>
>> Exactly.  Not a good one.
>>
>>> It's probably not a bad idea for commands where the primary use is
>>> something else than producing visual output though, such as tag or
>>> branch, but those handle creation/deletion of stuff, so the default
>>> action for them is to list stuff of the kind they operate on. I
>>> fail to see why stash should be any different.
>>
>> I also fail to see why stash should be any different.  And that's why
>> I expect it to have a default operation, which is -- you guessed it -- 
>> "stash the changes!"
> 
> Actually, I guessed "list the stashes".
> 
>> If I am not sure what I am about to do, there is -- wonder of wonders -- 
>> the "-h" option!  And indeed:
>> 	$ git stash -h
>> 	Usage: /home/gitte/bin/git-stash [  | save | list | show |
>> apply | 		clear | create ]
>> So what exactly was your point again?
>>
> 
> 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"?

In my opinion _basic_ usage of git-stash is simply using it with
one stash only: "git stash" / "git unstash" (i.e. "git stash apply";
by the way this is one (beside "git view") use case for builtin
predefined aliases).  Using it with multiple stashes (only then
"git stash list" is needed) is advanced usage; and for advanced
usage longer form is preferred, I think.

"git branch", "git log" and "git remote" are horse of differenc color
because the _cannot_ function without name of branch/tag/remote given,
so hey provide "list" when no name was given.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] Add format-patch option --no-name-prefix.
From: Pascal Obry @ 2007-12-18 15:42 UTC (permalink / raw)
  To: git; +Cc: pascal

This option can be used to generate a patch file
where file names are relative to the Git root
directory. Such a patch can then be applied with
the standard patch tool using option -p0.

Signed-off-by: Pascal Obry <pascal@obry.net>
---
 Documentation/git-format-patch.txt |    6 +++++-
 builtin-log.c                      |    7 ++++++-
 diff.c                             |   10 ++++++++--
 diff.h                             |    1 +
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 6fb9429..5a642ad 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -15,7 +15,7 @@ SYNOPSIS
                    [-n | --numbered | -N | --no-numbered]
                    [--start-number <n>] [--numbered-files]
                    [--in-reply-to=Message-Id] [--suffix=.<sfx>]
-                   [--ignore-if-in-upstream]
+                   [--ignore-if-in-upstream] [--no-name-prefix]
                    [--subject-prefix=Subject-Prefix]
 		   [ <since> | <revision range> ]
 
@@ -90,6 +90,10 @@ include::diff-options.txt[]
 	without the default first line of the commit appended.
 	Mutually exclusive with the --stdout option.
 
+--no-name-prefix::
+	Generate a patch file that can be applied with a patch(1) -p0
+	from the Git root directory.
+
 -k|--keep-subject::
 	Do not strip/add '[PATCH]' from the first line of the
 	commit log message.
diff --git a/builtin-log.c b/builtin-log.c
index cc3cc90..36582bd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -599,6 +599,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int subject_prefix = 0;
 	int ignore_if_in_upstream = 0;
 	int thread = 0;
+	int no_name_prefix = 0;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
 	char *add_signoff = NULL;
@@ -636,6 +637,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		}
 		else if (!prefixcmp(argv[i], "--start-number="))
 			start_number = strtol(argv[i] + 15, NULL, 10);
+		else if (!prefixcmp(argv[i], "--no-name-prefix"))
+			no_name_prefix = 1;
 		else if (!strcmp(argv[i], "--numbered-files"))
 			numbered_files = 1;
 		else if (!strcmp(argv[i], "--start-number")) {
@@ -719,8 +722,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		die ("unrecognized argument: %s", argv[1]);
 
 	if (!rev.diffopt.output_format)
-		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
+		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH | DIFF_FORMAT_NAME_PREFIX;
 
+	if (no_name_prefix)
+		rev.diffopt.output_format &= ~DIFF_FORMAT_NAME_PREFIX;
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT))
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
diff --git a/diff.c b/diff.c
index e26584c..f07d9c0 100644
--- a/diff.c
+++ b/diff.c
@@ -1212,8 +1212,14 @@ 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 == '/'));
+	if (o->output_format & DIFF_FORMAT_NAME_PREFIX) {
+		a_one = quote_two("a/", name_a + (*name_a == '/'));
+		b_two = quote_two("b/", name_b + (*name_b == '/'));
+	}
+	else {
+		a_one = quote_two("", name_a + (*name_a == '/'));
+		b_two = quote_two("", 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);
diff --git a/diff.h b/diff.h
index 7e8000a..86458a3 100644
--- a/diff.h
+++ b/diff.h
@@ -30,6 +30,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_FORMAT_SUMMARY	0x0008
 #define DIFF_FORMAT_PATCH	0x0010
 #define DIFF_FORMAT_SHORTSTAT	0x0020
+#define DIFF_FORMAT_NAME_PREFIX 0x0040
 
 /* These override all above */
 #define DIFF_FORMAT_NAME	0x0100
-- 
1.5.4.rc0.56.g6fbe

^ permalink raw reply related

* Re: [PATCH] Add format-patch option --no-name-prefix.
From: Pascal Obry @ 2007-12-18 15:47 UTC (permalink / raw)
  To: git
In-Reply-To: <1197992574-3464-1-git-send-email-pascal@obry.net>


To give a bit of context about this patch. I need to send changeset to a
server by e-mail for testing purpose before committing. The server is
assuming that the patch can be applied with "patch -p0 < file" from the
repository root. The option --no-name-prefix does just that, removing
the leading 'a/' and 'b/'.

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: [PATCH] Add format-patch option --no-name-prefix.
From: Johannes Sixt @ 2007-12-18 15:54 UTC (permalink / raw)
  To: Pascal Obry; +Cc: git, pascal
In-Reply-To: <1197992574-3464-1-git-send-email-pascal@obry.net>

Pascal Obry schrieb:
> This option can be used to generate a patch file
> where file names are relative to the Git root
> directory. Such a patch can then be applied with
> the standard patch tool using option -p0.

While I've always wondered what the a/ and b/ prefixes were good for (and
I still do), I also wonder what's so different between typing

     patch -p0
and
     patch -p1

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... ??

-- Hannes

^ permalink raw reply

* Re: [PATCH] Add format-patch option --no-name-prefix.
From: Pascal Obry @ 2007-12-18 15:59 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pascal Obry, git
In-Reply-To: <4767ED52.9010004@viscovery.net>

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 :)

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: [PATCH] Add format-patch option --no-name-prefix.
From: Andreas Ericsson @ 2007-12-18 16:03 UTC (permalink / raw)
  To: Pascal Obry; +Cc: git, pascal
In-Reply-To: <1197992574-3464-1-git-send-email-pascal@obry.net>

Pascal Obry wrote:
>  	int thread = 0;
> +	int no_name_prefix = 0;

Do we not need no double negations, yes?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Andreas Ericsson @ 2007-12-18 16:06 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Johannes Schindelin, Sebastian Harl, Junio C Hamano,
	Benoit Sigoure, git
In-Reply-To: <m3lk7sovt0.fsf@roke.D-201>

Jakub Narebski wrote:
> Andreas Ericsson <ae@op5.se> writes:
>> Johannes Schindelin wrote:
>>> On Tue, 18 Dec 2007, Andreas Ericsson wrote:
>>>> Johannes Schindelin wrote:
>>>>
>>>>> In the alternative, you could just scrap all those default
>>>>> actions, showing synopses instead.  For all commands, including
>>>>> "git commit", "git log", "git fetch", etc.
>>>> Like we do for the git wrapper, you mean? Yes, that would be one
>>>> solution, although not a very good one for all commands.
>>> Exactly.  Not a good one.
>>>
>>>> It's probably not a bad idea for commands where the primary use is
>>>> something else than producing visual output though, such as tag or
>>>> branch, but those handle creation/deletion of stuff, so the default
>>>> action for them is to list stuff of the kind they operate on. I
>>>> fail to see why stash should be any different.
>>> I also fail to see why stash should be any different.  And that's why
>>> I expect it to have a default operation, which is -- you guessed it -- 
>>> "stash the changes!"
>> Actually, I guessed "list the stashes".
>>
>>> If I am not sure what I am about to do, there is -- wonder of wonders -- 
>>> the "-h" option!  And indeed:
>>> 	$ git stash -h
>>> 	Usage: /home/gitte/bin/git-stash [  | save | list | show |
>>> apply | 		clear | create ]
>>> So what exactly was your point again?
>>>
>> 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.

> In my opinion _basic_ usage of git-stash is simply using it with
> one stash only: "git stash" / "git unstash" (i.e. "git stash apply";
> by the way this is one (beside "git view") use case for builtin
> predefined aliases).  Using it with multiple stashes (only then
> "git stash list" is needed) is advanced usage; and for advanced
> usage longer form is preferred, I think.
> 

Perhaps. I'll stop quibbling about it. I don't care very deeply
about it anyway.

> "git branch", "git log" and "git remote" are horse of differenc color
> because the _cannot_ function without name of branch/tag/remote given,
> so hey provide "list" when no name was given.
> 

git stash takes a name too. It's optional though, and has caused any
number of source lines to be rewritten by grumbling authors who just
started to like git a little less because of it (yes, I know that has
been fixed, but it makes me look twice when discussing defaults for
git stash).

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: kha/safe and kha/experimental updated
From: Catalin Marinas @ 2007-12-18 16:09 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: David Kågedal, git
In-Reply-To: <20071218052115.GA13422@diana.vm.bytemark.co.uk>

Thanks again for maintaining these branches.

On 18/12/2007, Karl Hasselström <kha@treskal.com> wrote:
>   git://repo.or.cz/stgit/kha.git safe
>
> David Kågedal (17):
[...]
>       Simplify merge_recursive
>       Use the output from merge-recursive to list conflicts

I'll drop git.merge() entirely since it is only used by the 'sync'
command due to the performance issues I had in the past with rename
detection in git-merge-recursive. Hopefully, these are gone now.

I'll try to fix the automatic invocation of the interactive merger in
case of conflicts (it is only present in git.merge()).

> Karl Hasselström (24):
>       Fix "stg resolved" to work with new conflict representation

For some reason, the interactive resolving keeps invoking the merger.
I'll have a look.

>       "stg status --reset" is not needed anymore

I would keep this as an alias for 'git reset --hard' (see below as well).

>       Remove "stg add"
>       Remove "stg rm"
>       Remove "stg cp"

I plan to add a generic command for these kind of aliases. The reason
is that I don't really like mixing GIT and StGIT commands (I think at
some point I'll get confused and try to run stg commands with git).

>       Remove "stg resolved"

I'd like to keep this command. git-mergetool doesn't support the tool
I use (emacs + ediff and more stgit-specific file extensions like
current, patch etc.). I also don't find 'git add' to be meaningful for
marking a conflict as solved.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] Add format-patch option --no-name-prefix.
From: Pascal Obry @ 2007-12-18 16:11 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Pascal Obry, git
In-Reply-To: <4767EF5B.3010600@op5.se>

Andreas Ericsson a écrit :
> Pascal Obry wrote:
>>      int thread = 0;
>> +    int no_name_prefix = 0;
> 
> Do we not need no double negations, yes?

Not sure, looks clearer to use variable name corresponding to the option
name 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

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Johannes Schindelin @ 2007-12-18 16:11 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Jakub Narebski, Sebastian Harl, Junio C Hamano, Benoit Sigoure,
	git
In-Reply-To: <4767EFFA.1070909@op5.se>

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.

> > In my opinion _basic_ usage of git-stash is simply using it with one 
> > stash only: "git stash" / "git unstash" (i.e. "git stash apply"; by 
> > the way this is one (beside "git view") use case for builtin 
> > predefined aliases).  Using it with multiple stashes (only then "git 
> > stash list" is needed) is advanced usage; and for advanced usage 
> > longer form is preferred, I think.
> > 
> 
> Perhaps. I'll stop quibbling about it. I don't care very deeply about it 
> anyway.

Ah.  That explains why you made a case against the default operation ;-)

Ciao,
Dscho

^ permalink raw reply

* git-svn fix for broken symlinks
From: Sverre Johansen @ 2007-12-18 16:11 UTC (permalink / raw)
  To: git

Hi,

There is a bug in older versions of SVN that makes it possible to create
symlinks where the target is not the path to the file, but instead the
content of the target file. The bug is described here [0].

This breaks git-svn because it expects files which is marked as symlinks
to have the content "link: <filename>".

There was a thread about this back in July, resulting in this patch [1]. Is that
patch planned to be merged anytime soon? I'm using it, and it works great
for me.

[0]: http://subversion.tigris.org/issues/show_bug.cgi?id=2692
[1]: http://kerneltrap.org/mailarchive/git/2007/7/19/252025
-- 
           Sverre Johansen

^ permalink raw reply

* [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Johannes Schindelin @ 2007-12-18 16:21 UTC (permalink / raw)
  To: Pascal Obry; +Cc: git, pascal
In-Reply-To: <1197992574-3464-1-git-send-email-pascal@obry.net>


With the new option "--prefix=<prefix1>[:<prefix2>]" you can change
the shown prefix, or suppress it (by specifying the empty string).

Initial patch by Pascal Obry.

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

	On Tue, 18 Dec 2007, Pascal Obry wrote:

	> This option can be used to generate a patch file
	> where file names are relative to the Git root
	> directory. Such a patch can then be applied with
	> the standard patch tool using option -p0.

	How about this instead?  It is not much longer, but more 
	versatile, as you can actually change the prefix, and not only in 
	format-patch.

	Oh, and if somebody has a better idea for the name of the option, 
	I would appreciate your input.

 Documentation/diff-options.txt |    4 ++++
 diff.c                         |   29 +++++++++++++++++++++--------
 diff.h                         |    1 +
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9ecc1d7..672a2d0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -211,5 +211,9 @@ endif::git-format-patch[]
 --no-ext-diff::
 	Disallow external diff drivers.
 
+--prefix=<prefix1>[:<prefix2>]::
+	Show the given path prefixes instead of "a/" and "b/".  Leave
+	it empty to show no prefix at all.
+
 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..404ba91 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,16 @@ 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, "--prefix=")) {
+		char *colon = strchr(arg + 9, ':');
+		options->a_prefix = arg + 9;
+		if (colon) {
+			*colon = '\0';
+			options->b_prefix = colon + 1;
+		}
+		else
+			options->b_prefix = options->a_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: preventing a push
From: Linus Torvalds @ 2007-12-18 16:32 UTC (permalink / raw)
  To: Christoph Duelli; +Cc: git
In-Reply-To: <4767BDD8.9080404@melosgmbh.de>



On Tue, 18 Dec 2007, Christoph Duelli wrote:
>
> Is there a (recommended?) way to prevent accidental pushing (or pulling) from
> one repository into another (like the level command from bk days)?

I used BK for years, never knew about any level thing. I assume that was 
some way to introduce an "ordering" between repositories, where you could 
only push/pull in a controlled manner?

There's no obvious way to do exactly that, but the hooks git has may or 
may not be ok. For example, if you want to disallow pushing into some 
repository entirely (because you _only_ expect people to pull into it), 
you should be able to just make a "pre-receive" hook that always returns 
false. See Documentation/hooks.txt.

NOTE! There is no way to figure out what the pushing repository status is, 
which is why I say there is no way to do a "level"-equivalent thing 
(assuming I guessed what "level" does from the name). However, depending 
on how you allow people to access the machine, the hook obviously can look 
at things like $USER or other environment variables (ie you could make it 
look at what machine the user connected from etc).

But nothing really ever identifies the source repository (on a "git 
level") for a push: as far as git is concerned, all repositories are 
equal, and your hooks would invariably have to use non-git knowledge to 
figure out whether some operation should be allowed or not.

		Linus

^ permalink raw reply

* Re: kha/safe and kha/experimental updated
From: Jakub Narebski @ 2007-12-18 16:39 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Karl Hasselström, David Kågedal, git
In-Reply-To: <b0943d9e0712180809l4d2d01b8j32ab2a410885cc5e@mail.gmail.com>

"Catalin Marinas" <catalin.marinas@gmail.com> writes:

> Thanks again for maintaining these branches.
> 
> On 18/12/2007, Karl Hasselström <kha@treskal.com> wrote:
> >   git://repo.or.cz/stgit/kha.git safe

> >       Remove "stg resolved"
> 
> I'd like to keep this command. git-mergetool doesn't support the tool
> I use (emacs + ediff and more stgit-specific file extensions like
> current, patch etc.). I also don't find 'git add' to be meaningful for
> marking a conflict as solved.

I also would like to have this command kept (and shown in 'stg help'!).
Contrary to 'git add' it can check and add to index / update index
only for files with conflict; we have -r (ancestor|current|patched)
to choose one side, and we could add --check to check if there are
no conflict markers with files (useful with -a/--all).

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

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

Johannes Schindelin a écrit :
> With the new option "--prefix=<prefix1>[:<prefix2>]" you can change
> the shown prefix, or suppress it (by specifying the empty string).

Why not ? But do you have a motivation for this change ? I mean why
would you want to use a completely different prefix ?

My change was only for format-patch as this is used to build patch that
some other tools can process. If we find a sensible usage for your
prefix option I'm all for it as this solves also my problem. It is just
that I think all options must have at least one usage :)

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: [PATCH] Add format-patch option --no-name-prefix.
From: Linus Torvalds @ 2007-12-18 16:50 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Johannes Sixt, Pascal Obry, git
In-Reply-To: <4767EE6D.5070509@obry.net>



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 :)

Well, we'd also need something like that for doing recursive diffs of 
submodules (since then we'd want to do the internal diff with the 
submodule name appended to the prefix), so I do think this whole 
"--prefix" makes sense. 

But I obviously think the version by Dscho is better (exactly because it's 
*not* enough to just clear the name prefix entirely), although I think 
that one is broken too - using ':' to separate the prefixes is *not* 
acceptable, since ':' is very possibly part of the prefix.

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?

		Linus

^ permalink raw reply

* Re: kha/safe and kha/experimental updated
From: Catalin Marinas @ 2007-12-18 16:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Karl Hasselström, David Kågedal, git
In-Reply-To: <m3hcigot3h.fsf@roke.D-201>

On 18/12/2007, Jakub Narebski <jnareb@gmail.com> wrote:
> "Catalin Marinas" <catalin.marinas@gmail.com> writes:
>
> > Thanks again for maintaining these branches.
> >
> > On 18/12/2007, Karl Hasselström <kha@treskal.com> wrote:
> > >   git://repo.or.cz/stgit/kha.git safe
>
> > >       Remove "stg resolved"
> >
> > I'd like to keep this command. git-mergetool doesn't support the tool
> > I use (emacs + ediff and more stgit-specific file extensions like
> > current, patch etc.). I also don't find 'git add' to be meaningful for
> > marking a conflict as solved.
>
> I also would like to have this command kept (and shown in 'stg help'!).
> Contrary to 'git add' it can check and add to index / update index
> only for files with conflict; we have -r (ancestor|current|patched)
> to choose one side, and we could add --check to check if there are
> no conflict markers with files (useful with -a/--all).

I'd also like to re-add the stgit.keeporig option and additional
functionality so that the *.{ancestor,current,patched} can be left in
the working tree. Some people might use them when manually fixing
conflicts (I have a look at them from time to time when the emacs +
ediff shows a hard to understand conflict).

-- 
Catalin

^ permalink raw reply

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

Linus Torvalds a écrit :
>   --src-prefix=<string>		// default "a/"
>   --dst-prefix=<string>		// default "b/"
>   --no-prefix			// shorthand for --src-prefix="" --dst-prefix=""
> 
> would work for everybody?

Fine with 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

* Re: preventing a push
From: Christoph Duelli @ 2007-12-18 16:53 UTC (permalink / raw)
  To: Linus Torvalds, git
In-Reply-To: <alpine.LFD.0.9999.0712180819480.21557@woody.linux-foundation.org>

Linus Torvalds schrieb:
> 
> On Tue, 18 Dec 2007, Christoph Duelli wrote:
>> Is there a (recommended?) way to prevent accidental pushing (or pulling) from
>> one repository into another (like the level command from bk days)?
> 
> I used BK for years, never knew about any level thing. I assume that was 
> some way to introduce an "ordering" between repositories, where you could 
> only push/pull in a controlled manner?
Indeed, you can not move changes to a repo with a lower level.

> There's no obvious way to do exactly that, but the hooks git has may or 
> may not be ok. For example, if you want to disallow pushing into some 
> repository entirely (because you _only_ expect people to pull into it), 
> you should be able to just make a "pre-receive" hook that always returns 
> false. See Documentation/hooks.txt.
Ok, I will give hooks a try here.

> NOTE! There is no way to figure out what the pushing repository status is, 
> which is why I say there is no way to do a "level"-equivalent thing 
> (assuming I guessed what "level" does from the name). However, depending 
> on how you allow people to access the machine, the hook obviously can look 
> at things like $USER or other environment variables (ie you could make it 
> look at what machine the user connected from etc).
> 
> But nothing really ever identifies the source repository (on a "git 
> level") for a push: as far as git is concerned, all repositories are 
> equal, and your hooks would invariably have to use non-git knowledge to 
> figure out whether some operation should be allowed or not.
Perhaps it would be more git-like to use branches here. An accidental 
merge from a branch to another is probably less likely than an 
accidental push.

Thanks, and best regards

Christoph

^ permalink raw reply

* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Johannes Schindelin @ 2007-12-18 16:58 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Pascal Obry, git
In-Reply-To: <4767F935.8060207@obry.net>

Hi,

On Tue, 18 Dec 2007, Pascal Obry wrote:

> Johannes Schindelin a ?crit :
> > With the new option "--prefix=<prefix1>[:<prefix2>]" you can change 
> > the shown prefix, or suppress it (by specifying the empty string).
> 
> Why not ? But do you have a motivation for this change ? I mean why 
> would you want to use a completely different prefix ?

I vaguely remember that somebody once asked for something a la GNU patch's 
-B option.

Besides, why restrict ourselves?  I mean, really, my patch only adds 6/4 
added/removed lines relative to your patch (part of which stems from the 
fact that I did not forget the "diff --git" line).  Why not take the added 
value virtually for free?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/"
From: Matthieu Moy @ 2007-12-18 17:02 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Johannes Schindelin, Pascal Obry, git
In-Reply-To: <4767F935.8060207@obry.net>

Pascal Obry <pascal@obry.net> writes:

> Johannes Schindelin a écrit :
>> With the new option "--prefix=<prefix1>[:<prefix2>]" you can change
>> the shown prefix, or suppress it (by specifying the empty string).
>
> Why not ? But do you have a motivation for this change ? I mean why
> would you want to use a completely different prefix ?

It can make sense when you send the patch to someone who might not
know the context in which you wrote the patch, and who's not using
git. Then

--- your-version/foo.c
+++ my-version/foo.c

can be more expressive than a/ and b/. Some people like to have orig/
and mod/ also.

I can live without --prefix=... option, but doing it general at once
is a good idea, since adding this backward-compatibly on top of your
patch would mean having several redundant options.

-- 
Matthieu

^ permalink raw reply

* Re: [PATCH] Add format-patch option --no-name-prefix.
From: Johannes Schindelin @ 2007-12-18 17:06 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>

Hi,

On Tue, 18 Dec 2007, Linus Torvalds wrote:

> But I obviously think the version by Dscho is better (exactly because it's 
> *not* enough to just clear the name prefix entirely), although I think 
> that one is broken too - using ':' to separate the prefixes is *not* 
> acceptable, since ':' is very possibly part of the prefix.
> 
> 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?

If this is preferred, please squash this:

-- snipsnap --

 Documentation/diff-options.txt |   11 ++++++++---
 diff.c                         |   24 ++++++++++++++----------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 672a2d0..0d3dccc 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -211,9 +211,14 @@ endif::git-format-patch[]
 --no-ext-diff::
 	Disallow external diff drivers.
 
---prefix=<prefix1>[:<prefix2>]::
-	Show the given path prefixes instead of "a/" and "b/".  Leave
-	it empty to show no prefix at all.
+--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 095bbb5..9bc5fea 100644
--- a/diff.c
+++ b/diff.c
@@ -2317,16 +2317,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, "--prefix=")) {
-		char *colon = strchr(arg + 9, ':');
-		options->a_prefix = arg + 9;
-		if (colon) {
-			*colon = '\0';
-			options->b_prefix = colon + 1;
-		}
-		else
-			options->b_prefix = options->a_prefix;
-	}
+	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;

^ permalink raw reply related

* 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