Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Junio C Hamano]

Jeff King  writes:

>> It was a bit more painful than necessary to make sure I have
>> something that can be merged for 2.14.x maintenance track, but I
>> think the topic is now in a reasonable shape, and I've merged it to
>> 'next'.  On the first-parent chain from 'master' to 'pu', the merge
>> of this topic is the very first one, and after reading it over once
>> again, I think this is OK.
>
> Hmm. I think you would just want the top two commits for maint-2.14
> (reverting 136c8c8b8f and fixing up git-tag to check color config). But
> of course you can't do a partial merge because they come on top of the
> other dead-end/revert pair. You'd have to cherry-pick (and even then fix
> up a few bits, like adding in the "add -p" test).
>
> Though if we take all of jk/ui-color-always-to-auto-maint, and then do
> the whole reversion on top of that, I think that should work. It just
> doesn't look like that topic ever made it to "maint" (I see mention of a
> jk/ref-filter-colors-fix-maint in the log for master, but there's no
> such branch).

Yeah, that is what ended up to be jk/ref-filter-colors-fix; the
branch merges cleanly to 'master', but also to 'maint' without
dragging the rest of the recent development along with it---I did a
rebase before sending out the message you are responding to.

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Jeff King]

On Mon, Oct 16, 2017 at 11:51:01PM -0700, Jonathan Nieder wrote:

> > OK, so it seems we both have slight preference for the "peel back"
> > approach.  Adding Jonathan to Cc:
> 
> Which approach is "harder but right" / "peel back"?

"peel back" is reverting back to the pre-v2.14.2 state (Junio has the
patches queued in jk/ref-filter-colors-fix).

> I agree with the goal of making color.ui=always a synonym for auto in
> file-based config.  Peff found some problems with the warning patch
> (scripted commands produce too many warnings), which are not an issue
> for $dayjob but may be for upstream, so I see the value of holding off
> on the warning for now.
> 
> I'm also fine with "revert the proximate cause of the latest
> complaints" as a stepping stone toward making color.ui=always a
> synonym for auto in file-based config in a later release.

I do think "color.ui=always" is a foot-gun, but I wasn't happy with the
number of weird hacks we ended up with in trying to fix that (like "it
means one thing in the on-disk config and another thing with "git -c").

We can take it up as a topic post-release. At the very least, I think we
will want to change the documentation to make it more clear that you
almost certainly _don't_ want to use "always".

-Peff

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Jeff King]

On Tue, Oct 17, 2017 at 03:26:25PM +0900, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > Jeff King  writes:
> >
> >> After pondering over it, I have a slight preference for that, too. But
> >> I'm also happy to hear more input.
> >
> > OK, so it seems we both have slight preference for the "peel back"
> > approach.  Adding Jonathan to Cc:
> 
> It was a bit more painful than necessary to make sure I have
> something that can be merged for 2.14.x maintenance track, but I
> think the topic is now in a reasonable shape, and I've merged it to
> 'next'.  On the first-parent chain from 'master' to 'pu', the merge
> of this topic is the very first one, and after reading it over once
> again, I think this is OK.

Hmm. I think you would just want the top two commits for maint-2.14
(reverting 136c8c8b8f and fixing up git-tag to check color config). But
of course you can't do a partial merge because they come on top of the
other dead-end/revert pair. You'd have to cherry-pick (and even then fix
up a few bits, like adding in the "add -p" test).

Though if we take all of jk/ui-color-always-to-auto-maint, and then do
the whole reversion on top of that, I think that should work. It just
doesn't look like that topic ever made it to "maint" (I see mention of a
jk/ref-filter-colors-fix-maint in the log for master, but there's no
such branch).

I started to prepare a patch directly on v2.14.2 just to see what it
would look like. The first one (the revert) is fine, but we then have to
fixup tag and for-each-ref. And since they don't have --color added by
the dead-end fixups, the tests get harder...

-Peff

Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands

[Jeff King]

On Wed, Oct 18, 2017 at 05:55:18AM +0900, Junio C Hamano wrote:

> I'll take these three to replace what I tentatively queued, not
> necessarily because I like the change in 1/3 better, but because
> these are explained much better; besides I prefer a version that at
> least two people deeply looked at.

These look good to me. Between your two versions, I think the code in
Jonathan's is slightly preferable. It's possible that some other caller
of strbuf_check_branch_name() may run into the same thing and be fixed
(I am trying to think of a hypothetical caller that would be
inconvenienced by the new behavior, but I can't come up with one; and
certainly the existing code would BUG(), so this is an improvement).

-Peff

Re: Minor man page weirdness?

[Jeff King]

On Wed, Oct 18, 2017 at 12:41:03PM +0900, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > It does make me wonder if there are other instances of the missing-space
> > problem lurking. Grepping for backtick followed by single-quote shows
> > only one more case in user-manual.txt. I have no idea if that one hits
> > the same problem on older docbook versions.
> 
> I had an impression that this is only for roff, but we do not
> produce user-manual.[0-9], do we?

Yeah, I think you're right. We do produce XML of it, but it goes to
other formats like texi and pdf (and if this is a docbook bug, it may or
may not be present in those other output formats).

-Peff

Re: Minor man page weirdness?

[Junio C Hamano]

Jeff King  writes:

> It does make me wonder if there are other instances of the missing-space
> problem lurking. Grepping for backtick followed by single-quote shows
> only one more case in user-manual.txt. I have no idea if that one hits
> the same problem on older docbook versions.

I had an impression that this is only for roff, but we do not
produce user-manual.[0-9], do we?

> Or if it's a problem for any
> other punctuation combinations. I'm happy to punt on it until somebody
> with an affected docbook produces more bug reports. :)

Yup, we cannot _fix_ what we do not know are broken ;-)

Thanks.

Re: Minor man page weirdness?

[Jeff King]

On Wed, Oct 18, 2017 at 11:34:31AM +0900, Junio C Hamano wrote:

> -- >8 --
> branch doc: sprinkle a few commas for readability
> 
> The "--force" option can also be used when the named branch does not
> yet exist, and the point of the option is the user can (re)point the
> branch to the named commit even if it does.  Add 'even' before 'if'
> to clarify.  Also, insert another comma after "Without -f" before
> "the command refuses..." to make the text easier to parse.
> 
> Incidentally, this change should help certain versions of
> docbook-xsl-stylesheets that renders the original without any
> whitespace between "-f" and "git".

Thanks, this looks good.

It does make me wonder if there are other instances of the missing-space
problem lurking. Grepping for backtick followed by single-quote shows
only one more case in user-manual.txt. I have no idea if that one hits
the same problem on older docbook versions. Or if it's a problem for any
other punctuation combinations. I'm happy to punt on it until somebody
with an affected docbook produces more bug reports. :)

-Peff

Re: Minor man page weirdness?

[Junio C Hamano]

Junio C Hamano  writes:

> Andreas Schwab  writes:
>
>> On Okt 16 2017, Jeff King  wrote:
>>
>>> We do have some hacks/workarounds for broken versions of the toolchain.
>>> You can try tweaking various knobs you find in Documentation/Makefile).
>>> DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the
>>> opposite (removes extra spaces).
>>
>> An easy workaround would be to add a comma between `-f` and 'git
>> branch'.
>
> That sounds like a good idea, regardless of the mark-up issue.

-- >8 --
branch doc: sprinkle a few commas for readability

The "--force" option can also be used when the named branch does not
yet exist, and the point of the option is the user can (re)point the
branch to the named commit even if it does.  Add 'even' before 'if'
to clarify.  Also, insert another comma after "Without -f" before
"the command refuses..." to make the text easier to parse.

Incidentally, this change should help certain versions of
docbook-xsl-stylesheets that renders the original without any
whitespace between "-f" and "git".

Noticed-by: Lars Schneider 
Helped-by: Jeff King 
Helped-by: Andreas Schwab 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-branch.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index fe029ac6fc..d6587c5e96 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -104,8 +104,8 @@ OPTIONS
 
 -f::
 --force::
-   Reset  to  if  exists
-   already. Without `-f` 'git branch' refuses to change an existing branch.
+   Reset  to , even if  exists
+   already. Without `-f`, 'git branch' refuses to change an existing 
branch.
In combination with `-d` (or `--delete`), allow deleting the
branch irrespective of its merged status. In combination with
`-m` (or `--move`), allow renaming the branch even if the new

Re: Fwd: Has git-gui repo moved from location?

[Gilberto Stankiewicz]

Weird, it was not working for me earlier today, but now it works.

Thank you,
Gilberto


On Tue, Oct 17, 2017 at 5:57 PM, Junio C Hamano  wrote:
> Gilberto Stankiewicz  writes:
>
>> I am trying to clone git://repo.or.cz/git-gui.git as described at
>> https://github.com/git/git/blob/master/Documentation/SubmittingPatches
>> but it seems the repo does not exist.
>
> $ git fetch -v git-gui
> Looking up repo.or.cz ... done.
> Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done.
> From git://repo.or.cz/git-gui
>  = [up to date]master -> git-gui/master
>  = [up to date]pu -> git-gui/pu
>  = [up to date]todo   -> git-gui/todo
>
> $ git fetch -v git://repo.or.cz/git-gui.git
> Looking up repo.or.cz ... done.
> Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done.
> From git://repo.or.cz/git-gui
>  * branch  HEAD   -> FETCH_HEAD
>

Re: [PATCH] Added support for new configuration parameter push.pushOption

[Junio C Hamano]

Junio C Hamano  writes:

> base to show an incremental improvement, but here is how I would do
> the "code" part.
>
>  builtin/push.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 03846e8379..89ef029c67 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -32,6 +32,8 @@ static const char **refspec;
>  static int refspec_nr;
>  static int refspec_alloc;
>  
> +static struct string_list push_options_config = STRING_LIST_INIT_DUP;
> +
>  static void add_refspec(const char *ref)
>  {
>   refspec_nr++;
> @@ -503,6 +505,13 @@ static int git_push_config(const char *k, const char *v, 
> void *cb)
>   int val = git_config_bool(k, v) ?
>   RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
>   recurse_submodules = val;
> + } else if (!strcmp(k, "push.pushoption")) {
> + if (!v)
> + return config_error_nonbool(k);
> + else if (!*v)
> + string_list_clear(_options_config, 0);
> + else
> + string_list_append(_options_config, v);

Here needs to be

return 0;

as there is no point in falling through to call
git_default_config().

>   }
>  
>   return git_default_config(k, v, NULL);

Re: [PATCH] Added support for new configuration parameter push.pushOption

[Junio C Hamano]

Marius Paliga  writes:

> builtin/push.c: add push.pushOption config

This is a good title for this change; it would be perfect if it were
on the Subject: header ;-)

> Currently push options need to be given explicitly, via
> the command line as "git push --push-option".
>
> The UX of Git would be enhanced if push options could be
> configured instead of given each time on the command line.
>
> Add the config option push.pushOption, which is a multi
> string option, containing push options that are sent by default.

s/Currently p/P/ would be sufficient; we describe the status quo in
present tense, and give an order to the codebase to "become like so"
(or command a patch monkey to "make the codebase like so") by using
imperative mood.

I think something shorter like this would be sufficient:

Push options need to be given explicitly, via the command line as "git
push --push-option ".  Add the config option push.pushOption,
which is a multi-valued option, containing push options that are sent
by default.

