Git development
 help / color / mirror / Atom feed
* Re: How pretty is pretty? git cat-file -p inconsistency
From: Junio C Hamano @ 2011-10-07 18:04 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4E8EBC00.90909@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> That is, "cat file -p" pretty prints dates for tag objects but not for
> commit objects. In fact, "-p" on commit objects does not prettify at all
> compared to the raw content. Is that intentional?

"cat-file -p" is an ill-conceived half-ass afterthought, and I do not
think anybody sane considers it as part of the "plumbing" ultra stable
interface for machine consumption. See a0f15fa (Pretty-print tagger
dates., 2006-03-01).

> I'd suggest
> prettifying dates with "-p" for commit objects also.

Please make it so. It is your choice to do a patch to update this single
thing first, or to discuss the output with "-p" for all the other object
types at the same time to get the list concensus before proceeding.

Thanks.

^ permalink raw reply

* "Use of uninitialized value" running "git svn clone" when having svn tag branches
From: Luiz-Otavio Zorzella @ 2011-10-07 18:01 UTC (permalink / raw)
  To: git

I'm trying to convert a project (hosted in googlecode.com) from svn to
git, using the "git svn clone" command, and I'm getting an "Use of
uninitialized value" error. Here's the truncated output:

$ git svn clone https://test-libraries-for-java.googlecode.com/svn
--no-metadata -A ~/tmp/authors.txt -t tags -b branches -T trunk
test-libraries-for-java
 r1 = c3adafa93a420f19b1bcfb6765fe0eb90aaa751c (refs/remotes/trunk)
       A       .classpath
       A       .project
       A       COPYING
       A       build.properties
       A       build.xml
W: +empty_dir: trunk/src
[...]
r10 = 8d5d7fdebdb7f822388fd3e4f4061abbfd1fb0cf (refs/remotes/trunk)
       M       test/com/google/common/testing/junit3/JUnitAssertsTest.java
r11 = 4c8a77660bf353ed55c9d583b39e263203c685a4 (refs/remotes/trunk)
Found possible branch point:
https://test-libraries-for-java.googlecode.com/svn/trunk =>
https://test-libraries-for-java.googlecode.com/svn/tags/release-1.0,
11
Use of uninitialized value $u in substitution (s///) at
/usr/lib/git-core/git-svn line 1731.
Use of uninitialized value $u in concatenation (.) or string at
/usr/lib/git-core/git-svn line 1731.
refs/remotes/trunk:
'https://test-libraries-for-java.googlecode.com/svn' not found in ''

For completeness, here's the authors.txt file I'm using:

$ cat ~/tmp/authors.txt
zorzella = Luiz-Otavio 'Z' Zorzella <zorzella@gmail.com>
(no author) = Luiz-Otavio 'Z' Zorzella <zorzella@gmail.com>

**************

The same command works fine if I don't use the "-t tags" part, but
then it does not create the tag branches in my converted git.

Thanks in advance,

Z

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-07 18:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk48gwvyd.fsf@alter.siamese.dyndns.org>

On Fri, Oct 7, 2011 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> Implement "git merge [-e|--edit]" as "git merge --no-commit && git commit"
>> as a convenience for the user.
>>
>> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
>> ---
>> ...
>> @@ -1447,6 +1457,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>>       }
>>
>>       if (merge_was_ok) {
>> +             if (option_edit) {
>> +                     const char *args[] = {"commit", "-e", NULL};
>> +                     return run_command_v_opt(args, RUN_GIT_CMD);
>> +             }
>>               fprintf(stderr, _("Automatic merge went well; "
>>                       "stopped before committing as requested\n"));
>>               return 0;
>
>
> I wanted to like this approach, thinking this approach might be safer and
> with the least chance of breaking other codepaths, but this feels like an
> ugly hack.
>
> Are we still honoring all the hooks "git merge" honors?  More importantly,
> isn't this make it impossible for future maintainers of this command to
> enhance the command by adding other hooks after the commit is made?

Git is already inconsistent with respect to which hooks are called
when. Shouldn't post-merge be called on a merge commit regardless of
whether you use --no-commit or not? Well, it isn't, it's only called
when merge performs the commit internally. The post-merge hook was
probably a mistake -- git calls the post-commit hook passing the
context as an argument, so probably merge should just call post-commit
"merge". But that ship has sailed.

See also 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14).

> If we wanted to do this properly, we should update builtin/merge.c to call
> launch_editor() before it runs commit_tree(), in a way similar to how
> prepare_to_commit() in builtin/commit.c does so when e.g. "commit -m foo -e"
> is run. An editmsg is prepared (you already have it in MERGE_MSG), the
> editor is allowed to update it, and then the original code before such a
> patch will run using the updated contents of MERGE_MSG. That way, the _only_
> change in behaviour when "-e" is used is to let the user update the message
> used in the commit log, and everything else would run exactly the same way
> as if no "-e" was given, including the invocation of hooks.

I find git very difficult to reason about (and inconsistent in its
behavior) due to piecemeal hoisting of some functionality into
porcelain commands (another example, revert.c building in the
recursive merge strategy but not any others).

I actually think a better choice would be to remove commit_tree() from
merge and always have it run commit externally. I'm not seriously
suggesting that be done, but it would make git more consistent. But
I'm not going to send in a patch which makes the situation worse.

j.

^ permalink raw reply

* Re: [PATCH WIP 0/3] git log --exclude
From: Junio C Hamano @ 2011-10-07 17:57 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Jonathan Nieder
In-Reply-To: <CACsJy8DJs_cmCZegyPk=tB-bxWp4izrsTn8T=xeV1sU4fS5oTQ@mail.gmail.com>

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

> There may be a solution to mix depth limit and negative pathspec. I
> haven't thought carefully about that because I lean towards a simpler
> behaviour that only allows one negation level: a file is in if it
> matches any positive pathspecs and none of negative ones.

I tend to think it probably is acceptable solution to define "struct
pathspec" to have one positive and one negative set, traversing and
declaring a match on what matches something from the positive set only if
it does not match anything in the negative set.

That is similar to how we define revision ranges in the sense that the
range notation to have two ranges (A..B C..D) does not mean union of two
sets (A..B + C..D), but is difference between two unions ((B D)-(A C)).

