[Survey] Signed push

[Survey] Signed push

[Junio C Hamano]

[administrivia] This message is also Cc'ed to the kernel mailing list in
order to ask for opinions from members of one of the most important user
communities of Git, but people may want to drop the kernel list when
responding to this message to reduce the noise level over there. Thanks.

In the light of what happened to k.org recently, we've been discussing
things Git can do to help raising confidence levels perceived by the
general public on integrity of the source trees, especially for the kernel
community. As the article by Jonathan Corbet on lwn.net nicely described,
projects managed with Git are already pretty resistant from tampering, and
it is not my (nor anybody in the Git community's) intention to propose any
more unnecessary bureaucracy to the development process without merit.

There are two updates that may change the end user experience I would like
to ask your opinions on, both as the Git designer (emeritus?) and as the
top kernel developer.


1. Improved pull requests.

Currently a typical pull-request begins like this:

    The following changes since commit f696543dad6c7ba27b0c4fab167a5687263a9ba0:

      Flobar 2.4.3 (2011-09-13 12:34:56 +0900)

    are available in the git repository at:
      git://git.kernel.org/pub/flobar.git/ master

which is followed by the shortlog and expected diffstat.  This tells you
where the requester based his work on in excruciating detail, but does not
tell you what you should expect to fetch, any more than "whatever happened
to be at the named branch when you happened to notice the request."

We have a tentative patch to add an extra line after the "URL branch" line
that is for your cut & paste that looks like:

    are available in the git repository at:
      git://git.kernel.org/pub/flobar.git/ master
    for you to fetch changes up to 5738c9c21e53356ab5020912116e7f82fd2d428f

I often see you respond to a pull request on the kernel mailing list with
"I see nothing new; forgot to push?", and having this extra line may also
help communication.

Would it be just an added and useless noise that you nor your requesters
would not care much about?

An alternative that I am considering is to let the requester say this
instead:

    are available in the git repository at:
      git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f

without adding the extra line.

That is, to allow fetching the history up to an explicitly named commit
object. This would only involve a change to fetch-pack at the receiving
end; just match the commit object name given from the command line against
the ls-remote response and ask upload-pack to give the history leading to
it. The released versions of Git already will happily oblige, as long as
the commit object named in the request message still sits at the tip of
the intended branch.

Do you think it is worthwhile to pursue this alternative?


2. Signed pushes.

You tag official releases and release candidates with your GPG key, and
everybody who works within the kernel ecosystem trusts the history behind
the commits pointed by them, but there is no easy way to verify that
commits and merges between the last tagged commit and the tip of your
branch(es) are indeed from you, or if an intruder piled fake ones on top
of your commits (until you try to push again and discover that the history
does not fast-forward, that is).

We have been discussing an addition of "git push -s" to let people sign
their pushes (instead of having to sign every commit or add signed
tag). The implementation alternatives were being bikeshed but not of much
interest in this message, but the user experience would go like this:

 * You push out your work with "git push -s";

 * "git push" prepares a "push certificate" (it is meant to certify "these
   are the commits I place at the tips of these refs"), which is a human
   and machine readable text file in core, that may look like this:

        Push-Certificate-Version: 0
        Pusher: Junio C Hamano <gitster@pobox.com>
        Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
        Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next

   and asks you to GPG sign it. You only unlock your GPG key and the
   command internally runs GPG, just like "tag -s".

 * When "git push" finishes, the receiving end has this record in its
   refs/notes/signed-push notes tree, together with your previous pushes
   (as this is not a shared repository, it will record only your pushes).
   The notes annnotate the commits named on the "Update:" lines above.

 * People who want to verify commits that are not yet tagged near the tip
   in their clone of your tree can fetch refs/notes/signed-push and run

     $ git log --show-notes=signed-push --branches --not --tags

   to see your push certificates as annotations on commits that are not
   yet tagged. They can verify them using a tool (yet to be written) that
   acts like "git tag --verify".

It is hoped that it would help downstream with warm and fuzzy assurances
that all commits including the ones that are not yet tagged are genuine
(disclaimer: my employer is among the "downstream" that wants to have that
warm and fuzzy assurance) if we can see these push certificates published
at your public repository.

A few questions.

 * As a user, do you think "signed push" is a good idea, or is it merely
   an unnecessary bureaucracy, having to sign all pushes?

 * As a user, do you think it is a good thing that you could also verify
   the commits you receive from the Git-managed repositories of your
   lieutenants using this mechanism, or you wouldn't bother, perhaps
   because you are applying many patches sent via unsigned e-mail from
   Andrew anyway?

 * If the answers to the above points are both "yes", do you think it
   would make sense to also propagate the push certificates you obtain
   from your lieutenants to your public repository when you make your
   "push -s"? It will allow your downstream to follow the chain of trust
   in one-go (if you are pulling from public places, they can fetch the
   push certificates from your lieutenants themselves and merge them, so
   this is merely a convenience feature) by simply fetching from the
   refs/notes/signed-push notes tree from your public repository.  Do you
   think it is a useful and worthwhile thing to do?

[PATCH v2 0/2] State commit name explicitly in request-pull messages

[Junio C Hamano]

Here is an alternative approach to the earlier "request-pull" patch.

Junio C Hamano (2):
  fetch: allow asking for an explicit commit object by name
  request-pull: state exact commit object name

 git-request-pull.sh     |    2 +-
 remote.c                |   25 +++++++++++++++++++++++--
 t/t5150-request-pull.sh |   11 +++++++----
 3 files changed, 31 insertions(+), 7 deletions(-)

-- 
1.7.7.rc1.1.g1e5814

[PATCH v2 1/2] fetch: allow asking for an explicit commit object by name

[Junio C Hamano]

This teaches "git fetch" (hence "git pull") to accept an explicit commit
object name in the LHS of the refspec, as long as the named commit is at
the tip of an advertised ref. E.g.

    $ git pull origin 5738c9c21e53356ab5020912116e7f82fd2d428f
    $ git fetch origin 5738c9c21e53356ab5020912116e7f82fd2d428f:refs/remotes/origin

would behave exactly as if you asked

    $ git pull origin refs/heads/master
    $ git fetch origin refs/heads/master:refs/remotes/origin

when the output from "git ls-remote origin" said the remote side has the
commit object whose name is 5738c9c21e53356ab5020912116e7f82fd2d428f at
the tip of refs/heads/master branch ref.

This does not allow asking for a random object that may or may not exist
in the repository (this has been a longstanding security feature).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote.c |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/remote.c b/remote.c
index ca42a12..76c2943 100644
--- a/remote.c
+++ b/remote.c
@@ -1387,6 +1387,25 @@ struct ref *get_remote_ref(const struct ref *remote_refs, const char *name)
 	return copy_ref(ref);
 }
 
+/*
+ * Allow fetching an explicitly-named commit from the command line,
+ * but only if it exactly matches the commit at the tip of one of the
+ * advertised refs.
+ */
+static struct ref *get_remote_commit(const struct ref *remote_refs, const char *hex)
+{
+	const struct ref *ref;
+	unsigned char sha1[20];
+
+	if (get_sha1_hex(hex, sha1) || hex[40])
+		return NULL;
+
+	for (ref = remote_refs; ref; ref = ref->next)
+		if (!strchr(ref->name, '^') && !hashcmp(sha1, ref->old_sha1))
+			return copy_ref(ref);
+	return NULL;
+}
+
 static struct ref *get_local_ref(const char *name)
 {
 	if (!name || name[0] == '\0')
@@ -1416,8 +1435,10 @@ int get_fetch_map(const struct ref *remote_refs,
 		const char *name = refspec->src[0] ? refspec->src : "HEAD";
 
 		ref_map = get_remote_ref(remote_refs, name);
-		if (!missing_ok && !ref_map)
-			die("Couldn't find remote ref %s", name);
+		if (!ref_map)
+			ref_map = get_remote_commit(remote_refs, name);
+		if (!ref_map && !missing_ok)
+			die("Couldn't find remote ref that matches %s", name);
 		if (ref_map) {
 			ref_map->peer_ref = get_local_ref(refspec->dst);
 			if (ref_map->peer_ref && refspec->force)
-- 
1.7.7.rc1.1.g1e5814

[PATCH v2 2/2] request-pull: state exact commit object name

[Junio C Hamano]

A typical pull-request begins like this:

  The following changes since commit f696543dad6c7ba27b0c4fab167a5687263a9ba0:

    Flobar 2.4.3 (2011-09-13 12:34:56 +0900)

  are available in the git repository at:
    git://git.kernel.org/pub/flobar.git/ master

which is followed by the shortlog and expected diffstat. This tells you
where the requester based his work on in excruciating detail, but does not
tell you what you should expect to fetch, any more than "whatever happened
to be at the named branch when you happened to notice the request."

Update the message slightly to say:

    git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f ;# master

so that the line still can be cut&pasted after "git fetch" (or "git
pull"), to form a command line that looks like:

    $ git <repository> <full commit object name> ;# branch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-request-pull.sh     |    2 +-
 t/t5150-request-pull.sh |   11 +++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index fc080cc..b5a2d0f 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -70,7 +70,7 @@ git show -s --format='The following changes since commit %H:
   %s (%ci)
 
 are available in the git repository at:' $baserev &&
-echo "  $url $branch" &&
+echo "  $url $headrev ;# $branch" &&
 echo &&
 
 git shortlog ^$baserev $headrev &&
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9cc0a42..e9d657e 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -70,9 +70,10 @@ test_expect_success 'setup: two scripts for reading pull requests' '
 	/ in the git repository at:$/!d
 	n
 	/^$/ n
-	s/^[ 	]*\(.*\) \([^ ]*\)/please pull\
+	s/^[ 	]*\(.*\) \([^ ]*\) ;# \([^ ]*\)/please pull\
 	\1\
-	\2/p
+	\2\
+	\3/p
 	q
 	EOT
 
@@ -145,6 +146,7 @@ test_expect_success 'pull request after push' '
 	{
 		read task &&
 		read repository &&
+		read head &&
 		read branch
 	} <digest &&
 	(
@@ -153,6 +155,7 @@ test_expect_success 'pull request after push' '
 		git pull --ff-only "$repository" "$branch"
 	) &&
 	test "$branch" = for-upstream &&
+	test "$head" = "$(GIT_DIR=downstream.git git rev-parse for-upstream)" &&
 	test_cmp local/mnemonic.txt upstream-private/mnemonic.txt
 
 '
@@ -170,10 +173,10 @@ test_expect_success 'request names an appropriate branch' '
 		git request-pull initial "$downstream_url" >../request
 	) &&
 	sed -nf read-request.sed <request >digest &&
-	cat digest &&
 	{
 		read task &&
 		read repository &&
+		read head &&
 		read branch
 	} <digest &&
 	{
@@ -193,7 +196,7 @@ test_expect_success 'pull request format' '
 	  SUBJECT (DATE)
 
 	are available in the git repository at:
-	  URL BRANCH
+	  URL OBJECT_NAME ;# BRANCH
 
 	SHORTLOG
 
-- 
1.7.7.rc1.1.g1e5814

Re: [Survey] Signed push

[Guenter Roeck]

On Tue, Sep 13, 2011 at 12:45:37PM -0400, Junio C Hamano wrote:
[ ... ]

> 1. Improved pull requests.
> 
noise for me

[ ... ]

> 2. Signed pushes.
> 
Excellent idea, long since overdue.

Guenter

Re: [Survey] Signed push

[Junio C Hamano]

Guenter Roeck <guenter.roeck@ericsson.com> writes:

> On Tue, Sep 13, 2011 at 12:45:37PM -0400, Junio C Hamano wrote:
> [ ... ]
>
>> 1. Improved pull requests.
>> 
> noise for me

Are you among the ones who respond to pull requests?

Re: [Survey] Signed push

[Junio C Hamano]

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

> Guenter Roeck <guenter.roeck@ericsson.com> writes:
>
>> On Tue, Sep 13, 2011 at 12:45:37PM -0400, Junio C Hamano wrote:
>> [ ... ]
>>
>>> 1. Improved pull requests.
>>> 
>> noise for me
>
> Are you among the ones who respond to pull requests?

Sorry, this didn't come out quite the way I intended.

As I do not know every developer on earth, I would like to know in what
capacity you (figuratively---I mean everybody who gives his opinion on
this topic) fit in your ecosystem. Otherwise I cannot tell if many people
who receive pull requests find it noise but senders do not care, or many
people who send them find it noise but receivers do appreciate, etc.

For the purpose of commenting on "pull requests" topic, one can be (1)
a bystander, who does not request nor respond to pull requests, (2) who
gets requests to pull, or (3) who sends requests to pull.

The same for "signed pushes". In this case, one can be (1) who pushes, (2)
who fetches and wants to verify what he gets, (3) both (e.g. Linus
playing role (3) while fetching from his lieutenants and then role (1)
when pushing his integration results out).

Re: [Survey] Signed push

[Sam Vilain]

On 9/13/11 9:45 AM, Junio C Hamano wrote:
>   * You push out your work with "git push -s";
>
>   * "git push" prepares a "push certificate" (it is meant to certify "these
>     are the commits I place at the tips of these refs"), which is a human
>     and machine readable text file in core, that may look like this:
>
>          Push-Certificate-Version: 0
>          Pusher: Junio C Hamano<gitster@pobox.com>
>          Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
>          Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next
>
>     and asks you to GPG sign it. You only unlock your GPG key and the
>     command internally runs GPG, just like "tag -s".
>
>   * When "git push" finishes, the receiving end has this record in its
>     refs/notes/signed-push notes tree, together with your previous pushes
>     (as this is not a shared repository, it will record only your pushes).
>     The notes annnotate the commits named on the "Update:" lines above.

If the push certificate also has the previous commit IDs for the changed 
refs, then you actually have an audit log.  Otherwise, it does not 
certify the commit range they pushed.

This is an important prerequisite for a fully distributed, peer to peer 
git.  For this case it would also need something to distinguish which 
repository is to be updated; such as a canonical repository URL (or list 
of URLs), or just a short project name.  A P2P protocol can then know 
projects as (KEYID, projectname).

Sam

Re: [Survey] Signed push

[Shawn Pearce]

On Tue, Sep 13, 2011 at 17:31, Sam Vilain <sam@vilain.net> wrote:
> On 9/13/11 9:45 AM, Junio C Hamano wrote:
>>
>>  * "git push" prepares a "push certificate" (it is meant to certify "these
>>    are the commits I place at the tips of these refs"), which is a human
>>    and machine readable text file in core, that may look like this:
>>
>>         Push-Certificate-Version: 0
>>         Pusher: Junio C Hamano<gitster@pobox.com>
>>         Update: 3793ac56b4c4f9bf0bddc306a0cec21118683728 refs/heads/master
>>         Update: 12850bec0c24b529c9a9df6a95ad4bdeea39373e refs/heads/next
>
> If the push certificate also has the previous commit IDs for the changed
> refs, then you actually have an audit log.  Otherwise, it does not certify
> the commit range they pushed.

Is that necessary? The range they are certifying is that commit, and
its entire ancestry. If the pusher doesn't trust his ancestry, why is
he working with it? Similar to an annotated tag. I make a signed
annotated tag, I am asserting that revision and its ancestry is
something I like as far as a project build goes. You don't need the
old revision to realize I like this commit.

If you want to get into the game of, maybe I push a branch, then
rewind it, and push something differently, and you want to be able to
verify that the 2nd push is the "right thing" and the 1st push should
be ignored, you can already see that by looking at the timestamp of
the push certificates (/me assumes there is a timestamp in there). If
you can create multiple signed pushes by yourself, using your GPG key,
within the same second, and they are conflicting... well, stop using
automated tools to create conflicting assertions as yourself. If you
are creating signed pushes on systems with clock skew, learn how to
configure NTP date.

> This is an important prerequisite for a fully distributed, peer to peer git.
>  For this case it would also need something to distinguish which repository
> is to be updated; such as a canonical repository URL (or list of URLs), or
> just a short project name.  A P2P protocol can then know projects as (KEYID,
> projectname).

Why do we need a project name? Most Git based projects are uniquely
identified by the set of root commits they have. Why? Because most
root commits were created by different people, at different times,
with different commit messages, and different initial trees, resulting
in a unique commit SHA-1 for that root commit. Projects with more than
one root commit also disambiguate themselves from other projects that
maybe contain one of those roots (e.g. git.git vs. gitk).

If you wanted to identify a project on a P2P network, I think you
would want to do it based off the root commits, not some random name
people came up with and might try to publish forgeries under.

-- 
Shawn.

Re: [Survey] Signed push

[Sam Vilain]

On 9/13/11 5:39 PM, Shawn Pearce wrote:
> >  If the push certificate also has the previous commit IDs for the changed
> >  refs, then you actually have an audit log.  Otherwise, it does not certify
> >  the commit range they pushed.
> Is that necessary? The range they are certifying is that commit, and
> its entire ancestry. If the pusher doesn't trust his ancestry, why is
> he working with it? Similar to an annotated tag. I make a signed
> annotated tag, I am asserting that revision and its ancestry is
> something I like as far as a project build goes. You don't need the
> old revision to realize I like this commit.

Perhaps because they didn't notice what happened.  Someone else pushed 
to the server without a signed push somehow, and then they pulled, 
pushed ... and now as far as you know, those commits are certified like 
any other.  Having this extra information, not much information, will 
help figure out what happens in this sort of situation.

>> This is an important prerequisite for a fully distributed, peer to peer git.
>>   For this case it would also need something to distinguish which repository
>> is to be updated; such as a canonical repository URL (or list of URLs), or
>> just a short project name.  A P2P protocol can then know projects as (KEYID,
>> projectname).
> Why do we need a project name? Most Git based projects are uniquely
> identified by the set of root commits they have. Why? Because most
> root commits were created by different people, at different times,
> with different commit messages, and different initial trees, resulting
> in a unique commit SHA-1 for that root commit. Projects with more than
> one root commit also disambiguate themselves from other projects that
> maybe contain one of those roots (e.g. git.git vs. gitk).
>
> If you wanted to identify a project on a P2P network, I think you
> would want to do it based off the root commits, not some random name
> people came up with and might try to publish forgeries under.
>

Yes, this is true, but it also makes it a lot harder to figure out if 
two projects are from the same real project, or whether they just shared 
some history.  In general, git repositories are partitioned by URL or 
project, and so this makes a soft case for a distributed system to 
partition itself by URL or project also.

Sam

Fwd: [Survey] Signed push

[Linus Torvalds]

Recovering lost emails. Or maybe you get duplicates. Sorry about that if so,

                   Linus

---------- Forwarded message ----------
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, Sep 13, 2011 at 10:48 AM
Subject: Re: [Survey] Signed push
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, linux-kernel@vger.kernel.org


On Tue, Sep 13, 2011 at 9:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> We have a tentative patch to add an extra line after the "URL branch" line
> that is for your cut & paste that looks like:
>
>    are available in the git repository at:
>      git://git.kernel.org/pub/flobar.git/ master
>    for you to fetch changes up to 5738c9c21e53356ab5020912116e7f82fd2d428f
>
> I often see you respond to a pull request on the kernel mailing list with
> "I see nothing new; forgot to push?", and having this extra line may also
> help communication.

I think that would probably be a good idea, although I'd actually
prefer you to be more verbose, and more human-friendly, and actually
talk about the commit in a readable way. Get rid of the *horrible*
BRANCH-NOT-VERIFIED message (that actually messes up pull requests if
mirroring is a bit delayed and throws away more important
information), and instead just have a blurb afterwards saying
something human-readable like

 Top commit 1f51b001cccf: "Merge branches 'cns3xxx/fixes',
 'omap/fixes' and 'davinci/fixes' into fixes"

 and at *that* point you might have a "UNVERIFIED" notice for people
to check if they forgot to push.

So I'd much prefer something like that over:

> An alternative that I am considering is to let the requester say this
> instead:
>
>    are available in the git repository at:
>      git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>
> without adding the extra line.

The extra line in the pull request is cheap - it's not like we need to
ration them. The above format, in contrast, requires that the person
doing the *pull* have a recent enough git client, otherwise the merge
commit message will be just horrible.

And even if you do have a new git client that turns the commit into a
branch name, that's ambigious. What if both 'master' and
'experimental' have the same top commit, because experimental ended up
being tested and was percolated to master? Which branch name would you
pick? And what if the branch was updated since, so *no* branch name
matches - does that mean that you'd disallow the pull entirely?

> 2. Signed pushes.
>
> You tag official releases and release candidates with your GPG key, and
> everybody who works within the kernel ecosystem trusts the history behind
> the commits pointed by them, but there is no easy way to verify that
> commits and merges between the last tagged commit and the tip of your
> branch(es) are indeed from you, or if an intruder piled fake ones on top
> of your commits (until you try to push again and discover that the history
> does not fast-forward, that is).
>
> We have been discussing an addition of "git push -s" to let people sign
> their pushes (instead of having to sign every commit or add signed
> tag). The implementation alternatives were being bikeshed but not of much
> interest in this message, but the user experience would go like this:

Also, if we're adding branch information, I'd say that a description
of the branch is more important than a signature. Right now we lack
even that.

It would be lovely if people could annotate their branches with
descriptions, so that when I pull a "for-linus" branch, if it has a
description, the description of the branch makes it into the merge
message. Our merge messages are often not very informative.

I realize that cryptographic signature sound very important right now,
but in the end, *real* trust comes from people, not from signatures.
Realistically, I checked a few signatures this time around due to the
k.org issues, but at the same thing, the thing that made me trust most
of it was just looking at commits and the email messages. The
unconscious and non-cryptographic "signature" of a person acting like
you expect a person to act.

Technical measures can be subverted, and I think we should also think
about the social side. Every time somebody mentions a signature, I
want to also mention "human readability", because I think that matters
as much, if not more.

So I'm not against signed pushes, but quite frankly, if you add some
per-branch signature, I would argue against it unless that signature
also comes with information that allows us to do a better job of human
communication too. Like a branch description.

Imagine, for example, than when you do a

  git push -s ..

git would *require* you to actually write a message about what you are
pushing. And when somebody pulls it, and creates a merge commit, that
explanation would become part of the merge message. The "signature"
part of the "-s" should be thought of as the *much* less interesting
part - that's just a small detail that git can use to verify
something, but it doesn't actually matter for the contents of the
pull. Not like the actual human-readable message would.

Now *that* would be lovely. No?

                       Linus

Re: Fwd: [Survey] Signed push

[Michael Haggerty]

On 09/14/2011 09:06 AM, Linus Torvalds wrote:
> So I'm not against signed pushes, but quite frankly, if you add some
> per-branch signature, I would argue against it unless that signature
> also comes with information that allows us to do a better job of human
> communication too. Like a branch description.
> 
> Imagine, for example, than when you do a
> 
>   git push -s ..
> 
> git would *require* you to actually write a message about what you are
> pushing. And when somebody pulls it, and creates a merge commit, that
> explanation would become part of the merge message. The "signature"
> part of the "-s" should be thought of as the *much* less interesting
> part - that's just a small detail that git can use to verify
> something, but it doesn't actually matter for the contents of the
> pull. Not like the actual human-readable message would.
> 
> Now *that* would be lovely. No?

Instead of "like a branch description", why not implement branch
descriptions directly?

I wish that one could annotate a branch (e.g., at creation) and have the
annotation follow the branch around.  This would be a useful place to
record *why* you created the branch, your plans for it, etc.  The
annotation should be modifiable, because often a branch evolves in
unforeseen ways during its lifetime.  Anybody could read the annotation
to get a quick idea of what kind of work is in progress.

Such a branch annotation could be used in pull requests, the cover
letter of patch series emails, merge commit log messages, etc.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Fwd: [Survey] Signed push

[Matthieu Moy]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I wish that one could annotate a branch (e.g., at creation) and have the
> annotation follow the branch around.  This would be a useful place to
> record *why* you created the branch, your plans for it, etc.  The
> annotation should be modifiable, because often a branch evolves in
> unforeseen ways during its lifetime.  Anybody could read the annotation
> to get a quick idea of what kind of work is in progress.

Would the notes mechanism be able to annotate ref names instead of
commit sha1?

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

Re: Fwd: [Survey] Signed push

[Nguyen Thai Ngoc Duy]

On Wed, Sep 14, 2011 at 9:03 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> I wish that one could annotate a branch (e.g., at creation) and have the
>> annotation follow the branch around.  This would be a useful place to
>> record *why* you created the branch, your plans for it, etc.  The
>> annotation should be modifiable, because often a branch evolves in
>> unforeseen ways during its lifetime.  Anybody could read the annotation
>> to get a quick idea of what kind of work is in progress.
>
> Would the notes mechanism be able to annotate ref names instead of
> commit sha1?

Speaking from someone who has few experience with git-notes, no I
don't think current git-notes can do that. But similar mechanism can
be added, targeting ref instead of sha-1. But the question is, is
branch description local or public?

Branch description sounds local to me. I just record a branch's
purpose and status (the latter is more important to me). If it's
local, we just need to extend ref format to store extra text in
addition to SHA-1.

If it's public, perhaps if we have a good way to:
 - convert arbitrary text to a ref (maybe just converting spaces to hyphens)
 - specify a ref without writing the whole ref name (reminds me of
short sha-1 vs full sha-1)

then we could use branch name as branch description. The default merge
commit messages can be updated to convert back branch name (which is
also the branch description) to human-readable text.
-- 
Duy

Re: [Survey] Signed push

[Nguyen Thai Ngoc Duy]

On Wed, Sep 14, 2011 at 2:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 1. Improved pull requests.
>
> ...
>
> An alternative that I am considering is to let the requester say this
> instead:
>
>    are available in the git repository at:
>      git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>
> without adding the extra line.
>
> That is, to allow fetching the history up to an explicitly named commit
> object. This would only involve a change to fetch-pack at the receiving
> end; just match the commit object name given from the command line against
> the ls-remote response and ask upload-pack to give the history leading to
> it. The released versions of Git already will happily oblige, as long as
> the commit object named in the request message still sits at the tip of
> the intended branch.
>
> Do you think it is worthwhile to pursue this alternative?

Stupid question, if we agree to go with signed push, can we also sign
pull requests and verify them when we pull? I suppose most of the
time, pulling can be done automatically by extracting pull url from
the request. This would make pull/push both signed.

BTW, there's a third way (rsync is obsolete) to carry changes away in
human-unreadable way: bundles. Should we also sign the bundles too (I
guess we could just do the same as in signed push).
-- 
Duy

Re: Fwd: [Survey] Signed push

[Johan Herland]

On Wednesday 14. September 2011, Matthieu Moy wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> > I wish that one could annotate a branch (e.g., at creation) and
> > have the annotation follow the branch around.  This would be a
> > useful place to record *why* you created the branch, your plans
> > for it, etc.  The annotation should be modifiable, because often a
> > branch evolves in unforeseen ways during its lifetime.  Anybody
> > could read the annotation to get a quick idea of what kind of work
> > is in progress.
> 
> Would the notes mechanism be able to annotate ref names instead of
> commit sha1?

This has been discussed on the list before, but I'm too lazy to dig up a 
reference, so:

The notes mechanism can in principle annotate anything that has a SHA1 
sum. The notes tree is really only a key->value mapping using SHA1s as 
keys and Git objects (typically blobs) as values.

HOWEVER, "git notes prune" will assume that the SHA1 keys are supposed 
to identify existing git objects, and will delete any note whose SHA1 
key does not identify a reachable git object.

Hence, if you promise to never run "git notes prune" on 
refs/notes/branch-descriptions, you could use that ref to store your 
branch descriptions keyed by the SHA1 of your branch name.

The obvious deficiency with this scheme is that if your branch name is 
different in some repos, the branch description will be lost in those 
repos unless you rewrite the refs/notes/branch-descriptions notes tree 
accordingly.


...Johan

-- 
Johan Herland, <johanh@opera.com>
Core Developer, Opera Software ASA

Re: Fwd: [Survey] Signed push

[Ted Ts'o]

On Wed, Sep 14, 2011 at 02:28:52PM +0200, Johan Herland wrote:
> On Wednesday 14. September 2011, Matthieu Moy wrote:
> > Michael Haggerty <mhagger@alum.mit.edu> writes:
> > > I wish that one could annotate a branch (e.g., at creation) and
> > > have the annotation follow the branch around.  This would be a
> > > useful place to record *why* you created the branch, your plans
> > > for it, etc.  The annotation should be modifiable, because often a
> > > branch evolves in unforeseen ways during its lifetime.  Anybody
> > > could read the annotation to get a quick idea of what kind of work
> > > is in progress.
> > 
> HOWEVER, "git notes prune" will assume that the SHA1 keys are supposed 
> to identify existing git objects, and will delete any note whose SHA1 
> key does not identify a reachable git object.
> 
> Hence, if you promise to never run "git notes prune" on 
> refs/notes/branch-descriptions, you could use that ref to store your 
> branch descriptions keyed by the SHA1 of your branch name.

It seems like notes is the wrong place to encode this.  If people
really want this, what if there was a convention where there could be
a separate branch head: ref/heads/META

which contained a directory structure like this:

<e-mail>/key			# The developer's GPG key
<e-mail>/<tree>/URL		# URL of developer's tree named <tree>
<e-mail>/<tree>/description	# Descrition of <tree>
<e-mail>/<tree>/branch/<branch-name>	# A description of that branch

etc.

Since it's a separate branch head, the contents can be pushed around
and merged very easily, and there's no danger of the information
getting lost via a garbage collection or prune operation.

If there was an association between a local branch and <e-mail>/<tree>
that it was tracking, then either a modified git core or porcelein
command could get the information from the META tree.  It would also
make it easy to fetch a developer's GPG key without having to go to
outside GPG key servers, which is a minor benefit (although maybe
that's not worth it).

	     	   	   	      	 - Ted

Re: Fwd: [Survey] Signed push

[Linus Torvalds]

On Wed, Sep 14, 2011 at 3:45 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>
> Instead of "like a branch description", why not implement branch
> descriptions directly?

Mostly because of just human interface issues.

We already have a "repository description", and it's quite commonly
never even filled in. For branches, that would be doubly true, because
a lot of branches are throw-away.

So I think it would work if we made it part of of something like "git
push -s" - because that's when it starts mattering to others.

> I wish that one could annotate a branch (e.g., at creation) and have the
> annotation follow the branch around.  This would be a useful place to
> record *why* you created the branch, your plans for it, etc.  The
> annotation should be modifiable, because often a branch evolves in
> unforeseen ways during its lifetime.  Anybody could read the annotation
> to get a quick idea of what kind of work is in progress.

I wouldn't be against that as a concept, I just think you'd be a small
small minority, and most branches would never get annotated.

But I don't really care deeply how it actually works - my main issue
is that git makes it way too easy to have bad merge messages. I think
part of that is an even simpler idiocy: we never even fire up the
editor by default for a "git merge", but we do for a "git commit".
That was a design mistake, and it means that if you want to actually
add a note to a merge, you have to do extra work. So people don't.

Now people do "git merge" in scripts etc, so we can't fix it ;-(

                    Linus

Re: Fwd: [Survey] Signed push

[Linus Torvalds]

On Wed, Sep 14, 2011 at 4:03 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>
> Would the notes mechanism be able to annotate ref names instead of
> commit sha1?

That would be a horrible, horrible notion.

It's quite common to have multiple branches with the same SHA1. It
might be in the "experimental-development" branch, but it got through
testing with flying colors and deemed to be stable, so it got upgraded
to the "for-linus" branch, and there hasn't been any other development
since. So now both "for-linus" and "experimental-development" are the
same commit, but they are very much not the same branch!

So no, don't confuse branch *contents* with branch *descriptions*.

                          Linus

Re: Fwd: [Survey] Signed push

[Matthieu Moy]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Sep 14, 2011 at 4:03 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>
>> Would the notes mechanism be able to annotate ref names instead of
>> commit sha1?
>
> That would be a horrible, horrible notion.
>
> It's quite common to have multiple branches with the same SHA1. 

That's why my question was about annotating ref _names_, yes. As Johan
Herland pointed out, this would be possible in theory, but not really in
practice.

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

Re: Fwd: [Survey] Signed push

[Johan Herland]

On Wednesday 14. September 2011, Linus Torvalds wrote:
> On Wed, Sep 14, 2011 at 4:03 AM, Matthieu Moy
> 
> <Matthieu.Moy@grenoble-inp.fr> wrote:
> > Would the notes mechanism be able to annotate ref names instead of
> > commit sha1?
> 
> That would be a horrible, horrible notion.
> 
> It's quite common to have multiple branches with the same SHA1. It
> might be in the "experimental-development" branch, but it got through
> testing with flying colors and deemed to be stable, so it got
> upgraded to the "for-linus" branch, and there hasn't been any other
> development since. So now both "for-linus" and
> "experimental-development" are the same commit, but they are very
> much not the same branch!
> 
> So no, don't confuse branch *contents* with branch *descriptions*.

I don't think the suggestion was about annotating the branch tip as a 
way of describing the branch. Rather, you create a _new_ SHA1 that 
identifies the branch (e.g. SHA1(branch_name) ), and then annotate 
_that_ SHA1. As I said, that _can_ be done with the notes 
infrastructure, but - as Ted noted - there might be better solutions to 
storing branch descriptions.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: Fwd: [Survey] Signed push

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> I think that would probably be a good idea, although I'd actually
> prefer you to be more verbose, and more human-friendly, and actually
> talk about the commit in a readable way. Get rid of the *horrible*
> BRANCH-NOT-VERIFIED message (that actually messes up pull requests if
> mirroring is a bit delayed and throws away more important
> information), and instead just have a blurb afterwards saying
> something human-readable like
>
>  Top commit 1f51b001cccf: "Merge branches 'cns3xxx/fixes',
>  'omap/fixes' and 'davinci/fixes' into fixes"
>
>  and at *that* point you might have a "UNVERIFIED" notice for people
> to check if they forgot to push.

That UNVERIFIED thing was neither my favorite nor my idea, and I'd happily
rip it out in any second ;-)

>> An alternative that I am considering is to let the requester say this
>> instead:
>>
>>    are available in the git repository at:
>>      git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>>
>> without adding the extra line.
>
> The extra line in the pull request is cheap - it's not like we need to
> ration them. The above format, in contrast, requires that the person
> doing the *pull* have a recent enough git client, otherwise the merge
> commit message will be just horrible.

In a re-roll patch I've added ";# branch-name" at the end of that line for
people with older git, but existing git wouldn't allow you to fetch anything
but refs so you won't risk getting "just horrible" merge message ;-)

> ... And what if the branch was updated since, so *no* branch name
> matches - does that mean that you'd disallow the pull entirely?

You are right about ambiguities, but when the specified commit does not
match the branch, it was indeed my intention to claim it is a _feature_ 
that pull fails, as you would be getting something different from what you
thought was promised by the requester with bait-and-switch.

> Also, if we're adding branch information, I'd say that a description
> of the branch is more important than a signature. Right now we lack
> even that.

I do not particularly want to go into that tangent, and I do agree with
your later message in this thread that it may make sense to tie the
publishing (and possibly recording) of the description of the branch to
"push -s"; people simply do not have reason to name throw-away branches.

> It would be lovely if people could annotate their branches with
> descriptions, so that when I pull a "for-linus" branch, if it has a
> description, the description of the branch makes it into the merge
> message.

I'm wondering if this could be something we can share between the push
certificate "Into this repository, I pushed this commit to that branch,
whose pupose is..." and pull request "...so please pull it to merge into
your history." There are three possibile orders of things a lieutenant or
a contributor may want to do after perfecting his tree locally:

 (1) Write pull-request, and then "push -s".

 (2) "push -s", and then write pull-request.

 (3) "push -s" auto-mailing a pull-request.

> I realize that cryptographic signature sound very important right now,
> but in the end, *real* trust comes from people, not from signatures.
> ...
> Technical measures can be subverted, and I think we should also think
> about the social side. Every time somebody mentions a signature, I
> want to also mention "human readability", because I think that matters
> as much, if not more.

I obviously agree 100%, but that is an argument against trusting only
technical measures---right now, we do not have a good technical measure to
validate latest commits not yet contained in any tagged releases.

A piece of e-mail to the kernel list from you that says "I pushed it out
and the tip is this SHA-1", if it is written in good English with a bit of
your usual humor sprinkled in, would in practice be just as good as GPG 
for the kernel list regulars who can recognize your style and serve as
that "technical measure" (by the way "What's cooking" does have the tips
of master and next branches for this exact reason).

> Imagine, for example, than when you do a
>
>   git push -s ..
>
> git would *require* you to actually write a message about what you are
> pushing.

Yeah, we could go in that direction.

Re: Fwd: [Survey] Signed push

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Now people do "git merge" in scripts etc, so we can't fix it ;-(

Yes you can, by teaching "git merge -e" to open an editor ;-).

In the meantime, "commit --amend" is your friend.

Re: Fwd: [Survey] Signed push

[Linus Torvalds]

On Wed, Sep 14, 2011 at 10:52 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Now people do "git merge" in scripts etc, so we can't fix it ;-(
>
> Yes you can, by teaching "git merge -e" to open an editor ;-).
>
> In the meantime, "commit --amend" is your friend.

The problem with both of those are human: because it's extra work,
it's not going to get done.

In contrast, if it's extra work to *avoid* editing the merge message,
people probably would see the editor pop up, and start adding a few
small notes.

That's why "default behavior" matters so much. It's not that it's not
*possible* to edit a merge message, it's that nobody ever does!

                        Linus

Re: [Survey] Signed push

[Andy Lutomirski]

On 09/13/2011 09:45 AM, Junio C Hamano wrote:
> 
> An alternative that I am considering is to let the requester say this
> instead:
> 
>     are available in the git repository at:
>       git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
> 
> without adding the extra line.
> 
> That is, to allow fetching the history up to an explicitly named commit
> object. This would only involve a change to fetch-pack at the receiving
> end; just match the commit object name given from the command line against
> the ls-remote response and ask upload-pack to give the history leading to
> it. The released versions of Git already will happily oblige, as long as
> the commit object named in the request message still sits at the tip of
> the intended branch.

I would love this feature on the pull/fetch interface, but for a
completely different reason.  Sometimes I want to pull a particular
object (usually a commit, but sometimes just a tree or blob) from
*myself*, and having to stick it on a branch is annoying.

One use-case is when applying a patch in git's extended format.  If I
know where it came from, I ought to be able to pull the blobs it depends
on to enable three-way merge.  I think that this is essentially
impossible remotely right now.

Of course, merging with the result of the pull will result in terrible
automatically-generated messages, but it's easy to fix that up manually.

This is one thing that I think Mercurial handles better than git.  (And
apologies for the noise if I've missed a way to do this with current
git.  I've looked, but maybe I missed some magic way to do this.)

--Andy

Re: [Survey] Signed push

[Junio C Hamano]

Andy Lutomirski <luto@MIT.EDU> writes:

>> An alternative that I am considering is to let the requester say this
>> instead:
>> 
>>     are available in the git repository at:
>>       git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>> 
>> without adding the extra line.
>> 
>> That is, to allow fetching the history up to an explicitly named commit
>> object. This would only involve a change to fetch-pack at the receiving
>> end; just match the commit object name given from the command line against
>> the ls-remote response and ask upload-pack to give the history leading to
>> it....
>
> I would love this feature on the pull/fetch interface, but for a
> completely different reason.  Sometimes I want to pull a particular
> object (usually a commit, but sometimes just a tree or blob) from
> *myself*, and having to stick it on a branch is annoying.

I am afraind that it is not going to happen; see

    http://article.gmane.org/gmane.comp.version-control.git/181317

for a rationale.

Re: [Survey] Signed push

[Andrew Lutomirski]

On Wed, Sep 14, 2011 at 1:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Andy Lutomirski <luto@MIT.EDU> writes:
>
>>> An alternative that I am considering is to let the requester say this
>>> instead:
>>>
>>>     are available in the git repository at:
>>>       git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
>>>
>>> without adding the extra line.
>>>
>>> That is, to allow fetching the history up to an explicitly named commit
>>> object. This would only involve a change to fetch-pack at the receiving
>>> end; just match the commit object name given from the command line against
>>> the ls-remote response and ask upload-pack to give the history leading to
>>> it....
>>
>> I would love this feature on the pull/fetch interface, but for a
>> completely different reason.  Sometimes I want to pull a particular
>> object (usually a commit, but sometimes just a tree or blob) from
>> *myself*, and having to stick it on a branch is annoying.
>
> I am afraind that it is not going to happen; see
>
>    http://article.gmane.org/gmane.comp.version-control.git/181317
>
> for a rationale.
>

Do you mean that it's a security feature?  What if a .git/config
option existed to allow this use?  Or even a git upload-pack option
that turned it *on* and was stripped by git-shell?

--Andy

Re: Fwd: [Survey] Signed push

[Sam Vilain]

[re-send as text-only.  sorry, new system]

On 9/14/11 10:49 AM, Junio C Hamano wrote:
>> The extra line in the pull request is cheap - it's not like we need to
>> >  ration them. The above format, in contrast, requires that the person
>> >  doing the*pull*  have a recent enough git client, otherwise the merge
>> >  commit message will be just horrible.
> In a re-roll patch I've added ";# branch-name" at the end of that line for
> people with older git, but existing git wouldn't allow you to fetch anything
> but refs so you won't risk getting "just horrible" merge message;-)
>

If the system is watertight enough, then you could have verified pushes 
ONLY under some new refspace, like:

refs/forks/forkid/branch

"forkid" is set up when you choose to "follow" someone's pushes.  ie, 
you say something like:

git fork follow linustorvalds@osdl.org 
<http://pgp.mit.edu:11371/pks/lookup?op=vindex&search=0x17762C4676E21CBB>

"torvalds@osdl.org" here representing a PGP key ID search string.  It 
could be instead, "0x17762c4676e21cbb"

And then the "refs/forks/linus/xxx" space gets populated as signed 
pushes are seen on the network (perhaps when fetching from a pull 
request, or perhaps via a service).  A configuration option can be set 
to warn if you are not merging a signed branch.  If the new "pull 
request/push" object is a distinct object type or tag object, then it 
could be the destination of the 'refs/forks/...' ref, and the branch 
desription and hence the default merge message be retrieved from that.

Sam

Re: [Survey] Signed push

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:
> On Wed, Sep 14, 2011 at 2:45 AM, Junio C Hamano <gitster@pobox.com> wrote:

>> An alternative that I am considering is to let the requester say this
>> instead:
>>
>>    are available in the git repository at:
>>      git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
[...]
> Stupid question, if we agree to go with signed push, can we also sign
> pull requests and verify them when we pull? I suppose most of the
> time, pulling can be done automatically by extracting pull url from
> the request. This would make pull/push both signed.
>
> BTW, there's a third way (rsync is obsolete) to carry changes away in
> human-unreadable way: bundles. Should we also sign the bundles too (I
> guess we could just do the same as in signed push).

If I understand you correctly, then ordinary PGP email signing[1]
should work for that already.  In your first example, the receiver can
make sure whatever process grabs a pull request verifies it, and in
the second example, the receiver checks the signature on her email
before saving a bundle and passing it to "git fetch".

[1] http://www.phildev.net/pgp/gpgmua.html

Re: [Survey] Signed push

[Nguyen Thai Ngoc Duy]

On Thu, Sep 15, 2011 at 7:05 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>> On Wed, Sep 14, 2011 at 2:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>>> An alternative that I am considering is to let the requester say this
>>> instead:
>>>
>>>    are available in the git repository at:
>>>      git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f
> [...]
>> Stupid question, if we agree to go with signed push, can we also sign
>> pull requests and verify them when we pull? I suppose most of the
>> time, pulling can be done automatically by extracting pull url from
>> the request. This would make pull/push both signed.
>>
>> BTW, there's a third way (rsync is obsolete) to carry changes away in
>> human-unreadable way: bundles. Should we also sign the bundles too (I
>> guess we could just do the same as in signed push).
>
> If I understand you correctly, then ordinary PGP email signing[1]
> should work for that already.  In your first example, the receiver can
> make sure whatever process grabs a pull request verifies it, and in
> the second example, the receiver checks the signature on her email
> before saving a bundle and passing it to "git fetch".

Yes, I think we can do that already. It's just more convenient to
teach "git fetch/pull" to take pull requests and automatically verify
them. Some repositories may also want to enforce signing and we can do
that by setting config file and fetch/pull refuses if pull requests
are not signed. We can also store the sign as git notes, just like in
git-push (extra work if it has to be done manually).

> [1] http://www.phildev.net/pgp/gpgmua.html
-- 
Duy

Re: Fwd: [Survey] Signed push

[Philip Oakley]

From: "Johan Herland" <johan@herland.net>
.
> _that_ SHA1. As I said, that _can_ be done with the notes
> infrastructure, but - as Ted noted - there might be better solutions to
> storing branch descriptions.
>
Is one option to store the branch description (if any) on line two of the 
<branch name> file in .git\refs\heads.
That is, from byte 42 onward, after the 40 byte sha1 and its LF.
Older systems would simply overwrite it, while newer systems would be able 
to read it. The fixed format of the first 41 chars alllows sensible checks 
in the various places it is used.

Philip Oakley 

Re: Fwd: [Survey] Signed push

[Linus Torvalds]

On Wed, Sep 14, 2011 at 3:51 PM, Philip Oakley <philipoakley@iee.org> wrote:
>
> Is one option to store the branch description (if any) on line two of the
> <branch name> file in .git\refs\heads.

Or even on line one.

We already basically do that for the magic FETCH_HEAD branch, and use
it to populate the merge commit. Extending that kind of thing to all
branches might be a nice idea.

Of course, then the question becomes "what about packed refs"? Do you
just leave the unpacked ref in place for those?

                          Linus

Re: Fwd: [Survey] Signed push

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Sep 14, 2011 at 3:51 PM, Philip Oakley <philipoakley@iee.org> wrote:
>>
>> Is one option to store the branch description (if any) on line two of the
>> <branch name> file in .git\refs\heads.
>
> Or even on line one.
>
> We already basically do that for the magic FETCH_HEAD branch, and use
> it to populate the merge commit. Extending that kind of thing to all
> branches might be a nice idea.
>
> Of course, then the question becomes "what about packed refs"? Do you
> just leave the unpacked ref in place for those?

Seriously, storage format is not an issue at all and you know it.  The
semantics is.

What commands use the description and for what purpose, how it is updated
when the branch is repurposed, how it is propagated to other repositories,
if there is a situation where two descriptions need to be merged, and if
so how that merge happens, etc., etc.

We could store it in unused part of loose refs. We could add [branch
"master"] description = ...  in the .git/config. The latter would even be
easier for humans to edit by hand.

If we want to use the description when merging locally, for example,
fmt-merge-msg needs to be taught to read it, which would mean we would
need an internal API "read_branch_description()", regardless of what
storage format we choose to use. If we want to use it for "git pull", then
the transport layer needs to become aware of it.

Re: [Survey] Signed push

[Jeff King]

On Thu, Sep 15, 2011 at 08:42:40AM +1000, Nguyen Thai Ngoc Duy wrote:

> Yes, I think we can do that already. It's just more convenient to
> teach "git fetch/pull" to take pull requests and automatically verify
> them. Some repositories may also want to enforce signing and we can do
> that by setting config file and fetch/pull refuses if pull requests
> are not signed. We can also store the sign as git notes, just like in
> git-push (extra work if it has to be done manually).

Isn't there a human element in the verification? I.e., I see a pull
request, and we can computationally verify that it is signed by some
key. Now assuming GPG's web of trust works, that binds that key to an
email address and a real name. But how is that bound to the repository
you are actually fetching from (or more appropriately, that the commits
mentioned are appropriate to be pulled)?

That is a policy that the human must decide upon seeing "Oh, a pull
request from developer X; I should pull that into my local branch Y",
and which they do implicitly when they manually run the pull command
mentioned in the email.

Another way to think of it is that verifying the identity of the sender
(which GPG does) is only one step. You also need an ACL saying that the
sender is worth pulling from.

So either:

  1. The human is still in the loop, in which case having git-pull
     verify the sender's identity hasn't really done anything (because
     probably their MUA already told them it was really from the
     purported sender, and then they made the ACL decision in their head
     before deciding to pull from you).

  2. The human is not in the loop, and nothing is checking that ACL.

-Peff

[PATCH v3] request-pull: state what commit to expect

[Junio C Hamano]

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

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> I think that would probably be a good idea, although I'd actually
>> prefer you to be more verbose, and more human-friendly, and actually
>> talk about the commit in a readable way. Get rid of the *horrible*
>> BRANCH-NOT-VERIFIED message...
>>
>>  Top commit 1f51b001cccf: "Merge branches 'cns3xxx/fixes',
>>  'omap/fixes' and 'davinci/fixes' into fixes"
>>
>>  and at *that* point you might have a "UNVERIFIED" notice for people
>> to check if they forgot to push.
>
> That UNVERIFIED thing was neither my favorite nor my idea, and I'd happily
> rip it out in any second ;-)

So this is the third round.

-- >8 --
The message gives a detailed explanation of the commit the requester based
the changes on, but lacks information that is necessary for the person who
performs a fetch & merge in order to verify that the correct branch was
fetched when responding to the pull request.

Add a few more lines to describe the commit at the tip expected to be
fetched to the same level of detail as the base commit.

Also update the warning message slightly when the script notices that the
commit may not have been pushed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

A UI wart that we cannot fix without breaking backward compatibility is
that the "end" parameter (which defaults to HEAD and is assigned to $head
variable in the script) the requestor uses from the command line names a
commit (often the name of a local branch), but for the purpose of telling
which ref to pull from the public repository, that is a _wrong_ thing to
give to the recipient.

Because the act of generating a request-pull message and the act of
pushing to the public repository are not linked in any way, the script
does not know _how_ the requestor caused (or intends to cause) the commit
to sit at the tip of which branch. There is no guarantee that a lazy "git
push" that relies on the configured refspec will be (or have been) used,
so even parsing the output from "git push -n --porcelain -v $there" would
not tell the script which branch the commit to be pulled is to be pushed
out to, or if the branch is consistent with the request message.

The use of "git ls-remote" in the script and picking one of the refs that
matches the commit object at random from its output is unsatisfactory, but
that is unfortunately the best this script could do without correcting the
design mistake and redefining what the "end" parameter means.

If we can break the backward compatibility and redefine that the "end"
parameter now means the name of the branch at the public repository, it
would make the operation a lot more robust.  We could then:

 - $branch is what is given by the end user (it is an error not to give
   the "end" parameter);
 - run "git ls-remote $url $head" to find $headrev;
 - generate the message and shortlog using the information obtained from
   $url; and
 - get rid of "did you forget to push" message.

We could allow adding yet another argument which names a commit object
locally, and make sure if the $headrev observed by ls-remote does not
match it.

---
 git-request-pull.sh     |   34 +++++++++++++++++++---------------
 t/t5150-request-pull.sh |    6 ++++++
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index afb75e8..438e7eb 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,7 +35,7 @@ do
 	shift
 done
 
-base=$1 url=$2 head=${3-HEAD}
+base=$1 url=$2 head=${3-HEAD} status=0
 
 test -n "$base" && test -n "$url" || usage
 baserev=$(git rev-parse --verify "$base"^0) &&
@@ -51,25 +51,29 @@ find_matching_branch="/^$headrev	"'refs\/heads\//{
 }'
 branch=$(git ls-remote "$url" | sed -n -e "$find_matching_branch")
 url=$(git ls-remote --get-url "$url")
-if test -z "$branch"
-then
-	echo "warn: No branch of $url is at:" >&2
-	git log --max-count=1 --pretty='tformat:warn:   %h: %s' $headrev >&2
-	echo "warn: Are you sure you pushed $head there?" >&2
-	echo >&2
-	echo >&2
-	branch=..BRANCH.NOT.VERIFIED..
-	status=1
-fi
 
 git show -s --format='The following changes since commit %H:
 
   %s (%ci)
 
-are available in the git repository at:' $baserev &&
-echo "  $url $branch" &&
-echo &&
+are available in the git repository at:
+' $baserev &&
+echo "  $url${branch+ $branch}" &&
+git show -s --format='
+for you to fetch changes up to %H:
+
+  %s (%ci)
+
+----------------------------------------------------------------' $headrev &&
 
 git shortlog ^$baserev $headrev &&
-git diff -M --stat --summary $patch $merge_base..$headrev || exit
+git diff -M --stat --summary $patch $merge_base..$headrev || status=1
+
+if test -z "$branch"
+then
+	echo "warn: No branch of $url is at:" >&2
+	git show -s --format='warn:   %h: %s' $headrev >&2
+	echo "warn: Are you sure you pushed '$head' there?" >&2
+	status=1
+fi
 exit $status
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9cc0a42..5bd1682 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -193,8 +193,14 @@ test_expect_success 'pull request format' '
 	  SUBJECT (DATE)
 
 	are available in the git repository at:
+
 	  URL BRANCH
 
+	for you to fetch changes up to OBJECT_NAME:
+
+	  SUBJECT (DATE)
+
+	----------------------------------------------------------------
 	SHORTLOG
 
 	DIFFSTAT
-- 
1.7.7.rc1.3.g559357

Re: [PATCH v3] request-pull: state what commit to expect

[Junio C Hamano]

Here are three patches on top of the latest round of request-pull update.
The first two are to add a new option to "git branch" command so that a
descriptive text that explains what the purpose of the branch is, for use
by other commands. And the last one builds on top of the patch this
message is a response to, to use that information.

And here is the first one, just an unrelated doc clean-up.

-- >8 --
Subject: [PATCH 1/3] branch doc: minor formatting fix

We tend to use typewriter font when writing command flags that are
meant to be typed literally by the user.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-branch.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 507b8d0..79424a5 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -54,7 +54,7 @@ With a `-d` or `-D` option, `<branchname>` will be deleted.  You may
 specify more than one branch for deletion.  If the branch currently
 has a reflog then the reflog will also be deleted.
 
-Use -r together with -d to delete remote-tracking branches. Note, that it
+Use `-r` together with `-d` to delete remote-tracking branches. Note, that it
 only makes sense to delete remote-tracking branches if they no longer exist
 in the remote repository or if 'git fetch' was configured not to fetch
 them again. See also the 'prune' subcommand of linkgit:git-remote[1] for a
-- 
1.7.7.rc2.4.g5ec82

[PATCH 2/3] branch: teach --edit-description option

[Junio C Hamano]

Using branch.$name.description as the configuration key, give users a
place to write about what the purpose of the branch is and things like
that, so that various subsystems, e.g. "push -s", "request-pull", and
"format-patch --cover-letter", can later be taught to use this
information.

The "-m" option similar to "commit/tag" is deliberately omitted, as the
whole point of branch description is about giving descriptive information
(the name of the branch itself is a better place for information that fits
on a single-line).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-branch.txt |    5 +++
 builtin/branch.c             |   68 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 79424a5..12bdffc 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
+'git branch' --edit-description [<branchname>]
 
 DESCRIPTION
 -----------
@@ -144,6 +145,10 @@ start-point is either a local or remote-tracking branch.
 	like '--track' would when creating the branch, except that where
 	branch points to is not changed.
 
+--edit-description::
+	Open an editor and edit the text to explain what the branch is
+	for, to be used by various other commands (e.g. `request-pull`).
+
 --contains <commit>::
 	Only list branches which contain the specified commit.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index f49596f..94319c4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -606,11 +606,61 @@ static int opt_parse_merge_filter(const struct option *opt, const char *arg, int
 	return 0;
 }
 
+struct branch_desc_cb {
+	const char *config_name;
+	const char *value;
+};
+
+static int read_branch_desc(const char *var, const char *value, void *cb)
+{
+	struct branch_desc_cb *desc = cb;
+	if (strcmp(desc->config_name, var))
+		return 0;
+	free((char *)desc->value);
+	return git_config_string(&desc->value, var, value);
+}
+
+static const char edit_description[] = "BRANCH_DESCRIPTION";
+
+static int edit_branch_description(const char *branch_name)
+{
+	struct branch_desc_cb cb;
+	struct strbuf name = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	FILE *fp;
+
+	strbuf_addf(&name, "branch.%s.description", branch_name);
+	cb.config_name = name.buf;
+	cb.value = NULL;
+	git_config(read_branch_desc, &cb);
+
+	if (cb.value)
+		strbuf_addstr(&buf, cb.value);
+	if (!buf.len || buf.buf[buf.len-1] != '\n')
+		strbuf_addch(&buf, '\n');
+	strbuf_addf(&buf,
+		    "# Please edit the description for the branch\n"
+		    "#   %s\n"
+		    "# Lines starting with '#' will be stripped.\n",
+		    branch_name);
+	fp = fopen(git_path(edit_description), "w");
+	if (fwrite(buf.buf, 1, buf.len, fp) < buf.len) {
+		strbuf_release(&buf);
+		return error(_("could not write branch description template: %s\n"),
+			     strerror(errno));
+	}
+	strbuf_reset(&buf);
+	if (launch_editor(git_path(edit_description), &buf, NULL))
+		return -1;
+	stripspace(&buf, 1);
+	return git_config_set(name.buf, buf.buf);
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = -1, detached = 0;
-	int reflog = 0;
+	int reflog = 0, edit_description = 0;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
@@ -648,6 +698,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('m', NULL, &rename, "move/rename a branch and its reflog", 1),
 		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
 		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
+		OPT_BOOLEAN(0, "edit-description", &edit_description,
+			    "edit the description for the branch"),
 		OPT__FORCE(&force_create, "force creation (when already exists)"),
 		{
 			OPTION_CALLBACK, 0, "no-merged", &merge_filter_ref,
@@ -694,7 +746,19 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-	else if (argc == 0)
+	else if (edit_description) {
+		const char *branch_name;
+		if (detached)
+			die("Cannot give description to detached HEAD");
+		if (!argc)
+			branch_name = head;
+		else if (argc == 1)
+			branch_name = argv[0];
+		else
+			usage_with_options(builtin_branch_usage, options);
+		if (edit_branch_description(branch_name))
+			return 1;
+	} else if (argc == 0)
 		return print_ref_list(kinds, detached, verbose, abbrev, with_commit);
 	else if (rename && (argc == 1))
 		rename_branch(head, argv[0], rename > 1);
-- 
1.7.7.rc2.4.g5ec82

[PATCH] request-pull: use the branch description

[Junio C Hamano]

Now we have branch descriptions stored in the repository, we can
use it when preparing the request-pull message.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-request-pull.sh |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 438e7eb..626cf25 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,7 +35,18 @@ do
 	shift
 done
 
-base=$1 url=$2 head=${3-HEAD} status=0
+base=$1 url=$2 head=${3-HEAD} status=0 branch_name=
+
+headref=$(git symbolic-ref -q "$head")
+if git show-ref -q --verify "$headref"
+then
+	branch_name=${headref#refs/heads/}
+	if test "z$branch_name" = "z$headref" ||
+		! git config "branch.$branch_name.description" >/dev/null
+	then
+		branch_name=
+	fi
+fi
 
 test -n "$base" && test -n "$url" || usage
 baserev=$(git rev-parse --verify "$base"^0) &&
@@ -66,6 +77,13 @@ for you to fetch changes up to %H:
 
 ----------------------------------------------------------------' $headrev &&
 
+if test -n "$branch_name"
+then
+	echo "(from the branch description for $branch local branch)"
+	echo
+	git config "branch.$branch_name.description"
+	echo "----------------------------------------------------------------"
+fi &&
 git shortlog ^$baserev $headrev &&
 git diff -M --stat --summary $patch $merge_base..$headrev || status=1
 
-- 
1.7.7.rc2.4.g5ec82

Re: [PATCH 2/3] branch: teach --edit-description option

[Andrew Ardill]

On 21 September 2011 09:02, Junio C Hamano <gitster@pobox.com> wrote:
> Using branch.$name.description as the configuration key, give users a
> place to write about what the purpose of the branch is and things like
> that, so that various subsystems, e.g. "push -s", "request-pull", and
> "format-patch --cover-letter", can later be taught to use this
> information.
>
> The "-m" option similar to "commit/tag" is deliberately omitted, as the
> whole point of branch description is about giving descriptive information
> (the name of the branch itself is a better place for information that fits
> on a single-line).

I understand your reasoning here, however is there a way to allow
setting the branch description in, for example, a script?

Additionally I can imagine it would be useful to be able to set the
branch description from another tool, what is the recommended way of
doing that? Should tools modify the config directly??

Regards,
Andrew

Re: [PATCH 2/3] branch: teach --edit-description option

[Junio C Hamano]

Andrew Ardill <andrew.ardill@gmail.com> writes:

> I understand your reasoning here, however is there a way to allow
> setting the branch description in, for example, a script?

As you read in the patch, especially the documentation part, there is not
a way to do that.

I am not interested because it does not directly help my cause of helping
the human communication between the kernel developers who may want to
perform a signed push to their public repository and send their pull
requests with the same message to Linus.

That does _not_ mean I will _reject_ a patch to add such a feature. It
just means writing such a patch myself or reviewing and accepting such a
patch is not very high in my prioritized list at this point in the
evolution of the series. Teaching the use of the information to other
commands such as "format-patch --cover-letter" would have much higher
precedence.

> Additionally I can imagine it would be useful to be able to set the
> branch description from another tool, what is the recommended way of
> doing that? Should tools modify the config directly??

An obvious answer: "do whatever you want". The only rule that the programs
that need to follow is that branch.$name.description has the string to use
to obtain the explanation text.

How they achieve that (perhaps by running "git config") is of secondary
importance.

[PATCH 0/6] A handful of "branch description" patches

[Junio C Hamano]

Here are a few patches that I have queued in 'pu', redoing some of the
patches I already sent out to the list, around "branch description".

The original motivation was to make the push/pull workflow appear more
robust by allowing human-to-human communication to leave audit trail that
can be verified when it becomes necessary. Namely:

 * request-pull message carries the SHA-1 of what is expected to be
   merged; and

 * "signed push" leaves the SHA-1 of what was pushed to the remote,
   cryptographically signed.

Linus's reaction, as I understood him, was "if we are spending efforts to
add more information, the end result should be more informative to humans
not just to machines", and I agree.  An example of piece of information we
often talk about is branch description---what a particular branch is meant
to achieve. Both request-pull messages and declarations of what was pushed
are good places to record that piece of information.

So here is a partially re-rolled series to get us closer.

 * The logic to read from an existing branch description was in
   builtin/branch.c in the original series, but the first patch separates
   it out into branch.c as a helper function;

 * The second one is a digression; the branch description describes what
   the topic aims to achieve, so it was natural to use it to prime the
   cover letter while preparing a patch series with format-patch;

 * The third one that adds "branch --edit-description" is basically
   unchanged modulo small leakfix from the original round;

 * And the remainder of the series for request-pull is the same as the
   last round.

The second patch uses facility introduced in bk/ancestry-path topic, so
it would be the easiest to apply the series on top of a merge of c05b988
to 'master'.

I haven't updated the "signed push" patch to use this information yet.


Junio C Hamano (6):
  branch: add read_branch_desc() helper function
  format-patch: use branch description in cover letter
  branch: teach --edit-description option
  request-pull: modernize style
  request-pull: state what commit to expect
  request-pull: use the branch description

 Documentation/git-branch.txt |    5 +++
 branch.c                     |   31 ++++++++++++++++++
 branch.h                     |    5 +++
 builtin/branch.c             |   56 +++++++++++++++++++++++++++++++-
 builtin/log.c                |   71 +++++++++++++++++++++++++++++++++++++++--
 git-request-pull.sh          |   73 ++++++++++++++++++++++++++---------------
 t/t5150-request-pull.sh      |    6 +++
 7 files changed, 215 insertions(+), 32 deletions(-)

-- 
1.7.7.rc2.4.g5ec82

[PATCH 1/6] branch: add read_branch_desc() helper function

[Junio C Hamano]

This will be used by various callers that make use of the branch
description throughout the system, so that if we need to update
the implementation the callers do not have to be modified.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 branch.c |   31 +++++++++++++++++++++++++++++++
 branch.h |    5 +++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/branch.c b/branch.c
index fecedd3..88da275 100644
--- a/branch.c
+++ b/branch.c
@@ -135,6 +135,37 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 	return 0;
 }
 
+struct branch_desc_cb {
+	const char *config_name;
+	const char *value;
+};
+
+static int read_branch_desc_cb(const char *var, const char *value, void *cb)
+{
+	struct branch_desc_cb *desc = cb;
+	if (strcmp(desc->config_name, var))
+		return 0;
+	free((char *)desc->value);
+	return git_config_string(&desc->value, var, value);
+}
+
+int read_branch_desc(struct strbuf *buf, const char *branch_name)
+{
+	struct branch_desc_cb cb;
+	struct strbuf name = STRBUF_INIT;
+	strbuf_addf(&name, "branch.%s.description", branch_name);
+	cb.config_name = name.buf;
+	cb.value = NULL;
+	if (git_config(read_branch_desc_cb, &cb)) {
+		strbuf_release(&name);
+		return -1;
+	}
+	if (cb.value)
+		strbuf_addstr(buf, cb.value);
+	strbuf_release(&name);
+	return 0;
+}
+
 int validate_new_branchname(const char *name, struct strbuf *ref,
 			    int force, int attr_only)
 {
diff --git a/branch.h b/branch.h
index 1285158..1493f73 100644
--- a/branch.h
+++ b/branch.h
@@ -46,4 +46,9 @@ void remove_branch_state(void);
 #define BRANCH_CONFIG_VERBOSE 01
 extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
 
+/*
+ * Read branch description
+ */
+extern int read_branch_desc(struct strbuf *, const char *branch_name);
+
 #endif
-- 
1.7.7.rc2.4.g5ec82

[PATCH 2/6] format-patch: use branch description in cover letter

[Junio C Hamano]

Use the description for the branch when preparing the cover letter
when available.

While at it, mark a loosely written codepath that would do a random and
useless thing given an unusual input (e.g. "^master HEAD HEAD^"), which
we may want to fix someday.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 branch.c      |    2 +-
 builtin/log.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/branch.c b/branch.c
index 88da275..50088a4 100644
--- a/branch.c
+++ b/branch.c
@@ -156,7 +156,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
 	strbuf_addf(&name, "branch.%s.description", branch_name);
 	cb.config_name = name.buf;
 	cb.value = NULL;
-	if (git_config(read_branch_desc_cb, &cb)) {
+	if (git_config(read_branch_desc_cb, &cb) < 0) {
 		strbuf_release(&name);
 		return -1;
 	}
diff --git a/builtin/log.c b/builtin/log.c
index f5d4930..e80a925 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -19,6 +19,7 @@
 #include "remote.h"
 #include "string-list.h"
 #include "parse-options.h"
+#include "branch.h"
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;
@@ -746,10 +747,24 @@ static void print_signature(void)
 		printf("-- \n%s\n\n", signature);
 }
 
+static void add_branch_description(struct strbuf *buf, const char *branch_name)
+{
+	struct strbuf desc = STRBUF_INIT;
+	if (!branch_name || !*branch_name)
+		return;
+	read_branch_desc(&desc, branch_name);
+	if (desc.len) {
+		strbuf_addch(buf, '\n');
+		strbuf_add(buf, desc.buf, desc.len);
+		strbuf_addch(buf, '\n');
+	}
+}
+
 static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      int numbered, int numbered_files,
 			      struct commit *origin,
 			      int nr, struct commit **list, struct commit *head,
+			      const char *branch_name,
 			      int quiet)
 {
 	const char *committer;
@@ -807,6 +822,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	pp_user_info(&pp, NULL, &sb, committer, encoding);
 	pp_title_line(&pp, &msg, &sb, encoding, need_8bit_cte);
 	pp_remainder(&pp, &msg, &sb, 0);
+	add_branch_description(&sb, branch_name);
 	printf("%s\n", sb.buf);
 
 	strbuf_release(&sb);
@@ -1006,6 +1022,35 @@ static int cc_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static char *find_branch_name(struct rev_info *rev)
+{
+	int i, positive = -1;
+	unsigned char branch_sha1[20];
+	struct strbuf buf = STRBUF_INIT;
+	const char *branch;
+
+	for (i = 0; i < rev->cmdline.nr; i++) {
+		if (rev->cmdline.rev[i].flags & UNINTERESTING)
+			continue;
+		if (positive < 0)
+			positive = i;
+		else
+			return NULL;
+	}
+	if (positive < 0)
+		return NULL;
+	strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
+	branch = resolve_ref(buf.buf, branch_sha1, 1, 0);
+	if (!branch ||
+	    prefixcmp(branch, "refs/heads/") ||
+	    hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))
+		branch = NULL;
+	strbuf_release(&buf);
+	if (branch)
+		return xstrdup(rev->cmdline.rev[positive].name);
+	return NULL;
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -1027,6 +1072,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
 	int quiet = 0;
+	char *branch_name = NULL;
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
 			    "use [PATCH n/m] even with a single patch",
@@ -1217,8 +1263,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			 * origin" that prepares what the origin side still
 			 * does not have.
 			 */
+			unsigned char sha1[20];
+			const char *ref;
+
 			rev.pending.objects[0].item->flags |= UNINTERESTING;
 			add_head_to_pending(&rev);
+			ref = resolve_ref("HEAD", sha1, 1, NULL);
+			if (ref && !prefixcmp(ref, "refs/heads/"))
+				branch_name = xstrdup(ref + strlen("refs/heads/"));
+			else
+				branch_name = xstrdup(""); /* no branch */
 		}
 		/*
 		 * Otherwise, it is "format-patch -22 HEAD", and/or
@@ -1234,16 +1288,26 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.show_root_diff = 1;
 
 	if (cover_letter) {
-		/* remember the range */
+		/*
+		 * NEEDSWORK:randomly pick one positive commit to show
+		 * diffstat; this is often the tip and the command
+		 * happens to do the right thing in most cases, but a
+		 * complex command like "--cover-letter a b c ^bottom"
+		 * picks "c" and shows diffstat between bottom..c
+		 * which may not match what the series represents at
+		 * all and totally broken.
+		 */
 		int i;
 		for (i = 0; i < rev.pending.nr; i++) {
 			struct object *o = rev.pending.objects[i].item;
 			if (!(o->flags & UNINTERESTING))
 				head = (struct commit *)o;
 		}
-		/* We can't generate a cover letter without any patches */
+		/* There is nothing to show; it is not an error, though. */
 		if (!head)
 			return 0;
+		if (!branch_name)
+			branch_name = find_branch_name(&rev);
 	}
 
 	if (ignore_if_in_upstream) {
@@ -1294,7 +1358,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, use_stdout, numbered, numbered_files,
-				  origin, nr, list, head, quiet);
+				  origin, nr, list, head, branch_name, quiet);
 		total++;
 		start_number--;
 	}
@@ -1366,6 +1430,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			fclose(stdout);
 	}
 	free(list);
+	free(branch_name);
 	string_list_clear(&extra_to, 0);
 	string_list_clear(&extra_cc, 0);
 	string_list_clear(&extra_hdr, 0);
-- 
1.7.7.rc2.4.g5ec82

[PATCH 3/6] branch: teach --edit-description option

[Junio C Hamano]

Using branch.$name.description as the configuration key, give users a
place to write about what the purpose of the branch is and things like
that, so that various subsystems, e.g. "push -s", "request-pull", and
"format-patch --cover-letter", can later be taught to use this
information.

The "-m" option similar to "commit/tag" is deliberately omitted, as the
whole point of branch description is about giving descriptive information
(the name of the branch itself is a better place for information that fits
on a single-line).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-branch.txt |    5 +++
 builtin/branch.c             |   56 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 507b8d0..8871a4e 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
+'git branch' --edit-description [<branchname>]
 
 DESCRIPTION
 -----------
@@ -144,6 +145,10 @@ start-point is either a local or remote-tracking branch.
 	like '--track' would when creating the branch, except that where
 	branch points to is not changed.
 
+--edit-description::
+	Open an editor and edit the text to explain what the branch is
+	for, to be used by various other commands (e.g. `request-pull`).
+
 --contains <commit>::
 	Only list branches which contain the specified commit.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index f49596f..fffa319 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -606,11 +606,49 @@ static int opt_parse_merge_filter(const struct option *opt, const char *arg, int
 	return 0;
 }
 
+static const char edit_description[] = "BRANCH_DESCRIPTION";
+
+static int edit_branch_description(const char *branch_name)
+{
+	FILE *fp;
+	int status;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf name = STRBUF_INIT;
+
+	read_branch_desc(&buf, branch_name);
+	if (!buf.len || buf.buf[buf.len-1] != '\n')
+		strbuf_addch(&buf, '\n');
+	strbuf_addf(&buf,
+		    "# Please edit the description for the branch\n"
+		    "#   %s\n"
+		    "# Lines starting with '#' will be stripped.\n",
+		    branch_name);
+	fp = fopen(git_path(edit_description), "w");
+	if ((fwrite(buf.buf, 1, buf.len, fp) < buf.len) || fclose(fp)) {
+		strbuf_release(&buf);
+		return error(_("could not write branch description template: %s\n"),
+			     strerror(errno));
+	}
+	strbuf_reset(&buf);
+	if (launch_editor(git_path(edit_description), &buf, NULL)) {
+		strbuf_release(&buf);
+		return -1;
+	}
+	stripspace(&buf, 1);
+
+	strbuf_addf(&name, "branch.%s.description", branch_name);
+	status = git_config_set(name.buf, buf.buf);
+	strbuf_release(&name);
+	strbuf_release(&buf);
+
+	return status;
+}
+
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = -1, detached = 0;
-	int reflog = 0;
+	int reflog = 0, edit_description = 0;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
@@ -648,6 +686,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('m', NULL, &rename, "move/rename a branch and its reflog", 1),
 		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
 		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
+		OPT_BOOLEAN(0, "edit-description", &edit_description,
+			    "edit the description for the branch"),
 		OPT__FORCE(&force_create, "force creation (when already exists)"),
 		{
 			OPTION_CALLBACK, 0, "no-merged", &merge_filter_ref,
@@ -694,7 +734,19 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-	else if (argc == 0)
+	else if (edit_description) {
+		const char *branch_name;
+		if (detached)
+			die("Cannot give description to detached HEAD");
+		if (!argc)
+			branch_name = head;
+		else if (argc == 1)
+			branch_name = argv[0];
+		else
+			usage_with_options(builtin_branch_usage, options);
+		if (edit_branch_description(branch_name))
+			return 1;
+	} else if (argc == 0)
 		return print_ref_list(kinds, detached, verbose, abbrev, with_commit);
 	else if (rename && (argc == 1))
 		rename_branch(head, argv[0], rename > 1);
-- 
1.7.7.rc2.4.g5ec82

[PATCH 4/6] request-pull: modernize style

[Junio C Hamano]

Make it a bit more conforming to Documentation/Codingstyle

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-request-pull.sh |   29 +++++++++++++----------------
 1 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index fc080cc..afb75e8 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,27 +35,24 @@ do
 	shift
 done
 
-base=$1
-url=$2
-head=${3-HEAD}
+base=$1 url=$2 head=${3-HEAD}
 
-[ "$base" ] || usage
-[ "$url" ] || usage
+test -n "$base" && test -n "$url" || usage
+baserev=$(git rev-parse --verify "$base"^0) &&
+headrev=$(git rev-parse --verify "$head"^0) || exit
 
-baserev=`git rev-parse --verify "$base"^0` &&
-headrev=`git rev-parse --verify "$head"^0` || exit
-
-merge_base=`git merge-base $baserev $headrev` ||
+merge_base=$(git merge-base $baserev $headrev) ||
 die "fatal: No commits in common between $base and $head"
 
-branch=$(git ls-remote "$url" \
-	| sed -n -e "/^$headrev	refs.heads./{
-		s/^.*	refs.heads.//
-		p
-		q
-	}")
+find_matching_branch="/^$headrev	"'refs\/heads\//{
+	s/^.*	refs\/heads\///
+	p
+	q
+}'
+branch=$(git ls-remote "$url" | sed -n -e "$find_matching_branch")
 url=$(git ls-remote --get-url "$url")
-if [ -z "$branch" ]; then
+if test -z "$branch"
+then
 	echo "warn: No branch of $url is at:" >&2
 	git log --max-count=1 --pretty='tformat:warn:   %h: %s' $headrev >&2
 	echo "warn: Are you sure you pushed $head there?" >&2
-- 
1.7.7.rc2.4.g5ec82

[PATCH 5/6] request-pull: state what commit to expect

[Junio C Hamano]

The message gives a detailed explanation of the commit the requester based
the changes on, but lacks information that is necessary for the person who
performs a fetch & merge in order to verify that the correct branch was
fetched when responding to the pull request.

Add a few more lines to describe the commit at the tip expected to be
fetched to the same level of detail as the base commit.

Also update the warning message slightly when the script notices that the
commit may not have been pushed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-request-pull.sh     |   34 +++++++++++++++++++---------------
 t/t5150-request-pull.sh |    6 ++++++
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index afb75e8..438e7eb 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,7 +35,7 @@ do
 	shift
 done
 
-base=$1 url=$2 head=${3-HEAD}
+base=$1 url=$2 head=${3-HEAD} status=0
 
 test -n "$base" && test -n "$url" || usage
 baserev=$(git rev-parse --verify "$base"^0) &&
@@ -51,25 +51,29 @@ find_matching_branch="/^$headrev	"'refs\/heads\//{
 }'
 branch=$(git ls-remote "$url" | sed -n -e "$find_matching_branch")
 url=$(git ls-remote --get-url "$url")
-if test -z "$branch"
-then
-	echo "warn: No branch of $url is at:" >&2
-	git log --max-count=1 --pretty='tformat:warn:   %h: %s' $headrev >&2
-	echo "warn: Are you sure you pushed $head there?" >&2
-	echo >&2
-	echo >&2
-	branch=..BRANCH.NOT.VERIFIED..
-	status=1
-fi
 
 git show -s --format='The following changes since commit %H:
 
   %s (%ci)
 
-are available in the git repository at:' $baserev &&
-echo "  $url $branch" &&
-echo &&
+are available in the git repository at:
+' $baserev &&
+echo "  $url${branch+ $branch}" &&
+git show -s --format='
+for you to fetch changes up to %H:
+
+  %s (%ci)
+
+----------------------------------------------------------------' $headrev &&
 
 git shortlog ^$baserev $headrev &&
-git diff -M --stat --summary $patch $merge_base..$headrev || exit
+git diff -M --stat --summary $patch $merge_base..$headrev || status=1
+
+if test -z "$branch"
+then
+	echo "warn: No branch of $url is at:" >&2
+	git show -s --format='warn:   %h: %s' $headrev >&2
+	echo "warn: Are you sure you pushed '$head' there?" >&2
+	status=1
+fi
 exit $status
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9cc0a42..5bd1682 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -193,8 +193,14 @@ test_expect_success 'pull request format' '
 	  SUBJECT (DATE)
 
 	are available in the git repository at:
+
 	  URL BRANCH
 
+	for you to fetch changes up to OBJECT_NAME:
+
+	  SUBJECT (DATE)
+
+	----------------------------------------------------------------
 	SHORTLOG
 
 	DIFFSTAT
-- 
1.7.7.rc2.4.g5ec82

[PATCH 6/6] request-pull: use the branch description

[Junio C Hamano]

Now we have branch descriptions stored in the repository, we can
use it when preparing the request-pull message.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-request-pull.sh |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 438e7eb..626cf25 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -35,7 +35,18 @@ do
 	shift
 done
 
-base=$1 url=$2 head=${3-HEAD} status=0
+base=$1 url=$2 head=${3-HEAD} status=0 branch_name=
+
+headref=$(git symbolic-ref -q "$head")
+if git show-ref -q --verify "$headref"
+then
+	branch_name=${headref#refs/heads/}
+	if test "z$branch_name" = "z$headref" ||
+		! git config "branch.$branch_name.description" >/dev/null
+	then
+		branch_name=
+	fi
+fi
 
 test -n "$base" && test -n "$url" || usage
 baserev=$(git rev-parse --verify "$base"^0) &&
@@ -66,6 +77,13 @@ for you to fetch changes up to %H:
 
 ----------------------------------------------------------------' $headrev &&
 
+if test -n "$branch_name"
+then
+	echo "(from the branch description for $branch local branch)"
+	echo
+	git config "branch.$branch_name.description"
+	echo "----------------------------------------------------------------"
+fi &&
 git shortlog ^$baserev $headrev &&
 git diff -M --stat --summary $patch $merge_base..$headrev || status=1
 
-- 
1.7.7.rc2.4.g5ec82

Re: [PATCH 0/6] A handful of "branch description" patches

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 23.09.2011 00:09:
> Here are a few patches that I have queued in 'pu', redoing some of the
> patches I already sent out to the list, around "branch description".
> 
> The original motivation was to make the push/pull workflow appear more
> robust by allowing human-to-human communication to leave audit trail that
> can be verified when it becomes necessary. Namely:
> 
>  * request-pull message carries the SHA-1 of what is expected to be
>    merged; and
> 
>  * "signed push" leaves the SHA-1 of what was pushed to the remote,
>    cryptographically signed.
> 
> Linus's reaction, as I understood him, was "if we are spending efforts to
> add more information, the end result should be more informative to humans
> not just to machines", and I agree.  An example of piece of information we
> often talk about is branch description---what a particular branch is meant
> to achieve. Both request-pull messages and declarations of what was pushed
> are good places to record that piece of information.
> 
> So here is a partially re-rolled series to get us closer.
> 
>  * The logic to read from an existing branch description was in
>    builtin/branch.c in the original series, but the first patch separates
>    it out into branch.c as a helper function;
> 
>  * The second one is a digression; the branch description describes what
>    the topic aims to achieve, so it was natural to use it to prime the
>    cover letter while preparing a patch series with format-patch;
> 
>  * The third one that adds "branch --edit-description" is basically
>    unchanged modulo small leakfix from the original round;
> 
>  * And the remainder of the series for request-pull is the same as the
>    last round.

I'm afraid I've missed the first installment of the series, or rather the fact that it was about more than just signed pushes. I've been working at (and with) branch and tag annotations for quite a while now and should have probably pushed the WIP rather than just dropping the occasional note. So I'll describe briefly what I have (the branches are in any of my repos[1]), which is notes based:

  mjg/vob/branch-notes [mjg/vob/virtual-objects: ahead 4]
    Annotations for branches and tags
    
    Show notes for branches and tags when "branch" resp. "tag" is called with "--notes".
    The "--notes" argument can take on all usual forms.

  mjg/vob/format-patch-branch-note [mjg/vob/refrev-hash: ahead 1]
    Cover letter from notes
    
    Fill in the cover letter from a note to ref:HEAD if --notes is given.
    TODO: The current branch may not be the one the format-patch arguments refer to.

  mjg/vob/refrev-hash [mjg/vob/virtual-objects: ahead 2]
    Pseudo revs for refnames
    
    Introduce "ref:foo" to denote the (virtual) refname object for the ref named
    "foo". This is handy for now (editing branch and tag notes) but should
    become obsoleted by a better ui, such as "git branch --edit foo" or
    "git notes --refname edit foo".
    
    Introduce "ref:" as a shortcut to "ref:HEAD" which is the refname object
    for the current branch.

  mjg/vob/refrev-pretend [mjg/vob/virtual-objects: ahead 1]
    Pseudo revs for refnames
    
    An alternative implementation using pretend_sha1...
    Currently unused.

  mjg/vob/virtual-objects [origin/next: ahead 2, behind 10]
    Virtual refname objects
    
    For each existing refname, introduce virtual objects corresponding to a blob
    with the refname as the content. "virtual" refers to the fact that these
    objects are not written out but exist for all other purposes, such as
    attaching notes and keeping them from being pruned.

  mjg/vob/virtual-refs-for-rnos
    Virtual refs for refname objects
    
    For each ref, pretend that the corresponding refname object is referenced
    to keep it from being pruned. This still requires branch note code to
    write out these objects.
    (Unused earlier approach.)

  mjg/vob/virtual-refs-pretend-all
    Virtual refs for refname objects
    
    For each ref, pretend that the corresponding refname object is referenced
    to keep it from being pruned. This still requires branch note code to
    write out these objects.
    (Unused earlier approach using pretend_sha1....)


Yes, the above is (with added newlines and removed top commit info) the output of 'git branch -vv --notes --list mjg/vob\*' :)

Open questions:
* Should the refname object for ref "foo" really be identical to a blob with content "foo"? Or content "ref: foo? Or...?
* Should ref (branch and tag) annotations use the same default notes tree as commit notes?
* How best to view annotations on remote branches? This is connected with open questions about notes sharing and the ref namespace structure.

I do think that config based descriptions are a quick solution, but a very non-distributed, non-versioned approach when compared to the notes approach.

Michael

[1]
git://github.com/gitigit/git.git
git://gitorious.org/~mjg/git/mjg.git
git://repo.or.cz/git/mjg.git

Re: [PATCH 3/6] branch: teach --edit-description option

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 23.09.2011 00:09:
> Using branch.$name.description as the configuration key, give users a
> place to write about what the purpose of the branch is and things like
> that, so that various subsystems, e.g. "push -s", "request-pull", and
> "format-patch --cover-letter", can later be taught to use this
> information.
> 
> The "-m" option similar to "commit/tag" is deliberately omitted, as the
> whole point of branch description is about giving descriptive information
> (the name of the branch itself is a better place for information that fits
> on a single-line).

I don't think that is the only reason why we should not make "git branch
-m foo bar" set the description "foo" for branch "bar"...

Granted, your argument applies to "--set-description" ("-m"-like) also.

Michael

> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-branch.txt |    5 +++
>  builtin/branch.c             |   56 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 507b8d0..8871a4e 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -14,6 +14,7 @@ SYNOPSIS
>  'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
>  'git branch' (-m | -M) [<oldbranch>] <newbranch>

;)

>  'git branch' (-d | -D) [-r] <branchname>...
> +'git branch' --edit-description [<branchname>]

Re: [PATCH 3/6] branch: teach --edit-description option

[Nguyen Thai Ngoc Duy]

On Thu, Sep 22, 2011 at 03:09:19PM -0700, Junio C Hamano wrote:
> +	if (launch_editor(git_path(edit_description), &buf, NULL)) {
> +		strbuf_release(&buf);
> +		return -1;
> +	}
> +	stripspace(&buf, 1);
> +
> +	strbuf_addf(&name, "branch.%s.description", branch_name);
> +	status = git_config_set(name.buf, buf.buf);

I suppose a Windows editor mave save the description with \r\n
ending. Perhaps a patch like this to avoid messing up config file?

--8<--
Subject: [PATCH] config: quote \r in value

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 config.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/config.c b/config.c
index 4183f80..2e238ac 100644
--- a/config.c
+++ b/config.c
@@ -165,6 +165,9 @@ static char *parse_value(void)
 			case 'b':
 				c = '\b';
 				break;
+			case 'r':
+				c = '\r';
+				break;
 			case 'n':
 				c = '\n';
 				break;
@@ -1048,6 +1051,9 @@ static int store_write_pair(int fd, const char *key, const char *value)
 
 	for (i = 0; value[i]; i++)
 		switch (value[i]) {
+		case '\r':
+			strbuf_addstr(&sb, "\\r");
+			break;
 		case '\n':
 			strbuf_addstr(&sb, "\\n");
 			break;
-- 
1.7.3.1.256.g2539c.dirty
--8<--

Re: [PATCH 3/6] branch: teach --edit-description option

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Thu, Sep 22, 2011 at 03:09:19PM -0700, Junio C Hamano wrote:
>> +	if (launch_editor(git_path(edit_description), &buf, NULL)) {
>> +		strbuf_release(&buf);
>> +		return -1;
>> +	}
>> +	stripspace(&buf, 1);
>> +
>> +	strbuf_addf(&name, "branch.%s.description", branch_name);
>> +	status = git_config_set(name.buf, buf.buf);
>
> I suppose a Windows editor mave save the description with \r\n
> ending. Perhaps a patch like this to avoid messing up config file?

Doesn't stripspace() cleanse that already?

Re: [PATCH 0/6] A handful of "branch description" patches

[Jeff King]

On Fri, Sep 23, 2011 at 10:56:47AM +0200, Michael J Gruber wrote:

>   mjg/vob/refrev-pretend [mjg/vob/virtual-objects: ahead 1]
>     Pseudo revs for refnames
>     
>     An alternative implementation using pretend_sha1...
>     Currently unused.
> 
>   mjg/vob/virtual-objects [origin/next: ahead 2, behind 10]
>     Virtual refname objects
>     
>     For each existing refname, introduce virtual objects corresponding to a blob
>     with the refname as the content. "virtual" refers to the fact that these
>     objects are not written out but exist for all other purposes, such as
>     attaching notes and keeping them from being pruned.

Eww. :)

This seems like a clever solution to making git-notes store a ref as a
key instead of an arbitrary sha1. But I wonder if the end result is
really waht the user wants. The resulting notes tree is good for doing
lookups, but the entries are completely obfuscated. So I can't easily do
something like "list all of the refs which have descriptions". I can
only list the _hashes_ of the refs which have descriptions. And if I am
lucky, I can hash the refs I have and correlate them. But unknown ones
will simply be a mystery.

Wouldn't it be much more friendly to have a separate tree of refnames
that stores:

  refs/heads/foo -> (some blob with the "foo" description)
  refs/heads/bar -> (some blob with the "bar" description)

Yeah, you have to build another git-notes-like interface around it. But
the data structure is pleasant and flexible. You could even "git
checkout" the whole tree and edit the notes with your editor, without
having to deal with some obfuscated name.

-Peff

Re: [PATCH 0/6] A handful of "branch description" patches

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Eww. :)
>
> This seems like a clever solution to making git-notes store a ref as a
> key instead of an arbitrary sha1. But I wonder if the end result is
> really waht the user wants.