When push options are set in the lower-priority configulation file
(e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
the more specific repository config by the empty string.

> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 3e76e99f3..da9b17624 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -161,6 +161,9 @@ already exists on the remote side.
>  Transmit the given string to the server, which passes them to
>  the pre-receive as well as the post-receive hook. The given string
>  must not contain a NUL or LF character.
> +Default push options can also be specified with configuration
> +variable `push.pushOption`. String(s) specified here will always
> +be passed to the server without need to specify it using `--push-option`

"will always be passed" is not a "Default".  

If we really need to say it, say something like

When no `--push-option ` is given from the command
line, the values of this configuration variable are used
instead.

But that is what "Default" means, so dropping the second sentence
altogether would be more concise and better.

> diff --git a/builtin/push.c b/builtin/push.c
> index 2ac810422..10e520c8f 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -32,6 +32,8 @@ static const char **refspec;
>  static int refspec_nr;
>  static int refspec_alloc;
>
> +static struct string_list push_options_from_config = STRING_LIST_INIT_DUP;
> +
>  static void add_refspec(const char *ref)
>  {
>  refspec_nr++;
> @@ -503,6 +505,14 @@ static int git_push_config(const char *k, const
> char *v, void *cb)
>  int val = git_config_bool(k, v) ?
>  RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
>  recurse_submodules = val;
> +} else if (!strcmp(k, "push.pushoption")) {
> +if (v == NULL)
> +return config_error_nonbool(k);
> +else
> +if (v[0] == '\0')

Make this "else if (!*v)" on a single line and dedent the remainder.

> +string_list_clear(_options_from_config, 0);
> +else
> +string_list_append(_options_from_config, v);
>  }

> @@ -562,6 +572,8 @@ int cmd_push(int argc, const char **argv, const
> char *prefix)
>  packet_trace_identity("push");
>  git_config(git_push_config, );
>  argc = parse_options(argc, argv, prefix, options, push_usage, 0);
> +if (!push_options.nr)
> +push_options = push_options_from_config;

We encourage our developers to think twice before doing a structure
assignment.

I think this is a bad idea, primarily because at the point where
string_list_clear() is later called on _options, it is unclear
how to release the resources held by push_options_from_config().  It
also is bad to depend on the implementation detail that overwriting
the structure do not leak push_options.item when push_options.nr is
zero, but it is conceivable that STRING_LIST_INIT_* could later be
modified to pre-allocate a small array, at which point this will
become a memory leak.

> diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
> index 90a4b0d2f..463783789 100755
> --- a/t/t5545-push-options.sh
> +++ b/t/t5545-push-options.sh
> @@ -140,6 +140,83 @@ test_expect_success 'push options and submodules' '
> ...
> +git -c push.pushOption=default1 -c push.pushOption=default2
> push up master
> ...
> +test_expect_success 'push option from command line overrides
> from-config push option' '

It appears that your MUA is expanding tabs to spaces and wrapping
long lines in your patch?  Please double check and make sure that
will not happen.

I couldn't use your patch (because it was broken by your MUA) as a
base to show an incremental improvement, but here is how I would do
the "code" part.

 builtin/push.c | 22 ++
 1 file changed, 18 insertions(+), 

Re: Fwd: Has git-gui repo moved from location?

[Junio C Hamano]

Gilberto Stankiewicz  writes:

> I am trying to clone git://repo.or.cz/git-gui.git as described at
> https://github.com/git/git/blob/master/Documentation/SubmittingPatches
> but it seems the repo does not exist.

$ git fetch -v git-gui
Looking up repo.or.cz ... done.
Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done.
>From git://repo.or.cz/git-gui
 = [up to date]master -> git-gui/master
 = [up to date]pu -> git-gui/pu
 = [up to date]todo   -> git-gui/todo

$ git fetch -v git://repo.or.cz/git-gui.git
Looking up repo.or.cz ... done.
Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done.
>From git://repo.or.cz/git-gui
 * branch  HEAD   -> FETCH_HEAD

Re: [PATCH v2] doc: list filter-branch subdirectory-filter first

[Junio C Hamano]

David Glasser  writes:

> From: David Glasser 
>
> The docs claim that filters are applied in the listed order, so
> subdirectory-filter should come first.
>
> For consistency, apply the same order to the SYNOPSIS and the script's usage, 
> as
> well as the switch while parsing arguments.
>
> Add missing --prune-empty to the script's usage.
>
> Signed-off-by: David Glasser 
> ---

Thanks for being extra careful.  Will queue.

Re: [PATCH v4 3/3] submodule: simplify decision tree whether to or not to fetch

[Junio C Hamano]

Stefan Beller  writes:

> On Mon, Oct 16, 2017 at 6:59 AM, Heiko Voigt  wrote:
>> To make extending this logic later easier.
>>
>> Signed-off-by: Heiko Voigt 
>
> Thanks for this readability fix!
>
> Stefan

Thanks, both.

Re: [PATCH v4 2/3] implement fetching of moved submodules

[Junio C Hamano]

Stefan Beller  writes:

>> +   /* make sure name does not collide with existing one 
>> */
>> +   submodule = submodule_from_name(commit_oid, name);
>> +   if (submodule) {
>> +   warning("Submodule in commit %s at path: "
>> +   "'%s' collides with a submodule 
>> named "
>> +   "the same. Skipping it.",
>> +   oid_to_hex(commit_oid), name);
>> +   name = NULL;
>> +   }
>
> This is the ugly part of using one string list and storing names or
> path in it. I wonder if we could omit this warning if we had 2 string lists?

We are keying off of 'name', because that is what will give a module
its identity.  If we have a gitlink whose path is not in .gitmodules
in the same tree, then we are seeing an unregistered submodule.  If
we were to "git add" it, then we'd use its path as the default name,
but if we already have a submodule with that name (the most likely
explanation for its existence is because it started its life there
and then later moved), and the submodule is bound to a different
path, then that is a different submodule.  Skipping and warning both
are sensible thing to do.

I do not know what you see as ugly here, and more importantly, I am
not sure how having two lists would help.

Fwd: Has git-gui repo moved from location?

[Gilberto Stankiewicz]

Hello,

I am trying to clone git://repo.or.cz/git-gui.git as described at
https://github.com/git/git/blob/master/Documentation/SubmittingPatches
but it seems the repo does not exist.

Has the repo changed from location?

Thank you,
Gilberto

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Junio C Hamano]

René Scharfe  writes:

> Stop advertising -h as the short equivalent of --heads, because it's
> used for showing a short help text for almost all other git commands.
> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
> been working when used together with other parameters anyway.
>
> Signed-off-by: Rene Scharfe 
> ---
> That would be step on the way towards more consistent command line
> switches, in the same vein as d69155119 (t0012: test "-h" with
> builtins).

Sorry, but I am not sure whom this and the other approach are trying
to help.

The rule we have currently seems to be that "git cmd -h" (no other
arguments) consistently gives a short help, and if a subcommand
supports an option "-h" specific to it that is not about giving a
short help, the caller needs to be aware of it to invoke the option,
by making sure that there is some other arguments on the command
line if "-h" form of that subcommand specific option is used, by
doing e.g. "git ls-remote -h origin" or "git ls-remote --head".

I can see that this "alternative" approach makes it less convenent
for folks who have followed that rule by hiding "-h" (with the
intention of deprecating and possibly removing it in the future) and
encouraging (and later foring) "--head" to be used instead.  

The other approach burdens new users by changing the rule to "some
subcommands that have their own '-h' option cannot be asked for a
brief usage with 'git cmd -h'".  But the thing is, these new users
who do not know which subcommands do have their own '-h' and which
ones do not are the ones that benefit most from the consistent "'git
cmd -h' with no other argument gets short help" rule.

When making a backward incompatible change, it is asking us to go to
those who are inconvenienced and say "I am sorry that we are making
things less convenient for you, but by doing so we can gain this
greater good which is ...".  I can explain how this and the other
approach make things less convenient for existing or new users, but
I am having a hard time formulating what the greater good is.

In short, I cannot sell this change to our users.  Please help me do
so if you want this (or the other) change made to our system.

Re: [PATCH 2/1] mention git stash push first in the man page

[Jeff King]

On Tue, Oct 17, 2017 at 10:45:15PM +0100, Thomas Gummerer wrote:

> > Seems reasonable, though if we are deprecating "save" should we demote
> > it from being in the synopsis entirely?
> 
> I saw that as a next step, with the "official" deprecation of "save".
> I thought we might want to advertise "push" a bit more before actually
> officially deprecating "save" and sending the deprecation notice out
> in the release notes.

Right, my thinking was that it would still be documented in the manpage,
just lower down. And that would probably say something like "save is a
historical synonym for push, except that it differs in these ways...".

> OTOH, dropping it from the synopsis in the man page probably wouldn't
> cause much issue, as "push" directly replaces it, and is easily
> visible.  Not sure how slow we want to take the deprecation?

I don't think there's any reason to go slow in marking something as
deprecated. It's the part where we follow up and remove or change the
feature that must take a while.

-Peff

Re: [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)

[Thomas Gummerer]

On 10/17, Jeff King wrote:
> On Thu, Oct 05, 2017 at 09:00:49PM +0100, Thomas Gummerer wrote:
> 
> > Since you were asking :)  With the introduction of 'git stash push',
> > my hope was always that we could eventually get rid of 'git stash
> > save' and only keep one interface around.
> > 
> > As there still many references to it around on the internet, it
> > probably requires a bit of a longer deprecation plan.  What do you
> > think about the following:
> > 
> > - Change docs/man pages to use 'git stash push' everywhere instead of
> >   'git stash save'.
> > - Mention the deprecation in the release notes and in the man page.
> > - Print a warning when 'git stash save' is used.
> > - Eventually get rid of it (maybe something for 3.0?)
> > 
> > The steps above would all occur sequentially with a few releases
> > between each of them.
> 
> That sounds like a pretty good plan.
> 
> > A patch for the first step below.  I think that even makes sense if we
> > don't want to follow through with the deprecation.
> 
> Agreed. The patch mostly looks good, except:

Thanks for the review.

> > diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> > index b4d88af133..1c4e44892d 100644
> > --- a/Documentation/user-manual.txt
> > +++ b/Documentation/user-manual.txt
> > @@ -1556,7 +1556,7 @@ so on a different branch and then coming back), 
> > unstash the
> >  work-in-progress changes.
> >  
> >  
> > -$ git stash save "work in progress for foo feature"
> > +$ git stash push "work in progress for foo feature"
> >  
> 
> This needs "-m", doesn't it?

Yeah, this definitely needs "-m".  Will change.

