Git development
 help / color / mirror / Atom feed
* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21  4:40 UTC (permalink / raw)
  To: Kyle McMartin, Junio C Hamano, Git Mailing List, Raphael Assenat,
	Andrew Morton
  Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202009290.21557@woody.linux-foundation.org>



On Thu, 20 Dec 2007, Linus Torvalds wrote:
> 
> It only happened for a few files that had lots of repeated lines - so that 
> the diff could literally be done multiple different ways - and in fact, 
> the file that caused the problems really had a bogus commit that 
> duplicated *way* too much data, and caused lots of #define's to exist 
> twice.

Here's the example of this kind of behaviour: in the 2.6.26-rc5 tree the 
file drivers/video/mbx/reg_bits.h has the #defines for 

	/* DINTRS - Display Interrupt Status Register */
	/* DINTRE - Display Interrupt Enable Register */

duplicated twice due to commit ba282daa919f89c871780f344a71e5403a70b634 
("mbxfb: Improvements and new features") by Raphael Assenat mistakenly 
adding another copy of the same old set of defines that we already got 
added once before by commit fb137d5b7f2301f2717944322bba38039083c431 
("mbxfb: Add more registers bits access macros").

Now, that was a mistake - and one that probably happened because Rafael or 
more likely Andrew Morton used GNU patch with its insane defaults (which 
is to happily apply the same patch that adds things twice, because it 
doesn't really care if the context matches or not).

But what that kind of thing causes is that when you create a patch of the 
end result, it can show the now new duplicate lines two different (but 
equally valid) ways: it can show it as an addition of the _first_ set of 
lines, or it can show it as an addition of the _second_ set of lines. They 
are the same, after all.

Now, it doesn't really matter which way you choose to show it, although 
because of how "git diff" finds similarities, it tends to prefer to show 
the second set of identical lines as the "new" ones. Which is generally 
reasonable.

However, that interacted really badly with the new git logic that said 
that "if the two files end in the same sequence, just ignore the common 
tail of the file", because the latter copy of the identical lines would 
now show up as _part_ of that common tail, so the lines that the git diff 
machinery would normally like to show up as "new" did in fact end up being 
considered uninteresting, because they were part of an idential tail. 

So now "git diff" would happily pick _earlier_ lines as the new ones, and 
it would still be a conceptually valid diff, but because we had trimmed 
the tail of the file, that conceptually valid diff no longer had the 
expected shared context at the end.

And while it's a bit embarrassing, I'm really rather happy that both GNU 
patch and "git apply" actually refused to apply the patch. It may have 
been "conceptually correct" (ie it did really contain all of the changes!) 
but because it lacked the expected context it really wasn't a good patch. 

That was a rather long-winded explanation of what happened, mainly because 
it was all very unexpected to me, and I had personally mistakenly thought 
the git optimization was perfectly valid and actually had to go through 
the end result to see what was going on.

Anyway, the diff on kernel.org should be all ok now, and mirrored out too.

			Linus

^ permalink raw reply

* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21  4:22 UTC (permalink / raw)
  To: Kyle McMartin, Junio C Hamano, Git Mailing List; +Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712201937470.21557@woody.linux-foundation.org>



On Thu, 20 Dec 2007, Linus Torvalds wrote:
> 
> The tar-ball and the git archive itself is fine, but yes, the diff from 
> 2.6.23 to 2.6.24-rc6 is bad. It's the "trim_common_tail()" optimization 
> that has caused way too much pain.

Very interesting breakage. The patch was actually "correct" in a (rather 
limited) technical sense, but the context at the end was missing because 
while the trim_common_tail() code made sure to keep enough common context 
to allow a valid diff to be generated, the diff machinery itself could 
decide that it could generate the diff differently than the "obvious" 
solution.

It only happened for a few files that had lots of repeated lines - so that 
the diff could literally be done multiple different ways - and in fact, 
the file that caused the problems really had a bogus commit that 
duplicated *way* too much data, and caused lots of #define's to exist 
twice.

But the sad fact appears that the git optimization (which is very 
important for "git blame", which needs no context), is only really valid 
for that one case where we really don't need any context.

I uploaded a fixed patch. And here's the git patch to avoid this 
optimization when there is context.

		Linus

---
 xdiff-interface.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 9ee877c..0b7e057 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -110,22 +110,22 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
 {
 	const int blk = 1024;
-	long trimmed = 0, recovered = 0;
+	long trimmed = 0;
 	char *ap = a->ptr + a->size;
 	char *bp = b->ptr + b->size;
 	long smaller = (a->size < b->size) ? a->size : b->size;
 
+	if (ctx)
+		return;
+
 	while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
 		trimmed += blk;
 		ap -= blk;
 		bp -= blk;
 	}
 
-	while (recovered < trimmed && 0 <= ctx)
-		if (ap[recovered++] == '\n')
-			ctx--;
-	a->size -= (trimmed - recovered);
-	b->size -= (trimmed - recovered);
+	a->size -= trimmed;
+	b->size -= trimmed;
 }
 
 int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *xecb)

^ permalink raw reply related

* Re: Linux 2.6.24-rc6
From: Kyle McMartin @ 2007-12-21  3:50 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Git Mailing List, Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712201937470.21557@woody.linux-foundation.org>