A more fundamental issue I have with this is that names of the refs are
local by nature (what I call "master" branch is not "master" to you, but
rather it is "origin/master" or "jch/master") while notes is meant to be
the mechanism to share. The following shares the same issue, but at least
it does not abuse "notes", so in that sense it may be cleaner at the
design level...

> Wouldn't it be much more friendly to have a separate tree of refnames
> that stores:
>
>   refs/heads/foo -> (some blob with the "foo" description)
>   refs/heads/bar -> (some blob with the "bar" description)
>
> Yeah, you have to build another git-notes-like interface around it. But
> the data structure is pleasant and flexible. You could even "git
> checkout" the whole tree and edit the notes with your editor, without
> having to deal with some obfuscated name.

Re: [PATCH 0/6] A handful of "branch description" patches

[Jeff King]

On Fri, Sep 23, 2011 at 01:52:10PM -0700, Junio C Hamano wrote:

> A more fundamental issue I have with this is that names of the refs are
> local by nature (what I call "master" branch is not "master" to you, but
> rather it is "origin/master" or "jch/master") while notes is meant to be
> the mechanism to share. The following shares the same issue, but at least
> it does not abuse "notes", so in that sense it may be cleaner at the
> design level...

Good point. For that reason, your config-based solution perhaps makes
more sense.