> >  
> >  This command will save your changes away to the `stash`, and
> > diff --git a/git-stash.sh b/git-stash.sh
> > index 8b2ce9afda..8ce6929d7f 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -267,11 +267,11 @@ push_stash () {
> > # translation of "error: " takes in your language. E.g. 
> > in
> > # English this is:
> > #
> > -   #$ git stash save --blah-blah 2>&1 | head -n 2
> > -   #error: unknown option for 'stash save': --blah-blah
> > -   #   To provide a message, use git stash save -- 
> > '--blah-blah'
> > -   eval_gettextln "error: unknown option for 'stash save': 
> > \$option
> > -   To provide a message, use git stash save -- '\$option'"
> > +   #$ git stash push --blah-blah 2>&1 | head -n 2
> > +   #error: unknown option for 'stash push': --blah-blah
> > +   #   To provide a message, use git stash push -- 
> > '--blah-blah'
> > +   eval_gettextln "error: unknown option for 'stash push': 
> > \$option
> > +   To provide a message, use git stash push -- '\$option'"
> 
> And here, too?

Indeed, thanks for catching these.  Will change and re-roll the
patches.

> -Peff

Re: [PATCH 2/1] mention git stash push first in the man page

[Thomas Gummerer]

On 10/17, Jeff King wrote:
> On Thu, Oct 05, 2017 at 09:10:29PM +0100, Thomas Gummerer wrote:
> 
> > Because 'stash push' and 'stash save' are so closely related they share one
> > section in the man page.  Currently 'stash save' comes first, as that
> > was the command that people were historically using.  However this makes
> > the newer, more feature rich git stash push very easy to overlook.
> > Change the order to give the newer interface for creating a stash the
> > more prominent position.
> 
> Seems reasonable, though if we are deprecating "save" should we demote
> it from being in the synopsis entirely?

I saw that as a next step, with the "official" deprecation of "save".
I thought we might want to advertise "push" a bit more before actually
officially deprecating "save" and sending the deprecation notice out
in the release notes.

OTOH, dropping it from the synopsis in the man page probably wouldn't
cause much issue, as "push" directly replaces it, and is easily
visible.  Not sure how slow we want to take the deprecation?

> -Peff

Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick

[Igor Djordjevic]

On 17/10/2017 19:30, Johannes Sixt wrote:
> Am 17.10.2017 um 01:01 schrieb Rafael Ascensao:
>>> This is worth discussing, though not my preference. The picture to "pick
>>> cherries" has become quite common, and now that we use it for the name of
>>> the command, "cherry-pick", the direction of flow is quite obvious and
>>> strongly implied: from somewhere else to me (and not to somebody else).
>>
>> What if we borrow '--onto' from rebase and make it cherry-pick --onto
>> ?
> 
> I actually like this. Although I would miss the convenience that the 
> source defaults to HEAD. Unless we make a special case for --onto, 
> that is, of course.

I second this.

>From my point of view, I would say that "cherry-pick" in general 
seems to just imply "taking" commits (picking "cherries"), where 
direction flow is irrelevant, or at least not strongly implied. For 
me, it`s more "from somewhere else to somebody else", indeed.

The fact that it currently "posts" its picks "to me" (on top of 
current HEAD) could very well be justified/explained as the most 
obvious case (at time of implementation, at least), where usually one 
does work "here" (HEAD), thus cares to have commits picked over 
"here" as well - but it doesn`t have to be a restriction, where other 
destination besides HEAD could/should be allowed.

Anyway, I`d say current behaviour without "--onto" should stay, 
indeed, where HEAD destination is implied if not otherwise specified 
(and that seems to align well with some other Git commands, too). 
Also, in case "--onto" _is_ provided, maybe commit to be picked could 
be allowed to be omitted, defaulting to HEAD again, but as source 
this time.

That said, might be 'git commit' looks like a nice candidate for 
being taught this new trick as well (optionally commit to a revision 
other than HEAD, what Ævar already mentioned), following the same 
logic... ;) It would even be a better (more straightforward) fit for 
one of your previous examples:

> Another use case is when you receive a patch to be applied on a 
> different branch while you are in the middle of some work. If it can be 
> applied on the current branch, then you can post it to the destination, 
> rewind, and continue with your work.

p.s. I`m very interested in this functionality, and more - I have a 
beginner`s attempt in addressing a similar use-case this topic is 
concerned with (symptomatically using a variation of "--onto" as well 
:P), hopefully I`ll sent out something soon, for the sake of 
discussion/opinion, at least :(

Regards,
Buga

Re: Re* Is t5601 flaky for anybody else?

[Jeff King]

On Wed, Oct 18, 2017 at 06:02:59AM +0900, Junio C Hamano wrote:

> Brandon Williams  writes:
> 
> > I haven't noticed any issues myself but maybe this has something to do
> > with my changes to this test in the 'bw/protocol-v1' topic?
> 
> As I've seen this on 'master', too, I suspect the topic has nothing
> to do with it.
> 
> Here is what I have on 'pu'.

FWIW, I can't replicate the problem on either "master" or "pu". I wonder
why.

-Peff

Re: Multiple paths in GIT_EXEC_PATH

[Junio C Hamano]

Kevin Daudt  writes:

> The commit that changed what you described is 1073094f3 (git-sh-setup:
> be explicit where to dot-source git-sh-i18n from., 2016-10-29). That
> commit claims there were already scripts that assumed GIT_EXEC_PATH is
> just a single entry. That commit was included in v2.11.
>
> There was also a recent thread[0] about it that discussed this issue,
> where someone stated that indeed treating GIT_EXEC_PATH with the same
> semantics as PATH has been broken for a while, but it seems there are no
> real plans to fix it.

The variable was never meant to have more than one path concatenated
with ':' from day one.

In C code we've used it as a leading directory path to tack a
command name to form a path to give to exec(3), without any
intention to have it a list of paths, which is split at ':'.
sh-setup was doing "PATH=$GIT_EXEC_PATH:$PATH" without rejecting a
value in GIT_EXEC_PATH with a colon in it but that was merely being
lazy and made it "work" by accident.

Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> From: Junio C Hamano 
>>
>> "git check-ref-format --branch $name" feature was originally
>> introduced (and was advertised) as a way for scripts to take any
>> end-user supplied string (like "master", "@{-1}" etc.) and see if it
>> is usable when Git expects to see a branch name, and also obtain the
>> concrete branch name that the at-mark magic expands to.
>>
>> Emphasize that "see if it is usable" role in the description and
>> clarify that the @{...} expansion only occurs when run from within a
>> repository.
>>
>> [jn: split out from a larger patch]
>>
>> Helped-by: Jeff King 
>> Signed-off-by: Junio C Hamano 
>> ---
>
> Missing sign-off, unlike the other two, intended?

Oops.
Signed-off-by: Jonathan Nieder 

> I'll take these three to replace what I tentatively queued, not
> necessarily because I like the change in 1/3 better, but because
> these are explained much better; besides I prefer a version that at
> least two people deeply looked at.

Okay, thanks.  I'll add the rest of the change as a patch on top. :)

Thanks,
Jonathan

Re* Is t5601 flaky for anybody else?

[Junio C Hamano]

Brandon Williams  writes:

> I haven't noticed any issues myself but maybe this has something to do
> with my changes to this test in the 'bw/protocol-v1' topic?

As I've seen this on 'master', too, I suspect the topic has nothing
to do with it.

Here is what I have on 'pu'.


-- >8 --
From: Junio C Hamano 
Date: Tue, 17 Oct 2017 14:04:43 +0900
Subject: [PATCH] t5601: rm the target file of cp that could still be executing

"while sh t5601-clone.sh; do :; done" seems to fail sporadically at
around test #45 where fake-ssh wrapper is copied create plink.exe,
with an error message that says the "text is busy".

I have a mild suspicion that the root cause of the bug is that the
fake SSH process from the previous test is still running by the time
the next test wants to replace it with a new binary, but in the
meantime, removing the target that could still be executing before
copying something else over seems to work it around.

Signed-off-by: Junio C Hamano 
---
 t/t5601-clone.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 9c56f771b6..50e40abb11 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -308,6 +308,7 @@ test_expect_success 'clone checking out a tag' '
 
 setup_ssh_wrapper () {
test_expect_success 'setup ssh wrapper' '
+   rm -f "$TRASH_DIRECTORY/ssh-wrapper$X" &&
cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
"$TRASH_DIRECTORY/ssh-wrapper$X" &&
GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
@@ -318,6 +319,7 @@ setup_ssh_wrapper () {
 }
 
 copy_ssh_wrapper_as () {
+   rm -f "${1%$X}$X" &&
cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
GIT_SSH="${1%$X}$X" &&
export GIT_SSH
-- 
2.15.0-rc1-178-ge1264d9eb8

Re: [PATCH] fetch doc: src side of refspec could be full SHA-1

[Junio C Hamano]

Jonathan Nieder  writes:

> nits:
>
> s/most typically/typically/
> s/an fully/a fully/
>
> With those tweaks,
> Reviewed-by: Jonathan Nieder 

Thanks.

Re: Minor man page weirdness?

[Junio C Hamano]

Andreas Schwab  writes:

> On Okt 16 2017, Jeff King  wrote:
>
>> We do have some hacks/workarounds for broken versions of the toolchain.
>> You can try tweaking various knobs you find in Documentation/Makefile).
>> DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the
>> opposite (removes extra spaces).
>
> An easy workaround would be to add a comma between `-f` and 'git
> branch'.

That sounds like a good idea, regardless of the mark-up issue.

Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands

[Junio C Hamano]

Jonathan Nieder  writes:

> From: Junio C Hamano 
>
> "git check-ref-format --branch $name" feature was originally
> introduced (and was advertised) as a way for scripts to take any
> end-user supplied string (like "master", "@{-1}" etc.) and see if it
> is usable when Git expects to see a branch name, and also obtain the
> concrete branch name that the at-mark magic expands to.
>
> Emphasize that "see if it is usable" role in the description and
> clarify that the @{...} expansion only occurs when run from within a
> repository.
>
> [jn: split out from a larger patch]
>
> Helped-by: Jeff King 
> Signed-off-by: Junio C Hamano 
> ---

Missing sign-off, unlike the other two, intended?

I'll take these three to replace what I tentatively queued, not
necessarily because I like the change in 1/3 better, but because
these are explained much better; besides I prefer a version that at
least two people deeply looked at.

Thanks.

Re: Minor man page weirdness?

[Andreas Schwab]

On Okt 17 2017, Jeff King  wrote:

> One other thing to try:
>
>   rm git-branch.1
>   make NO_MAN_BOLD_LITERAL=1 git-branch.1

That doesn't help:

   Reset  to  if  exists already\&. Without
   \-f\fIgit branch\fR
   refuses to change an existing branch\&. In combination with

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] fetch doc: src side of refspec could be full SHA-1

[Jonathan Nieder]

Hi Junio,

Junio C Hamano wrote:

> Since a9d34933 ("Merge branch 'fm/fetch-raw-sha1'", 2015-06-01) we
> allow to fetch by an object name when the other side accepts such a
> request, but we never updated the documentation to match.
>
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/pull-fetch-param.txt | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Good catch.

> --- a/Documentation/pull-fetch-param.txt
> +++ b/Documentation/pull-fetch-param.txt
> @@ -23,9 +23,11 @@ ifdef::git-pull[]
>  endif::git-pull[]
>  +
>  The format of a  parameter is an optional plus
> -`+`, followed by the source ref , followed
> +`+`, followed by the source , followed
>  by a colon `:`, followed by the destination ref .
> -The colon can be omitted when  is empty.
> +The colon can be omitted when  is empty.   is most
> +typically a ref, but it can also be an fully spelled hex object
> +name.

nits:

s/most typically/typically/
s/an fully/a fully/

With those tweaks,
Reviewed-by: Jonathan Nieder 

Thanks.

Re: Minor man page weirdness?

[Jeff King]

On Tue, Oct 17, 2017 at 07:52:03PM +0200, Andreas Schwab wrote:

> > Yes, it's in that step, but xmlto is just driving the xslt
> > transformation done by docbook. So the interesting version is probably
> > docbook. I have docbook-xsl 1.79.1+dfsg-2 (from Debian unstable).
> 
> docbook-xsl-stylesheets-1.78.1+svn9743

Hmm. That could be it, though I was unable to bisect on the docbook repo
since I couldn't get their build to work reliably.

One other thing to try:

  rm git-branch.1
  make NO_MAN_BOLD_LITERAL=1 git-branch.1

It's possible our snippet to add in the bolding causes problems.

-Peff

Re: [PATCH v4 3/3] submodule: simplify decision tree whether to or not to fetch

[Stefan Beller]

On Mon, Oct 16, 2017 at 6:59 AM, Heiko Voigt  wrote:
> To make extending this logic later easier.
>
> Signed-off-by: Heiko Voigt 

Thanks for this readability fix!

Stefan

Re: Minor man page weirdness?

[Andreas Schwab]

On Okt 16 2017, Jeff King  wrote:

> We do have some hacks/workarounds for broken versions of the toolchain.
> You can try tweaking various knobs you find in Documentation/Makefile).
> DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the
> opposite (removes extra spaces).

An easy workaround would be to add a comma between `-f` and 'git
branch'.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible

[Stefan Beller]

On Mon, Oct 16, 2017 at 6:57 AM, Heiko Voigt  wrote:
> The current implementation of submodules supports on-demand fetch if
> there is no .gitmodules entry for a submodule. Let's add a test to
> document this behavior.
>
> Signed-off-by: Heiko Voigt 
> ---
>  t/t5526-fetch-submodules.sh | 42 +-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 42251f7f3a..43a22f680f 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly 
> recorded commits are alrea
> git fetch >../actual.out 2>../actual.err
> ) &&
> ! test -s actual.out &&
> -   test_i18ncmp expect.err actual.err
> +   test_i18ncmp expect.err actual.err &&
> +   (
> +   cd submodule &&
> +   git checkout -q master
> +   )

For few instructions inside another repo, I tend to use the
-C option:

  git -C submodule checkout -q master

That saves a shell, which is noticeable cost on Windows I was told.
(also fewer lines to type).

Oh, I see, that is consistent with the rest of the file. Oh well.
(Otherwise I would have lobbied to even move it further up and
put it inside a test_when_finished ""


> +'
> +
> +test_expect_success "'fetch.recurseSubmodules=on-demand' works also without 
> .gitmodule entry" '
> +   (
> +   cd downstream &&
> +   git fetch --recurse-submodules
> +   ) &&

This is consistent with the rest of the file as well, so I shall
refrain from complaining. ;)

> +   add_upstream_commit &&
> +   head1=$(git rev-parse --short HEAD) &&
> +   git add submodule &&
> +   git rm .gitmodules &&
> +   git commit -m "new submodule without .gitmodules" &&
> +   printf "" >expect.out &&

This could be just

: >expect.out

no need to invoke a function to print nothing.

> +   head2=$(git rev-parse --short HEAD) &&
> +   echo "From $pwd/." >expect.err.2 &&
> +   echo "   $head1..$head2  master -> origin/master" >>expect.err.2 
> &&
> +   head -3 expect.err >>expect.err.2 &&
> +   (
> +   cd downstream &&
> +   rm .gitmodules &&
> +   git config fetch.recurseSubmodules on-demand &&
> +   # fake submodule configuration to avoid skipping submodule 
> handling
> +   git config -f .gitmodules submodule.fake.path fake &&
> +   git config -f .gitmodules submodule.fake.url fakeurl &&
> +   git add .gitmodules &&
> +   git config --unset submodule.submodule.url &&
> +   git fetch >../actual.out 2>../actual.err &&
> +   # cleanup
> +   git config --unset fetch.recurseSubmodules &&
> +   git reset --hard
> +   ) &&
> +   test_i18ncmp expect.out actual.out &&
> +   test_i18ncmp expect.err.2 actual.err &&
> +   git checkout HEAD^ -- .gitmodules &&
> +   git add .gitmodules &&
> +   git commit -m "new submodule restored .gitmodules"
>  '

Thanks for writing this test.
With or without the nits addressed, this is

Reviewed-by: Stefan Beller 

Re: Minor man page weirdness?

[Andreas Schwab]

On Okt 17 2017, Jeff King  wrote:

> On Tue, Oct 17, 2017 at 07:25:28PM +0200, Andreas Schwab wrote:
>
>> >> I see this in git-branch.1:
>> >> 
>> >>   Reset  to  if  exists already\&. 
>> >> Without
>> >>   \fB\-f\fR\fIgit branch\fR
>> >>   refuses to change an existing branch\&. In combination with
>> >> 
>> >> This is with asciidoc 8.6.9.
>> >
>> > Thanks, that seems a likely culprit, then. What's in your git-branch.xml
>> > after you build the documentation?
>> 
>> Same as yours, with the space.  I'd guess it's rather xmlto (version
>> 0.0.25) that's the culprit here.
>
> Yes, it's in that step, but xmlto is just driving the xslt
> transformation done by docbook. So the interesting version is probably
> docbook. I have docbook-xsl 1.79.1+dfsg-2 (from Debian unstable).

