Git development
 help / color / mirror / Atom feed
* Re: [PATCH] commit: teach --gpg-sign option
From: Robin H. Johnson @ 2011-10-06 22:24 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <7vaa9f3pk8.fsf@alter.siamese.dyndns.org>

On Wed, Oct 05, 2011 at 05:56:55PM -0700,  Junio C Hamano wrote:
> And this uses the gpg-interface.[ch] to allow signing the commit, i.e.
> 
>     $ git commit --gpg-sign -m foo
>     You need a passphrase to unlock the secret key for
>     user: "Junio C Hamano <gitster@pobox.com>"
>     4096-bit RSA key, ID 96AFE6CB, created 2011-10-03 (main key ID 713660A7)
> 
>     [master 8457d13] foo
>      1 files changed, 1 insertions(+), 0 deletions(-)
I like it, but I have a couple of questions: 
1. Are the sig lines used in computed SHA1/commitid of a given commit (I
   see examples w/ --amend and that would usually change the SHA1)?
2. Can we allow more than one person sign a commit?
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?

I think this isn't a replacement for push certificates, but has value in
itself. It's certainly provides better integration than the
signature-in-note variants.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

^ permalink raw reply

* Re: Git Bug report
From: Aaron Schrab @ 2011-10-06 22:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Phil Hord, SZEDER Gábor, git, Fredrik Gustafsson,
	Federico Lucifredi
In-Reply-To: <7vy5wy145q.fsf@alter.siamese.dyndns.org>

At 09:22 -0700 06 Oct 2011, Junio C Hamano <gitster@pobox.com> wrote:
>Yeah, after thinking about it a bit more, whenever we see ".git" during
>the upward discovery process, we should always warn if we know it is _not_
>a GIT_DIR before looking for another ".git" at higher levels, as anything
>in that directory cannot be added. If we cannot tell if it is or is not
>a GIT_DIR, we should error out---the reason we cannot tell most likely is
>because we cannot read it, and such a file, if it is not a GIT_DIR, cannot
>be tracked in the real GIT_DIR at a higher level, and if it is a GIT_DIR,
>we cannot use it to record updates or inspect existing history.

Yes, I think that sounds like a good idea.  That should also solve a 
related problem that I noticed while checking out the current behaviour.

Currently if the .git directory of a submodule is inaccessible running 
`git status` from anywhere in the parent repository (including within 
the submodule) will cause git to recursively call itself until enough 
resources are used to prevent further forking.  I then tried this with 
the patch from earlier in the thread applied, but with the call to 
error() changed to call die() instead.  With that change it quickly 
failed with a useful error message.

^ permalink raw reply

* Re: [PATCH 1/3] completion: unite --reuse-message and --reedit-message handling
From: Junio C Hamano @ 2011-10-06 23:14 UTC (permalink / raw)
  To: Teemu Matilainen; +Cc: git, Shawn O. Pearce
In-Reply-To: <1317926431-609-1-git-send-email-teemu.matilainen@iki.fi>

All three patches make sense to me. Thanks.

^ permalink raw reply

* Re: Prompt for merge message?
From: Junio C Hamano @ 2011-10-06 23:17 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Todd A. Jacobs, git
In-Reply-To: <CAJo=hJvFytscxyx2z+Fdw9E1DS02wSXgoE3SHkxKq2OYOMQHgQ@mail.gmail.com>

Shawn Pearce <spearce@spearce.org> writes:

> to git merge? I know the reason we don't want to do it all of the time
> is because git merge is already used in a lot of scripts. But how many
> of those are running with an active terminal on all 3 standard fds
> when it runs git merge?

Ninety four?

$ git grep -l 'git merge' -- 't/t[0-9][0-9][0-9][0-9]-*.sh' | wc -l

^ permalink raw reply

* [PATCH] git-svn: Allow certain refs to be ignored
From: Michael Olson @ 2011-10-07  0:41 UTC (permalink / raw)
  To: git

Implement a new --ignore-refs option which specifies a regex of refs
to ignore while importing svn history.

This is a useful supplement to the --ignore-paths option, as that
option only operates on the contents of branches and tags, not the
branches and tags themselves.

Signed-off-by: Michael Olson <mwolson@gnu.org>
---
Re-sent by request of Piotr Krukowiecki.  This is against v1.7.4.1,
and I've been using it stably for a while.

 git-svn.perl |   38 +++++++++++++++++++++++++++++++++-----
 1 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 177dd25..541fa2d 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -90,7 +90,8 @@ $_q ||= 0;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
                     'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache,
-                    'ignore-paths=s' => \$SVN::Git::Fetcher::_ignore_regex );
+                    'ignore-paths=s' => \$SVN::Git::Fetcher::_ignore_regex,
+                    'ignore-refs=s' => \$Git::SVN::Ra::_ignore_refs_regex );
 my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
 		'authors-file|A=s' => \$_authors,
 		'authors-prog=s' => \$_authors_prog,
@@ -380,9 +381,12 @@ sub do_git_init_db {
 		command_noisy('config', "$pfx.$i", $icv{$i});
 		$set = $i;
 	}
-	my $ignore_regex = \$SVN::Git::Fetcher::_ignore_regex;
-	command_noisy('config', "$pfx.ignore-paths", $$ignore_regex)
-		if defined $$ignore_regex;
+	my $ignore_paths_regex = \$SVN::Git::Fetcher::_ignore_regex;
+	command_noisy('config', "$pfx.ignore-paths", $$ignore_paths_regex)
+		if defined $$ignore_paths_regex;
+	my $ignore_refs_regex = \$Git::SVN::Ra::_ignore_refs_regex;
+	command_noisy('config', "$pfx.ignore-refs", $$ignore_refs_regex)
+		if defined $$ignore_refs_regex;
 }

 sub init_subdir {
@@ -1831,6 +1835,8 @@ sub read_all_remotes {
 			$r->{$1}->{svm} = {};
 		} elsif (m!^(.+)\.url=\s*(.*)\s*$!) {
 			$r->{$1}->{url} = $2;
+		} elsif (m!^(.+)\.ignore-refs=\s*(.*)\s*$!) {
+			$r->{$1}->{ignore_refs_regex} = $2;
 		} elsif (m!^(.+)\.(branches|tags)=$svn_refspec$!) {
 			my ($remote, $t, $local_ref, $remote_ref) =
 			                                     ($1, $2, $3, $4);
@@ -1867,6 +1873,16 @@ sub read_all_remotes {
 		}
 	} keys %$r;

+	foreach my $remote (keys %$r) {
+		foreach ( grep { defined $_ }
+			  map { $r->{$remote}->{$_} } qw(branches tags) ) {
+			foreach my $rs ( @$_ ) {
+				$rs->{ignore_refs_regex} =
+				    $r->{$remote}->{ignore_refs_regex};
+			}
+		}
+	}
+
 	$r;
 }

