[PATCH 1/4] Git.pm: config_boolean() -> config_bool()

[PATCH 1/4] Git.pm: config_boolean() -> config_bool()

[Petr Baudis]

This patch renames config_boolean() to config_bool() for consistency with
the commandline interface and because it is shorter but still obvious. ;-)
It also changes the return value from some obscure string to real Perl
boolean, allowing for clean user code.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 git-remote.perl |    4 ++--
 perl/Git.pm     |   12 ++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/git-remote.perl b/git-remote.perl
index 52013fe..5763799 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -297,9 +297,9 @@ sub update_remote {
 	} elsif ($name eq 'default') {
 		undef @remotes;
 		for (sort keys %$remote) {
-			my $do_fetch = $git->config_boolean("remote." . $_ .
+			my $do_fetch = $git->config_bool("remote." . $_ .
 						    ".skipDefaultUpdate");
-			if (!defined($do_fetch) || $do_fetch ne "true") {
+			unless ($do_fetch) {
 				push @remotes, $_;
 			}
 		}
diff --git a/perl/Git.pm b/perl/Git.pm
index b5b1cf5..50c2993 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -516,9 +516,11 @@ sub config {
 }
 
 
-=item config_boolean ( VARIABLE )
+=item config_bool ( VARIABLE )
 
-Retrieve the boolean configuration C<VARIABLE>.
+Retrieve the bool configuration C<VARIABLE>. The return value
+is usable as a boolean in perl (and C<undef> if it's not defined,
+of course).
 
 Must be called on a repository instance.
 
@@ -526,14 +528,16 @@ This currently wraps command('config') s
 
 =cut
 
-sub config_boolean {
+sub config_bool {
 	my ($self, $var) = @_;
 	$self->repo_path()
 		or throw Error::Simple("not a repository");
 
 	try {
-		return $self->command_oneline('config', '--bool', '--get',
+		my $var = $self->command_oneline('config', '--bool', '--get',
 					      $var);
+		return undef unless defined $var;
+		return $var eq 'true';
 	} catch Git::Error::Command with {
 		my $E = shift;
 		if ($E->value() == 1) {

[PATCH 2/4] Git.pm: Add remote_refs() git-ls-remote frontend

[Petr Baudis]

Should support all the important features, I guess. Too bad that
	git-ls-remote --heads .
	
is subtly different from

	git-ls-remote . refs/heads/

so we have to provide the interface for specifying both.

This patch also converts git-svn.perl to use it.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 git-remote.perl |    5 +----
 perl/Git.pm     |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/git-remote.perl b/git-remote.perl
index 5763799..5403d86 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -128,10 +128,7 @@ sub update_ls_remote {
 	return if (($harder == 0) ||
 		   (($harder == 1) && exists $info->{'LS_REMOTE'}));
 
-	my @ref = map {
-		s|^[0-9a-f]{40}\s+refs/heads/||;
-		$_;
-	} $git->command(qw(ls-remote --heads), $info->{'URL'});
+	my @ref = keys %{$git->remote_refs($info->{'URL'}, [ 'heads' ])};
 	$info->{'LS_REMOTE'} = \@ref;
 }
 
diff --git a/perl/Git.pm b/perl/Git.pm
index 50c2993..230bf89 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -51,7 +51,7 @@ require Exporter;
 # Methods which can be called as standalone functions as well:
 @EXPORT_OK = qw(command command_oneline command_noisy
                 command_output_pipe command_input_pipe command_close_pipe
-                version exec_path hash_object git_cmd_try);
+                version exec_path hash_object git_cmd_try remote_refs);
 
 
 =head1 DESCRIPTION
@@ -550,6 +550,59 @@ sub config_bool {
 }
 
 
+=item remote_refs ( REPOSITORY [, GROUPS [, REFGLOBS ] ] )
+
+This function returns a hashref of refs stored in a given remote repository.
+The hash is in the format C<refname =\> hash>. For tags, the C<refname> entry
+contains the tag object while a C<refname^{}> entry gives the tagged objects.
+
+C<REPOSITORY> has the same meaning as the appropriate C<git-ls-remote>
+argument; either an URL or a remote name (if called on a repository instance).
+C<GROUPS> is an optional arrayref that can contain 'tags' to return all the
+tags and/or 'heads' to return all the heads. C<REFGLOB> is an optional array
+of strings containing a shell-like glob to further limit the refs returned in
+the hash; the meaning is again the same as the appropriate C<git-ls-remote>
+argument.
+
+This function may or may not be called on a repository instance. In the former
+case, remote names as defined in the repository are recognized as repository
+specifiers.
+
+=cut
+
+sub remote_refs {
+	my ($self, $repo, $groups, $refglobs) = _maybe_self(@_);
+	my @args;
+	if (ref $groups eq 'ARRAY') {
+		foreach (@$groups) {
+			if ($_ eq 'heads') {
+				push (@args, '--heads');
+			} elsif ($_ eq 'tags') {
+				push (@args, '--tags');
+			} else {
+				# Ignore unknown groups for future
+				# compatibility
+			}
+		}
+	}
+	push (@args, $repo);
+	if (ref $refglobs eq 'ARRAY') {
+		push (@args, @$refglobs);
+	}
+
+	my @self = $self ? ($self) : (); # Ultra trickery
+	my ($fh, $ctx) = Git::command_output_pipe(@self, 'ls-remote', @args);
+	my %refs;
+	while (<$fh>) {
+		chomp;
+		my ($hash, $ref) = split(/\t/, $_, 2);
+		$refs{$ref} = $hash;
+	}
+	Git::command_close_pipe(@self, $fh, $ctx);
+	return \%refs;
+}
+
+
 =item ident ( TYPE | IDENTSTR )
 
 =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )

[PATCH 3/4] [PATCH] git-mirror - exactly mirror another repository

[Petr Baudis]

Sometimes its handy to be able to efficiently backup or mirror one
Git repository to another Git repository by employing the native
Git object transfer protocol.  But when mirroring or backing up a
repository you really want:

  1) Every object in the source to go to the mirror.
  2) Every ref in the source to go to the mirror.
  3) Any ref removed from the source to be removed from the mirror.

and since git-fetch doesn't do 2 and 3, here's a tool that does.

This is based on Shawn Pearce's patch from 25 Sep 2006, updated to take
Junio's and Sergey's review into account, to use few newer pieces of Git
infrastructure and with few trivial tweaks. The repacking part was dropped
since git-fetch does that on its own now.

I actually still would kind of prefer this to be a git-fetch's feature but
the general mood seems to be to have this as a separate command and I can't
say I care at all.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 .gitignore                   |    1 
 Documentation/config.txt     |    6 ++
 Documentation/git-mirror.txt |   54 +++++++++++++++++++++
 Makefile                     |    2 -
 git-mirror.perl              |  110 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 172 insertions(+), 1 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4dc0c39..d0b67da 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ git-merge-resolve
 git-merge-stupid
 git-merge-subtree
 git-mergetool
+git-mirror
 git-mktag
 git-mktree
 git-name-rev
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e0aff53..e05e4c5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -513,6 +513,12 @@ log.showroot::
 	Tools like gitlink:git-log[1] or gitlink:git-whatchanged[1], which
 	normally hide the root commit will now show it. True by default.
 
+mirror.allowed::
+	If true, gitlink:git-mirror[1] will be allowed to run on the
+	repository.  Please see its documentation for all the implications.
+
+mirror.
+
 merge.summary::
 	Whether to include summaries of merged commits in newly created
 	merge commit messages. False by default.
diff --git a/Documentation/git-mirror.txt b/Documentation/git-mirror.txt
new file mode 100644
index 0000000..0909124
--- /dev/null
+++ b/Documentation/git-mirror.txt
@@ -0,0 +1,54 @@
+git-mirror(1)
+============
+
+NAME
+----
+git-mirror - Exactly mirror another repository
+
+
+SYNOPSIS
+--------
+'git-mirror' <repository>
+
+
+DESCRIPTION
+-----------
+Completely mirrors another repository into the local repository.
+
+All heads and tags from the other repository are copied to the
+local repository without any regard for merging.  This means
+that all heads and tags will be FORCIBLY CHANGED in the local
+repository to make them match the other repository.  Any local
+ref or tags which has been deleted from the other repository
+will also be deleted from the local repository.
+
+After mirroring is complete, in case the 'HEAD' symref does not
+seem to match locally the remote one, `git-mirror` will attempt
+to guess which head is now the 'HEAD'; if there are more candidates,
+one will be chosen roughly randomly.
+
+
+CONFIGURATION
+-------------
+
+Prior to updating the local repository 'git-mirror' requires
+that the user set 'mirror.allowed' to a true value in the local
+repository's config file.  This is considered to be a safety
+latch which is intended to prevent accidental overwriting of
+the local repository.
+
+
+SEE ALSO
+--------
+gitlink:git-fetch[1]
+gitlink:git-prune[1]
+gitlink:git-repack[1]
+
+
+Author
+------
+Written by Shawn Pearce <spearce@spearce.org> and Petr Baudis <pasky@suse.cz>
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index 60c41fd..5fa370f 100644
--- a/Makefile
+++ b/Makefile
@@ -208,7 +208,7 @@ SCRIPT_PERL = \
 	git-archimport.perl git-cvsimport.perl git-relink.perl \
 	git-cvsserver.perl git-remote.perl \
 	git-svnimport.perl git-cvsexportcommit.perl \
-	git-send-email.perl git-svn.perl
+	git-send-email.perl git-svn.perl git-mirror.perl
 
 SCRIPT_PYTHON = \
 	git-p4import.py
diff --git a/git-mirror.perl b/git-mirror.perl
new file mode 100644
index 0000000..22ea879
--- /dev/null
+++ b/git-mirror.perl
@@ -0,0 +1,110 @@
+#!/usr/bin/env perl
+# Copyright (C) 2006, Shawn Pearce <spearce@spearce.org>
+# This file is licensed under the GPL v2, or a later version
+# at the discretion of Linus.
+
+use warnings;
+use strict;
+use Git;
+
+my $remote = shift || 'origin';
+my $repo = Git->repository();
+
+# Verify its OK to execute in this repository.
+#
+unless ($repo->config_bool('mirror.allowed')) {
+	print STDERR <<EOF;
+Error: mirror.allowed is false.
+Error:
+Error: For safety reasons please set mirror.allowed in this repository's
+Error: config before using this command.
+Error:
+Error: Unless you are using this repository ONLY for mirroring another
+Error: repository you probably don't want to do this. Please see the
+Error: manpage for more details.
+EOF
+	exit 1;
+}
+
+# Build our list of refs.
+#
+my $remote_refs = ls_refs($repo, $remote);
+my $local_refs = ls_refs($repo, $repo->repo_path());
+my $remote_HEAD = $remote_refs->{'HEAD'};
+my $local_HEAD = $local_refs->{'HEAD'};
+delete $remote_refs->{'HEAD'};
+delete $local_refs->{'HEAD'};
+
+# Execute the fetch for any refs which differ from our own.
+# We don't worry about trying to optimize for rewinds or
+# exact branch copies as they are rather uncommon.
+#
+my @to_fetch;
+while (my ($ref, $hash) = each %$remote_refs) {
+	push(@to_fetch, "$ref:$ref")
+		if (!$local_refs->{$ref} || $local_refs->{$ref} ne $hash);
+}
+if (@to_fetch) {
+	git_cmd_try {
+		$repo->command_noisy('fetch',
+			'--force',
+			'--update-head-ok',
+			$remote, sort @to_fetch);
+	} '%s failed w/ code %d';
+} else {
+	print "No changed refs.  Skipping fetch.\n";
+}
+
+# See what the remote has HEAD pointing at and update our local
+# HEAD to point at some ref which points at the same hash.
+# Prefer to keep HEAD the same if possible to avoid HEAD drifting
+# between different branches.
+# Note that with dumb protocols, we don't get to *know* HEAD implicitly
+# with git-ls-remote...
+#
+git_cmd_try {
+	my $headref = $repo->command_oneline('symbolic-ref', 'HEAD');
+	my $HEAD;
+	if (not $remote_refs->{$headref}) {
+		$HEAD = 'refs/heads/master';
+		print "Local HEAD branch disappeared, falling back to refs/heads/master\n";
+	} elsif ($remote_HEAD and $remote_refs->{$headref} ne $remote_HEAD) {
+		my %by_hash = map {$remote_refs->{$_} => $_}
+			grep {m,^refs/heads/,}
+			sort keys %$remote_refs;
+		$HEAD = $by_hash{$remote_HEAD};
+		if ($HEAD) {
+			print "Setting HEAD to $HEAD ($remote_HEAD)\n";
+		} else {
+			print "Remote HEAD ($remote_HEAD) does not match any remote branch\n";
+		}
+	}
+	if ($HEAD) {
+		$repo->command_noisy('symbolic-ref', 'HEAD', $HEAD);
+	}
+} '%s failed w/ code %d';
+
+# Delete any local refs which the server no longer contains.
+#
+foreach my $ref (keys %$local_refs) {
+	next if $remote_refs->{$ref};
+	print "Removing $ref\n";
+	git_cmd_try {
+		$repo->command_noisy('update-ref', '-d', $ref, $local_refs->{$ref});
+	} '%s failed w/ code %d';
+}
+
+sub ls_refs {
+	my $repo = shift;
+	my $name = shift;
+	my $refs = $repo->remote_refs($name);
+	my @interesting = grep {
+		$_ eq 'HEAD' or (/^refs\// and not /\.\./ and not /\^{}$/);
+	} keys %$refs;
+
+	my %refs2;
+	# This funky-looking expression puts @interesting-subset of %$refs
+	# to %refs2.
+	@refs2{@interesting} = @{$refs}{@interesting};
+	\%refs2;
+}

Re: [PATCH 3/4] [PATCH] git-mirror - exactly mirror another repository

[Shawn O. Pearce]

Petr Baudis <pasky@suse.cz> wrote:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e0aff53..e05e4c5 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -513,6 +513,12 @@ log.showroot::
>  	Tools like gitlink:git-log[1] or gitlink:git-whatchanged[1], which
>  	normally hide the root commit will now show it. True by default.
>  
> +mirror.allowed::
> +	If true, gitlink:git-mirror[1] will be allowed to run on the
> +	repository.  Please see its documentation for all the implications.
> +
> +mirror.
> +

Uhhh.... what's that mirror. line for?

> +++ b/git-mirror.perl

Otherwise, Ack'd.

-- 
Shawn.

[PATCH 4/4] server info: Add HEAD to info/refs

[Petr Baudis]

For some reason, HEAD was not listed in info/refs until now, which means
that git-ls-remote does not return it for dumb transports while the native
transports show it. It would be nice to have it because of git-mirror and
possibly other nifty stuff.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 server-info.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/server-info.c b/server-info.c
index f9be5a7..6cddb11 100644
--- a/server-info.c
+++ b/server-info.c
@@ -36,6 +36,7 @@ static int update_info_refs(int force)
 	info_ref_fp = fopen(path1, "w");
 	if (!info_ref_fp)
 		return error("unable to update %s", path0);
+	head_ref(add_info_ref, NULL);
 	for_each_ref(add_info_ref, NULL);
 	fclose(info_ref_fp);
 	rename(path1, path0);

Re: [PATCH 4/4] server info: Add HEAD to info/refs

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> For some reason, HEAD was not listed in info/refs until now, which means
> that git-ls-remote does not return it for dumb transports while the native
> transports show it. It would be nice to have it because of git-mirror and
> possibly other nifty stuff.

Hmmm.  I know why you want to do this, but honestly speaking,
this is going somewhat backwards.  Maybe one step back, before
going two steps forward.

I always considered it was a bug that the native transport sends
SHA-1 of HEAD after dereferencing the symref, instead of saying
which branch it points at.  You would understand (or more
likely, "remember", as I suspect you have in the past looked at
what git-clone does and git-clone-pack used to do) what yuck
factor this introduced to git-clone.

The info/refs files is designed to mimick peek-remote output (I
know you looked at ls-remote implementation that has "case-esac"
that switches on transport, whose output is piped to the same
filtering loop), but HEAD SHA-1 is not needed in this file to
support dumb transports, as we try to read HEAD symref by hand
when cloning over http.

Longer term, I would want to have a protocol extension to the
native stuff to additionally send the symref information, so
that git-clone does not have to guess.  At the same time, we
would probably want to update the output from git-peek-remote so
that it can say which branch HEAD points at more explicitly, and
in such way that does not break older clients too badly.  We
would need to update the file format of info/refs at the same
time if we update git-peek-remote.

How about proceeding along these lines?

 * We take this patch to add HEAD SHA-1 to info/refs;

 * We add --symref option to git-ls-remote; without the option,
   its last "while read sha1 path" loop filters out lines that
   begin with "->";

 * We add native protocol extension to let upload-pack to say
   what HEAD symref points at, in addition to the SHA-1 HEAD
   points at.  Update peek-remote to show this information like
   this:

	->refs/heads/master<TAB>HEAD<LF>
        0d5e6c97...<TAB>HEAD<LF>
	...

 * We also enhance update-server-info to generate similar ->
   information.

 * We update git-clone to use --symref when it calls
   git-ls-remote; if it can figure out which branch the remote
   HEAD points at (because the remote side has newer git), it
   uses that information to set the HEAD branch instead of
   guessing.

Hmm?

Re: [PATCH 4/4] server info: Add HEAD to info/refs

[Petr Baudis]

On Fri, Apr 27, 2007 at 05:51:36AM CEST, Junio C Hamano wrote:
> I always considered it was a bug that the native transport sends
> SHA-1 of HEAD after dereferencing the symref, instead of saying
> which branch it points at.

Yes, you are right. That was what I was originally after, but then got
sidetracked into working on something else and this easy bit was the
only thing left done, so I just passed it along. :)

> How about proceeding along these lines?

Looks generally good.

>  * We add native protocol extension to let upload-pack to say
>    what HEAD symref points at, in addition to the SHA-1 HEAD
>    points at.  Update peek-remote to show this information like
>    this:
> 
> 	->refs/heads/master<TAB>HEAD<LF>
>         0d5e6c97...<TAB>HEAD<LF>
> 	...

Yet another obscure syntax? Why not just reusing the actual ref: syntax?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

Re: [PATCH 4/4] server info: Add HEAD to info/refs

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> On Fri, Apr 27, 2007 at 05:51:36AM CEST, Junio C Hamano wrote:
>> I always considered it was a bug that the native transport sends
>> SHA-1 of HEAD after dereferencing the symref, instead of saying
>> which branch it points at.
>
> Yes, you are right. That was what I was originally after, but then got
> sidetracked into working on something else and this easy bit was the
> only thing left done, so I just passed it along. :)
>
>> How about proceeding along these lines?
>
> Looks generally good.
>
>>  * We add native protocol extension to let upload-pack to say
>>    what HEAD symref points at, in addition to the SHA-1 HEAD
>>    points at.  Update peek-remote to show this information like
>>    this:
>> 
>> 	->refs/heads/master<TAB>HEAD<LF>
>>         0d5e6c97...<TAB>HEAD<LF>
>> 	...
>
> Yet another obscure syntax? Why not just reusing the actual ref: syntax?

Yup, ref: is fine.  Please make it so, but after 1.5.2 ;-).

Re: [PATCH 1/4] Git.pm: config_boolean() -> config_bool()

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> This patch renames config_boolean() to config_bool() for consistency with
> the commandline interface and because it is shorter but still obvious. ;-)
> It also changes the return value from some obscure string to real Perl
> boolean, allowing for clean user code.

