Git development
 help / color / mirror / Atom feed
* [PATCH v2 1/2] files_read_raw_ref: avoid infinite loop on broken symlinks
From: Jeff King @ 2016-10-06 19:41 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Michael Haggerty
In-Reply-To: <1c39b371-eb41-05d9-3c48-bd41764c9c9a@kdbg.org>

On Thu, Oct 06, 2016 at 09:31:22PM +0200, Johannes Sixt wrote:

> Am 06.10.2016 um 18:48 schrieb Jeff King:
> > +test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
> > +	ln -s does-not-exist .git/broken &&
> > +	test_must_fail git rev-parse --verify broken
> 
> Hm, lower-case named refs directly in .git are frowned upon, no? If we ever
> decide to forbid them outright, this ref-parse might still fail, but for the
> wrong reason. Should you not better pick an example below ref/?

I suppose so. The test case was adapted from a real-world example, but
it fails equally well with .git/refs/heads/broken. The only restriction
is that the symlink _destination_ cannot look like "refs/heads/...".

Here's a replacement 1/2. The second patch remains unchanged.

-- >8 --
Subject: files_read_raw_ref: avoid infinite loop on broken symlinks

Our ref resolution first runs lstat() on any path we try to
look up, because we want to treat symlinks specially (by
resolving them manually and considering them symrefs). But
if the results of `readlink` do _not_ look like a ref, we
fall through to treating it like a normal file, and just
read the contents of the linked path.

Since fcb7c76 (resolve_ref_unsafe(): close race condition
reading loose refs, 2013-06-19), that "normal file" code
path will stat() the file and if we see ENOENT, will jump
back to the lstat(), thinking we've seen inconsistent
results between the two calls. But for a symbolic ref, this
isn't a race: the lstat() found the symlink, and the stat()
is looking at the path it points to. We end up in an
infinite loop calling lstat() and stat().

We can fix this by avoiding the retry-on-inconsistent jump
when we know that we found a symlink. While we're at it,
let's add a comment explaining why the symlink case gets to
this code in the first place; without that, it is not
obvious that the correct solution isn't to avoid the stat()
code path entirely.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs/files-backend.c        | 7 ++++++-
 t/t1503-rev-parse-verify.sh | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0709f60..d826557 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1403,6 +1403,11 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 			ret = 0;
 			goto out;
 		}
+		/*
+		 * It doesn't look like a refname; fall through to just
+		 * treating it like a non-symlink, and reading whatever it
+		 * points to.
+		 */
 	}
 
 	/* Is it a directory? */