-Peff

Re: [PATCH 0/6] A handful of "branch description" patches

[Michael J Gruber]

Jeff King venit, vidit, dixit 23.09.2011 22:18:
> On Fri, Sep 23, 2011 at 10:56:47AM +0200, Michael J Gruber wrote:
> 
>>   mjg/vob/refrev-pretend [mjg/vob/virtual-objects: ahead 1]
>>     Pseudo revs for refnames
>>     
>>     An alternative implementation using pretend_sha1...
>>     Currently unused.
>>
>>   mjg/vob/virtual-objects [origin/next: ahead 2, behind 10]
>>     Virtual refname objects
>>     
>>     For each existing refname, introduce virtual objects corresponding to a blob
>>     with the refname as the content. "virtual" refers to the fact that these
>>     objects are not written out but exist for all other purposes, such as
>>     attaching notes and keeping them from being pruned.
> 
> Eww. :)
> 
> This seems like a clever solution to making git-notes store a ref as a
> key instead of an arbitrary sha1. But I wonder if the end result is
> really waht the user wants. The resulting notes tree is good for doing
> lookups, but the entries are completely obfuscated. So I can't easily do
> something like "list all of the refs which have descriptions". I can
> only list the _hashes_ of the refs which have descriptions. And if I am
> lucky, I can hash the refs I have and correlate them. But unknown ones
> will simply be a mystery.