> This is enough if narrow clones consist of positive pathspec only. I
> really doubt if users would want to make a narrow clone that "contains
> A but not A/B, storage-wise".

And if you define "struct pathspec" to have one positive and one negative
set, you do not have to limit narrow clone only to positive. The repository
specific narrow pathspec will have one such "struct pathspec", but the
user may give us from the command line another "struct pathspec". At
runtime, we would merge them to form into one negative and one positive
set, and apply the same rule: nothing that matches any negative will
appear in the traversal or in the output.

^ permalink raw reply

* unexpected behavior with `git log --skip filename`
From: Andrew McNabb @ 2011-10-07 17:15 UTC (permalink / raw)
  To: git

The "--skip" option to "git log" did not behave as I expected, but I'm
not sure whether this was user error, unclear documentation, or a bug.
Specifically, I ran the following, intending to find the previous
revision of a given file:

git log --skip=1 -n 1 --oneline some-filename

My expectation was that this would behave the same as:

git log -n 2 --oneline some-filename |tail -n 1

Instead, the --skip=1 parameter seemed to be ignored.  After I tried
several different values, it appears that the commits are skipped before
path matching with "some-filename".

Is this the intended behavior?  If so, should the documentation be
clarified by changing "Note that they are applied before commit ordering
and formatting options, such as --reverse" to something like "Note that
they are applied before path matching, commit ordering, and formatting
options, such as --reverse"?

--
Andrew McNabb
http://www.mcnabbs.org/andrew/
PGP Fingerprint: 8A17 B57C 6879 1863 DE55  8012 AB4D 6098 8826 6868

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-07 17:30 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <1318001347-11347-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> Implement "git merge [-e|--edit]" as "git merge --no-commit && git commit"
> as a convenience for the user.
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
> ...
> @@ -1447,6 +1457,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	if (merge_was_ok) {
> +		if (option_edit) {
> +			const char *args[] = {"commit", "-e", NULL};
> +			return run_command_v_opt(args, RUN_GIT_CMD);
> +		}
>  		fprintf(stderr, _("Automatic merge went well; "
>  			"stopped before committing as requested\n"));
>  		return 0;


I wanted to like this approach, thinking this approach might be safer and
with the least chance of breaking other codepaths, but this feels like an
ugly hack.

Are we still honoring all the hooks "git merge" honors?  More importantly,
isn't this make it impossible for future maintainers of this command to
enhance the command by adding other hooks after the commit is made?

If we wanted to do this properly, we should update builtin/merge.c to call
launch_editor() before it runs commit_tree(), in a way similar to how
prepare_to_commit() in builtin/commit.c does so when e.g. "commit -m foo -e"
is run. An editmsg is prepared (you already have it in MERGE_MSG), the
editor is allowed to update it, and then the original code before such a
patch will run using the updated contents of MERGE_MSG. That way, the _only_
change in behaviour when "-e" is used is to let the user update the message
used in the commit log, and everything else would run exactly the same way
as if no "-e" was given, including the invocation of hooks.

^ permalink raw reply

* Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Carlos Martín Nieto @ 2011-10-07 16:40 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <20111007163319.GC4399@sigill.intra.peff.net>

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

On Fri, 2011-10-07 at 12:33 -0400, Jeff King wrote:
> On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:
> 
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index b937d71..94b2bd3 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
> >  		free_refs(ref_map);
> >  		return 1;
> >  	}
> > -	if (prune)
> > +	if (prune) {
> > +		/* If --tags was specified, we need to tell prune_refs
> > +		 * that we're filtering the refs from the remote */
> > +		if (tags == TAGS_SET) {
> > +			const char * tags_refspec = "refs/tags/*:refs/tags/*";
> > +			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
> > +			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
> > +			ref_count++;
> > +		}
> >  		prune_refs(transport, refs, ref_count, ref_map);
> > +	}
> 
> I don't think we can realloc refs here. It's passed into do_fetch. When
> we realloc it, the old pointer value will be invalid. But when we return
> from do_fetch, the caller (fetch_one) will still have that old value,
> and will call free() on it.

Yes, you're right. I guess it's been working by luck and generous amount
of memory.

> 
> Instead, you have to make a whole new list, copy the old values in, add
> your new one, and then free the result.

Will do.

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Jeff King @ 2011-10-07 16:39 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <1318005433.4579.5.camel@centaur.lab.cmartin.tk>

On Fri, Oct 07, 2011 at 06:37:13PM +0200, Carlos Martín Nieto wrote:

> > I assume you mean s/tag/branch/ in the last line?
> 
> Yeah, maybe ref would be better?

Yeah, agreed.

> > Tests?
> 
> Good point.

It sounds like you already have a reproduction recipe for this, and for
the --tags thing in the next commit.

> OK, so take a step back and figure out what we want the rules to be
> before we call get_stale_heads. It does sound like a more elegant
> approach. I was trying to disrupt the callers as little as possible, but
> then again, there's only two. Will change.

Yeah. Sometimes we try hard to make a minimal patch, because it makes it
easier to review. At the same time, I think it's more important to make
the code clean if it needs it. Especially when there aren't many callers
to disrupt.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Carlos Martín Nieto @ 2011-10-07 16:37 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <20111007162625.GB4399@sigill.intra.peff.net>

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

On Fri, 2011-10-07 at 12:26 -0400, Jeff King wrote:
> On Thu, Oct 06, 2011 at 11:21:46PM +0200, Carlos Martín Nieto wrote:
> 
> > If the user gave us refspecs on the command line, we should use those
> > when deciding whether to prune a ref instead of relying on the
> > refspecs in the config.
> > 
> > Previously, running
> > 
> >     git fetch --prune origin refs/heads/master:refs/remotes/origin/master
> > 
> > would delete every other tag under the origin namespace because we
> 
> I assume you mean s/tag/branch/ in the last line?

Yeah, maybe ref would be better?

> 
> > ---
> >  builtin/fetch.c  |    6 ++--
> >  builtin/remote.c |    2 +-
> >  remote.c         |   78 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  remote.h         |    3 +-
> >  4 files changed, 77 insertions(+), 12 deletions(-)
> 
> Tests?