@@ -4876,7 +4892,7 @@ sub apply_diff {
 }

 package Git::SVN::Ra;
-use vars qw/@ISA $config_dir $_log_window_size/;
+use vars qw/@ISA $config_dir $_ignore_refs_regex $_log_window_size/;
 use strict;
 use warnings;
 my ($ra_invalid, $can_do_switch, %ignored_err, $RA);
@@ -5334,6 +5350,17 @@ sub get_dir_globbed {
 	@finalents;
 }

+# return value: 0 -- don't ignore, 1 -- ignore
+sub is_ref_ignored {
+	my ($g, $p) = @_;
+	my $refname = $g->{ref}->full_path($p);
+	return 1 if defined($g->{ignore_refs_regex}) &&
+	            $refname =~ m!$g->{ignore_refs_regex}!;
+	return 0 unless defined($_ignore_refs_regex);
+	return 1 if $refname =~ m!$_ignore_refs_regex!o;
+	return 0;
+}
+
 sub match_globs {
 	my ($self, $exists, $paths, $globs, $r) = @_;

@@ -5370,6 +5397,7 @@ sub match_globs {
 			next unless /$g->{path}->{regex}/;
 			my $p = $1;
 			my $pathname = $g->{path}->full_path($p);
+			next if is_ref_ignored($g, $p);
 			next if $exists->{$pathname};
 			next if ($self->check_path($pathname, $r) !=
 			         $SVN::Node::dir);
-- 
1.7.4.1

^ permalink raw reply related

* Re: git-svn: ignore-paths not enough (not ignoring refs?)
From: Michael Olson @ 2011-10-07  0:41 UTC (permalink / raw)
  To: Piotr Krukowiecki; +Cc: Eric Wong, Git Mailing List
In-Reply-To: <CAA01CsqEdoqSRrPDTrfOQPS7q-1tVALE_5vjeLUAeh0iXZi3_A@mail.gmail.com>

I've resent my patch.  I've been using it with v1.7.4.1.

On Thu, Oct 6, 2011 at 1:43 AM, Piotr Krukowiecki
<piotr.krukowiecki@gmail.com> wrote:
> Hi,
>
> I'm using --ignore-paths options but some paths which should be
> ignored (they math the regexp) are not ignored. I suspect this is
> because of http://thread.gmane.org/gmane.comp.version-control.git/145398
>
> It seems the patch from that url was never accepted to git. It does
> not apply anymore too. Is it possible to update the patch? From the
> discussion the patch looked ok (with exception to git-svn dying if a
> parent ref didn't exist :) )
>
>
> Here's my git-svn clone log, with some print lines added to is_path_ignored():
>
> $ git svn clone -s --no-follow-parent  --ignore-paths='$REGEXP -r 1230:1240 $URL
> [...]
> is_path_ignored: 'xtest/tags/rel1'  # this matches the regexp and
> should be ignored
> is_path_ignored: 'xtest/tags/rel1/common' # more paths which should be
> ignored follow
> [...]
> r1237 = c660a7e6ad81cafa0a64a7ec85a3e23f0c02bc09
> (refs/remotes/tags/rel1) # but it is not!
>
> # I can see the tag:
> $ git branch -a
>  remotes/tags/rel1
>
> # There are no changes in this change:
> $ git show -p c660a7e6ad81cafa0a64a7ec85a3e23f0c02bc09
> [...]
>    git-svn-id: $URL/rel1@1237 3e523551-a86b-4873-bcb6-76fcd93a4c5c
>
> # Under svn the r1237 is following:
> $ svn log -r 1237 -v $URL
> ------------------------------------------------------------------------
> r1237 | ...
> Changed paths:
>   A /xtest/tags/rel1/common (from /xtest/branches/xtest44/common:1235)
>
> # The xtest/branches/xtest44/common does not match the ignore-paths
> and is not ignored.
>
>
> --
> Piotr Krukowiecki
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Michael Olson  |  http://mwolson.org/

^ permalink raw reply

* Fix another file leak
From: Chris Wilson @ 2011-10-07  1:41 UTC (permalink / raw)
  To: git

Hi,

Vigilant Sentry (our C/C++ static analysis tool) found that
commit 6d4bb383, added a file leak to builtin/fetch.c.

static int store_updated_refs(...
{  
    FILE *fp;
    ...
    fp = fopen(filename, "a");
    if (!fp)
        return error(_("cannot open %s: %s\n"), filename, strerror(errno));
    ....

    if (check_everything_connected(iterate_ref_map, 0, &rm))
        return error(_("%s did not send all necessary objects\n"), url);

Please close the file handle before returning from the function.

Thanks,
Chris

-- 
Chris Wilson
http://vigilantsw.com/
Vigilant Software, LLC

^ permalink raw reply

* Re: [RFC/PATCH]: reverse bisect v 2.0
From: Andrew Ardill @ 2011-10-07  1:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Christian Couder, Jeff King, Michal Vyskocil, git,
	Sverre Rabbelier, Johannes Sixt
In-Reply-To: <7v62k4ban7.fsf@alter.siamese.dyndns.org>

Hi, I have given this some thought and have a slightly different
proposal for the options to use. I have written a commit message that
I think explains the need for the improvement, and justifies my
proposal. The options I propose are '--intoduced' and '--removed'.

--->8---

Bisect is used to look for a commit that causes a specific change.
Such a change can be classified by the user as either introducing
something, or removing something, and this distinction is arbitrary. A
change could be said to introduce a bug fix, or remove a bug - both
formulations have inherently the same meaning, but search behaviour
changes depending on which one we use.
Suppose I want to find the commit that removed a bug. If I examine a
snapshot and it contains the bug, the bug has not yet been *removed*
and I must look in the future for its removal. Conversely, if I
examine a snapshot and the bug fix exists, the bug fix must have been
*introduced* at some earlier point and so I must search backwards.

Confusion exists at this point because at each verification step a
question like 'Does this snapshot have X?' is asked, when what we
eventually want to know is 'When was X introduced?' or 'When was X
removed?'. Previously there has been no way to specify if we are
looking for X being introduced or removed; it was assumed that we were
looking for when something was introduced, for example when a bug was
introduced.

Teach git bisect if we are looking for when something was introduced
or when something was removed.

Examples.
Search for a feature add:
   $ git bisect start --introduced='feature: git frotz says xyzzy' v0.99 master
   Bisecting: 171 revisions left to test after this (roughly 8 steps)
   $ ... build and then test ...
   $ git bisect tested
   Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes
# already added, look backwards

Search for a feature regression:
   $ git bisect start --removed='feature: git frotz says xyzzy' v0.99 master
   Bisecting: 171 revisions left to test after this (roughly 8 steps)
   $ ... build and then test ...
   $ git bisect tested
   Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes
# not removed yet, look forwards

--->8---

Regards,

Andrew Ardill

^ permalink raw reply

* Re: 66 patches and counting
From: Martin Fick @ 2011-10-07  1:59 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git
In-Reply-To: <201110061616.39381.mfick@codeaurora.org>

On Thursday, October 06, 2011 04:16:39 pm Martin Fick wrote:
> On Wednesday, October 05, 2011 03:29:57 pm Michael
> Haggerty
> > [1] hierarchical-refs at
> > git://github.com/mhagger/git.git
> 
> I downloaded your patch series and tested it on my repos.
> 
>  * full fetch changes                  (> 1 hour) (bad!)

I bisected this problem, it was introduced in this commit:


  commit e12ce45b4f1bd8ed6652a742b7e6cf6f101b3604
  Author: Michael Haggerty <mhagger@alum.mit.edu>
  Date:   Wed Oct 5 11:30:06 2011 +0200

    Store references hierarchically
    
    This slightly changes the order of iteration over 
references; now
    references are strictly sorted componentwise rather than 
as
    "/"-containing strings as before.  For example, 
"subspace/one" now
    sorts before "subspace-x", whereas before the order was 
reversed.
    
    Tweak a test case to accept the new ordering.



Up until that point, the fetch looks pretty good,

-Martin

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

^ permalink raw reply

* Re: Prompt for merge message?
From: Todd A. Jacobs @ 2011-10-07  1:15 UTC (permalink / raw)
  To: git
In-Reply-To: <7vsjn5ye0x.fsf@alter.siamese.dyndns.org>

On Oct 6, 6:02 pm, Junio C Hamano <gits...@pobox.com> wrote:
> Others commented on the current practices and gave their own useful tips
> already, but an additional hint is to name your branch more sensibly, so
> that you do not feel it is useless to record it in the history.

While I can see your point of view, I don't think that it fits every
work-flow. In particular, if you want to maintain a linear history on
the master branch but record the completion of a feature as a commit
object separate from the last patch in a series, you have two choices
at the moment:

1. A fast-forward merge, followed by an empty commit with --allow-
empty. However, the empty commit seems to get discarded on merges, or
finds other ways to disappear outside of the feature branch on which
it was created. It probably isn't a bug, but it can be very surprising
to create an empty commit object on a feature branch, and then have it
disappear when you merge to master.

2. A merge commit with an explicit merge message, which is what we've
been talking about in this thread.

The point of the exercise is (mostly) to integrate with issue trackers
like Pivotal or GitHub, where a commit with certain keywords can
integrate with ticket history. But, IMHO, it doesn't necessarily make
sense that any given patch closes a ticket; sometimes they do, but
sometimes I really prefer a standalone commit to essentially say "and
now the ticket is really, really done."

The fact that there have been so many useful suggestions about how to
work around this issue seems to imply that it isn't a low bus-factor
issue. My personal vote is for your suggestion of an [-e|--edit] flag
for the merge; that would make it (mostly) consistent with commit's
behavior. I can certainly see that in the common case the default
merge message is the right thing to do, but I really do feel that
there ought to be *some* flag to allow a visual editor to edit the
merge message without having to amend the merge commit or merge
without a commit just so that the next commit will invoke the editor.

Ultimately, I guess what I'm really agitating for is just an editor
option for merge commits. If you take work-flow out of the equation,
isn't there still a case for easily-editable merge messages?

^ permalink raw reply

* Re: Prompt for merge message?
From: Junio C Hamano @ 2011-10-07  3:07 UTC (permalink / raw)
  To: Todd A. Jacobs; +Cc: git
In-Reply-To: <403e37d1-bdd3-46fc-9a9a-e8aab3a2d3ba@f6g2000vbm.googlegroups.com>

"Todd A. Jacobs" <nospam+listmail@codegnome.org> writes:

> On Oct 6, 6:02 pm, Junio C Hamano <gits...@pobox.com> wrote:
>> Others commented on the current practices and gave their own useful tips
>> already, but an additional hint is to name your branch more sensibly, so
>> that you do not feel it is useless to record it in the history.
> ...
> Ultimately, I guess what I'm really agitating for is just an editor
> option for merge commits. If you take work-flow out of the equation,
> isn't there still a case for easily-editable merge messages?

I do not see any reason for you to be agitated. All you need to do is to
read beyond what you chose to quote from my message, read that part before
omitting from your quote ;-).

^ permalink raw reply

* Re: 66 patches and counting
From: Michael Haggerty @ 2011-10-07  3:14 UTC (permalink / raw)
  To: Martin Fick; +Cc: git
In-Reply-To: <201110061616.39381.mfick@codeaurora.org>

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

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.

In the future, please tell me the SHA1 of any versions that you test, as
I am still frequently non-ff updating the hierarchical-refs branch.

> Here are the best timings for all the good patches that 
> others have submitted to fix many of the previous problems I 
> brought up:

What are, in your measurements, the "good patches" that you consider
contenders performance-wise?  (I've lost track because there have been
so many suggestions in this area.)

It would be great if you would serve as a kind of benchmarking
referee/clearinghouse for the various suggested patches.  I have been
benchmarking with some rough scripts that I wrote [1]; the current
status is appended below (the numbers are clock times in seconds).  FWIW
the attached output was generated using roughly the following commands:

t/make-refperf-repo --refs=10000 --commits=20000
cp t/refperf-many .
# Adjust REFPERF_BRANCH in ./refperf-many:
$EDITOR ./refperf-many
revs="v1.7.6 v1.7.7 origin/master origin/hierarchical-refs^^
origin/hierarchical-refs^ origin/hierarchical-refs origin/master"
./refperf-many $revs
t/refperf-summary $revs >refperf-summary.out

See the comments at the top of the scripts for more details.  Please
suggest more tests to be added to t/refperf!

> It would be nice if you could rebase your work on top of 
> some of the other patches also so that I could see those 
> results. I might give that a try if I have the time and it 
> is easy (or I might rebase those patches on yours).

I believe that at least some of the other patches will be superseded by
mine.  When I get my patch series done I will look into it in more detail...

Michael

[1] Branch "refperf" at git://github.com/mhagger/git.git

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

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

===================================  =======  =======  =======  =======  =======  =======
Test name                                [0]      [1]      [2]      [3]      [4]      [5]
===================================  =======  =======  =======  =======  =======  =======
branch-loose-cold                       5.84     5.38     6.05     0.53     0.47     0.56
branch-loose-warm                       0.12     0.11     0.13     0.00     0.02     0.03
for-each-ref-loose-cold                 6.83     6.45     6.13     8.21     8.69     8.28
for-each-ref-loose-warm                 0.25     0.25     0.25     0.84     0.84     0.84
checkout-loose-cold                     4.55     5.08     5.76     0.73     0.60     0.68
checkout-loose-warm                     0.11     0.11     0.12     0.03     0.02     0.04
checkout-orphan-loose                   0.10     0.10     0.11     0.01     0.02     0.01
checkout-from-detached-loose            0.97     1.05     0.95     7.80     8.27     8.37
branch-contains-loose-cold             14.98    15.44    18.54    16.46    16.91    17.54
branch-contains-loose-warm              9.44     8.97    10.15     9.60     9.58     9.71
pack-refs-loose                         0.97     1.00     1.37     1.69     1.62     1.65
branch-packed-cold                      0.49     0.63     0.81     1.38     1.37     1.45
branch-packed-warm                      0.02     0.02     0.05     0.77     0.74     0.80
for-each-ref-packed-cold                1.18     0.97     1.56     1.33     2.03     1.51
for-each-ref-packed-warm                0.15     0.15     0.16     0.52     0.52     0.50
checkout-packed-cold                    0.84     0.60     0.70     0.96     0.94     1.28
checkout-packed-warm                    0.02     0.09     0.04     0.41     0.41     0.44
checkout-orphan-packed                  0.01     0.02     0.10     0.38     0.39     0.42
checkout-from-detached-packed           6.94     7.35     9.18     1.31     1.35     1.45
branch-contains-packed-cold            11.19    10.81    12.35    10.91    10.09    10.16
branch-contains-packed-warm            10.24     9.90    11.19     9.13     9.07     9.13
clone-loose-cold                       93.77    90.65    98.07    99.03   103.72   101.85
clone-loose-warm                        3.12     3.19     3.24     3.56     3.60     3.50
fetch-nothing-loose                     0.85     0.84     0.87     1.20     1.18     1.21
pack-refs                               0.12     0.13     0.14     0.51     0.54     0.51
fetch-nothing-packed                    0.85     0.84     0.86     1.20     1.19     1.20
clone-packed-cold                       2.17     2.38     2.50     2.28     2.46     2.34
clone-packed-warm                       0.32     0.34     0.39     0.40     0.38     0.30
fetch-everything-cold                  92.37    95.96   102.20   100.80   104.72   106.85
fetch-everything-warm                   5.55     5.65     5.53     6.26     6.27     6.35
===================================  =======  =======  =======  =======  =======  =======


[0] v1.7.6 (refperf.times.d78c84e8698e750139667bc724b08eb34e795b65)
[1] v1.7.7 (refperf.times.a258e475eb74e183e9e68ca30e32c5253081356d)
[2] origin/master (refperf.times.27897d25f1b36d400b82b655701b87fd205dbc2f)
[3] origin/hierarchical-refs^^ (refperf.times.eabcf1ed884d95878f12ca5ea32a3e20c70194f2)
[4] origin/hierarchical-refs^ (refperf.times.be9118234275c4fa6002ef97d989b0d28c94c0bd)
[5] origin/hierarchical-refs (refperf.times.74d1ae38f70f4f97745caf235ef3f34b3e326ad4)


^ permalink raw reply

* [PATCH] fetch: plug two leaks on error exit in store_updated_refs
From: René Scharfe @ 2011-10-07  6:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: git, Junio C Hamano
In-Reply-To: <20111007014136.GB10839@localhost>

Close FETCH_HEAD and release the string url even if we have to leave the
function store_updated_refs() early.

Reported-by: Chris Wilson <cwilson@vigilantsw.com>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin/fetch.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7a4e41c..79db796 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -379,8 +379,12 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		url = xstrdup("foreign");
 
 	rm = ref_map;
-	if (check_everything_connected(iterate_ref_map, 0, &rm))
-		return error(_("%s did not send all necessary objects\n"), url);
+	if (check_everything_connected(iterate_ref_map, 0, &rm)) {
+		error(_("%s did not send all necessary objects\n"), url);
+		free(url);
+		fclose(fp);
+		return -1;
+	}
 
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
-- 
1.7.7

^ permalink raw reply related

* Re: Pull --rebase looses merge information
From: Philippe Vaucher @ 2011-10-07  6:46 UTC (permalink / raw)
  To: in-gitvger; +Cc: Duane Murphy, git
In-Reply-To: <201110062031.p96KVvsv018248@no.baka.org>

> I personally believe all pull should be --rebase, all merges should be
> --no-ff, and all rebases should be -p.  At least by default.  But that
> is just me.

+1

Philippe

^ permalink raw reply

* Re: [PATCH] fetch: plug two leaks on error exit in store_updated_refs
From: Tay Ray Chuan @ 2011-10-07  6:49 UTC (permalink / raw)
  To: René Scharfe; +Cc: Chris Wilson, git, Junio C Hamano
In-Reply-To: <4E8E98A7.8010008@lsrfire.ath.cx>

On Fri, Oct 7, 2011 at 2:13 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 7a4e41c..79db796 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -379,8 +379,12 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>                url = xstrdup("foreign");
>
>        rm = ref_map;
> -       if (check_everything_connected(iterate_ref_map, 0, &rm))
> -               return error(_("%s did not send all necessary objects\n"), url);
> +       if (check_everything_connected(iterate_ref_map, 0, &rm)) {
> +               error(_("%s did not send all necessary objects\n"), url);
> +               free(url);
> +               fclose(fp);
> +               return -1;
> +       }
>
>        for (rm = ref_map; rm; rm = rm->next) {
>                struct ref *ref = NULL;
> --
> 1.7.7

How about reusing the function's cleanup calls, like this?

-- >8 --
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc254b6..56267c4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -423,8 +423,10 @@ static int store_updated_refs(const char
*raw_url, const char *remote_name,
 	else
 		url = xstrdup("foreign");

-	if (check_everything_connected(ref_map, 0))
-		return error(_("%s did not send all necessary objects\n"), url);
+	if (check_everything_connected(ref_map, 0)) {
+		rc = error(_("%s did not send all necessary objects\n"), url);
+		goto abort;
+	}

 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
@@ -506,12 +508,15 @@ static int store_updated_refs(const char
*raw_url, const char *remote_name,
 				fprintf(stderr, " %s\n", note);
 		}
 	}
-	free(url);
-	fclose(fp);
+
 	if (rc & STORE_REF_ERROR_DF_CONFLICT)
 		error(_("some local refs could not be updated; try running\n"
 		      " 'git remote prune %s' to remove any old, conflicting "
 		      "branches"), remote_name);
+
+abort:
+	free(url);
+	fclose(fp);
 	return rc;
 }

--

-- 
Cheers,
Ray Chuan

^ permalink raw reply related

* [PATCH] fmt-merge-msg: use branch.$name.description
From: Junio C Hamano @ 2011-10-07  6:57 UTC (permalink / raw)
  To: git

This teaches "merge --log" and fmt-merge-msg to use branch description
information when merging a local topic branch into the mainline. The
description goes between the branch name label and the list of commit
titles.

The refactoring to share the common configuration parsing between
merge and fmt-merge-msg needs to be made into a separate patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This is a follow-up to the branch.$name.description series that has
   been queued in 'pu' (jc/request-pull-show-head-4). The two commands
   join the family of commands that are aware of the branch description,
   i.e. format-patch (cover letter), request-pull.

 Makefile                |    1 +
 builtin/fmt-merge-msg.c |   63 ++++++++++++++++++++++++++++++++++++++--------
 builtin/merge.c         |   16 +++++------
 fmt-merge-msg.h         |    6 ++++
 4 files changed, 66 insertions(+), 20 deletions(-)
 create mode 100644 fmt-merge-msg.h

diff --git a/Makefile b/Makefile
index 8d6d451..b499049 100644
--- a/Makefile
+++ b/Makefile
@@ -527,6 +527,7 @@ LIB_H += diffcore.h
 LIB_H += diff.h
 LIB_H += dir.h
 LIB_H += exec_cmd.h
+LIB_H += fmt-merge-msg.h
 LIB_H += fsck.h
 LIB_H += gettext.h
 LIB_H += git-compat-util.h
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 7581632..21efd25 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -5,6 +5,8 @@
 #include "revision.h"
 #include "tag.h"
 #include "string-list.h"
+#include "branch.h"
+#include "fmt-merge-msg.h"
 
 static const char * const fmt_merge_msg_usage[] = {
 	"git fmt-merge-msg [-m <message>] [--log[=<n>]|--no-log] [--file <file>]",
@@ -12,8 +14,9 @@ static const char * const fmt_merge_msg_usage[] = {
 };
 
 static int shortlog_len;
+static int use_branch_desc;
 
-static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
+int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 {
 	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
 		int is_bool;
@@ -22,6 +25,8 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 			return error("%s: negative length %s", key, value);
 		if (is_bool && shortlog_len)
 			shortlog_len = DEFAULT_MERGE_LOG_LEN;
+	} else if (!strcmp(key, "merge.branchdesc")) {
+		use_branch_desc = git_config_bool(key, value);
 	}
 	return 0;
 }
@@ -31,6 +36,11 @@ struct src_data {
 	int head_status;
 };
 
+struct origin_data {
+	unsigned char sha1[20];
+	int is_local_branch:1;
+};
+
 static void init_src_data(struct src_data *data)
 {
 	data->branch.strdup_strings = 1;
@@ -45,7 +55,7 @@ static struct string_list origins = STRING_LIST_INIT_DUP;
 static int handle_line(char *line)
 {
 	int i, len = strlen(line);
-	unsigned char *sha1;
+	struct origin_data *origin_data;
 	char *src, *origin;
 	struct src_data *src_data;
 	struct string_list_item *item;
@@ -61,11 +71,13 @@ static int handle_line(char *line)
 		return 2;
 
 	line[40] = 0;
-	sha1 = xmalloc(20);
-	i = get_sha1(line, sha1);
+	origin_data = xcalloc(1, sizeof(struct origin_data));
+	i = get_sha1(line, origin_data->sha1);
 	line[40] = '\t';
-	if (i)
+	if (i) {
+		free(origin_data);
 		return 3;
+	}
 
 	if (line[len - 1] == '\n')
 		line[len - 1] = 0;
@@ -93,6 +105,7 @@ static int handle_line(char *line)
 		origin = src;
 		src_data->head_status |= 1;
 	} else if (!prefixcmp(line, "branch ")) {
+		origin_data->is_local_branch = 1;
 		origin = line + 7;
 		string_list_append(&src_data->branch, origin);
 		src_data->head_status |= 2;
@@ -119,7 +132,9 @@ static int handle_line(char *line)
 		sprintf(new_origin, "%s of %s", origin, src);
 		origin = new_origin;
 	}
-	string_list_append(&origins, origin)->util = sha1;
+	if (strcmp(".", src))
+		origin_data->is_local_branch = 0;
+	string_list_append(&origins, origin)->util = origin_data;
 	return 0;
 }
 
@@ -140,9 +155,30 @@ static void print_joined(const char *singular, const char *plural,
 	}
 }
 
-static void shortlog(const char *name, unsigned char *sha1,
-		struct commit *head, struct rev_info *rev, int limit,
-		struct strbuf *out)
+static void add_branch_desc(struct strbuf *out, const char *name)
+{
+	struct strbuf desc = STRBUF_INIT;
+
+	if (!read_branch_desc(&desc, name)) {
+		const char *bp = desc.buf;
+		while (*bp) {
+			const char *ep = strchrnul(bp, '\n');
+			if (*ep)
+				ep++;
+			strbuf_addf(out, "  : %.*s", (int)(ep - bp), bp);
+			bp = ep;
+		}
+		if (out->buf[out->len - 1] != '\n')
+			strbuf_addch(out, '\n');
+	}
+	strbuf_release(&desc);
+}
+
+static void shortlog(const char *name,
+		     struct origin_data *origin_data,
+		     struct commit *head,
+		     struct rev_info *rev, int limit,
+		     struct strbuf *out)
 {
 	int i, count = 0;
 	struct commit *commit;
@@ -150,6 +186,7 @@ static void shortlog(const char *name, unsigned char *sha1,
 	struct string_list subjects = STRING_LIST_INIT_DUP;
 	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
 	struct strbuf sb = STRBUF_INIT;
+	const unsigned char *sha1 = origin_data->sha1;
 
 	branch = deref_tag(parse_object(sha1), sha1_to_hex(sha1), 40);
 	if (!branch || branch->type != OBJ_COMMIT)
@@ -188,6 +225,9 @@ static void shortlog(const char *name, unsigned char *sha1,
 	else
 		strbuf_addf(out, "\n* %s:\n", name);
 
+	if (origin_data->is_local_branch && use_branch_desc)
+		add_branch_desc(out, name);
+
 	for (i = 0; i < subjects.nr; i++)
 		if (i >= limit)
 			strbuf_addf(out, "  ...\n");
@@ -303,8 +343,9 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
 			strbuf_addch(out, '\n');
 
 		for (i = 0; i < origins.nr; i++)
-			shortlog(origins.items[i].string, origins.items[i].util,
-					head, &rev, shortlog_len, out);
+			shortlog(origins.items[i].string,
+				 origins.items[i].util,
+				 head, &rev, shortlog_len, out);
 	}
 	return 0;
 }
diff --git a/builtin/merge.c b/builtin/merge.c
index ab4077f..fe56aad 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -26,6 +26,7 @@
 #include "merge-recursive.h"
 #include "resolve-undo.h"
 #include "remote.h"
+#include "fmt-merge-msg.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -525,6 +526,8 @@ static void parse_branch_merge_options(char *bmo)
 
 static int git_merge_config(const char *k, const char *v, void *cb)
 {
+	int status;
+
 	if (branch && !prefixcmp(k, "branch.") &&
 		!prefixcmp(k + 7, branch) &&
 		!strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
@@ -541,15 +544,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 		return git_config_string(&pull_octopus, k, v);
 	else if (!strcmp(k, "merge.renormalize"))
 		option_renormalize = git_config_bool(k, v);
-	else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary")) {
-		int is_bool;
-		shortlog_len = git_config_bool_or_int(k, v, &is_bool);
-		if (!is_bool && shortlog_len < 0)
-			return error(_("%s: negative length %s"), k, v);
-		if (is_bool && shortlog_len)
-			shortlog_len = DEFAULT_MERGE_LOG_LEN;
-		return 0;
-	} else if (!strcmp(k, "merge.ff")) {
+	else if (!strcmp(k, "merge.ff")) {
 		int boolval = git_config_maybe_bool(k, v);
 		if (0 <= boolval) {
 			allow_fast_forward = boolval;
@@ -562,6 +557,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 		default_to_upstream = git_config_bool(k, v);
 		return 0;
 	}
+	status = fmt_merge_msg_config(k, v, cb);
+	if (status)
+		return status;
 	return git_diff_ui_config(k, v, cb);
 }
 
diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
new file mode 100644
index 0000000..9217fdb
--- /dev/null
+++ b/fmt-merge-msg.h
@@ -0,0 +1,6 @@
+#ifndef FMT_MERGE_MSG_H
+#define FMT_MERGE_MSG_H
+
+extern int fmt_merge_msg_config(const char *key, const char *value, void *cb);
+
+#endif /* FMT_MERGE_MSG_H */
-- 
1.7.7.138.g7f41b6

^ permalink raw reply related

* Re: [PATCH] fetch: plug two leaks on error exit in store_updated_refs
From: René Scharfe @ 2011-10-07  6:59 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Chris Wilson, git, Junio C Hamano
In-Reply-To: <CALUzUxp4Eo7j=kM7YPJbj70-rwuyFK5V1mZZMY7vBwwPYWS6gQ@mail.gmail.com>

Am 07.10.2011 08:49, schrieb Tay Ray Chuan:
> How about reusing the function's cleanup calls, like this?

Yes, that's better.

> -- >8 --
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index fc254b6..56267c4 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -423,8 +423,10 @@ static int store_updated_refs(const char
> *raw_url, const char *remote_name,
>  	else
>  		url = xstrdup("foreign");
> 
> -	if (check_everything_connected(ref_map, 0))
> -		return error(_("%s did not send all necessary objects\n"), url);
> +	if (check_everything_connected(ref_map, 0)) {
> +		rc = error(_("%s did not send all necessary objects\n"), url);
> +		goto abort;
> +	}
> 
>  	for (rm = ref_map; rm; rm = rm->next) {
>  		struct ref *ref = NULL;
> @@ -506,12 +508,15 @@ static int store_updated_refs(const char
> *raw_url, const char *remote_name,
>  				fprintf(stderr, " %s\n", note);
>  		}
>  	}
> -	free(url);
> -	fclose(fp);
> +
>  	if (rc & STORE_REF_ERROR_DF_CONFLICT)
>  		error(_("some local refs could not be updated; try running\n"
>  		      " 'git remote prune %s' to remove any old, conflicting "
>  		      "branches"), remote_name);
> +
> +abort:
> +	free(url);
> +	fclose(fp);
>  	return rc;
>  }
> 

Micro-nit: If you start the label with a space ("+ abort:") then the
code continues to play nice with git grep -W.

René

^ permalink raw reply

* Re: [PATCH WIP 0/3] git log --exclude
From: Nguyen Thai Ngoc Duy @ 2011-10-07  7:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7vhb3n8ie9.fsf@alter.siamese.dyndns.org>

2011/10/6 Junio C Hamano <gitster@pobox.com>:
> For the purpose of "log --exclude" [*2*], I do not mind too much if the UI
> expressed negative pathspecs using such a new command line option, but I
> think it would be more natural to say (notations aside):
>
>        $ git log -- ':(no):po' .
>
> and define the behaviour of user-supplied pathspec limiter this way:
>
>  * Paths are matched from left to right;
>
>  * First match determines the fate of the path;
>
>  * A match with negative pathspec means "the path in question does _not_
>   match".

Things have changed a bit since last time I touched negative pathspec.
Because of depth limit support [1], pathspecs have to be sorted, which
means we cannot keep pathspec in original order to match from left to
right.

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.

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

[1] 86e4ca6 (tree_entry_interesting(): fix depth limit with
overlapping pathspecs)
-- 
Duy

^ permalink raw reply

* [PATCH v2] fetch: plug two leaks on error exit in store_updated_refs
From: Tay Ray Chuan @ 2011-10-07  7:40 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Chris Wilson, René Scharfe
In-Reply-To: <4E8EA33E.5020009@lsrfire.ath.cx>

Close FETCH_HEAD and release the string url even if we have to leave the
function store_updated_refs() early.

Reported-by: Chris Wilson <cwilson@vigilantsw.com>
Helped-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin/fetch.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc254b6..9b7ce10 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -423,8 +423,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	else
 		url = xstrdup("foreign");
 
-	if (check_everything_connected(ref_map, 0))
-		return error(_("%s did not send all necessary objects\n"), url);
+	if (check_everything_connected(ref_map, 0)) {
+		rc = error(_("%s did not send all necessary objects\n"), url);
+		goto abort;
+	}
 
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
@@ -506,12 +508,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fprintf(stderr, " %s\n", note);
 		}
 	}
-	free(url);
-	fclose(fp);
+
 	if (rc & STORE_REF_ERROR_DF_CONFLICT)
 		error(_("some local refs could not be updated; try running\n"
 		      " 'git remote prune %s' to remove any old, conflicting "
 		      "branches"), remote_name);
+
+ abort:
+	free(url);
+	fclose(fp);
 	return rc;
 }
 
-- 
1.7.7.584.g16d0ea

^ permalink raw reply related

* Re: [PATCH] commit: teach --gpg-sign option
From: Michael J Gruber @ 2011-10-07  8:40 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Robin H. Johnson, Junio C Hamano
In-Reply-To: <robbat2-20111006T221637-481195848Z@orbis-terrarum.net>

[readding JCH to cc whom you dropped]
Robin H. Johnson venit, vidit, dixit 07.10.2011 00:24:
> On Wed, Oct 05, 2011 at 05:56:55PM -0700,  Junio C Hamano wrote:
>> And this uses the gpg-interface.[ch] to allow signing the commit, i.e.
>>
>>     $ git commit --gpg-sign -m foo
>>     You need a passphrase to unlock the secret key for
>>     user: "Junio C Hamano <gitster@pobox.com>"
>>     4096-bit RSA key, ID 96AFE6CB, created 2011-10-03 (main key ID 713660A7)
>>
>>     [master 8457d13] foo
>>      1 files changed, 1 insertions(+), 0 deletions(-)
> I like it, but I have a couple of questions: 
> 1. Are the sig lines used in computed SHA1/commitid of a given commit (I
>    see examples w/ --amend and that would usually change the SHA1)?

Yes, just like with tag objects.

> 2. Can we allow more than one person sign a commit?

I don't think we support it now (tags) but we could allow concatenating
signatures since they are detached.

There's a somewhat delicate issue here: The signature (tag/commit) is a
signature on the contents of the object, and is itself not part of the
contents (or else we would have a chicken-egg-problem).

The sha1 of the object is determined by the content+header, i.e.
including the signature.

So, by adding a signature, you change the sha1, but any existing
signature remains valid.

This is also how you can try to achieve a specific sha1 for a given
object content...

> 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 this isn't a replacement for push certificates, but has value in
> itself. It's certainly provides better integration than the
> signature-in-note variants.
> 

I do think it's meant as an implementation of push certificates. I don't
see any other value in it which could not be achieved by signed tags.
Can you describe any?

Michael

^ permalink raw reply

* How pretty is pretty? git cat-file -p inconsistency
From: Michael J Gruber @ 2011-10-07  8:44 UTC (permalink / raw)
  To: Git Mailing List

diff <(git cat-file -p tags/v1.7.7) <(git cat-file tag tags/v1.7.7)
4c4
< tagger Junio C Hamano <gitster@pobox.com> Fri Sep 30 14:21:06 2011 -0700
---
> tagger Junio C Hamano <gitster@pobox.com> 1317417666 -0700

vs.

diff <(git cat-file -p tags/v1.7.7^{}) <(git cat-file commit tags/v1.7.7^{})

(no diff)

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? I'd suggest
prettifying dates with "-p" for commit objects also. But just how
plumbing is the "-p" mode of "cat-file"?

Michael

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Michael J Gruber @ 2011-10-07  8:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobxtwaog.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 07.10.2011 08:57:
> This teaches "merge --log" and fmt-merge-msg to use branch description
> information when merging a local topic branch into the mainline. The
> description goes between the branch name label and the list of commit
> titles.
> 
> The refactoring to share the common configuration parsing between
> merge and fmt-merge-msg needs to be made into a separate patch.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * This is a follow-up to the branch.$name.description series that has
>    been queued in 'pu' (jc/request-pull-show-head-4). The two commands
>    join the family of commands that are aware of the branch description,
>    i.e. format-patch (cover letter), request-pull.


Uhm, I tried to start a discussion there about whether we really want
config based branch descriptions, together with suggestions and actual
series for notes based descriptions, notes based format-patch
cover-letter, notes based branch output. Making fmt-merge-msg use that
is a no-brainer.

Do we want an implementation race, or could we please reach some
consensus about the direction first? (Not many have spoken up yet.)

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

>  Makefile                |    1 +
>  builtin/fmt-merge-msg.c |   63 ++++++++++++++++++++++++++++++++++++++--------
>  builtin/merge.c         |   16 +++++------
>  fmt-merge-msg.h         |    6 ++++
>  4 files changed, 66 insertions(+), 20 deletions(-)
>  create mode 100644 fmt-merge-msg.h
> 
> diff --git a/Makefile b/Makefile
> index 8d6d451..b499049 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -527,6 +527,7 @@ LIB_H += diffcore.h
>  LIB_H += diff.h
>  LIB_H += dir.h
>  LIB_H += exec_cmd.h
> +LIB_H += fmt-merge-msg.h
>  LIB_H += fsck.h
>  LIB_H += gettext.h
>  LIB_H += git-compat-util.h
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index 7581632..21efd25 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -5,6 +5,8 @@
>  #include "revision.h"
>  #include "tag.h"
>  #include "string-list.h"
> +#include "branch.h"
> +#include "fmt-merge-msg.h"
>  
>  static const char * const fmt_merge_msg_usage[] = {
>  	"git fmt-merge-msg [-m <message>] [--log[=<n>]|--no-log] [--file <file>]",
> @@ -12,8 +14,9 @@ static const char * const fmt_merge_msg_usage[] = {
>  };
>  
>  static int shortlog_len;
> +static int use_branch_desc;
>  
> -static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
> +int fmt_merge_msg_config(const char *key, const char *value, void *cb)
>  {
>  	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
>  		int is_bool;
> @@ -22,6 +25,8 @@ static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
>  			return error("%s: negative length %s", key, value);
>  		if (is_bool && shortlog_len)
>  			shortlog_len = DEFAULT_MERGE_LOG_LEN;
> +	} else if (!strcmp(key, "merge.branchdesc")) {
> +		use_branch_desc = git_config_bool(key, value);
>  	}
>  	return 0;
>  }
> @@ -31,6 +36,11 @@ struct src_data {
>  	int head_status;
>  };
>  
> +struct origin_data {
> +	unsigned char sha1[20];
> +	int is_local_branch:1;
> +};
> +
>  static void init_src_data(struct src_data *data)
>  {
>  	data->branch.strdup_strings = 1;
> @@ -45,7 +55,7 @@ static struct string_list origins = STRING_LIST_INIT_DUP;
>  static int handle_line(char *line)
>  {
>  	int i, len = strlen(line);
> -	unsigned char *sha1;
> +	struct origin_data *origin_data;
>  	char *src, *origin;
>  	struct src_data *src_data;
>  	struct string_list_item *item;
> @@ -61,11 +71,13 @@ static int handle_line(char *line)
>  		return 2;
>  
>  	line[40] = 0;
> -	sha1 = xmalloc(20);
> -	i = get_sha1(line, sha1);
> +	origin_data = xcalloc(1, sizeof(struct origin_data));
> +	i = get_sha1(line, origin_data->sha1);
>  	line[40] = '\t';
> -	if (i)
> +	if (i) {
> +		free(origin_data);
>  		return 3;
> +	}
>  
>  	if (line[len - 1] == '\n')
>  		line[len - 1] = 0;
> @@ -93,6 +105,7 @@ static int handle_line(char *line)
>  		origin = src;
>  		src_data->head_status |= 1;
>  	} else if (!prefixcmp(line, "branch ")) {
> +		origin_data->is_local_branch = 1;
>  		origin = line + 7;
>  		string_list_append(&src_data->branch, origin);
>  		src_data->head_status |= 2;
> @@ -119,7 +132,9 @@ static int handle_line(char *line)
>  		sprintf(new_origin, "%s of %s", origin, src);
>  		origin = new_origin;
>  	}
> -	string_list_append(&origins, origin)->util = sha1;
> +	if (strcmp(".", src))
> +		origin_data->is_local_branch = 0;
> +	string_list_append(&origins, origin)->util = origin_data;
>  	return 0;
>  }
>  
> @@ -140,9 +155,30 @@ static void print_joined(const char *singular, const char *plural,
>  	}
>  }
>  
> -static void shortlog(const char *name, unsigned char *sha1,
> -		struct commit *head, struct rev_info *rev, int limit,
> -		struct strbuf *out)
> +static void add_branch_desc(struct strbuf *out, const char *name)
> +{
> +	struct strbuf desc = STRBUF_INIT;
> +
> +	if (!read_branch_desc(&desc, name)) {
> +		const char *bp = desc.buf;
> +		while (*bp) {
> +			const char *ep = strchrnul(bp, '\n');
> +			if (*ep)
> +				ep++;
> +			strbuf_addf(out, "  : %.*s", (int)(ep - bp), bp);
> +			bp = ep;
> +		}
> +		if (out->buf[out->len - 1] != '\n')
> +			strbuf_addch(out, '\n');
> +	}
> +	strbuf_release(&desc);
> +}
> +
> +static void shortlog(const char *name,
> +		     struct origin_data *origin_data,
> +		     struct commit *head,
> +		     struct rev_info *rev, int limit,
> +		     struct strbuf *out)
>  {
>  	int i, count = 0;
>  	struct commit *commit;
> @@ -150,6 +186,7 @@ static void shortlog(const char *name, unsigned char *sha1,
>  	struct string_list subjects = STRING_LIST_INIT_DUP;
>  	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
>  	struct strbuf sb = STRBUF_INIT;
> +	const unsigned char *sha1 = origin_data->sha1;
>  
>  	branch = deref_tag(parse_object(sha1), sha1_to_hex(sha1), 40);
>  	if (!branch || branch->type != OBJ_COMMIT)
> @@ -188,6 +225,9 @@ static void shortlog(const char *name, unsigned char *sha1,
>  	else
>  		strbuf_addf(out, "\n* %s:\n", name);
>  
> +	if (origin_data->is_local_branch && use_branch_desc)
> +		add_branch_desc(out, name);
> +
>  	for (i = 0; i < subjects.nr; i++)
>  		if (i >= limit)
>  			strbuf_addf(out, "  ...\n");
> @@ -303,8 +343,9 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
>  			strbuf_addch(out, '\n');
>  
>  		for (i = 0; i < origins.nr; i++)
> -			shortlog(origins.items[i].string, origins.items[i].util,
> -					head, &rev, shortlog_len, out);
> +			shortlog(origins.items[i].string,
> +				 origins.items[i].util,
> +				 head, &rev, shortlog_len, out);
>  	}
>  	return 0;
>  }
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ab4077f..fe56aad 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -26,6 +26,7 @@
>  #include "merge-recursive.h"
>  #include "resolve-undo.h"
>  #include "remote.h"
> +#include "fmt-merge-msg.h"
>  
>  #define DEFAULT_TWOHEAD (1<<0)
>  #define DEFAULT_OCTOPUS (1<<1)
> @@ -525,6 +526,8 @@ static void parse_branch_merge_options(char *bmo)
>  
>  static int git_merge_config(const char *k, const char *v, void *cb)
>  {
> +	int status;
> +
>  	if (branch && !prefixcmp(k, "branch.") &&
>  		!prefixcmp(k + 7, branch) &&
>  		!strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
> @@ -541,15 +544,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)
>  		return git_config_string(&pull_octopus, k, v);
>  	else if (!strcmp(k, "merge.renormalize"))
>  		option_renormalize = git_config_bool(k, v);
> -	else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary")) {
> -		int is_bool;
> -		shortlog_len = git_config_bool_or_int(k, v, &is_bool);
> -		if (!is_bool && shortlog_len < 0)
> -			return error(_("%s: negative length %s"), k, v);
> -		if (is_bool && shortlog_len)
> -			shortlog_len = DEFAULT_MERGE_LOG_LEN;
> -		return 0;
> -	} else if (!strcmp(k, "merge.ff")) {
> +	else if (!strcmp(k, "merge.ff")) {
>  		int boolval = git_config_maybe_bool(k, v);
>  		if (0 <= boolval) {
>  			allow_fast_forward = boolval;
> @@ -562,6 +557,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)
>  		default_to_upstream = git_config_bool(k, v);
>  		return 0;
>  	}
> +	status = fmt_merge_msg_config(k, v, cb);
> +	if (status)
> +		return status;
>  	return git_diff_ui_config(k, v, cb);
>  }
>  
> diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
> new file mode 100644
> index 0000000..9217fdb
> --- /dev/null
> +++ b/fmt-merge-msg.h
> @@ -0,0 +1,6 @@
> +#ifndef FMT_MERGE_MSG_H
> +#define FMT_MERGE_MSG_H
> +
> +extern int fmt_merge_msg_config(const char *key, const char *value, void *cb);
> +
> +#endif /* FMT_MERGE_MSG_H */

^ permalink raw reply

* [PATCH 1/2] submodule: whitespace fix
From: Tay Ray Chuan @ 2011-10-07  9:04 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Replace SPs with TAB.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 git-submodule.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 928a62f..ebea35b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -104,9 +104,9 @@ module_name()
 	re=$(printf '%s\n' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
 	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
 		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
-       test -z "$name" &&
-       die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
-       echo "$name"
+	test -z "$name" &&
+	die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
+	echo "$name"
 }
 
 #
-- 
1.7.7.584.g16d0ea

^ permalink raw reply related

* [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()
From: Tay Ray Chuan @ 2011-10-07  9:04 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1317978295-4796-1-git-send-email-rctay89@gmail.com>

The die() message that may occur in module_name() is not really relevant
to the user when called from module_clone(); the latter handles the
"failure" (no submodule mapping) anyway.

Leave other callers of module_name() unchanged, as the die() message
shown is either relevant for user consumption (such as those that exit()
when the call fails), or will not occur at all (when called with paths
returned by module_list()).

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index ebea35b..3adab93 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -130,7 +130,7 @@ module_clone()
 
 	gitdir=
 	gitdir_base=
-	name=$(module_name "$path")
+	name=$(module_name "$path" 2>/dev/null)
 	base_path=$(dirname "$path")
 
 	gitdir=$(git rev-parse --git-dir)
-- 
1.7.7.584.g16d0ea

^ permalink raw reply related

* 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