docbook-xsl-stylesheets-1.78.1+svn9743

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v4 2/3] implement fetching of moved submodules

[Stefan Beller]

On Mon, Oct 16, 2017 at 6:58 AM, Heiko Voigt  wrote:
> We store the changed submodules paths to calculate which submodule needs
> fetching. This does not work for moved submodules since their paths do
> not stay the same in case of a moved submodules. In case of new
> submodules we do not have a path in the current checkout, since they
> just appeared in this fetch.
>
> It is more general to collect the submodule names for changes instead of
> their paths to include the above cases. If we do not have a
> configuration for a gitlink we rely on constructing a default name from
> the path if a git repository can be found at its path. We skip
> non-configured gitlinks whose default name collides with a configured
> one.

Thanks for working on this!

As detailed below, I wonder if it is easier (in maintenance, explaining
correctness, reviewing) if we'd rather keep two lists around. One for
based on names, and if we cannot lookup a name for a submodule, we
rather use the second path based list as a fallback. That would avoid
potential namespace collisions between names and paths, as well as
not having the confusion of mapping back and forth.

Most functions would then need to operate on path, as the name->path
mapping can be looked up for the first list, but the path->name mapping
cannot be looked up for the second list.

Cheers,
Stefan

> With the change described above we implement 'on-demand' fetching of
> changes in moved submodules.
>
> Signed-off-by: Heiko Voigt 