Good point.

> 
> >  static int get_stale_heads_cb(const char *refname,
> >  	const unsigned char *sha1, int flags, void *cb_data)
> >  {
> >  	struct stale_heads_info *info = cb_data;
> >  	struct refspec refspec;
> > +	int ret;
> >  	memset(&refspec, 0, sizeof(refspec));
> >  	refspec.dst = (char *)refname;
> > -	if (!remote_find_tracking(info->remote, &refspec)) {
> > -		if (!((flags & REF_ISSYMREF) ||
> > -		    string_list_has_string(info->ref_names, refspec.src))) {
> > -			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
> > -			hashcpy(ref->new_sha1, sha1);
> > -		}
> > +
> > +	/*
> > +	 * If the user speicified refspecs on the command line, we
> > +	 * should only use those to check. Otherwise, look in the
> > +	 * remote's configuration for the branch.
> > +	 */
> > +	if (info->ref_count)
> > +		ret = find_in_refs(info->refs, info->ref_count, &refspec);
> > +	else
> > +		ret = remote_find_tracking(info->remote, &refspec);
> 
> Minor typo in the comment. But more importantly, this feels like a very
> low-level place to be thinking about things like what the user gave us
> on the command line.
> 
> Shouldn't get_stale_heads not get a remote at all, and just get a set of
> refspecs? Those should be the minimal information it needs to get its
> answer, right?

OK, so take a step back and figure out what we want the rules to be
before we call get_stale_heads. It does sound like a more elegant
approach. I was trying to disrupt the callers as little as possible, but
then again, there's only two. Will change.

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Jeff King @ 2011-10-07 16:33 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-4-git-send-email-cmn@elego.de>

On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index b937d71..94b2bd3 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
>  		free_refs(ref_map);
>  		return 1;
>  	}
> -	if (prune)
> +	if (prune) {
> +		/* If --tags was specified, we need to tell prune_refs
> +		 * that we're filtering the refs from the remote */
> +		if (tags == TAGS_SET) {
> +			const char * tags_refspec = "refs/tags/*:refs/tags/*";
> +			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
> +			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
> +			ref_count++;
> +		}
>  		prune_refs(transport, refs, ref_count, ref_map);
> +	}

I don't think we can realloc refs here. It's passed into do_fetch. When
we realloc it, the old pointer value will be invalid. But when we return
from do_fetch, the caller (fetch_one) will still have that old value,
and will call free() on it.

Instead, you have to make a whole new list, copy the old values in, add
your new one, and then free the result.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Jeff King @ 2011-10-07 16:26 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-3-git-send-email-cmn@elego.de>

On Thu, Oct 06, 2011 at 11:21:46PM +0200, Carlos Martín Nieto wrote:

> If the user gave us refspecs on the command line, we should use those
> when deciding whether to prune a ref instead of relying on the
> refspecs in the config.
> 
> Previously, running
> 
>     git fetch --prune origin refs/heads/master:refs/remotes/origin/master
> 
> would delete every other tag under the origin namespace because we

I assume you mean s/tag/branch/ in the last line?

> ---
>  builtin/fetch.c  |    6 ++--
>  builtin/remote.c |    2 +-
>  remote.c         |   78 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  remote.h         |    3 +-
>  4 files changed, 77 insertions(+), 12 deletions(-)

Tests?

>  static int get_stale_heads_cb(const char *refname,
>  	const unsigned char *sha1, int flags, void *cb_data)
>  {
>  	struct stale_heads_info *info = cb_data;
>  	struct refspec refspec;
> +	int ret;
>  	memset(&refspec, 0, sizeof(refspec));
>  	refspec.dst = (char *)refname;
> -	if (!remote_find_tracking(info->remote, &refspec)) {
> -		if (!((flags & REF_ISSYMREF) ||
> -		    string_list_has_string(info->ref_names, refspec.src))) {
> -			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
> -			hashcpy(ref->new_sha1, sha1);
> -		}
> +
> +	/*
> +	 * If the user speicified refspecs on the command line, we
> +	 * should only use those to check. Otherwise, look in the
> +	 * remote's configuration for the branch.
> +	 */
> +	if (info->ref_count)
> +		ret = find_in_refs(info->refs, info->ref_count, &refspec);
> +	else
> +		ret = remote_find_tracking(info->remote, &refspec);

Minor typo in the comment. But more importantly, this feels like a very
low-level place to be thinking about things like what the user gave us
on the command line.

Shouldn't get_stale_heads not get a remote at all, and just get a set of
refspecs? Those should be the minimal information it needs to get its
answer, right?

-Peff

^ permalink raw reply

* Re: [PATCH] show git tag output in pager
From: Jeff King @ 2011-10-07 16:16 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Michal Vyskocil, git
In-Reply-To: <vpqsjn4dfi4.fsf@bauges.imag.fr>

On Fri, Oct 07, 2011 at 04:48:35PM +0200, Matthieu Moy wrote:

> > On Mon, Oct 03, 2011 at 02:57:09PM +0200, Matthieu Moy wrote:
> >
> >> I like the try_subcommand_pager idea. Ideally, there would also be a
> >> nice mechanism to set defaults for subcommands, so that "git tag
> >> <whatever>" does the right thing without configuration.
> >
> > That's easy enough. Something like the patch below?
> 
> It may have been better with a big centralized array of configurations
> like
> 
> 	static struct cmd_struct commands[] = {
> 		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>                 ...
> 
> in git.c, but if we have only a few instances of this, your system is
> probably fine. I like it.

I don't think you can centralize it in the same way, because some of it
will have to be implemented in shell script (unlike the full-command
ones, which we can always trigger at the git.c wrapper layer).

So you could have:

  static struct pager_default subcommands[] = {
          { "tag.list", 1 },
          { "branch.list", 1 },
  };

but you'll never be able to put "stash.list" into that structure (and as
you probably guessed, my patch isn't enough to implement stash.list,
either; it would need a shell implementation of try_subcommand_pager).

-Peff

^ permalink raw reply

* Scalable reference handling
From: Michael Haggerty @ 2011-10-07 15:51 UTC (permalink / raw)
  To: Martin Fick; +Cc: git
In-Reply-To: <4E8E6E8E.5070909@alum.mit.edu>

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

On 10/07/2011 05:14 AM, Michael Haggerty wrote:
> On 10/07/2011 12:16 AM, Martin Fick wrote:
>> I downloaded your patch series and tested it on my repos.
> 
> Very cool (though a bit premature, as you discovered).  The patch series
> still has a known performance regression in the area of
> do_for_each_ref(), which I hope to figure out soon.  I will definitely
> tell you when I think that the patch series is ready for more serious
> testing (hopefully today) in the hopes that you can benchmark it against
> your repo.

I just pushed versions to github that I think are ready for some
preliminary testing.  There were some silly inefficiencies in the
version that you tested earlier, so this version is considerably faster
in a few key tests.

I don't have complete benchmarking results, but I have attached what I
have.  I wouldn't put much weight on small differences in the numbers
because the computer was not 100% quiescent while I ran the tests.  But
I think the results are impressive: the new code (columns 5-8) is a bit
slower in only a few cases but faster (sometimes by a large factor) in
many other cases.

I can't write more now, but Martin, if you have time to benchmark
9944c7faf903a95d4ed9de284ace32debe21cdc1 against your repository, I
would be very interested to learn the results.

BTW I am not asking anybody to review the patch series yet; I would like
to do some more tests and cleanup first.  But of course I wouldn't
object to feedback.  A good starting point would be the comments at the
top of refs.c, where the basic data structures are explained.

Michael

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

[-- Attachment #2: refperf-summary-3.out --]
[-- Type: text/plain, Size: 4983 bytes --]

===================================  =======  =======  =======  =======  =======  =======  =======  =======  =======
Test name                                [0]      [1]      [2]      [3]      [4]      [5]      [6]      [7]      [8]
===================================  =======  =======  =======  =======  =======  =======  =======  =======  =======
branch-loose-cold                       5.14     5.80     4.98     4.55     5.03     0.41     0.45     0.45     0.51
branch-loose-warm                       0.10     0.12     0.12     0.19     0.11     0.02     0.02     0.03     0.02
for-each-ref-loose-cold                 6.02     6.36     7.48     5.62     6.27     5.32     5.02     5.24     5.01
for-each-ref-loose-warm                 0.25     0.25     0.27     0.25     0.25     0.26     0.26     0.25     0.26
checkout-loose-cold                     5.47     5.03     5.66     5.39     5.72     0.65     0.77     0.95     0.59
checkout-loose-warm                     0.11     0.11     0.11     0.11     0.11     0.02     0.03     0.03     0.03
checkout-orphan-loose                   0.09     0.09     0.10     0.10     0.10     0.03     0.01     0.01     0.02
checkout-from-detached-loose-cold        N/A      N/A      N/A      N/A      N/A      N/A      N/A      N/A      N/A
checkout-from-detached-loose-warm        N/A      N/A      N/A      N/A      N/A      N/A      N/A      N/A      N/A
branch-contains-loose-cold             14.69    13.95    13.76    13.84    14.07    14.08    14.12    14.01    14.77
branch-contains-loose-warm              8.99     8.89     8.74     8.82     8.80     8.81     8.84     8.76     8.91
pack-refs-loose                         1.23     1.02     1.02     1.02     1.01     1.02     1.00     0.99     1.03
branch-packed-cold                      0.66     0.65     0.84     0.67     0.52     0.76     0.58     0.55     0.59
branch-packed-warm                      0.01     0.01     0.03     0.03     0.02     0.02     0.05     0.02     0.05
for-each-ref-packed-cold                1.38     1.29     1.07     1.13     1.05     1.08     1.26     1.05     1.27
for-each-ref-packed-warm                0.17     0.16     0.17     0.17     0.17     0.16     0.17     0.16     0.16
checkout-packed-cold                    8.74     7.75     7.75     1.59     1.61     1.79     1.54     1.55     1.53
checkout-packed-warm                    0.03     0.05     0.04     0.04     0.05     0.05     0.03     0.05     0.06
checkout-orphan-packed                  0.04     0.01     0.01     0.03     0.02     0.02     0.05     0.05     0.02
checkout-from-detached-packed-cold      8.88     8.22     8.12     1.98     1.81     1.94     1.99     1.88     1.97
checkout-from-detached-packed-warm      6.55     6.43     6.44     0.44     0.46     0.45     0.46     0.46     0.49
branch-contains-packed-cold            11.59    10.73    10.37     9.42     9.24     9.40     9.37     9.56     9.34
branch-contains-packed-warm            10.20     9.79     9.83     8.68     8.61     8.68     8.61     8.82     9.33
clone-loose-cold                      105.58    86.98    89.42    88.96    88.19    87.01    87.20    87.11    88.20
clone-loose-warm                        3.26     3.11     3.18     3.10     3.17     3.14     3.14     3.21     3.16
fetch-nothing-loose                     0.85     0.84     0.86     0.85     0.84     0.88     0.84     0.84     0.84
pack-refs                               0.12     0.14     0.16     0.13     0.15     0.14     0.14     0.15     0.14
fetch-nothing-packed                    0.84     0.84     0.86     0.85     0.89     0.85     0.84     0.84     0.83
clone-packed-cold                       2.72     2.33     2.11     2.19     2.23     2.14     2.24     2.01     2.16
clone-packed-warm                       0.40     0.40     0.33     0.40     0.28     0.29     0.33     0.31     0.31
fetch-everything-cold                 106.28    92.92    94.12    88.76    91.62    88.75    89.49    89.78    91.43
fetch-everything-warm                   5.56     5.63     5.70     5.62     5.63     5.66     5.69     5.71     5.78
===================================  =======  =======  =======  =======  =======  =======  =======  =======  =======


[0] v1.7.6 (refperf.times.d78c84e8698e750139667bc724b08eb34e795b65)
[1] v1.7.7 (refperf.times.a258e475eb74e183e9e68ca30e32c5253081356d)
[2] origin/master (refperf.times.27897d25f1b36d400b82b655701b87fd205dbc2f)
[3] 16583974c20b856bb60b5a733020425c16a19670^ (refperf.times.558b49c8489c95cf966b959c3444c95d177dc4dc)
[4] 16583974c20b856bb60b5a733020425c16a19670 (refperf.times.16583974c20b856bb60b5a733020425c16a19670)
[5] origin/hierarchical-refs^^^^ (refperf.times.5f5a126553eef88455f7deb2745c5f93073bfe69)
[6] origin/hierarchical-refs^^^ (refperf.times.a306af145856f8296bf2ff4a3ace5e86ac3b1fc8)
[7] origin/hierarchical-refs^ (refperf.times.fd53cf7a7fcc360f30ea0ec5b964cefeec70c11b)
[8] origin/hierarchical-refs (refperf.times.9944c7faf903a95d4ed9de284ace32debe21cdc1)


^ permalink raw reply

* Re: git remote doesn't show remotes from .git/remotes
From: Kirill Likhodedov @ 2011-10-07 15:49 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111007150423.GA2076@sigill.intra.peff.net>


07.10.2011, в 19:04, Jeff King wrote

> I'm not sure how much we
> care, though. We haven't generated .git/remotes files in a long time,
> and this is the first notice of the bug after 3.5 years. Is this an old
> repo that has remotes, or are you wondering if you should use them in a
> new repo?
> 

No, I never came across any usages of .git/remotes.
I am writing a Git integration plugin for an IDE, and was studying documentation for git-remote where I saw this method to define remotes. 
So I've just decided to report the lack of the 'git remote' output and make sure that I didn't miss anything. And also decide if I have to support them in the product - probably, not.

I agree that the bug is not worth fixing since nobody uses .git/remotes anyway.

Thanks for the clarification.

^ permalink raw reply

* [PATCH] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-07 15:29 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian, Junio C Hamano, Todd A. Jacobs

Implement "git merge [-e|--edit]" as "git merge --no-commit && git commit"
as a convenience for the user.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Documentation/merge-options.txt |    6 ++++++
 builtin/merge.c                 |   14 ++++++++++++++
 t/t7600-merge.sh                |   15 +++++++++++++++
 3 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index b613d4ed08..6bd0b041c3 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -7,6 +7,12 @@ With --no-commit perform the merge but pretend the merge
 failed and do not autocommit, to give the user a chance to
 inspect and further tweak the merge result before committing.
 
+--edit::
+-e::
++
+	Invoke editor before committing successful merge to further
+	edit the default merge message.
+
 --ff::
 --no-ff::
 	Do not generate a merge commit if the merge resolved as
diff --git a/builtin/merge.c b/builtin/merge.c
index ee56974371..815e151487 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
 
 static int show_diffstat = 1, shortlog_len, squash;
 static int option_commit = 1, allow_fast_forward = 1;
+static int option_edit = 0;
 static int fast_forward_only;
 static int allow_trivial = 1, have_message;
 static struct strbuf merge_msg;
@@ -190,6 +191,8 @@ static struct option builtin_merge_options[] = {
 		"create a single commit instead of doing a merge"),
 	OPT_BOOLEAN(0, "commit", &option_commit,
 		"perform a commit if the merge succeeds (default)"),
+	OPT_BOOLEAN('e', "edit", &option_edit,
+		"edit message before committing"),
 	OPT_BOOLEAN(0, "ff", &allow_fast_forward,
 		"allow fast-forward (default)"),
 	OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
@@ -1092,6 +1095,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_commit = 0;
 	}
 
+	/* if not committing, edit is nonsensical */
+	if (!option_commit)
+		option_edit = 0;
+	/* if editing, invoke 'git commit -e' after successful merge */
+	if (option_edit)
+		option_commit = 0;
+
 	if (!allow_fast_forward && fast_forward_only)
 		die(_("You cannot combine --no-ff with --ff-only."));
 
@@ -1447,6 +1457,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	}
 
 	if (merge_was_ok) {
+		if (option_edit) {
+			const char *args[] = {"commit", "-e", NULL};
+			return run_command_v_opt(args, RUN_GIT_CMD);
+		}
 		fprintf(stderr, _("Automatic merge went well; "
 			"stopped before committing as requested\n"));
 		return 0;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 87aac835a1..8c6b811718 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -643,4 +643,19 @@ test_expect_success 'amending no-ff merge commit' '
 
 test_debug 'git log --graph --decorate --oneline --all'
 
+cat >editor <<\EOF
+#!/bin/sh
+# strip comments and blank lines from end of message
+sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
+EOF
+chmod 755 editor
+
+test_expect_success 'merge --no-ff --edit' '
+	git reset --hard c0 &&
+	EDITOR=./editor git merge --no-ff --edit c1 &&
+	verify_parents $c0 $c1 &&
+	git cat-file commit HEAD | sed "1,/^$/d" > actual &&
+	test_cmp actual expected
+'
+
 test_done
-- 
1.7.7.147.g00fdf

^ permalink raw reply related

* Re: git remote doesn't show remotes from .git/remotes
From: Jeff King @ 2011-10-07 15:04 UTC (permalink / raw)
  To: Kirill Likhodedov; +Cc: git
In-Reply-To: <26866FC7-4D4D-46D0-89DE-85AF459AC48C@jetbrains.com>

On Thu, Oct 06, 2011 at 07:33:23PM +0400, Kirill Likhodedov wrote:

> It seems that 'git remote' doesn't display remotes registered not in
> .git/config but in .git/remotes/.
> Is it a bug?

It seems to have been lost in 211c896 (Make git-remote a builtin,
2008-02-29).

It wouldn't be that hard to add it back in. I'm not sure how much we
care, though. We haven't generated .git/remotes files in a long time,
and this is the first notice of the bug after 3.5 years. Is this an old
repo that has remotes, or are you wondering if you should use them in a
new repo?

> Btw, are there advantages in using .git/remotes/ instead of .git/config ?

No. They were an older format, and were replaced by having the remote
defined in the configuration (and there are many things you can specify
in the config that you can't do via .git/remotes).

> If not, are there plans to remove .git/remotes/ support in future versions?

I don't think there is a specific plan. They're kept for backwards
compatibility. But really, there is no reason to be using them at all at
this point.

-Peff

^ permalink raw reply

* Re: [PATCH] show git tag output in pager
From: Matthieu Moy @ 2011-10-07 14:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Michal Vyskocil, git
In-Reply-To: <20111007144438.GA30318@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Oct 03, 2011 at 02:57:09PM +0200, Matthieu Moy wrote:
>
>> I like the try_subcommand_pager idea. Ideally, there would also be a
>> nice mechanism to set defaults for subcommands, so that "git tag
>> <whatever>" does the right thing without configuration.
>
> That's easy enough. Something like the patch below?

It may have been better with a big centralized array of configurations
like

	static struct cmd_struct commands[] = {
		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
                ...

in git.c, but if we have only a few instances of this, your system is
probably fine. I like it.

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

^ permalink raw reply

* Re: [PATCH] show git tag output in pager
From: Jeff King @ 2011-10-07 14:44 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Michal Vyskocil, git
In-Reply-To: <vpqwrcmw7ve.fsf@bauges.imag.fr>

On Mon, Oct 03, 2011 at 02:57:09PM +0200, Matthieu Moy wrote:

> I like the try_subcommand_pager idea. Ideally, there would also be a
> nice mechanism to set defaults for subcommands, so that "git tag
> <whatever>" does the right thing without configuration.

That's easy enough. Something like the patch below?

Still thoroughly untested, but it looks Obviously Correct to me. :)

---
 builtin.h     |    1 +
 builtin/tag.c |    1 +
 git.c         |   13 +++++++++++++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/builtin.h b/builtin.h
index 0e9da90..b2badf8 100644
--- a/builtin.h
+++ b/builtin.h
@@ -35,6 +35,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
 void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
 
 extern int check_pager_config(const char *cmd);
+extern void try_subcommand_pager(const char *subcommand, int fallback);
 
 extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 9d89616..c77adc4 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -152,6 +152,7 @@ static int list_tags(const char **patterns, int lines,
 	filter.lines = lines;
 	filter.with_commit = with_commit;
 
+	try_subcommand_pager("tag.list", 1);
 	for_each_tag_ref(show_reference, (void *) &filter);
 
 	return 0;
diff --git a/git.c b/git.c
index 8e34903..11ee1a8 100644
--- a/git.c
+++ b/git.c
@@ -64,6 +64,19 @@ static void commit_pager_choice(void) {
 	}
 }
 
+void try_subcommand_pager(const char *subcommand, int fallback)
+{
+	/* it's too late to turn off a running pager */
+	if (pager_in_use())
+		return;
+
+	if (use_pager == -1)
+		use_pager = check_pager_config(subcommand);
+	if (use_pager == -1)
+		use_pager = fallback;
+	commit_pager_choice();
+}
+
 static int handle_options(const char ***argv, int *argc, int *envchanged)
 {
 	const char **orig_argv = *argv;
-- 
1.7.7.rc2.7.gdfc92

^ permalink raw reply related

* Re: [PATCH v2] post-receive-email: explicitly set Content-Type header
From: Alexey Shumkin @ 2011-10-07 12:52 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Johannes Sixt, Fabian Emmes, Alexander Gerasiov, git,
	Junio C Hamano
In-Reply-To: <20111007090601.GA22541@elie.hsd1.il.comcast.net>

> Johannes Sixt wrote:
> > Am 9/20/2011 12:42, schrieb Shumkin Alexey:
> 
> >> 1. post-send-mail uses description file of a repo
> >> 2. gitweb also uses this file and AFAIK it assumes one to be in
> >> UTF-8 (I do not know whether it can be changed there but I tested
> >> gitweb once long time ago)
> >> 3. So if i18n.logoutputencoding is not UTF-8 we get a message
> >> composed with mixed encodings. This fact oblidge us to encode
> >> headers (as quoted printable at least) and synchronize body
> >> message that contain repo description (in UTF-8) and diffstat (in
> >> i18n.logoutputencoding).
> [...]
> > In this case, it may make sense to have a separate setting, but you
> > should call git like this:
> >
> >    git -c "i18n.logoutputencoding=$emailcharset" show ...
> >    git -c "i18n.logoutputencoding=$emailcharset" rev-list
> > --pretty ...
> 
> Something like this, I suppose?
> 
> This teaches post-receive-email to use plumbing where possible and to
> explicitly declare what encoding it expects output to use.  Completely
> untested --- basic sanity checking, testing, and tweaks to allow
> overriding the choice of encoding left as an exercise to the reader.
> 
> Based on patches by Gerrit Pape and Jeff King and advice from Johannes
> Sixt.
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> More background:
> http://thread.gmane.org/gmane.comp.version-control.git/124350/focus=124355
> 
>  contrib/hooks/post-receive-email |   15 +++++++++------
>  1 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/contrib/hooks/post-receive-email
> b/contrib/hooks/post-receive-email index ba077c13..bc603b02 100755
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -234,6 +234,9 @@ generate_email_header()
>  	cat <<-EOF
>  	To: $recipients
>  	Subject: ${emailprefix}$projectdesc $refname_type
> $short_refname ${change_type}d. $describe
> +	MIME-Version: 1.0
> +	Content-Type: text/plain; charset=utf-8
> +	Content-Transfer-Encoding: 8bit
>  	X-Git-Refname: $refname
>  	X-Git-Reftype: $refname_type
>  	X-Git-Oldrev: $oldrev
> @@ -462,7 +465,7 @@ generate_delete_branch_email()
>  	echo "       was  $oldrev"
>  	echo ""
>  	echo $LOGEND
> -	git show -s --pretty=oneline $oldrev
> +	git diff-tree --encoding=UTF-8 -s --pretty=oneline $oldrev
>  	echo $LOGEND
>  }
>  
> @@ -538,11 +541,11 @@ generate_atag_email()
>  		# performed on them
>  		if [ -n "$prevtag" ]; then
>  			# Show changes since the previous release
> -			git rev-list --pretty=short
> "$prevtag..$newrev" | git shortlog
> +			git shortlog --encoding=UTF-8
> "$prevtag..$newrev" else
>  			# No previous tag, show all the changes
> since time # began
> -			git rev-list --pretty=short $newrev | git
> shortlog
> +			git shortlog --encoding=UTF-8 "$newrev"
>  		fi
>  		;;
>  	*)
> @@ -562,7 +565,7 @@ generate_delete_atag_email()
>  	echo "       was  $oldrev"
>  	echo ""
>  	echo $LOGEND
> -	git show -s --pretty=oneline $oldrev
> +	git diff-tree --encoding=UTF-8 -s --pretty=oneline $oldrev
>  	echo $LOGEND
>  }
>  
> @@ -608,7 +611,7 @@ generate_general_email()
>  	echo ""
>  	if [ "$newrev_type" = "commit" ]; then
>  		echo $LOGBEGIN
> -		git show --no-color --root -s --pretty=medium $newrev
> +		git diff-tree --encoding=UTF-8 --root -s
> --pretty=oneline $newrev echo $LOGEND
>  	else
>  		# What can we do here?  The tag marks an object that
> is not @@ -627,7 +630,7 @@ generate_delete_general_email()
>  	echo "       was  $oldrev"
>  	echo ""
>  	echo $LOGEND
> -	git show -s --pretty=oneline $oldrev
> +	git diff-tree --encoding=UTF-8 -s --pretty=oneline $oldrev
>  	echo $LOGEND
>  }
>  

As I understand, this patches make email message with explicitly set
Content-Type header (it's ok) and UTF-8 encoding (that is a subject
to discuss).
My proposition was in to send email message in explicitly defined
custom encoding. Why? In development process under Windows non-UTF-8
encoding is used (cp1251 in my case). So, filenames have this encoding,
and as we know Git stores their names as is - in cp1251 - without a
conversion. And filenames are also used in diff-stat (with
core.quotepath= false, BTW, I did not take into account this config)
without any conversion. So if we'll make all text in UTF-8 but
filenames are still non-UTF-8, email would look corrupted.

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Michael J Gruber @ 2011-10-07 12:14 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <20111007100646.GA23193@elie.hsd1.il.comcast.net>

Jonathan Nieder venit, vidit, dixit 07.10.2011 12:06:
> Michael J Gruber wrote:
> 
>> But my main point here is that we should discuss the pros and cons of
>> each approach in context (the context of the original thread), and I
>> haven't heard many pros and cons for either.
> 
> Ok, here are some thoughts (you can file them under the "pro" or "con"
> column according to your taste):
> 
> Branch names are local.

You can pull a branch only if you know its name, or using a wildcard
refspec. In both cases you know the translation from what is local on
the other side to your side. In the vast majority it will be the simple
refs/heads/foo -> refs/remotes/bar/foo mapping.

> The same tip commit can belong to multiple branches, which would make
> using the tip commit as a key for the branch description problematic.

Sure, tip commit is a no-go.

That's why I propose notes tacked onto refnames.

> I don't think git notes are a good fit for this purpose.

Why not?

I really haven't seen any convincing argument against yet. The details
(note attached to refname object, or literal refanmes in the notes tree
as per Jeff) should be discussed further, of course, but if branch
descriptions aren't "notesy" then I don't know what is.

Alternatively, one could store the description in a blob and refer to
that directly, of course. I.e., have

refs/description/foo

point to a blob whose content is the description of the ref

ref/foo

That would be unversioned, and one could decide more easily which
descriptions to share. (A notes tree you either push or don't.)

But it still has the advantage of being easily pushable and shareable.
Heck, config is config and not metadata ;)

> I personally would prefer my branch descriptions to be non-versioned,
> though I realize that is a matter of taste.

Do you prefer you commit notes to be non-versioned? Probably, but still
they are, and you don't need to care. Just don't run log on your notes ref.

> Some other information about a branch (such as which upstream branch
> it pulls from) is stored in the [branch "<branchname>"] section of the
> git configuration, so it makes a certain kind of sense to put the
> branch description there, too.  On the other hand, the possibility of
> a [branch "<branchname>"] section in ~/.gitconfig has always been
> strange, and this is no exception.

Note that most use cases for branch descriptions that have been
described so far (format-patch cover-letter, merge-msg, pull-request
message) are about distributing that information. Not very local.

>> I'd be surprised if we changed the protocol just to be able to share
>> some descriptions, when we have everything we need for sharing refs.
> 
> The impact of such a protocol change would not have to be as dramatic
> as you seem to be suggesting.  Virtual hosts and peeled ref values
> were added as backward-compatible extensions to the reference
> discovery protocol (see Documentation/technical/pack-protocol.txt)
> which can be taken as examples for inspiration.

I don't see how a protocol change could solve the perceived issue with
localality of name, and indeed what it could solve that can't be solved
with existing methods.

> Here's some discussion of the implementation based on branchname-keyed
> notes that you mentioned [1].  It is nice to have multiple designs
> available for trying out, and I hope experience using each reveals
> potential improvements in the other.
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/181953/focus=182000

Funny to point me at a thread I participated in, when I'm saying this
thread should have continued there ;)