@@ -1426,7 +1431,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 	 */
 	fd = open(path, O_RDONLY);
 	if (fd < 0) {
-		if (errno == ENOENT)
+		if (errno == ENOENT && !S_ISLNK(st.st_mode))
 			/* inconsistent with lstat; retry */
 			goto stat_ref;
 		else
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index ab27d0d..492edff 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -139,4 +139,9 @@ test_expect_success 'master@{n} for various n' '
 	test_must_fail git rev-parse --verify master@{$Np1}
 '
 
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+	ln -s does-not-exist .git/refs/heads/broken &&
+	test_must_fail git rev-parse --verify broken
+'
+
 test_done
-- 
2.10.1.506.g904834d


^ permalink raw reply related

* Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
From: Junio C Hamano @ 2016-10-06 19:44 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Jakub Narebski
In-Reply-To: <20161006091135.29590-3-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Change the minimum length of an abbreviated object identifier in the
> commit message gitweb tries to turn into link from 8 hexchars to 7.
>
> This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb:
> SHA-1 in commit log message links to "object" view", 2006-12-10), but
> the default abbreviation length is 7, and has been for a long time.
>
> It's still possible to reference SHA1s down to 4 characters in length,
> see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
> git actually produce that, so I doubt anyone is putting that into log
> messages in practice, but people definitely do put 7 character SHA1s
> into log messages.
>
> I think it's fairly dubious to link to things matching [0-9a-fA-F]
> here as opposed to just [0-9a-f], that dates back to the initial
> version of gitweb from 161332a ("first working version",
> 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
> them as far as I can tell.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

s/SHA1/SHA-1/g.  I agree that A-F are dubious.

>  gitweb/gitweb.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index cba7405..92b5e91 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,7 +2036,7 @@ sub format_log_line_html {
>  	my $line = shift;
>  
>  	$line = esc_html($line, -nbsp=>1);
> -	$line =~ s{\b([0-9a-fA-F]{8,40})\b}{
> +	$line =~ s{\b([0-9a-fA-F]{7,40})\b}{
>  		$cgi->a({-href => href(action=>"object", hash=>$1),
>  					-class => "text"}, $1);
>  	}eg;

^ permalink raw reply

* Re: [PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages
From: Junio C Hamano @ 2016-10-06 19:51 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Jakub Narebski
In-Reply-To: <20161006091135.29590-4-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Change the log formatting function to know about "git describe" output
> such as "v2.8.0-4-g867ad08", in addition to just plain "867ad08".
>
> There are still many valid refnames that we don't link to
> e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
> v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
> similarly it's trivially possible to create some refnames like
> "æ/var-gf6727b0" or which won't be picked up by this regex.

Not a serious counter-proposal or suggestion, and certainly not an
objection to the patch I am responding to, but I wonder if it is so
bad if we made the 867ad08 into link when showing v2.8.0-4-g867ad08.

IOW, in addition to \b followed by [0-9a-f]+ followed by \b, if we
allowed an optional 'g' in front of the hex, e.g.

-	$line =~ s{\b([0-9a-fA-F]{7,40})\b}{
+	$line =~ s{\bg?([0-9a-fA-F]{7,40})\b}{

wouldn't that be much simpler, covers more cases and sufficient?

> There's surely room for improvement here, but I just wanted to address
> the very common case of sticking "git describe" output into commit
> messages without trying to link to all possible refnames, that's going
> to be a rather futile exercise given that this is free text, and it
> would be prohibitively expensive to look up whether the references in
> question exist in our repository.
>
> There was on-list discussion about how we could do better than this
> patch. Junio suggested to update parse_commits() to call a new
> "gitweb--helper" command which would pass each of the revision
> candidates through "rev-parse --verify --quiet". That would cut down
> on our false positives (e.g. we'll link to "deadbeef"), and also allow
> us to be more aggressive in selecting candidate revisions.
>
> That may be too expensive to work in practice, or it may
> not. Investigating that would be a good follow-up to this patch.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  gitweb/gitweb.perl | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 92b5e91..7cf68f0 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
>  	my $line = shift;
>  
>  	$line = esc_html($line, -nbsp=>1);
> -	$line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> +	$line =~ s{
> +        \b
> +        (
> +            # The output of "git describe", e.g. v2.10.0-297-gf6727b0
> +            # or hadoop-20160921-113441-20-g094fb7d
> +            (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
> +            [A-Za-z0-9.-]+
> +            (?!\.) # refs can't end with ".", see check_refname_format()
> +            -g[0-9a-fA-F]{7,40}
> +            |
> +            # Just a normal looking Git SHA1
> +            [0-9a-fA-F]{7,40}
> +        )
> +        \b
> +    }{
>  		$cgi->a({-href => href(action=>"object", hash=>$1),
>  					-class => "text"}, $1);
> -	}eg;
> +	}egx;
>  
>  	return $line;
>  }

^ permalink raw reply

* Re: [PATCH 1/2] submodule add: extend force flag to add existing repos
From: Junio C Hamano @ 2016-10-06 20:11 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, hvoigt, torvalds, peff
In-Reply-To: <20161006193725.31553-2-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> Currently the force flag in `git submodule add` takes care of possibly
> ignored files or when a name collision occurs.
>
> However there is another situation where submodule add comes in handy:
> When you already have a gitlink recorded, but no configuration was
> done (i.e. no .gitmodules file nor any entry in .git/config) and you
> want to generate these config entries. For this situation allow
> `git submodule add` to proceed if there is already a submodule at the
> given path in the index.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

Yup, the goal makes perfect sense.  

I vaguely recall discussing this exact issue of "git submodule add"
that refuses to add a path that already is a gitlink (via "git add"
that has previously been run) elsewhere on this list some time ago.


^ permalink raw reply

* [BUG?] git log with --follow and --skip
From: Christian Couder @ 2016-10-06 20:24 UTC (permalink / raw)
  To: git

Hi,

At GitLab, we are annoyed by the fact that `git log` doesn't seem to
work well when using it with both --follow and --skip.

For example:

> git log --oneline -6 README.md
a299e3a README.md: format CLI commands with code syntax
c9a014e README.md: don't take 'commandname' literally
a217f07 README.md: move down historical explanation about the name
28513c4 README.md: don't call git stupid in the title
d9b297d README.md: move the link to git-scm.com up
6164972 README.md: add hyperlinks on filenames

> git log --skip 2 --oneline -6 README.md
a217f07 README.md: move down historical explanation about the name
28513c4 README.md: don't call git stupid in the title
d9b297d README.md: move the link to git-scm.com up
6164972 README.md: add hyperlinks on filenames
4ad21f5 README: use markdown syntax

The above 2 commands seem to work well, but:

> git log --follow --skip 2 --oneline -6 README.md
a299e3a README.md: format CLI commands with code syntax
c9a014e README.md: don't take 'commandname' literally
a217f07 README.md: move down historical explanation about the name
28513c4 README.md: don't call git stupid in the title
d9b297d README.md: move the link to git-scm.com up
6164972 README.md: add hyperlinks on filenames

So with --follow it looks like the top 2 commits are not skipped any more.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCHv4 2/2] push: change submodule default to check when submodules exist
From: Junio C Hamano @ 2016-10-06 20:25 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, hvoigt, torvalds, peff
In-Reply-To: <20161006193725.31553-3-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> When working with submodules, it is easy to forget to push the submodules.
> The setting 'check', which checks if any existing submodule is present on
> at least one remote of the submodule remotes, is designed to prevent this
> mistake.
>
> Flipping the default to check for submodules is safer than the current
> default of ignoring submodules while pushing.
>
> However checking for submodules requires additional work[1], which annoys
> users that do not use submodules, so we turn on the check for submodules
> based on a cheap heuristic, the existence of the .git/modules directory.

... and other things like .gitmodules, submodule stuff in
.git/config, etc.?

> +	} else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
> +		has_submodules_configured = 1;

An in-code comment to explain why ".url" is so special would be
helpful.  Documentation/config.txt says .path and .url are both
initially populated by 'git submodule init', which might be outdated
information that confuses readers of the above code.

> @@ -552,6 +564,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	packet_trace_identity("push");
> +	preset_submodule_default();
>  	git_config(git_push_config, &flags);
>  	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
>  	set_push_cert_flags(&flags, push_cert);
> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> index 198ce84..e690749 100755
> --- a/t/t5531-deep-submodule-push.sh
> +++ b/t/t5531-deep-submodule-push.sh
> @@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on remote' '
>  		git add gar/bage &&
>  		git commit -m "Third commit for gar/bage" &&
>  		# the push should fail with --recurse-submodules=check
> -		# on the command line...
> +		# on the command line. "check" is the default for repos in
> +		# which submodules are detected by existence of config,
> +		# .gitmodules file or an internal .git/modules/<submodule-repo>
> +		git submodule add -f ../submodule.git gar/bage &&
> +		test_must_fail git push ../pub.git master &&
>  		test_must_fail git push --recurse-submodules=check ../pub.git master &&
>  
>  		# ...or if specified in the configuration..

^ permalink raw reply

* Re: [PATCH v8 11/11] convert: add filter.<driver>.process option
From: Lars Schneider @ 2016-10-06 21:27 UTC (permalink / raw)
  To: Jakub Narębski
  Cc: git, Jeff King, Junio C Hamano, Stefan Beller,
	Martin-Louis Bright, Torsten Bögershausen, Ramsay Jones
In-Reply-To: <d4ad73af-d14b-03dd-0634-d8919a1d2ddd@gmail.com>


> On 04 Oct 2016, at 23:00, Jakub Narębski <jnareb@gmail.com> wrote:
> 
> [Some of answers and comments may got invalidated by v9]
> 
> W dniu 30.09.2016 o 21:38, Lars Schneider pisze:
>>> On 27 Sep 2016, at 17:37, Jakub Narębski <jnareb@gmail.com> wrote:
>>> 
>>> Part second of the review of 11/11.
> [...]
>>>> +
>>>> +	if (start_command(process)) {
>>>> +		error("cannot fork to run external filter '%s'", cmd);
>>>> +		kill_multi_file_filter(hashmap, entry);
>>>> +		return NULL;
>>>> +	}
>>> 
>>> I guess there is a reason why we init hashmap entry, try to start
>>> external process, then kill entry of unable to start, instead of
>>> trying to start external process, and adding hashmap entry when
>>> we succeed?
>> 
>> Yes. This way I can reuse the kill_multi_file_filter() function.
> 
> I don't quite understand.  If you didn't fill the entry before
> using start_command(process), you would not need kill_multi_file_filter(),
> which in that case IIUC just removes the just created entry from hashmap.
> Couldn't you add entry to hashmap in the 'else' part?  Or would it
> be racy?

You are right. I'll fix that.


>> 
>>>> +		if (pair[0] && pair[0]->len && pair[1]) {
>>>> +			if (!strcmp(pair[0]->buf, "status=")) {
>>>> +				strbuf_reset(status);
>>>> +				strbuf_addbuf(status, pair[1]);
>>>> +			}
>>> 
>>> So it is last status=<foo> line wins behavior?
>> 
>> Correct.
> 
> Perhaps this should be described in code comment.

OK


>>>> 
>>>> +	fflush(NULL);
>>> 
>>> Why this fflush(NULL) is needed here?
>> 
>> This flushes all open output streams. The single filter does the same.
> 
> I know what it does, but I don't know why.  But "single filter does it"
> is good enough for me.  Still would want to know why, though ;-)

TBH I am not 100% sure why, too. I think this ensures that we don't have 
any outdated/unrelated/previous data in the stream buffers.


>>>> 
>>>> +	if (fd >= 0 && !src) {
>>>> +		if (fstat(fd, &file_stat) == -1)
>>>> +			return 0;
>>>> +		len = xsize_t(file_stat.st_size);
>>>> +	}
>>> 
>>> Errr... is it necessary?  The protocol no longer provides size=<n>
>>> hint, and neither uses such hint if provided.
>> 
>> We require the size in write_packetized_from_buf() later.
> 
> Don't we use write_packetized_from_fd() in the case of fd >= 0?

Of course! Ah too many refactorings :-)
I'll remove that.

Thank you,
Lars


^ permalink raw reply

* Re: [PATCHv4 2/2] push: change submodule default to check when submodules exist
From: Stefan Beller @ 2016-10-06 21:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git@vger.kernel.org, Heiko Voigt, Linus Torvalds, Jeff King
In-Reply-To: <xmqqoa2xi5rd.fsf@gitster.mtv.corp.google.com>

On Thu, Oct 6, 2016 at 1:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> When working with submodules, it is easy to forget to push the submodules.
>> The setting 'check', which checks if any existing submodule is present on
>> at least one remote of the submodule remotes, is designed to prevent this
>> mistake.
>>
>> Flipping the default to check for submodules is safer than the current
>> default of ignoring submodules while pushing.
>>
>> However checking for submodules requires additional work[1], which annoys
>> users that do not use submodules, so we turn on the check for submodules
>> based on a cheap heuristic, the existence of the .git/modules directory.
>
> ... and other things like .gitmodules, submodule stuff in
> .git/config, etc.?

Oh right, I need to update this commit message to mention this, too

>
>> +     } else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
>> +             has_submodules_configured = 1;
>
> An in-code comment to explain why ".url" is so special would be
> helpful.  Documentation/config.txt says .path and .url are both

       submodule.<name>.path, submodule.<name>.url
           The path within this project and URL for a submodule. These
           variables are initially populated by git submodule init. See git-
           submodule(1) and gitmodules(5) for details.

The correct version is:

submodule.<name>.path::
    This is a config variable only found in .gitmodules files that
indicate at which
    path relative to the repository root the submodule is found.
    It is used to resolve the mapping (submodule name)<->path throughout all
    time, i.e. also later when a submodule is initialized and checked out, any
    subsequent operation that needs a submodule name/repsoitory uses it for
    lookup.

submodule.<name>.url::
    This is a config variable found both in .gitmodules files as well
as .git/config.
    In the .gitmodules files it serves as a hint where to obtain a
repository from
    to populate the gitlinks within the repository.
    On submodule-init the value will be copied over to the .git/config
file and there
    it serves as a holder of the value only until the first
submodule-update is run,
    as the initial submodule-update is using that url to clone the submodule.
    From then on it is only used as a boolean flag in the .git/config
to indicate
    whether the submodule is considered interesting for further submodule
    commands. (See the similar competing thing with submodule.<name>.ignore)

Given that it is obvious to my current me, that .url is the only
sensible thing as .path
is not found in the .git/config which we are searching in here.
So for the purpose of this patch I'd just add a line like this to
remind my future self:

    /* See if any submodule is considered interesting: */

I would like to avoid references to .path as that is not helping here.
(Why does that
comment point to a variable name that is not supposed to exist in the file?)
So I think we really need to think how to reduce confusion in the
future by readers that
have more or less overview of the submodules concept.

A future version of the man page may read:

submodule.<name>.path::
    <same as before>, we may want to consider if we copy it over to
.git/config, so the
    repository may trash the .gitmodules file and the submodules keep working.

submodule.<name>.url::
    indication in .gitmodules where the submodule is cloned from. It
will be copied over
    to .git/config on submodule-init. The user may adapt the
configured urls there before
    proceeding to submodule-update, which will delete the url setting
again, as the
    cloned submodule repository will hold the authoritative setting
where the remote
    of the submodule is.
    So it only exists in .git/config for submodules that are
initialized but have never
    been cloned. Even when the submodule is deinit'd and reinited we
don't even need
    to copy the url, as we still have the old submodule repository in
.git/module/<name>

submodule.<name>.exists::
    This setting is per working tree (unlike the previous 2) that
indicates if submodule
    related commands pay attention to the given submodule. It is still
unclear how this
    works together with the .ignore setting.


Note for this future to come true, we'd have to have only 2 big series
One that Duy started to introduce per working tree configs, see
    https://public-inbox.org/git/20160720172419.25473-1-pclouds@gmail.com/
The other one is a series that hasn't seen the mailing list yet, that
I started to
separate the 2 modes of the url both as a value holder (to point at a URL) as
well as just a boolean flag for commands to acknowledge the existence of the
submodule.


> initially populated by 'git submodule init', which might be outdated
> information that confuses readers of the above code.

A lot of text but I guess we can use some ideas from here to update the
.path and .url documentation. (The wording in this email is not of man
page quality).

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH v2] gpg-interface: use more status letters
From: Junio C Hamano @ 2016-10-06 21:43 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Ramsay Jones, git, Alex
In-Reply-To: <xmqq60pdbbxh.fsf@gitster.mtv.corp.google.com>

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

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> Also, I'm open to using another letter for EXPKEYSIG but couldn't decide
>> between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid.
>> 'Y' is next to 'X' and contained in 'KEY', it would be my first choice.
>
> Sounds good enough to me.  Thanks.

I really do not want to leave too many topics listed in the "What's
cooking" report to be in undecided / waiting state.  How about
squashing this in, with a fully updated log message to replace?

-- >8 --
From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Wed, 28 Sep 2016 16:24:13 +0200
Subject: [PATCH] SQUASH: gpg-interface: use more status letters

According to gpg2's doc/DETAILS:

    For each signature only one of the codes GOODSIG, BADSIG,
    EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted.

gpg1 ("classic") behaves the same (although doc/DETAILS differs).

Currently, we parse gpg's status output for GOODSIG, BADSIG and
trust information and translate that into status codes G, B, U, N
for the %G?  format specifier.

git-verify-* returns success in the GOODSIG case only. This is
somewhat in disagreement with gpg, which considers the first 5 of
the 6 above as VALIDSIG, but we err on the very safe side.

Introduce additional status codes E, X, Y, R for ERRSIG, EXPSIG,
EXPKEYSIG, and REVKEYSIG so that a user of %G? gets more information
about the absence of a 'G' on first glance.

Requested-by: Alex <agrambot@gmail.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-formats.txt | 3 ++-
 gpg-interface.c                  | 2 +-
 pretty.c                         | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index c28ff2b919..179c9389aa 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,7 +146,8 @@ endif::git-rev-list[]
 - '%G?': show "G" for a good (valid) signature,
   "B" for a bad signature,
   "U" for a good signature with unknown validity,
-  "X" for a good expired signature, or good signature made by an expired key,
+  "X" for a good signature that has expired,
+  "Y" for a good signature made by an expired key,
   "R" for a good signature made by a revoked key,
   "E" if the signature cannot be checked (e.g. missing key)
   and "N" for no signature
diff --git a/gpg-interface.c b/gpg-interface.c
index 6999e7b469..e44cc27da1 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -35,7 +35,7 @@ static struct {
 	{ 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
 	{ 'E', "\n[GNUPG:] ERRSIG "},
 	{ 'X', "\n[GNUPG:] EXPSIG "},
-	{ 'X', "\n[GNUPG:] EXPKEYSIG "},
+	{ 'Y', "\n[GNUPG:] EXPKEYSIG "},
 	{ 'R', "\n[GNUPG:] REVKEYSIG "},
 };
 
diff --git a/pretty.c b/pretty.c
index 39a36cd825..f98b271069 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1236,6 +1236,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 			case 'U':
 			case 'N':
 			case 'X':
+			case 'Y':
 			case 'R':
 				strbuf_addch(sb, c->signature_check.result);
 			}
-- 
2.10.1-520-ge127bfd383


^ permalink raw reply related

* What's cooking in git.git (Oct 2016, #02; Thu, 6)
From: Junio C Hamano @ 2016-10-06 22:24 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

A handful of topics have been merged to 'master' and it's time to
update the "What's cooking" report.  Linus's and Peff's auto scaling
of default abbreviation length will cook a bit longer in 'next' to
see if anybody screams; there may still be codepaths that assume
that default-abbrev would never be negative and a reasonable
fallback value that we need to locate and fix.

You can find the changes described here in the integration branches
of the repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[Graduated to "master"]

* jc/blame-abbrev (2016-09-28) 1 commit
  (merged to 'next' on 2016-10-03 at 8ec86ff1e1)
 + blame: use DEFAULT_ABBREV macro

 Almost everybody uses DEFAULT_ABBREV to refer to the default
 setting for the abbreviation, but "git blame" peeked into
 underlying variable bypassing the macro for no good reason.


* jk/ambiguous-short-object-names (2016-09-27) 11 commits
  (merged to 'next' on 2016-09-28 at 1b85295323)
 + get_short_sha1: make default disambiguation configurable
 + get_short_sha1: list ambiguous objects on error
 + for_each_abbrev: drop duplicate objects
 + sha1_array: let callbacks interrupt iteration
 + get_short_sha1: mark ambiguity error for translation
 + get_short_sha1: NUL-terminate hex prefix
 + get_short_sha1: refactor init of disambiguation code
 + get_short_sha1: parse tags when looking for treeish
 + get_sha1: propagate flags to child functions
 + get_sha1: avoid repeating ourselves via ONLY_TO_DIE
 + get_sha1: detect buggy calls with multiple disambiguators
 (this branch is used by jk/abbrev-auto and lt/abbrev-auto.)

 When given an abbreviated object name that is not (or more
 realistically, "no longer") unique, we gave a fatal error
 "ambiguous argument".  This error is now accompanied by hints that
 lists the objects that begins with the given prefix.  During the
 course of development of this new feature, numerous minor bugs were
 uncovered and corrected, the most notable one of which is that we
 gave "short SHA1 xxxx is ambiguous." twice without good reason.


* jk/graph-padding-fix (2016-09-29) 1 commit
  (merged to 'next' on 2016-10-03 at 3f526e0f38)
 + graph: fix extra spaces in graph_padding_line

 The "graph" API used in "git log --graph" miscounted the number of
 output columns consumed so far when drawing a padding line, which
 has been fixed; this did not affect any existing code as nobody
 tried to write anything after the padding on such a line, though.


* ps/http-gssapi-cred-delegation (2016-09-29) 1 commit
  (merged to 'next' on 2016-10-03 at 310fbe8f24)
 + http: control GSSAPI credential delegation

 In recent versions of cURL, GSSAPI credential delegation is
 disabled by default due to CVE-2011-2192; introduce a configuration
 to selectively allow enabling this.


* rs/c-auto-resets-attributes (2016-09-29) 1 commit
  (merged to 'next' on 2016-10-03 at 6a0b946a79)
 + pretty: avoid adding reset for %C(auto) if output is empty

 When "%C(auto)" appears at the very beginning of the pretty format
 string, it did not need to issue the reset sequence, but it did.
 This is a small optimization to already graduated topic.


* rs/cocci (2016-10-03) 4 commits
  (merged to 'next' on 2016-10-03 at 758cc6de9c)
 + coccicheck: make transformation for strbuf_addf(sb, "...") more precise
  (merged to 'next' on 2016-09-28 at 26462645f9)
 + use strbuf_add_unique_abbrev() for adding short hashes, part 2
 + use strbuf_addstr() instead of strbuf_addf() with "%s", part 2
 + gitignore: ignore output files of coccicheck make target

 Code clean-up with help from coccinelle tool continues.


* sg/ref-filter-parse-optim (2016-10-03) 1 commit
  (merged to 'next' on 2016-10-03 at 9af6bb63e9)
 + ref-filter: strip format option after a field name only once while parsing

 The code that parses the format parameter of for-each-ref command
 has seen a micro-optimization.


* vn/revision-shorthand-for-side-branch-log (2016-09-27) 1 commit
  (merged to 'next' on 2016-09-28 at c1237b24f6)
 + revision: new rev^-n shorthand for rev^n..rev

 "git log rev^..rev" is an often-used revision range specification
 to show what was done on a side branch merged at rev.  This has
 gained a short-hand "rev^-1".  In general "rev^-$n" is the same as
 "^rev^$n rev", i.e. what has happened on other branches while the
 history leading to nth parent was looking the other way.

--------------------------------------------------
[New Topics]

* jc/ws-error-highlight (2016-10-04) 4 commits
 - diff: introduce diff.wsErrorHighlight option
 - diff.c: move ws-error-highlight parsing helpers up
 - diff.c: refactor parse_ws_error_highlight()
 - t4015: split out the "setup" part of ws-error-highlight test

 "git diff/log --ws-error-highlight=<kind>" lacked the corresponding
 configuration variable to set it by default.

 Undecided.


* jk/abbrev-auto (2016-10-03) 1 commit
 - find_unique_abbrev: move logic out of get_short_sha1()
 (this branch uses lt/abbrev-auto.)

 Updates the way approximate count of total objects is computed
 while attempting to come up with a unique abbreviated object name,
 which in turn needs to estimate how many hexdigits are necessary to
 ensure uniqueness.

 Undecided.


* jk/clone-copy-alternates-fix (2016-10-05) 1 commit
 - clone: detect errors in normalize_path_copy

 "git clone" of a local repository can be done at the filesystem
 level, but the codepath did not check errors while copying and
 adjusting the file that lists alternate object stores.

 Will merge to 'next'.


* nd/commit-p-doc (2016-10-05) 1 commit
 - git-commit.txt: clarify --patch mode with pathspec

 Documentation for "git commit" was updated to clarify that "commit
 -p <paths>" adds to the current contents of the index to come up
 with what to commit.

 Will merge to 'next'.

--------------------------------------------------
[Stalled]

* jc/bundle (2016-03-03) 6 commits
 - index-pack: --clone-bundle option
 - Merge branch 'jc/index-pack' into jc/bundle
 - bundle v3: the beginning
 - bundle: keep a copy of bundle file name in the in-core bundle header
 - bundle: plug resource leak
 - bundle doc: 'verify' is not about verifying the bundle

 The beginning of "split bundle", which could be one of the
 ingredients to allow "git clone" traffic off of the core server
 network to CDN.

 While I think it would make it easier for people to experiment and
 build on if the topic is merged to 'next', I am at the same time a
 bit reluctant to merge an unproven new topic that introduces a new
 file format, which we may end up having to support til the end of
 time.  It is likely that to support a "prime clone from CDN", it
 would need a lot more than just "these are the heads and the pack
 data is over there", so this may not be sufficient.

 Will discard.


* jc/attr (2016-05-25) 18 commits
 - attr: support quoting pathname patterns in C style
 - attr: expose validity check for attribute names
 - attr: add counted string version of git_attr()
 - attr: add counted string version of git_check_attr()
 - attr: retire git_check_attrs() API
 - attr: convert git_check_attrs() callers to use the new API
 - attr: convert git_all_attrs() to use "struct git_attr_check"
 - attr: (re)introduce git_check_attr() and struct git_attr_check
 - attr: rename function and struct related to checking attributes
 - attr.c: plug small leak in parse_attr_line()
 - attr.c: tighten constness around "git_attr" structure
 - attr.c: simplify macroexpand_one()
 - attr.c: mark where #if DEBUG ends more clearly
 - attr.c: complete a sentence in a comment
 - attr.c: explain the lack of attr-name syntax check in parse_attr()
 - attr.c: update a stale comment on "struct match_attr"
 - attr.c: use strchrnul() to scan for one line
 - commit.c: use strchrnul() to scan for one line
 (this branch is used by jc/attr-more, sb/pathspec-label and sb/submodule-default-paths.)

 The attributes API has been updated so that it can later be
 optimized using the knowledge of which attributes are queried.

 I wanted to polish this topic further to make the attribute
 subsystem thread-ready, but because other topics depend on this
 topic and they do not (yet) need it to be thread-ready.

 As the authors of topics that depend on this seem not in a hurry,
 let's discard this and dependent topics and restart them some other
 day.

 Will discard.


* jc/attr-more (2016-06-09) 8 commits
 - attr.c: outline the future plans by heavily commenting
 - attr.c: always pass check[] to collect_some_attrs()
 - attr.c: introduce empty_attr_check_elems()
 - attr.c: correct ugly hack for git_all_attrs()
 - attr.c: rename a local variable check
 - fixup! d5ad6c13
 - attr.c: pass struct git_attr_check down the callchain
 - attr.c: add push_stack() helper
 (this branch uses jc/attr; is tangled with sb/pathspec-label and sb/submodule-default-paths.)

 The beginning of long and tortuous journey to clean-up attribute
 subsystem implementation.

 Needs to be redone.
 Will discard.


* sb/submodule-default-paths (2016-06-20) 5 commits
 - completion: clone can recurse into submodules
 - clone: add --init-submodule=<pathspec> switch
 - submodule update: add `--init-default-path` switch
 - Merge branch 'sb/pathspec-label' into sb/submodule-default-paths
 - Merge branch 'jc/attr' into sb/submodule-default-paths
 (this branch uses jc/attr and sb/pathspec-label; is tangled with jc/attr-more.)

 Allow specifying the set of submodules the user is interested in on
 the command line of "git clone" that clones the superproject.

 Will discard.


* sb/pathspec-label (2016-06-03) 6 commits
 - pathspec: disable preload-index when attribute pathspec magic is in use
 - pathspec: allow escaped query values
 - pathspec: allow querying for attributes
 - pathspec: move prefix check out of the inner loop
 - pathspec: move long magic parsing out of prefix_pathspec
 - Documentation: fix a typo
 (this branch is used by sb/submodule-default-paths; uses jc/attr; is tangled with jc/attr-more.)

 The pathspec mechanism learned ":(attr:X)$pattern" pathspec magic
 to limit paths that match $pattern further by attribute settings.
 The preload-index mechanism is disabled when the new pathspec magic
 is in use (at least for now), because the attribute subsystem is
 not thread-ready.

 Will discard.


* mh/connect (2016-06-06) 10 commits
 - connect: [host:port] is legacy for ssh
 - connect: move ssh command line preparation to a separate function
 - connect: actively reject git:// urls with a user part
 - connect: change the --diag-url output to separate user and host
 - connect: make parse_connect_url() return the user part of the url as a separate value
 - connect: group CONNECT_DIAG_URL handling code
 - connect: make parse_connect_url() return separated host and port
 - connect: re-derive a host:port string from the separate host and port variables
 - connect: call get_host_and_port() earlier
 - connect: document why we sometimes call get_port after get_host_and_port

 Rewrite Git-URL parsing routine (hopefully) without changing any
 behaviour.

 It has been two months without any support.  We may want to discard
 this.


* pb/bisect (2016-08-23) 27 commits
 . bisect--helper: remove the dequote in bisect_start()
 . bisect--helper: retire `--bisect-auto-next` subcommand
 . bisect--helper: retire `--bisect-autostart` subcommand
 . bisect--helper: retire `--check-and-set-terms` subcommand
 . bisect--helper: retire `--bisect-write` subcommand
 . bisect--helper: `bisect_replay` shell function in C
 . bisect--helper: `bisect_log` shell function in C
 . bisect--helper: retire `--write-terms` subcommand
 . bisect--helper: retire `--check-expected-revs` subcommand
 . bisect--helper: `bisect_state` & `bisect_head` shell function in C
 . bisect--helper: `bisect_autostart` shell function in C
 . bisect--helper: retire `--next-all` subcommand
 . bisect--helper: retire `--bisect-clean-state` subcommand
 . bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
 . bisect--helper: `bisect_start` shell function partially in C
 . bisect--helper: `get_terms` & `bisect_terms` shell function in C
 . bisect--helper: `bisect_next_check` & bisect_voc shell function in C
 . bisect--helper: `check_and_set_terms` shell function in C
 . bisect--helper: `bisect_write` shell function in C
 . bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
 . bisect--helper: `bisect_reset` shell function in C
 . wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
 . t6030: explicitly test for bisection cleanup
 . bisect--helper: `bisect_clean_state` shell function in C
 . bisect--helper: `write_terms` shell function in C
 . bisect: rewrite `check_term_format` shell function in C
 . bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

 GSoC "bisect" topic.

 I'd prefer to see early part solidified so that reviews can focus
 on the later part that is still in flux.  We are almost there but
 not quite yet.


* kn/ref-filter-branch-list (2016-05-17) 17 commits
 - branch: implement '--format' option
 - branch: use ref-filter printing APIs
 - branch, tag: use porcelain output
 - ref-filter: allow porcelain to translate messages in the output
 - ref-filter: add `:dir` and `:base` options for ref printing atoms
 - ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
 - ref-filter: introduce symref_atom_parser() and refname_atom_parser()
 - ref-filter: introduce refname_atom_parser_internal()
 - ref-filter: make "%(symref)" atom work with the ':short' modifier
 - ref-filter: add support for %(upstream:track,nobracket)
 - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
 - ref-filter: introduce format_ref_array_item()
 - ref-filter: move get_head_description() from branch.c
 - ref-filter: modify "%(objectname:short)" to take length
 - ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
 - ref-filter: include reference to 'used_atom' within 'atom_value'
 - ref-filter: implement %(if), %(then), and %(else) atoms

 The code to list branches in "git branch" has been consolidated
 with the more generic ref-filter API.

 Rerolled.
 Needs review.


* sb/bisect (2016-04-15) 22 commits
 . SQUASH???
 . bisect: get back halfway shortcut
 . bisect: compute best bisection in compute_relevant_weights()
 . bisect: use a bottom-up traversal to find relevant weights
 . bisect: prepare for different algorithms based on find_all
 . bisect: rename count_distance() to compute_weight()
 . bisect: make total number of commits global
 . bisect: introduce distance_direction()
 . bisect: extract get_distance() function from code duplication
 . bisect: use commit instead of commit list as arguments when appropriate
 . bisect: replace clear_distance() by unique markers
 . bisect: use struct node_data array instead of int array
 . bisect: get rid of recursion in count_distance()
 . bisect: make algorithm behavior independent of DEBUG_BISECT
 . bisect: make bisect compile if DEBUG_BISECT is set
 . bisect: plug the biggest memory leak
 . bisect: add test for the bisect algorithm
 . t6030: generalize test to not rely on current implementation
 . t: use test_cmp_rev() where appropriate
 . t/test-lib-functions.sh: generalize test_cmp_rev
 . bisect: allow 'bisect run' if no good commit is known
 . bisect: write about `bisect next` in documentation

 The internal algorithm used in "git bisect" to find the next commit
 to check has been optimized greatly.

 Was expecting a reroll, but now pb/bisect topic starts removinging
 more and more parts from git-bisect.sh, this needs to see a fresh
 reroll.

 Will discard.
 cf. <1460294354-7031-1-git-send-email-s-beyer@gmx.net>


* sg/completion-updates (2016-02-28) 21 commits
 . completion: cache the path to the repository
 . completion: extract repository discovery from __gitdir()
 . completion: don't guard git executions with __gitdir()
 . completion: consolidate silencing errors from git commands
 . completion: don't use __gitdir() for git commands
 . completion: respect 'git -C <path>'
 . completion: fix completion after 'git -C <path>'
 . completion: don't offer commands when 'git --opt' needs an argument
 . rev-parse: add '--absolute-git-dir' option
 . completion: list short refs from a remote given as a URL
 . completion: don't list 'HEAD' when trying refs completion outside of a repo
 . completion: list refs from remote when remote's name matches a directory
 . completion: respect 'git --git-dir=<path>' when listing remote refs
 . completion: fix most spots not respecting 'git --git-dir=<path>'
 . completion: ensure that the repository path given on the command line exists
 . completion tests: add tests for the __git_refs() helper function
 . completion tests: check __gitdir()'s output in the error cases
 . completion tests: consolidate getting path of current working directory
 . completion tests: make the $cur variable local to the test helper functions
 . completion tests: don't add test cruft to the test repository
 . completion: improve __git_refs()'s in-code documentation

 Has been waiting for a reroll for too long.
 cf. <1456754714-25237-1-git-send-email-szeder@ira.uka.de>

 Will discard.


* ec/annotate-deleted (2015-11-20) 1 commit
 - annotate: skip checking working tree if a revision is provided

 Usability fix for annotate-specific "<file> <rev>" syntax with deleted
 files.

 Has been waiting for a review for too long without seeing anything.

 Will discard.


* dk/gc-more-wo-pack (2016-01-13) 4 commits
 - gc: clean garbage .bitmap files from pack dir
 - t5304: ensure non-garbage files are not deleted
 - t5304: test .bitmap garbage files
 - prepare_packed_git(): find more garbage

 Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
 .bitmap and .keep files.

 Has been waiting for a reroll for too long.
 cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>

 Will discard.


* jc/diff-b-m (2015-02-23) 5 commits
 . WIPWIP
 . WIP: diff-b-m
 - diffcore-rename: allow easier debugging
 - diffcore-rename.c: add locate_rename_src()
 - diffcore-break: allow debugging

 "git diff -B -M" produced incorrect patch when the postimage of a
 completely rewritten file is similar to the preimage of a removed
 file; such a resulting file must not be expressed as a rename from
 other place.

 The fix in this patch is broken, unfortunately.

 Will discard.

--------------------------------------------------
[Cooking]

* nd/ita-empty-commit (2016-09-28) 3 commits
 - commit: don't be fooled by ita entries when creating initial commit
 - diff-lib.c: enable --shift-ita in index_differs_from()
 - Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

 When new paths were added by "git add -N" to the index, it was
 enough to circumvent the check by "git commit" to refrain from
 making an empty commit without "--allow-empty".  The same logic
 prevented "git status" to show such a path as "new file" in the
 "Changes not staged for commit" section.

 Expecting a reroll.
 cf. <xmqqzimrj03j.fsf@gitster.mtv.corp.google.com>
 cf. <xmqq8tubkgg5.fsf@gitster.mtv.corp.google.com>


* lt/abbrev-auto (2016-10-03) 3 commits
  (merged to 'next' on 2016-10-03 at bb188d00f7)
 + abbrev: auto size the default abbreviation
 + abbrev: prepare for new world order
 + abbrev: add FALLBACK_DEFAULT_ABBREV to prepare for auto sizing
 (this branch is used by jk/abbrev-auto.)

 Allow the default abbreviation length, which has historically been
 7, to scale as the repository grows.  The logic suggests to use 12
 hexdigits for the Linux kernel, and 9 to 10 for Git itself.

 Will hold to see if people scream.


* dt/http-empty-auth (2016-10-04) 1 commit
 - http: http.emptyauth should allow empty (not just NULL) usernames

 http.emptyauth configuration is a way to allow an empty username to
 pass when attempting to authenticate using mechanisms like
 Kerberos.  We took an unspecified (NULL) username and sent ":"
 (i.e. no username, no password) to CURLOPT_USERPWD, but did not do
 the same when the username is explicitly set to an empty string.

 Will merge to 'next'.


* jc/diff-unique-abbrev-comments (2016-09-30) 1 commit
 - diff_unique_abbrev(): document its assumption and limitation


* jk/quarantine-received-objects (2016-10-05) 5 commits
 - tmp-objdir: do not migrate files starting with '.'
 - tmp-objdir: put quarantine information in the environment
 - receive-pack: quarantine objects until pre-receive accepts
 - tmp-objdir: introduce API for temporary object directories
 - check_connected: accept an env argument
 (this branch uses jk/alt-odb-cleanup.)

 In order for the receiving end of "git push" to inspect the
 received history and decide to reject the push, the objects sent
 from the sending end need to be made available to the hook and
 the mechanism for the connectivity check, and this was done
 traditionally by storing the objects in the receiving repository
 and letting "git gc" to expire it.  Instead, store the newly
 received objects in a temporary area, and make them available by
 reusing the alternate object store mechanism to them only while we
 decide if we accept the check, and once we decide, either migrate
 them to the repository or purge them immediately.

 Will merge to 'next' after whipping jk/alt-odb-cleanup into shape.


* rs/qsort (2016-10-03) 6 commits
  (merged to 'next' on 2016-10-06 at 32a5bd3c88)
 + show-branch: use QSORT
 + use QSORT, part 2
 + coccicheck: use --all-includes by default
 + remove unnecessary check before QSORT
 + use QSORT
 + add QSORT

 We call "qsort(array, nelem, sizeof(array[0]), fn)", and most of
 the time third parameter is redundant.  A new QSORT() macro lets us
 omit it.

 Will merge to 'master'.


* jk/alt-odb-cleanup (2016-10-05) 19 commits
 - alternates: use fspathcmp to detect duplicates
 - sha1_file: always allow relative paths to alternates
 - count-objects: report alternates via verbose mode
 - fill_sha1_file: write into a strbuf
 - alternates: store scratch buffer as strbuf
 - fill_sha1_file: write "boring" characters
 - alternates: use a separate scratch space
 - alternates: encapsulate alt->base munging
 - alternates: provide helper for allocating alternate
 - alternates: provide helper for adding to alternates list
 - link_alt_odb_entry: refactor string handling
 - link_alt_odb_entry: handle normalize_path errors
 - fixup! t5613: clarify "too deep" recursion tests
 - t5613: clarify "too deep" recursion tests
 - t5613: do not chdir in main process
 - t5613: whitespace/style cleanups
 - t5613: use test_must_fail
 - t5613: drop test_valid_repo function
 - t5613: drop reachable_via function
 (this branch is used by jk/quarantine-received-objects.)

 Will merge to 'next' after squashing the fixup! in.


* va/i18n-perl-scripts (2016-09-25) 11 commits
 - i18n: difftool: mark warnings for translation
 - i18n: send-email: mark string with interpolation for translation
 - i18n: send-email: mark warnings and errors for translation
 - i18n: send-email: mark strings for translation
 - i18n: add--interactive: mark edit_hunk_manually message for translation
 - i18n: add--interactive: i18n of help_patch_cmd
 - i18n: add--interactive: mark message for translation
 - i18n: add--interactive: mark plural strings
 - i18n: add--interactive: mark strings with interpolation for translation
 - i18n: add--interactive: mark simple here documents for translation
 - i18n: add--interactive: mark strings for translation

 Porcelain scripts written in Perl are getting internationalized.

 Waiting for comments.


* jc/latin-1 (2016-09-26) 2 commits
  (merged to 'next' on 2016-09-28 at c8673e03c2)
 + utf8: accept "latin-1" as ISO-8859-1
 + utf8: refactor code to decide fallback encoding

 Some platforms no longer understand "latin-1" that is still seen in
 the wild in e-mail headers; replace them with "iso-8859-1" that is
 more widely known when conversion fails from/to it.

 Will hold to see if people scream.


* mg/gpg-richer-status (2016-10-06) 2 commits
 - SQUASH: gpg-interface: use more status letters
 - gpg-interface: use more status letters

 The GPG verification status shown in "%G?" pretty format specifier
 was not rich enough to differentiate a signature made by an expired
 key, a signature made by a revoked key, etc.  New output letters
 have been assigned to express them.

 Waiting for a response to ping/squash.


* jc/blame-reverse (2016-06-14) 2 commits
  (merged to 'next' on 2016-09-22 at d1a8e9ce99)
 + blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
 + blame: improve diagnosis for "--reverse NEW"

 It is a common mistake to say "git blame --reverse OLD path",
 expecting that the command line is dwimmed as if asking how lines
 in path in an old revision OLD have survived up to the current
 commit.

 Will merge to 'master'.


* js/libify-require-clean-work-tree (2016-10-05) 6 commits
 - wt-status: begin error messages with lower-case
 - wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
 - wt-status: export also the has_un{staged,committed}_changed()
 - wt-status: make the require_clean_work_tree() function reusable
 - pull: make code more similar to the shell script again
 - pull: drop confusing prefix parameter of die_on_unclean_work_tree()

 The require_clean_work_tree() helper was recreated in C when "git
 pull" was rewritten from shell; the helper is now made available to
 other callers in preparation for upcoming "rebase -i" work.

 Will merge to 'next'.


* bw/ls-files-recurse-submodules (2016-10-03) 4 commits
 - ls-files: add pathspec matching for submodules
 - ls-files: pass through safe options for --recurse-submodules
 - ls-files: optionally recurse into submodules
 - git: make super-prefix option

 "git ls-files" learned "--recurse-submodules" option that can be
 used to get a listing of tracked files across submodules (i.e. this
 only works with "--cached" option, not for listing untracked or
 ignored files).  This would be a useful tool to sit on the upstream
 side of a pipe that is read with xargs to work on all working tree
 files from the top-level superproject.

 Looking good.  Is this ready for 'next'?


* ls/filter-process (2016-10-04) 14 commits
 - contrib/long-running-filter: add long running filter example
 - convert: add filter.<driver>.process option
 - convert: prepare filter.<driver>.process option
 - convert: make apply_filter() adhere to standard Git error handling
 - pkt-line: add functions to read/write flush terminated packet streams
 - pkt-line: add packet_write_gently()
 - pkt-line: add packet_flush_gently()
 - pkt-line: add packet_write_fmt_gently()
 - pkt-line: extract set_packet_header()
 - pkt-line: rename packet_write() to packet_write_fmt()
 - run-command: add wait_on_exit
 - run-command: move check_pipe() from write_or_die to run_command
 - convert: modernize tests
 - convert: quote filter names in error messages

 The smudge/clean filter API expect an external process is spawned
 to filter the contents for each path that has a filter defined.  A
 new type of "process" filter API has been added to allow the first
 request to run the filter for a path to spawn a single process, and
 all filtering need is served by this single process for multiple
 paths, reducing the process creation overhead.


* hv/submodule-not-yet-pushed-fix (2016-09-14) 2 commits
 - serialize collection of refs that contain submodule changes
 - serialize collection of changed submodules

 The code in "git push" to compute if any commit being pushed in the
 superproject binds a commit in a submodule that hasn't been pushed
 out was overly inefficient, making it unusable even for a small
 project that does not have any submodule but have a reasonable
 number of refs.

 The last two in the original series seem to break a few tests when
 queued to 'pu', and dropped for now.

 Waiting for a reroll.


* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
 - versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
 - versioncmp: pass full tagnames to swap_prereleases()
 - t7004-tag: add version sort tests to show prerelease reordering issues
 - t7004-tag: use test_config helper
 - t7004-tag: delete unnecessary tags with test_when_finished

 The prereleaseSuffix feature of version comparison that is used in
 "git tag -l" did not correctly when two or more prereleases for the
 same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
 are there and the code needs to compare 2.0-beta1 and 2.0-beta2).

 Waiting for a reroll.
 cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>


* cp/completion-negative-refs (2016-08-24) 1 commit
  (merged to 'next' on 2016-09-22 at abd1585aa6)
 + completion: support excluding refs

 The command-line completion script (in contrib/) learned to
 complete "git cmd ^mas<HT>" to complete the negative end of
 reference to "git cmd ^master".

 Will merge to 'master'.


* sb/push-make-submodule-check-the-default (2016-10-06) 2 commits
 - push: change submodule default to check when submodules exist
 - submodule add: extend force flag to add existing repos

 Turn the default of "push.recurseSubmodules" to "check" when
 submodules seem to be in use.

 Will hold to wait for hv/submodule-not-yet-pushed-fix


* ak/curl-imap-send-explicit-scheme (2016-08-17) 1 commit
  (merged to 'next' on 2016-09-22 at 4449584c26)
 + imap-send: Tell cURL to use imap:// or imaps://

 When we started cURL to talk to imap server when a new enough
 version of cURL library is available, we forgot to explicitly add
 imap(s):// before the destination.  To some folks, that didn't work
 and the library tried to make HTTP(s) requests instead.

 Will merge to 'master'.


* jk/pack-objects-optim-mru (2016-08-11) 4 commits
  (merged to 'next' on 2016-09-21 at 97b919bdbd)
 + pack-objects: use mru list when iterating over packs
 + pack-objects: break delta cycles before delta-search phase
 + sha1_file: make packed_object_info public
 + provide an initializer for "struct object_info"

 Originally merged to 'next' on 2016-08-11

 "git pack-objects" in a repository with many packfiles used to
 spend a lot of time looking for/at objects in them; the accesses to
 the packfiles are now optimized by checking the most-recently-used
 packfile first.

 Will merge to 'master'.


* dp/autoconf-curl-ssl (2016-06-28) 1 commit
  (merged to 'next' on 2016-09-22 at 9c5aeeced9)
 + ./configure.ac: detect SSL in libcurl using curl-config

 The ./configure script generated from configure.ac was taught how
 to detect support of SSL by libcurl better.

 Will merge to 'master'.


* jc/pull-rebase-ff (2016-07-28) 1 commit
 - pull: fast-forward "pull --rebase=true"

 "git pull --rebase", when there is no new commits on our side since
 we forked from the upstream, should be able to fast-forward without
 invoking "git rebase", but it didn't.

 Needs a real log message and a few tests.


* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
  (merged to 'next' on 2016-09-21 at e19148ea63)
 + pathspec: warn on empty strings as pathspec

 Originally merged to 'next' on 2016-07-13

 An empty string used as a pathspec element has always meant
 'everything matches', but it is too easy to write a script that
 finds a path to remove in $path and run 'git rm "$paht"', which
 ends up removing everything.  Start warning about this use of an
 empty string used for 'everything matches' and ask users to use a
 more explicit '.' for that instead.

 The hope is that existing users will not mind this change, and
 eventually the warning can be turned into a hard error, upgrading
 the deprecation into removal of this (mis)feature.

 Will hold to see if people scream.


* nd/shallow-deepen (2016-06-13) 27 commits
  (merged to 'next' on 2016-09-22 at f0cf3e3385)
 + fetch, upload-pack: --deepen=N extends shallow boundary by N commits
 + upload-pack: add get_reachable_list()
 + upload-pack: split check_unreachable() in two, prep for get_reachable_list()
 + t5500, t5539: tests for shallow depth excluding a ref
 + clone: define shallow clone boundary with --shallow-exclude
 + fetch: define shallow boundary with --shallow-exclude
 + upload-pack: support define shallow boundary by excluding revisions
 + refs: add expand_ref()
 + t5500, t5539: tests for shallow depth since a specific date
 + clone: define shallow clone boundary based on time with --shallow-since
 + fetch: define shallow boundary with --shallow-since
 + upload-pack: add deepen-since to cut shallow repos based on time
 + shallow.c: implement a generic shallow boundary finder based on rev-list
 + fetch-pack: use a separate flag for fetch in deepening mode
 + fetch-pack.c: mark strings for translating
 + fetch-pack: use a common function for verbose printing
 + fetch-pack: use skip_prefix() instead of starts_with()
 + upload-pack: move rev-list code out of check_non_tip()
 + upload-pack: make check_non_tip() clean things up on error
 + upload-pack: tighten number parsing at "deepen" lines
 + upload-pack: use skip_prefix() instead of starts_with()
 + upload-pack: move "unshallow" sending code out of deepen()
 + upload-pack: remove unused variable "backup"
 + upload-pack: move "shallow" sending code out of deepen()
 + upload-pack: move shallow deepen code out of receive_needs()
 + transport-helper.c: refactor set_helper_option()
 + remote-curl.c: convert fetch_git() to use argv_array

 The existing "git fetch --depth=<n>" option was hard to use
 correctly when making the history of an existing shallow clone
 deeper.  A new option, "--deepen=<n>", has been added to make this
 easier to use.  "git clone" also learned "--shallow-since=<date>"
 and "--shallow-exclude=<tag>" options to make it easier to specify
 "I am interested only in the recent N months worth of history" and
 "Give me only the history since that version".

 Will merge to 'master'.


* jc/merge-drop-old-syntax (2015-04-29) 1 commit
 - merge: drop 'git merge <message> HEAD <commit>' syntax

 Stop supporting "git merge <message> HEAD <commit>" syntax that has
 been deprecated since October 2007, and issues a deprecation
 warning message since v2.5.0.

 It has been reported that git-gui still uses the deprecated syntax,
 which needs to be fixed before this final step can proceed.
 cf. <5671DB28.8020901@kdbg.org>

 Will merge to 'next'.

--------------------------------------------------
[Discarded]

* jn/fix-connect-unexpected-hangup-diag (2016-09-08) 1 commit
 . connect: tighten check for unexpected early hang up

 Now part of jt/accept-capability-advertisement-when-fetching-from-void
 topic.

^ permalink raw reply

* Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data
From: Jakub Narębski @ 2016-10-06 22:40 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin; +Cc: git, Johannes Sixt
In-Reply-To: <xmqqeg3tjn7n.fsf@gitster.mtv.corp.google.com>

W dniu 06.10.2016 o 21:23, Junio C Hamano pisze:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> If you prefer to accept such sloppy work, I will change it of course,
>> feeling dirty that it has my name on it.
> 
> I do want neither leaks nor sloppyness, and I thought that was
> understood by everybody, hence I thought the last round made it
> clear that _entrust() must not exist.

Also, the *_entrust() mechanism in v1 was quite sequencer-specific,
rather than be a generic mechanism.

> 
> Let's try again.
> 
> We manage lifetime of a field in a structure in one of three ways in
> our codebase [*1*].
> 
>  * A field can always point at a borrowed piece of memory that the
>    destruction of the containing structure or re-assignment to the
>    field do not have to free the current value of the field.  This
>    is a minority, simply because it is rare for a field to be always
>    able to borrow from others.  The destruction of the containing
>    structure or re-assignment to the field needs to do nothing
>    special.
> 
>  * A field can own the piece of memory that it points at.  The
>    destruction of the containing structure or re-assignment to the
>    field needs to free the current value of the field [*2*].  A
>    field that can be assigned a value from the configuration (which
>    is typically xstrdup()'ed) is an example of such a field, and if
>    a command line option also can assign to the field, we'd need to
>    xstrdup() it, even though we know we could borrow from argv[],
>    because cleaning-up needs to be able to free(3) it.
> 
>  * A field can sometimes own and sometimes borrow the memory, and it
>    is accompanied by another field to tell which case it is, so that
>    cleaning-up can tell when it needs to be free(3)d.  This is a
>    minority case, and we generally avoid it especially in modern
>    code for small allocation, as it makes the lifetime rule more
>    complex than it is worth.

Great writeup!

> The _entrust() thing may have been an excellent fourth option if it
> were cost-free.  xstrdup() that the second approach necessitates for
> literal strings (like part of argv[]) would become unnecessary and
> we can treat as if all the fields in a structure were all borrowing
> the memory from elsewhere (in fact, _entrust() creates the missing
> owners of pieces of memory that need to be freed later); essentially
> it turns a "mixed ownership" case into the first approach.
> 
> But it is not cost-free.  The mechanism needs to allocate memory to
> become the missing owner and keep track of which pieces of memory
> need to be freed later, and the resulting code does not become any
> easier to follow.  The programmer still needs to know which ones to
> _entrust() just like the programmer in the model #2 needs to make
> sure xstrdup() is done for appropriate pieces of memory--the only
> difference is that an _entrust() programmer needs to mark the pieces
> of memory to be freed, while a #2 programmer needs to xstrdup() the
> pieces of memory that are being "borrowed".
> 
> So let's not add the fourth way to our codebase.  "mixed ownership"
> case should be turned into "field owns the memory", i.e. approach #2.

On the other hand the _entrust() mechanism might be a good solution
if the amount of memory was large, for example order of magnitude more
than what would be needed to keep ownership info *and* borrowing would
not be possible for some reason.

But the _entrust() mechanism reminds me on one hand side of memory
debuggers like dmalloc, Electric Fence (eFence), or valgrind; combined
a bit with garbage collection.  Namely, when we are in a library call,
record all allocations (perhaps by redirecting alloc functions), then
free those resources at the exit of library call.
 
> 
> [Footnotes]
> 
> *1* It is normal for different fields in the same structure follow
>     different lifetime rules.
> 
> *2* Unless leaks are allowed, that is.  I think we have instances
>     where "git cmd --opt=A --opt=B" allocates and stores a new piece
>     of memory that is computed based on A and store it to a field,
>     and then overwrites the field with another allocation of a value
>     based on B without freeing old value, saying "the caller can
>     avoid passing the same thing twice, and such a leak is miniscule
>     anyway".  That is not nice, and we've been reducing them over
>     time.
> 


^ permalink raw reply

* Re: Regression: git no longer works with musl libc's regex impl
From: Ramsay Jones @ 2016-10-06 22:42 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, Johannes Schindelin
  Cc: Rich Felker, Jeff King, Git, musl
In-Reply-To: <CACBZZX4XPqZauD_M_ieOwVauT1fi3MQb4+6taELQaRG9M-Kz_w@mail.gmail.com>



On 06/10/16 20:18, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> As to making NO_REGEX conditional on REG_STARTEND: you are talking about
>> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
>> is a C preprocessor macro.
>>
>> Unless you can convince the rest of the Git developers (you would not
>> convince me) to simulate autoconf by compiling an executable every time
>> `make` is run, to determine whether REG_STARTEND is defined, this is a
>> no-go.
> 
> But just to clarify, does anyone have any objection to making our
> configure.ac compile a C program to check for this sort of thing?
> Because that seems like the easiest solution to this class of problem.

Err, you do know that we already do that, right?

[see commit a1e3b669 ("autoconf: don't use platform regex if it lacks REG_STARTEND", 17-08-2010)]

In fact, if you run the auto tools on cygwin, you get a different setting
for NO_REGEX than via config.mak.uname. Which is why I don't run configure
on cygwin. :-D

[The issue is exposed by t7008-grep-binary.sh, where the cygwin native
regex library matches '.' in a pattern with the NUL character. ie the
test_expect_failure test passes.]

ATB,
Ramsay Jones



^ permalink raw reply

* Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data
From: Junio C Hamano @ 2016-10-06 22:53 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: Johannes Schindelin, git, Johannes Sixt
In-Reply-To: <28935e27-5ba8-c261-ba44-424f7b91cdda@gmail.com>

Jakub Narębski <jnareb@gmail.com> writes:

>> We manage lifetime of a field in a structure in one of three ways in
>> our codebase [*1*].
>> 
>>  ...
>>  * A field can sometimes own and sometimes borrow the memory, and it
>>    is accompanied by another field to tell which case it is, so that
>>    cleaning-up can tell when it needs to be free(3)d.  This is a
>>    minority case, and we generally avoid it especially in modern
>>    code for small allocation, as it makes the lifetime rule more
>>    complex than it is worth.
> ...
> On the other hand the _entrust() mechanism might be a good solution
> if the amount of memory was large, for example order of magnitude more
> than what would be needed to keep ownership info *and* borrowing would
> not be possible for some reason.

We have approach #3 exactly for that usage pattern.

^ permalink raw reply

* [PATCHv5] push: change submodule default to check when submodules exist
From: Stefan Beller @ 2016-10-06 23:41 UTC (permalink / raw)
  To: gitster; +Cc: git, hvoigt, torvalds, peff, Stefan Beller
In-Reply-To: <xmqqoa2xi5rd.fsf@gitster.mtv.corp.google.com>

When working with submodules, it is easy to forget to push the submodules.
The setting 'check', which checks if any existing submodule is present on
at least one remote of the submodule remotes, is designed to prevent this
mistake.

Flipping the default to check for submodules is safer than the current
default of ignoring submodules while pushing.

However checking for submodules requires additional work[1], which annoys
users that do not use submodules, so we turn on the check for submodules
based on a cheap heuristic, the existence of
* any initialized submodules via checking submodule.<name>.url.
* the .git/modules directory. That directory doesn't exist when no
  submodules are used and is only created and populated when submodules
  are cloned/added.
* the existence of the .gitmodules file.

When the submodule directory doesn't exist, a user may have changed the
gitlinks via plumbing commands. Currently the default is to not check.
RECURSE_SUBMODULES_DEFAULT is effectively RECURSE_SUBMODULES_OFF currently,
though it may change in the future. When no submodules exist such a check
is pointless as it would fail anyway, so let's just turn it off.

[1] https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com/

Signed-off-by: Stefan Beller <sbeller@google.com>
---

 * Reworded the commit message
 * added comment on why we only check submodule.<name>.url

 The first patch of the series is still good, so please use
 https://public-inbox.org/git/20161006193725.31553-2-sbeller@google.com/
 first and then build this on top.
 
 Thanks,
 Stefan 

 builtin/push.c                 | 16 +++++++++++++++-
 t/t5531-deep-submodule-push.sh |  6 +++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..9e0b8db 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "refs.h"
+#include "dir.h"
 #include "run-command.h"
 #include "builtin.h"
 #include "remote.h"
@@ -22,6 +23,7 @@ static int deleterefs;
 static const char *receivepack;
 static int verbosity;
 static int progress = -1;
+static int has_submodules_configured;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static enum transport_family family;
 
@@ -31,6 +33,15 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
 
+static void preset_submodule_default(void)
+{
+	if (has_submodules_configured || file_exists(git_path("modules")) ||
+	    (!is_bare_repository() && file_exists(".gitmodules")))
+		recurse_submodules = RECURSE_SUBMODULES_CHECK;
+	else
+		recurse_submodules = RECURSE_SUBMODULES_OFF;
+}
+
 static void add_refspec(const char *ref)
 {
 	refspec_nr++;
@@ -495,7 +506,9 @@ static int git_push_config(const char *k, const char *v, void *cb)
 		const char *value;
 		if (!git_config_get_value("push.recursesubmodules", &value))
 			recurse_submodules = parse_push_recurse_submodules_arg(k, value);
-	}
+	} else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
+		/* The submodule.<name>.url is used as a bit to indicate existence */
+		has_submodules_configured = 1;
 
 	return git_default_config(k, v, NULL);
 }