> ---
>  submodule-config.h  |   3 +
>  submodule.c | 138 
> 
>  t/t5526-fetch-submodules.sh |  35 +++
>  3 files changed, 138 insertions(+), 38 deletions(-)
>
> diff --git a/submodule-config.h b/submodule-config.h
> index e3845831f6..a5503a5d17 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -22,6 +22,9 @@ struct submodule {
> int recommend_shallow;
>  };
>
> +#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \
> +   NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, {0}, -1 };
> +
>  struct submodule_cache;
>  struct repository;
>
> diff --git a/submodule.c b/submodule.c
> index 63e7094e16..71d1773e2e 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -21,7 +21,7 @@
>  #include "parse-options.h"
>
>  static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
> -static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP;
> +static struct string_list changed_submodule_names = STRING_LIST_INIT_DUP;
>  static int initialized_fetch_ref_tips;
>  static struct oid_array ref_tips_before_fetch;
>  static struct oid_array ref_tips_after_fetch;
> @@ -674,11 +674,11 @@ const struct submodule *submodule_from_ce(const struct 
> cache_entry *ce)
>  }
>
>  static struct oid_array *submodule_commits(struct string_list *submodules,
> -  const char *path)
> +  const char *name)
>  {
> struct string_list_item *item;
>
> -   item = string_list_insert(submodules, path);
> +   item = string_list_insert(submodules, name);
> if (item->util)
> return (struct oid_array *) item->util;
>
> @@ -687,39 +687,67 @@ static struct oid_array *submodule_commits(struct 
> string_list *submodules,
> return (struct oid_array *) item->util;
>  }
>
> +struct collect_changed_submodules_cb_data {
> +   struct string_list *changed;
> +   const struct object_id *commit_oid;
> +};
> +
> +/*
> + * this would normally be two functions: default_name_from_path() and

Please start the comment capitalised. (minor nit)

> + * path_from_default_name(). Since the default name is the same as
> + * the submodule path we can get away with just one function which only
> + * checks whether there is a submodule in the working directory at that
> + * location.

This is an interesting comment, as it hints that we ought to keep it that way.
Earlier I was wondering if we want to make the name distinctively different
than its path, as that will confuse users *less* IMHO. (I just remember
someone asking on the mailing list why their "rename" did not work, as
they just renamed everything in the .gitmodules that looked like the path)

As the path/name is confusing, I'd wish we'd be super concise, such that
errors are harder to sneak into. For example, the arguments name should
be "path" as that is the only thing we can look up using is_sub_pop_gently,
if a "name" is given, than it just works because the chosen default name
was its path.

> +static const char *default_name_or_path(const char *path_or_name)
> +{
> +   int error_code;
> +
> +   if (!is_submodule_populated_gently(path_or_name, _code))
> +   return NULL;
> +
> +   return path_or_name;
> +}
> +


> +   if (submodule)
> +   name = submodule->name;
> +   else {
> +

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Jonathan Nieder]

René Scharfe wrote:

> Stop advertising -h as the short equivalent of --heads, because it's
> used for showing a short help text for almost all other git commands.
> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
> been working when used together with other parameters anyway.
>
> Signed-off-by: Rene Scharfe 
> ---
>  Documentation/git-ls-remote.txt | 1 -
>  builtin/ls-remote.c | 4 +++-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Yes, I think this is a good step.
Reviewed-by: Jonathan Nieder 

I would be tempted to go even further and make "git ls-remote -h"
print a warning to stderr to encourage people to update their scripts
to use --heads instead.

Thanks.

Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

[Jonathan Nieder]

René Scharfe wrote:

> Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
> without any other options has shown the short help text of the command
> instead of acting as the short equivalent of --heads.  Fix this
> regression by turning off internal handling of -h for repository setup,
> and option parsing, as well as the corresponding test in t0012.
>
> Reported-by: Thomas Rikl 
> Analyzed-by: Martin Ågren 
> Signed-off-by: Rene Scharfe 
> ---
>  builtin/ls-remote.c  | 1 +
>  git.c| 2 +-
>  t/t0012-help.sh  | 1 +
>  t/t5512-ls-remote.sh | 6 ++
>  4 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Jonathan Nieder 

[...]
> --- a/git.c
> +++ b/git.c
> @@ -421,7 +421,7 @@ static struct cmd_struct commands[] = {
>   { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
>   { "log", cmd_log, RUN_SETUP },
>   { "ls-files", cmd_ls_files, RUN_SETUP },
> - { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
> + { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY | NO_INTERNAL_HELP },

There's only one other command that uses PARSE_OPT_NO_INTERNAL_HELP, and
that's "git archive".  Does it need the same treatment?

More generally, is there a straightforward way for parse-options or
some compile-time analysis to catch if we forget to add
NO_INTERNAL_HELP to a command in the commands[] table?

Thanks,
Jonathan

Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

[Martin Ågren]

On 17 October 2017 at 17:39, René Scharfe  wrote:
> Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
> without any other options has shown the short help text of the command
> instead of acting as the short equivalent of --heads.  Fix this
> regression by turning off internal handling of -h for repository setup,
> and option parsing, as well as the corresponding test in t0012.
>
> Reported-by: Thomas Rikl 
> Analyzed-by: Martin Ågren 
> Signed-off-by: Rene Scharfe 
> ---
>  builtin/ls-remote.c  | 1 +
>  git.c| 2 +-
>  t/t0012-help.sh  | 1 +
>  t/t5512-ls-remote.sh | 6 ++
>  4 files changed, 9 insertions(+), 1 deletion(-)

Documentation/gitcli.txt might need updating. It says that "[c]ommands
which have the enhanced option parser activated all understand ... -h".
Of course, it already was in an incorrect state, since it pretends like
no-one uses PARSE_OPT_NO_INTERNAL_HELP. Probably better to leave that
until it's clear in which direction "git ls-remote -h" should go.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Martin Ågren]

On 17 October 2017 at 17:39, René Scharfe  wrote:
> Stop advertising -h as the short equivalent of --heads, because it's
> used for showing a short help text for almost all other git commands.
> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
> been working when used together with other parameters anyway.
>
> Signed-off-by: Rene Scharfe 
> ---
> That would be step on the way towards more consistent command line
> switches, in the same vein as d69155119 (t0012: test "-h" with
> builtins).

FWIW, my first inclination would be to go with a patch like this instead
of the other two (where you introduce an exception so that git ls-remote
-h does not display the usage). ba5f28bf79 was in 2.8.0. That's 18
months ago. Not an eternity, but still some time ago. Not fixing this
regression has an obvious downside, but there's also a downside to
adding the exception.

As long as a lone -h may give the usage or do something else entirely,
then -- put bluntly -- to know whether you can request the usage with
git foo -h without risk of messing up your repo, you'll need to look up
the usage some other way. At which point you've solved your original
problem, without -h.

Of course, we could promise that -h will give the usage or, in case of
historical wart(s), do something harmless. But it seems a bit awkward,
and might limit the perceived usefulness/safeness or -h.

> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 5f2628c8f8..898836a111 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -21,7 +21,6 @@ commit IDs.
>
>  OPTIONS
>  ---
> --h::
>  --heads::
>  -t::
>  --tags::

Do we want to document -h as a deprecated alias, so that people have a
slightly larger chance of noticing and adapting?

Re: [PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins

[Jonathan Nieder]

Hi,

René Scharfe wrote:

> Builtin commands have skipped repo setup when called with just a single
> option -h and thus shown their short help text even outside of
> repositories since 99caeed05d3 (Let 'git  -h' show usage
> without a git dir).
>
> Add the flag NO_INTERNAL_HELP for builtins to opt out of this special
> treatment and provide a list of commands that are exempt from the
> corresponding test in t0012.  That allows builtins to handle -h
> themselves.
>
> Signed-off-by: Rene Scharfe 
> ---
>  git.c   | 6 +-
>  t/t0012-help.sh | 9 -
>  2 files changed, 13 insertions(+), 2 deletions(-)

Makes sense.

[...]
> +++ b/git.c
[...]
> @@ -312,7 +313,10 @@ static int run_builtin(struct cmd_struct *p, int argc, 
> const char **argv)
>   const char *prefix;
>  
>   prefix = NULL;
> - help = argc == 2 && !strcmp(argv[1], "-h");
> + if (p->option & NO_INTERNAL_HELP)
> + help = 0;
> + else
> + help = argc == 2 && !strcmp(argv[1], "-h");

optional: this could be part of the same expression:

help = !(p->option & NO_INTERNAL_HELP) && argc == 2 && ...;

[...]
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -53,12 +53,19 @@ test_expect_success 'generate builtin list' '
>   git --list-builtins >builtins
>  '
>  
> +cat >no_internal_help < +EOF
> +
> +test_expect_success 'generate list of builtins with internal help' '
> + grep -w -v -f no_internal_help builtins_internal_help
> +'

Hm, I don't see any instances of "grep -f" in the testsuite.  Are
there portability pitfalls in it I don't know about?  It's in POSIX,
so it looks pretty safe.

I would have been tempted to use comm, which is already used in
t6500-gc.sh:

comm -1 -3 no_internal_help builtins >builtins_internal_help

Other nits:

- preparatory 'cat' commands like the above tend to go inside
  test_expect_success these days

- test that set up for later tests get marked as 'setup' or 'set up'

With whatever subset of the changes below looks good squashed in,
Reviewed-by: Jonathan Nieder 

Thanks.

diff --git i/git.c w/git.c
index 9d1b8c4716..e4b340df7d 100644
--- i/git.c
+++ w/git.c
@@ -313,10 +313,8 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
const char *prefix;
 
prefix = NULL;
-   if (p->option & NO_INTERNAL_HELP)
-   help = 0;
-   else
-   help = argc == 2 && !strcmp(argv[1], "-h");
+   help = !(p->option & NO_INTERNAL_HELP) &&
+   argc == 2 && !strcmp(argv[1], "-h");
if (!help) {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
diff --git i/t/t0012-help.sh w/t/t0012-help.sh
index c5a748837c..73fdfd99ab 100755
--- i/t/t0012-help.sh
+++ w/t/t0012-help.sh
@@ -49,15 +49,11 @@ test_expect_success "--help does not work for guides" "
test_i18ncmp expect actual
 "
 
-test_expect_success 'generate builtin list' '
-   git --list-builtins >builtins
-'
-
-cat >no_internal_help no_internal_help <<-\EOF &&
+   EOF
+   git --list-builtins >builtins &&
+   comm -1 -3 no_internal_help builtins >builtins_internal_help
 '
 
 while read builtin

Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick

[Johannes Sixt]

Am 17.10.2017 um 01:01 schrieb Rafael Ascensao:

This is worth discussing, though not my preference. The picture to "pick
cherries" has become quite common, and now that we use it for the name of
the command, "cherry-pick", the direction of flow is quite obvious and
strongly implied: from somewhere else to me (and not to somebody else).


What if we borrow '--onto' from rebase and make it cherry-pick --onto
?


I actually like this. Although I would miss the convenience that the 
source defaults to HEAD. Unless we make a special case for --onto, that 
is, of course.


-- Hannes

Re: Minor man page weirdness?

[Jeff King]

On Tue, Oct 17, 2017 at 07:25:28PM +0200, Andreas Schwab wrote:

> >> I see this in git-branch.1:
> >> 
> >>   Reset  to  if  exists already\&. 
> >> Without
> >>   \fB\-f\fR\fIgit branch\fR
> >>   refuses to change an existing branch\&. In combination with
> >> 
> >> This is with asciidoc 8.6.9.
> >
> > Thanks, that seems a likely culprit, then. What's in your git-branch.xml
> > after you build the documentation?
> 
> Same as yours, with the space.  I'd guess it's rather xmlto (version
> 0.0.25) that's the culprit here.

Yes, it's in that step, but xmlto is just driving the xslt
transformation done by docbook. So the interesting version is probably
docbook. I have docbook-xsl 1.79.1+dfsg-2 (from Debian unstable).

-Peff

Re: Minor man page weirdness?

[Andreas Schwab]

On Okt 17 2017, Jeff King  wrote:

> On Tue, Oct 17, 2017 at 06:29:59PM +0200, Andreas Schwab wrote:
>
>> On Okt 16 2017, Jeff King  wrote:
>> 
>> > I get:
>> >
>> >   Reset  to  if  exists already\&. 
>> > Without
>> >   \fB\-f\fR
>> >   \fIgit branch\fR
>> >   refuses to change an existing branch\&. In combination with
>> 
>> I see this in git-branch.1:
>> 
>>   Reset  to  if  exists already\&. 
>> Without
>>   \fB\-f\fR\fIgit branch\fR
>>   refuses to change an existing branch\&. In combination with
>> 
>> This is with asciidoc 8.6.9.
>
> Thanks, that seems a likely culprit, then. What's in your git-branch.xml
> after you build the documentation?

Same as yours, with the space.  I'd guess it's rather xmlto (version
0.0.25) that's the culprit here.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Is t5601 flaky for anybody else?

[Brandon Williams]

On 10/17, Junio C Hamano wrote:
> I seem to be seeing sporadic errors with this test, and today I got
> annoyed enough to do
> 
>   cd t && while sh t5601-clone.sh -i -v; do :; done
> 
> I saw an error from "cp" saying "plink.exe - text file busy" or
> something like that at around test #45; here is an workaround that
> seems to work (the above loop is spinning without problem for
> several minutes now).

I haven't noticed any issues myself but maybe this has something to do
with my changes to this test in the 'bw/protocol-v1' topic?

> 
>  t/t5601-clone.sh | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 9c56f771b6..50e40abb11 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -308,6 +308,7 @@ test_expect_success 'clone checking out a tag' '
>  
>  setup_ssh_wrapper () {
>   test_expect_success 'setup ssh wrapper' '
> + rm -f "$TRASH_DIRECTORY/ssh-wrapper$X" &&
>   cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
>   "$TRASH_DIRECTORY/ssh-wrapper$X" &&
>   GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
> @@ -318,6 +319,7 @@ setup_ssh_wrapper () {
>  }
>  
>  copy_ssh_wrapper_as () {
> + rm -f "${1%$X}$X" &&
>   cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
>   GIT_SSH="${1%$X}$X" &&
>   export GIT_SSH

-- 
Brandon Williams

Re: Minor man page weirdness?

[Jeff King]

On Tue, Oct 17, 2017 at 06:29:59PM +0200, Andreas Schwab wrote:

> On Okt 16 2017, Jeff King  wrote:
> 
> > I get:
> >
> >   Reset  to  if  exists already\&. 
> > Without
> >   \fB\-f\fR
> >   \fIgit branch\fR
> >   refuses to change an existing branch\&. In combination with
> 
> I see this in git-branch.1:
> 
>   Reset  to  if  exists already\&. Without
>   \fB\-f\fR\fIgit branch\fR
>   refuses to change an existing branch\&. In combination with
> 
> This is with asciidoc 8.6.9.

Thanks, that seems a likely culprit, then. What's in your git-branch.xml
after you build the documentation?

-Peff

Re: Minor man page weirdness?

[Andreas Schwab]

On Okt 16 2017, Jeff King  wrote:

> I get:
>
>   Reset  to  if  exists already\&. Without
>   \fB\-f\fR
>   \fIgit branch\fR
>   refuses to change an existing branch\&. In combination with

I see this in git-branch.1:

  Reset  to  if  exists already\&. Without
  \fB\-f\fR\fIgit branch\fR
  refuses to change an existing branch\&. In combination with

This is with asciidoc 8.6.9.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH] l10n: de.po: translate 70 new messages

[Ralf Thielow]

Translate 70 new messages came from git.pot update in 25eab542b
(l10n: git.pot: v2.15.0 round 1 (68 new, 36 removed)) and 9c07fab78
(l10n: git.pot: v2.15.0 round 2 (2 new, 2 removed)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 253 ++-
 1 file changed, 106 insertions(+), 147 deletions(-)

diff --git a/po/de.po b/po/de.po
index c76fe575c..0619c4988 100644
--- a/po/de.po
+++ b/po/de.po
@@ -92,21 +92,20 @@ msgid ""
 "\n"
 "  git checkout -b \n"
 "\n"
 msgstr ""
 "Hinweis: Checke '%s' aus.\n"
 "\n"
-"Sie befinden sich im Zustand eines 'lösgelösten HEAD'. Sie können sich\n"
+"Sie befinden sich im Zustand eines 'losgelösten HEAD'. Sie können sich\n"
 "umschauen, experimentelle Änderungen vornehmen und diese committen, und\n"
 "Sie können alle möglichen Commits, die Sie in diesem Zustand machen,\n"
 "ohne Auswirkungen auf irgendeinen Branch verwerfen, indem Sie einen\n"
 "weiteren Checkout durchführen.\n"
 "\n"
 "Wenn Sie einen neuen Branch erstellen möchten, um Ihre erstellten Commits\n"
-"zu behalten, können Sie das (jetzt oder später) durch einen weiteren "
-"Checkout\n"
+"zu behalten, können Sie das (jetzt oder später) durch einen weiteren 
Checkout\n"
 "mit der Option -b tun. Beispiel:\n"
 "\n"
 "  git checkout -b \n"
 "\n"
 
 #: apply.c:58
@@ -1066,52 +1065,50 @@ msgstr ""
 #: branch.c:67
 #, c-format
 msgid "Not setting branch %s as its own upstream."
 msgstr "Branch %s kann nicht sein eigener Upstream-Branch sein."
 
 #: branch.c:93
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track remote branch '%s' from '%s' by rebasing."
-msgstr ""
-"Branch %s konfiguriert zum Folgen von Remote-Branch %s von %s durch Rebase."
+msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Branch '%s' von '%s' 
durch Rebase."
 
 #: branch.c:94
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track remote branch '%s' from '%s'."
-msgstr "Branch %s konfiguriert zum Folgen von Remote-Branch %s von %s."
+msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Branch '%s' von '%s'."
 
 #: branch.c:98
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track local branch '%s' by rebasing."
-msgstr "Branch %s konfiguriert zum Folgen von lokalem Branch %s durch Rebase."
+msgstr "Branch '%s' konfiguriert zum Folgen von lokalem Branch '%s' durch 
Rebase."
 
 #: branch.c:99
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track local branch '%s'."
-msgstr "Branch %s konfiguriert zum Folgen von lokalem Branch %s."
+msgstr "Branch '%s' konfiguriert zum Folgen von lokalem Branch '%s'."
 
 #: branch.c:104
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track remote ref '%s' by rebasing."
-msgstr "Branch %s konfiguriert zum Folgen von Remote-Referenz %s durch Rebase."
+msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Referenz '%s' durch 
Rebase."
 
 #: branch.c:105
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track remote ref '%s'."
-msgstr "Branch %s konfiguriert zum Folgen von Remote-Referenz %s."
+msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Referenz '%s'."
 
 #: branch.c:109
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track local ref '%s' by rebasing."
-msgstr ""
-"Branch %s konfiguriert zum Folgen von lokaler Referenz %s durch Rebase."
+msgstr "Branch '%s' konfiguriert zum Folgen von lokaler Referenz '%s' durch 
Rebase."
 
 #: branch.c:110
-#, fuzzy, c-format
+#, c-format
 msgid "Branch '%s' set up to track local ref '%s'."
-msgstr "Branch %s konfiguriert zum Folgen von lokaler Referenz %s."
+msgstr "Branch '%s' konfiguriert zum Folgen von lokaler Referenz '%s'."
 
 #: branch.c:119
 msgid "Unable to write upstream branch configuration"
 msgstr "Konnte Konfiguration zu Upstream-Branch nicht schreiben."
 
 #: branch.c:156
@@ -1623,13 +1620,13 @@ msgid "  Unknown dirstat parameter '%s'\n"
 msgstr "  Unbekannter \"dirstat\" Parameter '%s'\n"
 
 #: diff.c:281
 msgid ""
 "color moved setting must be one of 'no', 'default', 'zebra', 'dimmed_zebra', "
 "'plain'"
-msgstr ""
+msgstr "\"color moved\"-Einstellung muss eines von diesen sein: 'no', 
'default', 'zebra', 'dimmed_zebra', 'plain'"
 
 #: diff.c:341
 #, c-format
 msgid "Unknown value for 'diff.submodule' config variable: '%s'"
 msgstr "Unbekannter Wert in Konfigurationsvariable 'diff.dirstat': '%s'"
 
@@ -1706,15 +1703,14 @@ msgstr "Konnte Verzeichnisse für '%s' nicht erstellen."
 #: dir.c:2915
 #, c-format
 msgid "could not migrate git directory from '%s' to '%s'"
 msgstr "Konnte Git-Verzeichnis nicht von '%s' nach '%s' migrieren."
 
 #: entry.c:176
-#, fuzzy
 msgid "Filtering content"
-msgstr "Tag-Inhalte ausgeben"
+msgstr "Filtere Inhalt"
 
 #: entry.c:433
 #, c-format
 msgid "could not stat file '%s'"
 msgstr "konnte Datei '%s' nicht lesen"
 
@@ -2335,13 +2331,12 @@ msgstr ""
 #: merge-recursive.c:1917
 #, c-format
 msgid "Adding %s"
 msgstr "Füge %s hinzu"
 
 #: 

[PATCH] sequencer.c: unify an error message

[Ralf Thielow]

Change an error message in sequencer.c for the case that
we could not write to a file to match other instances.

Signed-off-by: Ralf Thielow 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 75f5356f6..f2a10cc4f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2948,7 +2948,7 @@ int rearrange_squash(void)
if (fd < 0)
res = error_errno(_("could not open '%s'"), todo_file);
else if (write(fd, buf.buf, buf.len) < 0)
-   res = error_errno(_("could not write '%s'"), todo_file);
+   res = error_errno(_("could not write to '%s'"), 
todo_file);
else if (ftruncate(fd, buf.len) < 0)
res = error_errno(_("could not truncate '%s'"),
   todo_file);
-- 
2.15.0.rc1.299.gda03b47c3

Re: Multiple paths in GIT_EXEC_PATH

[Kevin Daudt]

On Tue, Oct 17, 2017 at 06:21:22PM +0300, Nikolay Yakimov wrote:
> Hello. After updating to a recent git release (2.14, I believe, but
> possibly earlier), setting GIT_EXEC_PATH to multiple directories
> stopped working. It did work before, and I believe the culprit is
> 'git-sh-setup', which uses 'git --exec-path' output directly, while
> most other git components observe PATH syntax, i.e. multiple paths
> with colon separator. Is this intended, or is it an oversight?
> 
> For why I need that is another matter. Long story short, I need git to
> look for '.gitignore' in a particular non-standard location, since I
> have multiple git repositories in the same workdir (that workdir being
> $HOME and git repositories being stores for my different configs)

The commit that changed what you described is 1073094f3 (git-sh-setup:
be explicit where to dot-source git-sh-i18n from., 2016-10-29). That
commit claims there were already scripts that assumed GIT_EXEC_PATH is
just a single entry. That commit was included in v2.11.

There was also a recent thread[0] about it that discussed this issue,
where someone stated that indeed treating GIT_EXEC_PATH with the same
semantics as PATH has been broken for a while, but it seems there are no
real plans to fix it.

[0]:https://public-inbox.org/git/20170928223134.GA30744@varnish/

[PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

[René Scharfe]

Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
without any other options has shown the short help text of the command
instead of acting as the short equivalent of --heads.  Fix this
regression by turning off internal handling of -h for repository setup,
and option parsing, as well as the corresponding test in t0012.

Reported-by: Thomas Rikl 
Analyzed-by: Martin Ågren 
Signed-off-by: Rene Scharfe 
---
 builtin/ls-remote.c  | 1 +
 git.c| 2 +-
 t/t0012-help.sh  | 1 +
 t/t5512-ls-remote.sh | 6 ++
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..5f27d252b4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -68,6 +68,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
};
 
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
+PARSE_OPT_NO_INTERNAL_HELP |
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
 
diff --git a/git.c b/git.c
index 9d1b8c4716..056a123506 100644
--- a/git.c
+++ b/git.c
@@ -421,7 +421,7 @@ static struct cmd_struct commands[] = {
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
{ "ls-files", cmd_ls_files, RUN_SETUP },
-   { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
+   { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY | NO_INTERNAL_HELP },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
{ "mailsplit", cmd_mailsplit },
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 74eeead168..05d8cf9b49 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -54,6 +54,7 @@ test_expect_success 'generate builtin list' '
 '
 
 cat >no_internal_help long &&
+   test_cmp long short
+'
+
 test_done
-- 
2.14.2

[Alt. PATCH] ls-remote: deprecate -h as short for --heads

[René Scharfe]

Stop advertising -h as the short equivalent of --heads, because it's
used for showing a short help text for almost all other git commands.
Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
been working when used together with other parameters anyway.

Signed-off-by: Rene Scharfe 
---
That would be step on the way towards more consistent command line
switches, in the same vein as d69155119 (t0012: test "-h" with
builtins).

 Documentation/git-ls-remote.txt | 1 -
 builtin/ls-remote.c | 4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f8..898836a111 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -21,7 +21,6 @@ commit IDs.
 
 OPTIONS
 ---
--h::
 --heads::
 -t::
 --tags::
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..840deedbef 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -56,7 +56,9 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
   N_("path of git-upload-pack on the remote host"),
   PARSE_OPT_HIDDEN },
OPT_BIT('t', "tags", , N_("limit to tags"), REF_TAGS),
-   OPT_BIT('h', "heads", , N_("limit to heads"), REF_HEADS),
+   OPT_BIT(0, "heads", , N_("limit to heads"), REF_HEADS),
+   { OPTION_BIT, 'h', NULL, , NULL, N_("limit to heads"),
+   PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, REF_HEADS },
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
-- 
2.14.2

[PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins

[René Scharfe]

Builtin commands have skipped repo setup when called with just a single
option -h and thus shown their short help text even outside of
repositories since 99caeed05d3 (Let 'git  -h' show usage
without a git dir).

Add the flag NO_INTERNAL_HELP for builtins to opt out of this special
treatment and provide a list of commands that are exempt from the
corresponding test in t0012.  That allows builtins to handle -h
themselves.

Signed-off-by: Rene Scharfe 
---
 git.c   | 6 +-
 t/t0012-help.sh | 9 -
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 9e96dd4090..9d1b8c4716 100644
--- a/git.c
+++ b/git.c
@@ -298,6 +298,7 @@ static int handle_alias(int *argcp, const char ***argv)
 #define NEED_WORK_TREE (1<<3)
 #define SUPPORT_SUPER_PREFIX   (1<<4)
 #define DELAY_PAGER_CONFIG (1<<5)
+#define NO_INTERNAL_HELP   (1<<6)
 
 struct cmd_struct {
const char *cmd;
@@ -312,7 +313,10 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
const char *prefix;
 
prefix = NULL;
-   help = argc == 2 && !strcmp(argv[1], "-h");
+   if (p->option & NO_INTERNAL_HELP)
+   help = 0;
+   else
+   help = argc == 2 && !strcmp(argv[1], "-h");
if (!help) {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 487b92a5de..74eeead168 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -53,12 +53,19 @@ test_expect_success 'generate builtin list' '
git --list-builtins >builtins
 '
 
+cat >no_internal_help output 2>&1 &&
test_i18ngrep usage output
'
-done 

Multiple paths in GIT_EXEC_PATH

[Nikolay Yakimov]

Hello. After updating to a recent git release (2.14, I believe, but
possibly earlier), setting GIT_EXEC_PATH to multiple directories
stopped working. It did work before, and I believe the culprit is
'git-sh-setup', which uses 'git --exec-path' output directly, while
most other git components observe PATH syntax, i.e. multiple paths
with colon separator. Is this intended, or is it an oversight?

For why I need that is another matter. Long story short, I need git to
look for '.gitignore' in a particular non-standard location, since I
have multiple git repositories in the same workdir (that workdir being
$HOME and git repositories being stores for my different configs)

Attendees:- Society for Neuroscience - SfN-2017

[Donald Charles]

 

Hi ,

 

Would you be interested in the "Attendees list of Society for Neuroscience -
SfN-2017"

Let me know your interest to send you the number of Attendees and cost .

 

 

Regards,

Donald Charles,

If you do not wish to receive future emails from us, please reply as 'leave
out'.

 

 

 

<>

[PATCH] status: do not get confused by submodules in excluded directories

[Johannes Schindelin]

We meticulously pass the `exclude` flag to the `treat_directory()`
function so that we can indicate that files in it are excluded rather
than untracked when recursing.

But we did not yet treat submodules the same way.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/submodule-in-excluded-v1
Fetch-It-Via: git fetch https://github.com/dscho/git submodule-in-excluded-v1
 dir.c  |  2 +-
 t/t7061-wtstatus-ignore.sh | 14 ++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 1d17b800cf3..9987011da57 100644
--- a/dir.c
+++ b/dir.c
@@ -1392,7 +1392,7 @@ static enum path_treatment treat_directory(struct 
dir_struct *dir,
if (!(dir->flags & DIR_NO_GITLINKS)) {
unsigned char sha1[20];
if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
-   return path_untracked;
+   return exclude ? path_excluded : path_untracked;
}
return path_recurse;
}
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index fc6013ba3c8..8c849a4cd2f 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -272,4 +272,18 @@ test_expect_success 'status ignored tracked directory with 
uncommitted file in t
test_cmp expected actual
 '
 
+cat >expected <<\EOF
+!! tracked/submodule/
+EOF
+
+test_expect_success 'status ignores submodule in excluded directory' '
+   git init tracked/submodule &&
+   (
+   cd tracked/submodule &&
+   test_commit initial
+   ) &&
+   git status --porcelain --ignored -u tracked/submodule >actual &&
+   test_cmp expected actual
+'
+
 test_done

base-commit: 111ef79afe185f8731920569450f6a65320f5d5f
-- 
2.14.2.windows.3

Re: Deleting a branch after merging it results in "there may be uncommitted changes"

[Florian Weimer]

On 10/05/2017 05:04 AM, Joshua Lamusga wrote:

Anyway, I follow a very simple merging model for this one-person
project. Recently, I made a new local branch off of develop called
feature-printing. After checking out feature-printing, making my
changes, and committing changes, I merged it with develop. I then
immediately tried to delete feature-printing, which resulted in a
prompt asking if I was sure since it might contain uncommitted
changes. Though I've seen this problem many times on the internet, I
haven't seen it in the context of literally just merging. There are 0
steps between merging and deleting the old branch.


What's the exact warning?  It seems unlikely that your Git interface 
would complain about *uncommitted* changes in this context.



All of this is done in Visual Studio's GUI for Git. Any ideas?


Maybe ask on a Visual Studio forum instead, in case this is something 
generated by the front end?


Florian

Salut mon bon ami!.

[Mr Aapo Issa]

Salut mon bon ami!

Vous avez été désigné comme le plus proche parent de mon client en
retard qui a déposé les 7,3 millions de dollars avant sa mort et votre
profil a été ajouté à notre base de données. Vous êtes maintenant ravi
de répondre avec effet immédiat pour plus information.

Voici votre numéro de badge d'inscription: 678378

[PATCH v2] doc: list filter-branch subdirectory-filter first

[David Glasser]

From: David Glasser 

The docs claim that filters are applied in the listed order, so
subdirectory-filter should come first.

For consistency, apply the same order to the SYNOPSIS and the script's usage, as
well as the switch while parsing arguments.

Add missing --prune-empty to the script's usage.

Signed-off-by: David Glasser 
---
 Documentation/git-filter-branch.txt | 20 ++--
 git-filter-branch.sh| 20 ++--
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index 9e5169aa64f4f..394f74451a659 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -8,11 +8,11 @@ git-filter-branch - Rewrite branches
 SYNOPSIS
 
 [verse]
-'git filter-branch' [--setup ] [--env-filter ]
-   [--tree-filter ] [--index-filter ]
-   [--parent-filter ] [--msg-filter ]
-   [--commit-filter ] [--tag-name-filter ]
-   [--subdirectory-filter ] [--prune-empty]
+'git filter-branch' [--setup ] [--subdirectory-filter ]
+   [--env-filter ] [--tree-filter ]
+   [--index-filter ] [--parent-filter ]
+   [--msg-filter ] [--commit-filter ]
+   [--tag-name-filter ] [--prune-empty]
[--original ] [-d ] [-f | --force]
[--] [...]
 
@@ -89,6 +89,11 @@ OPTIONS
can be used or modified in the following filter steps except
the commit filter, for technical reasons.
 
+--subdirectory-filter ::
+   Only look at the history which touches the given subdirectory.
+   The result will contain that directory (and only that) as its
+   project root. Implies <>.
+
 --env-filter ::
This filter may be used if you only need to modify the environment
in which the commit will be performed.  Specifically, you might
@@ -167,11 +172,6 @@ be removed, buyer beware. There is also no support for 
changing the
 author or timestamp (or the tag message for that matter). Tags which point
 to other tags will be rewritten to point to the underlying commit.
 
---subdirectory-filter ::
-   Only look at the history which touches the given subdirectory.
-   The result will contain that directory (and only that) as its
-   project root. Implies <>.
-
 --prune-empty::
Some filters will generate empty commits that leave the tree untouched.
This option instructs git-filter-branch to remove such commits if they
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 3a74602ef3771..b7827e745a92a 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -81,12 +81,12 @@ set_ident () {
finish_ident COMMITTER
 }
 
-USAGE="[--setup ] [--env-filter ]
-   [--tree-filter ] [--index-filter ]
-   [--parent-filter ] [--msg-filter ]
-   [--commit-filter ] [--tag-name-filter ]
-   [--subdirectory-filter ] [--original ]
-   [-d ] [-f | --force]
+USAGE="[--setup ] [--subdirectory-filter ]
+   [--env-filter ] [--tree-filter ]
+   [--index-filter ] [--parent-filter ]
+   [--msg-filter ] [--commit-filter ]
+   [--tag-name-filter ] [--prune-empty]
+   [--original ] [-d ] [-f | --force]
[--] [...]"
 
 OPTIONS_SPEC=
@@ -153,6 +153,10 @@ do
--setup)
filter_setup="$OPTARG"
;;
+   --subdirectory-filter)
+   filter_subdir="$OPTARG"
+   remap_to_ancestor=t
+   ;;
--env-filter)
filter_env="$OPTARG"
;;
@@ -174,10 +178,6 @@ do
--tag-name-filter)
filter_tag_name="$OPTARG"
;;
-   --subdirectory-filter)
-   filter_subdir="$OPTARG"
-   remap_to_ancestor=t
-   ;;
--original)
orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
;;

--
https://github.com/git/git/pull/415

Hi Dear,

[Lisa Williams]

Hi Dear,

how are you today I hope that everything is OK with you as it is my great 
pleasure to contact you in having communication with you starting from today, i 
was just going through the Internet search when i found your email address, I 
want to make a very new and special friend, so i decided to contact you to see 
how we can make it work if we can. Please i wish you will have the desire with 
me so that we can get to know each other better and see what happens in future.

My name is Lisa Williams, I am an American  presently I live in the UK, I will 
be very happy if you can write me through my private email 
address(lisa.williams...@yahoo.com) for easy communication so that we can know 
each other, I will give you my pictures and details about me.

bye
Lisa

Re: [ANNOUNCE] Git for Windows 2.14.2(3)

[Johannes Schindelin]

Hi Steve,

On Mon, 16 Oct 2017, Steve Hoelzer wrote:

> On Mon, Oct 16, 2017 at 5:57 AM, Johannes Schindelin
>  wrote:
> > Hi Steve,
> >
> > On Sun, 15 Oct 2017, Johannes Schindelin wrote:
> >
> >> On Fri, 13 Oct 2017, Steve Hoelzer wrote:
> >>
> >> > On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin
> >> >  wrote:
> >> > >
> >> > > It is my pleasure to announce that Git for Windows 2.14.2(3) is
> >> > > available from:
> >> > >
> >> > > https://git-for-windows.github.io/
> >> > >
> >> > > Changes since Git for Windows v2.14.2(2) (October 5th 2017)
> >> > >
> >> > > New Features
> >> > >
> >> > >   * Comes with Git LFS v2.3.3.
> >> >
> >> > I just ran "git update" and afterward "git version" reported
> >> > 2.14.2(3), but "git lfs version" still said 2.3.2.
> >> >
> >> > I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2.
> >>
> >> Ah bummer. I forgot to actually update it in the VM where I build the
> >> releases :-(
> >>
> >> Will work on it tomorrow.
> >
> > I'll actually use this opportunity to revamp a part of Git for Windows'
> > release engineering process to try to prevent similar things from
> > happening in the future.
> >
> > Also, cURL v7.56.1 is slated to be released in exactly one week, and I
> > have some important installer work to do this week, so I'll just defer the
> > new Git for Windows version tentatively to Monday, 23rd.
> >
> > Git LFS users can always install Git LFS v2.3.3 specifically in the
> > meantime, or use Git for Windows' snapshot versions
> > (https://wingit.blob.core.windows.net/files/index.html).
> 
> Sounds like a good plan.
> 
> I think I have successfully updated to LFS 2.3.3 by copying the new
> git-lfs.exe into C:\Program Files\Git\mingw64\bin. Is that right way
> to do it?

That should be enough in your case, as the config is already written by
the Git for Windows installer.

In general, the best way to install a new Git LFS version seems to be to
download and run the Git LFS installer:

https://github.com/git-lfs/git-lfs/releases/download/v2.3.3/git-lfs-windows-2.3.3.exe

Ciao,
Johannes

Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Jonathan Nieder  writes:

> Junio C Hamano wrote:
>> Jonathan Nieder  writes:
>
>>> Handles the nongit case in strbuf_check_branch_ref instead of
>>> introducing a new check_branch_ref_format helper.
>>
>> I view that as a regression, actually.  Don't we want a function
>> that does not require a strbuf when asking a simple question: "I
>> have a string, and I want to see if that is a valid name"?
>
> *shrug* I found the change easier to read, and it also sidesteps the
> which-header question.  It also ensures that other
> strbuf_check_branch_ref callers are safe without having to audit them.

Please ignore the above, which was merely an impression _without_
and before having received any patch to comment on ;-)

Quite frankly, this is a Meh topic that won't have to hit even
'next' before the final.  The color.ui=always thing has a lot more
urgency, and this was merely what I did while waiting for others to
react to that topic.

Re: [PATCH] check-ref-format: require a repository for --branch

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> And in that spirit, I think the patch you replied with aims to go in
>> the right direction, by providing the core functionality when in a
>> repository while avoiding breaking such a script outside of one
>> (though I do not understand it fully yet).
>
> Given that, is it safe for me to ignore this earlier one
>
>> For what it's worth, I don't agree with this repurposing of
>> "check-ref-format --branch" at all.
>
> as reacting to the patch without reading what it does?

On second reading, yes, I was reacting to the discussion leading up to
the patch instead of the patch.  The patch looks good.

Sorry for the noise,
Jonathan

Re: [L10N] Kickoff of translation for Git 2.15.0 round 2

[Ralf Thielow]

2017-10-17 8:53 GMT+02:00 Junio C Hamano :
> Jiang Xin  writes:
>
>> Git v2.15.0-rc1 released with a typo fix from commit dfab1eac23
>> ("i18n: add a missing space in message", Sun Oct 8 14:18:39 2017 +0200).
>> This time there are 2 updated messages need to be translated since last
>> update.  Let's start new round of translation for Git 2.15.0.
>
> Today's pushout included 3247edbb ("sequencer.c: fix and unify error
> messages in rearrange_squash()", 2017-10-15), which unfortunately
> added this to the mix:
>
> error_errno(_("could not write '%s'"), todo_file);
>
> There is another one "could not truncate '%s'" added by the same
> commit, but the codebase already had another instance of the same
> string, so it won't cause trouble.
>
> Regarding the "could not write" thing, there are quite a few
> instances of "coulld not write to '%s'" with %s set to the filename,
> and I strongly suspect that this new one should be further updated
> to match them (Ralf promoted from Cc: to To: for this).  That would
> have a nice side effect of making today's last-minute merges a
> non-event from translators' point of view ;-)
>
>
>

I didn't expect this message to be a new one for translation. I'll send
a patch later to update the message to match the other instances.

Re: [PATCH] check-ref-format: require a repository for --branch

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> And in that spirit, I think the patch you replied with aims to go in
>> the right direction, by providing the core functionality when in a
>> repository while avoiding breaking such a script outside of one
>> (though I do not understand it fully yet).
>
> Given that, is it safe for me to ignore this earlier one
>
>> For what it's worth, I don't agree with this repurposing of
>> "check-ref-format --branch" at all.
>
> as reacting to the patch without reading what it does?

Are you saying I should have ignored the commit message?  Recording
future plans via the commit message is part of what the patch does,
after all.

>> Junio C Hamano wrote:

>>>(e.g. a wrapper to "git
>>> clone" that wants to verify the name it is going to give to the "-b"
>>> option), and a check desired in such a context is different from
>>> (and is stricter than) feeding refs/heads/$name to the same command
>>> without the "--branch" option.
>>
>> Can you say more about this example?  E.g. why is this hypothetical
>> wrapper unable to rely on "git clone -b"'s own error handling?
>
> If I have to guess what you meant, perhaps the Porcelain wanted to
> diagnose bad parameter that would be rejected _before_ letting clone
> spend cycles and possibly network bandwidth?
>
> But this was my way of rephrasing an earlier example you used while
> objecting to Peff's change
[...]
> so my answer to the question in the message I am directly responding
> to would be "You tell me." ;-)

Hm.  Does the example in
https://public-inbox.org/git/20171017070808.plddffhzdobye...@aiede.mtv.corp.google.com/
make it clearer?

The goal of such a script is *not* error handling --- that is just a
pleasant side-benefit.  It is to be able to handle all branch
specifiers from the user (and even a default branch that is not from
the user) uniformly.

Thanks,
Jonathan

Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> Handles the nongit case in strbuf_check_branch_ref instead of
>> introducing a new check_branch_ref_format helper.
>
> I view that as a regression, actually.  Don't we want a function
> that does not require a strbuf when asking a simple question: "I
> have a string, and I want to see if that is a valid name"?

*shrug* I found the change easier to read, and it also sidesteps the
which-header question.  It also ensures that other
strbuf_check_branch_ref callers are safe without having to audit them.

But feel free to tweak that back if you like, or I can tomorrow.

Jonathan

Re: [PATCH] patch reply

[Marius Paliga]

I just sent a patch to a new thread which is not what I wanted.
However I already sent the same patch a few days ago:
https://public-inbox.org/git/CAK7vU=2ePR3jQsgu=rxsmrxytaahqxc0sfrn5yozlzqzp2z...@mail.gmail.com/


2017-10-17 6:01 GMT+02:00 Junio C Hamano :
> Thais Diniz  writes:
>
>> +Just to clarify I did not see Marius patch.
>> +Did see Marius' comment saying he would look it in the leftoverbits list,
>> +but since i didn't see any patch i thought i could work on it and did so 
>> based on Stephan's comment
>> +(which i suppose Mario also did and that is why the code resulted to be 
>> similar).
>
> In any case, both versions share exactly the same "don't call
> get_multi() to grab the same configuration values from inside the
> callback of git_config()" issue, so whoever works on it to complete
> the topic, it needs further work.

[PATCH 3/3] check-ref-format doc: --branch validates and expands

[Jonathan Nieder]

From: Junio C Hamano 

"git check-ref-format --branch $name" feature was originally
introduced (and was advertised) as a way for scripts to take any
end-user supplied string (like "master", "@{-1}" etc.) and see if it
is usable when Git expects to see a branch name, and also obtain the
concrete branch name that the at-mark magic expands to.

Emphasize that "see if it is usable" role in the description and
clarify that the @{...} expansion only occurs when run from within a
repository.

[jn: split out from a larger patch]

Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-check-ref-format.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-check-ref-format.txt 
b/Documentation/git-check-ref-format.txt
index 92777cef25..cf0a0b7df2 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -77,7 +77,14 @@ reference name expressions (see linkgit:gitrevisions[7]):
 
 . at-open-brace `@{` is used as a notation to access a reflog entry.
 
-With the `--branch` option, it expands the ``previous branch syntax''
+With the `--branch` option, the command takes a name and checks if
+it can be used as a valid branch name (e.g. when creating a new
+branch).  The rule `git check-ref-format --branch $name` implements
+may be stricter than what `git check-ref-format refs/heads/$name`
+says (e.g. a dash may appear at the beginning of a ref component,
+but it is explicitly forbidden at the beginning of a branch name).
+When run with `--branch` option in a repository, the input is first
+expanded for the ``previous branch syntax''
 `@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
 were on.  This option should be used by porcelains to accept this
 syntax anywhere a branch name is expected, so they can act as if you
-- 
2.15.0.rc1.287.g2b38de12cc

Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Jonathan Nieder  writes:

> Handles the nongit case in strbuf_check_branch_ref instead of
> introducing a new check_branch_ref_format helper.

I view that as a regression, actually.  Don't we want a function
that does not require a strbuf when asking a simple question: "I
have a string, and I want to see if that is a valid name"?

[PATCH 2/3] check-ref-format --branch: strip refs/heads/ using skip_prefix

[Jonathan Nieder]

From: Junio C Hamano 

The expansion returned from strbuf_check_branch_ref always starts with
"refs/heads/" by construction, but there is nothing about its name or
advertised API making that obvious.  This command is used to process
human-supplied input from the command line and is usually not the
inner loop, so we can spare some cycles to be more defensive.  Instead
of hard-coding the offset strlen("refs/heads/") to skip, verify that
the expansion actually starts with refs/heads/.

[jn: split out from a larger patch, added explanation]

Signed-off-by: Junio C Hamano 
Signed-off-by: Jonathan Nieder 
---
 builtin/check-ref-format.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 6c40ff110b..bc67d3f0a8 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -39,12 +39,14 @@ static char *collapse_slashes(const char *refname)
 static int check_ref_format_branch(const char *arg)
 {
struct strbuf sb = STRBUF_INIT;
+   const char *name;
int nongit;
 
setup_git_directory_gently();
-   if (strbuf_check_branch_ref(, arg))
+   if (strbuf_check_branch_ref(, arg) ||
+   !skip_prefix(sb.buf, "refs/heads/", ))
die("'%s' is not a valid branch name", arg);
-   printf("%s\n", sb.buf + 11);
+   printf("%s\n", name);
strbuf_release();
return 0;
 }
-- 
2.15.0.rc1.287.g2b38de12cc

[PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository

[Jonathan Nieder]

From: Junio C Hamano 

Running "git check-ref-format --branch @{-1}" from outside any
repository produces

$ git check-ref-format --branch @{-1}
BUG: environment.c:182: git environment hasn't been setup

This is because the expansion of @{-1} must come from the HEAD reflog,
which involves opening the repository.  @{u} and @{push} (which are
more unusual because they typically would not expand to a local
branch) trigger the same assertion.

This has been broken since day one.  Before v2.13.0-rc0~48^2
(setup_git_env: avoid blind fall-back to ".git", 2016-10-02), the
breakage was more subtle: Git would read reflogs from ".git" within
the current directory even if it was not a valid repository.  Usually
that is harmless because Git is not being run from the root directory
of an invalid repository, but in edge cases such accesses can be
confusing or harmful.  Since v2.13.0, the problem is easier to
diagnose because Git aborts with a BUG message.

Erroring out is the right behavior: when asked to interpret a branch
name like "@{-1}", there is no reasonable answer in this context.
But we should print a message saying so instead of an assertion failure.

We do not forbid "check-ref-format --branch" from outside a repository
altogether because it is ok for a script to pre-process branch
arguments without @{...} in such a context.  For example, with
pre-2.13 Git, a script that does

branch='master'; # default value
parse_options
branch=$(git check-ref-format --branch "$branch")

to normalize an optional branch name provided by the user would work
both inside a repository (where the user could provide '@{-1}') and
outside (where '@{-1}' should not be accepted).

So disable the "expand @{...}" half of the feature when run outside a
repository, but keep the check of the syntax of a proposed branch
name. This way, when run from outside a repository, "git
check-ref-format --branch @{-1}" will gracefully fail:

$ git check-ref-format --branch @{-1}
fatal: '@{-1}' is not a valid branch name

and "git check-ref-format --branch master" will succeed as before:

$ git check-ref-format --branch master
master

restoring the usual pre-2.13 behavior.

[jn: split out from a larger patch; moved conditional to
 strbuf_check_branch_ref instead of its caller; fleshed out commit
 message; some style tweaks in tests]

Reported-by: Marko Kungla 
Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
Signed-off-by: Jonathan Nieder 
---
 sha1_name.c |  5 -
 t/t1402-check-ref-format.sh | 16 
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index c7c5ab376c..603e667faa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1331,7 +1331,10 @@ void strbuf_branchname(struct strbuf *sb, const char 
*name, unsigned allowed)
 
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 {
-   strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+   if (startup_info->have_repository)
+   strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+   else
+   strbuf_addstr(sb, name);
if (name[0] == '-')
return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 0790edf60d..98e4a8613b 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -144,6 +144,11 @@ test_expect_success "check-ref-format --branch @{-1}" '
refname2=$(git check-ref-format --branch @{-2}) &&
test "$refname2" = master'
 
+test_expect_success 'check-ref-format --branch -naster' '
+   test_must_fail git check-ref-format --branch -naster >actual &&
+   test_must_be_empty actual
+'
+
 test_expect_success 'check-ref-format --branch from subdir' '
mkdir subdir &&
 
@@ -161,6 +166,17 @@ test_expect_success 'check-ref-format --branch from 
subdir' '
test "$refname" = "$sha1"
 '
 
+test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
+   nongit test_must_fail git check-ref-format --branch @{-1} >actual &&
+   test_must_be_empty actual
+'
+
+test_expect_success 'check-ref-format --branch master from non-repo' '
+   echo master >expect &&
+   nongit git check-ref-format --branch master >actual &&
+   test_cmp expect actual
+'
+
 valid_ref_normalized() {
prereq=
case $1 in
-- 
2.15.0.rc1.287.g2b38de12cc

[PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

[Jonathan Nieder]

Hi,

Junio C Hamano wrote:

> Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a 
> repository

How about this?  It is written to be more conservative than the patch
I am replying to, but except for the commit message, it should be
pretty much equivalent.

[...]
> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -38,13 +38,22 @@ static char *collapse_slashes(const char *refname)
>  
>  static int check_ref_format_branch(const char *arg)
>  {
> + int nongit, malformed;
>   struct strbuf sb = STRBUF_INIT;
> - int nongit;
> + const char *name = arg;
>  
>   setup_git_directory_gently();
> - if (strbuf_check_branch_ref(, arg))
> +
> + if (!nongit)
> + malformed = (strbuf_check_branch_ref(, arg) ||
> +  !skip_prefix(sb.buf, "refs/heads/", ));
> + else
> + malformed = check_branch_ref_format(arg);

Handles the nongit case in strbuf_check_branch_ref instead of
introducing a new check_branch_ref_format helper.

[...]
> --- a/cache.h
> +++ b/cache.h
> @@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct 
> object_id *oid, const char **en
>  #define INTERPRET_BRANCH_HEAD (1<<2)
>  extern int interpret_branch_name(const char *str, int len, struct strbuf *,
>unsigned allowed);
> +
> +/*
> + * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref()
> + * here, not in strbuf.h
> + */

As a result, it doesn't touch headers.  I agree that these functions
don't belong in strbuf.h (sorry for not updating the headers at the
same time I moved their implementations) but suspect e.g. branch.h,
revision.h, or some new header like revision-syntax.h would be a
better place.