[mjg@localhost git]$ git rev-parse ref:mjg/vob/virtual-objects
3f8aa9bb80fe241306aafd3d76af50739ba88268
[mjg@localhost git]$ git show 3f8aa9bb80fe241306aafd3d76af50739ba88268
refs/heads/mjg/vob/virtual-objects

:)

The only problem is with notes for non-existing refs. [You only have to
invoke the inverse mapping to sha1, of course... Uhm.]

> Wouldn't it be much more friendly to have a separate tree of refnames
> that stores:
> 
>   refs/heads/foo -> (some blob with the "foo" description)
>   refs/heads/bar -> (some blob with the "bar" description)

Given the above, I don't think it's more friendly.

In fact, in my first attempt, I wrote out the blobs, and referenced them
just like above from a different subtree within the notes tree, in order
to keep them from being pruned. So the virtual approach is pretty
equivalent, though leaner.

> Yeah, you have to build another git-notes-like interface around it. But
> the data structure is pleasant and flexible. You could even "git
> checkout" the whole tree and edit the notes with your editor, without
> having to deal with some obfuscated name.

Well, "git branch --edit-description" and such should be the way to edit
them, shouldn't it?

I really think the only issue is remote refnames. As Junio points out,
they are local by nature. OTOH, you typically use a non-renaming refspec
which puts them under refs/remotes/foo/bar with "bar" being the same
name as the local one on the remote, foo something you have chosen. So,
teaching the code that the note for