On Thu, Dec 20, 2007 at 07:49:05PM -0800, Linus Torvalds wrote:
> 
> 
> On Thu, 20 Dec 2007, Kyle McMartin wrote:
> > 
> > I think I see the problem, it's lack of context in the diff,
> 
> No, the problem is that "git diff" is apparently broken by a recent 
> optimization. The diff is simply broken.
> 
> The tar-ball and the git archive itself is fine, but yes, the diff from 
> 2.6.23 to 2.6.24-rc6 is bad. It's the "trim_common_tail()" optimization 
> that has caused way too much pain.
> 
> Sorry about that, I'll fix it up asap.
> 

no biggie, thanks!

cheers, Kyle

^ permalink raw reply

* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21  3:49 UTC (permalink / raw)
  To: Kyle McMartin, Junio C Hamano, Git Mailing List; +Cc: Linux Kernel Mailing List
In-Reply-To: <20071221030152.GC8535@fattire.cabal.ca>



On Thu, 20 Dec 2007, Kyle McMartin wrote:
> 
> I think I see the problem, it's lack of context in the diff,

No, the problem is that "git diff" is apparently broken by a recent 
optimization. The diff is simply broken.

The tar-ball and the git archive itself is fine, but yes, the diff from 
2.6.23 to 2.6.24-rc6 is bad. It's the "trim_common_tail()" optimization 
that has caused way too much pain.

Sorry about that, I'll fix it up asap.

		Linus

^ permalink raw reply

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-21  0:49 UTC (permalink / raw)
  To: Bj?rn Steinbrink; +Cc: Alex Riesen, git
In-Reply-To: <20071221001449.GA10607@atjola.homenet>



On Fri, 21 Dec 2007, Bj?rn Steinbrink wrote:
> 
> Hm, this is a bit more intrusive, but should catch most cases.
> 
> At the top of the comments in the commit message template add:
> #GIT CUT HERE
> (And adjust the descriptive text)

Ouch. I'd personally hate to see something like that at the top and then 
have to save it. I always add my text to the top of the message, and all 
the pre-made messages for me are at the top (ie the kinds you get with 
"git commit --amend", where the top of the thing is the old message).

That said, I might well agree with this approach if we made the marker 
line be the *last* line of the message, ie make it be something that ends 
with (ignoring empty whitespace at the end, of course, since those are 
invisible in most editors):

	# Remove this line to keep all comment lines

or something like that.

That still keeps the question about whitespace cleanups. But if it's just 
about whitespace or no whitespace, then a simple "--verbatim" flag would 
work.

			Linus

^ permalink raw reply

* Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR
From: Steven Grimm @ 2007-12-21  0:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Luciano Rocha, git
In-Reply-To: <7vhcidovxt.fsf@gitster.siamese.dyndns.org>

On Dec 20, 2007, at 2:14 PM, Junio C Hamano wrote:
> I think Steven stopped after you poked holes in that patch.

Nah, just entered a particularly busy period in my day job and haven't  
had time to do much more git stuff than occasionally skim the mailing  
list. I do plan to revisit that at some point unless the patch in your  
mail ends up being what we go with. (It seems like a sensible approach  
to me.)

-Steve

^ permalink raw reply

* 1.5.4-rc2 plans
From: Junio C Hamano @ 2007-12-21  0:32 UTC (permalink / raw)
  To: git

I've tagged -rc1 last night.  The changes are mostly fixes.  There are
some remaining issues I'd like to see fixed/decided before 1.5.4.

One important issue is to identify and fix regressions since 1.5.3
series.  No "rewrite scripted git-foo completely in C" can be regression
free, and we had quite a few internal changes during 1.5.4 cycle (not
just rewrite to C, but C level uses new and improved API such as strbuf
and parse-options).  Currently I am aware of these regressions:

 * handling of options, "--abbrev 10 HEAD", "--abbrev=10 HEAD" and
   "--abbrev HEAD".  The last one does not work for commands that use
   parse-options.  Pierre is on top of this, I hope.

 * handling of EDITOR in git commit and git tag is currently different.
   It expects "vi" not "vi --some-funny-option".  I sent out a
   for-discussion patch after seeing Steven's and Luciano's.

but I am sure there must be others that we haven't even identified.

Also there have been handful usability issues, which can be solved
without incompatible changes:

 * Should "git stash" stop doing anything useful?  I think the patch
   from Nana today may be a reasonable compromise, although I still
   think fundamentally different behaviour for the same command
   configurable per-user is not very nice (we have precedent in "git
   clean" already, though, but "git clean" is inherently dangerous
   command, and "git stash" is much more useful and the issue impacts
   more people).

 * Introduction of "<tree>:./path" (Dscho).  I could be talked into
   accepting the patch if it is useful to people who live deep within
   subdirectories.

 * Making commit log message cleansing optionally less aggressive.  I do
   not think we have seen the end of the thread yet, but I think Linus's
   "three cleansing levels" approach is on the right track.

I'd like to see the above resolved and hopefully 12 other regressions
identified and fixed before the end of the year, when -rc2 can hopefully
happen.

The following technical issues have been raised but not resolved and I
do not expect the resolution to be part of 1.5.4.

 * Authenticated pserver (Ævar Arnfjörð Bjarmason).  It needs security
   audit of the code and also the password storage needs to be decided.
   This can wait post 1.5.4.

 * Threaded "repack -a -d -f" when having a verify tight pack suffers
   from massive malloc(3) memory fragmentation, which we cannot do much
   about.

 * Rebase using "format-patch | am" machinery has issues when dealing
   with a mostly-text change that includes NUL (or anything you cannot
   e-mail patch for).  The workaround is "rebase -m" which unfortunatly
   is slow.  I am working on rewriting cherry-pick whenever I find time
   to address it, though.

 * Handling of trailing blank lines does not mesh well with the way diff
   and apply whitespace logic is done.