@@ -552,6 +565,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	};
 
 	packet_trace_identity("push");
+	preset_submodule_default();
 	git_config(git_push_config, &flags);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 	set_push_cert_flags(&flags, push_cert);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 198ce84..e690749 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on remote' '
 		git add gar/bage &&
 		git commit -m "Third commit for gar/bage" &&
 		# the push should fail with --recurse-submodules=check
-		# on the command line...
+		# on the command line. "check" is the default for repos in
+		# which submodules are detected by existence of config,
+		# .gitmodules file or an internal .git/modules/<submodule-repo>
+		git submodule add -f ../submodule.git gar/bage &&
+		test_must_fail git push ../pub.git master &&
 		test_must_fail git push --recurse-submodules=check ../pub.git master &&
 
 		# ...or if specified in the configuration..
-- 
2.10.1.353.g1629400


^ permalink raw reply related

* [PATCH 1/4] mergetool: add copyright
From: David Aguilar @ 2016-10-06 23:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..300ce7f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -3,6 +3,7 @@
 # This program resolves merge conflicts in git
 #
 # Copyright (c) 2006 Theodore Y. Ts'o
+# Copyright (c) 2009-2016 David Aguilar
 #
 # This file is licensed under the GPL v2, or a later version
 # at the discretion of Junio C Hamano.