refs/remotes/foo/bar

is in the notes tree

refs/remotes/foo/notes/commits (or .../refames, or whatever we do with
the namespaces)

as a note attached to sha1("refs/bar")

is really a non-issue. It's not done yet, in part because of the
possible namespace restructuring.

Michael

Re: [PATCH 3/6] branch: teach --edit-description option

[Nguyen Thai Ngoc Duy]

On Sat, Sep 24, 2011 at 5:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Thu, Sep 22, 2011 at 03:09:19PM -0700, Junio C Hamano wrote:
>>> +    if (launch_editor(git_path(edit_description), &buf, NULL)) {
>>> +            strbuf_release(&buf);
>>> +            return -1;
>>> +    }
>>> +    stripspace(&buf, 1);
>>> +
>>> +    strbuf_addf(&name, "branch.%s.description", branch_name);
>>> +    status = git_config_set(name.buf, buf.buf);
>>
>> I suppose a Windows editor mave save the description with \r\n
>> ending. Perhaps a patch like this to avoid messing up config file?
>
> Doesn't stripspace() cleanse that already?
>

Yes, isspace() indeed treats \r as a space and stripspace() does the
right thing.
-- 
Duy

Re: [PATCH 0/6] A handful of "branch description" patches

[Jeff King]

On Sat, Sep 24, 2011 at 04:42:18PM +0200, Michael J Gruber wrote:

> > This seems like a clever solution to making git-notes store a ref as a
> > key instead of an arbitrary sha1. But I wonder if the end result is
> > really waht the user wants. The resulting notes tree is good for doing
> > lookups, but the entries are completely obfuscated. So I can't easily do
> > something like "list all of the refs which have descriptions". I can
> > only list the _hashes_ of the refs which have descriptions. And if I am
> > lucky, I can hash the refs I have and correlate them. But unknown ones
> > will simply be a mystery.
> 
> [mjg@localhost git]$ git rev-parse ref:mjg/vob/virtual-objects
> 3f8aa9bb80fe241306aafd3d76af50739ba88268
> [mjg@localhost git]$ git show 3f8aa9bb80fe241306aafd3d76af50739ba88268
> refs/heads/mjg/vob/virtual-objects

Sure, but what about:

  git notes list

which is just filled with meaningless nonsense.

> > Wouldn't it be much more friendly to have a separate tree of refnames
> > that stores:
> > 
> >   refs/heads/foo -> (some blob with the "foo" description)
> >   refs/heads/bar -> (some blob with the "bar" description)
> 
> Given the above, I don't think it's more friendly.
> 
> In fact, in my first attempt, I wrote out the blobs, and referenced them
> just like above from a different subtree within the notes tree, in order
> to keep them from being pruned. So the virtual approach is pretty
> equivalent, though leaner.