^ permalink raw reply

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Björn Steinbrink @ 2007-12-21  0:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Alex Riesen, git
In-Reply-To: <alpine.LFD.0.9999.0712201545450.21557@woody.linux-foundation.org>

On 2007.12.20 15:55:18 -0800, Linus Torvalds wrote:
> 
> 
> On Fri, 21 Dec 2007, Alex Riesen wrote:
> > 
> > Yes, I afraid I need both. I use "git commit -t" almost (submission in
> > perforce takes careful planning) every day. I also would like to keep
> > the empty leading and trailing lines (perforce default GUI P4Win does
> > not show them, but our scripts which check the descriptions will test
> > the description text according to template which does have trailing
> > empty lines).
> 
> Hmm. I think your updated patch was pretty good, although I still think it 
> could be improved a bit. In particular, thinking more about it, I think we 
> have more than an "on/off" switch - we really have three cases:
> 
>  a) strip whitespace _and_ comments
>  b) strip unnecessary whitespace only
>  c) leave things _totally_ alone
> 
> and on top of that we also have the issue of an editor.
> 
> So my patch basically said that in the absense of an editor, we'll still 
> clean up whitespace, but not comments (ie "no_edit" implies doing (b) 
> rather than (b)), while your patch basically results in (c) regardless of 
> whether we run an editor or not.
> 
> But that still leaves one case: do we ever want to do (b) even *if* we use 
> an editor? There's another possible choice: our old behaviour of (a) in 
> the presense of an editor is now gone.
> 
> Now, that last choice (ie "case (a) without an editor") is not only 
> unlikely to be anything people want to do anyway, it's also easy enough to 
> do by just using "git stripspace -s" on whatever non-editor thing you feed 
> to "git commit", so I don't think we need to worry about that one.
> 
> But the "maybe you want to run an editor, and you _do_ want case (b)" 
> sounds like a case that is not at all unlikely. I could easily see the 
> case where you want to have a template that uses '#', and *despite* that 
> you want to (a) allow the user to edit things _and_ (b) clean up 
> whitespace too.
> 
> So I'd almost suggest you make the "--verbatim" flag a three-way switch, 
> to allow "totally verbatim" (leave everything in place) and a "don't touch 
> comments" (just fix up whitespace) mode.
> 
> Hmm? Does that make sense to you?

Hm, this is a bit more intrusive, but should catch most cases.

At the top of the comments in the commit message template add:
#GIT CUT HERE
(And adjust the descriptive text)

That line hopefully being uncommon enough to not affect any existing
stuff.

If that line is present, comment lines above it are kept, otherwise
they're removed. Whitespace is always fixed(?).

Results:
 - git commit -m "# Foo and bar"
   * keeps the comment, looks like the expected thing

 - git commit with editor
   * Comments that are manually added are kept
   * For the (probably seldom) case, that you want manually added
     comments to be stripped, you can still remove the "#GIT CUR HERE"
     line

 - git commit with template
   * The existing templates probably won't have a "#GIT CUT HERE" line,
     so we're backwards compatible
   * Templates that want to keep the comments can simple get a "#GIT CUT
     HERE" line at the end and Just Work, regardless of whether or not
     you forget to pass --verbatim.


Hmm?

thanks,
Björn

^ permalink raw reply

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-21  0:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <7vlk7plydv.fsf@gitster.siamese.dyndns.org>



On Thu, 20 Dec 2007, Junio C Hamano wrote:
> 
> But I wonder if people are using a workflow like this:
> 
> 	$ cp $company_template my_message
>         $ edit my_message
>         $ git commit -F my_message

Yes. See the email I just sent to Alex.

I think that the above kind of workflow is trivial to support with the 
"new" semantics (ie the patch under discussion), by just rewriting that 
last step as

	$ git stripspace -s < my_message | git-commit -F -

so in that sense, we don't really "lose" anything, although I do agree 
that backwards compatibility (in case somebody actually does this!) 
suffers.

But for the case of actually using the built-in editing capability, we 
don't have that choice, so I'd argue that regardless, we should make the 
"--verbatim" switch be a three-way choice between (a) not touching things 
at all, (b) touching up just whitespace, or (c) doing the full enchilada.

And if we introduce such a flag, then I think we can make the *default* 
(in the absense of an explicit flag) be something like

	if (no_edit)
		default = touch_up_just_whitespace;
	else
		default = whole_enchilada

and obviously it could also be a configuration option.

That way, you could always get the existing behaviour with

	git commit --verbatim=full-enchilada -F my_message

(yeah, bad name - I'm not seriously suggesting it be called 
"full-enchilada", and I'm also not sure the flag should be called 
"--verbatim" any more if it has choices, it's more likely that we should 
call it something like "--cleanup={verbatim,whitespace,strip}" or 
something like that.

			Linus

^ permalink raw reply

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-20 23:55 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <20071220233324.GB3052@steel.home>



On Fri, 21 Dec 2007, Alex Riesen wrote:
> 
> Yes, I afraid I need both. I use "git commit -t" almost (submission in
> perforce takes careful planning) every day. I also would like to keep
> the empty leading and trailing lines (perforce default GUI P4Win does
> not show them, but our scripts which check the descriptions will test
> the description text according to template which does have trailing
> empty lines).

Hmm. I think your updated patch was pretty good, although I still think it 
could be improved a bit. In particular, thinking more about it, I think we 
have more than an "on/off" switch - we really have three cases:

 a) strip whitespace _and_ comments
 b) strip unnecessary whitespace only
 c) leave things _totally_ alone