Doesn't this break send-email?

> @@ -526,14 +528,16 @@ This currently wraps command('config') s
>  
>  =cut
>  
> -sub config_boolean {
> +sub config_bool {
>  	my ($self, $var) = @_;
>  	$self->repo_path()
>  		or throw Error::Simple("not a repository");
>  
>  	try {
> -		return $self->command_oneline('config', '--bool', '--get',
> +		my $var = $self->command_oneline('config', '--bool', '--get',
>  					      $var);
> +		return undef unless defined $var;
> +		return $var eq 'true';

Did you mean to hide $var in the nested scope?

[PATCH] Git.pm: config_boolean() -> config_bool()

[Petr Baudis]

On Sat, Apr 28, 2007 at 08:27:42AM CEST, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > This patch renames config_boolean() to config_bool() for consistency with
> > the commandline interface and because it is shorter but still obvious. ;-)
> > It also changes the return value from some obscure string to real Perl
> > boolean, allowing for clean user code.
> 
> Doesn't this break send-email?

Oh, sorry! My grep must've been broken or something.

> > @@ -526,14 +528,16 @@ This currently wraps command('config') s
> >  
> >  =cut
> >  
> > -sub config_boolean {
> > +sub config_bool {
> >  	my ($self, $var) = @_;
> >  	$self->repo_path()
> >  		or throw Error::Simple("not a repository");
> >  
> >  	try {
> > -		return $self->command_oneline('config', '--bool', '--get',
> > +		my $var = $self->command_oneline('config', '--bool', '--get',
> >  					      $var);
> > +		return undef unless defined $var;
> > +		return $var eq 'true';
> 
> Did you mean to hide $var in the nested scope?

Hmm, I agree that having a different name for it will be more readable,
fixed.

So, I realized that I'm not sure again how to stick a mail reply and new
patch version in the same mail - originally I wanted to reply to this
mail and send the patch as another reply, but that seemed wasteful. Now
it seems that the only option is to stuff the mail reply in the diffstat
area, but I refuse to do that since that's just plainly stupid.

---

This patch renames config_boolean() to config_bool() for consistency with
the commandline interface and because it is shorter but still obvious. ;-)
It also changes the return value from some obscure string to real Perl
boolean, allowing for clean user code.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 git-remote.perl     |    4 ++--
 git-send-email.perl |    4 ++--
 perl/Git.pm         |   14 +++++++++-----
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/git-remote.perl b/git-remote.perl
index 52013fe..5763799 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -297,9 +297,9 @@ sub update_remote {
 	} elsif ($name eq 'default') {
 		undef @remotes;
 		for (sort keys %$remote) {
-			my $do_fetch = $git->config_boolean("remote." . $_ .
+			my $do_fetch = $git->config_bool("remote." . $_ .
 						    ".skipDefaultUpdate");
-			if (!defined($do_fetch) || $do_fetch ne "true") {
+			unless ($do_fetch) {
 				push @remotes, $_;
 			}
 		}
diff --git a/git-send-email.perl b/git-send-email.perl
index a6e3e02..404095f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -154,8 +154,8 @@ if ($@) {
 	$term = new FakeTerm "$@: going non-interactive";
 }
 
-my $def_chain = $repo->config_boolean('sendemail.chainreplyto');
-if ($def_chain and $def_chain eq 'false') {
+my $def_chain = $repo->config_bool('sendemail.chainreplyto');
+if (defined $def_chain and not $def_chain) {
     $chain_reply_to = 0;
 }
 
diff --git a/perl/Git.pm b/perl/Git.pm
index b5b1cf5..924470a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -516,9 +516,11 @@ sub config {
 }
 
 
-=item config_boolean ( VARIABLE )
+=item config_bool ( VARIABLE )
 
-Retrieve the boolean configuration C<VARIABLE>.
+Retrieve the bool configuration C<VARIABLE>. The return value
+is usable as a boolean in perl (and C<undef> if it's not defined,
+of course).
 
 Must be called on a repository instance.
 
@@ -526,14 +528,16 @@ This currently wraps command('config') so it is not so fast.
 
 =cut
 
-sub config_boolean {
+sub config_bool {
 	my ($self, $var) = @_;
 	$self->repo_path()
 		or throw Error::Simple("not a repository");
 
 	try {
-		return $self->command_oneline('config', '--bool', '--get',
-					      $var);
+		my $val = $self->command_oneline('config', '--bool', '--get',
+					      $val);
+		return undef unless defined $val;
+		return $val eq 'true';
 	} catch Git::Error::Command with {
 		my $E = shift;
 		if ($E->value() == 1) {



-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

Re: [PATCH] Git.pm: config_boolean() -> config_bool()

[Petr Baudis]

On Wed, May 09, 2007 at 12:49:41PM CEST, Petr Baudis wrote:
> @@ -526,14 +528,16 @@ This currently wraps command('config') so it is not so fast.
>  
>  =cut
>  
> -sub config_boolean {
> +sub config_bool {
>  	my ($self, $var) = @_;
>  	$self->repo_path()
>  		or throw Error::Simple("not a repository");
>  
>  	try {
> -		return $self->command_oneline('config', '--bool', '--get',
> -					      $var);
> +		my $val = $self->command_oneline('config', '--bool', '--get',
> +					      $val);
                                               ^^^

Sorry, should be $var. Looks like I forgot to make install or something
when "testing" the patch, also judging by the error messages repo.or.cz
spewed furiously in the last update round.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

Re: [PATCH] Git.pm: config_boolean() -> config_bool()

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Looks like I forgot to make install or something
> when "testing" the patch, also judging by the error messages repo.or.cz
> spewed furiously in the last update round.

Well, at least I can trust Git.pm and gitweb patches coming thru
you better than patches from elsewhere, as I would know if they
are broken too badly.  People would immediately notice the
breakage at repo.or.cz ;-).

Thanks.

Re: [PATCH] Git.pm: config_boolean() -> config_bool()

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> So, I realized that I'm not sure again how to stick a mail reply and new
> patch version in the same mail - originally I wanted to reply to this
> mail and send the patch as another reply, but that seemed wasteful. Now
> it seems that the only option is to stuff the mail reply in the diffstat
> area, but I refuse to do that since that's just plainly stupid.

That's your opinion to go against a convention, which is fine,
as long as you use something other than what begins with three
dashes here as a separator.

> ---
>
> This patch renames config_boolean() to config_bool() for consistency with
> the commandline interface and because it is shorter but still obvious. ;-)
> It also changes the return value from some obscure string to real Perl
> boolean, allowing for clean user code.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
>
>  git-remote.perl     |    4 ++--
>  git-send-email.perl |    4 ++--
>  perl/Git.pm         |   14 +++++++++-----
>  3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/git-remote.perl b/git-remote.perl
> index 52013fe..5763799 100755
> --- a/git-remote.perl
> +++ b/git-remote.perl
> @@ -297,9 +297,9 @@ sub update_remote {