Michael

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Nguyen Thai Ngoc Duy @ 2011-10-07 11:18 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Robin H. Johnson, Junio C Hamano
In-Reply-To: <4E8EBAFE.8020805@drmicha.warpmail.net>

On Fri, Oct 7, 2011 at 7:40 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
>> 3. If I have prepared a series on a local branch, and I want to sign all
>>    of them, is this a variant of rebase or?
>
> If you really want to sign all you can rebase-i and use "exec" to do
> that automatically, but there's no point: signing the top-most commit
> serves the same purpose.

I think Gentoo wants identity, who (typically a Gentoo dev) signs this
particular commit and takes responsibility for making sure the commit
follows all kinds of Gentoo requirements. Commits can be passed
around, author and committer are not a reliable source.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Jonathan Nieder @ 2011-10-07 10:06 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <4E8ECA25.205@drmicha.warpmail.net>

Michael J Gruber wrote:

> But my main point here is that we should discuss the pros and cons of
> each approach in context (the context of the original thread), and I
> haven't heard many pros and cons for either.

Ok, here are some thoughts (you can file them under the "pro" or "con"
column according to your taste):

Branch names are local.

The same tip commit can belong to multiple branches, which would make
using the tip commit as a key for the branch description problematic.
I don't think git notes are a good fit for this purpose.

