* Re: [PATCH v0] sha1_name: grok <revision>:./<relative-path>
From: Johannes Schindelin @ 2007-12-22 14:33 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <fcaeb9bf0712210617x2bafa33cp15815a59fc631f45@mail.gmail.com>
Hi,
On Fri, 21 Dec 2007, Nguyen Thai Ngoc Duy wrote:
> [...] from my user perspective, the right approach is to make
> <treeish>:path always be relative to current directory.
As said by Junio, this would be a bad decision.
BTW please do not quote parts of the email that you do not comment on; it
takes half a minute of _everybody_ who tries to read your mail, only to
realise that it was time wasted.
Ciao,
Dscho
^ permalink raw reply
* Re: Problem with git-svn
From: Pascal Obry @ 2007-12-22 14:38 UTC (permalink / raw)
To: Eric Wong; +Cc: Pascal Obry, git list
In-Reply-To: <20071222042924.GA18812@soma>
Eric,
> Since r48468 was where /importfromcvs/trunk got renamed into /trunk/PROJ
> (from your previous message http://mid.gmane.org/4764FE2C.1010103@obry.net)
>
> /importfromcvs/trunk exists at r45775, but /trunk/PROJ does not; and
> git-svn at least follows that (which is what I suppose everybody wants).
Right.
> However...
>
> Did /importfromcvs/trunk exist all the way between r9458 and r48468? Or
> was that directory replaced entirely by something else along the way?
It was replaced along the way. This very same tree /importfromcvs/trunk
was used to import completely unrelated projects. Then moved, so it did
not exist all the way.
In my case the previous version on this tree was 45546. There is nothing
in between 45546 and 45775 for example. So looking at any revision in
between return an error:
$ svn log -v svn+ssh://myserver/importfromcvs/trunk@45760
svn: File not found: revision 45760, path '/importfromcvs/trunk'
> git-svn may be following copy history too aggressively, in this case.
Looks like it is the case. Hence the reported errors:
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 100, path '/trunk/PROJ'
and
W: Ignoring error from SVN, path probably does not exist: (160013):
Filesystem has no item: File not found: revision 101, path
'/importfromcvs/trunk'
For sure there was no revision 101 on /importfromcvs/trunk:
$ svn log -v svn+ssh://myserver/importfromcvs/trunk@101
svn: File not found: revision 101, path '/importfromcvs/trunk'
> On the other hand, this was somewhat intended because it could also
> be a way to track merges as "moving" tags[1].
>
>> $ 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.
>
> Does --no-follow-parent work in your case? Or does it go too far
> in stopping at r48468 (probably).
No it works on my case. That's what I'm using to import the directory in
two steps. I just want to go to the bottom of this to see if we can
somehow improve git-svn.
> Maybe another switch should be added (--merge-foster-parent?)
I'm not sure to understand what you are proposing here. To look only at
parents that are directly "related" to the current branch ?
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: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR
From: Johannes Schindelin @ 2007-12-22 14:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Luciano Rocha, git
In-Reply-To: <7vhcidovxt.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 20 Dec 2007, Junio C Hamano wrote:
> I think something like this patch is probably more appropriate.
Looks obviously fine, especially thinking about this:
> GIT_EDITOR='emacs -l $HOME/my-customization.el'
Ciao,
Dscho
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Johannes Schindelin @ 2007-12-22 15:04 UTC (permalink / raw)
To: Mike Frysinger; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <8bd0f97a0712210109q7805d967sc9b4cd13d4131360@mail.gmail.com>
Hi,
On Fri, 21 Dec 2007, Mike Frysinger wrote:
> ive started to transition from using svn everywhere to trying out git,
> and saw reference to this "stash" command on another list. i wanted to
> learn more about it, so i started off with `git-stash` to get some info,
> and wondered what just happened. then i typoed the --help option and
> wondered even more what just happened :).
I'm very sorry for you, but I, for one, refuse to let decisions be
influenced by people who did not have so much as a glimpse in the
documentation.
It may be okay for a certain nation state to award people spilling that
hot coffee that they ordered over there laps, but the rest of the world
laughs about such a behaviour.
If you got rope, stuck your neck through the noose, and jumped, without
reading the manual first, well, that's not my problem.
But I guess that we'll get that no-default-action behaviour, and I will
have to change my ways. Sigh.
Ciao,
Dscho
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Johannes Schindelin @ 2007-12-22 15:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 20 Dec 2007, Junio C Hamano wrote:
> * Introduction of "<tree>:./path" (Dscho). I could be talked into
> accepting the patch if it is useful to people who live deep within
> subdirectories.
IMHO this can safely await post-1.5.4.
Ciao,
Dscho
^ permalink raw reply
* Re: Serious bug with pretty format strings & empty bodies?
From: Jonathan del Strother @ 2007-12-22 15:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcibyvh0.fsf@gitster.siamese.dyndns.org>
On Dec 22, 2007 8:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> On Dec 19, 2007 6:44 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
> > Could you try
> >
> > git cat-file 18d2480ab689b483ef1ebbdb3f7420904049ba0b
> >
> > (or any other problematic commit) and post its output here?
>
> You mean git cat-file commit ... ?
> I get the normal output, but the problematic commits don't show a
> newline character at the end of the cat-file output.
>
> "I get the normal output" is not what Alex asked you to supply,
> nor would be sufficient information. There may be some
> abnormality in the commit object that you probably did not spot,
> but Alex or other people may have been able to if you were
> actually posted its output here.
========
$ git cat-file commit 18d2480ab689b483ef1ebbdb3f7420904049ba0b
tree 2416c620c6f7c1864065a6e778588b71b3e0bd5d
parent 9fc80d0a05835c68885f253844ab586b38d09202
author Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197907511 +0000
committer Jonathan del Strother <jon.delStrother@bestbefore.tv> 1197907511 +0000
Try to flush log files before terminating the app%
========
The '%' suffix is zsh showing that there's no newline there. Under
bash I get no '%' and the next prompt starts immediately following
'app'.
> One thing I noticed funny in your original message was "-1-".
> Is it essential that the number is spelled incorrectly to
> reproduce this problem?
No, sorry - I don't know how that got there. I get identical output
whether it's "rev-list -1" or "rev-list -1-"
I've been trying to reproduce this on my home repo, and have only been
partially successful. I haven't been able to make git rev-list -1
show a malformed message body. I can only get the problem to occur
when printing multiple commits (eg git rev-list -20 ...) Bizarrely,
the first 2 commits shown in rev-list never show a problem. After the
first 2, commits with the broken message bodies will always appear
broken, regardless of their position in the list. However, the
content that appears instead of the body will change depending on the
commits that appear prior to that one.
I realise this doesn't appear to mesh with my observations the other
day, where I was seeing the broken bodies even when just using
rev-list -1 - I'm at a loss to describe it. Both machines are intel
macs (one a MacPro, one a MacBook Pro), running OS X 10.5.1, with the
same git version (1.5.4.rc0.1162.g3bfea). Unfortunately I've switched
my work machine off for christmas, or I'd ssh in and try to re-verify
things.
I do appreciate people's help in trying to reproduce & fix this - I
realise I've not given people much to go on.
^ permalink raw reply
* Re: [PATCH] Emit helpful status for accidental "git stash" save
From: Junio C Hamano @ 2007-12-22 16:22 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <1198333369-17788-1-git-send-email-win@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> 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 () {
I like that "To restore them..." insn, and I like that this is
much less invasive than anything we have seen during the
discussion. But can we do this only for an accidental "git
stash" not for a deliberate "git stash save"?
^ permalink raw reply
* Re: cvs -> git tools?
From: David Soria Parra @ 2007-12-22 16:00 UTC (permalink / raw)
To: git
In-Reply-To: <m3zlw3oged.fsf@roke.D-201>
> There are many CVS -> git tools: git-cvsimport (uses cvsps), parsecvs
> (accesses ,v files directly), fromcvs (in Ruby), cvs2svn development
> branch. But I think only git-cvsimport allows incremental import.
there is also gc-utils (http://sf.net/projects/gcutils) which wraps
git-cvsimport and git-cvsexportcommit to make commiting back into CVS a
little bit easier.
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Pierre Habouzit @ 2007-12-22 17:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]
On ven, déc 21, 2007 at 12:32:28 +0000, Junio C Hamano wrote:
> 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.
About that, I know we talked about the -default thing and so on, I'm
not sure we should hurry that for 1.5.4 for the following reasons:
* I grepped through the source and the _sole_ instance of
parse-options enabled option arguments are --abbrev ones for now
(even in the shell scripts migrated to git rev-parse --parseopt).
* Not adding *-default and lax parsing for optional arguments is _not_
a regression for the migrated commands.
* I don't want to urge that because well, I still have the hope we
could come up with something even better.
So I'd argue in favor of that:
+ push the patch that forces the stuck forms (that I already posted)
for 1.5.4.
+ prepare a series in pu with *-default post 1.5.4 to evaluate this
and see what people think.
Most of the function with optional arguments are the git diff ones,
and we'll see about them post 1.5.4 anyways.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Junio C Hamano @ 2007-12-22 17:16 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071222170315.GB23262@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> So I'd argue in favor of that:
> + push the patch that forces the stuck forms (that I already posted)
> for 1.5.4.
Ok, sounds sane. You posted multiple serieses; which ones?
^ permalink raw reply
* cvsimport: trying to convert freebsd cvs to git
From: Miklos Vajna @ 2007-12-22 17:18 UTC (permalink / raw)
To: git; +Cc: Stefan Sperling
[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]
hi,
recently Stefan reported that he tried to convert the freebsd cvs' src
module to git and he failed. i tried to help him, but i failed, too.
here are my efforts:
he made the cvs available on a (relatively) fast rsync mirror at:
rsync://ftp.spline.de/FreeBSD-CVS
(so you don't have to use cvsup if it's a problem for you)
so after mirroring it, i had:
$ ls cvs
CVSROOT/ CVSROOT-ports/ CVSROOT-src/ ports/ src/
then i tried:
$ time git cvsimport -d `pwd`/cvs -C src.git src
Initialized empty Git repository in /home/vmiklos/git/freebsd/src.git/.git/
malformed revision
fatal: refs/heads/origin: not a valid SHA1
fatal: master: not a valid SHA1
warning: You appear to be on a branch yet to be born.
warning: Forcing checkout of HEAD.
fatal: just how do you expect me to merge 0 trees?
checkout failed: 256
real 15m11.529s
user 0m46.212s
sys 0m6.680s
my questions:
1) does cvsimport supports the case when the source if on the local
filesystem, and not in not on a cvs server?
first i wanted to make sure that cvsimport itself works properly here:
$ git cvsimport -d :pserver:anonymous@tcpflow.cvs.sourceforge.net:/cvsroot/tcpflow -C tcpflow tcpflow
and it converted this small repo fine
2) if it supports, then i think the real error message is 'malformed
revision'. what is the proper way to see where is that revision?
of course if cvsimport is not the right tool to incrementally convert
such a big repo, then i would be interested in other advices, too.
thanks,
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH v2] Emit helpful status for accidental "git stash" save
From: Wincent Colaiuta @ 2007-12-22 17:31 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <7vmys2ya0l.fsf@gitster.siamese.dyndns.org>
El 22/12/2007, a las 17:22, Junio C Hamano escribió:
> I like that "To restore them..." insn, and I like that this is
> much less invasive than anything we have seen during the
> discussion. But can we do this only for an accidental "git
> stash" not for a deliberate "git stash save"?
Something like this then?
-- 8< --
Emit helpful status for accidental "git stash" save
If the user types "git stash" mistakenly thinking that this will list
their stashes he/she may be surprised to see that it actually saved
a new stash and reset their working tree and index.
In the worst case they might not know how to recover the state. So
help them by telling them exactly what was saved and also how to
restore it immediately.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
git-stash.sh | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..ab52b6f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -13,6 +13,7 @@ TMP="$GIT_DIR/.git-stash.$$"
trap 'rm -f "$TMP-*"' 0
ref_stash=refs/stash
+ARGC=$#
no_changes () {
git diff-index --quiet --cached HEAD -- &&
@@ -99,7 +100,9 @@ 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"
+ test $ARGC -eq 0 &&
+ echo >&2 '(To restore them type "git stash apply")'
}
have_stash () {
--
1.5.4.rc0.68.g15eb8-dirty
^ permalink raw reply related
* Re: 1.5.4-rc2 plans
From: Pierre Habouzit @ 2007-12-22 17:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vir2qy7ie.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 729 bytes --]
On Sat, Dec 22, 2007 at 05:16:41PM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > So I'd argue in favor of that:
> > + push the patch that forces the stuck forms (that I already posted)
> > for 1.5.4.
>
> Ok, sounds sane. You posted multiple serieses; which ones?
All the patches I think should go in 1.5.4 are in that very thread.
Forcing the stuck forms for optional arguments is in
Message-Id: <20071221104704.GC17701@artemis.madism.org>, or is on
git.madism.org in branch ph/parseopt.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Junio C Hamano @ 2007-12-22 17:52 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <7vir2qy7ie.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Pierre Habouzit <madcoder@debian.org> writes:
>
>> So I'd argue in favor of that:
>> + push the patch that forces the stuck forms (that I already posted)
>> for 1.5.4.
>
> Ok, sounds sane. You posted multiple serieses; which ones?
Ah, my apologies. I can fetch them from your git repository at
madism.org, of course.
^ permalink raw reply
* Re: 1.5.4-rc2 plans
From: Mike Frysinger @ 2007-12-22 17:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0712221554030.14355@wbgn129.biozentrum.uni-wuerzburg.de>
On Dec 22, 2007 10:04 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Fri, 21 Dec 2007, Mike Frysinger wrote:
> > ive started to transition from using svn everywhere to trying out git,
> > and saw reference to this "stash" command on another list. i wanted to
> > learn more about it, so i started off with `git-stash` to get some info,
> > and wondered what just happened. then i typoed the --help option and
> > wondered even more what just happened :).
>
> I'm very sorry for you, but I, for one, refuse to let decisions be
> influenced by people who did not have so much as a glimpse in the
> documentation.
>
> <snip pointless garbage>
why do you insist on wasting people's time if your point is merely to
insult ? if that is your goal, go somewhere else.
-mike
^ permalink raw reply
* Re: [PATCH v2] Emit helpful status for accidental "git stash" save
From: Junio C Hamano @ 2007-12-22 17:58 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git, gitster
In-Reply-To: <1198344685-24156-1-git-send-email-win@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> El 22/12/2007, a las 17:22, Junio C Hamano escribió:
>
>> I like that "To restore them..." insn, and I like that this is
>> much less invasive than anything we have seen during the
>> discussion. But can we do this only for an accidental "git
>> stash" not for a deliberate "git stash save"?
>
> Something like this then?
Sure, even like this.
git-stash.sh | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..ade300c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -99,7 +99,7 @@ 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"
}
have_stash () {
@@ -228,7 +228,9 @@ create)
*)
if test $# -eq 0
then
- save_stash && git-reset --hard
+ save_stash &&
+ echo >&2 '(To restore them type "git stash apply")' &&
+ git-reset --hard
else
usage
fi
^ permalink raw reply related
* Re: [FIXED PATCH] git-describe: Add a --match option to limit considered tags.
From: Pierre Habouzit @ 2007-12-22 18:02 UTC (permalink / raw)
To: Git ML
In-Reply-To: <20071221214954.GA16481@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]
Like I said on IRC, I saw that git describe --contains has a bad
behaviour:
$ git describe --match='asd*' HEAD; echo $?
fatal: cannot describe 'e272415ab7da3bde51af2ce95c88d7be3abfba28'
128
$ git describe --contains HEAD; echo $?
undefined
0
THe "undefined" output is on stdout (not stderr), and returns 0.
The issue here is that it internally uses git-name-rev by exec-ing it, which
makes it hard to fix. Though I suppose that we could instead of fork-ing
share some logic with builtin-name-rev.c, but I'm not at home yet, so
won't likely have a patch for this issue.
Note that the use of the "new" --match here was just to be unable to
describe the HEAD to show the difference, the inconsistency has nothing
to do with the patch I propose, I just happen to noticed that.
AFAICT it's not a regression, it's just a misfeature :)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Don't dereference NULL upon lookup_tree failure.
From: Junio C Hamano @ 2007-12-22 18:32 UTC (permalink / raw)
To: Jim Meyering; +Cc: git list, Matthew Farrellee
In-Reply-To: <87prwyu9r0.fsf@rho.meyering.net>
Thanks.
^ permalink raw reply
* Re: [PATCH v2] Emit helpful status for accidental "git stash" save
From: Wincent Colaiuta @ 2007-12-22 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabo2y5la.fsf@gitster.siamese.dyndns.org>
El 22/12/2007, a las 18:58, Junio C Hamano escribió:
> Wincent Colaiuta <win@wincent.com> writes:
>
>> Something like this then?
>
> Sure, even like this.
>
> git-stash.sh | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index f16fd9c..ade300c 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -99,7 +99,7 @@ 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"
> }
>
> have_stash () {
> @@ -228,7 +228,9 @@ create)
> *)
> if test $# -eq 0
> then
> - save_stash && git-reset --hard
> + save_stash &&
> + echo >&2 '(To restore them type "git stash apply")' &&
> + git-reset --hard
> else
> usage
> fi
Preserves the behaviour while avoiding the need for the creation of
the ARGC variable, so if you prefer this solution then it seems fine
by me.
Cheers,
Wincent
^ permalink raw reply
* [PATCH] Allow selection of different cleanup modes for commit messages
From: Alex Riesen @ 2007-12-22 18:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7v3atv17o9.fsf@gitster.siamese.dyndns.org>
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.
The cleanup modes are default, verbatim, whitespace and strip. The
default mode depends on if the message is being edited and will either
strip whitespace and comments (if editor active) or just strip the
whitespace (for where the message is given explicitely).
Suggested by Linus.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Junio C Hamano, Sat, Dec 22, 2007 08:59:34 +0100:
> > + removed. The 'verbatim' mode wont change message at all,
> > + 'whitespace' removes just leading/trailing whitespace lines
> > + and 'strip' removes both whitespace and commentary.
>
> (minor) s/wont/does not/
>
Done.
> > + if (cleanup_mode == CLEANUP_DEFAULT)
> > + fprintf(fp,
> > + "# (Comment lines starting with '#' will not be included)\n");
>
> Can't cleanup_mode be CLEANUP_ALL at this point, and if so
> shouldn't this insn be given as well?
Rewritten as suggested (set cleanup_mode depending on no_edit in
parse_and_validate_options).
> In addition, if:
>
> - we are talking with editor, and
> - the mode is verbatim, and
> - we added any "#" line ourselves (e.g. in_merge adds the insn
> shown above, or perhaps we added "git status" output),
>
> then perhaps we would want to say that "#" lines need to be
> stripped by the user; otherwise it will be left in the commit.
Well, I don't know. I find these click-through (look-through)
instructions uninteresting, and I sometimes feel tempted to suggest
a config option like "core.autohelp_level" and make it 0 by default.
As soon as someone complains - he'll be told to increase it. The
commit message hints would go at 1. We have plenty of space for
other helpfulness levels in 31 bits.
Seriously though, I agree with Linus: we better just make sure that
whatever was generated by Git never ends up in the commit message. And
make very sure user knows it. Maybe just don't put anything in commit
message? Show the info elsewhere? Go with something like Björns
suggestion?
> > if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {
> > - stripspace(&tmpl, 1);
> > + stripspace(&tmpl, !no_edit);
>
> We at this point know that some stripping would happen. The
> template needs to be cleaned the same way as the commit message
> was stripped. Is checking no_edit the right thing to do? What
> if cleanup_mode was CLEANUP_ALL and there is no editing going
> on?
The cleanup_mode initialization rewrite took care of this as well
> > @@ -813,7 +846,9 @@ 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 (cleanup_mode != CLEANUP_NONE)
> > + stripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?
> > + !no_edit: cleanup_mode == CLEANUP_ALL);
>
> When writing such a long ? : expression, laying it out like:
>
> condition
> ? if-true
> : if-false
>
> would be easier to read. You tilt your head sideways 90
> degrees, just like you read a smiley ;-), and you will see the
> parse tree.
I just tried it. Still don't know if I like it :)
Sadly, the mentioned rewrite removed the need for it.
> But I suspect, assuming that the two issues around CLEANUP_DEFAULT vs
> CLEANUP_ALL I mentioned above are indeed bugs, it may be cleaner
> to:
>
> - parse the options; you get one of NONE/DEFAULT/SPACE/ALL
>
> - convert DEFAULT to SPACE or ALL if editor is in effect.
>
> - do not worry about no_editor or DEFAULT in the rest of the
> code when calling stripspace(). The only values you will
> see are NONE, SPACE or ALL.
Well, why didn't you said this at start?! :)
> > +test_expect_success 'verbatim commit messages' '
> > +
>
> These tests check too many things at once, and it would be very
> hard to diagnose when breakage does happen. Please split them
> perhaps into three separate tests like this:
Done
> > +'
>
> Also, the tests do not check "-F file -e", "-m msg -e" etc. We
> would want to make sure the right insn are shown to the user in
> the editor, wouldn't we?
As I am not convinced whether they are needed at all, I just added a
placeholder which expects what we have today.
Documentation/git-commit.txt | 12 ++++++-
builtin-commit.c | 72 ++++++++++++++++++++++++++++++-----------
t/t7502-commit.sh | 65 +++++++++++++++++++++++++++++++++++++
3 files changed, 128 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 4261384..96383b6 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>...]
+ [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
DESCRIPTION
-----------
@@ -95,6 +95,16 @@ OPTIONS
from making such a commit. This option bypasses the safety, and
is primarily for use by foreign scm interface scripts.
+--cleanup=<mode>::
+ This option sets how the commit message is cleaned up.
+ The '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
+ and 'default'. The 'default' mode will strip leading and
+ trailing empty lines and #commentary from the commit message
+ only if the message is to be edited. Otherwise only whitespace
+ removed. The 'verbatim' mode does not change message at all,
+ 'whitespace' removes just leading/trailing whitespace lines
+ and 'strip' removes both whitespace and commentary.
+
-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 96410de..34c5fd2 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -47,6 +47,19 @@ 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;
+/*
+ * The default commit message cleanup mode will remove the lines
+ * beginning with # (shell comments) and leading and trailing
+ * whitespaces (empty lines or containing only whitespaces)
+ * if editor is used, and only the whitespaces if the message
+ * is specified explicitly.
+ */
+static enum {
+ CLEANUP_SPACE,
+ CLEANUP_NONE,
+ CLEANUP_ALL,
+} cleanup_mode;
+static char *cleanup_arg;
static int no_edit, initial_commit, in_merge;
const char *only_include_assumed;
@@ -88,6 +101,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_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
OPT_END()
};
@@ -346,7 +360,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 (cleanup_mode != CLEANUP_NONE)
+ stripspace(&sb, 0);
if (signoff) {
struct strbuf sob;
@@ -398,23 +413,26 @@ static int prepare_log_message(const char *index_file, const char *prefix)
return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
}
- if (in_merge && !no_edit)
- fprintf(fp,
- "#\n"
- "# It looks like you may be committing a MERGE.\n"
- "# If this is not correct, please remove the file\n"
- "# %s\n"
- "# and try again.\n"
- "#\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 (only_include_assumed)
- fprintf(fp, "# %s\n", only_include_assumed);
-
+ if (!no_edit) {
+ if (in_merge)
+ fprintf(fp,
+ "#\n"
+ "# It looks like you may be committing a MERGE.\n"
+ "# If this is not correct, please remove the file\n"
+ "# %s\n"
+ "# and try again.\n"
+ "#\n",
+ git_path("MERGE_HEAD"));
+ if (cleanup_mode != CLEANUP_NONE)
+ fprintf(fp,
+ "\n"
+ "# Please enter the commit message for your changes.\n");
+ if (cleanup_mode == CLEANUP_ALL)
+ fprintf(fp,
+ "# (Comment lines starting with '#' will not be included)\n");
+ if (only_include_assumed)
+ fprintf(fp, "# %s\n", only_include_assumed);
+ }
saved_color_setting = wt_status_use_color;
wt_status_use_color = 0;
commitable = run_status(fp, index_file, prefix, 1);
@@ -435,10 +453,13 @@ static int message_is_empty(struct strbuf *sb, int start)
const char *nl;
int eol, i;
+ if (cleanup_mode == CLEANUP_NONE && sb->len)
+ return 0;
+
/* 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, cleanup_mode == CLEANUP_ALL);
if (start + tmpl.len <= sb->len &&
memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
start += tmpl.len;
@@ -591,6 +612,16 @@ static int parse_and_validate_options(int argc, const char *argv[],
only_include_assumed = "Explicit paths specified without -i nor -o; assuming --only paths...";
also = 0;
}
+ if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
+ cleanup_mode = no_edit ? CLEANUP_SPACE: CLEANUP_ALL;
+ else if (!strcmp(cleanup_arg, "verbatim"))
+ cleanup_mode = CLEANUP_NONE;
+ else if (!strcmp(cleanup_arg, "whitespace"))
+ cleanup_mode = CLEANUP_SPACE;
+ else if (!strcmp(cleanup_arg, "strip"))
+ cleanup_mode = CLEANUP_ALL;
+ else
+ die("Invalid cleanup mode %s", cleanup_arg);
if (all && argc > 0)
die("Paths with -a does not make sense.");
@@ -817,7 +848,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 (cleanup_mode != CLEANUP_NONE)
+ stripspace(&sb, cleanup_mode == CLEANUP_ALL);
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..aaf497e 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -89,4 +89,69 @@ test_expect_success 'verbose' '
'
+test_expect_success 'cleanup commit messages (verbatim,-t)' '
+
+ echo >>negative &&
+ { echo;echo "# text";echo; } >expect &&
+ git commit --cleanup=verbatim -t expect -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+ diff -u expect actual
+
+'
+
+test_expect_success 'cleanup commit messages (verbatim,-F)' '
+
+ echo >>negative &&
+ git commit --cleanup=verbatim -F expect -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ diff -u expect actual
+
+'
+
+test_expect_success 'cleanup commit messages (verbatim,-m)' '
+
+ echo >>negative &&
+ git commit --cleanup=verbatim -m "$(cat expect)" -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ diff -u expect actual
+
+'
+
+test_expect_success 'cleanup commit messages (whitespace,-F)' '
+
+ echo >>negative &&
+ { echo;echo "# text";echo; } >text &&
+ echo "# text" >expect &&
+ git commit --cleanup=whitespace -F text -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ diff -u expect actual
+
+'
+
+test_expect_success 'cleanup commit messages (strip,-F)' '
+
+ echo >>negative &&
+ { echo;echo "# text";echo sample;echo; } >text &&
+ echo sample >expect &&
+ git commit --cleanup=strip -F text -a &&
+ git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+ diff -u expect actual
+
+'
+
+echo "sample
+
+# Please enter the commit message for your changes.
+# (Comment lines starting with '#' will not be included)" >expect
+
+test_expect_success 'cleanup commit messages (strip,-F,-e)' '
+
+ echo >>negative &&
+ { echo;echo sample;echo; } >text &&
+ git commit -e -F text -a &&
+ head -n 4 .git/COMMIT_EDITMSG >actual &&
+ diff -u expect actual
+
+'
+
test_done
--
1.5.4.rc1.48.gb232
^ permalink raw reply related
* Re: [PATCH] Allow selection of different cleanup modes for commit messages
From: Linus Torvalds @ 2007-12-22 19:18 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <20071222184624.GA4167@steel.home>
On Sat, 22 Dec 2007, Alex Riesen wrote:
>
> + if (!no_edit) {
This is unrelated to the rest of the patch, but I do think double
negations are horrible, so I thoink we should probably make the "no_edit"
flag change meaning, and call it "run_editor" or something.
That said, the thing that I'm actually reacting to is:
> + if (in_merge)
> + fprintf(fp,
> + "#\n"
> + "# It looks like you may be committing a MERGE.\n"
> + "# If this is not correct, please remove the file\n"
> + "# %s\n"
> + "# and try again.\n"
> + "#\n",
> + git_path("MERGE_HEAD"));
> + if (cleanup_mode != CLEANUP_NONE)
> + fprintf(fp,
> + "\n"
> + "# Please enter the commit message for your changes.\n");
> + if (cleanup_mode == CLEANUP_ALL)
> + fprintf(fp,
> + "# (Comment lines starting with '#' will not be included)\n");
> + if (only_include_assumed)
> + fprintf(fp, "# %s\n", only_include_assumed);
The thing that still worries me about this set is that if you have
"cleanup_mode == CLEANUP_NONE" or "cleanup_mode == CLEANUP_SPACE", we're
now adding all these messages, and we depend on the user knowing that *he*
has to remove them.
So I wonder if we should perhaps:
(a) Only add these messages at all when we do *not* do CLEANUP_ALL
In other words, change the
if (!no_edit) {
line to a
if (cleanup_mode == CLEANUP_ALL && !no_edit) {
instead, or
(b) Add a a new line to replace he "will not be included" message, ie
if (cleanup_mode != CLEANUP_ALL)
fprintf(p,
"# Please remove these comment lines manually, "
"they will not be automatically removed\n");
or something similar.
I personally would prefer (a) - since anybody who then explicitly uses
--cleanup={space|none} would presumably already know what he is doing.
But this is not a huge deal. Regardless, the patch looks ok, so you can
add a "Acked-by:" from me.
Linus
^ permalink raw reply
* Re: [PATCH] Allow selection of different cleanup modes for commit messages
From: Junio C Hamano @ 2007-12-22 19:41 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alex Riesen, git
In-Reply-To: <alpine.LFD.0.9999.0712221107240.21557@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sat, 22 Dec 2007, Alex Riesen wrote:
>>
>> + if (!no_edit) {
>
> This is unrelated to the rest of the patch, but I do think double
> negations are horrible, so I thoink we should probably make the "no_edit"
> flag change meaning, and call it "run_editor" or something.
> ...
> So I wonder if we should perhaps:
>
> (a) Only add these messages at all when we do *not* do CLEANUP_ALL
> ...
> (b) Add a a new line to replace he "will not be included" message, ie
> ...
> I personally would prefer (a) - since anybody who then explicitly uses
> --cleanup={space|none} would presumably already know what he is doing.
>
> But this is not a huge deal. Regardless, the patch looks ok, so you can
> add a "Acked-by:" from me.
I was composing essentially the same message, except my
preference was (as you can guess by the fact that I hinted the
additional instruction) (b), but I agree that (a) is better at
least for now because the user has to ask for verbatim every
time (i.e. there is no config).
By the way, the "if (!no_edit)" conditional itself you quoted
above as the first thing in your message is not needed at all.
If no_edit is set, the function already has returned and we
would not reach that point.
So a third round?
^ permalink raw reply
* Pushing and fetching sha1s directly
From: Finn Arne Gangstad @ 2007-12-22 21:13 UTC (permalink / raw)
To: git
Currently there seems to be no way of pusing a sha1 directly, or to
fetch a sha1 directly. When working with submodules, it is convenient
to be able to work with detached HEADs, so it would be good if this
could be supported also by fetch and push.
What would be a resonable syntax for this? I'm thinking something like
git fetch --sha1 <repository> <sha1>
git push --sha1 <repository> <sha1>
Where <sha1> could really be a <commit> I guess, but the option name
"--commit" seems wrong somehow. Another option is to extend refspecs
so sha1s can be allowed in there directly, so this would just work:
git fetch <repository> <sha1>
What do you prefer?
- Finn Arne
^ permalink raw reply
* Re: Pushing and fetching sha1s directly
From: Junio C Hamano @ 2007-12-22 23:12 UTC (permalink / raw)
To: Finn Arne Gangstad; +Cc: git
In-Reply-To: <20071222211308.GA27281@pvv.org>
Finn Arne Gangstad <finnag@pvv.org> writes:
> Currently there seems to be no way of pusing a sha1 directly, or to
> fetch a sha1 directly. When working with submodules, it is convenient
> to be able to work with detached HEADs, so it would be good if this
> could be supported also by fetch and push.
At least push of an arbitrary commit is already supported, I
think.
For fetch, I would normally say "check the list archives", but
it was very long time ago that it was proposed and discussed.
It has some security and performance implications.
^ permalink raw reply
* sane, stable renames; when a commit should commit twice
From: Zenaan Harkness @ 2007-12-23 2:03 UTC (permalink / raw)
To: git
When should a commit, commit twice?
When one or more git mv file renames/ moves are involved.
In such a case the commit ought to be split into two. Perhaps move the
files in the first commit, then make the changes needed to support the
move in the build chain (including changes in the moved files) in the
second commit.
This keeps a clean record of the move, making the move, and the
associated changes (as two commits) a clean cherry.
Does this make sense?
I develop in the java world, and we use packages (directories, and
subdirectories, sub-sub... etc) a lot, and so it is not uncommon in my
10 years development, to decide to reorganise some package/dir every now
and then, and files, and whole dirs, get moved.
I've only been using git for a few weeks, but finding it truly awesome!
A little demanding in the initial learning curve - took me three days of
reading and a little experiementation here and there, before I finally
felt comfortable with rebasing, branching, etc, to effect my work
pattern.
Have used arch/tla, a little bzr, aegis for a couple of years long time
ago, some cvs, and bk for four months or so.
I'm hoping that the above workflow, which has just crystallized for me
in the last two days, makes sense.
zen
--
Homepage: www.SoulSound.net -- Free Australia: www.UPMART.org
Please respect the confidentiality of this email as sensibly warranted.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox