Git development
 help / color / mirror / Atom feed
* [PATCH v4] Docs: git checkout --orphan: Copyedit, and s/root commit/orphan branch/
From: Michael Witten @ 2011-09-29 15:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Carlos Martín Nieto, vra5107, Michael J Gruber, Matthieu Moy,
	Eric Raible, Philip Oakley, Jeff King, Jay Soffian, git
In-Reply-To: <271cc2ed03774b4988bb61cb3e79750e-mfwitten@gmail.com>

It's copyediting; it's best just to read the damn patch, particularly when
there are no subtle details to be considered beyond the exhausting game of
making everybody involved in the email thread feel like he or she has gotten
a portion of the bikeshed painted a certain color.

See:

  Re: Can a git changeset be created with no parent
  Carlos Martín Nieto <cmn@elego.de>
  Message-ID: <1317073309.5579.9.camel@centaur.lab.cmartin.tk>

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-checkout.txt |   69 ++++++++++++++++++++++++++++-----------
 1 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index c0a96e6..bf042c2 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -125,29 +125,58 @@ explicitly give a name with '-b' in such a case.
 	below for details.
 
---orphan::
-	Create a new 'orphan' branch, named <new_branch>, started from
-	<start_point> and switch to it.  The first commit made on this
-	new branch will have no parents and it will be the root of a new
-	history totally disconnected from all the other branches and
-	commits.
-+
-The index and the working tree are adjusted as if you had previously run
-"git checkout <start_point>".  This allows you to start a new history
-that records a set of paths similar to <start_point> by easily running
-"git commit -a" to make the root commit.
-+
-This can be useful when you want to publish the tree from a commit
-without exposing its full history. You might want to do this to publish
-an open source branch of a project whose current tree is "clean", but
-whose full history contains proprietary or otherwise encumbered bits of
-code.
-+
-If you want to start a disconnected history that records a set of paths
-that is totally different from the one of <start_point>, then you should
-clear the index and the working tree right after creating the orphan
-branch by running "git rm -rf ." from the top level of the working tree.
-Afterwards you will be ready to prepare your new files, repopulating the
-working tree, by copying them from elsewhere, extracting a tarball, etc.
+--orphan::
+	Perform these two tasks:
++
+--
+	* Adjust the working tree and index as if you ran
+	  "git checkout <start_point>".
+
+	* Set up git to turn the next commit you create into a root commit
+	  (that is, a commit without any parent); creating the next commit
+	  is similar to creating the first commit after running "git init",
+	  except that the new commit will be referenced by <new_branch>
+	  rather than "master".
+--
++
+Then, by just running "git commit", you can create a root commit
+with a tree that is exactly the same as the tree of <start_point>.
+Alternatively, before creating the commit, you may manipulate the
+index in any way you want; for example, to create a root commit with
+a tree that is totally different from the tree of <start_point>,
+just clear the working tree and index first: From the top level of
+the working tree, run "git rm -rf .", and then prepare your new
+working tree and index as desired.
++
+There are two common uses for this option:
++
+--
+	Separate history::
+		Suppose that for convenience, you want to maintain
+		in the same repository as your project some ancillary
+		material that is infrequently altered.  In such a case,
+		it may not make much sense to interleave the history of
+		that material with the history of your project; you can
+		use the "--orphan" option in order to create completely
+		separate histories.
+
+	Hidden history::
+		Suppose you have a project that has proprietary
+		material that is never meant to be released to the
+		public, yet you now want to maintain an open source
+		history that may be published widely.
++
+In this case, it would not be enough just to remove the proprietary
+material from the working tree and then create a new commit, because
+the proprietary material would still be accessible through the new
+commit's ancestry; the proprietary history must be hidden from the new
+commit, and the "--orphan" option allows you to do so by ensuring that
+the new commit has no parent.
++
+However, when there are multiple commits that are already suitable for
+the open source history (or that you want to make suitable), you should
+instead consider working with linkgit:git-filter-branch[1] and possibly
+linkgit:git-rebase[1].
+--
 
 -m::
 --merge::
-- 
1.7.6.409.ge7a85

^ permalink raw reply related

* Re: [PATCH v3] Docs: git checkout --orphan: `root commit' and `branch head'
From: Michael Witten @ 2011-09-29 15:52 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Carlos Martín Nieto, vra5107, Michael J Gruber, Matthieu Moy,
	Eric Raible, Philip Oakley, Jeff King, Jay Soffian, git
In-Reply-To: <7vaa9oz9rl.fsf@alter.siamese.dyndns.org>

On Wed, 28 Sep 2011 13:34:22 -0700, Junio C Hamano wrote:

> As to what the updated text wants to talk about, I think most of them are
> improvement, but there are a few glitches and nits.
>
>> ...
>>
>> +--orphan::
>> +	Tell git to turn the next commit you create into a root commit
>> +	(that is, a commit without any parent); creating the next commit
>> +	is similar to creating the first commit after running "git{nbsp}init",
>
> I do not think we ever used {nbsp} in our documentation set. Is it
> absolutely necessary to use it in this context, and are you absolutely
> sure this does not break various versions of documentation toolchain
> people have?

I think it's absolutely necessary, but I'm not sure about how fragile
asciidoc is across versions with regard to this... I guess I'll remove
them.

>> +	except that the new commit will be referenced by the branch head
>> +	<new_branch> rather than "master".
>
> The option works as a substitute for -B/-b in the sense that it takes a
> name of the new branch, and the only difference is that the new branch
> will start with no commit (yet).  To reduce confusion, it might make sense
> to update this part (and description of -b/-B) to begin like this:
>
> 	--orphan <new_branch>::
>
> to match the new explanation text, and the output from "git checkout -h".
> By the way, shouldn't the entry for -b in "git checkout -h" output also
> say <new branch>?

I took the "<new_branch>" from the synopsis:

  SYNOPSIS
  --------
  [verse]
  'git checkout' [-q] [-f] [-m] [<branch>]
  'git checkout' [-q] [-f] [-m] [--detach] [<commit>]
  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
  'git checkout' [-p|--patch] [<tree-ish>] [--] [<paths>...]

It would seem strange to include "<new_branch>" but not "<start_point>"
in each of the individual option descriptions.

In any case, I think we should leave that for another patch.

Moreover, I think it would be better to remove all `-h' output from
all git commands; they are always incomplete and/or different. Indeed,
at this point in history, programmers absolutely stink at writing
documentation, so it's probably a good idea just to centralize all
information as much as possible. Of course, I realize this suggestion
will never fly.

> Micronit: "referenced by the branch head <new_branch>" might be easier to
> read and understand with s/branch head/branch name/. Or perhaps
>
> 	except that the new commit will be made on <new_branch> branch
>         rather than "master".
>
> might be even better.

I have qualms with git's terminology, and using "branch head" as I have is
not only the closest thing to what I like, but it is also the only language
that is actually in complete agreement with git's own terminolgy as currently
defined by "git help glossary".

In the glossary, the term "branch name" only occurs once as part of:

  branch named "master"

which is undefined.

However, how skirting the whole issue:

  except that the new commit will be referenced by <new_branch>
  rather than "master".

>> +Furthermore, the working tree and index are adjusted as if you ran
>> +"git{nbsp}checkout{nbsp}<start_point>"; thus, by just running
>> +"git{nbsp}commit", you can create a root commit with a tree that is
>> +exactly the same as the tree of <start_point>.
>> ++
>> +Naturally, before creating the commit, you may manipulate the index
>> +in any way you want. ...
>
> What value does "Naturally, " add to the understanding here? I would
> understand if it were "Alternatively, ", though.

"Naturally" reassures the reader that nothing really special is happening
in terms of how the next commit can be made; there are no special cases.

"Alternatively" is too strong; these 2 paragraphs are not mutually
exclusive: The bit about running just "git commit" is only a
clarification of the preceding sentence.

Nevertheless, I have rearranged the text to use "Alternatively" in 
[PATCH v4]:

  Message-ID: <3cba6bb85bde4f96903b2b617190a2b8-mfwitten@gmail.com>
  Subject: [PATCH v4] Docs: git checkout --orphan: Copyedit,
			    and s/root commit/orphan branch/

It should be a reply to this email.

>> +... For example, if you want to create a root commit
>> +with a tree that is totally different from the tree of <start_point>,
>> +then just clear the working tree and index first: From the top level
>> +of the working tree, run "git{nbsp}rm{nbsp}-rf{nbsp}.", and then
>> +prepare your new working tree and index as desired.
>> ++
>> +There are two common uses for this option:
>> ++
>> +--
>> +	Separate history::
>> +		Suppose that for convenience, you want to maintain
>> +		the website for your project in the same repository
>> +		as the project itself. In such a case, it may not
>> +		make much sense to interleave the history of the
>> +		website with the history of the project; you can use
>> +		the "--orphan" option in order to create these two
>> +		completely separate histories.
>
> I suspect this is *not* common at all, and also because you would need a
> working tree to update both lines of histories that are not related to
> each other at all at the content level, I do not believe that "for
> convenience" supports or justifies the use case at all. It would be less
> convenient to update these two unrelated histories (switching between
> branches would need to nuke the whole working tree, and the semantics to
> carry local changes floating on top of the tip commit of the current
> branch across branch switching actively works against you).
>
> You would be better off using another repository to keep track of "the
> website for your project".
>
> In short, I do not think we would want to list the above as if we are
> somehow encouraging the use of this option for such a use. It falls into
> "because with --orphan you _could_", and definitely not "because having
> these two unrelated projects in the same repository you work in is
> convenient and/or necessary".

I was thinking of something like git's repo's "man", "html", and "todo",
but I was trying to express that along the lines of a more widely
recognizable scenario.

See [PATCH v4] for a more generic version.

>> +	Hidden history::
>> +		Suppose you have a project that has proprietary
>> +		material that is never meant to be released to the
>> +		public, yet you now want to maintain an open source
>> +		history that may be published widely.
>
> This cause does make sense; you would want the local changes floating on
> top of the tip commit of the current branch carried across branch
> switching.
>
>> +In this case, it would not be enough just to remove the proprietary
>> +material from the working tree and then create a new commit, because
>> +the proprietary material would still be accessible through the new
>> +commit's ancestry; the proprietary history must be hidden from the new
>> +commit, and the "--orphan" option allows you to do so by ensuring that
>> +the new commit has no parent.
>
> Does this "In this case" paragraph format as part of the "Hidden history"
> paragraph?

Yes. Have you no faith in me?

>> +However, removing proprietary material from ancestry is usually a task
>> +that is better performed by linkgit:git-filter-branch[1] and
>> +linkgit:git-rebase[1], especially when there are multiple commits that
>> +are already suitable for the open source history.
>> +--
>
> In general it is true, and not "especially".
> ...
> I suspect this is not exactly "filter-branch is better but you could use
> checkout --orphan" like you made it sound in the above paragraph.  If you
> are bootstrapping a new open source project from the tip of a proprietary
> tree, "checkout --orphan && edit to sanitize && commit" to start your
> history afresh would be perfectly adequate for your PR people to say "Now
> we are open", especially when you do not intend to accept fixes to older
> revisions; you could filter-branch the older proprietary history but you
> would not get much benefit out of the cost for doing so, I think.

I don't really see how what I wrote is fundamentally different. However,
I've rearranged it in [PATCH v4].

>> See:
>>
>>   Re: Can a git changeset be created with no parent
>>   Carlos Mart=C3=ADn Nieto <cmn@elego.de>
>>   Message-ID: <1317073309.5579.9.camel@centaur.lab.cmartin.tk>
>>   http://article.gmane.org/gmane.comp.version-control.git/182170
>>
>> and:
>>
>>   git help glossary
>
> I think I had to tell somebody on this list not to do this no more than a
> month ago.

What's your point?

> It is a good practice to point to earlier discussions while polishing
> patch, and it also is good to include pointers in the commit log message
> as a _supporting material_ (additional reading), but that is *NOT* a
> substitute for a properly written commit log message. You need to state
> what problem you are trying to fix and how the proposed patch fixes it.

That's a good rule of thumb. Although, interestingly, one my last code
patches for git was accompanied by a very detailed, precisely formulated
description of the problem and the solution, and yet it was dumbed down
behind my back.

In this case, though, I think the message title gives enough information,
particularly when combined with the diff: It's a commit that alters the
documentation for "git checkout --orphan", and somehow involves the terms
"root commit" and "branch head" (though no longer in [PATCH v4]); the rest
of the log message is just meant to be cake icing for the interested.

It's copyediting; it's best just to read the damn patch, particularly when
there are no subtle details to be considered beyond the exhausting game of
making everybody involved in the email thread feel like he or she has gotten
a portion of the bikeshed painted a certain color.

Hmmm... You know what? I think I'll make that last paragraph the message.

Sincerely,
Michael Witten

^ permalink raw reply

* Re: [BUG?] gitk assumes initial commit is empty
From: Junio C Hamano @ 2011-09-29 16:40 UTC (permalink / raw)
  To: Zbigniew Jędrzejewski-Szmek; +Cc: git
In-Reply-To: <4E8481A2.6060301@in.waw.pl>

Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:

> 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.

It is not about assuming but by unfortunate design.

In early days, all projects managed by git (except for git itself) had the
product of a fairly mature development hsitory in their first commit, and
it was deemed unnecessary clutter to show additions of these thousands of
paths as a patch.

This was not just about gitk but git itself. "git log" learned to show the
patch for the initial commit without requiring --root command line option
only at 0f03ca9 (config option log.showroot to show the diff of root
commits, 2006-11-23).

These days I think gitk should learn to do the same ;-)

^ permalink raw reply

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

On Wednesday, September 28, 2011 08:19:16 pm Julian Phillips 
wrote:
> 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;
>   }


Yes, but in some strange ways. :)

First, let me clarify that all the tests here involve your 
"sort fix" from 2 days ago applied first.

In the packed ref repo, it brings the time down to about 
~10s (from > 5 mins).  In the unpacked ref repo, it brings 
it down to about the same thing ~10s, but it was only 
starting at about ~20s.

So, I have to ask, what does that change do, I don't quite 
understand it?  Does it just do only one lookup per ref by 
normalizing it?  Is the list still being traversed, just 
about 7 time less now?  Should the packed_ref list simply be 
put in an array which could be binary searched instead, it 
is a fixed list once loaded, no?  

I prototyped a packed_ref implementation using the hash.c 
provided in the git sources and it seemed to speed a 
checkout up to almost instantaneous, but I was getting a few 
collisions so the implementation was not good enough.  That 
is when I started to wonder if an array wouldn't be better 
in this case?



Now I also decided to go back and test a noop fetch (a 
refetch) of all the changes (since this use case is still 
taking way longer than I think it should, even with the 
submodule fix posted earlier).  Up until this point, even 
the sorting fix did not help.  So I tried it with this fix.  
In the unpackref case, it did not seem to change (2~4mins).  
However, in the packed ref change (which was previously also 
about 2-4mins), this now only takes about 10-15s!

Any clues as to why the unpacked refs would still be so slow 
on noop fetches and not be sped up by this?


-Martin


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

^ permalink raw reply

* git bundle unbundle and "check-outable" refs
From: Todd A. Jacobs @ 2011-09-29 15:51 UTC (permalink / raw)
  To: git

Never having needed git bundle before, I've recently been using it as
a sneakernet. In particular, I'm using bundles to work around
limitations of filesystem semantics on vfat and hpfs+ drives when
shared between Linux and OS X systems. The systems are air-gapped, so
sneakernet is essential.

At any rate, the issue I'm dealing with is that "git bundle unbundle"
is sort of non-intuitive to deal with. It seems to add the commits
directly to the local repository, but doesn't give me any of the
branch refs that I'm expecting from such an operation. In other words,
if I bundle branch foo on machine A, then unbundle on machine B, I
expect to be able to "git checkout foo" and get on with life.

Instead, it seems that I have to figure out what commits were
unbundled, and create a new branch ref pointing to the head that I
want. I assume this is to prevent namespace collisions, but it seems
really, really cumbersome. Wouldn't it make more sense to include
branch names in the bundle, and simply prompt the user to rename refs
that conflict?

I'm certainly open to other ideas of how to accomplish this workflow,
and if there's an invocation to simplify this that I'm unaware of,
please advise. Otherwise, I really think the default behavior of the
unbundle sub-command ought to be more intuitive.

^ permalink raw reply

* 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


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