[...]
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -568,6 +568,12 @@ static inline void strbuf_complete_line(struct strbuf 
> *sb)
>   strbuf_complete(sb, '\n');
>  }
>  
> +/*
> + * NEEDSWORK: the following two functions should not be in this file;
> + * these are about refnames, and should be declared next to
> + * interpret_branch_name() in cache.h
> + */

Didn't touch headers.

[...]
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -161,6 +161,18 @@ test_expect_success 'check-ref-format --branch from 
> subdir' '
>   test "$refname" = "$sha1"
>  '
>  
> +test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
> + test_must_fail nongit git check-ref-format --branch @{-1}
> +'

Swapped test_must_fail and nongit to match existing tests.

Junio C Hamano (3):
  check-ref-format --branch: do not expand @{...} outside repository
  check-ref-format --branch: strip refs/heads/ using skip_prefix
  check-ref-format doc: --branch validates and expands 

 Documentation/git-check-ref-format.txt |  9 -
 builtin/check-ref-format.c |  6 --
 sha1_name.c|  5 -
 t/t1402-check-ref-format.sh| 16 
 4 files changed, 32 insertions(+), 4 deletions(-)

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Jonathan Nieder  writes:

> And in that spirit, I think the patch you replied with aims to go in
> the right direction, by providing the core functionality when in a
> repository while avoiding breaking such a script outside of one
> (though I do not understand it fully yet).