I personally would prefer my branch descriptions to be non-versioned,
though I realize that is a matter of taste.

Some other information about a branch (such as which upstream branch
it pulls from) is stored in the [branch "<branchname>"] section of the
git configuration, so it makes a certain kind of sense to put the
branch description there, too.  On the other hand, the possibility of
a [branch "<branchname>"] section in ~/.gitconfig has always been
strange, and this is no exception.

> I'd be surprised if we changed the protocol just to be able to share
> some descriptions, when we have everything we need for sharing refs.

The impact of such a protocol change would not have to be as dramatic
as you seem to be suggesting.  Virtual hosts and peeled ref values
were added as backward-compatible extensions to the reference
discovery protocol (see Documentation/technical/pack-protocol.txt)
which can be taken as examples for inspiration.

Here's some discussion of the implementation based on branchname-keyed
notes that you mentioned [1].  It is nice to have multiple designs
available for trying out, and I hope experience using each reveals
potential improvements in the other.

[1] http://thread.gmane.org/gmane.comp.version-control.git/181953/focus=182000

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Michael J Gruber @ 2011-10-07  9:45 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20111007091636.GA22822@elie.hsd1.il.comcast.net>

Jonathan Nieder venit, vidit, dixit 07.10.2011 11:16:
> Michael J Gruber wrote:
> 
>> config based is so non-distributed, local in nature.
> 
> I don't follow.  Wouldn't a protocol change be enough to fix that, by
> sharing branch descriptions analagously to how refs themselves are
> shared?