and on top of that we also have the issue of an editor.

So my patch basically said that in the absense of an editor, we'll still 
clean up whitespace, but not comments (ie "no_edit" implies doing (b) 
rather than (b)), while your patch basically results in (c) regardless of 
whether we run an editor or not.

But that still leaves one case: do we ever want to do (b) even *if* we use 
an editor? There's another possible choice: our old behaviour of (a) in 
the presense of an editor is now gone.

Now, that last choice (ie "case (a) without an editor") is not only 
unlikely to be anything people want to do anyway, it's also easy enough to 
do by just using "git stripspace -s" on whatever non-editor thing you feed 
to "git commit", so I don't think we need to worry about that one.

But the "maybe you want to run an editor, and you _do_ want case (b)" 
sounds like a case that is not at all unlikely. I could easily see the 
case where you want to have a template that uses '#', and *despite* that 
you want to (a) allow the user to edit things _and_ (b) clean up 
whitespace too.

So I'd almost suggest you make the "--verbatim" flag a three-way switch, 
to allow "totally verbatim" (leave everything in place) and a "don't touch 
comments" (just fix up whitespace) mode.

Hmm? Does that make sense to you?

			Linus

^ permalink raw reply

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Junio C Hamano @ 2007-12-20 23:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Alex Riesen, git
In-Reply-To: <alpine.LFD.0.9999.0712201324270.21557@woody.linux-foundation.org>

I said in the log message when applying your version:

    We even stripped "#" lines from messages given this way:

        git commit -m "# Message starting with a hash-mark"

    which was argurably a bug.

But I wonder if people are using a workflow like this:

	$ cp $company_template my_message
        $ edit my_message
        $ git commit -F my_message

To them, this change is a regression, as $company_template may have had
insn like "# Here you describe your changes..." and the workflow relied
on the current behaviour of stripping them.

^ permalink raw reply

* [PATCH] Fix thinko in checking for commit message emptiness
From: Alex Riesen @ 2007-12-20 23:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Junio C Hamano
In-Reply-To: <20071220233539.GC3052@steel.home>

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Sorry. It must be late. It it must have been late yesterday
and still is today.

 builtin-commit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 1356d20..eae7661 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -436,7 +436,7 @@ static int message_is_empty(struct strbuf *sb, int start)
 	int eol, i;
 
 	if (verbatim_message && sb->len)
-		return 1;
+		return 0;
 
 	/* See if the template is just a prefix of the message. */
 	strbuf_init(&tmpl, 0);
-- 
1.5.4.rc1.33.gbd32b

^ permalink raw reply related

* Re: [PATCH] Only filter "#" comments from commits if the editor is used
From: Alex Riesen @ 2007-12-20 23:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vtzmdlyva.fsf@gitster.siamese.dyndns.org>

Junio C Hamano, Fri, Dec 21, 2007 00:39:37 +0100:
> Thanks.

Please hold on. The test fails.

^ permalink raw reply

* Re: [PATCH] Only filter "#" comments from commits if the editor is used
From: Junio C Hamano @ 2007-12-20 23:43 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Linus Torvalds, git
In-Reply-To: <20071220233714.GD3052@steel.home>

I said thanks a bit too early.  That is on top of verbatim, which I have
doubts about putting in 1.5.4.  I on the other hand think Linus's is a
pure bugfix.

Will tweak and apply, thanks.

^ permalink raw reply

* Re: [PATCH] Only filter "#" comments from commits if the editor is used
From: Junio C Hamano @ 2007-12-20 23:39 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Linus Torvalds, git
In-Reply-To: <20071220233714.GD3052@steel.home>

Thanks.

^ permalink raw reply

* [PATCH] Only filter "#" comments from commits if the editor is used
From: Alex Riesen @ 2007-12-20 23:37 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <20071220233539.GC3052@steel.home>

