Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/6] push: mention --verbose option in documentation
From: Steffen Prohaska @ 2007-11-11 14:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <11947897083381-git-send-email-prohaska@zib.de>


On Nov 11, 2007, at 3:01 PM, Steffen Prohaska wrote:

> From: Steffen Prohaska <gitster@pobox.com>

This is obviously wrong. I have no clear idea how this happend.
I remember I took the topic branch sp/push-refspec from pu and
started to refactor. I used 'rebase' and 'rebase -i' a couple
of times and now I have my name with Junios email address.

I'll investigate how this could happen.

	Steffen

^ permalink raw reply

* Re: [PATCH 1/6] push: mention --verbose option in documentation
From: Steffen Prohaska @ 2007-11-11 14:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <51B9B630-85F7-42BA-BCFF-4374A2527733@zib.de>


On Nov 11, 2007, at 3:10 PM, Steffen Prohaska wrote:

>
> On Nov 11, 2007, at 3:01 PM, Steffen Prohaska wrote:
>
>> From: Steffen Prohaska <gitster@pobox.com>
>
> This is obviously wrong. I have no clear idea how this happend.
> I remember I took the topic branch sp/push-refspec from pu and
> started to refactor. I used 'rebase' and 'rebase -i' a couple
> of times and now I have my name with Junios email address.
>
> I'll investigate how this could happen.

The Author line was already wrong in Junio's pu branch, commit
f31447f5f06200305393ca16e2eb9485e65dcccc,  and I carried this
over without noticing.

	Steffen

^ permalink raw reply

* Re: [PATCH 3/6] push: support pushing HEAD to real branch name
From: Andreas Ericsson @ 2007-11-11 14:17 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Steffen Prohaska
In-Reply-To: <11947897083159-git-send-email-prohaska@zib.de>