I'd be surprised if we changed the protocol just to be able to share
some descriptions, when we have everything we need for sharing refs.
Also note that config is non-versioned etc.

But my main point here is that we should discuss the pros and cons of
each approach in context (the context of the original thread), and I
haven't heard many pros and cons for either.

Michael

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Jonathan Nieder @ 2011-10-07  9:16 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git
In-Reply-To: <4E8EBDA7.2040007@drmicha.warpmail.net>

Michael J Gruber wrote:

> config based is so non-distributed, local in nature.

I don't follow.  Wouldn't a protocol change be enough to fix that, by
sharing branch descriptions analagously to how refs themselves are
shared?

^ permalink raw reply

* Re: [PATCH v2] post-receive-email: explicitly set Content-Type header
From: Jonathan Nieder @ 2011-10-07  9:06 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: zapped, Fabian Emmes, Alexander Gerasiov, git, Junio C Hamano
In-Reply-To: <4E7874B9.2060909@viscovery.net>

Johannes Sixt wrote:
> Am 9/20/2011 12:42, schrieb Shumkin Alexey:

>> 1. post-send-mail uses description file of a repo
>> 2. gitweb also uses this file and AFAIK it assumes one to be in UTF-8
>>   (I do not know whether it can be changed there but I tested gitweb once long
>>     time ago)
>> 3. So if i18n.logoutputencoding is not UTF-8 we get a message composed
>> 	with mixed encodings. This fact oblidge us to encode headers
>> 	(as quoted printable at least) and synchronize body message that contain
>> 	repo description (in UTF-8) and diffstat (in i18n.logoutputencoding).
[...]
> In this case, it may make sense to have a separate setting, but you should
> call git like this:
>
>    git -c "i18n.logoutputencoding=$emailcharset" show ...
>    git -c "i18n.logoutputencoding=$emailcharset" rev-list --pretty ...