Originally-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 builtin-commit.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 3127247..1356d20 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -441,7 +441,7 @@ static int message_is_empty(struct strbuf *sb, int start)
 	/* See if the template is just a prefix of the message. */
 	strbuf_init(&tmpl, 0);
 	if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {
-		stripspace(&tmpl, 1);
+		stripspace(&tmpl, !no_edit);
 		if (start + tmpl.len <= sb->len &&
 		    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
 			start += tmpl.len;
@@ -821,7 +821,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		strbuf_setlen(&sb, p - sb.buf + 1);
 
 	if (!verbatim_message)
-		stripspace(&sb, 1);
+		stripspace(&sb, !no_edit);
 	if (sb.len < header_len || message_is_empty(&sb, header_len)) {
 		rollback_index_files();
 		die("no commit message?  aborting commit.");
-- 
1.5.4.rc1.33.gbd32b

^ permalink raw reply related

* [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Alex Riesen @ 2007-12-20 23:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <20071220233324.GB3052@steel.home>

Sometimes the message just have to be the way user wants it.
For instance, a template can contain "#" characters, or the message
must be kept as close to its original source as possible for reimport
reasons. Or maybe the user just copied a shell script including its
comments into the commit message for future reference.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Updated patch. It conflicts with yours a bit. Will update it

 Documentation/git-commit.txt |    7 ++++++-
 builtin-commit.c             |   20 ++++++++++++++------
 t/t7502-commit.sh            |   18 ++++++++++++++++++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 4261384..862543f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git-commit' [-a | --interactive] [-s] [-v] [-u]
 	   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]
 	   [--allow-empty] [--no-verify] [-e] [--author <author>]
-	   [--] [[-i | -o ]<file>...]
+	   [--verbatim] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------
@@ -95,6 +95,11 @@ OPTIONS
 	from making such a commit.  This option bypasses the safety, and
 	is primarily for use by foreign scm interface scripts.
 
+--verbatim::
+	Inhibits stripping of leading and trailing spaces,
+	empty lines and #commentary from the commit message.
+	Implies --allow-empty.
+
 -e|--edit::
 	The message taken from file with `-F`, command line with
 	`-m`, and from file with `-C` are usually used as the
diff --git a/builtin-commit.c b/builtin-commit.c
index 0a91013..3127247 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -47,6 +47,7 @@ static char *logfile, *force_author, *template_file;
 static char *edit_message, *use_message;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, untracked_files, no_verify, allow_empty;
+static int verbatim_message;
 
 static int no_edit, initial_commit, in_merge;
 const char *only_include_assumed;
@@ -88,6 +89,7 @@ static struct option builtin_commit_options[] = {
 	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
 	OPT_BOOLEAN(0, "untracked-files", &untracked_files, "show all untracked files"),
 	OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"),
+	OPT_BOOLEAN(0, "verbatim", &verbatim_message, "do not strip spaces and #comments from message"),
 
 	OPT_END()
 };
@@ -346,7 +348,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
 
-	stripspace(&sb, 0);
+	if (!verbatim_message)
+		stripspace(&sb, 0);
 
 	if (signoff) {
 		struct strbuf sob;
@@ -404,10 +407,11 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 			"#\n",
 			git_path("MERGE_HEAD"));
 
-	fprintf(fp,
-		"\n"
-		"# Please enter the commit message for your changes.\n"
-		"# (Comment lines starting with '#' will not be included)\n");
+	if (!verbatim_message)
+		fprintf(fp,
+			"\n"
+			"# Please enter the commit message for your changes.\n"
+			"# (Comment lines starting with '#' will not be included)\n");
 	if (only_include_assumed)
 		fprintf(fp, "# %s\n", only_include_assumed);
 
@@ -431,6 +435,9 @@ static int message_is_empty(struct strbuf *sb, int start)
 	const char *nl;
 	int eol, i;
 
+	if (verbatim_message && sb->len)
+		return 1;
+
 	/* See if the template is just a prefix of the message. */
 	strbuf_init(&tmpl, 0);
 	if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {
@@ -813,7 +820,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (p != NULL)
 		strbuf_setlen(&sb, p - sb.buf + 1);
 
-	stripspace(&sb, 1);
+	if (!verbatim_message)
+		stripspace(&sb, 1);
 	if (sb.len < header_len || message_is_empty(&sb, header_len)) {
 		rollback_index_files();
 		die("no commit message?  aborting commit.");
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 21ac785..d549728 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -89,4 +89,22 @@ test_expect_success 'verbose' '
 
 '
 
+test_expect_success 'verbatim commit messages' '
+
+	echo >>negative &&
+	{ echo;echo "# text";echo; } >expect &&
+	git commit --verbatim -t expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+	diff -u expect actual &&
+	echo >>negative &&
+	git commit --verbatim -F expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual &&
+	echo >>negative &&
+	git commit --verbatim -m "$(cat expect)" -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual
+
+'
+
 test_done
-- 
1.5.4.rc1.33.gbd32b

^ permalink raw reply related

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Alex Riesen @ 2007-12-20 23:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <alpine.LFD.0.9999.0712201324270.21557@woody.linux-foundation.org>

Linus Torvalds, Thu, Dec 20, 2007 22:40:13 +0100:
> On Thu, 20 Dec 2007, Alex Riesen wrote:
> > 
> > I just happen to have a corporate template (for perforce messages, I
> > reuse it my git mirror repo) which contains "#" and at least one time
> > lost my bash comments in a commit.
> 
> I think that this is a real bug, but I don't think this is something that 
> we should add a flag for.
> 
> Basically, I don't think we should really strip lines starting with '#' 
> unless *we* added them. In particular, I don't think we should strip them 
> at all unless we're running the editor.

Right

> That may be enough for your case, although it still does leave the "use 
> editor on a template thing", so if that is your usage scenario, I guess we 
> still do need a flag for it.

Yes, I afraid I need both. I use "git commit -t" almost (submission in
perforce takes careful planning) every day. I also would like to keep
the empty leading and trailing lines (perforce default GUI P4Win does
not show them, but our scripts which check the descriptions will test
the description text according to template which does have trailing
empty lines).

> But even if we *do* add a flag (like "--verbatim") you should at the 
> *least* also then remove the
> 
> 	"# (Comment lines starting with '#' will not be included)\n"
> 
> printout! Which you didn't.

I did think about it. It wont be read, I believe (at least I ignore
the status listing git-commit generates today). But then, it can be
removed for verbatim message as no one (I think) will probably care.
Including the "Please..." so it does not look stupid in the commit
message later. Will do.

> > It also implies --allow-empty.
> 
> I disagree with this one too.
> 

I agree. Will remove (I am not even sure myself, why I did that. It is
not even tested)

^ permalink raw reply

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Junio C Hamano @ 2007-12-20 22:31 UTC (permalink / raw)
  To: しらいしななこ
  Cc: git, Jörg Sommer
In-Reply-To: <200712202145.lBKLj7Fu015050@mi0.bluebottle.com>

しらいしななこ  <nanako3@bluebottle.com> writes:

> How about making this behavior configurable?

First, as a general principle, I'd like to avoid having commands that
changes their behaviour drastically depending on who the user is.  It
makes it harder for people experienced a bit more than totally new to
help others.  If they are truly experts and are familiar about the
configuration stash.quick, then they will be fine, but others would say
"Well, it works for me -- 'git stash' itself won't stash but list.  Why
isn't it working for you, I don't know" and scratch head.

Having said that, I reserve rights to change my mind later and start
liking this approach as a compromise.

There are a few suggestions and comments.

> +allow_quick_stash () {
> +	
> +	quick=$(git config stash.quick)
> +	if test $? != 0
> +	then

I think this is not a per-repository but per-person configuration (I
already said I do not want per-person configuration to affect the
fundamental behaviour of commands, but let's put that objection on hold
for now).  "git config --global" would be more appropriate.

So if the user hasn't seen this behaviour before, then...

> +		if ! test -t 0 || ! test -t 1
> +		then
> +			return 0
> +		fi

If it is not interactively called, allow "git stash" sans parameters as
before.  Nice attention to the details.

> +		echo '
> +*** First time users ***
> ...
> +		git config stash.quick $quick
> +		echo '
> +You can reconfigure this by editing your $HOME/.gitconfig file'
> +
> +	fi

Again, you would want --global here.  Also hint about explicit "save"
and "list" in addition to "you can reconfigure" might be helpful.

> +	case "$quick" in
> +	true)	return 0 ;;
> +	false)	return 1 ;;
> +	ask)	: do not return ;;
> +	esac
> +	
> +	if ! test -t 0 || ! test -t 1
> +	then
> +		return 0
> +	fi

Even if it is configured to 'ask', we allow it for non-interactive
session (aka scripts).  Although I would agree with this logic, it could
be debatable.

> @@ -226,11 +289,16 @@ create)
>  	create_stash "$*" && echo "$w_commit"
>  	;;
>  *)
> -	if test $# -eq 0
> +	if test $# -ne 0
> +	then
> +		usage
> +	fi
> +	if allow_quick_stash
>  	then
>  		save_stash && git-reset --hard
>  	else
> -		usage
> +		echo "*** Stash List ***"
> +		list_stash
>  	fi

I was scratching my head about this extra "echo" and tried your version
after removing it, to realize this is another nice attention to the
details.  Without it, what's output from the command is not very clear
to people who do not know what "git stash" is configured to do for the
session.

^ permalink raw reply

* Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR
From: Junio C Hamano @ 2007-12-20 22:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Luciano Rocha, git
In-Reply-To: <Pine.LNX.4.64.0712201255510.14355@wbgn129.biozentrum.uni-wuerzburg.de>

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

> Anything wrong with that patch?
>
> http://article.gmane.org/gmane.comp.version-control.git/68444

I think Steven stopped after you poked holes in that patch.

The way scripted commands spawned editor is:

	eval "${GIT_EDITOR:=vi}" '"$@"'

which meant that $IFS characters in $GIT_EDITOR separated words
and $environment_variables were substituted.

IOW, this is possible:

	GIT_EDITOR='emacs -l $HOME/my-customization.el'

I think something like this patch is probably more appropriate.
It avoids potential bugs in splitting arguments by hand and lets the
shell deal with the issue.

---
 builtin-tag.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..fae2487 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -47,7 +47,19 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
 		editor = "vi";
 
 	if (strcmp(editor, ":")) {
-		const char *args[] = { editor, path, NULL };
+		size_t len = strlen(editor);
+		int i = 0;
+		const char *args[6];
+
+		if (strcspn(editor, "$ \t'") != len) {
+			/* there are specials */
+			args[i++] = "sh";
+			args[i++] = "-c";
+			args[i++] = "$0 \"$@\"";
+		}
+		args[i++] = editor;
+		args[i++] = path;
+		args[i] = NULL;
 
 		if (run_command_v_opt_cd_env(args, 0, NULL, env))
 			die("There was a problem with the editor %s.", editor);

	

^ permalink raw reply related

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: しらいしななこ @ 2007-12-20 21:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jörg Sommer
In-Reply-To: <7vabo7y762.fsf@gitster.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>:
> Jörg Sommer <joerg@alea.gnuu.de> writes:
>
>> When it should go quick why don't use an alias. git stash can print the
>> list and everyone who wants a quick stash can create an alias for this.
>
> You are taking this completely backwards.  The stash mechanism is all
> about creating a quickie temporary pair of commits.  Anybody who wants
> otherwise can use alias or choose not to use stash at all.

You are of course right.  That was the reason I made 
git-stash command behave that way in the first place.

But I see that some people on the list find this behavior 
dangerous and I can understand their fears.  Until one 
learns that one can go back to the state before running 
git-stash by running "git-stash apply" soon after that, 
it appears to one that the work is lost.

How about making this behavior configurable?

-- 8< --
[PATCH] Make "git stash" configurable

"git stash" without argument originally created an unnamed 
stash, but some people felt this can be confusing to new 
users.  This introduces config variable stash.quick to 
control this behavior.

The variable can take one of three values: true, false, ask.

When set to "true", the command allows to create a quick 
stash without any user interaction.  When set to "false", 
the command shows the list of stash instead.  When set to 
"ask", the command asks the user.

For the first time users, when the variable is not set, 
the command helps the user to set it interactively.

Signed-off-by: Nanako Shiraishi <nanako3@bluebottle.com>

---

 git-stash.sh |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..4bb7134 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -192,6 +192,69 @@ apply_stash () {
 	fi
 }
 
+allow_quick_stash () {
+	
+	quick=$(git config stash.quick)
+	if test $? != 0
+	then
+		if ! test -t 0 || ! test -t 1
+		then
+			return 0
+		fi
+	
+		echo '
+*** First time users ***
+
+"git stash" can create an unnamed stash entry without user interaction.
+This is a quick way to save away your work in progress.  Some people
+find this behaviour confusing or dangerous to new users.  You can
+configure the command to list the existing stash entries instead.'
+		
+		while :
+		do
+			echo '
+Do you want the command without argument to always...
+
+1. Ask for confirmation
+2. Create an unnamed stash
+3. List existing stash entries
+'
+			printf 'Which one? [1/2/3] '
+			read reply
+			quick=
+			case "$reply" in
+			1|A*)	quick=ask ;;
+			2|C*)	quick=true ;;
+			3|L*)	quick=false ;;
+			*)	continue ;;
+			esac
+			break
+		done
+		git config stash.quick $quick
+		echo '
+You can reconfigure this by editing your $HOME/.gitconfig file'
+
+	fi
+	
+	case "$quick" in
+	true)	return 0 ;;
+	false)	return 1 ;;
+	ask)	: do not return ;;
+	esac
+	
+	if ! test -t 0 || ! test -t 1
+	then
+		return 0
+	fi
+	
+	printf 'Do you want to create an unnamed stash? [Y/n] '
+	read reply
+	case "$reply" in
+	[nN]*)	return 1 ;;
+	*)	return 0 ;;
+	esac
+}
+
 # Main command set
 case "$1" in
 list)
