Git development
 help / color / mirror / Atom feed
* Re: RFC: reverse bisect
From: Johannes Sixt @ 2011-09-29 16:27 UTC (permalink / raw)
  To: Michal Vyskocil; +Cc: git
In-Reply-To: <20110929142027.GA4936@zelva.suse.cz>

Am 29.09.2011 16:20, schrieb Michal Vyskocil:
> git bisect start --reverse HEAD~999 HEAD

With the regular meaning of the start subcommand, the revs given are
ordered: bad good good...

With the reversed meaning, this would have to become: good bad bad...

This would have to be mentioned clearly in the documentation.

> git bisect good/bad/skip/run

Last time this came up on the list I suggested to add the following
commands:

   git bisect regression  # a synonym for git bisect start
   git bisect improvement # your --reverse

-- Hannes

^ permalink raw reply

* Re: [PATCH v4] send-email: auth plain/login fix
From: Joe Perches @ 2011-09-29 15:01 UTC (permalink / raw)
  To: Zbigniew Jędrzejewski-Szmek; +Cc: gitster, git, peff
In-Reply-To: <20110929141616.GU10763@in.waw.pl>

On Thu, 2011-09-29 at 16:16 +0200, Zbigniew Jędrzejewski-Szmek wrote:
> git send-email was not authenticating properly when communicating over
> TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is
> e.g. the standard server setup under debian with exim4 and probably
> everywhere where system accounts are used.
> 
> The problem (only?) exists when libauthen-sasl-cyrus-perl
> (Authen::SASL::Cyrus) is installed. Importing Authen::SASL::Perl
> makes Authen::SASL use the perl implementation which works
> better.
[]
> diff --git a/git-send-email.perl b/git-send-email.perl
[]
> @@ -1098,6 +1098,10 @@ X-Mailer: git-send-email $gitversion
>  		}
>  
>  		if (defined $smtp_authuser) {
> +			eval {
> +				require Authen::SASL;
> +				Authen::SASL->import(qw(Perl));
> +			};

Thanks for keeping at this.

One comment:

This is a workaround for a nominal defect.

As such, I think the code should be commented
to note why it exists.

How about adding a comment like:

 		if (defined $smtp_authuser) {
			# Workaround AUTH PLAIN/LOGIN interaction defect
			# with Authen::SASL::Cyrus
			eval {
				require Authen::SASL;

^ permalink raw reply

* Re: Lack of detached signatures
From: Sverre Rabbelier @ 2011-09-29 14:52 UTC (permalink / raw)
  To: Ted Ts'o
  Cc: Junio C Hamano, Jeff King, Joseph Parmelee,
	Carlos Martín Nieto, Olsen, Alan R, Michael Witten,
	git@vger.kernel.org
In-Reply-To: <20110929145046.GC13705@thunk.org>

Heya,

On Thu, Sep 29, 2011 at 16:50, Ted Ts'o <tytso@mit.edu> wrote:
> On Thu, Sep 29, 2011 at 04:40:54PM +0200, Sverre Rabbelier wrote:
>>
>> This all sounds very interesting. Where is this discussion on a new
>> web of trust taking place? The kernel mailing list? Do you have a
>> message-id / gmane.org link for me to read more about this perhaps?
>
> It's been taking place on private e-mails and on conference calls
> amongst those of us who have been organizing the kernel.org recovery
> efforts.  There will be a more detailed discussion and a GPG key
> signing party that I will be organizing at the upcoming kernel summit
> meeting in Prague.
>
> What are your concerns?

Not concerned at all. I just would have enjoyed listening in to those
more knowledgeable than me discussing security and trust :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Lack of detached signatures
From: Ted Ts'o @ 2011-09-29 14:50 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Jeff King, Joseph Parmelee,
	Carlos Martín Nieto, Olsen, Alan R, Michael Witten,
	git@vger.kernel.org
In-Reply-To: <CAGdFq_h6shMp+d4f1bG=if6L11M_5ixN5JF7KgCrZJ44QBt0QQ@mail.gmail.com>

On Thu, Sep 29, 2011 at 04:40:54PM +0200, Sverre Rabbelier wrote:
> 
> This all sounds very interesting. Where is this discussion on a new
> web of trust taking place? The kernel mailing list? Do you have a
> message-id / gmane.org link for me to read more about this perhaps?

It's been taking place on private e-mails and on conference calls
amongst those of us who have been organizing the kernel.org recovery
efforts.  There will be a more detailed discussion and a GPG key
signing party that I will be organizing at the upcoming kernel summit
meeting in Prague.

What are your concerns?

					- Ted

^ permalink raw reply

* Re: RFC: reverse bisect
From: Sverre Rabbelier @ 2011-09-29 14:42 UTC (permalink / raw)
  To: Michal Vyskocil; +Cc: git
In-Reply-To: <20110929142027.GA4936@zelva.suse.cz>

Heya,

On Thu, Sep 29, 2011 at 16:20, Michal Vyskocil <mvyskocil@suse.cz> wrote:
>        fprintf(stderr, "Some good revs are not ancestor of the bad rev.\n"
>                "git bisect cannot work properly in this case.\n"
> -               "Maybe you mistake good and bad revs?\n");
> +               "Try --reverse to switch the bisect logic.\n");

Heh, glad to see the "Maybe you mistake" phrasing removed if nothing else :P.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Lack of detached signatures
From: Sverre Rabbelier @ 2011-09-29 14:40 UTC (permalink / raw)
  To: Ted Ts'o
  Cc: Junio C Hamano, Jeff King, Joseph Parmelee,
	Carlos Martín Nieto, Olsen, Alan R, Michael Witten,
	git@vger.kernel.org
In-Reply-To: <20110929131845.GQ19250@thunk.org>

Heya,

On Thu, Sep 29, 2011 at 15:18, Ted Ts'o <tytso@mit.edu> wrote:
> Handset manufacturers such as Samsung, HTC, Motorola, etc., there will
> be many of those reprsenatives at LinuxCon Europe and CELF (Consumer
> Electronics Linux Forum) Europe conferences, which will be colocated
> with the Kernel Summit in Prague.
>
> If you are thinking of random developers located in far-flung places
> of the world who don't have any contact with other Linux developers,
> this is a previously unsolved problem.  There are links into the
> developing Kernel GPG tree that are signed by the GPG web trust used
> by Debian, OpenSuSE, and (soon) Fedora.  Given that people generally
> have to trust one or more of those web of trusts, that's the best we
> can do, at least as far as I know.  If you can suggest something
> better, please let me know!

This all sounds very interesting. Where is this discussion on a new
web of trust taking place? The kernel mailing list? Do you have a
message-id / gmane.org link for me to read more about this perhaps?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [BUG?] gitk assumes initial commit is empty
From: Zbigniew Jędrzejewski-Szmek @ 2011-09-29 14:33 UTC (permalink / raw)
  To: git

I was looking at gitk display for git itself, and scrolling all the way
down to initial commit show this:

Author: Linus Torvalds <torvalds@ppc970.osdl.org>  2005-04-08 00:13:13
Committer: Linus Torvalds <torvalds@ppc970.osdl.org>  2005-04-08 00:13:13
Child:  8bc9a0c769ac1df7820f2dbf8f7b7d64835e3c68 (Add copyright notices.)

    Initial revision of "git", the information manager from hell

with no files added or modified. But e.g. 'git show' shows that files
were added in the initial commit:

$ git show e83c5163316f89bfbde7d9ab23ca2e25604af290 --stat
Author: Linus Torvalds <torvalds@ppc970.osdl.org>
Date:   Thu Apr 7 15:13:13 2005 -0700
...
   11 files changed, 1244 insertions(+), 0 deletions(-)

In gitk, the next commit shows changes to some files, like if they existed
in the parent commit. So it seems that gitk assumes that initial commit
is empty, which doesn't have to be true.

This is with gitk from master.

-
Zbyszek

^ permalink raw reply

* RFC: reverse bisect
From: Michal Vyskocil @ 2011-09-29 14:20 UTC (permalink / raw)
  To: git

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

Hi all,

Following proposed patch try to implement reverse mode for git bisect.

The git bisect command is written in the regression-finding in mind. IOW
it expects the good commit is older than the later one, which caused a
regression.

However common usage for (at least) package maintainer is not to find a
regression and fix it. The main task it to identify a bugfix!. In this
case git bisect is still helpfull as it reduces a time a lot, but user
needs to exchange the good<->bad in his mind, which is confusing and in
case there are delays in the work, it's trivial to forgot that I have to
type git bisect good, when I'm in the bad revision.

This simple patch try to address the problem of poor package
maintainer's brain and introduces --reverse argument for the git bisect
start command.

In this mode, bisect internally exchange the behavior of good/bad
itself, so there's no need to do it manually. I did some basic testing
and

git bisect start --reverse HEAD~999 HEAD
git bisect good/bad/skip/run

really works well, allowing user to identify a first good commit instead
of the first bad one. I did not test other commands like visualize or
replay.

What do you think about it? Do you see other problems I'm not aware of?
---
 bisect.c      |    2 +-
 git-bisect.sh |   49 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/bisect.c b/bisect.c
index c7b7d79..33aaeaa 100644
--- a/bisect.c
+++ b/bisect.c
@@ -768,7 +768,7 @@ static void handle_bad_merge_base(void)
 
 	fprintf(stderr, "Some good revs are not ancestor of the bad rev.\n"
 		"git bisect cannot work properly in this case.\n"
-		"Maybe you mistake good and bad revs?\n");
+		"Try --reverse to switch the bisect logic.\n");
 	exit(1);
 }
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 2524060..5c95f25 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -33,6 +33,25 @@ OPTIONS_SPEC=
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 
+bisect_reverse_mode() {
+	test -f "$GIT_DIR/BISECT_REVERSE"
+}
+
+bisect_reverse_state() {
+
+	if bisect_reverse_mode; then
+		if test "$1" = "good"; then
+			echo "bad"
+			return 0
+		elif test "$1" = "bad"; then
+			echo "good"
+			return 0
+		fi
+	fi
+
+	echo $1
+}
+
 bisect_head()
 {
 	if test -f "$GIT_DIR/BISECT_HEAD"
@@ -69,6 +88,11 @@ bisect_start() {
 	# Check for one bad and then some good revisions.
 	#
 	has_double_dash=0
+	#
+	# Exchange the internal meainng of good/bad allowing bisect to find
+	# a commit fixing a bug, not "only" the one causes a regression
+	#
+	reverse_mode=1
 	for arg; do
 		case "$arg" in --) has_double_dash=1; break ;; esac
 	done
@@ -91,6 +115,9 @@ bisect_start() {
 		--no-checkout)
 			mode=--no-checkout
 			shift ;;
+		--reverse)
+			reverse_mode=0
+			shift ;;
 		--*)
 			die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
 		*)