Something like this, I suppose?

This teaches post-receive-email to use plumbing where possible and to
explicitly declare what encoding it expects output to use.  Completely
untested --- basic sanity checking, testing, and tweaks to allow
overriding the choice of encoding left as an exercise to the reader.

Based on patches by Gerrit Pape and Jeff King and advice from Johannes
Sixt.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
More background:
http://thread.gmane.org/gmane.comp.version-control.git/124350/focus=124355

 contrib/hooks/post-receive-email |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index ba077c13..bc603b02 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -234,6 +234,9 @@ generate_email_header()
 	cat <<-EOF
 	To: $recipients
 	Subject: ${emailprefix}$projectdesc $refname_type $short_refname ${change_type}d. $describe
+	MIME-Version: 1.0
+	Content-Type: text/plain; charset=utf-8
+	Content-Transfer-Encoding: 8bit
 	X-Git-Refname: $refname
 	X-Git-Reftype: $refname_type
 	X-Git-Oldrev: $oldrev
@@ -462,7 +465,7 @@ generate_delete_branch_email()
 	echo "       was  $oldrev"
 	echo ""
 	echo $LOGEND
-	git show -s --pretty=oneline $oldrev
+	git diff-tree --encoding=UTF-8 -s --pretty=oneline $oldrev
 	echo $LOGEND
 }
 