@@ -226,11 +289,16 @@ create)
 	create_stash "$*" && echo "$w_commit"
 	;;
 *)
-	if test $# -eq 0
+	if test $# -ne 0
+	then
+		usage
+	fi
+	if allow_quick_stash
 	then
 		save_stash && git-reset --hard
 	else
-		usage
+		echo "*** Stash List ***"
+		list_stash
 	fi
 	;;
 esac
--
1.5.3.7

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

----------------------------------------------------------------------
Find out how you can get spam free email.
http://www.bluebottle.com/tag/3

^ permalink raw reply related

* Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Linus Torvalds @ 2007-12-20 21:40 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <20071220211835.GA3052@steel.home>



On Thu, 20 Dec 2007, Alex Riesen wrote:
> 
> I just happen to have a corporate template (for perforce messages, I
> reuse it my git mirror repo) which contains "#" and at least one time
> lost my bash comments in a commit.

I think that this is a real bug, but I don't think this is something that 
we should add a flag for.

Basically, I don't think we should really strip lines starting with '#' 
unless *we* added them. In particular, I don't think we should strip them 
at all unless we're running the editor.

So I think that instead of your thing, we should do somethign like the 
appended, which allows you to do things like

	git commit -m "# Message starting with a hash-mark"

which the current code makes impossible ("empty commit message").