Given that, is it safe for me to ignore this earlier one

> For what it's worth, I don't agree with this repurposing of
> "check-ref-format --branch" at all.

as reacting to the patch without reading what it does?

>>(e.g. a wrapper to "git
>> clone" that wants to verify the name it is going to give to the "-b"
>> option), and a check desired in such a context is different from
>> (and is stricter than) feeding refs/heads/$name to the same command
>> without the "--branch" option.
>
> Can you say more about this example?  E.g. why is this hypothetical
> wrapper unable to rely on "git clone -b"'s own error handling?

If I have to guess what you meant, perhaps the Porcelain wanted to
diagnose bad parameter that would be rejected _before_ letting clone
spend cycles and possibly network bandwidth?

But this was my way of rephrasing an earlier example you used while
objecting to Peff's change

For example, if you have a script with an $opt_branch variable
that defaults to "master", it may do

resolved_branch=$(git check-ref-format --branch "$opt_branch")

even though it is in a mode that not going to have to use
$resolved_branch and it is not running from a repository.

so my answer to the question in the message I am directly responding
to would be "You tell me." ;-)

> --symbolic-full-name seems like a good fit.  Do you remember why
> check-ref-format was introduced instead?

It really serves two purposes, and at this point, I do not think it
really matters why it also does the @{-1} expansion as well as name
validation.  604e0cb5 ("Documentation: describe check-ref-format
--branch", 2009-10-12) happened 8 years ago, and since then it has
been advertised long enough as if the 80% primary purpose of "c-r-f
--branch" were to expand @{-1} to a branch name; even though the
text hints that it also does the usual checks, by definition
validation of the result of expanding @{-1} ought to succeed; after
all that was the branch we _were_ on ;-).