-- 
2.10.1.355.g33afd83.dirty


^ permalink raw reply related

* [PATCH 2/4] mergetool: move main program flow into a main() function
From: David Aguilar @ 2016-10-06 23:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
In-Reply-To: <1475797679-32712-1-git-send-email-davvid@gmail.com>

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool.sh | 180 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 93 insertions(+), 87 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 300ce7f..b2cd0a4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -366,51 +366,6 @@ merge_file () {
 	return 0
 }
 
-prompt=$(git config --bool mergetool.prompt)
-guessed_merge_tool=false
-
-while test $# != 0
-do
-	case "$1" in
-	--tool-help=*)
-		TOOL_MODE=${1#--tool-help=}
-		show_tool_help
-		;;
-	--tool-help)
-		show_tool_help
-		;;
-	-t|--tool*)
-		case "$#,$1" in
-		*,*=*)
-			merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
-			;;
-		1,*)
-			usage ;;
-		*)
-			merge_tool="$2"
-			shift ;;
-		esac
-		;;
-	-y|--no-prompt)
-		prompt=false
-		;;
-	--prompt)
-		prompt=true
-		;;
-	--)
-		shift
-		break
-		;;
-	-*)
-		usage
-		;;
-	*)
-		break
-		;;
-	esac
-	shift
-done
-
 prompt_after_failed_merge () {
 	while true
 	do
@@ -427,57 +382,108 @@ prompt_after_failed_merge () {
 	done
 }
 
-git_dir_init
-require_work_tree
+main () {
+	prompt=$(git config --bool mergetool.prompt)
+	guessed_merge_tool=false
+
+	while test $# != 0
+	do
+		case "$1" in
+		--tool-help=*)
+			TOOL_MODE=${1#--tool-help=}
+			show_tool_help
+			;;
+		--tool-help)
+			show_tool_help
+			;;
+		-t|--tool*)
+			case "$#,$1" in
+			*,*=*)
+				merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
+				;;
+			1,*)
+				usage ;;
+			*)
+				merge_tool="$2"
+				shift ;;
+			esac
+			;;
+		-y|--no-prompt)
+			prompt=false
+			;;
+		--prompt)
+			prompt=true
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	git_dir_init
+	require_work_tree
 