That may be enough for your case, although it still does leave the "use 
editor on a template thing", so if that is your usage scenario, I guess we 
still do need a flag for it.

But even if we *do* add a flag (like "--verbatim") you should at the 
*least* also then remove the

	"# (Comment lines starting with '#' will not be included)\n"

printout! Which you didn't.

So I say NAK on this patch.

> It also implies --allow-empty.

I disagree with this one too.

We have had *way* too many problems with various tools generating bogus 
empty commits. I get them from stgit users (and I think this is a serious 
BUG in stgit, dammit!), but I have this memory of some other usage 
scenario that did it too. 

In other words, empty commits are almost always just bogus. And dammit, if 
they aren't bogus, you should *say* so. No "implied" permissions, please. 
If you really want your commits to be empty, what's the downside of just 
adding an explicit "--allow-empty"?

		Linus

---
 builtin-commit.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 0a91013..4685938 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -434,7 +434,7 @@ static int message_is_empty(struct strbuf *sb, int start)
 	/* See if the template is just a prefix of the message. */
 	strbuf_init(&tmpl, 0);
 	if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {
-		stripspace(&tmpl, 1);
+		stripspace(&tmpl, !no_edit);
 		if (start + tmpl.len <= sb->len &&
 		    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
 			start += tmpl.len;
@@ -813,7 +813,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (p != NULL)
 		strbuf_setlen(&sb, p - sb.buf + 1);
 
-	stripspace(&sb, 1);
+	stripspace(&sb, !no_edit);
 	if (sb.len < header_len || message_is_empty(&sb, header_len)) {
 		rollback_index_files();
 		die("no commit message?  aborting commit.");

^ permalink raw reply related

* [PATCH] git-commit: add --verbatim to allow unstripped commit messages
From: Alex Riesen @ 2007-12-20 21:18 UTC (permalink / raw)
  To: git

It also implies --allow-empty.

Sometimes the message just have to be the way user wants it.
For instance, a template can contain "#" characters, or the message
must be kept as close to its original source as possible for reimport
reasons. Or maybe the user just copied a shell script including its
comments into the commit message for future reference.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

I just happen to have a corporate template (for perforce messages, I
reuse it my git mirror repo) which contains "#" and at least one time
lost my bash comments in a commit.

 Documentation/git-commit.txt |    7 ++++++-
 builtin-commit.c             |   10 ++++++++--
 t/t7502-commit.sh            |   17 +++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 4261384..862543f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git-commit' [-a | --interactive] [-s] [-v] [-u]
 	   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]
 	   [--allow-empty] [--no-verify] [-e] [--author <author>]
-	   [--] [[-i | -o ]<file>...]
+	   [--verbatim] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------
@@ -95,6 +95,11 @@ OPTIONS
 	from making such a commit.  This option bypasses the safety, and
 	is primarily for use by foreign scm interface scripts.
 
+--verbatim::
+	Inhibits stripping of leading and trailing spaces,
+	empty lines and #commentary from the commit message.
+	Implies --allow-empty.
+
 -e|--edit::
 	The message taken from file with `-F`, command line with
 	`-m`, and from file with `-C` are usually used as the
diff --git a/builtin-commit.c b/builtin-commit.c
index 0a91013..cc77ba9 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -47,6 +47,7 @@ static char *logfile, *force_author, *template_file;
 static char *edit_message, *use_message;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, untracked_files, no_verify, allow_empty;
+static int verbatim_message;
 
 static int no_edit, initial_commit, in_merge;
 const char *only_include_assumed;
@@ -88,6 +89,7 @@ static struct option builtin_commit_options[] = {
 	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
 	OPT_BOOLEAN(0, "untracked-files", &untracked_files, "show all untracked files"),
 	OPT_BOOLEAN(0, "allow-empty", &allow_empty, "ok to record an empty change"),
+	OPT_BOOLEAN(0, "verbatim", &verbatim_message, "do not strip spaces and #comments from message"),
 
 	OPT_END()
 };
@@ -346,7 +348,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
 
-	stripspace(&sb, 0);
+	if (!verbatim_message)
+		stripspace(&sb, 0);
 
 	if (signoff) {
 		struct strbuf sob;
@@ -512,6 +515,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		no_edit = 1;
 	if (edit_flag)
 		no_edit = 0;
+	if (verbatim_message)
+		allow_empty = 1;
 
 	if (get_sha1("HEAD", head_sha1))
 		initial_commit = 1;
@@ -813,7 +818,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (p != NULL)
 		strbuf_setlen(&sb, p - sb.buf + 1);
 
-	stripspace(&sb, 1);
+	if (!verbatim_message)
+		stripspace(&sb, 1);
 	if (sb.len < header_len || message_is_empty(&sb, header_len)) {
 		rollback_index_files();
 		die("no commit message?  aborting commit.");
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 21ac785..42f98bc 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -89,4 +89,21 @@ test_expect_success 'verbose' '
 
 '
 
+test_expect_success 'verbatim commit messages' '
+
+	echo >>negative &&
+	git add negative &&
+	{ echo;echo "# text";echo; } >expect &&
+	git commit --verbatim -t expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+	diff -u expect actual &&
+	git commit --verbatim -F expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual &&
+	git commit --verbatim -m "$(cat expect)" -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	diff -u expect actual
+
+'
+
 test_done
-- 
1.5.4.rc1.33.gbd32b

^ permalink raw reply related

* Re: Problem with git-svn
From: Pascal Obry @ 2007-12-20 20:34 UTC (permalink / raw)
  To: Eric Wong; +Cc: git list
In-Reply-To: <20071220183007.GA26767@untitled>

Eric,

> Ah, oops, I was off-by-one with the revision number.  But git-svn does
> look to be doing the right thing here, because it followed history into
> /importfromcvs/trunk/ and file.el was part of it.

Yes part of it but before the creation of the /importfromcvs/trunk/ that
was moved later as /trunk/PROJ.

In /importfromcvs/trunk/ there was many projects imported. One per one,
each time moving it into /trunk/PROJ.

If I look at history of /trunk/PROJ:

   $ svn log svn+ssh://myserver/trunk/PROJ

The last revision is 45775, so I think git-svn should not look past this
revision. So I'm very surprised by the current behavior and think it is
a bug to import file.el at revision 9458. Note that the workaround for
me is to use:

   $ git svn clone svn+ssh://myserver/trunk/PROJ --revision=45775:HEAD

But it would be lot cleaner to have git-svn handling this properly I think.

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: Problem with git-svn
From: Pascal Obry @ 2007-12-20 20:33 UTC (permalink / raw)
  To: Eric Wong; +Cc: git list
In-Reply-To: <20071220183007.GA26767@untitled>

Eric,

> Ah, oops, I was off-by-one with the revision number.  But git-svn does
> look to be doing the right thing here, because it followed history into
> /importfromcvs/trunk/ and file.el was part of it.

Yes part of it but before the creation of the /importfromcvs/trunk/ that
was moved later as /trunk/PROJ.

In /importfromcvs/trunk/ there was many projects imported. One per one,
each time moving it into /trunk/PROJ.

If I look at history of /trunk/PROJ:

   $ svn log svn+ssh://myserver/trunk/PROJ

The last revision is 45775, so I think git-svn should not look past this
revision. So I'm very surprised by the current behavior and think it is
a bug to import file.el at revision 9458. Note that the workaround for
me is to use:

   $ git svn clone svn+ssh://myserver/trunk/PROJ --revision=45775:HEAD

But it would be lot cleaner to have git-svn handling this properly I think.

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


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