Hmm. So your mapping of $ref to $desc is:

  sha1($ref) -> sha1(blob($desc))

>From what you wrote there, I think maybe you think I meant to store:

  sha1(blob($ref)) -> sha1(blob($desc))

But what I meant was actually:

  $ref -> sha1(blob($desc))

I.e., not to use "notes" at all, but rather a tree that mirrors the
refs/ hierarchy in its names.

> > Yeah, you have to build another git-notes-like interface around it. But
> > the data structure is pleasant and flexible. You could even "git
> > checkout" the whole tree and edit the notes with your editor, without
> > having to deal with some obfuscated name.
> 
> Well, "git branch --edit-description" and such should be the way to edit
> them, shouldn't it?

It's one way. I assume that if we store things in a reasonable,
readable state, then people like that because they can hack on the data
structure using more flexible tools.

> I really think the only issue is remote refnames. As Junio points out,
> they are local by nature. OTOH, you typically use a non-renaming refspec
> which puts them under refs/remotes/foo/bar with "bar" being the same
> name as the local one on the remote, foo something you have chosen. So,
> teaching the code that the note for

If they are local by nature, is it worth putting them into a notes tree
at all? That provides versioning and backup. But I wonder if it is worth
the hassle, when one could just put them in the config.

-Peff

Annotated branch ≈ annotated tag?