-if test -z "$merge_tool"
-then
-	# Check if a merge tool has been configured
-	merge_tool=$(get_configured_merge_tool)
-	# Try to guess an appropriate merge tool if no tool has been set.
 	if test -z "$merge_tool"
 	then
-		merge_tool=$(guess_merge_tool) || exit
-		guessed_merge_tool=true
+		# Check if a merge tool has been configured
+		merge_tool=$(get_configured_merge_tool)
+		# Try to guess an appropriate merge tool if no tool has been set.
+		if test -z "$merge_tool"
+		then
+			merge_tool=$(guess_merge_tool) || exit
+			guessed_merge_tool=true
+		fi
 	fi
-fi
-merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
-merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-files=
+	merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
+	merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
 
-if test $# -eq 0
-then
-	cd_to_toplevel
+	files=
 
-	if test -e "$GIT_DIR/MERGE_RR"
+	if test $# -eq 0
 	then
-		files=$(git rerere remaining)
+		cd_to_toplevel
+
+		if test -e "$GIT_DIR/MERGE_RR"
+		then
+			files=$(git rerere remaining)
+		else
+			files=$(git ls-files -u |
+				sed -e 's/^[^	]*	//' | sort -u)
+		fi
 	else
-		files=$(git ls-files -u | sed -e 's/^[^	]*	//' | sort -u)
+		files=$(git ls-files -u -- "$@" |
+			sed -e 's/^[^	]*	//' | sort -u)
 	fi