@@ -538,11 +541,11 @@ generate_atag_email()
 		# performed on them
 		if [ -n "$prevtag" ]; then
 			# Show changes since the previous release
-			git rev-list --pretty=short "$prevtag..$newrev" | git shortlog
+			git shortlog --encoding=UTF-8 "$prevtag..$newrev"
 		else
 			# No previous tag, show all the changes since time
 			# began
-			git rev-list --pretty=short $newrev | git shortlog
+			git shortlog --encoding=UTF-8 "$newrev"
 		fi
 		;;
 	*)
@@ -562,7 +565,7 @@ generate_delete_atag_email()
 	echo "       was  $oldrev"
 	echo ""
 	echo $LOGEND
-	git show -s --pretty=oneline $oldrev
+	git diff-tree --encoding=UTF-8 -s --pretty=oneline $oldrev
 	echo $LOGEND
 }
 
@@ -608,7 +611,7 @@ generate_general_email()
 	echo ""
 	if [ "$newrev_type" = "commit" ]; then
 		echo $LOGBEGIN
-		git show --no-color --root -s --pretty=medium $newrev
+		git diff-tree --encoding=UTF-8 --root -s --pretty=oneline $newrev
 		echo $LOGEND
 	else
 		# What can we do here?  The tag marks an object that is not
@@ -627,7 +630,7 @@ generate_delete_general_email()
 	echo "       was  $oldrev"
 	echo ""
 	echo $LOGEND
-	git show -s --pretty=oneline $oldrev
+	git diff-tree --encoding=UTF-8 -s --pretty=oneline $oldrev
 	echo $LOGEND
 }
 
-- 
1.7.7.rc1

^ permalink raw reply related


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