Steffen Prohaska wrote:
> diff --git a/builtin-push.c b/builtin-push.c
> index 6d1da07..a99ba0c 100644
> --- a/builtin-push.c
> +++ b/builtin-push.c
> @@ -44,6 +44,15 @@ static void set_refspecs(const char **refs, int nr)
>  			strcat(tag, refs[i]);
>  			ref = tag;
>  		}
> +		if (!strcmp("HEAD", ref)) {
> +			unsigned char sha1_dummy[20];
> +			ref = resolve_ref(ref, sha1_dummy, 1, NULL);
> +			if (!ref)
> +				die("HEAD cannot be resolved.");
> +			if (strncmp(ref, "refs/heads/", 11))

Why not prefixcmp(ref, "refs/heads/")?

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

^ permalink raw reply

* Re: [PATCH 3/3] git-svn log: handle unreachable revisions like "svn log"
From: David D. Kilzer @ 2007-11-11 14:20 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: git, gitster
In-Reply-To: <D6A0D2B9-A355-4216-8D15-84993C26B503@lrde.epita.fr>

Benoit Sigoure <tsuna@lrde.epita.fr> wrote:

> thanks for the patches, the series looks good to me, I added some  
> comments below, for this patch.

Thanks for the review, Benoit!  Comments below.

> On Nov 11, 2007, at 7:10 AM, David D Kilzer wrote:
> 
> >  sub find_rev_before {
> > -	my ($self, $rev, $eq_ok) = @_;
> > +	my ($self, $rev, $eq_ok, $min_rev) = @_;
> 
> Could you please document this function?  I guess that you had to  
> figure out what each argument was for, so please save the time of the  
> contributors that will read this code after you :)

What is the format for documenting functions in git Perl scripts?  I haven't
see any "perlpod" use anywhere.  Do you just want comments before the function?

This method returns the git commit hash and svn revision of the first svn
revision that exists on the current branch that is less than $rev (or
less-than-or-equal-to $rev if $eq_ok is true).

Please note that I don't have a full understanding of how find_rev_before()
works (other than it's computing an offset into a sparse? data file based on
the revision number) since I'm still new to git.

> > +sub find_rev_after {
> > +	my ($self, $rev, $eq_ok, $max_rev) = @_;
> > +	++$rev unless $eq_ok;
> > +	$max_rev ||= $self->rev_db_max();
> > +	while ($rev <= $max_rev) {
> > +		if (my $c = $self->rev_db_get($rev)) {
> > +			return ($rev, $c);
> > +		}
> > +		++$rev;
> > +	}
> > +	return (undef, undef);
> > +}
> 
> Too much code duplication.  It should be possible to write a sub  
> find_rev_ (or _find_rev, don't know what's the naming convention for  
> internal details) that takes a 5th argument, an anonymous sub that  
> does the comparison.  So that basically, find_rev_before will be  
> something along these (untested) lines:
> 
> sub find_rev_before {
> 	my ($self, $rev, $eq_ok, $min_rev) = @_;
> 	return find_rev_($self, $rev, $eq_ok, $min_rev, sub { my ($a, $b) =  
> @_; return $a >= $b; });
> }

I think that combining find_rev_before() and find_rev_after() would greatly
sacrifice readability of the code in exchange for removing ~10 lines of code. 
Also, you must do more than just replace the comparison in the while() loop:

- before() decrements $rev while after() increments it
- stop limits are different ($max_rev versus $min_rev)

This is what such a method might look like (untested).  Since you already
requested find_rev_before() be documented, is this really going to help?

sub find_rev_ {
	my ($self, $rev, $eq_ok, $is_before, $limit_rev) = @_;
	($is_before ? --$rev : ++$rev) unless $eq_ok;
	$limit_rev ||= ($is_before ? 1 : $self->rev_db_max());
	while ($is_before ? $rev >= $limit_rev : $rev <= $limit_rev) {
		if (my $c = $self->rev_db_get($rev)) {
			return ($rev, $c);
		}
			$is_before ? --$rev : ++$rev;
	}
	return (undef, undef);
}

Defining wrapper functions would help, but I still think it's just as clear to
keep the two methods separate.

sub find_rev_before() {
	my ($self, $rev, $eq_ok, $min_rev) = @_;
	return $self->find_rev_($rev, $eq_ok, 1, $min_rev);
}

sub find_rev_after() {
	my ($self, $rev, $eq_ok, $max_rev) = @_;
	return $self->find_rev_($rev, $eq_ok, 0, $max_rev);
}

Do you agree, or do you think the methods should still be combined?

> > +		if ($r_max < $r_min) {
> > +			($r_min, $r_max) = ($r_max, $r_min);
> > +		}
> > +		my (undef, $c_max) = $gs->find_rev_before($r_max, 1, $r_min);
> > +		my (undef, $c_min) = $gs->find_rev_after($r_min, 1, $r_max);
> > +		# If there are no commits in the range, both $c_max and $c_min
> > +		# will be undefined.  If there is at least 1 commit in the
> > +		# range, both will be defined.
> > +		return () if !defined $c_min;
> 
> Fair enough but I'd strengthen the test by writing something like:
> 		return () if not defined $c_min || not defined $c_max;
> unless you can prove that `rev_db_get' can never return `undef' or  
> something like that.

Will make this change.

> > +sub commit_log_separator {
> > +    return ('-' x 72) . "\n";
> > +}
> > +
> 
> This is basically a constant, I think that declaring it with a  
> prototype helps Perl to optimize it:
> sub commit_log_separator() {

Will do.

> > +echo  
> > ---------------------------------------------------------------------- 
> > -- > expected-separator
> 
> This will choke on shells with buggy/fragile `echo'.  I think it'd be  
> safer to use printf here.

Will do.

Thanks!

Dave

^ permalink raw reply

* [REPLACEMENT PATCH 3/6] push: support pushing HEAD to real branch name
From: Steffen Prohaska @ 2007-11-11 14:35 UTC (permalink / raw)
  To: git, ericsson Andreas; +Cc: Steffen Prohaska
In-Reply-To: <47370EDF.3090907@op5.se>

This teaches "push <remote> HEAD" to resolve HEAD on the local
side to its real branch name, e.g. master, and then act as if
the real branch name was specified. So we have a shorthand for
pushing the current branch. Besides HEAD, no other symbolic ref
is resolved.

Thanks to Daniel Barkalow <barkalow@iabervon.org> for suggesting
this implementation, which is much simpler than the
implementation proposed before.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 builtin-push.c        |    9 +++++++++
 t/t5516-fetch-push.sh |   17 +++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

prefixcmp() is simpler.

    Steffen

diff --git a/builtin-push.c b/builtin-push.c
index 6d1da07..54fba0e 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -44,6 +44,15 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
+		if (!strcmp("HEAD", ref)) {
+			unsigned char sha1_dummy[20];
+			ref = resolve_ref(ref, sha1_dummy, 1, NULL);
+			if (!ref)
+				die("HEAD cannot be resolved.");
+			if (prefixcmp(ref, "refs/heads/"))
+				die("HEAD cannot be resolved to branch.");
+			ref = xstrdup(ref + 11);
+		}
 		add_refspec(ref);
 	}
 }
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 86f9b53..b0ff488 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -244,6 +244,23 @@ test_expect_success 'push with colon-less refspec (4)' '
 
 '
 
+test_expect_success 'push with HEAD' '
+
+	mk_test heads/master &&
+	git checkout master &&
+	git push testrepo HEAD &&
+	check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with HEAD nonexisting at remote' '
+
+	mk_test heads/master &&
+	git checkout -b local master &&
+	git push testrepo HEAD &&
+	check_push_result $the_commit heads/local
+'
+
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&
-- 
1.5.3.5.643.g20245

^ permalink raw reply related

* Re: [PATCH 6/6] refactor fetch's ref matching to use ref_abbrev_matches_full_with_rules()
From: Jakub Narebski @ 2007-11-11 14:55 UTC (permalink / raw)
  To: git
In-Reply-To: <11947897092576-git-send-email-prohaska@zib.de>

Steffen Prohaska wrote:

> The old rules used by fetch were coded as a series of ifs.  The old
> rules are:
> 1) match full refname if it starts with "refs/" or matches "HEAD"
> 2) verify that full refname starts with "refs/"
> 3) match abbreviated name in "refs/" if it starts with "heads/",
>     "tags/", or "remotes/".
> 4) match abbreviated name in "refs/heads/"
> 
> This is replaced by the new rules
> a) match full refname
> b) match abbreviated name prefixed with "refs/"
> c) match abbreviated name prefixed with "refs/heads/"
> 
> The details of the new rules are different from the old rules.  We no
> longer verify that the full refname starts with "refs/".  The new rule
> (a) matches any full string.  The old rules (1) and (2) were stricter.
> Now, the caller is responsible for using sensible full refnames.  This
> should be the case for the current code.  The new rule (b) is less
> strict than old rule (3).  The new rule accepts abbreviated names that
> start with a non-standard prefix below "refs/".
> 
> Despite this modifications the new rules should handle all cases as
> expected.  Two tests are added to verify that fetch does not resolve
> short tags or HEAD in remotes.
> 
> We may even think about loosening the rules a bit more and unify them
> with the rev-parse rules.  This would be done by replacing
> ref_ref_fetch_rules with ref_ref_parse_rules.  Note, the two new test
> would break.

Does still "origin" matches "origin/HEAD" if we have emote "origin"?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: (no subject)
From: Johannes Schindelin @ 2007-11-11 15:22 UTC (permalink / raw)
  To: Michael Dressel; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711111359590.9401@castor.milkiway.cos>

Hi,

On Sun, 11 Nov 2007, Michael Dressel wrote:

> 
> >Michael Dressel wrote:
> >Ok nice. Another thing is that git-push will push all the tracking 
> >branches in refs/remotes/origin. 
> 
> I learned that I only have to edit the .git/config file to avoid that 
> git-push pushes everything. 

It is documented that you can use "git push origin <branchname>".

> [remote "origin1"]
>         url = /home/repo/src
>         fetch = +refs/heads/master:refs/remotes/origin/master
>         push = +refs/heads/master:refs/heads/master

With "push", it is not necessary to specify the ":<target>".

Also, if "master" is unambiguous, you can write just "master" instead of 
"refs/heads/master".

Furthermore, I suggest not forcing (that's  what "+" does) the push, since 
it is quite possible that you push something old in the wrong direction.  

Hth,
Dscho

^ permalink raw reply

* Re: t7005 and vi in GIT_EXEC_PATH
From: Johannes Schindelin @ 2007-11-11 15:58 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git Mailing List
In-Reply-To: <9A9986E7-E03D-458A-9A19-A3EF0E7B203D@silverinsanity.com>

Hi,

On Sat, 10 Nov 2007, Brian Gernhardt wrote:

> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the real 
> vi is invoked instead of the test vi script.  This is because the git 
> wrapper puts GIT_EXEC_PATH ahead of ".".  I see no easy solution to this 
> problem, and thought I should bring it up with the list.

I don't understand.  GIT_EXEC_PATH should be set to the build directory 
when you are running the tests.  Unless you copy vi _there_, you should 
not have any problem.

Ciao,
Dscho

^ permalink raw reply

* Re: t7005 and vi in GIT_EXEC_PATH
From: Brian Gernhardt @ 2007-11-11 16:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0711111557370.4362@racer.site>


On Nov 11, 2007, at 10:58 AM, Johannes Schindelin wrote:

>> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the  
>> real
>> vi is invoked instead of the test vi script.  This is because the git
>> wrapper puts GIT_EXEC_PATH ahead of ".".  I see no easy solution to  
>> this
>> problem, and thought I should bring it up with the list.
>
> I don't understand.  GIT_EXEC_PATH should be set to the build  
> directory
> when you are running the tests.  Unless you copy vi _there_, you  
> should
> not have any problem.

I'm sorry, I should have been more clear.  I was referring to the  
GIT_EXEC_PATH build variable, not the environment variable.  The git  
wrapper always adds the path determined during build to the front of  
PATH.  When I was changing my build script, this got set to "/usr/ 
local/bin" (I usually use /usr/local/stow/git, instead).  Since I have  
a /usr/local/bin/vim, PATH for git-commit.sh during the test was:

- my git build directory
- /usr/local/bin (containing a symlink vi -> vim)
- the t/trash directory, added by the test via `PATH=".:$PATH"`  
(containing the test vi script)
- my normal path

The test appeared to hang when running it normally.  When I ran it  
with -v, I saw that vim was started.

~~ Brian

^ permalink raw reply

* Re: t7005 and vi in GIT_EXEC_PATH
From: Johannes Schindelin @ 2007-11-11 16:28 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git Mailing List
In-Reply-To: <FCFF59B3-D3F1-4BEB-B3C3-D07DD5D5D8EF@silverinsanity.com>

Hi,

On Sun, 11 Nov 2007, Brian Gernhardt wrote:

> On Nov 11, 2007, at 10:58 AM, Johannes Schindelin wrote:
> 
> > > If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the 
> > > real vi is invoked instead of the test vi script.  This is because 
> > > the git wrapper puts GIT_EXEC_PATH ahead of ".".  I see no easy 
> > > solution to this problem, and thought I should bring it up with the 
> > > list.
> > 
> > I don't understand.  GIT_EXEC_PATH should be set to the build 
> > directory when you are running the tests.  Unless you copy vi _there_, 
> > you should not have any problem.
> 
> I'm sorry, I should have been more clear.  I was referring to the 
> GIT_EXEC_PATH build variable, not the environment variable.  The git 
> wrapper always adds the path determined during build to the front of 
> PATH.  When I was changing my build script, this got set to 
> "/usr/local/bin" (I usually use /usr/local/stow/git, instead).  Since I 
> have a /usr/local/bin/vim, PATH for git-commit.sh during the test was:
> 
> - my git build directory
> - /usr/local/bin (containing a symlink vi -> vim)
> - the t/trash directory, added by the test via `PATH=".:$PATH"` (containing
> the test vi script)
> - my normal path
> 
> The test appeared to hang when running it normally.  When I ran it with 
> -v, I saw that vim was started.

The obvious solution would be to copy "vi" into the git build directory 
for the test, or skip the test if that copy could not be performed.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 3/3] git-svn log: handle unreachable revisions like "svn log"
From: Benoit Sigoure @ 2007-11-11 16:36 UTC (permalink / raw)
  To: ddkilzer; +Cc: Git Mailing List
In-Reply-To: <189577.85054.qm@web52407.mail.re2.yahoo.com>

[-- Attachment #1: Type: text/plain, Size: 3769 bytes --]

On Nov 11, 2007, at 3:20 PM, David D. Kilzer wrote:

> Benoit Sigoure <tsuna@lrde.epita.fr> wrote:
>
>> On Nov 11, 2007, at 7:10 AM, David D Kilzer wrote:
>>
>>>  sub find_rev_before {
>>> -	my ($self, $rev, $eq_ok) = @_;
>>> +	my ($self, $rev, $eq_ok, $min_rev) = @_;
>>
>> Could you please document this function?  I guess that you had to
>> figure out what each argument was for, so please save the time of the
>> contributors that will read this code after you :)
>
> What is the format for documenting functions in git Perl scripts?   
> I haven't
> see any "perlpod" use anywhere.  Do you just want comments before  
> the function?
>
> This method returns the git commit hash and svn revision of the  
> first svn
> revision that exists on the current branch that is less than $rev (or
> less-than-or-equal-to $rev if $eq_ok is true).

Personally, I don't care.  Maybe Eric has his own preference.  For  
me, as long as the code is documented one way or another, that's fine  
by me.  Under-documented code hinders new contributors, so I think  
it's important to add documentation whenever possible.

> Please note that I don't have a full understanding of how  
> find_rev_before()
> works (other than it's computing an offset into a sparse? data file  
> based on
> the revision number) since I'm still new to git.
>
>>> +sub find_rev_after {
>>> +	my ($self, $rev, $eq_ok, $max_rev) = @_;
>>> +	++$rev unless $eq_ok;
>>> +	$max_rev ||= $self->rev_db_max();
>>> +	while ($rev <= $max_rev) {
>>> +		if (my $c = $self->rev_db_get($rev)) {
>>> +			return ($rev, $c);
>>> +		}
>>> +		++$rev;
>>> +	}
>>> +	return (undef, undef);
>>> +}
>>
>> Too much code duplication.  It should be possible to write a sub
>> find_rev_ (or _find_rev, don't know what's the naming convention for
>> internal details) that takes a 5th argument, an anonymous sub that
>> does the comparison.  So that basically, find_rev_before will be
>> something along these (untested) lines:
>>
>> sub find_rev_before {
>> 	my ($self, $rev, $eq_ok, $min_rev) = @_;
>> 	return find_rev_($self, $rev, $eq_ok, $min_rev, sub { my ($a, $b) =
>> @_; return $a >= $b; });
>> }
>
> I think that combining find_rev_before() and find_rev_after() would  
> greatly
> sacrifice readability of the code in exchange for removing ~10  
> lines of code.
> Also, you must do more than just replace the comparison in the while 
> () loop:
>
> - before() decrements $rev while after() increments it
> - stop limits are different ($max_rev versus $min_rev)
>
> This is what such a method might look like (untested).  Since you  
> already
> requested find_rev_before() be documented, is this really going to  
> help?
>
> sub find_rev_ {
> 	my ($self, $rev, $eq_ok, $is_before, $limit_rev) = @_;
> 	($is_before ? --$rev : ++$rev) unless $eq_ok;
> 	$limit_rev ||= ($is_before ? 1 : $self->rev_db_max());
> 	while ($is_before ? $rev >= $limit_rev : $rev <= $limit_rev) {
> 		if (my $c = $self->rev_db_get($rev)) {
> 			return ($rev, $c);
> 		}
> 			$is_before ? --$rev : ++$rev;
> 	}
> 	return (undef, undef);
> }
>
> Defining wrapper functions would help, but I still think it's just  
> as clear to
> keep the two methods separate.
>
> sub find_rev_before() {
> 	my ($self, $rev, $eq_ok, $min_rev) = @_;
> 	return $self->find_rev_($rev, $eq_ok, 1, $min_rev);
> }
>
> sub find_rev_after() {
> 	my ($self, $rev, $eq_ok, $max_rev) = @_;
> 	return $self->find_rev_($rev, $eq_ok, 0, $max_rev);
> }
>
> Do you agree, or do you think the methods should still be combined?

Er... you're right, I overlooked the differences between the two  
functions.  So merging them would make the code more complex than it  
needs to be, or so I think.

Thanks!

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* Installing without rebuilding
From: Brian Gernhardt @ 2007-11-11 16:49 UTC (permalink / raw)
  To: Git Mailing List

Git has a very clever Makefile.  Sometimes its a little overly clever.

1) I use stow to manage my /usr/local directory.  With many other  
programs I am able to build with a prefix of /usr/local and install  
with a prefix of /usr/local/stow/$program.  Git detects a change in  
the build variables and recompiles pretty much everything.

2) If I remove the old copy of git before installing the new, git will  
notice this and again start a (smaller) recompile because the GIT- 
VERSION-FILE file changes from something detailed like  
"1.5.3.5.622.g6fd7a" to "1.5.3.GIT".

Is there a way to tell git to be a bit less clever and just install  
the already compiled program?  If not, would changing the Makefile to  
read something like the following be accepted?

----- 8< -----

install: all install-dumb

install-dumb: # No rebuild!
   # Current install process

----- 8< -----

~~ Brian

^ permalink raw reply

* Re: Installing without rebuilding
From: Benoit Sigoure @ 2007-11-11 16:55 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git Mailing List
In-Reply-To: <8B92E213-17DB-4E83-B045-01CE0E7D26CB@silverinsanity.com>

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

On Nov 11, 2007, at 5:49 PM, Brian Gernhardt wrote:

> Git has a very clever Makefile.  Sometimes its a little overly clever.
>
> 1) I use stow to manage my /usr/local directory.  With many other  
> programs I am able to build with a prefix of /usr/local and install  
> with a prefix of /usr/local/stow/$program.  Git detects a change in  
> the build variables and recompiles pretty much everything.
>
> 2) If I remove the old copy of git before installing the new, git  
> will notice this and again start a (smaller) recompile because the  
> GIT-VERSION-FILE file changes from something detailed like  
> "1.5.3.5.622.g6fd7a" to "1.5.3.GIT".
>
> Is there a way to tell git to be a bit less clever and just install  
> the already compiled program?  If not, would changing the Makefile  
> to read something like the following be accepted?
>
> ----- 8< -----
>
> install: all install-dumb
>
> install-dumb: # No rebuild!
>   # Current install process
>
> ----- 8< -----

I also stow git (which doesn't come with a make uninstall grrrrrr)  
and what I do is that I dropped the dependency of `install' on  
`all'.  Also, I always install the new version before removing (stow - 
D) the previous one.

Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* [PATCH] Move stripspace() and launch_editor() to strbuf.c
From: Johannes Schindelin @ 2007-11-11 17:29 UTC (permalink / raw)
  To: git, gitster


These functions are already declared in strbuf.h, so it is only
logical to move their implementations to the corresponding file.
Particularly, since strbuf.h is in LIB_H, but both functions
were missing from libgit.a.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-stripspace.c |   67 --------------------------------
 builtin-tag.c        |   35 -----------------
 strbuf.c             |  103 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 102 deletions(-)

diff --git a/builtin-stripspace.c b/builtin-stripspace.c
index c0b2130..5de5a3f 100644
--- a/builtin-stripspace.c
+++ b/builtin-stripspace.c
@@ -1,73 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
 
-/*
- * Returns the length of a line, without trailing spaces.
- *
- * If the line ends with newline, it will be removed too.
- */
-static size_t cleanup(char *line, size_t len)
-{
-	while (len) {
-		unsigned char c = line[len - 1];
-		if (!isspace(c))
-			break;
-		len--;
-	}
-
-	return len;
-}
-
-/*
- * Remove empty lines from the beginning and end
- * and also trailing spaces from every line.
- *
- * Note that the buffer will not be NUL-terminated.
- *
- * Turn multiple consecutive empty lines between paragraphs
- * into just one empty line.
- *
- * If the input has only empty lines and spaces,
- * no output will be produced.
- *
- * If last line does not have a newline at the end, one is added.
- *
- * Enable skip_comments to skip every line starting with "#".
- */
-void stripspace(struct strbuf *sb, int skip_comments)
-{
-	int empties = 0;
-	size_t i, j, len, newlen;
-	char *eol;
-
-	/* We may have to add a newline. */
-	strbuf_grow(sb, 1);
-
-	for (i = j = 0; i < sb->len; i += len, j += newlen) {
-		eol = memchr(sb->buf + i, '\n', sb->len - i);
-		len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
-
-		if (skip_comments && len && sb->buf[i] == '#') {
-			newlen = 0;
-			continue;
-		}
-		newlen = cleanup(sb->buf + i, len);
-
-		/* Not just an empty line? */
-		if (newlen) {
-			if (empties > 0 && j > 0)
-				sb->buf[j++] = '\n';
-			empties = 0;
-			memmove(sb->buf + j, sb->buf + i, newlen);
-			sb->buf[newlen + j++] = '\n';
-		} else {
-			empties++;
-		}
-	}
-
-	strbuf_setlen(sb, j);
-}
-
 int cmd_stripspace(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf;
diff --git a/builtin-tag.c b/builtin-tag.c
index 566b9d1..c70564b 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -17,41 +17,6 @@ static const char builtin_tag_usage[] =
 
 static char signingkey[1000];
 
-void launch_editor(const char *path, struct strbuf *buffer)
-{
-	const char *editor, *terminal;
-
-	editor = getenv("GIT_EDITOR");
-	if (!editor && editor_program)
-		editor = editor_program;
-	if (!editor)
-		editor = getenv("VISUAL");
-	if (!editor)
-		editor = getenv("EDITOR");
-
-	terminal = getenv("TERM");
-	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
-		fprintf(stderr,
-		"Terminal is dumb but no VISUAL nor EDITOR defined.\n"
-		"Please supply the message using either -m or -F option.\n");
-		exit(1);
-	}
-
-	if (!editor)
-		editor = "vi";
-
-	if (strcmp(editor, ":")) {
-		const char *args[] = { editor, path, NULL };
-
-		if (run_command_v_opt(args, 0))
-			die("There was a problem with the editor %s.", editor);
-	}
-
-	if (strbuf_read_file(buffer, path, 0) < 0)
-		die("could not read message file '%s': %s",
-		    path, strerror(errno));
-}
-
 struct tag_filter {
 	const char *pattern;
 	int lines;
diff --git a/strbuf.c b/strbuf.c
index 536b432..6e99358 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "run-command.h"
 
 /*
  * Used as the default ->buf value, so that people can always assume
@@ -224,3 +225,105 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
 
 	return len;
 }
+
+/*
+ * Returns the length of a line, without trailing spaces.
+ *
+ * If the line ends with newline, it will be removed too.
+ */
+static size_t cleanup(char *line, size_t len)
+{
+	while (len) {
+		unsigned char c = line[len - 1];
+		if (!isspace(c))
+			break;
+		len--;
+	}
+
+	return len;
+}
+
+/*
+ * Remove empty lines from the beginning and end
+ * and also trailing spaces from every line.
+ *
+ * Note that the buffer will not be NUL-terminated.
+ *
+ * Turn multiple consecutive empty lines between paragraphs
+ * into just one empty line.
+ *
+ * If the input has only empty lines and spaces,
+ * no output will be produced.
+ *
+ * If last line does not have a newline at the end, one is added.
+ *
+ * Enable skip_comments to skip every line starting with "#".
+ */
+void stripspace(struct strbuf *sb, int skip_comments)
+{
+	int empties = 0;
+	size_t i, j, len, newlen;
+	char *eol;
+
+	/* We may have to add a newline. */
+	strbuf_grow(sb, 1);
+
+	for (i = j = 0; i < sb->len; i += len, j += newlen) {
+		eol = memchr(sb->buf + i, '\n', sb->len - i);
+		len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
+
+		if (skip_comments && len && sb->buf[i] == '#') {
+			newlen = 0;
+			continue;
+		}
+		newlen = cleanup(sb->buf + i, len);
+
+		/* Not just an empty line? */
+		if (newlen) {
+			if (empties > 0 && j > 0)
+				sb->buf[j++] = '\n';
+			empties = 0;
+			memmove(sb->buf + j, sb->buf + i, newlen);
+			sb->buf[newlen + j++] = '\n';
+		} else {
+			empties++;
+		}
+	}
+
+	strbuf_setlen(sb, j);
+}
+
+void launch_editor(const char *path, struct strbuf *buffer)
+{
+	const char *editor, *terminal;
+
+	editor = getenv("GIT_EDITOR");
+	if (!editor && editor_program)
+		editor = editor_program;
+	if (!editor)
+		editor = getenv("VISUAL");
+	if (!editor)
+		editor = getenv("EDITOR");
+
+	terminal = getenv("TERM");
+	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
+		fprintf(stderr,
+		"Terminal is dumb but no VISUAL nor EDITOR defined.\n"
+		"Please supply the message using either -m or -F option.\n");
+		exit(1);
+	}
+
+	if (!editor)
+		editor = "vi";
+
+	if (strcmp(editor, ":")) {
+		const char *args[] = { editor, path, NULL };
+
+		if (run_command_v_opt(args, 0))
+			die("There was a problem with the editor %s.", editor);
+	}
+
+	if (strbuf_read_file(buffer, path, 0) < 0)
+		die("could not read message file '%s': %s",
+		    path, strerror(errno));
+}
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* Re: [PATCH 6/6] refactor fetch's ref matching to use ref_abbrev_matches_full_with_rules()
From: Steffen Prohaska @ 2007-11-11 17:31 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <fh7548$15u$1@ger.gmane.org>


On Nov 11, 2007, at 3:55 PM, Jakub Narebski wrote:

> Steffen Prohaska wrote:
>
>> The old rules used by fetch were coded as a series of ifs.  The old
>> rules are:
>> 1) match full refname if it starts with "refs/" or matches "HEAD"
>> 2) verify that full refname starts with "refs/"
>> 3) match abbreviated name in "refs/" if it starts with "heads/",
>>     "tags/", or "remotes/".
>> 4) match abbreviated name in "refs/heads/"
>>
>> This is replaced by the new rules
>> a) match full refname
>> b) match abbreviated name prefixed with "refs/"
>> c) match abbreviated name prefixed with "refs/heads/"
>>
>> The details of the new rules are different from the old rules.  We no
>> longer verify that the full refname starts with "refs/".  The new  
>> rule
>> (a) matches any full string.  The old rules (1) and (2) were  
>> stricter.
>> Now, the caller is responsible for using sensible full refnames.   
>> This
>> should be the case for the current code.  The new rule (b) is less
>> strict than old rule (3).  The new rule accepts abbreviated names  
>> that
>> start with a non-standard prefix below "refs/".
>>
>> Despite this modifications the new rules should handle all cases as
>> expected.  Two tests are added to verify that fetch does not resolve
>> short tags or HEAD in remotes.
>>
>> We may even think about loosening the rules a bit more and unify them
>> with the rev-parse rules.  This would be done by replacing
>> ref_ref_fetch_rules with ref_ref_parse_rules.  Note, the two new test
>> would break.
>
> Does still "origin" matches "origin/HEAD" if we have emote "origin"?

No, and it did not before. fetch does _not_ match origin as
refs/remotes/origin/HEAD. I added a test to confirm the old
behaviour. Only the git-rev-parse rules match origin as
refs/remotes/origin/HEAD.

	Steffen

^ permalink raw reply

* [PATCH 0/6] Various (replacement) patches to builtin-commit
From: Johannes Schindelin @ 2007-11-11 17:35 UTC (permalink / raw)
  To: git, krh, gitster

Hi,

I did some more work on top of kh/commit (rebased to next, + Kristian's 
refresh_cache() patch which is still waiting to be enhanced with 
refresh_cache()-after-commit-with-paths).

They are in chronological order.  In particular, all patches except 2/6 
resurrect old behaviour.

Ciao,
Dscho

^ permalink raw reply

* [PATCH 1/6] builtin-commit: fix author date with --amend --author=<author>
From: Johannes Schindelin @ 2007-11-11 17:35 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711111730580.4362@racer.site>


When amending a commit with a new author, the author date is taken
from the original commit.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-commit.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index a84a729..6be6def 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -527,6 +527,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	} else if (amend) {
 		struct commit_list *c;
 		struct commit *commit;
+		const char *author;
 
 		reflog_msg = "commit (amend)";
 		commit = lookup_commit(head_sha1);
@@ -536,6 +537,21 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		for (c = commit->parents; c; c = c->next)
 			strbuf_addf(&sb, "parent %s\n",
 				      sha1_to_hex(c->item->object.sha1));
+
+		/* determine author date */
+		author = strstr(commit->buffer, "\nauthor");
+		if (author && !memmem(commit->buffer,
+					author + 1 - commit->buffer,
+					"\n\n", 2)) {
+			const char *email_end = strchr(author + 1, '>');
+			const char *line_end = strchr(author + 1, '\n');
+			if (email_end && line_end && email_end < line_end) {
+				char *date = xstrndup(email_end + 1,
+					line_end - email_end - 1);
+				setenv("GIT_AUTHOR_DATE", date, 1);
+				free(date);
+			}
+		}
 	} else if (in_merge) {
 		struct strbuf m;
 		FILE *fp;
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* [PATCH 2/6] git status: show relative paths when run in a subdirectory
From: Johannes Schindelin @ 2007-11-11 17:35 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711111730580.4362@racer.site>


To show the relative paths, the function formerly called quote_crlf()
(now called quote_path()) takes the prefix as an additional argument.

While at it, the static buffers were replaced by strbufs.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-commit.c    |   13 ++++---
 builtin-runstatus.c |    1 +
 t/t7502-status.sh   |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++
 wt-status.c         |   69 ++++++++++++++++++++++++++-------------
 wt-status.h         |    1 +
 5 files changed, 146 insertions(+), 29 deletions(-)
 create mode 100755 t/t7502-status.sh

diff --git a/builtin-commit.c b/builtin-commit.c
index 6be6def..5c7d4df 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -118,11 +118,12 @@ static char *prepare_index(const char **files, const char *prefix)
 	return next_index_lock->filename;
 }
 
-static int run_status(FILE *fp, const char *index_file)
+static int run_status(FILE *fp, const char *index_file, const char *prefix)
 {
 	struct wt_status s;
 
 	wt_status_prepare(&s);
+	s.prefix = prefix;
 
 	if (amend) {
 		s.amend = 1;
@@ -140,7 +141,7 @@ static int run_status(FILE *fp, const char *index_file)
 
 static const char sign_off_header[] = "Signed-off-by: ";
 
-static int prepare_log_message(const char *index_file)
+static int prepare_log_message(const char *index_file, const char *prefix)
 {
 	struct stat statbuf;
 	int commitable;
@@ -216,7 +217,7 @@ static int prepare_log_message(const char *index_file)
 	if (only_include_assumed)
 		fprintf(fp, "# %s\n", only_include_assumed);
 
-	commitable = run_status(fp, index_file);
+	commitable = run_status(fp, index_file, prefix);
 
 	fclose(fp);
 
@@ -409,7 +410,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	index_file = prepare_index(argv, prefix);
 
-	commitable = run_status(stdout, index_file);
+	commitable = run_status(stdout, index_file, prefix);
 
 	rollback_lock_file(&lock_file);
 
@@ -503,8 +504,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		exit(1);
 
-	if (!prepare_log_message(index_file) && !in_merge) {
-		run_status(stdout, index_file);
+	if (!prepare_log_message(index_file, prefix) && !in_merge) {
+		run_status(stdout, index_file, prefix);
 		unlink(commit_editmsg);
 		return 1;
 	}
diff --git a/builtin-runstatus.c b/builtin-runstatus.c
index 2db25c8..8d167a9 100644
--- a/builtin-runstatus.c
+++ b/builtin-runstatus.c
@@ -14,6 +14,7 @@ int cmd_runstatus(int argc, const char **argv, const char *prefix)
 
 	git_config(git_status_config);
 	wt_status_prepare(&s);
+	s.prefix = prefix;
 
 	for (i = 1; i < argc; i++) {
 		if (!strcmp(argv[i], "--color"))
diff --git a/t/t7502-status.sh b/t/t7502-status.sh
new file mode 100755
index 0000000..269b334
--- /dev/null
+++ b/t/t7502-status.sh
@@ -0,0 +1,91 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='git-status'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	: > tracked &&
+	: > modified &&
+	mkdir dir1 &&
+	: > dir1/tracked &&
+	: > dir1/modified &&
+	mkdir dir2 &&
+	: > dir1/tracked &&
+	: > dir1/modified &&
+	git add . &&
+	test_tick &&
+	git commit -m initial &&
+	: > untracked &&
+	: > dir1/untracked &&
+	: > dir2/untracked &&
+	echo 1 > dir1/modified &&
+	echo 2 > dir2/modified &&
+	echo 3 > dir2/added &&
+	git add dir2/added
+'
+
+cat > expect << \EOF
+# On branch master
+# Changes to be committed:
+#   (use "git reset HEAD <file>..." to unstage)
+#
+#	new file:   dir2/added
+#
+# Changed but not updated:
+#   (use "git add <file>..." to update what will be committed)
+#
+#	modified:   dir1/modified
+#
+# Untracked files:
+#   (use "git add <file>..." to include in what will be committed)
+#
+#	dir1/untracked
+#	dir2/modified
+#	dir2/untracked
+#	expect
+#	output
+#	untracked
+EOF
+
+test_expect_success 'status' '
+
+	git status > output &&
+	git diff expect output
+
+'
+
+cat > expect << \EOF
+# On branch master
+# Changes to be committed:
+#   (use "git reset HEAD <file>..." to unstage)
+#
+#	new file:   ../dir2/added
+#
+# Changed but not updated:
+#   (use "git add <file>..." to update what will be committed)
+#
+#	modified:   ../dir1/modified
+#
+# Untracked files:
+#   (use "git add <file>..." to include in what will be committed)
+#
+#	untracked
+#	../dir2/modified
+#	../dir2/untracked
+#	../expect
+#	../output
+#	../untracked
+EOF
+
+test_expect_success 'status with relative paths' '
+
+	(cd dir1 && git status) > output &&
+	git diff expect output
+
+'
+
+test_done
diff --git a/wt-status.c b/wt-status.c
index 03b5ec4..0d25362 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -82,33 +82,46 @@ static void wt_status_print_trailer(struct wt_status *s)
 	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
 }
 
-static const char *quote_crlf(const char *in, char *buf, size_t sz)
+static char *quote_path(const char *in, int len,
+		struct strbuf *out, const char *prefix)
 {
-	const char *scan;
-	char *out;
-	const char *ret = in;
+	if (len > 0)
+		strbuf_grow(out, len);
+	strbuf_setlen(out, 0);
+
+	if (prefix) {
+		int off = 0;
+		while (prefix[off] && off < len && prefix[off] == in[off])
+			if (prefix[off] == '/') {
+				prefix += off + 1;
+				in += off + 1;
+				len -= off + 1;
+				off = 0;
+			} else
+				off++;
+
+		for (; *prefix; prefix++)
+			if (*prefix == '/')
+				strbuf_addstr(out, "../");
+	}
 
-	for (scan = in, out = buf; *scan; scan++) {
-		int ch = *scan;
-		int quoted;
+	for (; (len < 0 && *in) || len > 0; in++, len--) {
+		int ch = *in;
 
 		switch (ch) {
 		case '\n':
-			quoted = 'n';
+			strbuf_addstr(out, "\\n");
 			break;
 		case '\r':
-			quoted = 'r';
+			strbuf_addstr(out, "\\r");
 			break;
 		default:
-			*out++ = ch;
+			strbuf_addch(out, ch);
 			continue;
 		}
-		*out++ = '\\';
-		*out++ = quoted;
-		ret = buf;
 	}
-	*out = '\0';
-	return ret;
+
+	return out->buf;
 }
 
 static void wt_status_print_filepair(struct wt_status *s,
@@ -116,10 +129,12 @@ static void wt_status_print_filepair(struct wt_status *s,
 {
 	const char *c = color(t);
 	const char *one, *two;
-	char onebuf[PATH_MAX], twobuf[PATH_MAX];
+	struct strbuf onebuf, twobuf;
 
-	one = quote_crlf(p->one->path, onebuf, sizeof(onebuf));
-	two = quote_crlf(p->two->path, twobuf, sizeof(twobuf));
+	strbuf_init(&onebuf, 0);
+	strbuf_init(&twobuf, 0);
+	one = quote_path(p->one->path, -1, &onebuf, s->prefix);
+	two = quote_path(p->two->path, -1, &twobuf, s->prefix);
 
 	color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
 	switch (p->status) {
@@ -151,6 +166,8 @@ static void wt_status_print_filepair(struct wt_status *s,
 		die("bug: unhandled diff status %c", p->status);
 	}
 	fprintf(s->fp, "\n");
+	strbuf_release(&onebuf);
+	strbuf_release(&twobuf);
 }
 
 static void wt_status_print_updated_cb(struct diff_queue_struct *q,
@@ -205,8 +222,9 @@ static void wt_read_cache(struct wt_status *s)
 static void wt_status_print_initial(struct wt_status *s)
 {
 	int i;
-	char buf[PATH_MAX];
+	struct strbuf buf;
 
+	strbuf_init(&buf, 0);
 	wt_read_cache(s);
 	if (active_nr) {
 		s->commitable = 1;
@@ -215,11 +233,12 @@ static void wt_status_print_initial(struct wt_status *s)
 	for (i = 0; i < active_nr; i++) {
 		color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
 		color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
-				quote_crlf(active_cache[i]->name,
-					   buf, sizeof(buf)));
+				quote_path(active_cache[i]->name, -1,
+					   &buf, s->prefix));
 	}
 	if (active_nr)
 		wt_status_print_trailer(s);
+	strbuf_release(&buf);
 }
 
 static void wt_status_print_updated(struct wt_status *s)
@@ -254,7 +273,9 @@ static void wt_status_print_untracked(struct wt_status *s)
 	const char *x;
 	int i;
 	int shown_header = 0;
+	struct strbuf buf;
 
+	strbuf_init(&buf, 0);
 	memset(&dir, 0, sizeof(dir));
 
 	dir.exclude_per_dir = ".gitignore";
@@ -291,9 +312,11 @@ static void wt_status_print_untracked(struct wt_status *s)
 			shown_header = 1;
 		}
 		color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
-		color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), "%.*s",
-				ent->len, ent->name);
+		color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), "%s",
+				quote_path(ent->name, ent->len,
+					&buf, s->prefix));
 	}
+	strbuf_release(&buf);
 }
 
 static void wt_status_print_verbose(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 7744932..f58ebcb 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -23,6 +23,7 @@ struct wt_status {
 	int workdir_untracked;
 	const char *index_file;
 	FILE *fp;
+	const char *prefix;
 };
 
 int git_status_config(const char *var, const char *value);
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* [PATCH 3/6] builtin-commit: fix --signoff
From: Johannes Schindelin @ 2007-11-11 17:35 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711111730580.4362@racer.site>


The Signed-off-by: line contained a spurious timestamp.  The reason was
a call to git_committer_info(1), which automatically added the
timestamp.

Instead, fmt_ident() was taught to interpret an empty string for the
date (as opposed to NULL, which still triggers the default behavior)
as "do not bother with the timestamp", and builtin-commit.c uses it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-commit.c  |   29 ++++++++++++++++++-----------
 ident.c           |   10 +++++++---
 t/t7500-commit.sh |   13 +++++++++++++
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 5c7d4df..6b1507d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -183,21 +183,28 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 		die("could not open %s\n", git_path(commit_editmsg));
 
 	stripspace(&sb, 0);
-	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
-		die("could not write commit template: %s\n",
-		    strerror(errno));
 
 	if (signoff) {
-		const char *info, *bol;
-
-		info = git_committer_info(1);
-		strbuf_addch(&sb, '\0');
-		bol = strrchr(sb.buf + sb.len - 1, '\n');
-		if (!bol || prefixcmp(bol, sign_off_header))
-			fprintf(fp, "\n");
-		fprintf(fp, "%s%s\n", sign_off_header, git_committer_info(1));
+		struct strbuf sob;
+		int i;
+
+		strbuf_init(&sob, 0);
+		strbuf_addstr(&sob, sign_off_header);
+		strbuf_addstr(&sob, fmt_ident(getenv("GIT_COMMITTER_NAME"),
+                         getenv("GIT_COMMITTER_EMAIL"), "", 1));
+		strbuf_addch(&sob, '\n');
+
+		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
+			; /* do nothing */
+		if (prefixcmp(sb.buf + i, sob.buf))
+			strbuf_addbuf(&sb, &sob);
+		strbuf_release(&sob);
 	}
 
+	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
+		die("could not write commit template: %s\n",
+		    strerror(errno));
+
 	strbuf_release(&sb);
 
 	if (in_merge && !no_edit)
diff --git a/ident.c b/ident.c
index 9b2a852..5be7533 100644
--- a/ident.c
+++ b/ident.c
@@ -224,13 +224,17 @@ const char *fmt_ident(const char *name, const char *email,
 	}
 
 	strcpy(date, git_default_date);
-	if (date_str)
-		parse_date(date_str, date, sizeof(date));
+	if (date_str) {
+		if (*date_str)
+			parse_date(date_str, date, sizeof(date));
+		else
+			date[0] = '\0';
+	}
 
 	i = copy(buffer, sizeof(buffer), 0, name);
 	i = add_raw(buffer, sizeof(buffer), i, " <");
 	i = copy(buffer, sizeof(buffer), i, email);
-	i = add_raw(buffer, sizeof(buffer), i, "> ");
+	i = add_raw(buffer, sizeof(buffer), i, date[0] ? "> " : ">");
 	i = copy(buffer, sizeof(buffer), i, date);
 	if (i >= sizeof(buffer))
 		die("Impossibly long personal identifier");
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index abbf54b..13d5a0c 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' '
 	commit_msg_is "standard input msg"
 '
 
+cat > expect << EOF
+zort
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+
+test_expect_success '--signoff' '
+	echo "yet another content *narf*" >> foo &&
+	echo "zort" |
+		GIT_EDITOR=../t7500/add-content git commit -s -F - foo &&
+	git cat-file commit HEAD | sed "1,/^$/d" > output &&
+	git diff expect output
+'
+
 test_done
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* [PATCH 4/6] builtin-commit --s: add a newline if the last line was no S-O-B
From: Johannes Schindelin @ 2007-11-11 17:36 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711111730580.4362@racer.site>


The rule is this: if the last line already contains the sign off by the
current committer, do nothing.  If it contains another sign off, just
add the sign off of the current committer.  If the last line does not
contain a sign off, add a new line before adding the sign off.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-commit.c  |    5 ++++-
 t/t7500-commit.sh |    1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 6b1507d..66d7e5e 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -196,8 +196,11 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 
 		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
 			; /* do nothing */
-		if (prefixcmp(sb.buf + i, sob.buf))
+		if (prefixcmp(sb.buf + i, sob.buf)) {
+			if (prefixcmp(sb.buf + i, sign_off_header))
+				strbuf_addch(&sb, '\n');
 			strbuf_addbuf(&sb, &sob);
+		}
 		strbuf_release(&sob);
 	}
 
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 13d5a0c..e0be2ed 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -95,6 +95,7 @@ test_expect_success 'commit message from file should override template' '
 
 cat > expect << EOF
 zort
+
 Signed-off-by: C O Mitter <committer@example.com>
 EOF
 
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* [PATCH 5/6] builtin-commit: resurrect behavior for multiple -m options
From: Johannes Schindelin @ 2007-11-11 17:36 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711111730580.4362@racer.site>


When more than one -m option is given, the message does not replace
the previous, but is appended.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-commit.c |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 66d7e5e..069d180 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -30,13 +30,27 @@ static char *use_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
 static struct lock_file lock_file;
 
-static char *logfile, *force_author, *message, *template_file;
+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;
 
 static int no_edit, initial_commit, in_merge;
 const char *only_include_assumed;
+struct strbuf message;
+
+static int opt_parse_m(const struct option *opt, const char *arg, int unset)
+{
+	struct strbuf *buf = opt->value;
+	if (unset)
+		strbuf_setlen(buf, 0);
+	else {
+		strbuf_addstr(buf, arg);
+		strbuf_addch(buf, '\n');
+		strbuf_addch(buf, '\n');
+	}
+	return 0;
+}
 
 static struct option builtin_commit_options[] = {
 	OPT__QUIET(&quiet),
@@ -45,7 +59,7 @@ static struct option builtin_commit_options[] = {
 
 	OPT_STRING('F', "file", &logfile, "FILE", "read log from file"),
 	OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
-	OPT_STRING('m', "message", &message, "MESSAGE", "specify commit message"),
+	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
@@ -150,8 +164,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 	FILE *fp;
 
 	strbuf_init(&sb, 0);
-	if (message) {
-		strbuf_add(&sb, message, strlen(message));
+	if (message.len) {
+		strbuf_addbuf(&sb, &message);
 	} else if (logfile && !strcmp(logfile, "-")) {
 		if (isatty(0))
 			fprintf(stderr, "(reading log message from standard input)\n");
@@ -321,7 +335,7 @@ static int parse_and_validate_options(int argc, const char *argv[])
 	argc = parse_options(argc, argv, builtin_commit_options,
 			     builtin_commit_usage, 0);
 
-	if (logfile || message || use_message)
+	if (logfile || message.len || use_message)
 		no_edit = 1;
 	if (edit_flag)
 		no_edit = 0;
@@ -346,7 +360,7 @@ static int parse_and_validate_options(int argc, const char *argv[])
 		f++;
 	if (f > 1)
 		die("Only one of -c/-C/-F can be used.");
-	if (message && f > 0)
+	if (message.len && f > 0)
 		die("Option -m cannot be combined with -c/-C/-F.");
 	if (edit_message)
 		use_message = edit_message;
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* [PATCH 6/6] builtin-commit: Add newline when showing which commit was created
From: Johannes Schindelin @ 2007-11-11 17:36 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711111730580.4362@racer.site>


The function log_tree_commit() does not break the line, so we have to
do it ourselves.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-commit.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 069d180..3739bfc 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -493,6 +493,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 	printf("Created %scommit ", initial_commit ? "initial " : "");
 
 	log_tree_commit(&rev, commit);
+	printf("\n");
 }
 
 int git_commit_config(const char *k, const char *v)
-- 
1.5.3.5.1693.g26ed

^ permalink raw reply related

* [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH
From: Björn Steinbrink @ 2007-11-11 17:38 UTC (permalink / raw)
  To: benji; +Cc: aroben, dak, Johannes.Schindelin, git
In-Reply-To: <9A9986E7-E03D-458A-9A19-A3EF0E7B203D@silverinsanity.com>

The git wrapper executable always prepends the GIT_EXEC_PATH build
variable to the current PATH, so prepending "." to the PATH is not
enough to give precedence to the fake vi executable.

The --exec-path option allows to prepend a directory to PATH even before
GIT_EXEC_PATH (which is added anyway), so we can use that instead.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
 t/t7005-editor.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index 01cc0c0..0b36ee1 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -61,7 +61,7 @@ do
 		;;
 	esac
 	test_expect_success "Using $i" '
-		git commit --amend &&
+		git --exec-path=. commit --amend &&
 		git show -s --pretty=oneline |
 		sed -e "s/^[0-9a-f]* //" >actual &&
 		diff actual expect
@@ -83,7 +83,7 @@ do
 		;;
 	esac
 	test_expect_success "Using $i (override)" '
-		git commit --amend &&
+		git --exec-path=. commit --amend &&
 		git show -s --pretty=oneline |
 		sed -e "s/^[0-9a-f]* //" >actual &&
 		diff actual expect
-- 
1.5.3.5.622.g6fd7a

^ permalink raw reply related

* Re: [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH
From: Johannes Schindelin @ 2007-11-11 17:44 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: benji, aroben, dak, git
In-Reply-To: <1194802691-27610-1-git-send-email-B.Steinbrink@gmx.de>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 675 bytes --]

Hi,

On Sun, 11 Nov 2007, Björn Steinbrink wrote:

> The git wrapper executable always prepends the GIT_EXEC_PATH build
> variable to the current PATH, so prepending "." to the PATH is not
> enough to give precedence to the fake vi executable.
> 
> The --exec-path option allows to prepend a directory to PATH even before
> GIT_EXEC_PATH (which is added anyway), so we can use that instead.

Hmm.  This will probably stop working when you do not have git installed, 
because you now tell git to search for git programs in ".", where they are 
not.  Probably git-commit executes your installed write-tree, commit-tree 
and friends, instead of the compiled ones.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH
From: Brian Gernhardt @ 2007-11-11 17:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Björn Steinbrink, aroben, dak, git
In-Reply-To: <Pine.LNX.4.64.0711111742010.4362@racer.site>


On Nov 11, 2007, at 12:44 PM, Johannes Schindelin wrote:

> Hi,
>
> On Sun, 11 Nov 2007, Björn Steinbrink wrote:
>
>> The git wrapper executable always prepends the GIT_EXEC_PATH build
>> variable to the current PATH, so prepending "." to the PATH is not
>> enough to give precedence to the fake vi executable.
>>
>> The --exec-path option allows to prepend a directory to PATH even  
>> before
>> GIT_EXEC_PATH (which is added anyway), so we can use that instead.
>
> Hmm.  This will probably stop working when you do not have git  
> installed,
> because you now tell git to search for git programs in ".", where  
> they are
> not.  Probably git-commit executes your installed write-tree, commit- 
> tree
> and friends, instead of the compiled ones.

You are wrong there.  From exec_cmd.c:setup_path() (lines 51-54):

     add_path(&new_path, argv_exec_path);
     add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
     add_path(&new_path, builtin_exec_path);
     add_path(&new_path, cmd_path);

So the path with this patch will still include the build directory  
before the install location.

~~ Brian

^ 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