-else
-	files=$(git ls-files -u -- "$@" | sed -e 's/^[^	]*	//' | sort -u)
-fi
-
-if test -z "$files"
-then
-	echo "No files need merging"
-	exit 0
-fi
-
-printf "Merging:\n"
-printf "%s\n" "$files"
-
-rc=0
-for i in $files
-do
-	printf "\n"
-	if ! merge_file "$i"
+
+	if test -z "$files"
 	then
-		rc=1
-		prompt_after_failed_merge || exit 1
+		echo "No files need merging"
+		exit 0
 	fi
-done
 
-exit $rc
+	printf "Merging:\n"
+	printf "%s\n" "$files"
+
+	rc=0
+	for i in $files
+	do
+		printf "\n"
+		if ! merge_file "$i"
+		then
+			rc=1
+			prompt_after_failed_merge || exit 1
+		fi
+	done
+
+	exit $rc
+}
+
+main "$@"
-- 
2.10.1.355.g33afd83.dirty


^ permalink raw reply related

* [PATCH 3/4] mergetool: honor diff.orderFile
From: David Aguilar @ 2016-10-06 23:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
In-Reply-To: <1475797679-32712-1-git-send-email-davvid@gmail.com>

Teach mergetool to get the list of files to edit via `diff` so that we
gain support for diff.orderFile.

