Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: John Keeping @ 2013-01-29 19:48 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <1359334346-5879-4-git-send-email-davvid@gmail.com>

On Sun, Jan 27, 2013 at 04:52:25PM -0800, David Aguilar wrote:
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -2,6 +2,35 @@
>  # git-mergetool--lib is a library for common merge tool functions
>  MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>  
> +mode_ok () {
> +	diff_mode && can_diff ||
> +	merge_mode && can_merge
> +}
> +
> +is_available () {
> +	merge_tool_path=$(translate_merge_tool_path "$1") &&
> +	type "$merge_tool_path" >/dev/null 2>&1
> +}
> +
> +show_tool_names () {
> +	condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
> +
> +	( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |

Is the '*' necessary here?  I would expect ls to list the current
directory if given no arguments, but perhaps some platforms behave
differently?

> +	while read toolname
> +	do
> +		if setup_tool "$toolname" 2>/dev/null &&
> +			(eval "$condition" "$toolname")
> +		then
> +			if test -n "$preamble"
> +			then
> +				echo "$preamble"
> +				preamble=
> +			fi
> +			printf "%s%s\n" "$per_line_prefix" "$tool"
> +		fi
> +	done
> +}
> +
>  diff_mode() {
>  	test "$TOOL_MODE" = diff
>  }
> @@ -199,35 +228,21 @@ list_merge_tool_candidates () {
>  }
>  
>  show_tool_help () {
> -	unavailable= available= LF='
> -'
> -	for i in "$MERGE_TOOLS_DIR"/*
> -	do
> -		tool=$(basename "$i")
> -		setup_tool "$tool" 2>/dev/null || continue
> -
> -		merge_tool_path=$(translate_merge_tool_path "$tool")
> -		if type "$merge_tool_path" >/dev/null 2>&1
> -		then
> -			available="$available$tool$LF"
> -		else
> -			unavailable="$unavailable$tool$LF"
> -		fi
> -	done
> -
> -	cmd_name=${TOOL_MODE}tool
> +	tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
> +	available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
> +		"$tool_opt may be set to one of the following:")
> +	unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
> +		"The following tools are valid, but not currently available:")
>  	if test -n "$available"
>  	then
> -		echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
> -		echo "$available" | sort | sed -e 's/^/	/'
> +		echo "$available"
>  	else
>  		echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
>  	fi
>  	if test -n "$unavailable"
>  	then
>  		echo
> -		echo 'The following tools are valid, but not currently available:'
> -		echo "$unavailable" | sort | sed -e 's/^/	/'
> +		echo "$unavailable"
>  	fi
>  	if test -n "$unavailable$available"
>  	then

You haven't taken full advantage of the simplification Junio suggested
in response to v1 here.  We can change the "unavailable" block to be:

    show_tool_names 'mode_ok && ! is_available' "$TAB$TAB" \
        "${LF}The following tools are valid, but not currently available:"

If you also add a "not_found_msg" parameter to show_tool_names then the
"available" case is also simplified:

    show_tool_names 'mode_ok && is_available' "$TAB$TAB" \
        "$tool_opt may be set to one of the following:" \
        "No suitable tool for 'git $cmd_name --tool=<tool>' found."

with this at the end of show_tool_names:

    test -n "$preamble" && test -n "$not_found_msg" && \
        echo "$not_found_msg"


John

^ permalink raw reply

* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Junio C Hamano @ 2013-01-29 19:53 UTC (permalink / raw)
  To: Michal Nazarewicz; +Cc: git, Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <2f93ce7b6b5d3f6c6d1b99958330601a5560d4ba.1359486391.git.mina86@mina86.com>

Michal Nazarewicz <mpn@google.com> writes:

> From: Michal Nazarewicz <mina86@mina86.com>
>
> Make git-send-email read password from a ~/.authinfo file instead of
> requiring it to be stored in git configuration, passed as command line
> argument or typed in.

Makes one wonder why .authinfo and not .netrc; 

http://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html

phrases it amusingly:

        “Netrc” files are usually called .authinfo or .netr
        nowadays .authinfo seems to be more popular and the
        auth-source library encourages this confusion by accepting
        both

Either way it still encourages a plaintext password to be on disk,
which may not be what we want, even though it may be slight if not
really much of an improvement.  Again the Help-for-users has this
amusing bit:

	You could just say (but we don't recommend it, we're just
	showing that it's possible)

	     password mypassword

	to use the same password everywhere. Again, DO NOT DO THIS
	or you will be pwned as the kids say.

> +The '~/.authinfo' file is read if Text::CSV Perl module is installed
> +on the system; if it's missing, a notification message will be printed
> +and the file ignored altogether.  The file should contain a line with
> +the following format:
> ++
> +  machine <domain> port <port> login <user> password <pass>

It is rather strange to require a comma-separated-values parser to
read a file format this simple, isn't it?

> ++
> +Contrary to other tools, 'git-send-email' does not support symbolic
> +port names like 'imap' thus `<port>` must be a number.

Perhaps you can convert at least some popular ones yourself?  After
all, the user may be using an _existing_ .authinfo/.netrc that she
has been using with other programs that do understand symbolic port
names.  Rather than forcing all such users to update their files,
the patch can work a bit harder for them and the world will be a
better place, no?

^ permalink raw reply

* Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines
From: John Keeping @ 2013-01-29 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzjzrokag.fsf@alter.siamese.dyndns.org>

On Tue, Jan 29, 2013 at 11:34:31AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > Changes since v1:
> >
> > - Set 3.1 as the minimum Python 3 version
> >
> > - Remove the section on Unicode literals - it just adds confusion and
> >   doesn't apply to the current code; we can deal with any issues if they
> >   ever arise.
> > ...
> > + - We use the 'b' prefix for bytes literals.  Note that even though
> > +   the Python documentation for version 2.6 does not mention this
> > +   prefix it is supported since version 2.6.0.
> 
> Do we still need to single out the 'b' prefix?  Even if it were
> necessary, I'd like to see it toned down a bit to make it clear that
> most of the time you can write strings as strings without having to
> worry about the "is it unicode string or a string string" mess.
> Like
> 
>     - When you must make distinction between Unicode literals and
>       byte string literals, it is OK to use 'b' prefix.  Even though
>       ...
> 
> perhaps?

Yeah, that's better.

I want to include it because if you look in the Python documentation
then you'll be misled into thinking that it's not available on 2.6 and
we will be supporting that for a while since it is the version included
in RHEL 6.


John

^ permalink raw reply

* Re: gitk doesn't always shows boths tags in "gitk tag1..tag2"
From: Jonathan Nieder @ 2013-01-29 19:57 UTC (permalink / raw)
  To: Toralf Förster; +Cc: git
In-Reply-To: <510825B2.6070805@gmx.de>

Hi Toralf,

Toralf Förster wrote:

> $> git clone git://boinc.berkeley.edu/boinc.git
>
> the following 2 commands shows both starting and ending revisions :
>
> $> gitk client_release_7.0.41..client_release_7.0.42

gitk is running something similar to

	git log --graph --decorate --boundary --oneline <revs>

In this example, that means 7.0.41 is shown with an open circle
because it is at the boundary of the requested commit set (it is not
in that set and one of its children is).

[...]
> however this command doesn't show the tag "client_release_7.0.44" :
>
> $> gitk client_release_7.0.44..client_release_7.0.45

As you guessed, 7.0.45 seems to live in a different area of history. :)
I don't know why it was built that way --- there may or may not be a
good reason.

"gitk --simplify-by-decoration client_release_7.0.44...client_release_7.0.45"
can help to compare the positions of the two tags in history.

Thanks for a fun example.
Jonathan

^ permalink raw reply

* Re: gitk doesn't always shows boths tags in "gitk tag1..tag2"
From: Toralf Förster @ 2013-01-29 20:11 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git
In-Reply-To: <20130129195718.GD18266@google.com>

On 01/29/2013 08:57 PM, Jonathan Nieder wrote:
> As you guessed, 7.0.45 seems to live in a different area of history. :)
Well, seems be point to the root cause ..

BTW
$> gitk  --simplify-by-decoration  client_release_7.0.44..client_release_7.0.45

only 3 rows in the main window where 
$> gitk client_release_7.0.44..client_release_7.0.45

shows 468 rows. 


Thx for the quick answer :-)

-- 
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3

^ permalink raw reply

* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: Junio C Hamano @ 2013-01-29 20:22 UTC (permalink / raw)
  To: John Keeping; +Cc: David Aguilar, git
In-Reply-To: <20130129194846.GD1342@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

> On Sun, Jan 27, 2013 at 04:52:25PM -0800, David Aguilar wrote:
>> --- a/git-mergetool--lib.sh
>> +++ b/git-mergetool--lib.sh
>> @@ -2,6 +2,35 @@
>>  # git-mergetool--lib is a library for common merge tool functions
>>  MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>>  
>> +mode_ok () {
>> +	diff_mode && can_diff ||
>> +	merge_mode && can_merge
>> +}
>> +
>> +is_available () {
>> +	merge_tool_path=$(translate_merge_tool_path "$1") &&
>> +	type "$merge_tool_path" >/dev/null 2>&1
>> +}
>> +
>> +show_tool_names () {
>> +	condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
>> +
>> +	( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
>
> Is the '*' necessary here?

No, it was just from a bad habit (I have ls aliased to ls -A or ls
-a in my interactive environment, which trained my fingers to this).

I also think you can lose -1, although it does not hurt.
>> +	tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
>> +	available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
>> +		"$tool_opt may be set to one of the following:")
>> +	unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
>> +		"The following tools are valid, but not currently available:")
>>  	if test -n "$available"
>>  	then
>> -		echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
>> -		echo "$available" | sort | sed -e 's/^/	/'
>> +		echo "$available"
>>  	else
>>  		echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
>>  	fi
>>  	if test -n "$unavailable"
>>  	then
>>  		echo
>> -		echo 'The following tools are valid, but not currently available:'
>> -		echo "$unavailable" | sort | sed -e 's/^/	/'
>> +		echo "$unavailable"
>>  	fi
>>  	if test -n "$unavailable$available"
>>  	then
>
> You haven't taken full advantage of the simplification Junio suggested
> in response to v1 here.  We can change the "unavailable" block to be:
>
>     show_tool_names 'mode_ok && ! is_available' "$TAB$TAB" \
>         "${LF}The following tools are valid, but not currently available:"

Actually I was hoping that we can enhance show_tool_names so that we
can do without the $available and $unavailable variables at all.

> If you also add a "not_found_msg" parameter to show_tool_names then the
> "available" case is also simplified:
>
>     show_tool_names 'mode_ok && is_available' "$TAB$TAB" \
>         "$tool_opt may be set to one of the following:" \
>         "No suitable tool for 'git $cmd_name --tool=<tool>' found."
>
> with this at the end of show_tool_names:
>
>     test -n "$preamble" && test -n "$not_found_msg" && \
>         echo "$not_found_msg"

Yes, something along that line.

^ permalink raw reply

* Re: gitk doesn't always shows boths tags in "gitk tag1..tag2"
From: Jonathan Nieder @ 2013-01-29 20:25 UTC (permalink / raw)
  To: Toralf Förster; +Cc: git
In-Reply-To: <51082CFA.4050501@gmx.de>

Toralf Förster wrote:
> On 01/29/2013 08:57 PM, Jonathan Nieder wrote:

>> As you guessed, 7.0.45 seems to live in a different area of history. :)
>
> Well, seems be point to the root cause ..
>
> BTW
> $> gitk  --simplify-by-decoration  client_release_7.0.44..client_release_7.0.45
>
> only 3 rows in the main window where 
> $> gitk client_release_7.0.44..client_release_7.0.45

Easiest to look at

  gitk --simplify-by-decoration client_release_7.0.44...client_release_7.0.45

(three dots --- search for "symmetric difference" in gitrevisions(7) for
details).

Ciao,
Jonathan

^ permalink raw reply

* Re: [PATCH/RFC] git-blame.el: truncate author to avoid jagged left edge of code
From: David Kågedal @ 2013-01-29 20:17 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Jakub Narebski, Alexandre Julliard, git, Kevin Ryde,
	Sergei Organov, Rüdiger Sonderfeld
In-Reply-To: <20120610082437.GA29674@burratino>

Sorry for being absent a long time. I hope you have managed to sort out
the git-blame fixes anyway.

Jonathan Nieder <jrnieder@gmail.com> writes:

>Without this patch, the author column in git-blame-mode spills
>over in some rows:
>
>	822a7d <ramsay@ramsay1.demon.co.uk>:const char git_usage_
>	f2dd8c <jon.seymour@gmail.com>: "git [--version] [--exec-
>	a1bea2 <josh@joshtriplett.org>: "           [-p|--paginat
>	a1bea2 <josh@joshtriplett.org>: "           [--git-dir=<p
>	293b07 <trast@student.ethz.ch>: "           [-c name=valu
>	62b469 <stepan.nemec@gmail.com>:        "           <comm
>	822a7d <ramsay@ramsay1.demon.co.uk>:
>
>As a result, code meant to line up does not line up correctly and the
>colored code area has a jagged left edge.
>
>Specify a maximum width for the autohr email address in the default
>blame prefix (i.e., use %20.20A instead of %20A) to fix it.
>
>	822a7d <ramsay@ramsay1.demon.c:const char git_usage_strin
>	f2dd8c <jon.seymour@gmail.com>: "git [--version] [--exec-
>
>The (format) function used to implement format-spec has supported
>precision specifiers like ".20" in emacs since 2002-12-09, so this
>should be safe.
>
>Helped-by: Kevin Ryde <user42@zip.com.au>
>Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>---
>In February of 2011, I wrote:
>
>>  - email addresses are often longer than 20 characters.  Does
>>    format-spec provide a way to truncate to a certain length,
>>    so the prefixes can line up?
>
>Better late than never, I guess.  Sensible?
>
> contrib/emacs/git-blame.el |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
>index d351cfb6..137d5ba9 100644
>--- a/contrib/emacs/git-blame.el
>+++ b/contrib/emacs/git-blame.el
>@@ -102,7 +102,7 @@
>   :group 'git-blame)
> 
> (defcustom git-blame-prefix-format
>-  "%h %20A:"
>+  "%h %20.20A:"
>   "The format of the prefix added to each line in `git-blame'
> mode. The format is passed to `format-spec' with the following format keys:

-- 
David Kågedal

^ permalink raw reply

* [PATCHv2] git-send-email: add ~/.authinfo parsing
From: Michal Nazarewicz @ 2013-01-29 21:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <7vvcafojf4.fsf@alter.siamese.dyndns.org>

From: Michal Nazarewicz <mina86@mina86.com>

Make git-send-email read password from a ~/.authinfo or a ~/.netrc
file instead of requiring it to be stored in git configuration, passed
as command line argument or typed in.

There are various other applications that use this file for
authentication information so letting users use it for git-send-email
is convinient.  Furthermore, some users store their ~/.gitconfig file
in a public repository and having to store password there makes it
easy to publish the password.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 Documentation/git-send-email.txt |  34 +++++++++--
 git-send-email.perl              | 124 +++++++++++++++++++++++++++++++++++----
 2 files changed, 140 insertions(+), 18 deletions(-)

On Tue, Jan 29 2013, Junio C Hamano wrote:
> Makes one wonder why .authinfo and not .netrc; 

Fine… Let's parse both. ;)

> Either way it still encourages a plaintext password to be on disk,
> which may not be what we want, even though it may be slight if not
> really much of an improvement.

Well… Users store passwords on disks in a lot of places.  I wager that
most have mail clients configured not to ask for password but instead
store it on hard drive.  I don't see that changing any time soon, so
at least we can try and minimise number of places where a password is
stored.

> It is rather strange to require a comma-separated-values parser to
> read a file format this simple, isn't it?

I was worried about spaces in password.  CVS should handle such case
nicely, whereas simple split won't.  Nonetheless, I guess that in the
end this is not likely enough to add the dependency.

> Perhaps you can convert at least some popular ones yourself?  After
> all, the user may be using an _existing_ .authinfo/.netrc that she
> has been using with other programs that do understand symbolic port
> names.  Rather than forcing all such users to update their files,
> the patch can work a bit harder for them and the world will be a
> better place, no?

Parsing /etc/services added.

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..ee20714 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -158,14 +158,36 @@ Sending
 --smtp-pass[=<password>]::
 	Password for SMTP-AUTH. The argument is optional: If no
 	argument is specified, then the empty string is used as
-	the password. Default is the value of 'sendemail.smtppass',
-	however '--smtp-pass' always overrides this value.
+	the password. Default is the value of 'sendemail.smtppass'
+	or value read from ~/.authinfo file, however '--smtp-pass'
+	always overrides this value.
 +
-Furthermore, passwords need not be specified in configuration files
-or on the command line. If a username has been specified (with
+Furthermore, passwords need not be specified in configuration files or
+on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass', 'sendemail.smtppass' or via
+~/.authinfo file), then the user is prompted for a password while
+the input is masked for privacy.
++
+The ~/.authinfo file should contain a line with the following
+format:
++
+  machine <domain> port <port> login <user> password <pass>
++
+Each pair (expect for `password <pass>`) can be omitted which will
+skip matching of the given value.  Lines are interpreted in order and
+password from the first line that matches will be used.  `<port>` can
+be either an integer or a symbolic name.  In the latter case, it is
+looked up in `/etc/services` file (if it exists).  For instance, you
+can put
++
+  machine example.com login testuser port ssmtp password smtppassword
+  machine example.com login testuser            password testpassword
++
+if you want to use `smtppassword` for authenticating to a service at
+port 465 (SSMTP) and `testpassword` for all other services.  As shown
+in the example, `<port>` can use   If ~/.authinfo file is
+missing, 'git-send-email' will also try ~/.netrc file.
 
 --smtp-server=<host>::
 	If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..2d8fd1b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,117 @@ sub maildomain {
 	return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+
+sub read_password_from_stdin {
+	my $line;
+
+	system "stty -echo";
+
+	do {
+		print "Password: ";
+		$line = <STDIN>;
+		print "\n";
+	} while (!defined $line);
+
+	system "stty echo";
+
+	chomp $line;
+	return $line;
+}
+
+sub read_etc_services {
+	my $fd;
+	if (!open $fd, '<', '/etc/services') {
+		return {};
+	}
+
+	my $ret = {};
+	while (my $line = <$fd>) {
+		$line =~ s/^\s+|\s*(?:#.*)?$//g;
+		my @line = split /\s+/, $line;
+		if (@line < 2 || $line[1] !~ m~^(\d+)/tcp$~) {
+			next;
+		}
+
+		my $num = int $1;
+		undef $line[1];
+		for my $service (@line) {
+			if (defined $service && !defined $ret->{$service}) {
+				$ret->{$service} = $num;
+			}
+		}
+	}
+
+	close $fd;
+	return $ret;
+}
+
+my $authinfo_parse_port;
+
+sub authinfo_is_eq_port {
+	my ($from_file, $value, $filename) = @_;
+
+	if (!defined $from_file) {
+		return 1;
+	} elsif ($from_file =~ /^\d+$/) {
+		return $from_file == $value;
+	}
+
+	if (!defined $authinfo_parse_port) {
+		$authinfo_parse_port = read_etc_services;
+	}
+
+	my $port = $authinfo_parse_port->{$from_file};
+	if (!defined $port) {
+		print STDERR "$filename: invalid port name: $from_file\n";
+		return;
+	}
+
+	return $port == $value;
+}
+
+sub authinfo_is_eq {
+	my ($from_file, $value) = @_;
+	return defined $from_file || $from_file eq $value;
+}
+
+sub read_password_from_authinfo {
+	my $filename = join '/', $ENV{'HOME'}, $_[0] // '.authinfo';
+	my $fd;
+	if (!open $fd, '<', $filename) {
+		return;
+	}
+
+	my $password;
+	while (my $line = <$fd>) {
+		$line =~ s/^\s+|\s+$//g;
+		my @line = split /\s+/, $line;
+		if (@line % 2) {
+			next;
+		}
+
+		my %line = @line;
+		if (defined $line{'password'} &&
+		    authinfo_is_eq $line{'machine'}, $smtp_server &&
+		    authinfo_is_eq $line{'login'}, $smtp_authuser &&
+		    authinfo_is_eq_port $line{'port'}, $smtp_server_port, $filename) {
+			$password = $line{'password'};
+			last;
+		}
+	}
+
+	close $fd;
+	return $password;
+}
+
+sub read_password {
+	return
+	  read_password_from_authinfo '.authinfo' ||
+	  read_password_from_authinfo '.netrc' ||
+	  read_password_from_stdin;
+}
+
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1194,18 +1305,7 @@ X-Mailer: git-send-email $gitversion
 			};
 
 			if (!defined $smtp_authpass) {
-
-				system "stty -echo";
-
-				do {
-					print "Password: ";
-					$_ = <STDIN>;
-					print "\n";
-				} while (!defined $_);
-
-				chomp($smtp_authpass = $_);
-
-				system "stty echo";
+				$smtp_authpass = read_password
 			}
 
 			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
-- 
1.8.1

^ permalink raw reply related

* Re: [PATCH 0/2] optimizing pack access on "read only" fetch repos
From: Jeff King @ 2013-01-29 21:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7vwquwrng6.fsf@alter.siamese.dyndns.org>

On Tue, Jan 29, 2013 at 07:58:01AM -0800, Junio C Hamano wrote:

> The point is not about space.  Disk is cheap, and it is not making
> it any worse than what happens to your target audience, that is a
> fetch-only repository with only "gc --auto" in it, where nobody
> passes "-f" to "repack" to cause recomputation of delta.
> 
> What I was trying to seek was a way to reduce the runtime penalty we
> pay every time we run git in such a repository.
> 
>  - Object look-up cost will become log2(50*n) from 50*log2(n), which
>    is about 50/log2(50) improvement;

Yes and no. Our heuristic is to look at the last-used pack for an
object. So assuming we have locality of requests, we should quite often
get "lucky" and find the object in the first log2 search. Even if we
don't assume locality, a situation with one large pack and a few small
packs will have the large one as "last used" more often than the others,
and it will also have the looked-for object more often than the others

So I can see how it is something we could potentially optimize, but I
could also see it being surprisingly not a big deal. I'd be very
interested to see real measurements, even of something as simple as a
"master index" which can reference multiple packfiles.

>  - System resource cost we incur by having to keep 50 file
>    descriptors open and maintaining 50 mmap windows will reduce by
>    50 fold.

I wonder how measurable that is (and if it matters on Linux versus less
efficient platforms).

> > I would be interested to see the timing on how quick it is compared to a
> > real repack,...
> 
> Yes, that is what I meant by "wonder if we would be helped by" ;-)

There is only one way to find out... :)

Maybe I am blessed with nice machines, but I have mostly found the
repack process not to be that big a deal these days (especially with
threaded delta compression).

> > But how do these somewhat mediocre concatenated packs get turned into
> > real packs?
> 
> How do they get processed in a fetch-only repositories that
> sometimes run "gc --auto" today?  By runninng "repack -a -d -f"
> occasionally, perhaps?

Do we run "repack -adf" regularly? The usual "git gc" procedure will not
use "-f", and without that, we will not even consider making deltas
between objects that were formerly in different packs (but now are in
the same pack).

So you are avoiding doing medium-effort packs ("repack -ad") in favor of
doing potentially quick packs, but occasionally doing a big-effort pack
("repack -adf"). It may be reasonable advice to "repack -adf"
occasionally, but I suspect most people are not doing it regularly (if
only because "git gc" does not do it by default).

-Peff

^ permalink raw reply

* Re: [PATCH v2 1/4] mergetool--lib: Simplify command expressions
From: David Aguilar @ 2013-01-29 22:09 UTC (permalink / raw)
  To: John Keeping; +Cc: Junio C Hamano, git
In-Reply-To: <20130129192204.GC1342@serenity.lan>

On Tue, Jan 29, 2013 at 11:22 AM, John Keeping <john@keeping.me.uk> wrote:
> On Sun, Jan 27, 2013 at 04:52:23PM -0800, David Aguilar wrote:
>> Update variable assignments to always use $(command "$arg")
>> in their RHS instead of "$(command "$arg")" as the latter
>> is harder to read.  Make get_merge_tool_cmd() simpler by
>> avoiding "echo" and $(command) substitutions completely.
>>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>> @@ -300,9 +292,9 @@ get_merge_tool_path () {
>>       fi
>>       if test -z "$merge_tool_path"
>>       then
>> -             merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
>> +             merge_tool_path=$(translate_merge_tool_path "$merge_tool")
>>       fi
>> -     if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
>> +     if test -z $(get_merge_tool_cmd "$merge_tool") &&
>
> This change should be reverted to avoid calling "test -z" without any
> other arguments, as Johannes pointed out in v1.
>
> The rest of this patch looks good to me.

You're right.  My eyes have probably been staring at it too long and I
missed this (even though I thought I had checked).

Junio, how would you like these patches?
Incrementals on top of da/mergetool-docs?

I won't be able to get to them until later tonight (PST) at the
earliest, though.
-- 
David

^ permalink raw reply

* Re: [PATCH 0/2] optimizing pack access on "read only" fetch repos
From: Junio C Hamano @ 2013-01-29 22:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20130129211932.GA17377@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> > But how do these somewhat mediocre concatenated packs get turned into
>> > real packs?
>> 
>> How do they get processed in a fetch-only repositories that
>> sometimes run "gc --auto" today?  By runninng "repack -a -d -f"
>> occasionally, perhaps?
>
> Do we run "repack -adf" regularly? The usual "git gc" procedure will not
> use "-f", and without that, we will not even consider making deltas
> between objects that were formerly in different packs (but now are in
> the same pack).

Correct.  It is not a new problem, and while I think it would need
some solution, the "coalesce 50 small young packs into one" is not
an attempt to address it.

> So you are avoiding doing medium-effort packs ("repack -ad") in favor of
> doing potentially quick packs, but occasionally doing a big-effort pack
> ("repack -adf").

So I think "but occasionally" part is not correct.  In either way,
the packs use suboptimal delta, and you have to eventually pack with
"-f", whether you coalesce 50 packs into 1 often or keep paying the
cost of having 50 packs longer.

The trade-off is purely between "one potentially quick coalescing
per fetch in fetch-only repository" vs "any use of the data in the
fetch-only repository (what do they do?  build?  serving gitweb
locally?) has to deal with 25 packs on average, and we still need to
pay medium repack cost every 50 fetches".

^ permalink raw reply

* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: David Aguilar @ 2013-01-29 22:27 UTC (permalink / raw)
  To: John Keeping; +Cc: git, Junio C Hamano
In-Reply-To: <7vr4l3oi1z.fsf@alter.siamese.dyndns.org>

On Tue, Jan 29, 2013 at 12:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> John Keeping <john@keeping.me.uk> writes:
>
>> On Sun, Jan 27, 2013 at 04:52:25PM -0800, David Aguilar wrote:
>>> --- a/git-mergetool--lib.sh
>>> +++ b/git-mergetool--lib.sh
>>> @@ -2,6 +2,35 @@
>>>  # git-mergetool--lib is a library for common merge tool functions
>>>  MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>>>
>>> +mode_ok () {
>>> +    diff_mode && can_diff ||
>>> +    merge_mode && can_merge
>>> +}
>>> +
>>> +is_available () {
>>> +    merge_tool_path=$(translate_merge_tool_path "$1") &&
>>> +    type "$merge_tool_path" >/dev/null 2>&1
>>> +}
>>> +
>>> +show_tool_names () {
>>> +    condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
>>> +
>>> +    ( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
>>
>> Is the '*' necessary here?
>
> No, it was just from a bad habit (I have ls aliased to ls -A or ls
> -a in my interactive environment, which trained my fingers to this).
>
> I also think you can lose -1, although it does not hurt.
>>> +    tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
>>> +    available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
>>> +            "$tool_opt may be set to one of the following:")
>>> +    unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
>>> +            "The following tools are valid, but not currently available:")
>>>      if test -n "$available"
>>>      then
>>> -            echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
>>> -            echo "$available" | sort | sed -e 's/^/ /'
>>> +            echo "$available"
>>>      else
>>>              echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
>>>      fi
>>>      if test -n "$unavailable"
>>>      then
>>>              echo
>>> -            echo 'The following tools are valid, but not currently available:'
>>> -            echo "$unavailable" | sort | sed -e 's/^/       /'
>>> +            echo "$unavailable"
>>>      fi
>>>      if test -n "$unavailable$available"
>>>      then
>>
>> You haven't taken full advantage of the simplification Junio suggested
>> in response to v1 here.  We can change the "unavailable" block to be:
>>
>>     show_tool_names 'mode_ok && ! is_available' "$TAB$TAB" \
>>         "${LF}The following tools are valid, but not currently available:"
>
> Actually I was hoping that we can enhance show_tool_names so that we
> can do without the $available and $unavailable variables at all.
>
>> If you also add a "not_found_msg" parameter to show_tool_names then the
>> "available" case is also simplified:
>>
>>     show_tool_names 'mode_ok && is_available' "$TAB$TAB" \
>>         "$tool_opt may be set to one of the following:" \
>>         "No suitable tool for 'git $cmd_name --tool=<tool>' found."
>>
>> with this at the end of show_tool_names:
>>
>>     test -n "$preamble" && test -n "$not_found_msg" && \
>>         echo "$not_found_msg"
>
> Yes, something along that line.

I don't want to stomp on your feet and poke at this file too much if
you're planning on building on top of it (I already did a few times
;-).  My git time is a bit limited for the next few days so I don't
want to hold you up.  I can help shepherd through small fixups that
come up until the weekend rolls around and I have more time, but I
also don't want to hold you back until then.

I will have some time tonight.  If you guys would prefer an
incremental patch I can send one that changes the "ls" expression and
the way the unavailable block is structured.  Otherwise, I can send
replacements to handle the "test -z" thing, $TAB$TAB, and the
simplification of the unavailable block.

Later patches (that are working towards the new feature) can
generalize show_tool_names() further and eliminate the need for the
available/unavailable variables altogether.  John, I would imagine
that you'd want to pick that up since you're driving towards having
--tool-help honor custom tools.

What do you think?
-- 
David

^ permalink raw reply

* Re: [PATCH v2 1/4] mergetool--lib: Simplify command expressions
From: Junio C Hamano @ 2013-01-29 22:27 UTC (permalink / raw)
  To: David Aguilar; +Cc: John Keeping, git
In-Reply-To: <CAJDDKr4CFyQrAec3jCxyDCx0+4BMXmQAciG1YcnMYS=PAeW-Mw@mail.gmail.com>

David Aguilar <davvid@gmail.com> writes:

> On Tue, Jan 29, 2013 at 11:22 AM, John Keeping <john@keeping.me.uk> wrote:
>> On Sun, Jan 27, 2013 at 04:52:23PM -0800, David Aguilar wrote:
>>> Update variable assignments to always use $(command "$arg")
>>> in their RHS instead of "$(command "$arg")" as the latter
>>> is harder to read.  Make get_merge_tool_cmd() simpler by
>>> avoiding "echo" and $(command) substitutions completely.
>>>
>>> Signed-off-by: David Aguilar <davvid@gmail.com>
>>> ---
>>> @@ -300,9 +292,9 @@ get_merge_tool_path () {
>>>       fi
>>>       if test -z "$merge_tool_path"
>>>       then
>>> -             merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
>>> +             merge_tool_path=$(translate_merge_tool_path "$merge_tool")
>>>       fi
>>> -     if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
>>> +     if test -z $(get_merge_tool_cmd "$merge_tool") &&
>>
>> This change should be reverted to avoid calling "test -z" without any
>> other arguments, as Johannes pointed out in v1.
>>
>> The rest of this patch looks good to me.
>
> You're right.  My eyes have probably been staring at it too long and I
> missed this (even though I thought I had checked).

By now you (and people who were following this thread) are beginning
to see why I said "I'd feel safer with extra dq" ;-)

I'll amend locally and push the result out.

^ permalink raw reply

* Re: [PATCHv2] git-send-email: add ~/.authinfo parsing
From: Junio C Hamano @ 2013-01-29 22:38 UTC (permalink / raw)
  To: Michal Nazarewicz; +Cc: git, Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <d58b0709cd86e0d336902b52d72e06dd9b52d70d.1359493459.git.mina86@mina86.com>

Michal Nazarewicz <mpn@google.com> writes:

>> It is rather strange to require a comma-separated-values parser to
>> read a file format this simple, isn't it?
>
> I was worried about spaces in password.  CVS should handle such case
> nicely, whereas simple split won't.  Nonetheless, I guess that in the
> end this is not likely enough to add the dependency.

But .netrc/.authinfo format separates its entries with SP, HT, or
LF.  An entry begins with "machine <remote-hostname>" token pair.

split(/\s+/) will not work for an entry that span multiple lines but
CSV will not help, either.

Is it bad to use Net::Netrc instead?  This looks like exactly the
use case that module was written for, no?

>> Perhaps you can convert at least some popular ones yourself?  After
>> all, the user may be using an _existing_ .authinfo/.netrc that she
>> has been using with other programs that do understand symbolic port
>> names.  Rather than forcing all such users to update their files,
>> the patch can work a bit harder for them and the world will be a
>> better place, no?
>
> Parsing /etc/services added.

Hmph.  I would have expected to see getservbyname.

^ permalink raw reply

* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: Junio C Hamano @ 2013-01-29 22:55 UTC (permalink / raw)
  To: David Aguilar; +Cc: John Keeping, git
In-Reply-To: <CAJDDKr4e=pg=YJ4CfUk7guUCcikBtXTveVX9j6CV5NtGvPB=9Q@mail.gmail.com>

David Aguilar <davvid@gmail.com> writes:

> I don't want to stomp on your feet and poke at this file too much if
> you're planning on building on top of it (I already did a few times
> ;-).  My git time is a bit limited for the next few days so I don't
> want to hold you up.  I can help shepherd through small fixups that
> come up until the weekend rolls around and I have more time, but I
> also don't want to hold you back until then.

I can work with John to get this part into a shape to support his
extended use sometime toward the end of this week, by which time
hopefully you have some time to comment on the result.  John, how
does that sound?

^ permalink raw reply

* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: John Keeping @ 2013-01-29 23:02 UTC (permalink / raw)
  To: David Aguilar; +Cc: git, Junio C Hamano
In-Reply-To: <CAJDDKr4e=pg=YJ4CfUk7guUCcikBtXTveVX9j6CV5NtGvPB=9Q@mail.gmail.com>

On Tue, Jan 29, 2013 at 02:27:21PM -0800, David Aguilar wrote:
> I don't want to stomp on your feet and poke at this file too much if
> you're planning on building on top of it (I already did a few times
> ;-).  My git time is a bit limited for the next few days so I don't
> want to hold you up.  I can help shepherd through small fixups that
> come up until the weekend rolls around and I have more time, but I
> also don't want to hold you back until then.
> 
> I will have some time tonight.  If you guys would prefer an
> incremental patch I can send one that changes the "ls" expression and
> the way the unavailable block is structured.  Otherwise, I can send
> replacements to handle the "test -z" thing, $TAB$TAB, and the
> simplification of the unavailable block.
> 
> Later patches (that are working towards the new feature) can
> generalize show_tool_names() further and eliminate the need for the
> available/unavailable variables altogether.  John, I would imagine
> that you'd want to pick that up since you're driving towards having
> --tool-help honor custom tools.
> 
> What do you think?

I was planning to hold off until this series is in a reasonable state -
there's no rush as far as I'm concerned, but if Junio's happy to leave
these patches with just the small fixups I'm happy to build on that with
a patch that removes the available and unavailable variables before
adding the tools from git-config.


John

^ permalink raw reply

* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: John Keeping @ 2013-01-29 23:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Aguilar, git
In-Reply-To: <7va9rroazl.fsf@alter.siamese.dyndns.org>

On Tue, Jan 29, 2013 at 02:55:26PM -0800, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> 
> > I don't want to stomp on your feet and poke at this file too much if
> > you're planning on building on top of it (I already did a few times
> > ;-).  My git time is a bit limited for the next few days so I don't
> > want to hold you up.  I can help shepherd through small fixups that
> > come up until the weekend rolls around and I have more time, but I
> > also don't want to hold you back until then.
> 
> I can work with John to get this part into a shape to support his
> extended use sometime toward the end of this week, by which time
> hopefully you have some time to comment on the result.  John, how
> does that sound?

My email crossed with yours - that sounds good to me.  If
da/mergetool-docs is in a reasonable state by tomorrow evening (GMT) I
should be able to have a look at it then - if not I'm happy to hold off
longer.


John

^ permalink raw reply

* [PATCHv3] git-send-email: add ~/.authinfo parsing
From: Michal Nazarewicz @ 2013-01-30  0:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <7vehh3obs0.fsf@alter.siamese.dyndns.org>

From: Michal Nazarewicz <mina86@mina86.com>

Make git-send-email read password from a ~/.authinfo or ~/.netrc file
instead of requiring it to be stored in git configuration, passed as
command line argument or typed in.

There are various other applications that use this file for
authentication information so letting users use it for git-send-email
is convinient.  Furthermore, some users store their ~/.gitconfig file
in a public repository and having to store password there makes it
easy to publish the password.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 Documentation/git-send-email.txt | 47 +++++++++++++++++---
 git-send-email.perl              | 93 ++++++++++++++++++++++++++++++++++------
 2 files changed, 122 insertions(+), 18 deletions(-)

On Tue, Jan 29 2013, Junio C Hamano wrote:
> But .netrc/.authinfo format separates its entries with SP, HT, or
> LF.  An entry begins with "machine <remote-hostname>" token pair.
>
> split(/\s+/) will not work for an entry that span multiple lines but
> CSV will not help, either.
>
> Is it bad to use Net::Netrc instead?  This looks like exactly the
> use case that module was written for, no?

I don't think that's the case.  For one, Net::Netrc does not seem to
process port number.

There is a Text::Authinfo module but it just uses Text::CSV.

I can change the code to use Net::Netrc, but I dunno if that's really
the best option, since I feel people would expect parsing to be
somehow compatible with
<http://www.gnu.org/software/emacs/manual/html_node/gnus/NNTP.html>
rather than the original .netrc file format.

> Hmph.  I would have expected to see getservbyname.

Ha!  Even better. :]

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..ac020d1 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -158,14 +158,49 @@ Sending
 --smtp-pass[=<password>]::
 	Password for SMTP-AUTH. The argument is optional: If no
 	argument is specified, then the empty string is used as
-	the password. Default is the value of 'sendemail.smtppass',
-	however '--smtp-pass' always overrides this value.
+	the password. Default is the value of 'sendemail.smtppass'
+	or value read from ~/.authinfo file, however '--smtp-pass'
+	always overrides this value.
 +
-Furthermore, passwords need not be specified in configuration files
-or on the command line. If a username has been specified (with
+Furthermore, passwords need not be specified in configuration files or
+on the command line. If a username has been specified (with
 '--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass', 'sendemail.smtppass' or via
+~/.authinfo file), then the user is prompted for a password while
+the input is masked for privacy.
++
+The ~/.authinfo file should contain a line with the following
+format:
++
+  machine <domain> port <port> login <user> password <pass>
++
+Instead of `machine <domain>` pair a `default` token can be used
+instead in which case all domains will match.  Similarly, `port
+<port>` and `login <user>` pairs can be omitted in which case matching
+of the given value will be skipped.  `<port>` can be either an integer
+or a symbolic name.  Lines are interpreted in order and password from
+the first line that matches will be used.  For instance, one may end
+up with:
++
+  machine example.com login jane port ssmtp password smtppassword
+  machine example.com login jane            password janepassword
+  default             login janedoe         password doepassword
++
+if she wants to use `smtppassword` for authenticating as `jane` to
+a service at example.com:465 (SSMTP), `janepassword` for all other
+services at example.com; and `doepassword` when authonticating as
+`janedoe` to any service.  If ~/.authinfo file is missing,
+'git-send-email' will also try ~/.netrc file (even though parsing is
+not fully compatible with ftp's .netrc file format).
++
+Note that you should never make ~/.authinfo file world-readable.  To
+help guarantee that, you might want to create the file with the
+following command:
++
+  ( umask 077; cat >~/.authinfo <<EOF
+  ... file contents ...
+  EOF
+  )
 
 --smtp-server=<host>::
 	If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..a62dfa4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,86 @@ sub maildomain {
 	return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
 }
 
+
+sub read_password_from_stdin {
+	my $line;
+
+	system "stty -echo";
+
+	do {
+		print "Password: ";
+		$line = <STDIN>;
+		print "\n";
+	} while (!defined $line);
+
+	system "stty echo";
+
+	chomp $line;
+	return $line;
+}
+
+sub authinfo_is_port_eq {
+	my ($from_file, $value, $filename) = @_;
+
+	if (!defined $from_file) {
+		return 1;
+	} elsif ($from_file =~ /^\d+$/) {
+		return $from_file == $value;
+	}
+
+	my $port = getservbyname $from_file, 'tcp';
+	if (!defined $port) {
+		print STDERR "$filename: invalid port name: $from_file\n";
+		return;
+	}
+
+	return $port == $value;
+}
+
+sub read_password_from_authinfo {
+	my $filename = join '/', $ENV{'HOME'}, $_[0] // '.authinfo';
+	my $fd;
+	if (!open $fd, '<', $filename) {
+		return;
+	}
+
+	my $password;
+	while (my $line = <$fd>) {
+		$line =~ s/^\s+|\s+$//g;
+		my @line = split /\s+/, $line;
+		my %line;
+		while (@line) {
+			my $token = shift @line;
+			if ($token eq 'default') {
+				$line{'machine'} = $smtp_server;
+			} elsif (@line) {
+				$line{$token} = shift @line;
+			}
+		}
+
+		if (defined $line{'password'} &&
+		    defined $line{'machine'} &&
+		    $line{'machine'} eq $smtp_server &&
+		    (!defined $line{'login'} ||
+		     $line{'login'} eq $smtp_authuser) &&
+		    authinfo_is_port_eq($line{'port'}, $smtp_server_port, $filename)) {
+			$password = $line{'password'};
+			last;
+		}
+	}
+
+	close $fd;
+	return $password;
+}
+
+sub read_password {
+	return
+	  read_password_from_authinfo '.authinfo' ||
+	  read_password_from_authinfo '.netrc' ||
+	  read_password_from_stdin;
+}
+
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1194,18 +1274,7 @@ X-Mailer: git-send-email $gitversion
 			};
 
 			if (!defined $smtp_authpass) {
-
-				system "stty -echo";
-
-				do {
-					print "Password: ";
-					$_ = <STDIN>;
-					print "\n";
-				} while (!defined $_);
-
-				chomp($smtp_authpass = $_);
-
-				system "stty echo";
+				$smtp_authpass = read_password
 			}
 
 			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
-- 
1.8.1

^ permalink raw reply related

* Re: Cloning remote HTTP repository: Can only see 'master' branch
From: Michael Tyson @ 2013-01-30  0:06 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <20130129082317.GA6396@sigill.intra.peff.net>

Ah!  Lovely, thank you, Jeff!

Alas, it's a shared server so I'm limited to what the host provides, but that solves my problem.

Cheers!


On 29 Jan 2013, at 19:23, Jeff King <peff@peff.net> wrote:

> On Tue, Jan 29, 2013 at 04:54:13PM +1100, Michael Tyson wrote:
> 
>> I've a readonly git repository that I'm hosting via HTTP (a bare git
>> repository located within the appropriate directory on the server). I
>> push to it via my own SSH account (local repository with a remote
>> pointing to the ssh:// URL).
>> 
>> This has all worked fine so far - I push via ssh, and others can clone
>> and pull via the HTTP URL.
>> 
>> I've recently added a branch - "beta" - which pushed just fine, but
>> now cloning via the HTTP URL doesn't seem to show the new branch -
>> just master:
> 
> If you are using the "dumb" http protocol (i.e., the web server knows
> nothing about git, and just serves the repo files), you need to run "git
> update-server-info" after each push in order to update the static file
> that tells the git client about each ref. You can have git do it
> automatically for you by setting receive.updateServerInfo in the server
> repo's config.
> 
> If the server is yours to control, consider setting up the "smart" http
> protocol, as it is much more efficient. Details are in "git help
> http-backend".
> 
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv3] git-send-email: add ~/.authinfo parsing
From: Junio C Hamano @ 2013-01-30  0:34 UTC (permalink / raw)
  To: Michal Nazarewicz; +Cc: git, Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <5d18d777d6ddf6f01bbf460f37af637d3dc28ed5.1359503987.git.mina86@mina86.com>

Michal Nazarewicz <mpn@google.com> writes:

>> Is it bad to use Net::Netrc instead?  This looks like exactly the
>> use case that module was written for, no?
>
> I don't think that's the case.  For one, Net::Netrc does not seem to
> process port number.
>
> There is a Text::Authinfo module but it just uses Text::CSV.
>
> I can change the code to use Net::Netrc, but I dunno if that's really
> the best option, since I feel people would expect parsing to be
> somehow compatible with
> <http://www.gnu.org/software/emacs/manual/html_node/gnus/NNTP.html>
> rather than the original .netrc file format.

Thanks for pushing back (I wish more contributors did so when I
suggest nonsense ;-)); you are right that both canned modules are
lacking.

Will queue.  Thanks.

^ permalink raw reply

* Re: [PATCH v2] status: show the branch name if possible in in-progress info
From: Duy Nguyen @ 2013-01-30  1:13 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Matthieu Moy
In-Reply-To: <20130129184435.GA18266@google.com>

On Wed, Jan 30, 2013 at 1:44 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> -     # You are currently rebasing.
>> +     # You are currently rebasing branch '\''rebase_conflicts'\'' on '\''000106f'\''.
>
> SHA1-in-tests radar blinking.
>
> Would it be possible to compute the expected output, as in
>
>         dest=$(git rev-parse --short HEAD^^)
>         cat >expected <<-EOF &&
>         # Not currently on any branch.
>         # You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$dest'\''.
>
> ?

That may be better. Yeah.

> I'm not sure what to think about the actual change itself yet.  Can you
> give an example of when you felt the need for this, so it can be
> included in the commit message or documentation?

http://thread.gmane.org/gmane.comp.version-control.git/214932/focus=214937
-- 
Duy

^ permalink raw reply

* Re: [PATCH 3/6] introduce pack metadata cache files
From: Duy Nguyen @ 2013-01-30  1:30 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130129091555.GC9999@sigill.intra.peff.net>

On Tue, Jan 29, 2013 at 4:15 PM, Jeff King <peff@peff.net> wrote:
> +static void write_meta_header(struct metapack_writer *mw, const char *id,
> +                             uint32_t version)
> +{
> +       version = htonl(version);
> +
> +       sha1write(mw->out, "META", 4);
> +       sha1write(mw->out, "\0\0\0\1", 4);
> +       sha1write(mw->out, mw->pack->sha1, 20);
> +       sha1write(mw->out, id, 4);
> +       sha1write(mw->out, &version, 4);
> +}

Because you go with all-commit-info-in-one-file, perhaps we should
have an uint32_t bitmap to describe what info this cache contains? So
far we need 4 bits for date, tree, 1st and 2nd parents (yes, I still
want to check if storing 1-parent commits only gains us anything on
some other repos). When commit count comes, it can take the fifth bit.
Reachability bitmap offsets can take the sixth bit, if we just append
the bitmaps at the end of the same file.
-- 
Duy

^ permalink raw reply

* Re: [RFC v2] git-multimail: a replacement for post-receive-email
From: Chris Hiestand @ 2013-01-30  2:27 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Michael Haggerty, git discussion list, Andy Parkins,
	Sitaram Chamarty, Matthieu Moy, Junio C Hamano, Marc Branchaud
In-Reply-To: <CACBZZX7RA7dLcFhaHmmK97Kxfa9zLmozfdx5s9C=29DJOceq-A@mail.gmail.com>

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

On Jan 29, 2013, at 7:25 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> On Sun, Jan 27, 2013 at 9:37 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> A while ago, I submitted an RFC for adding a new email notification
>> script to "contrib" [1].  The reaction seemed favorable and it was
>> suggested that the new script should replace post-receive-email rather
>> than be added separately, ideally with some kind of migration support.
> 
> I just want to say since I think this thread hasn't been getting the
> attention it deserves: I'm all for this. I've used git-multimail and
> it's a joy to configure and extend compared to the existing hacky
> shellscript.


This seems good to me as long as it's okay for git contrib to depend on python.
I've started testing git-multimail in my environment.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4798 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
From: Junio C Hamano @ 2013-01-30  3:08 UTC (permalink / raw)
  To: John Keeping; +Cc: David Aguilar, git
In-Reply-To: <20130129230607.GG1342@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

> On Tue, Jan 29, 2013 at 02:55:26PM -0800, Junio C Hamano wrote:
> ...
>> I can work with John to get this part into a shape to support his
>> extended use sometime toward the end of this week, by which time
>> hopefully you have some time to comment on the result.  John, how
>> does that sound?
>
> My email crossed with yours - that sounds good to me.  If
> da/mergetool-docs is in a reasonable state by tomorrow evening (GMT) I
> should be able to have a look at it then - if not I'm happy to hold off
> longer.

Heh, I actually was hoping that you will send in a replacement for
David's patch ;-)

Here is what I will squash into the one we have been discussing.  In
a few hours, I expect I'll be able to push this out in the 'pu'
branch.

-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 29 Jan 2013 18:57:55 -0800
Subject: [PATCH] [SQUASH] mergetools: tweak show_tool_names and its users

Use show_tool_names as a function to produce output, not as a
function to compute a string.  Indicate if any output was given
with its return status, so that the caller can say "if it didn't
give any output, I'll say this instead" easily.

To be squashed into the previous; no need to keep this log message.
---
 git-mergetool--lib.sh | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 135da96..79cbdc7 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -22,7 +22,7 @@ is_available () {
 show_tool_names () {
 	condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
 
-	( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
+	( cd "$MERGE_TOOLS_DIR" && ls ) |
 	while read toolname
 	do
 		if setup_tool "$toolname" 2>/dev/null &&
@@ -36,6 +36,7 @@ show_tool_names () {
 			printf "%s%s\n" "$per_line_prefix" "$tool"
 		fi
 	done
+	test -n "$preamble"
 }
 
 diff_mode() {
@@ -236,27 +237,30 @@ list_merge_tool_candidates () {
 
 show_tool_help () {
 	tool_opt="'git ${TOOL_MODE}tool --tool-<tool>'"
-	available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
-		"$tool_opt may be set to one of the following:")
-	unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
-		"The following tools are valid, but not currently available:")
-	if test -n "$available"
+
+	tab='	' av_shown= unav_shown=
+
+	if show_tool_names 'mode_ok && is_available' "$tab$tab" \
+		"$tool_opt may be set to one of the following:"
 	then
-		echo "$available"
+		av_shown=yes
 	else
 		echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
+		av_shown=no
 	fi
-	if test -n "$unavailable"
+
+	if show_tool_names 'mode_ok && ! is_available' "$tab$tab" \
+		"The following tools are valid, but not currently available:"
 	then
-		echo
-		echo "$unavailable"
+		unav_shown=yes
 	fi
-	if test -n "$unavailable$available"
-	then
+
+	case ",$av_shown,$unav_shown," in
+	*,yes,*)
 		echo
 		echo "Some of the tools listed above only work in a windowed"
 		echo "environment. If run in a terminal-only session, they will fail."
-	fi
+	esac
 	exit 0
 }
 
-- 
1.8.1.2.555.gedafe79

^ permalink raw reply related


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