Re: [L10N] Kickoff of translation for Git 2.15.0 round 2

[Junio C Hamano]

Jiang Xin  writes:

> Git v2.15.0-rc1 released with a typo fix from commit dfab1eac23
> ("i18n: add a missing space in message", Sun Oct 8 14:18:39 2017 +0200).
> This time there are 2 updated messages need to be translated since last
> update.  Let's start new round of translation for Git 2.15.0.

Today's pushout included 3247edbb ("sequencer.c: fix and unify error
messages in rearrange_squash()", 2017-10-15), which unfortunately
added this to the mix:

error_errno(_("could not write '%s'"), todo_file);

There is another one "could not truncate '%s'" added by the same
commit, but the codebase already had another instance of the same
string, so it won't cause trouble.

Regarding the "could not write" thing, there are quite a few
instances of "coulld not write to '%s'" with %s set to the filename,
and I strongly suspect that this new one should be further updated
to match them (Ralf promoted from Cc: to To: for this).  That would
have a nice side effect of making today's last-minute merges a
non-event from translators' point of view ;-)

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Jonathan Nieder]

Hi,

Junio C Hamano wrote:
> Jeff King  writes:
>> On Sat, Oct 14, 2017 at 12:01:46PM +0900, Junio C Hamano wrote:

>>> True.  Let's see what others think.  I know Jonathan is running
>>> the fork at $work with "downgrade always to auto" patches, and while
>>> I think both approaches would probably work well in practice, I have
>>> preference for this "harder but right" approach, so I'd want to see
>>> different views discussed on the list before we decide.
>>
>> After pondering over it, I have a slight preference for that, too. But
>> I'm also happy to hear more input.
>
> OK, so it seems we both have slight preference for the "peel back"
> approach.  Adding Jonathan to Cc:

Which approach is "harder but right" / "peel back"?

I agree with the goal of making color.ui=always a synonym for auto in
file-based config.  Peff found some problems with the warning patch
(scripted commands produce too many warnings), which are not an issue
for $dayjob but may be for upstream, so I see the value of holding off
on the warning for now.

I'm also fine with "revert the proximate cause of the latest
complaints" as a stepping stone toward making color.ui=always a
synonym for auto in file-based config in a later release.

Another issue is diff-files paying attention to this configuration.
If I'm reading Documentation/config.txt correctly, that was simply a
bug.  diff-files and diff-index are never supposed to use color,
regardless of configuration.

I'm fine with "revert the proximate cause of the latest complaints" as
a stepping stone toward fixing that, too. :)

Sorry I don't have more detailed advice.  I was planning to look more
closely at how these features evolved over time and haven't had enough
time for it yet.

Jonathan

[PATCH] Added support for new configuration parameter push.pushOption

[Marius Paliga]

builtin/push.c: add push.pushOption config

Currently push options need to be given explicitly, via
the command line as "git push --push-option".

The UX of Git would be enhanced if push options could be
configured instead of given each time on the command line.

Add the config option push.pushOption, which is a multi
string option, containing push options that are sent by default.

When push options are set in the system wide config
(/etc/gitconfig), they can be unset later in the more specific
repository config by setting the string to the empty string.

Add tests and documentation as well.

Signed-off-by: Marius Paliga 
---
 Documentation/git-push.txt |  3 ++
 builtin/push.c | 12 
 t/t5545-push-options.sh| 77 ++
 3 files changed, 92 insertions(+)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..da9b17624 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -161,6 +161,9 @@ already exists on the remote side.
 Transmit the given string to the server, which passes them to
 the pre-receive as well as the post-receive hook. The given string
 must not contain a NUL or LF character.
+Default push options can also be specified with configuration
+variable `push.pushOption`. String(s) specified here will always
+be passed to the server without need to specify it using `--push-option`

 --receive-pack=::
 --exec=::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..10e520c8f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -32,6 +32,8 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;

+static struct string_list push_options_from_config = STRING_LIST_INIT_DUP;
+
 static void add_refspec(const char *ref)
 {
 refspec_nr++;
@@ -503,6 +505,14 @@ static int git_push_config(const char *k, const
char *v, void *cb)
 int val = git_config_bool(k, v) ?
 RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
 recurse_submodules = val;
+} else if (!strcmp(k, "push.pushoption")) {
+if (v == NULL)
+return config_error_nonbool(k);
+else
+if (v[0] == '\0')
+string_list_clear(_options_from_config, 0);
+else
+string_list_append(_options_from_config, v);
 }

 return git_default_config(k, v, NULL);
@@ -562,6 +572,8 @@ int cmd_push(int argc, const char **argv, const
char *prefix)
 packet_trace_identity("push");
 git_config(git_push_config, );
 argc = parse_options(argc, argv, prefix, options, push_usage, 0);
+if (!push_options.nr)
+push_options = push_options_from_config;
 set_push_cert_flags(, push_cert);

 if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL |
TRANSPORT_PUSH_MIRROR
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index 90a4b0d2f..463783789 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -140,6 +140,83 @@ test_expect_success 'push options and submodules' '
 test_cmp expect parent_upstream/.git/hooks/post-receive.push_options
 '

+test_expect_success 'default push option' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.pushOption=default push up master
+) &&
+test_refs master master &&
+echo "default" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'two default push options' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.pushOption=default1 -c push.pushOption=default2
push up master
+) &&
+test_refs master master &&
+printf "default1\ndefault2\n" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'push option from command line overrides
from-config push option' '
+mk_repo_pair &&
+git -C upstream config receive.advertisePushOptions true &&
+(
+cd workbench &&
+test_commit one &&
+git push --mirror up &&
+test_commit two &&
+git -c push.pushOption=default push --push-option=manual up master
+) &&
+test_refs master master &&
+echo "manual" >expect &&
+test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'empty value of push.pushOption in config clears
the list' '
+mk_repo_pair &&
+git -C upstream 

Benötigen Sie finanzielle Hilfe?

[Mr . Robert]

Brauchen Sie ein Darlehen, um Ihr Geschäft zu verbessern ?, Darlehen, um Ihre 
Schulden zu konsolidieren, Darlehen für den persönlichen Gebrauch, Darlehen für 
Kreditkarten, medizinische Versorgung Darlehen, Autokredit, Hypothekendarlehen, 
Studentendarlehen, Darlehen für irgendwelche Zwecke? etc. erhalten Sie einen 
jährlichen Zinssatz von 1%, beeilen Sie sich jetzt und füllen Sie das unten 
stehende Formular aus, wenn Sie Interesse haben: Kontaktieren Sie uns unter: 
iceblueinvestmentgrou...@gmail.com

ANWENDUNGSDETAILS
Vollständiger Name---
Geschlecht:___
Kontakt Adresse:
Land:__
Bundesland:__
Als Darlehen benötigter Betrag: ___
Darlehenslaufzeit: 
Beruf:___ _
Zweck des Darlehens: 
Telefon:  ___
Betrachten,

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>
>> After pondering over it, I have a slight preference for that, too. But
>> I'm also happy to hear more input.
>
> OK, so it seems we both have slight preference for the "peel back"
> approach.  Adding Jonathan to Cc:

It was a bit more painful than necessary to make sure I have
something that can be merged for 2.14.x maintenance track, but I
think the topic is now in a reasonable shape, and I've merged it to
'next'.  On the first-parent chain from 'master' to 'pu', the merge
of this topic is the very first one, and after reading it over once
again, I think this is OK.

Thanks.