Suggested-by: Luis Gutierrez <luisgutz@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Documentation/git-mergetool.txt |  5 +++++
 git-mergetool.sh                | 30 +++++++++++++++---------------
 t/t7610-mergetool.sh            | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..c4c1a9b 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,11 @@ success of the resolution after the custom tool has exited.
 	Prompt before each invocation of the merge resolution program
 	to give the user a chance to skip the path.
 
+DIFF ORDER FILES
+----------------
+`git mergetool` honors the `diff.orderFile` configuration variable
+used by `git diff`.  See linkgit:git-config[1] for more details.
+
 TEMPORARY FILES
 ---------------
 `git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b2cd0a4..65696d8 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -382,6 +382,11 @@ prompt_after_failed_merge () {
 	done
 }
 
+print_noop_and_exit () {
+	echo "No files need merging"
+	exit 0
+}
+
 main () {
 	prompt=$(git config --bool mergetool.prompt)
 	guessed_merge_tool=false
@@ -445,28 +450,23 @@ main () {
 	merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
 	merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
 
-	files=
-
-	if test $# -eq 0
+	if test $# -eq 0 && test -e "$GIT_DIR/MERGE_RR"
 	then
-		cd_to_toplevel
-
-		if test -e "$GIT_DIR/MERGE_RR"
+		set -- $(git rerere remaining)
+		if test $# -eq 0
 		then
-			files=$(git rerere remaining)
-		else
-			files=$(git ls-files -u |
-				sed -e 's/^[^	]*	//' | sort -u)
+			print_noop_and_exit
 		fi
-	else
-		files=$(git ls-files -u -- "$@" |
-			sed -e 's/^[^	]*	//' | sort -u)
 	fi
 
+	files=$(git -c core.quotePath=false \
+		diff --name-only --diff-filter=U -- "$@")
+
+	cd_to_toplevel
+
 	if test -z "$files"
 	then
-		echo "No files need merging"
-		exit 0
+		print_noop_and_exit
 	fi
 
 	printf "Merging:\n"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 7217f39..fb2aeef 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -606,4 +606,37 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
 	git reset --hard master >/dev/null 2>&1
 '
 
+test_expect_success 'diff.orderFile configuration is honored' '
+	test_config diff.orderFile order-file &&
+	test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+	test_config mergetool.myecho.trustExitCode true &&
+	echo b >order-file &&
+	echo a >>order-file &&
+	git checkout -b order-file-start master &&
+	echo start >a &&
+	echo start >b &&
+	git add a b &&
+	git commit -m start &&
+	git checkout -b order-file-side1 order-file-start &&
+	echo side1 >a &&
+	echo side1 >b &&
+	git add a b &&
+	git commit -m side1 &&
+	git checkout -b order-file-side2 order-file-start &&
+	echo side2 >a &&
+	echo side2 >b &&
+	git add a b &&
+	git commit -m side2 &&
+	test_must_fail git merge order-file-side1 &&
+	cat >expect <<-\EOF &&
+		Merging:
+		b
+		a
+	EOF
+	git mergetool --no-prompt --tool myecho | grep -A 2 Merging: >actual &&
+	test_cmp expect actual &&
+	git reset --hard >/dev/null
+'
+'
+
 test_done
-- 
2.10.1.355.g33afd83.dirty


^ permalink raw reply related

* [PATCH 4/4] mergetool: honor -O<order-file>
From: David Aguilar @ 2016-10-06 23:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
In-Reply-To: <1475797679-32712-1-git-send-email-davvid@gmail.com>

Teach mergetool to pass "-O<order-file>" down to `git diff` when
specified on the command-line.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Documentation/git-mergetool.txt | 10 ++++++----
 git-mergetool.sh                | 14 ++++++++++++--
 t/t7610-mergetool.sh            | 27 +++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c4c1a9b..3622d66 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,10 +79,12 @@ success of the resolution after the custom tool has exited.
 	Prompt before each invocation of the merge resolution program
 	to give the user a chance to skip the path.
 
-DIFF ORDER FILES
-----------------
-`git mergetool` honors the `diff.orderFile` configuration variable
-used by `git diff`.  See linkgit:git-config[1] for more details.
+-O<orderfile>::
+	Process files in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+	This overrides the `diff.orderFile` configuration variable
+	(see linkgit:git-config[1]).  To cancel `diff.orderFile`,
+	use `-O/dev/null`.
 
 TEMPORARY FILES
 ---------------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..9dca58b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
 # at the discretion of Junio C Hamano.
 #
 
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O<order-file>] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
 main () {
 	prompt=$(git config --bool mergetool.prompt)
 	guessed_merge_tool=false
+	order_file=
 
 	while test $# != 0
 	do
@@ -419,6 +420,9 @@ main () {
 		--prompt)
 			prompt=true
 			;;
+		-O*)
+			order_file="$1"
+			;;
 		--)
 			shift
 			break
@@ -459,8 +463,14 @@ main () {
 		fi
 	fi
 
+	if test -n "$order_file"
+	then
+		set -- "$order_file" -- "$@"
+	else
+		set -- -- "$@"
+	fi
 	files=$(git -c core.quotePath=false \
-		diff --name-only --diff-filter=U -- "$@")
+		diff --name-only --diff-filter=U "$@")
 
 	cd_to_toplevel
 
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index fb2aeef..0f9869a 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -637,6 +637,33 @@ test_expect_success 'diff.orderFile configuration is honored' '
 	test_cmp expect actual &&
 	git reset --hard >/dev/null
 '
+
+test_expect_success 'mergetool -Oorder-file is honored' '
+	test_config diff.orderFile order-file &&
+	test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+	test_config mergetool.myecho.trustExitCode true &&
+	test_must_fail git merge order-file-side1 &&
+	cat >expect <<-\EOF &&
+		Merging:
+		a
+		b
+	EOF
+	git mergetool -O/dev/null --no-prompt --tool myecho |
+	grep -A 2 Merging: >actual &&
+	test_cmp expect actual &&
+	git reset --hard >/dev/null 2>&1 &&
+
+	git config --unset diff.orderFile &&
+	test_must_fail git merge order-file-side1 &&
+	cat >expect <<-\EOF &&
+		Merging:
+		b
+		a
+	EOF
+	git mergetool -Oorder-file --no-prompt --tool myecho |
+	grep -A 2 Merging: >actual &&
+	test_cmp expect actual &&
+	git reset --hard >/dev/null 2>&1
 '
 
 test_done
-- 
2.10.1.355.g33afd83.dirty


^ permalink raw reply related

* [PATCH] documentation: clarify submodule.<name>.[url, path] variables
From: Stefan Beller @ 2016-10-06 23:51 UTC (permalink / raw)
  To: gitster; +Cc: git, Jens.Lehmann, hvoigt, Stefan Beller

Signed-off-by: Stefan Beller <sbeller@google.com>
---

This was raised in the discussion of 
https://public-inbox.org/git/20161006193725.31553-1-sbeller@google.com/raw

However this can go as a separate patch instead of adding it to the series.

Thanks,
Stefan

 Documentation/config.txt | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e78293b..1f68d05 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2812,11 +2812,18 @@ stash.showStat::
 	See description of 'show' command in linkgit:git-stash[1].
 
 submodule.<name>.path::
+	The path within this project for a submodule. This variable is
+	kept in the .gitmodules file. See linkgit:git-submodule[1] and
+	linkgit:gitmodules[5] for details.
+
 submodule.<name>.url::
-	The path within this project and URL for a submodule. These
-	variables are initially populated by 'git submodule init'. See
-	linkgit:git-submodule[1] and linkgit:gitmodules[5] for
-	details.
+	The URL for a submodule. This variable is copied from the .gitmodules
+	file to the git config via 'git submodule init'. The user can change
+	the configured URL before obtaining the submodule via 'git submodule
+	update'. After obtaining the submodule, this variable is kept in the
+	config as a boolean flag to indicate whether the submodule is of
+	interest to git commands.  See linkgit:git-submodule[1] and
+	linkgit:gitmodules[5] for details.
 
 submodule.<name>.update::
 	The default update procedure for a submodule. This variable
-- 
2.10.1.353.g1629400


^ permalink raw reply related

* Re: [PATCH 2/2] use strbuf_add_unique_abbrev() for adding short hashes, part 2
From: Jeff King @ 2016-10-07  0:46 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <29e75b7b-6dd0-8c52-e444-cad1ba613cd0@web.de>

On Tue, Sep 27, 2016 at 09:11:58PM +0200, René Scharfe wrote:

> Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
> instead of taking detours through find_unique_abbrev() and its static
> buffer.  This is shorter and a bit more efficient.
> [...]
> diff --git a/diff.c b/diff.c
> index a178ed3..be11e4e 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3109,7 +3109,7 @@ static void fill_metainfo(struct strbuf *msg,
>  		}
>  		strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
>  			    find_unique_abbrev(one->oid.hash, abbrev));
> -		strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
> +		strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);
>  		if (one->mode == two->mode)
>  			strbuf_addf(msg, " %06o", one->mode);
>  		strbuf_addf(msg, "%s\n", reset);

This one is an interesting case, and maybe a good example of why blind
coccinelle usage can have some pitfalls. :)

We get rid of the strbuf_addstr(), but notice that we leave untouched
the find_unique_abbrev() call immediately above. There was a symmetry to
the two that has been lost.

Probably either:

  strbuf_addf(msg, "%s%sindex %s..%s", line_prefix, set
	find_unique_abbrev(one->oid.hash, abbrev),
	find_unique_abbrev(two->oid.hash, abbrev));

or:

  strbuf_addf(msg, "%s%sindex ", line_prefix, set);
  strbuf_add_unique_abbrev(msg, one->oid.hash, abbrev);
  strbuf_addstr(msg, "..");
  strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);

would be a more appropriate refactoring. The problem is in the original
patch (which also lacks symmetry; either this predates the multi-buffer
find_unique_abbrev, or the original author didn't know about it), but I
think your refactor makes it slightly worse.

I noticed because I have another series which touches these lines, and
it wants to symmetrically swap out find_unique_abbrev for something
else. :) I don't think it's a big enough deal to switch now (and I've
already rebased my series which will touch these lines), but I wanted to
mention it as a thing to watch out for as we do more of these kinds of
automated transformations.

> --- a/submodule.c
> +++ b/submodule.c
> @@ -396,7 +396,7 @@ static void show_submodule_header(FILE *f, const char *path,
>  			find_unique_abbrev(one->hash, DEFAULT_ABBREV));
>  	if (!fast_backward && !fast_forward)
>  		strbuf_addch(&sb, '.');
> -	strbuf_addstr(&sb, find_unique_abbrev(two->hash, DEFAULT_ABBREV));
> +	strbuf_add_unique_abbrev(&sb->hash, two, DEFAULT_ABBREV);

This one is a similar situation, I think.

-Peff

^ permalink raw reply

* [PATCH v2 4/4] mergetool: honor -O<orderfile>
From: David Aguilar @ 2016-10-07  4:05 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez

Teach mergetool to pass "-O<orderfile>" down to `git diff` when
specified on the command-line.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
This is a replacement patch for 4/4 from the original series.

The changes are stylistic -- the "order_file" variable name and
"-O<order-file>" in the usage were changed to "orderfile" and
"-O<orderfile>" for consistency with the documentation.

 Documentation/git-mergetool.txt | 10 ++++++----
 git-mergetool.sh                | 14 ++++++++++++--
 t/t7610-mergetool.sh            | 27 +++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c4c1a9b..3622d66 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,10 +79,12 @@ success of the resolution after the custom tool has exited.
 	Prompt before each invocation of the merge resolution program
 	to give the user a chance to skip the path.
 
-DIFF ORDER FILES
-----------------
-`git mergetool` honors the `diff.orderFile` configuration variable
-used by `git diff`.  See linkgit:git-config[1] for more details.
+-O<orderfile>::
+	Process files in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+	This overrides the `diff.orderFile` configuration variable
+	(see linkgit:git-config[1]).  To cancel `diff.orderFile`,
+	use `-O/dev/null`.
 
 TEMPORARY FILES
 ---------------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..1a0fe7a 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
 # at the discretion of Junio C Hamano.
 #
 
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O<orderfile>] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
 main () {
 	prompt=$(git config --bool mergetool.prompt)
 	guessed_merge_tool=false
+	orderfile=
 
 	while test $# != 0
 	do
@@ -419,6 +420,9 @@ main () {
 		--prompt)
 			prompt=true
 			;;
+		-O*)
+			orderfile="$1"
+			;;
 		--)
 			shift
 			break
@@ -459,8 +463,14 @@ main () {
 		fi
 	fi
 
+	if test -n "$orderfile"
+	then
+		set -- "$orderfile" -- "$@"
+	else
+		set -- -- "$@"
+	fi
 	files=$(git -c core.quotePath=false \
-		diff --name-only --diff-filter=U -- "$@")
+		diff --name-only --diff-filter=U "$@")
 
 	cd_to_toplevel
 
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index fb2aeef..0f9869a 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -637,6 +637,33 @@ test_expect_success 'diff.orderFile configuration is honored' '
 	test_cmp expect actual &&
 	git reset --hard >/dev/null
 '
+
+test_expect_success 'mergetool -Oorder-file is honored' '
+	test_config diff.orderFile order-file &&
+	test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+	test_config mergetool.myecho.trustExitCode true &&
+	test_must_fail git merge order-file-side1 &&
+	cat >expect <<-\EOF &&
+		Merging:
+		a
+		b
+	EOF
+	git mergetool -O/dev/null --no-prompt --tool myecho |
+	grep -A 2 Merging: >actual &&
+	test_cmp expect actual &&
+	git reset --hard >/dev/null 2>&1 &&
+
+	git config --unset diff.orderFile &&
+	test_must_fail git merge order-file-side1 &&
+	cat >expect <<-\EOF &&
+		Merging:
+		b
+		a
+	EOF
+	git mergetool -Oorder-file --no-prompt --tool myecho |
+	grep -A 2 Merging: >actual &&
+	test_cmp expect actual &&
+	git reset --hard >/dev/null 2>&1
 '
 
 test_done
-- 
2.10.1.386.g42aabb0


^ permalink raw reply related

* Re: What's cooking in git.git (Oct 2016, #02; Thu, 6)
From: Kevin Daudt @ 2016-10-07  4:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqd1jdi0a6.fsf@gitster.mtv.corp.google.com>

On Thu, Oct 06, 2016 at 03:24:17PM -0700, Junio C Hamano wrote:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.  The ones marked with '.' do not appear in any of
> the integration branches, but I am still holding onto them.
> 
> A handful of topics have been merged to 'master' and it's time to
> update the "What's cooking" report.  Linus's and Peff's auto scaling
> of default abbreviation length will cook a bit longer in 'next' to
> see if anybody screams; there may still be codepaths that assume
> that default-abbrev would never be negative and a reasonable
> fallback value that we need to locate and fix.
> 
> You can find the changes described here in the integration branches
> of the repositories listed at
> 
>     http://git-blame.blogspot.com/p/git-public-repositories.html
> 
> --------------------------------------------------
> [Graduated to "master"]
> 

Just wondering why the topic "kd/mailinfo-quoted-string (2016-09-28) 2
commits" is not listed anymore. Previous what's cooking said it was
merged to "next".

^ permalink raw reply

* Re: [PATCH 2/4] mergetool: move main program flow into a main() function
From: Johannes Sixt @ 2016-10-07  5:51 UTC (permalink / raw)
  To: David Aguilar; +Cc: Git Mailing List, Junio C Hamano, Luis Gutierrez
In-Reply-To: <1475797679-32712-2-git-send-email-davvid@gmail.com>

Am 07.10.2016 um 01:47 schrieb David Aguilar:
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---

An answer to "why?" is missing here. I guess it is just so that 
everything happens in functions, and that there is only the invocation 
of main at the top-level of the script. I am unsure whether this is 
sufficient justification; you are adding a level of indentation after all.

-- Hannes


^ permalink raw reply

* [PATCH v2 2/4] mergetool: move main program flow into a main() function
From: David Aguilar @ 2016-10-07  6:24 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez

Make it easier to follow the program's flow by isolating all
logic into functions.  Isolate the main execution code path into
a single unit instead of having prompt_after_failed_merge()
interrupt it partyway through.

The use of a main() function is borrowing a convention from C,
Python, Perl, and many other languages.  This helps readers more
familiar with other languages understand the purpose of each
function when diving into the codebase with fresh eyes.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
As suggested by Hannes, v2 provides a better commit message.

This is a stylistic choice, but since I was about to start hacking on it,
I couldn't help but notice the organic sprawl that could use some tidying
before adding new features.

 git-mergetool.sh | 180 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 93 insertions(+), 87 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 300ce7f..b2cd0a4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -366,51 +366,6 @@ merge_file () {
 	return 0
 }
 
-prompt=$(git config --bool mergetool.prompt)
-guessed_merge_tool=false
-
-while test $# != 0
-do
-	case "$1" in
-	--tool-help=*)
-		TOOL_MODE=${1#--tool-help=}
-		show_tool_help
-		;;
-	--tool-help)
-		show_tool_help
-		;;
-	-t|--tool*)
-		case "$#,$1" in
-		*,*=*)
-			merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
-			;;
-		1,*)
-			usage ;;
-		*)
-			merge_tool="$2"
-			shift ;;
-		esac
-		;;
-	-y|--no-prompt)
-		prompt=false
-		;;
-	--prompt)
-		prompt=true
-		;;
-	--)
-		shift
-		break
-		;;
-	-*)
-		usage
-		;;
-	*)
-		break
-		;;
-	esac
-	shift
-done
-
 prompt_after_failed_merge () {
 	while true
 	do
@@ -427,57 +382,108 @@ prompt_after_failed_merge () {
 	done
 }
 
-git_dir_init
-require_work_tree
+main () {
+	prompt=$(git config --bool mergetool.prompt)
+	guessed_merge_tool=false
+
+	while test $# != 0
+	do
+		case "$1" in
+		--tool-help=*)
+			TOOL_MODE=${1#--tool-help=}
+			show_tool_help
+			;;
+		--tool-help)
+			show_tool_help
+			;;
+		-t|--tool*)
+			case "$#,$1" in
+			*,*=*)
+				merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
+				;;
+			1,*)
+				usage ;;
+			*)
+				merge_tool="$2"
+				shift ;;
+			esac
+			;;
+		-y|--no-prompt)
+			prompt=false
+			;;
+		--prompt)
+			prompt=true
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	git_dir_init
+	require_work_tree
 
-if test -z "$merge_tool"
-then
-	# Check if a merge tool has been configured
-	merge_tool=$(get_configured_merge_tool)
-	# Try to guess an appropriate merge tool if no tool has been set.
 	if test -z "$merge_tool"
 	then
-		merge_tool=$(guess_merge_tool) || exit
-		guessed_merge_tool=true
+		# Check if a merge tool has been configured
+		merge_tool=$(get_configured_merge_tool)
+		# Try to guess an appropriate merge tool if no tool has been set.
+		if test -z "$merge_tool"
+		then
+			merge_tool=$(guess_merge_tool) || exit
+			guessed_merge_tool=true
+		fi
 	fi
-fi
-merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
-merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-files=
+	merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
+	merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
 
-if test $# -eq 0
-then
-	cd_to_toplevel
+	files=
 
-	if test -e "$GIT_DIR/MERGE_RR"
+	if test $# -eq 0
 	then
-		files=$(git rerere remaining)
+		cd_to_toplevel
+
+		if test -e "$GIT_DIR/MERGE_RR"
+		then
+			files=$(git rerere remaining)
+		else
+			files=$(git ls-files -u |
+				sed -e 's/^[^	]*	//' | sort -u)
+		fi
 	else
-		files=$(git ls-files -u | sed -e 's/^[^	]*	//' | sort -u)
+		files=$(git ls-files -u -- "$@" |
+			sed -e 's/^[^	]*	//' | sort -u)
 	fi
-else
-	files=$(git ls-files -u -- "$@" | sed -e 's/^[^	]*	//' | sort -u)
-fi
-
-if test -z "$files"
-then
-	echo "No files need merging"
-	exit 0
-fi
-
-printf "Merging:\n"
-printf "%s\n" "$files"
-
-rc=0
-for i in $files
-do
-	printf "\n"
-	if ! merge_file "$i"
+
+	if test -z "$files"
 	then
-		rc=1
-		prompt_after_failed_merge || exit 1
+		echo "No files need merging"
+		exit 0
 	fi
-done
 
-exit $rc
+	printf "Merging:\n"
+	printf "%s\n" "$files"
+
+	rc=0
+	for i in $files
+	do
+		printf "\n"
+		if ! merge_file "$i"
+		then
+			rc=1
+			prompt_after_failed_merge || exit 1
+		fi
+	done
+
+	exit $rc
+}
+
+main "$@"
-- 
2.10.1.386.g42aabb0


^ permalink raw reply related

* [PATCH] clean up confusing suggestion for commit references
From: Heiko Voigt @ 2016-10-07  9:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The description for referencing commits looks as if it is contradicting
the example, since it is itself enclosed in double quotes. Lets use
single quotes around the description and include the double quotes in
the description so it matches the example.
---
Sorry for opening this up again but I just looked up the format and was
like: "Umm, which one is now the correct one..."

For this makes more sense. What do others think?

Cheers Heiko

 Documentation/SubmittingPatches | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..692f4ce 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -122,7 +122,7 @@ without external resources. Instead of giving a URL to a mailing list
 archive, summarize the relevant points of the discussion.
 
 If you want to reference a previous commit in the history of a stable
-branch, use the format "abbreviated sha1 (subject, date)",
+branch, use the format 'abbreviated sha1 ("subject", date)',
 with the subject enclosed in a pair of double-quotes, like this:
 
     Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
-- 
2.10.0.645.g54f1e86


^ 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