@@ -99,10 +126,17 @@ bisect_start() {
 				die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
 				break
 			}
-			case $bad_seen in
-			0) state='bad' ; bad_seen=1 ;;
-			*) state='good' ;;
-			esac
+			if test $reverse_mode -ne 0; then
+				case $bad_seen in
+				0) state='bad' ; bad_seen=1 ;;
+				*) state='good' ;;
+				esac
+			else
+				case $bad_seen in
+				0) state='good' ; bad_seen=1 ;;
+				*) state='bad' ;;
+				esac
+			fi
 			eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
 			shift
 			;;
@@ -170,6 +204,9 @@ bisect_start() {
 	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
 	eval "$eval true" &&
 	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
+	if test $reverse_mode -eq 0; then
+		/bin/touch "$GIT_DIR/BISECT_REVERSE" || exit
+	fi
 	#
 	# Check if we can proceed to the next bisect state.
 	#
@@ -225,7 +262,7 @@ bisect_skip() {
 
 bisect_state() {
 	bisect_autostart
-	state=$1
+	state=$(bisect_reverse_state $1)
 	case "$#,$state" in
 	0,*)
 		die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
@@ -377,6 +414,7 @@ bisect_clean_state() {
 	rm -f "$GIT_DIR/BISECT_LOG" &&
 	rm -f "$GIT_DIR/BISECT_NAMES" &&
 	rm -f "$GIT_DIR/BISECT_RUN" &&
+	rm -f "$GIT_DIR/BISECT_REVERSE" &&
 	# Cleanup head-name if it got left by an old version of git-bisect
 	rm -f "$GIT_DIR/head-name" &&
 	git update-ref -d --no-deref BISECT_HEAD &&
@@ -402,6 +440,7 @@ bisect_replay () {
 			cmd="bisect_start $rev"
 			eval "$cmd" ;;
 		good|bad|skip)
+			command=$(bisect_reverse_state $1)
 			bisect_write "$command" "$rev" ;;
 		*)
 			die "$(gettext "?? what are you talking about?")" ;;
-- 
1.7.6.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply related

* [PATCH v4] send-email: auth plain/login fix
From: Zbigniew Jędrzejewski-Szmek @ 2011-09-29 14:16 UTC (permalink / raw)
  To: gitster, joe, git, peff
In-Reply-To: <7v62kcz5rr.fsf@alter.siamese.dyndns.org>

git send-email was not authenticating properly when communicating over
TLS with a server supporting only AUTH PLAIN and AUTH LOGIN. This is
e.g. the standard server setup under debian with exim4 and probably
everywhere where system accounts are used.

The problem (only?) exists when libauthen-sasl-cyrus-perl
(Authen::SASL::Cyrus) is installed. Importing Authen::SASL::Perl
makes Authen::SASL use the perl implementation which works
better.

The solution is based on this forum thread:
http://www.perlmonks.org/?node_id=904354.

This patch is tested by sending it. Without this fix, the interaction with
the server failed like this:

$ git send-email --smtp-encryption=tls --smtp-server=... --smtp-debug=1 change1.patch
...
Net::SMTP::SSL=GLOB(0x238f668)<<< 250-AUTH LOGIN PLAIN
Password:
Net::SMTP::SSL=GLOB(0x238f668)>>> AUTH
Net::SMTP::SSL=GLOB(0x238f668)<<< 501 5.5.2 AUTH mechanism must be specified
5.5.2 AUTH mechanism must be specified

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

---
v2:  - the import is performed only if it will be used
v3:  - the import is performed only if it will be used, and failure is ignored
v4:  - improved commit message

 git-send-email.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 37dfbe7..dbc435a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1098,6 +1098,10 @@ X-Mailer: git-send-email $gitversion
 		}
 
 		if (defined $smtp_authuser) {
+			eval {
+				require Authen::SASL;
+				Authen::SASL->import(qw(Perl));
+			};
 
 			if (!defined $smtp_authpass) {
 
-- 
1.7.6

^ permalink raw reply related

* Re: Lack of detached signatures
From: Ted Ts'o @ 2011-09-29 13:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Joseph Parmelee, Carlos Martín Nieto,
	Olsen, Alan R, Michael Witten, git@vger.kernel.org
In-Reply-To: <7vbou4uhuu.fsf@alter.siamese.dyndns.org>

On Wed, Sep 28, 2011 at 08:50:49PM -0700, Junio C Hamano wrote:
> 
> I was actually more worried about helping consumers convince themselves
> that thusly signed keys indeed belong to producers like Linus, Peter,
> etc. There are those who worry that DNS record to code.google.com/ for
> them may point at an evil place to give them rogue download material.
> "Here are the keys you can verify our trees with" message on the mailing
> list, even with the message is signed with GPG, would not be satisfactory
> to them.

What do you mean by "consumers" in this context?  Most end users don't
actually download tarballs from www.kernel.org or code.google.com!  :-)

If you mean developers at Linux distributions Red Hat, SuSE, or
Handset manufacturers such as Samsung, HTC, Motorola, etc., there will
be many of those reprsenatives at LinuxCon Europe and CELF (Consumer
Electronics Linux Forum) Europe conferences, which will be colocated
with the Kernel Summit in Prague.

If you are thinking of random developers located in far-flung places
of the world who don't have any contact with other Linux developers,
this is a previously unsolved problem.  There are links into the
developing Kernel GPG tree that are signed by the GPG web trust used
by Debian, OpenSuSE, and (soon) Fedora.  Given that people generally
have to trust one or more of those web of trusts, that's the best we
can do, at least as far as I know.  If you can suggest something
better, please let me know!


						- Ted

^ permalink raw reply

* Re: Merge seems to overwrite unstaged local changes
From: Sebastian Schuberth @ 2011-09-29 13:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa9o1yf7.fsf@alter.siamese.dyndns.org>

On Wed, Sep 28, 2011 at 17:25, Junio C Hamano <gitster@pobox.com> wrote:

>> ... I'm seeing this on Linux and Windows, with versions 1.7.4.3 and 1.7.6.
>
> There recently have been quite a change in merge-recursive implementation
> and it would be really nice if you can try this again with the tip of
> 'master' before 1.7.7 final ships.

The unstaged changes do not seem to get lost during the merge anymore
when using git version 1.7.7.rc3.4.g8d714 on Linux. I guess that
somewhat confirms that there's a bug in git < 1.7.7. I'll write a word
of warning to our in-house git users that they should always commit
before merging ...

-- 
Sebastian Schuberth

^ permalink raw reply

* What's cooking in git.git (Sep 2011, #08; Wed, 28)
From: Theo Niessink @ 2011-09-29 11:56 UTC (permalink / raw)
  To: 'Junio C Hamano'; +Cc: git

Junio C Hamano wrote:
> * po/cygwin-backslash (2011-08-05) 2 commits
>  - On Cygwin support both UNIX and DOS style path-names
>  - git-compat-util: add generic find_last_dir_sep that respects is_dir_sep
> 
> Incomplete with respect to backslash processing in prefix_filename(), and
> also loses the ability to escape glob specials. Perhaps drop?

Yeah, I guess.

BTW, the git-compat-util commit might still be useful, should anyone ever want to reuse MinGW's find_last_dir_sep.

^ permalink raw reply

* Re: What's cooking in git.git (Sep 2011, #04; Mon, 12)
From: Pascal Obry @ 2011-09-29 12:03 UTC (permalink / raw)
  To: kusmabite; +Cc: Junio C Hamano, Ramsay Jones, Johannes Sixt, git
In-Reply-To: <CABPQNSae3MU34pRw87CNkEUBbTpE5h9UVT3cqv3iFnWs1wQ5FQ@mail.gmail.com>

Le 13/09/2011 11:46, Erik Faye-Lund a écrit :
> On Tue, Sep 13, 2011 at 12:56 AM, Junio C Hamano<gitster@pobox.com>  wrote:
>> Junio C Hamano<gitster@pobox.com>  writes:
>>
>>> [Stalled]
>>>
>>> * po/cygwin-backslash (2011-08-05) 2 commits
>>>   - On Cygwin support both UNIX and DOS style path-names
>>>   - git-compat-util: add generic find_last_dir_sep that respects is_dir_sep
>>
>> Honestly I lost track of this one. How would we want to proceed on this
>> topic after 1.7.7?
>>
>> Asking help from Windows folks.
>
> I believe Hannes pointed out that there were some work left to be done
> on it ("enable backslash processing in setup.c:prefix_filename()"),
> and I didn't spot a new version after that. He also pointed out that
> enabling backslash processing would cause you to lose the ability to
> escape special characters, but it sounds to me like this is something
> that simply "comes with the territory" of supporting win32-paths in a
> POSIX-ish environment, and is already the governing convention in
> Cygwin. But I'm not an expert on this topic; Cygwin is not something I
> usually care much about.

Same here, not expert. I just can say that this at least fixes a real 
problem and the patches (provided by Theo and I) are going in the right 
direction. They may be some other issues about Windows backslash (my 
experiences show that there is very entertaining issues with this!) but 
I don't think we should hold those patches except if someone prove them 
to be wrong.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B

^ permalink raw reply

* Re: What's next for "signed push"?
From: Dmitry Ivankov @ 2011-09-29 11:50 UTC (permalink / raw)
  To: git
In-Reply-To: <7vfwjgui8s.fsf_-_@alter.siamese.dyndns.org>

Junio C Hamano <gitster <at> pobox.com> writes:

>  - It also was hoped that pre-receive or pre-update hook on the receiving
>    end can be used to authenticate and authorize the push itself with the
>    approach by v3, but when the check happens, the signed-notes tree to be
>    used for verification is not connected to any ref in the refs/notes/
>    hierarchy yet (otherwise it won't be pre-* hook). The query interface
>    "git notes show" needs to be updated so that it takes not just a ref
>    via the GIT_NOTES_REF interface, which is defined to specify a ref
>    because some subcommands of "git notes" need to create a new commit and
>    update it, but a bare notes tree commit object name [*1*]. We may need
>    to update "git notes" (at least "show" subcommand) for the use of
>    receiving end; v3 is no longer a simpler "sender only" solution.
> 
> *1* I wouldn't be surprised if it already worked when you give the object
> name of the notes-tree commit to GIT_NOTES_REF when running "git show",
> but that is not really a documented interface and working by accident. The
> environment variable was designed to take a name of the ref.
There's also my old request for comments on refs/notes/ ([RFC] plumbing git-
notes, link below). Unexpected thing is that "refs/notes/commits^" is silently 
accepted, but notes aren't displayed at all.


http://thread.gmane.org/gmane.comp.version-control.git/178149

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: John Szakmeister @ 2011-09-29 10:03 UTC (permalink / raw)
  To: Jeff King; +Cc: Jay Soffian, git, Junio C Hamano
In-Reply-To: <20110929075627.GB14022@sigill.intra.peff.net>

On Thu, Sep 29, 2011 at 3:56 AM, Jeff King <peff@peff.net> wrote:
[snip]
> This was my first one. I kind of expected there to be some kind of
> graphical password dialog. Especially because keychain will pop up a
> dialog and ask you "is it OK for git to access this password?". So I
> sort of assumed that people would assume that credentials happened
> outside of the regular terminal session (I see the same thing on Linux,
> for example, with gpg-agent, which will open a new window and grab
> focus).
>
> But I have no idea what's "normal" on OS X.

I've been working on a version of the keychain credential cache as
well.  I did create a gui, although it's a bit painful.

> I wondered if you were trying to be friendly to people who were
> connecting via ssh. But that doesn't seem to work at all. I couldn't get
> either version of your helper to actually do anything in an ssh session
> (even with the same user logged in on console).  I guess there is some
> magic to hook it into the keychain manager.

I'm not sure there is.  For instance, ssh will look up passphrases for
ssh keys in the keychain, but if you ssh into the box, and then try to
ssh to somewhere else, it'll prompt you for the passphrase for the
key.  In my own experiments, the lookup will come back with a failure,
despite the fact that the keychain is already unlocked. :-(

Looking at the Subversion source, they recognize the issue too:

> * XXX (2005-12-07): If no GUI is available (e.g. over a SSH session),
> * you won't be prompted for credentials with which to unlock your
> * keychain.  Apple recognizes lack of TTY prompting as a known
> * problem.

...obviously, Apple hasn't fixed this issue. :-(

> Also, regarding opening /dev/tty yourself versus using getpass. There
> are a few magic things getpass will do that your helper won't:
>
>  1. It respects core.askpass, GIT_ASKPASS, and SSH_ASKPASS if they are
>     set.
>
>  2. The "get the username from the config" feature is triggered at the
>     time of prompting the user (so instead of asking for the username,
>     we check the config and pretend the user told us).
>
>     I did it this way originally so that helpers would have the first
>     crack at setting a username, and we would fall back to the config.
>     Thinking on it more, that may be backwards. If the user has told
>     git "for github.com, I am user 'foo'", then that should probably
>     take effect first, and --username=foo get passed to the helper.

I think that makes sense.  I think one thing we have to be careful
about partial matches.  I wouldn't want the credential cache to send
off the wrong password to a service.  This may be me being cautious,
but if I don't have all the necessary bits, I'd rather we fail that to
guess which entry is right.

[snip]
> Hrm. I was really hoping people wouldn't need to pick apart the "unique"
> token, and it could remain an opaque blob. If helpers are going to do
> this sort of parsing, then I'd just as soon have git break it down for
> them, and do something like:
>
>  git credential-osxkeychain \
>    --protocol=https \
>    --host=github.com \
>    --path=peff/git.git
>    --username=peff
>
> to just hand over as much information as possible, and let the helper
> throw it all together if it wants to.

I think this is a good idea too.  Keychain definitely wants this
information stored in separate fields, and I suspect the secrets api
does too (I haven't found any formal docs about the preferred way of
storing attributes related to urls though).

>> +     /* "GitHub for Mac" compatibility */
>> +     if (!strcmp(hostname, "github.com"))
>> +             hostname = "github.com/mac";
>
> Nice touch. :)

I honestly don't understand why this needs to be done.  I don't use
GitHub for Mac... does that mean this is busted for me?

[snip]
> My series will also produce "cert:/path/to/certificate" when unlocking a
> certificate. The other candidates for conversion are smtp-auth (for
> send-email) and imap (for imap-send).  I guess for certs, you'd want to
> use the "generic" keychain type.

There is a method for adding a certificate to the keychain:
   <http://developer.apple.com/library/mac/#documentation/Security/Reference/certifkeytrustservices/Reference/reference.html#//apple_ref/doc/uid/TP30000157>

I'm not sure what that does exactly, but I do have a cert, and it
shows up as "certificate" in the keychain.

> I wonder if some people would not want to cache cert passwords. Speaking
> of which, I remember keychain asking me "do you want to let git see this
> password?", but I don't ever remember it asking "do you want to save
> this password?". Is that usually automatic? Again, I was kind of
> expecting a dialog with a "remember this" checkbox.

By the time you get Keychain involved, the decision has been made.
Most applications offer that ability... and you're right, this should
probably offer the same capability.  That also means stashing that
data somewhere. :-(  OTOH, it does make for a better user experience.

[snip]
> I noticed that when using the python helper, the dialog asking something
> like: "security wants to know this password. Allow it?"
>
> Which was kind of lame. I would hope we could convince it to say "git".
> But I didn't see any option in the "security" tool for specifying the
> context[1]. The C helper says "git-credential-osxkeychain". Which isn't
> the end of the world, but it would be prettier if it just said "git".

I'm not sure how easy it is to do that.  I think it's meant to be a
security measure that it points out the executable name... but it does
make it less friendly for users. :-(

> -Peff
>
> [1] I can kind of see why they might not want you to set this for
> security reasons (because it makes impersonating other programs easy).
> On the other hand, saying "security" conveys absolutely nothing. And as
> far as I can tell, I could just call my program /tmp/iTunes, and it
> would say "iTunes wants to know this password...".

Yep, I agree.  And it's worse when using the security command line
tool... when you grant security access to the key, then any app could
technically gain access to the item via the security tool.  That's one
of the reasons I didn't pursue that route early on.

-John

^ permalink raw reply

* Re: [PATCH] show git tag output in pager
From: Michal Vyskocil @ 2011-09-29  9:37 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqehz2cbk4.fsf@bauges.imag.fr>

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

On Tue, Sep 27, 2011 at 04:19:39PM +0200, Matthieu Moy wrote:
> The commit message should explain why this is needed, and in particular
> why you prefer this to setting pager.tag in your ~/.gitconfig.

Opps! I read a documentation, but I did not realize this works for all
commands and not only for them calling setup_pager(). Then sorry, no
change is needed.

> 
> Michal Vyskocil <mvyskocil@suse.cz> writes:
> 
> > --- a/builtin/tag.c
> > +++ b/builtin/tag.c
> > @@ -147,6 +147,8 @@ static int list_tags(const char **patterns, int lines,
> >  			struct commit_list *with_commit)
> >  {
> >  	struct tag_filter filter;
> > +        
> > +        setup_pager();
> 
> Git indents with tabs, not spaces, and does not leave trailing
> whitespaces.

OK, thanks for the feedbac. I will try to be more carefull in the
future.

Regards
Michal Vyskocil

> 
> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Chris Mear @ 2011-09-29  8:19 UTC (permalink / raw)
  To: Jeff King
  Cc: Jay Soffian, git@vger.kernel.org, Junio C Hamano,
	John Szakmeister
In-Reply-To: <20110929075627.GB14022@sigill.intra.peff.net>

On 29 Sep 2011, at 08:56, Jeff King <peff@peff.net> wrote:

>> Here's a C version that no longer links to git. I also kept the original
>> Python version as an example. I decided not to call out to
>> 'git credential-gitpass' as it was simple enough to manage /dev/tty
>> and there's no portability issues since this is OS X specific.
> 
> This was my first one. I kind of expected there to be some kind of
> graphical password dialog. Especially because keychain will pop up a
> dialog and ask you "is it OK for git to access this password?". So I
> sort of assumed that people would assume that credentials happened
> outside of the regular terminal session (I see the same thing on Linux,
> for example, with gpg-agent, which will open a new window and grab
> focus).
> 
> But I have no idea what's "normal" on OS X.

It's normal for applications on OS X to present their own UI when asking for credentials in order to save them to the keychain. For example, web passwords stored by Safari are captured via the normal web page form elements, but stored in the keychain.

That said, most terminal applications aren't aware of the Mac OS X keychain. So I suppose it may surprise users to come across the password prompt in the terminal UI, even though that is the same pattern that graphical programs follow.

But it's not without precedent: I'm pretty certain Subversion collects credentials directly in the terminal for storage in the keychain.

Chris

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Jeff King @ 2011-09-29  7:56 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <1316055113-2353-1-git-send-email-jaysoffian@gmail.com>

On Wed, Sep 14, 2011 at 10:51:53PM -0400, Jay Soffian wrote:

> This credential helper adds, searches, and removes entries from
> the Mac OS X keychain. The C version links against the Security
> framework and is probably the best choice for daily use.
> 
> A python version is also included primarily as a more readable
> example and uses the /usr/bin/security CLI to access the keychain.
> 
> Tested with 10.6.8.

So I finally got a nice working OS X setup (10.7) to play around with
these. Overall, works as advertised. :) I have a few comments, though.

> Here's a C version that no longer links to git. I also kept the original
> Python version as an example. I decided not to call out to
> 'git credential-gitpass' as it was simple enough to manage /dev/tty
> and there's no portability issues since this is OS X specific.

This was my first one. I kind of expected there to be some kind of
graphical password dialog. Especially because keychain will pop up a
dialog and ask you "is it OK for git to access this password?". So I
sort of assumed that people would assume that credentials happened
outside of the regular terminal session (I see the same thing on Linux,
for example, with gpg-agent, which will open a new window and grab
focus).

But I have no idea what's "normal" on OS X.

I wondered if you were trying to be friendly to people who were
connecting via ssh. But that doesn't seem to work at all. I couldn't get
either version of your helper to actually do anything in an ssh session
(even with the same user logged in on console).  I guess there is some
magic to hook it into the keychain manager.

Also, regarding opening /dev/tty yourself versus using getpass. There
are a few magic things getpass will do that your helper won't:

  1. It respects core.askpass, GIT_ASKPASS, and SSH_ASKPASS if they are
     set.

  2. The "get the username from the config" feature is triggered at the
     time of prompting the user (so instead of asking for the username,
     we check the config and pretend the user told us).

     I did it this way originally so that helpers would have the first
     crack at setting a username, and we would fall back to the config.
     Thinking on it more, that may be backwards. If the user has told
     git "for github.com, I am user 'foo'", then that should probably
     take effect first, and --username=foo get passed to the helper.

     It doesn't make a big difference with long-term storage helpers,
     because you tell them your username once and they remember it. But
     for things like credential-cache, it lets you store the username
     for a long time, but only cache the password (which means not
     typing the username every time).

So I think maybe reason (2) should go away. But (1) is definitely worth
considering.

> +       if (!unique)
> +               die("Must specify --unique=TOKEN; try --help");

My test harness checks that this case just asks for the password without
bothering to do any lookup or storage. It probably doesn't really matter
in practice; I think git should always be providing _some_ context.

> +	hostname = strchr(unique, ':');
> +	if (!hostname)
> +		die("Invalid token `%s'", unique);
> +	*hostname++ = '\0';

Hrm. I was really hoping people wouldn't need to pick apart the "unique"
token, and it could remain an opaque blob. If helpers are going to do
this sort of parsing, then I'd just as soon have git break it down for
them, and do something like:

  git credential-osxkeychain \
    --protocol=https \
    --host=github.com \
    --path=peff/git.git
    --username=peff

to just hand over as much information as possible, and let the helper
throw it all together if it wants to.

> +	/* "GitHub for Mac" compatibility */
> +	if (!strcmp(hostname, "github.com"))
> +		hostname = "github.com/mac";

Nice touch. :)

> +	if (!strcmp(unique, "https")) {
> +		protocol = kSecProtocolTypeHTTPS;
> +	} else if (!strcmp(unique, "http")) {
> +		protocol = kSecProtocolTypeHTTP;
> +	}
> +	else
> +		die("Unrecognized protocol `%s'", unique);

My series will also produce "cert:/path/to/certificate" when unlocking a
certificate. The other candidates for conversion are smtp-auth (for
send-email) and imap (for imap-send).  I guess for certs, you'd want to
use the "generic" keychain type.

I wonder if some people would not want to cache cert passwords. Speaking
of which, I remember keychain asking me "do you want to let git see this
password?", but I don't ever remember it asking "do you want to save
this password?". Is that usually automatic? Again, I was kind of
expecting a dialog with a "remember this" checkbox.

> +def add_internet_password(protocol, hostname, username, password):
> +    # We do this over a pipe so that we can provide the password more
> +    # securely than as an argument which would show up in ps output.
> +    # Unfortunately this is possibly less robust since the security man
> +    # page does not document how to quote arguments. Emprically it seems
> +    # that using the double-quote, escaping \ and " works properly.
> +    username = username.replace('\\', '\\\\').replace('"', '\\"')
> +    password = password.replace('\\', '\\\\').replace('"', '\\"')
> +    command = ' '.join([
> +        'add-internet-password', '-U',
> +        '-r', protocol,
> +        '-s', hostname,
> +        '-a "%s"' % username,
> +        '-w "%s"' % password,
> +        '-j default',
> +        '-l "%s (%s)"' % (hostname, username),
> +    ]) + '\n'
> +    args = ['/usr/bin/security', '-i']
> +    p = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
> +    p.communicate(command)

I noticed that when using the python helper, the dialog asking something
like: "security wants to know this password. Allow it?"

Which was kind of lame. I would hope we could convince it to say "git".
But I didn't see any option in the "security" tool for specifying the
context[1]. The C helper says "git-credential-osxkeychain". Which isn't
the end of the world, but it would be prettier if it just said "git".

-Peff

[1] I can kind of see why they might not want you to set this for
security reasons (because it makes impersonating other programs easy).
On the other hand, saying "security" conveys absolutely nothing. And as
far as I can tell, I could just call my program /tmp/iTunes, and it
would say "iTunes wants to know this password...".

^ permalink raw reply

* Re: Annotated branch ≈ annotated tag?
From: Jeff King @ 2011-09-29  6:44 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Michael J Gruber, Junio C Hamano, git
In-Reply-To: <4E82A13B.2080509@alum.mit.edu>

On Wed, Sep 28, 2011 at 06:23:23AM +0200, Michael Haggerty wrote:

> It seems to me that an annotated branch is very much like an (unsigned)
> annotated tag, except that it is movable and disposable like a normal
> branch.  What would be the ramifications of using an annotated-tag-like
> object to record metainformation about a branch?  (Let's just call it an
> "annotation object" for this discussion.)
> 
> * The branch would point not at a commit but at an annotation object
> that points at a commit.
> 
> * Obviously, a new annotation object would have to be written every time
> the branch is updated.

Leaving aside for a moment whether this is a good system or not, I think
it's infeasible at this point simply because it is so far from what
current git does, and in such a visible way.

Consider the interactions between this system and older versions of git.
Won't all of the older clients see this annotation cruft at the tip of
each branch? How will they react? It would no longer be correct to make
commits with "git commit-tree $tree `git rev-parse HEAD`", would it?

-Peff

^ permalink raw reply

* Re: Lack of detached signatures
From: Junio C Hamano @ 2011-09-29  3:50 UTC (permalink / raw)
  To: Ted Ts'o
  Cc: Jeff King, Joseph Parmelee, Carlos Martín Nieto,
	Olsen, Alan R, Michael Witten, git@vger.kernel.org
In-Reply-To: <20110929015919.GL19250@thunk.org>

Ted Ts'o <tytso@mit.edu> writes:

>> That would improve the situation (I suspect that there
>> were some people who misunderstood that these GPG signature were to
>> protect against break-in at the master machine), but at the same time, it
>> may create the chicken-and-egg bootstrapping problem if public keys of too
>> many people need to be published securely.
>
> We are in the process of bootstrapping a GPG web of trust.  Linus has
> generated a new GPG key which has been signed by Peter Anvin, Dirk,
> and myself.  We'll get a much richer set of cross signatures at the
> Kernel Summit in Prague in a few months.

I was actually more worried about helping consumers convince themselves
that thusly signed keys indeed belong to producers like Linus, Peter,
etc. There are those who worry that DNS record to code.google.com/ for
them may point at an evil place to give them rogue download material.
"Here are the keys you can verify our trees with" message on the mailing
list, even with the message is signed with GPG, would not be satisfactory
to them.

^ permalink raw reply

* What's next for "signed push"?
From: Junio C Hamano @ 2011-09-29  3:42 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Robin H. Johnson
In-Reply-To: <7v62kc1v7m.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> "Robin H. Johnson" <robbat2@gentoo.org> writes:
>
>> from CVS to Git (we're very close now), we've decided that the signed
>> pushes will provide better security than our plan of previous plan of
>> using signed notes, so we'd like to see signed pushes succeed.
>
> Could you elaborate on your "previous plan" a bit? What is a signed note,
> how would it help validate the authenticity, how do developers interact
> using it and what do you perceive as weaknesses compared to the signed
> push that we discussed a few weeks ago?

I was hoping that I could get another food-for-thought from people who
thought about signed commits before sending this out, but here is my
current thinking (I retitled your "Signed push progress" as there is
nothing to "progress" on without an active discussion).

Originally, I very much wanted to like the approach of v3 that was meant
to be simpler by having the logic to record the signed push certificate
only on the sending end. At the mechanism level, v3 looked simpler to me,
but from the point of view of end users, I doubt that it is simpler than
the approach of v2, where the sender prepares a push record, signs and
sends it, and the receiver records it to its notes tree to publish so that
others can fetch and verify.

Here are some of the issues, from end users' point of view, that v3 would
have that v2 would not, off the top of my head:

 - Unless you are pushing into a repository solely for your own push, you
   have to first fetch the notes ref from where you are about to push to,
   then hope that your push does not conflict with others. If your push is
   rejected for non-fast-forward of the signed-push notes tree (but not
   for your real branches), you would have to rewind the push certificate
   the failed "git push" prepared (you could probably add a patch to the
   v3 to do so automatically, but I haven't looked closely for all
   possible failure cases), run "git fetch", and then run "git push -s"
   again which would create a new push record for you to sign. Because the
   "signed-push" namespace for notes is meant to cover all the branches,
   this will not work on a busy site that uses CVS/SVN style "shared
   central repository" workflow at all.

 - If you are pushing into multiple places, you would somehow need to
   configure your end to keep one signed-push notes tree per remote that
   you intend to push to with signature, to avoid contaminating remote
   repositories of records of your push into other remote repositories.
   It could be worked around by even more code on the sending end, but the
   need for configuring alone is already an additional mental burden.

 - It also was hoped that pre-receive or pre-update hook on the receiving
   end can be used to authenticate and authorize the push itself with the
   approach by v3, but when the check happens, the signed-notes tree to be
   used for verification is not connected to any ref in the refs/notes/
   hierarchy yet (otherwise it won't be pre-* hook). The query interface
   "git notes show" needs to be updated so that it takes not just a ref
   via the GIT_NOTES_REF interface, which is defined to specify a ref
   because some subcommands of "git notes" need to create a new commit and
   update it, but a bare notes tree commit object name [*1*]. We may need
   to update "git notes" (at least "show" subcommand) for the use of
   receiving end; v3 is no longer a simpler "sender only" solution.

I've shown how both v2 and v3 models would look to the end users with
working code, thought about the pros and cons probably longer than anybody
else, and at this point, if I were to choose between the two approaches
[*2*], I am inclined to suggest that we go with the v2 model [*3*].

Either that, or we will see follow-up patches to work around the above
(and there may be others we may later discover) issues from people who
still think v3 is a better approach.

Whether we go with v2 or v3, for people who want to verify the commits
against signed push certificates stored in notes tree:

 - We need a wrapper like "tag --verify".

 - We also need a way to merge these signed pushed certificates [*4*]. I
   think the default notes merge is to concatenate, which would result in
   duplicates of the records that was present in the common ancestor (and
   no, "union" merge is not a safe way to remove these duplicates).

But see footnote *2* below.


[Footnotes]

*1* I wouldn't be surprised if it already worked when you give the object
name of the notes-tree commit to GIT_NOTES_REF when running "git show",
but that is not really a documented interface and working by accident. The
environment variable was designed to take a name of the ref.

*2* I say "if I were to choose between the two" for a reason. It will make
things simpler if we drop "add signature separately to notes" altogether,
and instead adopt a "signed commit" approach.  Embed GPG signature in a
commit object, and allow the receiving end of the "push" to be configured
to reject a push that tries to place a non-signed commit at the tip of a
ref. The same "tip of a ref must be a signed commit" check can be done for
"fetch".

The most attractive part of the "signed commit" approach is that it does
not force Linus to fetch push-signature notes trees from his lieutenants,
and merge them to his push-signature notes tree, which is an unnecessary
chore. The most likely thing to happen, especially under v3 design, would
be that higher level maintainers will not bother to fetch/verify/merge the
signed-push notes trees from their feeders, and the final publishing site
will only have the push certificates from the owner of the repository at
the top-level, without downstream contributors' signature.  The v2 design
already relies on the final verifier to independently collect signed-push
notes from publishing repositories of Linus and all the key repositories
Linus pulled from before verification, which feels more cumbersome, but
the same needs to be done in v3 if the higher level maintainers do not buy
in the fetch/verify/merge overhead for the push-signature notes tree.

If signatures are embeded in the commits themselves, the issue of merging
push-signature notes tree disappears. Whenever the top level maintainer
pulls from his lieutenants, his fetch can (and should) check the signature
of these lieutenants, and their signatures stay in the history the top
level maintainer integrates and eventually pushes out with his own
signature.

As to the embedding of the signature in the commit, I am inclined to put
the lines of GPG detached signature in new header lines, after the
standard tree/parent/author/committer headers, instead of tucking it at
the end of the commit log message text, for multiple reasons:

 - The signature won't clutter output from "git log" and friends if it is
   in the extra header. If we place it at the end of the log message, we
   would need to teach "git log" and friends to strip the signature block
   with an option.

 - Teaching new versions of "git log" and "gitk" to optionally verify and
   show signatures is cleaner if we structurally know where the signature
   block is (instead of scanning in the commit log message).

 - The signature needs to be stripped upon various commit rewriting
   operations, e.g. rebase, filter-branch, etc. They all already ignore
   unknown headers, but if we place signature in the log message, all of
   these tools (and third-party tools) also need to learn how a signature
   block would look like.

 - When we added the optional encoding header, all the tools (both in tree
   and third-party) that acts on the raw commit object should have been
   fixed to ignore headers they do not understand, so it is not like that
   new header would be more likely to break than extra text in the commit.

*3* Honestly speaking, I myself was disturbed by the v2 model where the
signed-push is recorded primarily at the receiver. At the philosophical
level, the approach seems to go very much against the "distributed" nature
of Git. Sending what you want to be committed to the server and having the
server make a commit feels so very SVN/CVS. The usual "push" workflow for
Git users is to fetch first to come close to the other end, integrate your
work to prepare what you push out contains all of what the other end has,
and then pushing the result out, hoping that you are the latest and nobody
else had a chance to update the same thing.

But after thinking about it a bit more, I came to realize that the record
of push is quite unlike the branches you and others work on. First of all,
you are recording what happens at the receiving end, "I updated these refs
to these values in this repository". Making the record at the receiving
end is more natural than writing "I plan to update these refs", sending it
over to a dumb receiver and hoping it will fast-forward.

Don't get me wrong. The offline distributed workflow is a great enabler in
a distributed system like Git. The work you do on your branches can be
fully asynchronous to outside world, and being able to have local history
that can later be merged in order to avoid losing work by others while
still allowing us to be asynchronous is a great thing to have, but the act
of pushing the end result and recording the fact you pushed them into a
repository is inherently a synchronous event---you and the receiving
repository have to be connected when your "push" happens. I do not see a
need to be dogmatic and insist that everything we do is asynchronous.

*4* For the purpose of pushing things out, even with the v3 design, a
pusher does not have to worry about merging the signed-push notes tree
(there is no merge issue for pushers with the v2 model), I think.

"git push -s" will add a push record to the notes tree, and if the result
does not fast forward, "git fetch $remote +refs/notes/signed-push" (or use
per-remote signed-push hierarchy "+refs/notes/$remote/signed-push") that
discards the single failed push record would be all that is needed before
attempting "git push -s" again without losing any information.

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Julian Phillips @ 2011-09-29  2:19 UTC (permalink / raw)
  To: Martin Fick; +Cc: Christian Couder, git, Christian Couder, Thomas Rast
In-Reply-To: <960aacbf-8d4d-4b2a-8902-f6380ff9febd@email.android.com>

On Wed, 28 Sep 2011 19:37:18 -0600, Martin Fick wrote:
> On Wednesday 28 September 2011 18:59:09 Martin Fick wrote:
>> Julian Phillips <julian@quantumfyre.co.uk> wrote:
-- snip --
>> I've created a test repo with ~100k refs/changes/... style refs, and
>> ~40000 refs/heads/... style refs, and checkout can walk the list of
>> ~140k refs seven times in 85ms user time including doing whatever 
>> other
>> processing is needed for checkout. The real time is only 114ms - but
>> then my test repo has no real data in.
>
> If I understand what you are saying, it sounds like you do not have a
> very good test case. The amount of time it takes for checkout depends
> on how long it takes to find a ref with the sha1 that you are on. If
> that sha1 is so early in the list of refs that it only took you 7
> traversals to find it, then that is not a very good testcase. I think
> that you should probably try making an orphaned ref (checkout a
> detached head, commit to it), that is probably the worst testcase
> since it should then have to search all 140K refs to eventually give
> up.
>
> Again, if I understand what you are saying, if it took 85ms for 7
> traversals, then it takes approximately 10ms per traversal, that's
> only 100/s! If you have to traverse it 140K times, that should work
> out to 1400s ~ 23mins.

Well, it's no more than 10ms per traversal - since the rest of the work 
presumably takes some time too ...

However, I had forgotten to make the orphaned commit as you suggest - 
and then _bang_ 7N^2, it tries seven different variants of each ref 
(which is silly as they are all fully qualified), and with packed refs 
it has to search for them each time, all to turn names into hashes that 
we already know to start with.

So, yes - it is that list traversal.

Does the following help?

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5e356a6..f0f4ca1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -605,7 +605,7 @@ static int add_one_ref_to_rev_list_arg(const char 
*refname,
                                        int flags,
                                        void *cb_data)
  {
-       add_one_rev_list_arg(cb_data, refname);
+       add_one_rev_list_arg(cb_data, strdup(sha1_to_hex(sha1)));
         return 0;
  }

-- 
Julian

^ permalink raw reply related

* Re: Lack of detached signatures
From: Ted Ts'o @ 2011-09-29  1:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Joseph Parmelee, Carlos Martín Nieto,
	Olsen, Alan R, Michael Witten, git@vger.kernel.org
In-Reply-To: <7vd3ekxkca.fsf@alter.siamese.dyndns.org>

On Wed, Sep 28, 2011 at 05:28:53PM -0700, Junio C Hamano wrote:
> 
> I suspect that letting GPG do the compression and shipping foo.tar.gpg
> would work just fine as well, 

Good point.  If only "tar -xW foo.tar.gpg" automatically verified the
gpg signature, that would work really well indeed.  :-)

> I understand that the automated GPG signature k.org used to use on the
> master machine was primarily to protect the copies that the mirrors serve
> from getting tampered after they leave the master machine. Do you happen
> to know what the new policy will be? Will the developers who distribute
> their snapshot tarballs from the site be GPG signing them themselves
> before uploading?

This is still being negotiated.  Given that developers are starting to
sign their release tags (and of course Linus has been doing this
already), one of the things that I've proposed is that we support is
to have the developer do something like this:

git archive --format=tar -o e2fsprogs-1.41.12.tar v1.41.12
gzip -9n e2fsprogs-1.41.12.tar
gpg --sign --detach -a e2fsprogs-1.41.12.tar.gz

and then just uploading the tar.gz.gpg file, the URL for the git tree,
and the tag that the server should use do the extraction.

> That would improve the situation (I suspect that there
> were some people who misunderstood that these GPG signature were to
> protect against break-in at the master machine), but at the same time, it
> may create the chicken-and-egg bootstrapping problem if public keys of too
> many people need to be published securely.

We are in the process of bootstrapping a GPG web of trust.  Linus has
generated a new GPG key which has been signed by Peter Anvin, Dirk,
and myself.  We'll get a much richer set of cross signatures at the
Kernel Summit in Prague in a few months.

Also, there's a pretty good intersection between kernel developers and
the Debian web of trust; there's been some talk of using that as an
auxiliary bootstrap for isolated kernel developers in distant part of
the world.

					- Ted

^ permalink raw reply

* Re: Lack of detached signatures
From: Jeff King @ 2011-09-29  1:41 UTC (permalink / raw)
  To: Ted Ts'o
  Cc: Junio C Hamano, Joseph Parmelee, Carlos Martín Nieto,
	Olsen, Alan R, Michael Witten, git@vger.kernel.org
In-Reply-To: <20110928230958.GJ19250@thunk.org>

On Wed, Sep 28, 2011 at 07:09:58PM -0400, Ted Ts'o wrote:

> On Wed, Sep 28, 2011 at 06:25:43PM -0400, Jeff King wrote:
> > [1] This is a minor nit, and probably not worth breaking away from the
> > way the rest of the world does it, but it is somewhat silly to sign the
> > compressed data. I couldn't care less about the exact bytes in the
> > compressed version; what I care about is the actual tar file. The
> > compression is just a transport.
> 
> The worry I have is that many users don't check the GPG checksum files
> as it is.  If they have to decompress the file, and then run gpg to
> check the checksum, they might never get around to doing it.

It shouldn't really be any more cumbersome. But at the same time, it's
different than the way everyone else does it, so any minor convenience
we get is probably nullified by simply confusing anybody.

I wonder how many people actually check gpg checksums on downloaded
files. I don't usually. But I do expect something like a package manager
building from upstream source (e.g., freebsd-style ports, or distro
packagers pulling a new upstream) to bother to check it.

> That being said, I'm not sure I have a good solution.  One is to ship
> the file without using detached signatures, and ship a foo.tar.gz.gpg
> file, and force them to use GPG to unwrap the file before it can be
> unpacked.  But users would yell and scream if we did that...

And rightly so. I mentioned the cost of implementing the mechanism
before. If it's just "Junio runs gpg and throws the detached signature
up on the ftp site", it's not a big deal. But if it's "you can't
download and install git until you have gpg installed", that is raising
the bar quite a bit.

It should be the recipient's decision how much they want to trust the
data. We would just be helping them out by providing more information.

-Peff

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-29  1:37 UTC (permalink / raw)
  To: Julian Phillips; +Cc: Christian Couder, git, Christian Couder, Thomas Rast
In-Reply-To: <c76d7f65203c0fc2c6e4e14fe2f33274@quantumfyre.co.uk>

On Wednesday 28 September 2011 18:59:09 Martin Fick wrote: 
> Julian Phillips <julian@quantumfyre.co.uk> wrote: 
> > On Wed, 28 Sep 2011 16:10:48 -0600, Martin Fick wrote: 
> >> So with that bug fixed, the thing taking the most time 
> >> now for a git checkout with ~100K refs seems to be the 
> >> orphan check as Thomas predicted. The strange part with 
> >> this, is that the orphan check seems to take only about 
> >> ~20s in the repo where the refs aren't packed. However, 
> >> in the repo where they are packed, this check takes at 
> >> least 5min! This seems a bit unusual, doesn't it? Is 
> >> the filesystem that much better at indexing refs than 
> >> git's pack mechanism? Seems unlikely, the unpacked refs 
> >> take 312M in the FS, the packed ones only take about 
> >> 4.3M. I suspect their is something else unexpected 
> >> going on here in the packed ref case. 
> >> 
> >> Any thoughts? I will dig deeper... 
> > 
> > I think the problem is that resolve_ref() walks a linked 
> > list of searching for the packed ref. Does this mean that 
> > packed refs are not indexed at all? 
> > Are you sure that it is walking the linked list that is the problem?

It sure seems like it.

> I've created a test repo with ~100k refs/changes/... style refs, and 
> ~40000 refs/heads/... style refs, and checkout can walk the list of 
> ~140k refs seven times in 85ms user time including doing whatever other 
> processing is needed for checkout. The real time is only 114ms - but 
> then my test repo has no real data in.

If I understand what you are saying, it sounds like you do not have a very good test case. The amount of time it takes for checkout depends on how long it takes to find a ref with the sha1 that you are on. If that sha1 is so early in the list of refs that it only took you 7 traversals to find it, then that is not a very good testcase. I think that you should probably try making an orphaned ref (checkout a detached head, commit to it), that is probably the worst testcase since it should then have to search all 140K refs to eventually give up.

Again, if I understand what you are saying, if it took 85ms for 7 traversals, then it takes approximately 10ms per traversal, that's only 100/s! If you have to traverse it 140K times, that should work out to 1400s ~ 23mins.

> If resolve_ref() walking the linked list of refs was the problem, then > I would expect my test repo to show the same problem. It doesn't, a pre 
> ref-packing checkout took minutes (~0.5s user time), whereas a 
> ref-packed checkout takes ~0.1s. So, I would suggest that the problem > lies elsewhere. 
> 
> Have you tried running a checkout whilst profiling?

No, to be honest, I am not familiar with any profilling tools.

-Martin

Employee of Qualcomm Innovation Center,Inc. which is a member of Code Aurora Forum

^ 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