[Michael Haggerty]

On 09/27/2011 11:58 PM, Jeff King wrote:
> On Sat, Sep 24, 2011 at 04:42:18PM +0200, Michael J Gruber wrote:
>> I really think the only issue is remote refnames. As Junio points out,
>> they are local by nature. OTOH, you typically use a non-renaming refspec
>> which puts them under refs/remotes/foo/bar with "bar" being the same
>> name as the local one on the remote, foo something you have chosen. So,
>> teaching the code that the note for
> 
> If they are local by nature, is it worth putting them into a notes tree
> at all? That provides versioning and backup. But I wonder if it is worth
> the hassle, when one could just put them in the config.

I don't think that branch descriptions should be local-only.  They would
be a good way to share information with others about work in progress.

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.

  * Presumably, by default, the old description would be copied from the
old annotation object to the new one; the committer would be set to
those of the user doing the update.

  * The old annotation object would become unreachable after every
branch update, though locally it would still be reachable via the
branch's reflog [1].

* Creating a new branch from an annotated branch should perhaps open an
editor to allow the user to set a new comment.  If the user deletes the
whole comment in the editor, then an unannotated branch is created
instead of an annotated branch.

* We would need rules for merging annotation objects:

  * There is no such thing as a fast-forward merge to an annotated
branch, because the merged-from branch, even if annotated, can never
have the merged-to annotation object in its history.  So proceed to the
following rules.

  * If both branches are unannotated, then the result should be an
unannotated branch (like today).

  * If both branches have the same comment, the comment should be
carried over to the result without prompting.

  * If the merge-to branch is annotated and the merge-from branch is
not, keep the merge-to branch's annotation.

  * If the merge-to branch is unannotated and the merge-from branch is
annotated, the result should be a conflict.  This cannot be resolved
automatically because there are two likely scenarios:

    * A remote branch is being merged into a remote-tracking branch.  In
this case somebody upstream probably added a comment to the branch, and
one would like to preserve this comment in a local annotated branch.

    * A feature branch is being merged back to mainline.  In such a case
one would *not* like to carry over the branch annotation (which
presumably describes the feature that was developed on the branch),
though it would often be convenient to integrate the merged-from branch
annotation into the log message of the merge commit.

    I'm not sure how to let the user distinguish between the two cases
above, but it probably involves $EDITOR.

  * If the merge-to branch and the merge-from branch have conflicting
annotations, there are two possibilities much like in the previous case.

* Annotated tag objects include the name of the tag that is being
annotated.  Annotated branch objects should *not* include this
information, because it does not carry across from one repo to another
when branches are renamed.  (Perhaps the presence/absence of the tag
name could be what distinguishes an (unmovable) annotated tag object
from a (movable) annotated branch object.)

It would even be possible to allow signatures in the annotated objects;
these could play the role of push certificates.  Whenever a signed
annotated branch is updated, the signature would go away (it would
probably be too cumbersome to prompt the user to generate it a new
signature for every branch update).  It should be easy to add a
signature to an existing annotated or unannotated branch (writing a new
annotation object, of course).

ISTM that the semantics would be very close to what is desired, and
would satisfy a few needs: a place to describe work-in-progress in a
sharable way, branch descriptions that can be used for constructing pull
requests and merge commit messages, and (optionally) a way to implement
signed pushes.  It would surely be a lot of work to implement, but being
based on annotated tags would mean that a lot of code, documentation,
and training would be common to the two concepts.

Michael

[1] If the retention of annotation history were considered a
requirement, the annotation object could record as a "parent" the object
name of the annotation object that it is succeeding.  But I don't think
that this is a good idea; it would make branches too heavyweight and
every branch update would be recorded permanently, both of which are
contrary to the git philosophy.

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Annotated branch ≈ annotated tag?

[Andrew Ardill]

On 28 September 2011 14:23, Michael Haggerty <mhagger@alum.mit.edu> wrote:
[snip]
>
> [1] If the retention of annotation history were considered a
> requirement, the annotation object could record as a "parent" the object
> name of the annotation object that it is succeeding.  But I don't think
> that this is a good idea; it would make branches too heavyweight and
> every branch update would be recorded permanently, both of which are
> contrary to the git philosophy.

If this was required, a better way would be to update the parent object only
if the description changed. You would then have a nice little DAG that
records changes to the description and could be used in 3-way merges etc.
You would of course get lots of 'dead' annotation objects pointing to the
previous change, however that shouldn't be too much of an issue.

At this point, however, I ask how is an annotation object any different to
placing an annotation file in our repository. Perhaps there is no difference,
except that one is a convention and the other is provided.

Regards,
Andrew

Re: Annotated branch ≈ annotated tag?

[Michael Haggerty]

On 09/28/2011 09:12 AM, Andrew Ardill wrote:
> On 28 September 2011 14:23, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> [snip]
>>
>> [1] If the retention of annotation history were considered a
>> requirement, the annotation object could record as a "parent" the object
>> name of the annotation object that it is succeeding.  But I don't think
>> that this is a good idea; it would make branches too heavyweight and
>> every branch update would be recorded permanently, both of which are
>> contrary to the git philosophy.
> 
> If this was required, a better way would be to update the parent object only
> if the description changed. You would then have a nice little DAG that
> records changes to the description and could be used in 3-way merges etc.
> You would of course get lots of 'dead' annotation objects pointing to the
> previous change, however that shouldn't be too much of an issue.
> 
> At this point, however, I ask how is an annotation object any different to
> placing an annotation file in our repository. Perhaps there is no difference,
> except that one is a convention and the other is provided.

Yes, if history is being preserved, then the annotation objects would
not be much different than storing a file in the repository.  But even
then, there are differences:

- A branch annotation would be separate from the source code and not
appear in the working tree, which seems more appropriate for metadata.

- git and other tools would know where to find the annotation instead of
having to configure whether a particular project uses annotations and if
so where to find them.  This would make it easier to use the annotations
in git workflow like the generation of pull requests.

- The merge rules for annotations would be different than those for
other files.

But I believe that branch annotation history should *not* be retained,
so storing the annotations in the source tree is not even an option
(except perhaps in another artificial branch used only for annotations).

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Branch annotations [Re: Annotated branch ≈ annotated tag?]

[Michael J Gruber]

I'm not tied to a particular implementation (not even mine), but we
should think through the concept before baking something in. That was my
impetus for throwing in a (half-baked) notes based solution before a
config based is baked^Wset in stone ;)

For me, commit annotations as currently implemented (notes) have the
following positives:

* easy ui (add/edit/copy)
* easy scripting (-F/-m)
* can be shared *if I want* (by pushing refspec; note that share=backup
as well as share=publish, depending on where I push to); ui could be
somewhat better
* multiple sources possible
* is versioned (ui could be better, e.g. git notes log)

For branch annotations, I would want to have all of the above. Depending
on the use case, I want to treat branch annotations as purely local (but
may still want to push them to backup) or share and publish them. I
might be interested in their history or not, etc. In addition, we would
want to have the obvious:
* git branch -m moves annotation
* git branch --list --younameit shows annotation
* etc.

If we agree that we want the above properties (and that is a big if)
then using notes seem very natural. [Having to rewrite an annotated tag
object at each branch head change appears unnatural.] They should
probably live in a separate default tree (one per remote for remote
branches), and the actual mechanics (virtual objects, real objects,
textconvcache like...) is not dictated by those requirements.

Note though that we might be interested in annotating more general names
than just refnames, e.g. paths, or names like "description". Since tags
should be immutable, adding notes to a tag seems not much different from
annotating the referenced commit, but it is different in concept and
could be treated differently (as tag notes would be in a separate tree
from the commit notes tree).

So, if we want to keep that path open (annotate more general names), a
mapping from names to notes becomes mandatory. Again, that does not
dictate a specific implementation.

Michael

Re: Annotated branch ≈ annotated tag?

[Jeff King]

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