Re: Request re git status

[Samuel Lijin]

On Wed, Feb 8, 2017 at 12:30 AM, Jacob Keller  wrote:
> On Tue, Feb 7, 2017 at 10:13 PM, Duy Nguyen  wrote:
>> On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller  wrote:
>>> Personally, I think that the fact that Git forces the user to think
>>> about it in terms of "oh I have to fetch" instead of that happening
>>> automatically, it helps teach the model to the user. If it happened in
>>> the background then the user might not be confronted with the
>>> distributed nature of the tool.
>>
>> I agree. But I think there is some room for improvement. Do we know
>> when the last fetch of the relevant upstream is? If we do, and if it's
>> been "a while" (configurable), then we should make a note suggesting
>> fetching again in git-status.
>>
>> This is not exactly my own idea. Gentoo's portage (i.e. friends with
>> apt-get, yum... if you're not familiar) also has this explicit "fetch"
>> operation, which is called sync. If you haven't sync'd in a while and
>> try to install new package, you get a friendly message (that helps me
>> a couple times).
>> --
>> Duy

Arch's pacman -S sync operation also has the -y flag, which updates
the local package databases, and can be used in conjunction with the
-u upgrade flag to upgrade repositories.

> That seems reasonable.
>
> Thanks,
> Jake

To be clear, I'm not advocating changing the *default* behavior of git
status; I agree that it wouldn't make sense. And although personally I
constantly update remotes manually (to the point where I abhor using
pull), I do think there's room to add an option to "fetch the
remote-tracking branch" to git status.

Re: "disabling bitmap writing, as some objects are not being packed"?

[Duy Nguyen]

On Wed, Feb 8, 2017 at 8:03 AM, David Turner  wrote:
> On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
>> And we can't grep for fatal errors anyway. The problem that led to
>> 329e6e8794 was this line
>>
>> warning: There are too many unreachable loose objects; run 'git
>> prune' to remove them.
>>
>> which is not fatal.
>
> So, speaking of that message, I noticed that our git servers were
> getting slow again and found that message in gc.log.
>
> I propose to make auto gc not write that message either. Any objections?

Does that really help? auto gc would run more often, but unreachable
loose objects are still present and potentially make your servers
slow? Should these servers run periodic and explicit gc/prune?
-- 
Duy

Re: Fwd: Possibly nicer pathspec syntax?

[Duy Nguyen]

On Wed, Feb 8, 2017 at 12:12 PM, Linus Torvalds
 wrote:
> Two-patch series to follow.

glossary-content.txt update for both patches would be nice.
-- 
Duy

Re: Request re git status

[Jacob Keller]

On Tue, Feb 7, 2017 at 10:13 PM, Duy Nguyen  wrote:
> On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller  wrote:
>> Personally, I think that the fact that Git forces the user to think
>> about it in terms of "oh I have to fetch" instead of that happening
>> automatically, it helps teach the model to the user. If it happened in
>> the background then the user might not be confronted with the
>> distributed nature of the tool.
>
> I agree. But I think there is some room for improvement. Do we know
> when the last fetch of the relevant upstream is? If we do, and if it's
> been "a while" (configurable), then we should make a note suggesting
> fetching again in git-status.
>
> This is not exactly my own idea. Gentoo's portage (i.e. friends with
> apt-get, yum... if you're not familiar) also has this explicit "fetch"
> operation, which is called sync. If you haven't sync'd in a while and
> try to install new package, you get a friendly message (that helps me
> a couple times).
> --
> Duy

That seems reasonable.

Thanks,
Jake

Re: The --name-only option for git log does not play nicely with --graph

[Duy Nguyen]

On Wed, Feb 8, 2017 at 6:11 AM, Davide Del Vento  wrote:
> `git log --graph  --name-only` works fine, but the name is not
> properly indented as it is for `git log --graph  --name-status`
>
> I tested this in git v1.9.1 the only one I have access at the moment

Confirmed still happens on master. --stat plays nicely with --graph
though, so it's probably just some small fixes somewhere in diff*.c.
-- 
Duy

Re: Request re git status

[Duy Nguyen]

On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller  wrote:
> Personally, I think that the fact that Git forces the user to think
> about it in terms of "oh I have to fetch" instead of that happening
> automatically, it helps teach the model to the user. If it happened in
> the background then the user might not be confronted with the
> distributed nature of the tool.

I agree. But I think there is some room for improvement. Do we know
when the last fetch of the relevant upstream is? If we do, and if it's
been "a while" (configurable), then we should make a note suggesting
fetching again in git-status.

This is not exactly my own idea. Gentoo's portage (i.e. friends with
apt-get, yum... if you're not familiar) also has this explicit "fetch"
operation, which is called sync. If you haven't sync'd in a while and
try to install new package, you get a friendly message (that helps me
a couple times).
-- 
Duy

Re: [PATCH] dir: avoid allocation in fill_directory()

[Duy Nguyen]

On Wed, Feb 8, 2017 at 5:04 AM, René Scharfe  wrote:
> Pass the match member of the first pathspec item directly to
> read_directory() instead of using common_prefix() to duplicate it first,
> thus avoiding memory duplication, strlen(3) and free(3).

How about killing common_prefix()? There are two other callers in
ls-files.c and commit.c and it looks safe to do (but I didn't look
very hard).

> diff --git a/dir.c b/dir.c
> index 65c3e681b8..4541f9e146 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -174,20 +174,19 @@ char *common_prefix(const struct pathspec *pathspec)
>
>  int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
>  {
> -   char *prefix;
> +   const char *prefix;
> size_t prefix_len;
>
> /*
>  * Calculate common prefix for the pathspec, and
>  * use that to optimize the directory walk
>  */
> -   prefix = common_prefix(pathspec);
> -   prefix_len = prefix ? strlen(prefix) : 0;
> +   prefix_len = common_prefix_len(pathspec);
> +   prefix = prefix_len ? pathspec->items[0].match : "";

There's a subtle difference. Before the patch, prefix[prefix_len] is
NUL. After the patch, it's not always true. If some code (incorrectly)
depends on that, this patch exposes it. I had a look inside
read_directory() though and it looks like no such code exists. So, all
good.

>
> /* Read the directory and prune it */
> read_directory(dir, prefix, prefix_len, pathspec);
>
> -   free(prefix);
> return prefix_len;
>  }
-- 
Duy

[PATCH v2] rev-list-options.txt: update --all about HEAD

[Nguyễn Thái Ngọc Duy]

This is the document patch for f0298cf1c6 (revision walker: include a
detached HEAD in --all - 2009-01-16).

Even though that commit is about detached HEAD, as Jeff pointed out,
always adding HEAD in that case may have subtle differences with
--source or --exclude. So the document mentions nothing about the
detached-ness.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 v2 drops "detached".

 Documentation/rev-list-options.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 5da7cf5a8d..a02f7324c0 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -133,8 +133,8 @@ parents) and `--max-parents=-1` (negative numbers denote no 
upper limit).
for all following revision specifiers, up to the next `--not`.
 
 --all::
-   Pretend as if all the refs in `refs/` are listed on the
-   command line as ''.
+   Pretend as if all the refs in `refs/`, along with `HEAD`, are
+   listed on the command line as ''.
 
 --branches[=]::
Pretend as if all the refs in `refs/heads` are listed
-- 
2.11.0.157.gd943d85

[PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns

[Linus Torvalds]

From: Linus Torvalds 
Date: Tue, 7 Feb 2017 21:08:15 -0800
Subject: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec 
patterns

Instead of erroring out and telling the user that they should add a 
positive pattern that covers everything else, just _do_ that.

For commands where we honor the current cwd by default (ie grep, ls-files 
etc), we make that default positive pathspec be the current working 
directory.  And for commands that default to the whole project (ie diff, 
log, etc), the default positive pathspec is the whole project.

Signed-off-by: Linus Torvalds 
---
 pathspec.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index ecad03406..d8f78088c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -522,7 +522,7 @@ void parse_pathspec(struct pathspec *pathspec,
}
 
pathspec->nr = n;
-   ALLOC_ARRAY(pathspec->items, n);
+   ALLOC_ARRAY(pathspec->items, n+1);
item = pathspec->items;
prefixlen = prefix ? strlen(prefix) : 0;
 
@@ -546,10 +546,16 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->magic |= item[i].magic;
}
 
-   if (nr_exclude == n)
-   die(_("There is nothing to exclude from by :(exclude) 
patterns.\n"
- "Perhaps you forgot to add either ':/' or '.' ?"));
-
+   /*
+* If everything is an exclude pattern, add one positive pattern
+* that matches everyting. We allocated an extra one for this.
+*/
+   if (nr_exclude == n) {
+   if (!(flags & PATHSPEC_PREFER_CWD))
+   prefixlen = 0;
+   init_pathspec_item(item + n, 0, prefix, prefixlen, "");
+   pathspec->nr++;
+   }
 
if (pathspec->magic & PATHSPEC_MAXDEPTH) {
if (flags & PATHSPEC_KEEP_ORDER)
-- 
2.12.0.rc0.1.g02555c1b2.dirty

[PATCH 1/2] pathspec magic: add '^' as alias for '!'

[Linus Torvalds]

From: Linus Torvalds 
Date: Tue, 7 Feb 2017 21:05:28 -0800
Subject: [PATCH 1/2] pathspec magic: add '^' as alias for '!'

The choice of '!' for a negative pathspec ends up not only not matching
what we do for revisions, it's also a horrible character for shell
expansion since it needs quoting.

So add '^' as an alternative alias for an excluding pathspec entry.

Signed-off-by: Linus Torvalds 
---
 pathspec.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/pathspec.c b/pathspec.c
index 7ababb315..ecad03406 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -224,6 +224,12 @@ static const char *parse_short_magic(unsigned *magic, 
const char *elem)
char ch = *pos;
int i;
 
+   /* Special case alias for '!' */
+   if (ch == '^') {
+   *magic |= PATHSPEC_EXCLUDE;
+   continue;
+   }
+
if (!is_pathspec_magic(ch))
break;
 
-- 
2.12.0.rc0.1.g02555c1b2.dirty

Re: Fwd: Possibly nicer pathspec syntax?

[Linus Torvalds]

On Tue, 7 Feb 2017, Junio C Hamano wrote:
> > +   // Special case alias for '!'
> 
> /* style? */

Will fix.

> I somehow do not think this is correct.  I expect
> 
>   cd t && git grep -e foo -- :^perf/
> 
> to look into things in 't' except for things in 't/perf', but the
> above will grab hits from ../Documentation/ etc.  We need to pay
> attention to PATHSPEC_PREFER_CWD bit in the flags word, I think.

Ok, that's easy enough.

Two-patch series to follow.

  Linus

Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano]

Linus Torvalds  writes:

> People don't expect set theory from their pathspecs. They expect their
> pathspecs to limit the output. They've learnt that within a
> subdirectory, the pathspec limits to that subdirectory. And now it
> suddenly starts showing things outside the subdirectory?
>
> At that point no amount of "but but think about set theory" will
> matter, methinks.

I do not feel too strongly about it, either, but when we invoke
"what would people who go down into subdirectories expect", the
issue becomes a lot bigger.

"git diff/log" without any pathspec in a subdirectory still shows
the whole diff.  "git grep" looks for hits inside that subdirectory,
on the other hand.

I think the former design decision is mostly a historical accident.
"The project tree as the whole is what matters" was one reason, and
another is that historically we didn't have ":/"--defaulting to the
whole tree and telling people to give "." was easier than defaulting
to the current and telling people to give "../.." after counting how
many levels deep you started at.  If we were designing the system
today with "." and ":/", I wouldn't be surprised if we chose "always
limit to cwd if there is no pathspec" for any command for the sake
of consistency.  We can easily say "if you want whole-tree, pass :/"
instead.

But we do not live in that world, and I do not think change them to
make things consistent is what we are discussing in this thread.
Given users accept and expect that "diff" without pathspec is whole
tree, while "grep" without pathspec is limited to cwd, what is the
reasonable definition of "everything from which negative ones are
excluded"?  That is the question I am trying to answer.

As Mike mentioned earlier in the thread, if we used "." (limit to
cwd) for "diff/log" when there are only negative pathspec elements,
the resulting UI would become inconsistent within a single command,
as in my world view, giving no pathspec means "work on everything
you would ordinarily work on", a positive pathspec means "among
everything you would ordinarily work on, only work on the ones that
match this pattern", and giving only a negative one ought to mean
"among everything you would ordinarily work on, only work on the
ones that do NOT match this pattern."

>  pathspec.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/pathspec.c b/pathspec.c
> index 7ababb315..2a91247bc 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -224,6 +224,12 @@ static const char *parse_short_magic(unsigned *magic, 
> const char *elem)
>   char ch = *pos;
>   int i;
>  
> + // Special case alias for '!'

/* style? */

> + if (ch == '^') {
> + *magic |= PATHSPEC_EXCLUDE;
> + continue;
> + }
> +
>   if (!is_pathspec_magic(ch))
>   break;
>  
> + /*
> +  * If everything is an exclude pattern, add one positive pattern
> +  * that matches everyting. We allocated an extra one for this.
> +  */
> + if (nr_exclude == n) {
> + init_pathspec_item(item + n, 0, "", 0, "");
> + pathspec->nr++;
> + }

I somehow do not think this is correct.  I expect

cd t && git grep -e foo -- :^perf/

to look into things in 't' except for things in 't/perf', but the
above will grab hits from ../Documentation/ etc.  We need to pay
attention to PATHSPEC_PREFER_CWD bit in the flags word, I think.

A real change probably wants to become a two-patch series (one for
the new :^ alias, the other is for "everything except...") with log
message ;-)

Re: Fwd: Possibly nicer pathspec syntax?

[Linus Torvalds]

On Tue, Feb 7, 2017 at 7:12 PM, Junio C Hamano  wrote:
>
> But that is not what I was talking about.  Let's simplify.  I'd say
> for any command that acts on "everything" when pathspec is not
> given, the two sets of actual paths affected by these two:
>
> git cmd -- "net/"
> git cmd -- ":!net/"
>
> should have no overlap (obviously) and when you take union of the
> two sets, that should equal to
>
> git cmd --
>
> i.e. no pathspecs.

Well, as mentioned, I won't ever care. I'm certainly ok with the "make
the default positive entry be everything".

I just suspect that from a user perspective that actually delves into
the subdirectories, the much bigger question will be: "I gave you a
pathspec, and suddenly you start giving me stuff from outside the area
entirely".

And then you can say "well, just add '.' to the pathspec", and you'd
be right, but I still think it's not what a naive user would expect.

People don't expect set theory from their pathspecs. They expect their
pathspecs to limit the output. They've learnt that within a
subdirectory, the pathspec limits to that subdirectory. And now it
suddenly starts showing things outside the subdirectory?

At that point no amount of "but but think about set theory" will
matter, methinks.

But I really don't feel strongly about it. The path I sent out (and
the slightly modified version attached in this email) actually acts
the way you suggest. It's certainly the simplest implementation. I
just suspect it's not the implementation people who go down into
subdirectories would want/expect.

>>>  2. I am not sure what ctype.c change is about.  Care to elaborate?
>>
>> I didn't see the need for it either until I made the rest of the
>> patch, and it didn't work at all.
>>
>> The pathspec.c code uses "if (is_pathspec_magic(..))" to test whether
>> a character is a short magiic pathspec character.  But '^' wasn't in
>> that set, because it was already marked as being (only) in the regex
>> set.
>>
>> Does that whole is_pathspec_magic() thing make any sense when we have
>> an array that specifies the special characters we react to? No it does
>> not.
>>
>> But it is what the code does, and I just made that code work.
>
> Ah, OK.

Side note: here's an alternative patch that avoids that issue
entirely, and also avoids a problem with prefix_magic() being uphappy
when one bit shows up multiple times in the array.

It's slightly hacky in parse_short_magic(), but it's smaller and
simpler, and avoids the two subtle cases. No need for ctype changes,
and no issues with prefix_magic() being somewhat stupid.

   Linus
 pathspec.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index 7ababb315..2a91247bc 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -224,6 +224,12 @@ static const char *parse_short_magic(unsigned *magic, 
const char *elem)
char ch = *pos;
int i;
 
+   // Special case alias for '!'
+   if (ch == '^') {
+   *magic |= PATHSPEC_EXCLUDE;
+   continue;
+   }
+
if (!is_pathspec_magic(ch))
break;
 
@@ -516,7 +522,7 @@ void parse_pathspec(struct pathspec *pathspec,
}
 
pathspec->nr = n;
-   ALLOC_ARRAY(pathspec->items, n);
+   ALLOC_ARRAY(pathspec->items, n+1);
item = pathspec->items;
prefixlen = prefix ? strlen(prefix) : 0;
 
@@ -540,10 +546,14 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->magic |= item[i].magic;
}
 
-   if (nr_exclude == n)
-   die(_("There is nothing to exclude from by :(exclude) 
patterns.\n"
- "Perhaps you forgot to add either ':/' or '.' ?"));
-
+   /*
+* If everything is an exclude pattern, add one positive pattern
+* that matches everyting. We allocated an extra one for this.
+*/
+   if (nr_exclude == n) {
+   init_pathspec_item(item + n, 0, "", 0, "");
+   pathspec->nr++;
+   }
 
if (pathspec->magic & PATHSPEC_MAXDEPTH) {
if (flags & PATHSPEC_KEEP_ORDER)

Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano]

Linus Torvalds  writes:

> No. The thing is, "git diff" is relative too - for path
> specifications. And the negative entries are pathspecs - and they act
> as relative ones.
>
> IOW, that whole
>
>   cd drivers
>   git diff A..B -- net/
>
> will actually show the diff for drivers/net - so the pathspec very
> much acts as relative to the cwd.

But that is not what I was talking about.  Let's simplify.  I'd say
for any command that acts on "everything" when pathspec is not
given, the two sets of actual paths affected by these two:

git cmd -- "net/"
git cmd -- ":!net/"

should have no overlap (obviously) and when you take union of the
two sets, that should equal to

git cmd --

i.e. no pathspecs.

>>  2. I am not sure what ctype.c change is about.  Care to elaborate?
>
> I didn't see the need for it either until I made the rest of the
> patch, and it didn't work at all.
>
> The pathspec.c code uses "if (is_pathspec_magic(..))" to test whether
> a character is a short magiic pathspec character.  But '^' wasn't in
> that set, because it was already marked as being (only) in the regex
> set.
>
> Does that whole is_pathspec_magic() thing make any sense when we have
> an array that specifies the special characters we react to? No it does
> not.
>
> But it is what the code does, and I just made that code work.

Ah, OK.

Re: Fwd: Possibly nicer pathspec syntax?

[Mike Hommey]

On Tue, Feb 07, 2017 at 05:48:26PM -0800, Linus Torvalds wrote:
> 
> 
> On Tue, 7 Feb 2017, Linus Torvalds wrote:
> > 
> > [ Clarification from original message, since Junio asked: I didn't
> >   actually want the semantics of '.' at all, since in a subdirectory it
> >   limits to the current subdirectory. So I'd suggest that in the absence
> >   of any positive pattern, there is simply no filtering at all, so
> >   whenever I say '.' as a pattern, I really meant ":(top)." which is
> >   even more of a cumbersom syntax that the current model really
> >   encourages. Crazy. Since I tend to always work in the top directory,
> >   the two are the same for me ]
> 
> So here's an RFC patch, and I'm quoting the above part of my thinking 
> because it's what the patch does, but it turns out that it's probably not 
> what we want, and I suspect the "." behavior (as opposed to "no filtering 
> at all") is actually better.
> 
> Now _I_ don't much care, since I only work from the top level, but without 
> the "." behavior, you get into an odd situation that the negative match 
> will be relative to the current directory, but then the positive matches 
> will be everywhere else. 
> 
> Obviously, a negative match that has "top" set would change that logic. So 
> this patch is purely a request for further discussion.
> 
> When I wrote the patch, I actually also removed the now stale entries from 
> the 'po' files, but I'm not including that part here because it just 
> distracts from the meat of it all. So this diff was actually generated 
> with the new syntax:
> 
>   git diff -p --stat -- :^po/
> 
> and the only thing even remotely subtle here is that it changes our ctype 
> array to make '^' be both a regex and a pathspec magic character.
> 
> Everything else should be pretty darn obvious.
> 
> The code *could* just track all the 'relative to top or not' bits in the 
> exclusion pattern, and then use whatever top-ness the exclusion patterns 
> have (and maybe fall back to the old warning if it had a mixture of 
> exclusionary patterns). I'll happily change it to act that way if people 
> think that makes sense.
> 
> Comments?

It seems to me that `git diff` and `git diff -- :^stuff` should have the
same output if there aren't changes in stuff, and `git diff` does the
same as `git diff -- :/` if you are in a subdirectory, not the same as
`git diff .`.

As such, the default positive match should be ':/' (which is shorter and
less cumbersome than ':(top)', btw)

Mike

Re: Fwd: Possibly nicer pathspec syntax?

[Mike Hommey]

On Tue, Feb 07, 2017 at 06:49:24PM -0800, Linus Torvalds wrote:
> On Tue, Feb 7, 2017 at 6:40 PM, Mike Hommey  wrote:
> >
> > As such, the default positive match should be ':/' (which is shorter and
> > less cumbersome than ':(top)', btw)
> 
> So that's what my patch does.
> 
> However, it's actually very counter-intuitive in a subdirectory.
> 
> Git doesn't do much of that, but let me give you an example from the
> kernel. Again, this is not an example of anything I would do (because
> I'm always at the top), but:
> 
>   [torvalds@i7 linux]$ cd drivers/
>   [torvalds@i7 drivers]$ ll
> 
>   .. whee, *lots* of diorectories ..
>   .. lets see what happened in net/ ..
> 
>   [torvalds@i7 drivers]$ git diff -M --dirstat=1,cumulative
> v4.10-rc6..v4.10-rc7 -- net/
>  7.4% drivers/net/ethernet/adaptec/
> 47.9% drivers/net/ethernet/cadence/
>  7.1% drivers/net/ethernet/emulex/benet/
>  1.1% drivers/net/ethernet/freescale/
>  3.6% drivers/net/ethernet/mellanox/mlx4/
> 23.5% drivers/net/ethernet/mellanox/mlx5/core/
> 27.2% drivers/net/ethernet/mellanox/
> 92.5% drivers/net/ethernet/
>  5.3% drivers/net/wireless/intel/iwlwifi/mvm/
>  5.9% drivers/net/wireless/intel/iwlwifi/
>100.0% drivers/net/
> 
>   .. let's see what happened *outside* of net/ ..
> 
> [torvalds@i7 drivers]$ git diff -M --dirstat=1,cumulative
> v4.10-rc6..v4.10-rc7 -- :^net/
>2.4% arch/arm64/crypto/
>2.1% arch/powerpc/include/asm/
>1.5% arch/powerpc/kernel/
>3.9% arch/powerpc/
>3.5% arch/sparc/kernel/
>4.1% arch/sparc/
>8.3% arch/x86/events/intel/
>1.7% arch/x86/kernel/cpu/mcheck/
>1.6% arch/x86/kernel/cpu/microcode/
>3.3% arch/x86/kernel/cpu/
>3.8% arch/x86/kernel/
>1.0% arch/x86/platform/efi/
>   13.3% arch/x86/
>   24.0% arch/
>1.1% drivers/base/
>2.9% drivers/dma/
>   12.3% drivers/gpu/drm/i915/
>1.0% drivers/gpu/drm/nouveau/
>   16.2% drivers/gpu/drm/
>3.9% drivers/hid/
>1.6% drivers/iio/
>2.3% drivers/regulator/
>...
> 
> Notice? When you say "show only the net subdirectory" it does the
> obvious thing relative to the current working directory, but if you
> say "show everything _but_ the net subdirectory" it suddenly starts
> showing other things.

I can totally see how this can be confusing, but the case can be made
that the problem is that `git diff` would be showing everything if you
didn't pass an exclusion list. Now, what you're suggesting introduces
some inconsistency, which, in itself, can cause confusion.

I'm not sure which confusion is worse.

Mike

Re: Fwd: Possibly nicer pathspec syntax?

[Linus Torvalds]

On Tue, Feb 7, 2017 at 6:42 PM, Junio C Hamano  wrote:
>
>  1. I think some commands limit their operands to cwd and some work
> on the whole tree when given no pathspec. I think the "no
> positive?  then let's give you everything except these you
> excluded" should base the definition of "everything" to that.
> IOW, "cd t && git grep -e foo" shows everything in t/ directory,
> so the default you would add would be "." for "grep"; "cd t &&
> git diff HEAD~100 HEAD" would show everything, so you would give
> ":(top)." for "diff".

No. The thing is, "git diff" is relative too - for path
specifications. And the negative entries are pathspecs - and they act
as relative ones.

IOW, that whole

  cd drivers
  git diff A..B -- net/

will actually show the diff for drivers/net - so the pathspec very
much acts as relative to the cwd.

So no, absolute (ie ":(top)" or ":/") doesn't actually make sense for
"diff" either, even though diff by default is absolute when not given
a pathname at all.

But if you do

  cd drivers
  git diff A..B -- :^/arch

then suddenly an absolute positive root _does_ make sense,. because
now the negative pathspec was absolute..

Odd? Yes it is. But the positive pathspec rules are what they are, and
they are actually what I suspect everybody really wants. The existing
negative ones match the rules for the positive ones.

So I suspect that the best thing is if the "implicit positive rule
when there are no explicit ones" ends up matching the same semantics
as the (explicit) negative entries have..

>  2. I am not sure what ctype.c change is about.  Care to elaborate?

I didn't see the need for it either until I made the rest of the
patch, and it didn't work at all.

The pathspec.c code uses "if (is_pathspec_magic(..))" to test whether
a character is a short magiic pathspec character.  But '^' wasn't in
that set, because it was already marked as being (only) in the regex
set.

Does that whole is_pathspec_magic() thing make any sense when we have
an array that specifies the special characters we react to? No it does
not.

But it is what the code does, and I just made that code work.

>  3. I think our recent trend is to wean ourselves away from "an
> empty element in pathspec means all paths match", and I think we
> even have accepted a patch to emit a warning.  Doesn't the
> warning trigger for the new code below?

It didn't trigger for me in my testing, I suspect the warning is at an
earlier level when it walks through the argv[] array and fills in the
pathspec arrays. But I didn't actually look for it.

   Linus

Re: Fwd: Possibly nicer pathspec syntax?

[Linus Torvalds]

On Tue, Feb 7, 2017 at 6:40 PM, Mike Hommey  wrote:
>
> As such, the default positive match should be ':/' (which is shorter and
> less cumbersome than ':(top)', btw)

So that's what my patch does.

However, it's actually very counter-intuitive in a subdirectory.

Git doesn't do much of that, but let me give you an example from the
kernel. Again, this is not an example of anything I would do (because
I'm always at the top), but:

  [torvalds@i7 linux]$ cd drivers/
  [torvalds@i7 drivers]$ ll

  .. whee, *lots* of diorectories ..
  .. lets see what happened in net/ ..

  [torvalds@i7 drivers]$ git diff -M --dirstat=1,cumulative
v4.10-rc6..v4.10-rc7 -- net/
 7.4% drivers/net/ethernet/adaptec/
47.9% drivers/net/ethernet/cadence/
 7.1% drivers/net/ethernet/emulex/benet/
 1.1% drivers/net/ethernet/freescale/
 3.6% drivers/net/ethernet/mellanox/mlx4/
23.5% drivers/net/ethernet/mellanox/mlx5/core/
27.2% drivers/net/ethernet/mellanox/
92.5% drivers/net/ethernet/
 5.3% drivers/net/wireless/intel/iwlwifi/mvm/
 5.9% drivers/net/wireless/intel/iwlwifi/
   100.0% drivers/net/

  .. let's see what happened *outside* of net/ ..

[torvalds@i7 drivers]$ git diff -M --dirstat=1,cumulative
v4.10-rc6..v4.10-rc7 -- :^net/
   2.4% arch/arm64/crypto/
   2.1% arch/powerpc/include/asm/
   1.5% arch/powerpc/kernel/
   3.9% arch/powerpc/
   3.5% arch/sparc/kernel/
   4.1% arch/sparc/
   8.3% arch/x86/events/intel/
   1.7% arch/x86/kernel/cpu/mcheck/
   1.6% arch/x86/kernel/cpu/microcode/
   3.3% arch/x86/kernel/cpu/
   3.8% arch/x86/kernel/
   1.0% arch/x86/platform/efi/
  13.3% arch/x86/
  24.0% arch/
   1.1% drivers/base/
   2.9% drivers/dma/
  12.3% drivers/gpu/drm/i915/
   1.0% drivers/gpu/drm/nouveau/
  16.2% drivers/gpu/drm/
   3.9% drivers/hid/
   1.6% drivers/iio/
   2.3% drivers/regulator/
   ...

Notice? When you say "show only the net subdirectory" it does the
obvious thing relative to the current working directory, but if you
say "show everything _but_ the net subdirectory" it suddenly starts
showing other things.

Now, it would be easy enough to say "if you don't give a positive
path, we'll just use the empty path that matches the negative paths".
So if you ask for a negative relative "net" directory, we'll use the
relative empty path. And if you ask for a negative absolute path,
we'll use the empty absolute path.

It's a couple of lines more, and I think it might avoid some confusion.

And I suspect almost nobody has ever done any of this before,. because
the syntax was/is so cumbersome.

Linus

Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano]

Linus Torvalds  writes:

> So here's an RFC patch, and I'm quoting the above part of my thinking 
> because it's what the patch does, but it turns out that it's probably not 
> what we want, and I suspect the "." behavior (as opposed to "no filtering 
> at all") is actually better.
> ...
>
> Comments?

 1. I think some commands limit their operands to cwd and some work
on the whole tree when given no pathspec.  I think the "no
positive?  then let's give you everything except these you
excluded" should base the definition of "everything" to that.
IOW, "cd t && git grep -e foo" shows everything in t/ directory,
so the default you would add would be "." for "grep"; "cd t &&
git diff HEAD~100 HEAD" would show everything, so you would give
":(top)." for "diff".

 2. I am not sure what ctype.c change is about.  Care to elaborate?

 3. I think our recent trend is to wean ourselves away from "an
empty element in pathspec means all paths match", and I think we
even have accepted a patch to emit a warning.  Doesn't the
warning trigger for the new code below?

> - if (nr_exclude == n)
> - die(_("There is nothing to exclude from by :(exclude) 
> patterns.\n"
> -   "Perhaps you forgot to add either ':/' or '.' ?"));
> -
> + /*
> +  * If everything is an exclude pattern, add one positive pattern
> +  * that matches everyting. We allocated an extra one for this.
> +  */
> + if (nr_exclude == n) {
> + init_pathspec_item(item + n, 0, "", 0, "");
> + pathspec->nr++;
> + }
>  
>   if (pathspec->magic & PATHSPEC_MAXDEPTH) {
>   if (flags & PATHSPEC_KEEP_ORDER)

RE: [RFC] Add support for downloading blobs on demand

[Ben Peart]

Thanks Jakub.  

Just so you are aware, this isn't a separate effort, it actually is the same 
effort as the GVFS effort from Microsoft.  For pragmatic reasons, we 
implemented the lazy clone support and on demand object downloading in our own 
codebase (GVFS) first and are now are working to move it into git natively so 
that it will be available everywhere git is available.  This RFC is just one 
step in that process.

As we mentioned at Git Merge, we looked into Mercurial but settled on Git as 
our version control solution.  We are, however, in active communication with 
the team from Facebook to share ideas.

Ben

> -Original Message-
> From: Jakub Narębski [mailto:jna...@gmail.com]
> Sent: Tuesday, February 7, 2017 4:57 PM
> To: Ben Peart ; 'Christian Couder'
> 
> Cc: 'Jeff King' ; 'git' ; 'Johannes
> Schindelin' ; Ben Peart
> 
> Subject: Re: [RFC] Add support for downloading blobs on demand
> 
> I'd like to point to two (or rather one and a half) solutions that I got 
> aware of
> when watching streaming of "Git Merge 2017"[0].  There should be here
> people who were there; and hopefully video of those presentations and
> slides / notes would be soon available.
> 
> [0]: http://git-merge.com/
> 
> First tool that I'd like to point to is Git Virtual File System, or GVFS in 
> short
> (which unfortunately shares abbreviation with GNOME Virtual File System).
> 
> The presentation was "Scaling Git at Microsoft" by Saeed Noursalehi,
> Microsoft.  You can read about this solution in ArsTechnica article[1], and on
> Microsoft blog[2].  The code (or early version of thereof) is also 
> available[3] -
> I wonder why on GitHub and not Codeplex...
> 
> [1]: https://arstechnica.com/information-technology/2017/02/microsoft-
> hosts-the-windows-source-in-a-monstrous-300gb-git-repository/
> [2]:
> https://blogs.msdn.microsoft.com/visualstudioalm/2017/02/03/announcing-
> gvfs-git-virtual-file-system/
> [3]: https://github.com/Microsoft/GVFS
> 
> 
> The second presentation that might be of some interest is "Scaling Mercurial
> at Facebook: Insights from the Other Side" by Durham Goode, Facebook.
> The code is supposedly available as open-source; though I don't know how
> useful their 'blob storage' solution would be of use for your problem.
> 
> 
> HTH
> --
> Jakub Narębski

Re: Fwd: Possibly nicer pathspec syntax?

[Linus Torvalds]

On Tue, 7 Feb 2017, Linus Torvalds wrote:
> 
> [ Clarification from original message, since Junio asked: I didn't
>   actually want the semantics of '.' at all, since in a subdirectory it
>   limits to the current subdirectory. So I'd suggest that in the absence
>   of any positive pattern, there is simply no filtering at all, so
>   whenever I say '.' as a pattern, I really meant ":(top)." which is
>   even more of a cumbersom syntax that the current model really
>   encourages. Crazy. Since I tend to always work in the top directory,
>   the two are the same for me ]

So here's an RFC patch, and I'm quoting the above part of my thinking 
because it's what the patch does, but it turns out that it's probably not 
what we want, and I suspect the "." behavior (as opposed to "no filtering 
at all") is actually better.

Now _I_ don't much care, since I only work from the top level, but without 
the "." behavior, you get into an odd situation that the negative match 
will be relative to the current directory, but then the positive matches 
will be everywhere else. 

Obviously, a negative match that has "top" set would change that logic. So 
this patch is purely a request for further discussion.

When I wrote the patch, I actually also removed the now stale entries from 
the 'po' files, but I'm not including that part here because it just 
distracts from the meat of it all. So this diff was actually generated 
with the new syntax:

git diff -p --stat -- :^po/

and the only thing even remotely subtle here is that it changes our ctype 
array to make '^' be both a regex and a pathspec magic character.

Everything else should be pretty darn obvious.

The code *could* just track all the 'relative to top or not' bits in the 
exclusion pattern, and then use whatever top-ness the exclusion patterns 
have (and maybe fall back to the old warning if it had a mixture of 
exclusionary patterns). I'll happily change it to act that way if people 
think that makes sense.

Comments?

Linus

---
 ctype.c|  3 ++-
 pathspec.c | 15 ++-
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ctype.c b/ctype.c
index fc0225ceb..250e2ce15 100644
--- a/ctype.c
+++ b/ctype.c
@@ -14,6 +14,7 @@ enum {
P = GIT_PATHSPEC_MAGIC, /* other non-alnum, except for ] and } */
X = GIT_CNTRL,
U = GIT_PUNCT,
+   Y = GIT_REGEX_SPECIAL | GIT_PATHSPEC_MAGIC,
Z = GIT_CNTRL | GIT_SPACE
 };
 
@@ -23,7 +24,7 @@ const unsigned char sane_ctype[256] = {
S, P, P, P, R, P, P, P, R, R, G, R, P, P, R, P, /*  32.. 47 */
D, D, D, D, D, D, D, D, D, D, P, P, P, P, P, G, /*  48.. 63 */
P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /*  64.. 79 */
-   A, A, A, A, A, A, A, A, A, A, A, G, G, U, R, P, /*  80.. 95 */
+   A, A, A, A, A, A, A, A, A, A, A, G, G, U, Y, P, /*  80.. 95 */
P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /*  96..111 */
A, A, A, A, A, A, A, A, A, A, A, R, R, U, P, X, /* 112..127 */
/* Nothing in the 128.. range */
diff --git a/pathspec.c b/pathspec.c
index 7ababb315..ef59d080d 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -72,6 +72,7 @@ static struct pathspec_magic {
{ PATHSPEC_GLOB,'\0', "glob" },
{ PATHSPEC_ICASE,   '\0', "icase" },
{ PATHSPEC_EXCLUDE,  '!', "exclude" },
+   { PATHSPEC_EXCLUDE,  '^', "exclude" },
 };
 
 static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
@@ -516,7 +517,7 @@ void parse_pathspec(struct pathspec *pathspec,
}
 
pathspec->nr = n;
-   ALLOC_ARRAY(pathspec->items, n);
+   ALLOC_ARRAY(pathspec->items, n+1);
item = pathspec->items;
prefixlen = prefix ? strlen(prefix) : 0;
 
@@ -540,10 +541,14 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->magic |= item[i].magic;
}
 
-   if (nr_exclude == n)
-   die(_("There is nothing to exclude from by :(exclude) 
patterns.\n"
- "Perhaps you forgot to add either ':/' or '.' ?"));
-
+   /*
+* If everything is an exclude pattern, add one positive pattern
+* that matches everyting. We allocated an extra one for this.
+*/
+   if (nr_exclude == n) {
+   init_pathspec_item(item + n, 0, "", 0, "");
+   pathspec->nr++;
+   }
 
if (pathspec->magic & PATHSPEC_MAXDEPTH) {
if (flags & PATHSPEC_KEEP_ORDER)

[PATCH] push options: fail properly in the stateless case

[Stefan Beller]

When using non-builtin protocols relying on a transport helper
(such as http), push options are not propagated to the helper.

Fix this by propagating the push options to the transport helper and
adding a test that push options using http fail properly.

Signed-off-by: Stefan Beller 
---
 Documentation/gitremote-helpers.txt |  3 +++
 t/t5545-push-options.sh | 15 +++
 transport-helper.c  |  7 +++
 3 files changed, 25 insertions(+)

diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index 9e8681f9e1..6145d4d8df 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -462,6 +462,9 @@ set by Git if the remote helper has the 'option' capability.
 'option pushcert {'true'|'false'}::
GPG sign pushes.
 
+'option push-option ::
+   Transmit this push option.
+
 SEE ALSO
 
 linkgit:git-remote[1]
diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index ea813b9383..9a57a7d8f2 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh
@@ -3,6 +3,8 @@
 test_description='pushing to a repository using push options'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
 
 mk_repo_pair () {
rm -rf workbench upstream &&
@@ -100,4 +102,17 @@ test_expect_success 'two push options work' '
test_cmp expect upstream/.git/hooks/post-receive.push_options
 '
 
+test_expect_success 'push option denied properly by http remote helper' '\
+   mk_repo_pair &&
+   git -C upstream config receive.advertisePushOptions false &&
+   git -C upstream config http.receivepack true &&
+   cp -R upstream/.git "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git &&
+   git clone "$HTTPD_URL"/smart/upstream test_http_clone &&
+   test_commit -C test_http_clone one &&
+   test_must_fail git -C test_http_clone push --push-option=asdf origin 
master &&
+   git -C test_http_clone push origin master
+'
+
+stop_httpd
+
 test_done
diff --git a/transport-helper.c b/transport-helper.c
index 91aed35ebb..1258d6aedd 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -826,6 +826,13 @@ static void set_common_push_options(struct transport 
*transport,
if (set_helper_option(transport, TRANS_OPT_PUSH_CERT, 
"if-asked") != 0)
die("helper %s does not support --signed=if-asked", 
name);
}
+
+   if (flags & TRANSPORT_PUSH_OPTIONS) {
+   struct string_list_item *item;
+   for_each_string_list_item(item, transport->push_options)
+   if (set_helper_option(transport, "push-option", 
item->string) != 0)
+   die("helper %s does not support 'push-option'", 
name);
+   }
 }
 
 static int push_refs_with_push(struct transport *transport,
-- 
2.12.0.rc0.1.g018cb5e6f4

Re: "disabling bitmap writing, as some objects are not being packed"?

[David Turner]

On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
> And we can't grep for fatal errors anyway. The problem that led to
> 329e6e8794 was this line
> 
> warning: There are too many unreachable loose objects; run 'git
> prune' to remove them.
> 
> which is not fatal.

So, speaking of that message, I noticed that our git servers were
getting slow again and found that message in gc.log.

I propose to make auto gc not write that message either. Any objections?

Re: Fwd: Possibly nicer pathspec syntax?

[Junio C Hamano]

Linus Torvalds  writes:

> [ Duh, I sent this just to Junio initially due to a brainfart. Here
> goes the list also ]

And my earlier response goes to the list ;-)

Linus Torvalds  writes:

> Most of the time when I use pathspecs, I just use the bog-standard
> normal ones, and everything works wonderfully.

It is, I think, a no-brainer to lift the "you must have at least one
positive".  If the user says "not this and that", it is reasonable
to assume that "but include everything else" is implied.

As to "!" that triggers history substitution without quoting, it may
be annoying and I think it is probably OK to pick a synonym letter,
perhaps "^", now that the set of pathspec magics do not seem to be
growing rapidly and there may not be any other existing magic that
the natural meaning of "^" would match better than "negate".  The
primary reason why we used ! is, I think, to match patterns in the
exclude files.

As to the leading ":", that is shared between the ":(long form)" and
the short form, I am a bit hesitant to lose it.  It allows the users
to be trained only once, i.e. "if you want to match a path without
magic in your working tree, you need to watch out for an unusual
path that begins with a colon, which may be quite minority to begin
with.  You just prefix ./ in front to defeat it.  Everything else
you can type as-is, modulo wildcard metacharacters, but you know
that already." and their brains need no upgrading.  Once we start
accepting short forms without the ":", every time we add a short
form magic, the users need to be retrained.

In short, this

> Or even just allowing ^ in addition to ! for negation, and otherwise
> keeping the current syntax.

in addition to "no positives?  let's pretend you also said '.' as a
positive", would not be too bad, methinks.  And that allows this

> git diff -M --dirstat .. -- ':!drivers' ':!arch' .

to become

git diff -M --dirstat .. -- :^{drivers,arch}

which is a bit shorter.  I personally am perfectly fine without ^, i.e.

git diff -M --dirstat .. -- :\!{drivers,arch}

though.

By the way, I am wondering why this is private, not cc'ed to the
mailing list.  As messages addressed to gitster@ without git@vger
bypass my Inbox and gets thrown into spam box, which I only
occasionally scan to resurrect messages worth responding, and this
is one of those cases ;-)

Re: git/git-scm.com GH Issue Tracker

[Samuel Lijin]

Finished going through and nailed the rest of the open issues!


# Irrelevant but it seems like someone should take a look

511 466


# Irrelevant to git-scm.com and should be closed

599 596 570 565 563 558 538 537 520 511 509 507 501 494 465


# Resolved, duplicate, or non-issue

596 593 592 588 587 585 583 576 575 573 572 547 546 543 540 539 529 521
516 515 504 503 502 496 491 490 476 473 470 467 463 460 456 454 451 413
377 265 257 95



# Relevant and should be kept open

597 595 591 586 578 544 532 518 513 512 500 493 466 448 416 410 381 379
140 13 12 11


That's all of them!

- Sam

On Mon, Feb 6, 2017 at 12:34 PM, Jeff King  wrote:
> On Mon, Feb 06, 2017 at 12:15:08AM -0600, Samuel Lijin wrote:
>
>> I've went through a bunch of open issues on the git/git-scm.com repo
>> (specifically, everything after #600) and I think the bulk of them can
>> be closed.
>>
>> I've taken the liberty of classifying them as shown below.
>
> Thanks, this is incredibly helpful. I'll close the appropriate ones you
> identified.
>
> -Peff

Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)

[Jonathan Tan]

Looking back at the comments I have received in reply, I think that
there were two major concerns: (i) the case where a server ACKs a client
"have" line and the client forever thinks that the server has it, but it
may not be the case for future servers (or future invocations of the
same server), and (ii) what the client does with 2 "versions" of remote
refs.

For (i), the issue already exists and as far as I can tell, this patch
set does not directly impact it, positively or negatively. The
"have"/"ACK" part of negotiation is kept the same - the only difference
in this patch set is that wants can be specified by name instead of by
SHA-1 hash. This patch set does not help the "have"/"ACK" part of
negotiation, but it helps the "want" part.

For (ii), I have prepared a patch to be squashed, and extended the
commit message with an explanation of what is happening. (The commit
message and the patch are appended to this e-mail).

(There was also some discussion of the client being required to send
exact matches in its "want-ref" lines.)

Please let me know if you have any other opinions or thoughts. It does
seem to me like such a protocol update (or something similar) would help
for large repositories with many ever-changing refs (like refs/changes
in Gerrit or refs/pull in GitHub) - and the creation of a ref would look
like a deletion depending on the order in which we access the servers in
a load-balancing arrangement and the order in which those servers
synchronize themselves. And deletion of refs does not work with the
current protocol, but would work with a protocol that supports wildcards
(like this one).

-- 8< --

fetch: send want-ref and receive fetched refs

Teach fetch to send refspecs to the underlying transport, and teach all
components used by the HTTP transport (remote-curl, transport-helper,
fetch-pack) to understand and propagate the names and SHA-1s of the refs
fetched.

The do_fetch method in builtin/fetch.c originally had only one
remote-local ref map, generated from the already-fetched list of remote
refs. This patch introduces a new ref map generated from the list of
fetched refs. Usages of the list of remote refs or the remote-local ref
map are updated as follows:
 - check_not_current_branch (which checks that the current branch is not
   affected by the fetch) is performed both on the original ref map (to
   die before the fetch if we can, as an optimization) and on the new
   ref map (since the new ref map is the one actually applied).
 - Pruning is done based on the new ref map.
 - backfill_tags (for tag following) is performed using the original
   list of remote refs because the new list of fetched refs is not
   guaranteed to contain tag information. (Since backfill_tags performs
   another fetch, it does not need to be fully consistent with the
   just-returned packfile.)
The list of remote refs and the remote-local ref map are not otherwise
used by do_fetch or any function in its invocation chain (fetch_one and
cmd_fetch).
---
 builtin/fetch.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 87de00e49..b8432394c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1177,6 +1177,20 @@ static int do_fetch(struct transport *transport,
 
if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
+   if (fetch_refs(transport, e_rs, e_rs_nr, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (new_remote_refs) {
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, new_remote_refs,
+ refs, ref_count, tags, autotags);
+   if (!update_head_ok)
+   check_not_current_branch(ref_map);
+   free_refs(new_remote_refs);
+   }
+
if (prune) {
/*
 * We only prune based on refspecs specified
@@ -1192,18 +1206,6 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, e_rs, e_rs_nr, ref_map, _remote_refs)) {
-   free_refs(ref_map);
-   retcode = 1;
-   goto cleanup;
-   }
-   if (new_remote_refs) {
-   free_refs(ref_map);
-   ref_map = get_ref_map(transport->remote, new_remote_refs,
- refs, ref_count, tags, autotags);
-   free_refs(new_remote_refs);
-   }
-
if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
-- 
2.11.0.483.g087da7b7c-goog

Re: Trying to use xfuncname without success.

[René Scharfe]

Am 07.02.2017 um 20:21 schrieb Jack Adrian Zappa:

I'm trying to setup a hunk header for .natvis files. For some reason,
it doesn't seem to be working. I'm following their instructions from
here, which doesn't say much in terms of restrictions of the regex,
such as, is the matched item considered the hunk header or do I need a
group? I have tried both with no success. This is what I have:

[diff "natvis"]
xfuncname = "^[\\\t ]*

Fwd: Possibly nicer pathspec syntax?

[Linus Torvalds]

[ Duh, I sent this just to Junio initially due to a brainfart. Here
goes the list also ]

Most of the time when I use pathspecs, I just use the bog-standard
normal ones, and everything works wonderfully.

But then occasionally I want to exclude a directory (usually
"drivers/"), and it all works fine, but the syntax for that is just
really cumbersome.

That's due to two issues:

 - the magic characters seem to have been chosen to be annoying on purpose

 - there's an extra "you can't exclude things without having positive
patterns" check

and both of those are rather nasty.

So to explain where I come from, during releases I do things like this:

git diff -M --dirstat=2,cumulative v4.10-rc6..v4.10-rc7

and this is literaly why that "dirstat" diff exists - I find this very
useful for a project like the kernel that has a good hierarchical
directory structure. So the whole dirstat option came about from my
statistics gathering (see more explanations in my original commit
7df7c019c ("Add "--dirstat" for some directory statistics").

However, what often happens for the kernel is that a few big
subsystems end up dominating the discussion (usually drivers and
architecture), and then you want to drill down into everything else to
get that part. Long ago, that used to be painful, and I did things
like

git diff -M --dirstat ... -- $(ls | egrep -v '(drivers)|(arch)')

which works, and gives me the dirstat for stuff that isn't arch or
driver updates.

However, git actually added exclude patterns, and I don't need to do
that crazy thing with shell expansion any more. Now I can do this
crazy thing with git natively instead:

git diff -M --dirstat .. -- ':!drivers' ':!arch' .

but honestly, the git native interface really isn't much simpler than
what I used to do.

Is there really any reason for requiring the '.'?

[ Clarification from original message, since Junio asked: I didn't
actually want the semantics of '.' at all, since in a subdirectory it
limits to the current subdirectory. So I'd suggest that in the absense
of any positive pattern, there is simply no filtering at all, so
whenever I say '.' as a pattern, I really meant ":(top)." which is
even more of a cumbersom syntax that the current model really
encourages. Crazy. Since I tend to always work in the top directory,
the two are the same for me ]

And did we really have to pick such annoying characters that we need
the shell escaping?

(I never use the other ones with long forms, but they have the same
issue: parenthesis need escaping too, so you have to write them as

':(exclude,icase)drivers'

and you have to remember that a final colon is *not* allowed, and they
still need the escaping).

It really isn't all that wonderful to use from the command line.

In revisions, we use "^" for negation, partly exactly because '!' is
such a nasty character for shell users. With exclusion being the only
case I particularly use, I'd like that for pathspecs too, but maybe
others use icase etc?

IOW, what I'd like to do is just

git diff -M --dirstat .. ^drivers ^arch

without needing the ugly quoting, and without needing the pointless
positive 'match everything else' marker.

Or even just allowing ^ in addition to ! for negation, and otherwise
keeping the current syntax.

[ Clarification from original message, since Junio asked: yes, this
suggestion still assumes the "don't need to specify the positive
pattern", so you could just do

git diff :^drivers

  to avoid the drivers directory ]

Comments? Other ideas?

This is certainly not a high priority, but I hit it once again when
doing the 4.10-rc7 statistics, which is why I bring up the
discussion..

 Linus

The --name-only option for git log does not play nicely with --graph

[Davide Del Vento]

`git log --graph  --name-only` works fine, but the name is not
properly indented as it is for `git log --graph  --name-status`

I tested this in git v1.9.1 the only one I have access at the moment

Regards,
Davide Del Vento,
NCAR Computational & Information Services Laboratory

Re: git-daemon shallow checkout fail

[Bob Proulx]

Duy Nguyen wrote:
> I wonder if we should make this "git/1.9.1" information more visible. We could
> 
> 1) Always print it when cloning
> 2) Print it when cloning with -v (printing all capabilities with -v
> might not be a bad idea)
> 3) Include it in error messages when clone/fetch fails

I don't think I would want to see it all of the time.  It isn't needed
all of the time.  But having it printable upon demand is nice.  Being
able to use GIT_TRACE_PACKET=1 worked very well.  The only thing I
needed was to know it was available so that I could use it.  I am not
sure where that is documented.

Therefore if and only if a change was made I would vote for printing
only under --verbose or other explicit other information option.  I
would not add it to the normal operation output.

Bob

Re: [RFC] mailmap.blob overrides default .mailmap

[Cornelius Weig]

On 02/07/2017 08:28 PM, Jeff King wrote:
> 
> I think it was mostly that I had to define _some_ order. This made sense
> to me as similar to things like attributes or excludes, where we prefer
> clone-specific data over in-history data (so .git/info/attributes takes
> precedence over .gitattributes).
> 
> So any mailmap.* would take precedence over the in-tree .mailmap file.
> And then between mailmap.file and mailmap.blob, the "blob" form is
> more "in-tree" than the "file" form (especially because we turn it on by
> default in bare repos, so it really is identical to the in-tree form
> there).

So the clone-specific data wins over in-history makes perfect sense to me. 
Therefore, mailmap.file should win over mailmap.blob, agreed.

On the other hand, a checked-in .mailmap file and a mailmap-blob are both as 
in-history as the other to me. Now consider the following settings:

$ git config --unset mailmap.file
$ git config mailmap.blob HEAD:.mailmap
$ sed -i 's:p...@peff.com:no-valid-address:' .mailmap
$ git log -1 --author 'Jeff King'

So with the .mailmap being dirty, which address would one expect to be printed? 
I would expect 'no-valid-address', but it's 'p...@peff.com'.

Even though the .mailmap file is more visible and also closer to the user, the 
mailmap.blob wins over it. I find that somewhat counter-intuitive. Of course, 
setting `git config mailmap.file .mailmap`, would do the trick.

Re: [PATCH 1/5] add SWAP macro

[Junio C Hamano]

René Scharfe  writes:

> Swapping between different types would then still have to be done
> manually, but I wonder how common that is -- couldn't find such a case
> in our tree.

I do not think it is a common thing to do, and more importantly, I
doubt we want to hide such a swap inside a macro.  And that is why
I said the seemingly extra "type" thing may be an improvement over
your original SWAP() thing if it gives us more type safety.

It seems that the thread has been quite for a while. Perhaps people
are happy enough with your patches?  If so, let's move it forward,
but I'll wait for a while in case follow-up discussion appears
soonish.  The changes are fairly well isolated and I do not think we
are in a hurry.

Re: [PATCH 1/5] add SWAP macro

[René Scharfe]

Am 01.02.2017 um 19:33 schrieb Junio C Hamano:
> René Scharfe  writes:
> 
>> Size checks could be added to SIMPLE_SWAP as well.
> 
> Between SWAP() and SIMPLE_SWAP() I am undecided.
> 
> If the compiler can infer the type and the size of the two
> "locations" given to the macro, there is no technical reason to
> require the caller to specify the type as an extra argument, so
> SIMPLE_SWAP() may not necessarily an improvement over SWAP() from
> that point of view.  If the redundancy is used as a sanity check,
> I'd be in favor of SIMPLE_SWAP(), though.
> 
> If the goal of SIMPLE_SWAP(), on the other hand, were to support the
> "only swap char with int for small value" example earlier in the
> thread, it means you cannot sanity check the type of things being
> swapped in the macro, and the readers of the code need to know about
> the details of what variables are being swapped.  It looks to me
> that it defeats the main benefit of using a macro.

Full type inference could be done with C11's _Generic for basic types,
while typeof would be needed for complex ones, I guess.  Checking that
sizes match is better than nothing and portable to ancient platforms,
though.  Having an explicit type given is portable and easy to use for
checks, of course, e.g. like this:

#define SIMPLE_SWAP(T, a, b) do { \
T swap_tmp_ = a + BUILD_ASSERT_OR_ZERO(sizeof(T) == sizeof(a)); \
a = b + BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b)); \
b = swap_tmp_; \
} while(0)

It doesn't support expressions with side effects, but that's probably
not much of a concern.

Swapping between different types would then still have to be done
manually, but I wonder how common that is -- couldn't find such a case
in our tree.

René

[PATCH] dir: avoid allocation in fill_directory()

[René Scharfe]

Pass the match member of the first pathspec item directly to
read_directory() instead of using common_prefix() to duplicate it first,
thus avoiding memory duplication, strlen(3) and free(3).

Signed-off-by: Rene Scharfe 
---
This updates 966de3028 (dir: convert fill_directory to use the pathspec
struct interface).  The result is closer to the original, yet still
using the modern interface.

This patch eerily resembles 2b189435 (dir: simplify fill_directory()).

 dir.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/dir.c b/dir.c
index 65c3e681b8..4541f9e146 100644
--- a/dir.c
+++ b/dir.c
@@ -174,20 +174,19 @@ char *common_prefix(const struct pathspec *pathspec)
 
 int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
 {
-   char *prefix;
+   const char *prefix;
size_t prefix_len;
 
/*
 * Calculate common prefix for the pathspec, and
 * use that to optimize the directory walk
 */
-   prefix = common_prefix(pathspec);
-   prefix_len = prefix ? strlen(prefix) : 0;
+   prefix_len = common_prefix_len(pathspec);
+   prefix = prefix_len ? pathspec->items[0].match : "";
 
/* Read the directory and prune it */
read_directory(dir, prefix, prefix_len, pathspec);
 
-   free(prefix);
return prefix_len;
 }
 
-- 
2.11.1

Re: [RFC] Add support for downloading blobs on demand

[Jakub Narębski]

I'd like to point to two (or rather one and a half) solutions that I got
aware of when watching streaming of "Git Merge 2017"[0].  There should
be here people who were there; and hopefully video of those presentations
and slides / notes would be soon available.

[0]: http://git-merge.com/

First tool that I'd like to point to is Git Virtual File System, or
GVFS in short (which unfortunately shares abbreviation with GNOME Virtual
File System).

The presentation was "Scaling Git at Microsoft" by Saeed Noursalehi, 
Microsoft.  You can read about this solution in ArsTechnica article[1],
and on Microsoft blog[2].  The code (or early version of thereof) is
also available[3] - I wonder why on GitHub and not Codeplex...

[1]: 
https://arstechnica.com/information-technology/2017/02/microsoft-hosts-the-windows-source-in-a-monstrous-300gb-git-repository/
[2]: 
https://blogs.msdn.microsoft.com/visualstudioalm/2017/02/03/announcing-gvfs-git-virtual-file-system/
[3]: https://github.com/Microsoft/GVFS


The second presentation that might be of some interest is "Scaling
Mercurial at Facebook: Insights from the Other Side" by Durham Goode,
Facebook.  The code is supposedly available as open-source; though
I don't know how useful their 'blob storage' solution would be of use
for your problem.


HTH
-- 
Jakub Narębski

git push - 401 unauthorized

[Alessio Rocchi]

I try to push my commit on a private repository (which has been working
since about five years).
This is the output of git push:

me@superstar:/var/www/scorte$ git push --verbose
Pushing to http://isisenscorte:mypassword@mymachine/scorte_git
Getting pack list
Fetching remote heads...
  refs/
  refs/tags/
  refs/heads/
updating 'refs/heads/master'
  from d9fd2e49cb0c32a6d8fddcff2954f04b4104d176
  to   23d8edfb7fa70bce44c21a7f93064c08a7288e23
sending 6 objects
MOVE 33fcba80fdec82f43f995e5c693255da075358be failed, aborting (52/0)
MOVE 60e1a097d50fe62319413ed20129580cf175d1ca failed, aborting (52/0)
MOVE cfea41ef02f9aef5cecfbf7eac5a9e55975113f4 failed, aborting (52/0)
MOVE 3d87ab6ff7652f2b30e48612b70c8335d4625699 failed, aborting (52/0)
MOVE 4adb1b39e0446e0bfc3182258ff1cd7077871f7f failed, aborting (52/0)
Updating remote server info
fatal: git-http-push failed

Looking at apache logs, I've got this output

192.168.240.127 - myusername [07/Feb/2017:19:57:01 +0100] "PROPFIND
/scorte_git/objects/23/ HTTP/1.1" 207 6003 "-" "git/1.7.0.4"
192.168.240.127 - myusername [07/Feb/2017:19:57:01 +0100] "PROPFIND
/scorte_git/objects/60/ HTTP/1.1" 207 7651 "-" "git/1.7.0.4"
192.168.240.127 - myusername [07/Feb/2017:19:57:02 +0100] "PROPFIND
/scorte_git/objects/4a/ HTTP/1.1" 207 3640 "-" "git/1.7.0.4"
192.168.240.127 - myusername [07/Feb/2017:19:57:02 +0100] "PROPFIND
/scorte_git/objects/3d/ HTTP/1.1" 207 13742 "-" "git/1.7.0.4"
192.168.240.127 - myusername [07/Feb/2017:19:57:02 +0100] "PROPFIND
/scorte_git/objects/cf/ HTTP/1.1" 207 13799 "-" "git/1.7.0.4"
192.168.240.127 - myusername [07/Feb/2017:19:57:02 +0100] "PROPFIND
/scorte_git/objects/33/ HTTP/1.1" 207 13783 "-" "git/1.7.0.4"
192.168.240.127 - - [07/Feb/2017:19:57:02 +0100] "PUT
/scorte_git/objects/3d/87ab6ff7652f2b30e48612b70c8335d4625699_8d42f74642dae7
7465d1fbfafbd720f67a1919f4 HTTP/1.1" 401 810 "-" "git/1.7.0.4"
192.168.240.127 - - [07/Feb/2017:19:57:02 +0100] "PUT
/scorte_git/objects/cf/ea41ef02f9aef5cecfbf7eac5a9e55975113f4_8d42f74642dae7
7465d1fbfafbd720f67a1919f4 HTTP/1.1" 401 810 "-" "git/1.7.0.4"
192.168.240.127 - myusername [07/Feb/2017:19:57:02 +0100] "PUT
/scorte_git/objects/33/fcba80fdec82f43f995e5c693255da075358be_8d42f74642dae7
7465d1fbfafbd720f67a1919f4 HTTP/1.1" 201 809 "-" "git/1.7.0.4"
192.168.240.127 - - [07/Feb/2017:19:57:02 +0100] "PUT
/scorte_git/objects/4a/db1b39e0446e0bfc3182258ff1cd7077871f7f_8d42f74642dae7
7465d1fbfafbd720f67a1919f4 HTTP/1.1" 401 810 "-" "git/1.7.0.4"
192.168.240.127 - - [07/Feb/2017:19:57:02 +0100] "PUT
/scorte_git/objects/60/e1a097d50fe62319413ed20129580cf175d1ca_8d42f74642dae7
7465d1fbfafbd720f67a1919f4 HTTP/1.1" 401 810 "-" "git/1.7.0.4"

It looks like I'm getting 401 errors on every line where username is
missing.

Permissions on the unauthorized object folders are 777 everywhere. My git
version is 1.7.0.4 on both client and server. Do you have any clue of this
strange behaviour?

Thank you in advance, Alex

Re: Git clonebundles

[Junio C Hamano]

Johannes Schindelin  writes:

>> If people think it might be useful to have it around to experiment, I
>> can resurrect and keep that in 'pu' (or rather 'jch'), as long as it
>> does not overlap and conflict with other topics in flight.  Let me try
>> that in today's integration cycle.
>
> I would like to remind you of my suggestion to make this more publicly
> visible and substantially easier to play with, by adding it as an
> experimental feature (possibly guarded via an explicit opt-in config
> setting).

I do not understand why you want to give this topic undue prominence
ovver any other random topic that cook in 'pu' and later merged down
to 'next' and then 'master' only after they turn out to be useful
(or at least harmless).

If there were somebody who is the champion of that topic, advocating
that any clone-bundle solution must be based on this topic, it would
be different.  Even though I am not opposed to the topic myself, I
am not that somebody.  That is why I kept it around to wait to see
if somebody finds it potentially useful and then discarded it after
seeing no such person stepped up.

That champion of the topic would spend the necessaly engineering
effort to document it as experimental, to make sure that there is a
reasonable upgrade/transition route if the "v3" format turns out to
be not very useful, etc. by rerolling the patches or following-up on
them to advance it from 'pu' down to 'next' and to 'master' just
like any other topic.

Judging from the tone of his message (i.e. "unfortunately" in it),
Christian may want to be one, or somebody else may want to be one.

[PATCH/RFC v2] WIP: log: allow "-" as a short-hand for "previous branch"

[Siddharth Kannan]

Teach revision.c:setup_revisions that an argument starting with "-" can be an
argument also. `left` variable needs to be incremented only when the supplied
arg is neither an argument, nor an option.

Teach sha1_name.c:get_sha1_1 that "-" is equivalent to "@{-1}"

Signed-off-by: Siddharth Kannan 
---
I have run the test suite locally and on Travis CI. [1]

Whenever the argument begins with a "-" then the function "handle_revision_arg" 
is called twice. I can fix this using a variable that would store whether the
function has been called earlier or not. For doing that I have to investigate
some more on what the valid return values for "handle_revision_arg" are. (Or
a simpler approach would be to use an integer flag, this would also not be
affected if in case "handle_revision_arg" is changed in the future)

I have also written a very basic test for git-log only. I have based this on the
tests that were added in 696acf4 (checkout: implement "-" abbreviation, add docs
and tests, 2009-01-17). It tests revisions, revision ranges, and open-ended
revision ranges (where the start or the finish argument is inferred) If the
code in this patch is okay, or close to okay, then please tell me if further
tests need to be added for git-log and/or other commands.

This change touches a few commands, other than log. notably, git-format-patch,
git-whatchanged and git-show, all of which are defined inside builtin/log.c. In
general, it affects commands that call setup_revisions at some point in their
codepath. (eg: git-diff-index)

Thanks a lot, Junio, for your comments on the previous version of this patch and
further discussion on that thread!

[1]: https://travis-ci.org/icyflame/git/builds/199350136

 revision.c   |  9 +--
 sha1_name.c  |  5 
 t/t4214-log-shorthand.sh | 62 
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100755 t/t4214-log-shorthand.sh

diff --git a/revision.c b/revision.c
index b37dbec..e14f62c 100644
--- a/revision.c
+++ b/revision.c
@@ -2206,7 +2206,7 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
if (*arg == '-') {
-   int opts;
+   int opts, args;
 
opts = handle_revision_pseudo_opt(submodule,
revs, argc - i, argv + i,
@@ -2234,7 +2234,12 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
}
if (opts < 0)
exit(128);
-   continue;
+
+   args = handle_revision_arg(arg, revs, flags, 
revarg_opt);
+   if (args)
+   continue;
+   else
+   --left;
}
 
 
diff --git a/sha1_name.c b/sha1_name.c
index 73a915f..d774e46 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -947,6 +947,11 @@ static int get_sha1_1(const char *name, int len, unsigned 
char *sha1, unsigned l
if (!ret)
return 0;
 
+   if (!strcmp(name, "-")) {
+   name = "@{-1}";
+   len = 5;
+   }
+
ret = get_sha1_basic(name, len, sha1, lookup_flags);
if (!ret)
return 0;
diff --git a/t/t4214-log-shorthand.sh b/t/t4214-log-shorthand.sh
new file mode 100755
index 000..95cf2d4
--- /dev/null
+++ b/t/t4214-log-shorthand.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='log can show previous branch using shorthand - for @{-1}'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   echo hello >world &&
+   git add world &&
+   git commit -m initial &&
+   echo "hello second time" >>world &&
+   git add world &&
+   git commit -m second &&
+   echo "hello other file" >>planet &&
+   git add planet &&
+   git commit -m third &&
+   echo "hello yet another file" >>city &&
+   git add city &&
+   git commit -m fourth
+'
+
+test_expect_success '"log -" should not work initially' '
+   test_must_fail git log -
+'
+
+test_expect_success '"log -" should work' '
+   git checkout -b testing-1 master^ &&
+   git checkout -b testing-2 master~2 &&
+   git checkout master &&
+
+   git log testing-2 >expect &&
+   git log - >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'revision range should work when one end is left empty' '
+   git checkout testing-2 &&
+   git checkout master &&
+   git log ...@{-1} > expect.first_empty &&
+   git log @{-1}... > expect.last_empty &&
+   git log ...- > actual.first_empty &&
+   git log -... > actual.last_empty &&
+   test_cmp expect.first_empty actual.first_empty &&
+   test_cmp expect.last_empty 

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Junio C Hamano]

Junio C Hamano  writes:

Sorry, one shouldn't type while being sick and in bed X-<.

> I am not sure if you are shooting for is "work correctly" to begin
> with, to be honest.  The current code always shows the "correct"
> output which is "the tree-ish object name (expressed in a way easier
> to understand by the humans), followed by a colon, followed by the
> path in the tree-ish the hit lies".  You are making it "incorrect
> but often more convenient", and sometimes that is a worth goal, but

s/worth//;

> for the particular use cases you presented, i.e.
>
> $ git grep -e "$pattern" "$commit:path"
>
> a more natural way to express "I want to find this pattern in the
> commit under that path" exists:
>
> $ git grep -e "$pattern" "$commit" -- path
>
> and because of that, I do not think the former form of the query

s/do not think/do think/

> should happen _less_ often in the first place, which would make it
> "incorrect but more convenient if the user gives an unusual query".
>
> So I am not sure if the change to "grep" is worth it.

Also, it may be fairer to do s/incorrect/inconsistent/.

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Junio C Hamano]

Stefan Hajnoczi  writes:

> Perhaps it's better to leave this than to merge code that doesn't work
> correctly 100% of the time.

I am not sure if you are shooting for is "work correctly" to begin
with, to be honest.  The current code always shows the "correct"
output which is "the tree-ish object name (expressed in a way easier
to understand by the humans), followed by a colon, followed by the
path in the tree-ish the hit lies".  You are making it "incorrect
but often more convenient", and sometimes that is a worth goal, but
for the particular use cases you presented, i.e.

$ git grep -e "$pattern" "$commit:path"

a more natural way to express "I want to find this pattern in the
commit under that path" exists:

$ git grep -e "$pattern" "$commit" -- path

and because of that, I do not think the former form of the query
should happen _less_ often in the first place, which would make it
"incorrect but more convenient if the user gives an unusual query".

So I am not sure if the change to "grep" is worth it.

Having said that, I actually think "make it more convenient" without
making anything incorrect would be to teach the revision parser to
understand

Re: [PATCH] difftool: fix bug when printing usage

[Junio C Hamano]

Junio C Hamano  writes:

> Johannes Schindelin  writes:
>
>>> > Likewise, this would become
>>> >
>>> >   GIT_CEILING_DIRECTORIES="$PWD/not" \
>>> >   test_expect_code 129 git -C not/repo difftool -h >output &&
>>> >   grep ^usage: output
>>> 
>>> I agree with the intent, but the execution here is "Not quite".
>>> test_expect_code being a shell function, it does not take the
>>> "one-shot environment assignment for this single invocation," like
>>> external commands do.
>>
>> So now that we know what is wrong, can you please enlighten me about what
>> is right?
>
> David's original is just fine, isn't it?

I've also seen people use "env VAR=VAL git command" as the command
to be tested in t/ scripts.  You can run that under test_expect_code,
methinks.

Re: [PATCH] difftool: fix bug when printing usage

[Junio C Hamano]

Johannes Schindelin  writes:

>> > Likewise, this would become
>> >
>> >GIT_CEILING_DIRECTORIES="$PWD/not" \
>> >test_expect_code 129 git -C not/repo difftool -h >output &&
>> >grep ^usage: output
>> 
>> I agree with the intent, but the execution here is "Not quite".
>> test_expect_code being a shell function, it does not take the
>> "one-shot environment assignment for this single invocation," like
>> external commands do.
>
> So now that we know what is wrong, can you please enlighten me about what
> is right?

David's original is just fine, isn't it?

Re: What's cooking in git.git (Feb 2017, #02; Mon, 6)

[Junio C Hamano]

SZEDER Gábor  writes:

> All failing tests fail with the same error:
>
>   fatal: unrecognized %(refname:strip=2) argument: strip=2
>
> That's because of this topic:
>
>> * kn/ref-filter-branch-list (2017-01-31) 20 commits

Ahh, of course.

Let's make sure the series won't escape to 'master' before the
"strip" breakage is fixed.  How about queuing this on top of the
ref-filter topic?  

It seems to unblock your completion-refs-speedup topic and makes the
test pass ;-)

Thanks.

-- >8 --
Subject: [PATCH] ref-filter: resurrect "strip" as a synonym to "lstrip"

We forgot that "strip" was introduced at 0571979bd6 ("tag: do not
show ambiguous tag names as "tags/foo"", 2016-01-25) as part of Git
2.8 (and 2.7.1), yet in the update to ref-filter, we started calling
it "lstrip" to make it easier to explain the new "rstrip" operation.

We shouldn't have renamed the existing one; "lstrip" should have
been a new synonym that means the same thing as "strip".  Scripts
in the wild are surely using the original form already.

Signed-off-by: Junio C Hamano 
---
 Documentation/git-for-each-ref.txt |  2 ++
 ref-filter.c   |  3 ++-
 t/t6300-for-each-ref.sh| 12 
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 2008600e7e..111e1be6f5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -107,6 +107,8 @@ refname::
enough components, the result becomes an empty string if
stripping with positive , or it becomes the full refname if
stripping with negative .  Neither is an error.
++
+`strip` can be used as a synomym to `lstrip`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index 01b5c18ef0..2a94d6da98 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -112,7 +112,8 @@ static void refname_atom_parser_internal(struct 
refname_atom *atom,
atom->option = R_NORMAL;
else if (!strcmp(arg, "short"))
atom->option = R_SHORT;
-   else if (skip_prefix(arg, "lstrip=", )) {
+   else if (skip_prefix(arg, "lstrip=", ) ||
+skip_prefix(arg, "strip=", )) {
atom->option = R_LSTRIP;
if (strtol_i(arg, 10, >lstrip))
die(_("Integer value expected refname:lstrip=%s"), arg);
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 25a9973ce9..c87dc1f8bc 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -59,18 +59,26 @@ test_atom head refname:rstrip=1 refs/heads
 test_atom head refname:rstrip=2 refs
 test_atom head refname:rstrip=-1 refs
 test_atom head refname:rstrip=-2 refs/heads
+test_atom head refname:strip=1 heads/master
+test_atom head refname:strip=2 master
+test_atom head refname:strip=-1 master
+test_atom head refname:strip=-2 heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head upstream:short origin/master
 test_atom head upstream:lstrip=2 origin/master
 test_atom head upstream:lstrip=-2 origin/master
 test_atom head upstream:rstrip=2 refs/remotes
 test_atom head upstream:rstrip=-2 refs/remotes
+test_atom head upstream:strip=2 origin/master
+test_atom head upstream:strip=-2 origin/master
 test_atom head push refs/remotes/myfork/master
 test_atom head push:short myfork/master
 test_atom head push:lstrip=1 remotes/myfork/master
 test_atom head push:lstrip=-1 master
 test_atom head push:rstrip=1 refs/remotes/myfork
 test_atom head push:rstrip=-1 refs
+test_atom head push:strip=1 remotes/myfork/master
+test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
@@ -636,6 +644,10 @@ EOF
 test_expect_success 'Verify usage of %(symref:lstrip) atom' '
git for-each-ref --format="%(symref:lstrip=2)" refs/heads/sym > actual 
&&
git for-each-ref --format="%(symref:lstrip=-2)" refs/heads/sym >> 
actual &&
+   test_cmp expected actual &&
+
+   git for-each-ref --format="%(symref:strip=2)" refs/heads/sym > actual &&
+   git for-each-ref --format="%(symref:strip=-2)" refs/heads/sym >> actual 
&&
test_cmp expected actual
 '
 
-- 
2.12.0-rc0-144-g99fe1a5456

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Jeff King]

On Tue, Feb 07, 2017 at 03:04:14PM +, Stefan Hajnoczi wrote:

> > I assume Stefan just grabbed my naive suggestion hence why it checks
> > equality with a commit.  So that's my fault :)  Either of these may
> > not be enough though, since if you do 'git grep malloc v2.9.3^{tree}'
> > with this change the output prefix is 'v2.9.3^{tree}/' instead of the
> > correct prefix 'v2.9.3^{tree}:'
> 
> I revisited this series again today and am coming to the conclusion that
> forming output based on the user's rev is really hard to get right in
> all cases.  I don't have a good solution to the v2.9.3^{tree} problem.

I think the rule you need is not "are we at a tree", but rather "did we
traverse a path while resolving the name?". Only the get_sha1() parser
can tell you that. I think:

  char delim = ':';
  struct object_context oc;
  if (get_sha1_with_context(name, 0, sha1, ))
  die("...");
  if (oc.path[0])
  delim = '/'; /* name had a partial path */

would work. Root trees via "v2.9.3^{tree}" or "v2.9.3:" would have no
path, but "v2.9.3:Documentation" would. I think you'd still need to
avoid duplicating a trailing delimiter, but I can't think of a case
where it is wrong to do that based purely on the name.

-Peff

Re: [PATCH] rev-list-options.txt: update --all about detached HEAD

[Jeff King]

On Tue, Feb 07, 2017 at 08:38:49PM +0700, Nguyễn Thái Ngọc Duy wrote:

> This is the document patch for f0298cf1c6 (revision walker: include a
> detached HEAD in --all - 2009-01-16)
> [...]
>  --all::
> - Pretend as if all the refs in `refs/` are listed on the
> - command line as ''.
> + Pretend as if all the refs in `refs/` (and HEAD if detached)
> + are listed on the command line as ''.

I think this is an improvement, but I'm not sure about the "if detached"
bit. We always read HEAD, no matter what.

If you only care about reachability, then reading HEAD only has an
impact if it is detached, since otherwise we know that we will grab the
ref via refs/.

I'm not sure if it would matter for some other cases, though. For
example, with "--source", do we report HEAD or the matching ref? It
looks like the latter (because we read the refs first).

I suspect you could also construct a case with excludes like:

  $ git checkout foo
  $ git rev-list --exclude=refs/heads/foo --all

where it is relevant that we read HEAD separately from refs/heads/foo.

So I think just "and HEAD" is better, like:

  Pretend as if all the refs in `refs/`, along with `HEAD`, are
  listed...

-Peff

Re: subtree merging fails

[David Aguilar]

On Tue, Feb 07, 2017 at 08:59:06AM -0600, Samuel Lijin wrote:
> Have you tried using (without -s subtree) -X subtree=path/to/add/subtree/at?
> 
> From the man page:
> 
>   subtree[=]
>This option is a more advanced form of subtree
> strategy, where the strategy
>makes a guess on how two trees must be shifted to match
> with each other when
>merging. Instead, the specified path is prefixed (or
> stripped from the
>beginning) to make the shape of two trees to match.

I'm not 100% certain, but it's highly likely that the subtree=
argument needs to include a trailing slash "/" in the prefix,
otherwise files will be named e.g. "fooREADME" instead of
"foo/README" when prefix=foo.

These days I would steer users towards the "git-subtree" command in
contrib/ so that users don't need to deal with these details.  It
handles all of this stuff for you.

https://github.com/git/git/blob/master/contrib/subtree/git-subtree.txt

https://github.com/git/git/tree/master/contrib/subtree

Updating the progit book to also mention git-subtree, in addition to the
low-level methods, would probably be a good user-centric change.
-- 
David

Re: ``git clean -xdf'' and ``make clean''

[Jacob Keller]

On Tue, Feb 7, 2017 at 7:07 AM, Cornelius Weig
 wrote:
> On 02/07/2017 03:17 PM, Hongyi Zhao wrote:
>> Hi all,
>>
>> In order to delete all of the last build stuff, does the following two
>> methods equivalent or not?
>>
>> ``git clean -xdf'' and ``make clean''
>
> No, it is not equivalent.
>
> * `make clean` removes any build-related files (assuming that the
> `clean` target is properly written). To see exactly what it would do,
> run `make clean -n`. Judging from your question, I think this is what
> you want to do.
>
> * `git clean -xdf` would remove any files that git does not track. This
> also includes build-related files, but also any other files that happen
> to be in your working directory. For example, any output from `git
> format-patch` would be removed by this, but not `make clean`.

Make clean can run arbitrary code, and really depends on the
implementation. git clean -xdf will result in all non-tracked files
being removed, which should restore you to a pristine pre-build state.
However, this can have unfortunate side effect of destroying files
which you might not expect.

Properly written, a make clean shouldn't remove anything except what
could be regenerated by make. But that's just a strong convention.

Regards,
Jake

Re: [RFC] mailmap.blob overrides default .mailmap

[Jeff King]

On Tue, Feb 07, 2017 at 09:27:19AM -0800, Stefan Beller wrote:

> > The code shows why (mailmap.c):
> > err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
> > if (startup_info->have_repository)
> > err |= read_mailmap_blob(map, git_mailmap_blob, 
> > repo_abbrev);
> > err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
> >
> >
> > Apparently this is not an oversight, because there is an explicit
> > test for this overriding behavior (t4203 'mailmap.blob overrides
> > .mailmap').
> 
> which is blamed to 08610900 (mailmap: support reading mailmap from
> blobs, 2012-12-12),
> cc'ing Jeff who may remember what he was doing back then, as the
> commit message doesn't discuss the implications on ordering.

I think it was mostly that I had to define _some_ order. This made sense
to me as similar to things like attributes or excludes, where we prefer
clone-specific data over in-history data (so .git/info/attributes takes
precedence over .gitattributes).

So any mailmap.* would take precedence over the in-tree .mailmap file.
And then between mailmap.file and mailmap.blob, the "blob" form is
more "in-tree" than the "file" form (especially because we turn it on by
default in bare repos, so it really is identical to the in-tree form
there).

I think the easiest way to think of it is the same as we do config. We
read the files in a particular order, least-important to most-important,
and apply "last one wins" (so more-important entries overwrite
less-important ones).

-Peff

Trying to use xfuncname without success.

[Jack Adrian Zappa]

I'm trying to specify a hunk header using xfuncname, and it just
doesn't want to work.

The full question is on SO here:

http://stackoverflow.com/questions/42078376/why-isnt-my-xfuncname-working-in-my-gitconfig-file

But the basic gist is that no matter what regex I specify, git will
not recognise the hunk header.  Am I doing something wrong or is this
a bug?

For those who don't want to jump to the SO site, I've copied the text below:

-8<8<8<8<8<8<8<8<---

I'm trying to setup a hunk header for .natvis files. For some reason,
it doesn't seem to be working. I'm following their instructions from
here, which doesn't say much in terms of restrictions of the regex,
such as, is the matched item considered the hunk header or do I need a
group? I have tried both with no success. This is what I have:

[diff "natvis"]
xfuncname = "^[\\\t ]*

  

  var













  
  added_var


  var2

  


with the added_var being the new line added.

I'm really not sure why this is so difficult.

EDIT:

Here is a sample output of what I am getting:

$ git diff --word-diff
diff --git a/test.natvis b/test.natvis
index 73c06bc..bc0f549 100644
--- a/test.natvis
+++ b/test.natvis
@@ -18,6 +18,7 @@


  
  {+added_var+}


  var2
warning: LF will be replaced by CRLF in test.natvis.
The file will have its original line endings in your working directory.

Even using xfuncname = "^.*$" I would have expected that  would have shown up as my hunk header, but I get
nothing. :(

EDIT:

I've tried the solution proposed by torek, but to no avail. It's like
it doesn't know what to do with the xfuncname entry. :(

Re: Request re git status

[Jacob Keller]

On Tue, Feb 7, 2017 at 6:54 AM, Samuel Lijin  wrote:
> On Mon, Feb 6, 2017 at 6:45 PM, Phil Hord  wrote:
>> On Mon, Feb 6, 2017 at 3:36 PM Ron Pero  wrote:
>>> I almost got bit by git: I knew there were changes on the remote
>>> server, but git status said I was uptodate with the remote.
>>>
>>
>> Do you mean you almost pushed some changed history with "--force"
>> which would have lost others' changes?  Use of this option is
>> discouraged on shared branches for this very reason.  But if you do
>> use it, the remote will tell you the hash of the old branch so you can
>> undo the damage.
>>
>> But if you did not use --force, then you were not in danger of being
>> bit.  Git would have prevented the push in that case.
>>
>>
>>> Why ... not design it to [optionally] DO a fetch and THEN declare
>>> whether it is up to date?
>>
>> It's because `git status` does not talk to the remote server, by
>> design.  The only Git commands that do talk to the remote are push,
>> pull and fetch.  All the rest work off-line and they do so
>> consistently.
>>
>> Imagine `git status` did what you requested; that is, it first did a
>> fetch and then reported the status.  Suppose someone pushed a commit
>> to the remote immediately after your fetch completed.  Now git will
>> still report "up to date" but it will be wrong as soon as the remote
>> finishes adding the new push.  Yet the "up to date" message will
>> remain on your console, lying to you.  If you leave and come back in
>> two days, the message will remain there even if it is no longer
>> correct.
>>
>> So you should accept that `git status` tells you the status with
>> respect to your most recent fetch, and that you are responsible for
>> the timing of the most recent fetch.  To have git try to do otherwise
>> would be misleading.
>
> This argument doesn't work for me. Race conditions in *any*
> asynchronous work flow are inevitable; in commits, particularly to a
> shared branch, I also can't imagine them being common. It's like
> saying because there's lag between the remote's response and the
> output on the local, `git fetch` shouldn't bother saying that the
> local remote has been updated.
>
> It wouldn't be hard, though, to define an alias that fetches the
> remote-tracking branch and then reports the status.
>
> Nevertheless, this is one of those cases where I think Git suffers
> from a poor UI/UX - it's letting the underlying model define the
> behavior, rather than using the underlying model to drive the
> behavior.
>

Personally, I think that the fact that Git forces the user to think
about it in terms of "oh I have to fetch" instead of that happening
automatically, it helps teach the model to the user. If it happened in
the background then the user might not be confronted with the
distributed nature of the tool.

An alias to fetch and then show status is very straight forward, and
you can do so locally if you want.

Thanks,
Jake

Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"

[Siddharth Kannan]

On Mon, Feb 06, 2017 at 03:09:47PM -0800, Junio C Hamano wrote:
> The focus of GSoC being mentoring those who are new to the open
> source development, and hopefully retain them in the community after
> GSoC is over, we do expect microprojects to be suitable for those
> who are new to the codebase.

Okay, understood! Since I have spent time here anyway, I guess I will
continue on this instead of going over to a new micro project.

> 
> > (c) -> Else look for "r1^-"
> > ...
> > Case (c) is a bit confusing. This could be something like "-^-", and
> > something like "^-" could mean "Not commits on previous branch" or it
> > could mean "All commits on this branch except for the parent of HEAD"
> 
> Do you mean:
> 
> "git rev-parse ^-" does not mean "git rev-parse HEAD^-", but we
> probably would want to, and if that is what is going to happen,
> "^-" should mean "HEAD^-", and cannot be used for "^@{-1}"?
> 
> It's friend "^!" does not mean "HEAD^!", and "^@" does not mean
> "HEAD^@", either (the latter is somewhat borked, though, and "^@"
> translates to "^HEAD" because confusingly "@" stands for "HEAD"
> sometimes).  

Yes, I meant that whether we should use ^- as ^@{-1} or HEAD^-.

Oh! So, that's why running `git log ^@` leads to an empty set!
> 
> So my gut feeling is that it is probably OK to make "^-" mean
> "^@{-1}"; it may be prudent to at least initially keep "^-" an error
> like it currently is already, though.

I agree with your gut feeling, and would like to _not_ exclude only
this case. This way, across the code and implementation, there
wouldn't be any particular cases which would have to be excluded.

> > So, this patch reduces to the following 2 tasks:
> > 
> > 1. Teach setup_revisions that something starting with "-" can be
> > an
> > argument as well
> > 2. Teach get_sha1_basic that "-" means the tip of the previous
> > branch
> > perhaps by replacing it with "@{-1}" just before the reflog
> > parsing is
> > done

Making a change in sha1_name.c will touch a lot of commands
(setup_revisions is called from everywhere in the codebase), so, I am
still trying to figure out how to do this such that the rest of the
codepath remains unchanged.

I hope that you do not mind this side-effect, but rather, you intended
for this to happen, right? More commands will start supporting this
shorthand, suddenly.  (such as format-patch, whatchanged, diff to name
a very few).

Best Regards,

Siddharth.

RE: [RFC] Add support for downloading blobs on demand

[Ben Peart]

No worries about a late response, I'm sure this is the start of a long 
conversation. :)

> -Original Message-
> From: Christian Couder [mailto:christian.cou...@gmail.com]
> Sent: Sunday, February 5, 2017 9:04 AM
> To: Ben Peart 
> Cc: Jeff King ; git ; Johannes Schindelin
> 
> Subject: Re: [RFC] Add support for downloading blobs on demand
> 
> (Sorry for the late reply and thanks to Dscho for pointing me to this thread.)
> 
> On Tue, Jan 17, 2017 at 10:50 PM, Ben Peart  wrote:
> >> From: Jeff King [mailto:p...@peff.net] On Fri, Jan 13, 2017 at
> >> 10:52:53AM -0500, Ben Peart wrote:
> >>
> >> > Clone and fetch will pass a  --lazy-clone  flag (open to a better
> >> > name
> >> > here) similar to  --depth  that instructs the server to only return
> >> > commits and trees and to ignore blobs.
> >> >
> >> > Later during git operations like checkout, when a blob cannot be
> >> > found after checking all the regular places (loose, pack,
> >> > alternates, etc), git will download the missing object and place it
> >> > into the local object store (currently as a loose object) then resume the
> operation.
> >>
> >> Have you looked at the "external odb" patches I wrote a while ago,
> >> and which Christian has been trying to resurrect?
> >>
> >>
> >> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpubli
> >> c-
> >> inbox.org%2Fgit%2F20161130210420.15982-1-
> >>
> chriscool%40tuxfamily.org%2F=02%7C01%7CBen.Peart%40microsoft.c
> >>
> om%7C9596d3bf32564f123e0c08d43f08a9e1%7C72f988bf86f141af91ab2d7c
> >>
> d011db47%7C1%7C0%7C636202753822020527=a6%2BGOAQoRhjFoxS
> >> vftY8JZAVUssmrXuDZ9OBy3xqNZk%3D=0
> >>
> >> This is a similar approach, though I pushed the policy for "how do
> >> you get the objects" out into an external script. One advantage there
> >> is that large objects could easily be fetched from another source
> >> entirely (e.g., S3 or equivalent) rather than the repo itself.
> >>
> >> The downside is that it makes things more complicated, because a push
> >> or a fetch now involves three parties (server, client, and the
> >> alternate object store). So questions like "do I have all the objects
> >> I need" are hard to reason about.
> >>
> >> If you assume that there's going to be _some_ central Git repo which
> >> has all of the objects, you might as well fetch from there (and do it
> >> over normal git protocols). And that simplifies things a bit, at the cost 
> >> of
> being less flexible.
> >
> > We looked quite a bit at the external odb patches, as well as lfs and
> > even using alternates.  They all share a common downside that you must
> > maintain a separate service that contains _some_ of the files.
> 
> Pushing the policy for "how do you get the objects" out into an external
> helper doesn't mean that the external helper cannot use the main service.
> The external helper is still free to do whatever it wants including calling 
> the
> main service if it thinks it's better.

That is a good point and you're correct, that means you can avoid having to 
build out multiple services.

> 
> > These
> > files must also be versioned, replicated, backed up and the service
> > itself scaled out to handle the load.  As you mentioned, having
> > multiple services involved increases flexability but it also increases
> > the complexity and decreases the reliability of the overall version
> > control service.
> 
> About reliability, I think it depends a lot on the use case. If you want to 
> get
> very big files over an unreliable connection, it can better if you send those 
> big
> files over a restartable protocol and service like HTTP/S on a regular web
> server.
> 

My primary concern about reliability was the multiplicative effect of making 
multiple requests across multiple servers to complete a single request.  Having 
putting this all in a single service like you suggested above brings us back to 
parity on the complexity.

> > For operational simplicity, we opted to go with a design that uses a
> > single, central git repo which has _all_ the objects and to focus on
> > enhancing it to handle large numbers of files efficiently.  This
> > allows us to focus our efforts on a great git service and to avoid
> > having to build out these other services.
> 
> Ok, but I don't think it prevents you from using at least some of the same
> mechanisms that the external odb series is using.
> And reducing the number of mechanisms in Git itself is great for its
> maintainability and simplicity.

I completely agree with the goal of reducing the number of mechanisms in Git 
itself.  Our proposal is primarily targeting speeding operations when dealing 
with large numbers of files.  ObjectDB is primarily targeting large objects but 
there is a lot of similarity in how we're approaching the solution.  I 
hope/believe we can come to a common solution that will solve both.

> 
> >> > To prevent git 

Re: [RFC] mailmap.blob overrides default .mailmap

[Stefan Beller]

On Tue, Feb 7, 2017 at 3:56 AM, Cornelius Weig
 wrote:
> Hi,
>
>  I was reading into the mailmap handling today and I'm a bit puzzled by the 
> overriding behavior.
>
> This is what the documentation says about precedence (emphasis mine):
> -
> mailmap.file
> The location of an augmenting mailmap file. The default mailmap, located
> in the root of the repository, is loaded first, then the mailmap file
> pointed to by this variable. The location of the mailmap file may be in a
> repository subdirectory, or somewhere outside of the repository itself.
> See git-shortlog(1) and git-blame(1).
>
> mailmap.blob
> Like mailmap.file, but consider the value as a reference to a blob in the
> repository. If both mailmap.file and mailmap.blob are given, both are
> !!! parsed, with _entries from mailmap.file taking precedence_. In a bare
> repository, this defaults to HEAD:.mailmap. In a non-bare repository, it
> defaults to empty.
> 
>
> So from the doc I would have expected that files always get precedence over 
> the blob. IOW entries from .mailmap override entries from mailmap.blob. 
> However, this is not the case.
>
> The code shows why (mailmap.c):
> err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
> if (startup_info->have_repository)
> err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
> err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
>
>
> Apparently this is not an oversight, because there is an explicit test for 
> this overriding behavior (t4203 'mailmap.blob overrides .mailmap').

which is blamed to 08610900 (mailmap: support reading mailmap from
blobs, 2012-12-12),
cc'ing Jeff who may remember what he was doing back then, as the
commit message doesn't discuss the implications on ordering.

>
> So I wonder: what is the rationale behind this? I find this mixed overriding 
> behavior hard to explain and difficult to understand.
>

Re: Git clonebundles

[Stefan Beller]

On Tue, Feb 7, 2017 at 4:04 AM, Johannes Schindelin
 wrote:
> Hi Junio,
>
> On Mon, 6 Feb 2017, Junio C Hamano wrote:
>
>> Christian Couder  writes:
>>
>> > There is also Junio's work on Bundle v3 that was unfortunately
>> > recently discarded.  Look for "jc/bundle" in:
>> >
>> > http://public-inbox.org/git/xmqq4m0cry60@gitster.mtv.corp.google.com/
>> >
>> > and previous "What's cooking in git.git" emails.
>>
>> If people think it might be useful to have it around to experiment, I
>> can resurrect and keep that in 'pu' (or rather 'jch'), as long as it
>> does not overlap and conflict with other topics in flight.  Let me try
>> that in today's integration cycle.
>
> I would like to remind you of my suggestion to make this more publicly
> visible and substantially easier to play with, by adding it as an
> experimental feature (possibly guarded via an explicit opt-in config
> setting).
>
> Ciao,
> Johannes

For making this more publicly visible, I want to look into publishing
the cooking reports on the git-scm.com. Maybe we can have a "dev"
section there, that has
* a "getting started" section
  linking to
Documentation/SubmittingPatches
How to setup your travis
* "current state of development" section
  e.g. the cooking reports, the
  release calender, description of the workflow
  (which branches do exist and serve which purpose),

Most of the static information is already covered quite
well in Documentation/ so there is definitively overlap,
hence lots of links to the ground truth.

The dynamic information however (release calender,
cooking reports) are not described well enough in
Documentation/ so I think we'd want to focus on these
in that dev section.

Re: The design of refs backends, linked worktrees and submodules

[Duy Nguyen]

On Thu, Jan 19, 2017 at 6:55 PM, Duy Nguyen  wrote:
> I've started working on fixing the "git gc" issue with multiple
> worktrees, which brings me back to this. Just some thoughts. Comments
> are really appreciated.
>
> In the current code, files backend has special cases for both
> submodules (explicitly) and linked worktrees (hidden behind git_path).

It just occurs to me that, since the refs directory structure of a
linked worktree is exactly like one in a normal single-worktree setup,
minus the shared (or packed) refs. The "files" refs backend can just
see this "per-worktree only" refs directory as a remote refs storage,
which is just another name for "submodule".

So, we could just use the exact same submodule code path in refs to
create a per-worktree refs storage. Doing it this way, files backedn
do not need to learn about linked worktrees at all. To retrieve a
per-worktree refs storage, we do
"get_ref_store(".git/worktrees/foobar")". To get all per-worktree refs
do for_each_ref_submodule(".git/worktrees/foobar", ...).

Does it make sense? Should we go this way?
-- 
Duy

Re: ``git clean -xdf'' and ``make clean''

[Cornelius Weig]

On 02/07/2017 03:17 PM, Hongyi Zhao wrote:
> Hi all,
> 
> In order to delete all of the last build stuff, does the following two
> methods equivalent or not?
> 
> ``git clean -xdf'' and ``make clean''

No, it is not equivalent.

* `make clean` removes any build-related files (assuming that the
`clean` target is properly written). To see exactly what it would do,
run `make clean -n`. Judging from your question, I think this is what
you want to do.

* `git clean -xdf` would remove any files that git does not track. This
also includes build-related files, but also any other files that happen
to be in your working directory. For example, any output from `git
format-patch` would be removed by this, but not `make clean`.

Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

[Stefan Hajnoczi]

On Fri, Jan 20, 2017 at 03:51:33PM -0800, Brandon Williams wrote:
> On 01/20, Junio C Hamano wrote:
> > Stefan Hajnoczi  writes:
> > 
> > > If the tree contains a sub-directory then git-grep(1) output contains a
> > > colon character instead of a path separator:
> > >
> > >   $ git grep malloc v2.9.3:t
> > >   v2.9.3:t:test-lib.sh:   setup_malloc_check () {
> > >   $ git show v2.9.3:t:test-lib.sh
> > >   fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
> > >
> > > This patch attempts to use the correct delimiter:
> > >
> > >   $ git grep malloc v2.9.3:t
> > >   v2.9.3:t/test-lib.sh:   setup_malloc_check () {
> > >   $ git show v2.9.3:t/test-lib.sh
> > >   (success)
> > >
> > > Signed-off-by: Stefan Hajnoczi 
> > > ---
> > >  builtin/grep.c  | 4 +++-
> > >  t/t7810-grep.sh | 5 +
> > >  2 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/builtin/grep.c b/builtin/grep.c
> > > index 90a4f3d..7a7aab9 100644
> > > --- a/builtin/grep.c
> > > +++ b/builtin/grep.c
> > > @@ -494,7 +494,9 @@ static int grep_object(struct grep_opt *opt, const 
> > > struct pathspec *pathspec,
> > >  
> > >   /* Add a delimiter if there isn't one already */
> > >   if (name[len - 1] != '/' && name[len - 1] != ':') {
> > > - strbuf_addch(, ':');
> > > + /* rev: or rev:path/ */
> > > + char delim = obj->type == OBJ_COMMIT ? ':' : 
> > > '/';
> > 
> > Why check the equality with commit, rather than un-equality with
> > tree?  Wouldn't you want to treat $commit:path and $tag:path the
> > same way?
> 
> I assume Stefan just grabbed my naive suggestion hence why it checks
> equality with a commit.  So that's my fault :)  Either of these may
> not be enough though, since if you do 'git grep malloc v2.9.3^{tree}'
> with this change the output prefix is 'v2.9.3^{tree}/' instead of the
> correct prefix 'v2.9.3^{tree}:'

I revisited this series again today and am coming to the conclusion that
forming output based on the user's rev is really hard to get right in
all cases.  I don't have a good solution to the v2.9.3^{tree} problem.

Perhaps it's better to leave this than to merge code that doesn't work
correctly 100% of the time.

Stefan


signature.asc
Description: PGP signature

Re: subtree merging fails

[Samuel Lijin]

Have you tried using (without -s subtree) -X subtree=path/to/add/subtree/at?

>From the man page:

  subtree[=]
   This option is a more advanced form of subtree
strategy, where the strategy
   makes a guess on how two trees must be shifted to match
with each other when
   merging. Instead, the specified path is prefixed (or
stripped from the
   beginning) to make the shape of two trees to match.

On Tue, Feb 7, 2017 at 2:16 AM, Stavros Liaskos  wrote:
> Following the instructions here:
> https://git-scm.com/book/en/v1/Git-Tools-Subtree-Merging
> will lead to an error.
>
> In particular, if the subtree is merged and then updated, this command
> that is supposed to update the local subtree fails with a fatal:
> refusing to merge unrelated histories error.
>
> $ git merge --squash -s subtree --no-commit rack_branch
>
> A workaround could be using the --allow-unrelated-histories option
>
> $ git merge --squash --allow-unrelated-histories -s subtree
> --no-commit rack_branch
>
> But this completely destroys my project by pushing the subtree
> contents into a completely irrelevant directory in my project (no in
> the subtree).
>
> Any ideas??
>
> https://github.com/git/git-scm.com/issues/896#issuecomment-277587626

Re: Request re git status

[Samuel Lijin]

On Mon, Feb 6, 2017 at 6:45 PM, Phil Hord  wrote:
> On Mon, Feb 6, 2017 at 3:36 PM Ron Pero  wrote:
>> I almost got bit by git: I knew there were changes on the remote
>> server, but git status said I was uptodate with the remote.
>>
>
> Do you mean you almost pushed some changed history with "--force"
> which would have lost others' changes?  Use of this option is
> discouraged on shared branches for this very reason.  But if you do
> use it, the remote will tell you the hash of the old branch so you can
> undo the damage.
>
> But if you did not use --force, then you were not in danger of being
> bit.  Git would have prevented the push in that case.
>
>
>> Why ... not design it to [optionally] DO a fetch and THEN declare
>> whether it is up to date?
>
> It's because `git status` does not talk to the remote server, by
> design.  The only Git commands that do talk to the remote are push,
> pull and fetch.  All the rest work off-line and they do so
> consistently.
>
> Imagine `git status` did what you requested; that is, it first did a
> fetch and then reported the status.  Suppose someone pushed a commit
> to the remote immediately after your fetch completed.  Now git will
> still report "up to date" but it will be wrong as soon as the remote
> finishes adding the new push.  Yet the "up to date" message will
> remain on your console, lying to you.  If you leave and come back in
> two days, the message will remain there even if it is no longer
> correct.
>
> So you should accept that `git status` tells you the status with
> respect to your most recent fetch, and that you are responsible for
> the timing of the most recent fetch.  To have git try to do otherwise
> would be misleading.

This argument doesn't work for me. Race conditions in *any*
asynchronous work flow are inevitable; in commits, particularly to a
shared branch, I also can't imagine them being common. It's like
saying because there's lag between the remote's response and the
output on the local, `git fetch` shouldn't bother saying that the
local remote has been updated.

It wouldn't be hard, though, to define an alias that fetches the
remote-tracking branch and then reports the status.

Nevertheless, this is one of those cases where I think Git suffers
from a poor UI/UX - it's letting the underlying model define the
behavior, rather than using the underlying model to drive the
behavior.

>> Or change the message to tell what it really
>> did, e.g. "Your branch was up-to-date with 'origin/master' when last
>> checked at {timestamp}"? Or even just say, "Do a fetch to find out
>> whether your branch is up to date"?
>
> These are reasonable suggestions, but i don't think the extra wording
> adds anything for most users.  Adding a timestamp seems generally
> useful, but it could get us into other trouble since we have to depend
> on outside sources for timestamps.  :-\

``git clean -xdf'' and ``make clean''

[Hongyi Zhao]

Hi all,

In order to delete all of the last build stuff, does the following two
methods equivalent or not?

``git clean -xdf'' and ``make clean''

Regards
-- 
Hongsheng Zhao 
Institute of Semiconductors, Chinese Academy of Sciences
GnuPG DSA: 0xD108493

[PATCH] rev-list-options.txt: update --all about detached HEAD

[Nguyễn Thái Ngọc Duy]

This is the document patch for f0298cf1c6 (revision walker: include a
detached HEAD in --all - 2009-01-16)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/rev-list-options.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 5da7cf5a8d..72212ac6ec 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -133,8 +133,8 @@ parents) and `--max-parents=-1` (negative numbers denote no 
upper limit).
for all following revision specifiers, up to the next `--not`.
 
 --all::
-   Pretend as if all the refs in `refs/` are listed on the
-   command line as ''.
+   Pretend as if all the refs in `refs/` (and HEAD if detached)
+   are listed on the command line as ''.
 
 --branches[=]::
Pretend as if all the refs in `refs/heads` are listed
-- 
2.11.0.157.gd943d85

Assalamu`Alaikum.

[mohammad ouattara]

Dear Sir/Madam.

Assalamu`Alaikum.

I am Dr mohammad ouattara, I have  ($10.6 Million us dollars) to transfer into 
your account,

I will send you more details about this deal and the procedures to follow when 
I receive a positive response from you, 

Have a great day,
Dr mohammad ouattara.

Re: Git clonebundles

[Johannes Schindelin]

Hi Junio,

On Mon, 6 Feb 2017, Junio C Hamano wrote:

> Christian Couder  writes:
> 
> > There is also Junio's work on Bundle v3 that was unfortunately
> > recently discarded.  Look for "jc/bundle" in:
> >
> > http://public-inbox.org/git/xmqq4m0cry60@gitster.mtv.corp.google.com/
> >
> > and previous "What's cooking in git.git" emails.
> 
> If people think it might be useful to have it around to experiment, I
> can resurrect and keep that in 'pu' (or rather 'jch'), as long as it
> does not overlap and conflict with other topics in flight.  Let me try
> that in today's integration cycle.

I would like to remind you of my suggestion to make this more publicly
visible and substantially easier to play with, by adding it as an
experimental feature (possibly guarded via an explicit opt-in config
setting).

Ciao,
Johannes

Re: [PATCH 2/2] t7800: replace "wc -l" with test_line_count

[Johannes Schindelin]

Hi David,

On Tue, 7 Feb 2017, David Aguilar wrote:

> Make t7800 easier to debug by capturing output into temporary files and
> using test_line_count to make assertions on those files.
> 
> Signed-off-by: David Aguilar 

Both patches look like an obvious improvement with no obvious bugs to me.

In this case, I allowed myself to forego the more thorough code review in
favor of merely glancing over the diffs, seeing as the changes do not
really need a lot of context.

Thank you,
Johannes

[RFC] mailmap.blob overrides default .mailmap

[Cornelius Weig]

Hi,

 I was reading into the mailmap handling today and I'm a bit puzzled by the 
overriding behavior.

This is what the documentation says about precedence (emphasis mine):
-
mailmap.file
The location of an augmenting mailmap file. The default mailmap, located
in the root of the repository, is loaded first, then the mailmap file
pointed to by this variable. The location of the mailmap file may be in a
repository subdirectory, or somewhere outside of the repository itself.
See git-shortlog(1) and git-blame(1).

mailmap.blob
Like mailmap.file, but consider the value as a reference to a blob in the
repository. If both mailmap.file and mailmap.blob are given, both are
!!! parsed, with _entries from mailmap.file taking precedence_. In a bare
repository, this defaults to HEAD:.mailmap. In a non-bare repository, it
defaults to empty.


So from the doc I would have expected that files always get precedence over the 
blob. IOW entries from .mailmap override entries from mailmap.blob. However, 
this is not the case.

The code shows why (mailmap.c):
err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
if (startup_info->have_repository)
err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);


Apparently this is not an oversight, because there is an explicit test for this 
overriding behavior (t4203 'mailmap.blob overrides .mailmap').

So I wonder: what is the rationale behind this? I find this mixed overriding 
behavior hard to explain and difficult to understand.

Re: Google Doc about the Contributors' Summit

[Johannes Schindelin]

Hi team,

On Thu, 2 Feb 2017, Johannes Schindelin wrote:

> I just started typing stuff up in a Google Doc, and made it editable to
> everyone, feel free to help and add things:
> 
> https://docs.google.com/document/d/1KDoSn4btbK5VJCVld32he29U0pUeFGhpFxyx9ZJDDB0/edit?usp=sharing

I am terribly sorry... yesterday I simply tried to restrict editing so
that nobody would just spam the document, but in my haste I even disabled
viewing.

The link is functional again, sorry for that.

Thanks, Tim, for reporting the problem!

Ciao,
Dscho

Non-zero exit code without error

[Serdar Sahin]

Hi,

When we execute the following lines, the exit code is 1, but it is
unclear what is the reason of this exit code. Do you have any idea?

git clone --mirror --depth 50 --no-single-branch
g...@github.hede.com:Casual/hodo-server.git
Cloning into bare repository 'hodo-server.git'...
remote: Counting objects: 3371, done.
remote: Compressing objects: 100% (1219/1219), done.
remote: Total 3371 (delta 2344), reused 2971 (delta 2098), pack-reused 0
Receiving objects: 100% (3371/3371), 56.77 MiB | 2.18 MiB/s, done.
Resolving deltas: 100% (2344/2344), done.

echo $?
0

cd hodo-server.git/

GIT_CURL_VERBOSE=1 GIT_TRACE=1  git fetch --depth 50 origin
cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee
14:12:35.215889 git.c:350   trace: built-in: git 'fetch'
'--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
14:12:35.217273 run-command.c:336   trace: run_command: 'ssh'
'g...@github.hede.com' 'git-upload-pack '\''Casual/hodo-server.git'\'''
14:12:37.301122 run-command.c:336   trace: run_command: 'gc' '--auto'
14:12:37.301866 exec_cmd.c:189  trace: exec: 'git' 'gc' '--auto'
14:12:37.304473 git.c:350   trace: built-in: git 'gc' '--auto'

echo $?
1

Re: [PATCH] difftool: fix bug when printing usage

[Johannes Schindelin]

Hi Junio,

On Mon, 6 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > [... and he quoted someone ...]
> >
> >> +  # create a ceiling directory to prevent Git from finding a repo
> >> +  mkdir -p not/repo &&
> >> +  ceiling="$PWD/not" &&
> >> +  lines=$(cd not/repo &&
> >> +  GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h |
> >> +  grep ^usage: | wc -l) &&
> >> +  test "$lines" -eq 1 &&
> >
> > Likewise, this would become
> >
> > GIT_CEILING_DIRECTORIES="$PWD/not" \
> > test_expect_code 129 git -C not/repo difftool -h >output &&
> > grep ^usage: output
> 
> I agree with the intent, but the execution here is "Not quite".
> test_expect_code being a shell function, it does not take the
> "one-shot environment assignment for this single invocation," like
> external commands do.

So now that we know what is wrong, can you please enlighten me about what
is right?

Thanks,
Johannes

Re: [PATCH] worktree: fix option descriptions for `prune`

[Duy Nguyen]

On Tue, Feb 7, 2017 at 1:59 AM, Junio C Hamano  wrote:
> Patrick Steinhardt  writes:
>
>>   struct option options[] = {
>>   OPT__DRY_RUN(_only, N_("do not remove, show only")),
>> - OPT__VERBOSE(, N_("report pruned objects")),
>> + OPT__VERBOSE(, N_("report pruned working trees")),
>>   OPT_EXPIRY_DATE(0, "expire", ,
>> - N_("expire objects older than ")),
>> + N_("expire working trees older than ")),
>
> Thanks for sharp eyes.

Yep.

This message never made it to git@vger right, because I didn't see it?
Oh no, gmail classified the original mail as spam again...

-- 
Duy

Re: git-daemon shallow checkout fail

[Duy Nguyen]

On Tue, Feb 7, 2017 at 7:56 AM, Bob Proulx  wrote:
> Bob Proulx wrote:
>>   17:20:40.590177 pkt-line.c:46   packet:clone< 
>> 34e7202270c381f4e5734180dc19426ce69f2e1e HEAD\0multi_ack thin-pack side-band 
>> side-band-64k ofs-delta shallow no-progress include-tag multi_ack_detailed 
>> symref=HEAD:refs/heads/master agent=git/1.9.1
>
> The evidence of it running the wrong version being the 1.9.1 which is
> not the bits I built.

I wonder if we should make this "git/1.9.1" information more visible. We could

1) Always print it when cloning
2) Print it when cloning with -v (printing all capabilities with -v
might not be a bad idea)
3) Include it in error messages when clone/fetch fails
-- 
Duy

[PATCH 2/2] t7800: replace "wc -l" with test_line_count

[David Aguilar]

Make t7800 easier to debug by capturing output into temporary files and
using test_line_count to make assertions on those files.

Signed-off-by: David Aguilar 
---
 t/t7800-difftool.sh | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 97bae54d83..25241f4096 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -290,8 +290,8 @@ test_expect_success 'difftool + mergetool config variables' 
'
 test_expect_success 'difftool..path' '
test_config difftool.tkdiff.path echo &&
git difftool --tool=tkdiff --no-prompt branch >output &&
-   lines=$(grep file output | wc -l) &&
-   test "$lines" -eq 1
+   grep file output >grep-output &&
+   test_line_count = 1 grep-output
 '
 
 test_expect_success 'difftool --extcmd=cat' '
@@ -428,9 +428,12 @@ run_dir_diff_test 'difftool --dir-diff branch from 
subdirectory' '
git difftool --dir-diff $symlinks --extcmd ls branch >output &&
# "sub" must only exist in "right"
# "file" and "file2" must be listed in both "left" and "right"
-   test "1" = $(grep sub output | wc -l) &&
-   test "2" = $(grep file"$" output | wc -l) &&
-   test "2" = $(grep file2 output | wc -l)
+   grep sub output > sub-output &&
+   test_line_count = 1 sub-output &&
+   grep file"$" output >file-output &&
+   test_line_count = 2 file-output &&
+   grep file2 output >file2-output &&
+   test_line_count = 2 file2-output
)
 '
 
@@ -440,9 +443,11 @@ run_dir_diff_test 'difftool --dir-diff v1 from 
subdirectory' '
git difftool --dir-diff $symlinks --extcmd ls v1 >output &&
# "sub" and "file" exist in both v1 and HEAD.
# "file2" is unchanged.
-   test "2" = $(grep sub output | wc -l) &&
-   test "2" = $(grep file output | wc -l) &&
-   test "0" = $(grep file2 output | wc -l)
+   grep sub output >sub-output &&
+   test_line_count = 2 sub-output &&
+   grep file output >file-output &&
+   test_line_count = 2 file-output &&
+   ! grep file2 output
)
 '
 
@@ -452,8 +457,9 @@ run_dir_diff_test 'difftool --dir-diff branch from 
subdirectory w/ pathspec' '
git difftool --dir-diff $symlinks --extcmd ls branch -- 
.>output &&
# "sub" only exists in "right"
# "file" and "file2" must not be listed
-   test "1" = $(grep sub output | wc -l) &&
-   test "0" = $(grep file output | wc -l)
+   grep sub output >sub-output &&
+   test_line_count = 1 sub-output &&
+   ! grep file output
)
 '
 
@@ -463,8 +469,9 @@ run_dir_diff_test 'difftool --dir-diff v1 from subdirectory 
w/ pathspec' '
git difftool --dir-diff $symlinks --extcmd ls v1 -- .>output &&
# "sub" exists in v1 and HEAD
# "file" is filtered out by the pathspec
-   test "2" = $(grep sub output | wc -l) &&
-   test "0" = $(grep file output | wc -l)
+   grep sub output >sub-output &&
+   test_line_count = 2 sub-output &&
+   ! grep file output
)
 '
 
-- 
2.12.0.rc0.228.g6c028b8e94

[PATCH 1/2] t7800: simplify basic usage test

[David Aguilar]

Use "test_line_count" instead of "wc -l", use "git -C" instead of a
subshell, and use test_expect_code when calling difftool.  Ease
debugging by capturing output into temporary files.

Suggested-by: Johannes Schindelin 
Signed-off-by: David Aguilar 
---
This patch applies on top of js/difftool-builtin in next
"difftool: fix bug when printing usage"

 t/t7800-difftool.sh | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 21e2ac4ad6..97bae54d83 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -24,16 +24,15 @@ prompt_given ()
 }
 
 test_expect_success 'basic usage requires no repo' '
-   lines=$(git difftool -h | grep ^usage: | wc -l) &&
-   test "$lines" -eq 1 &&
+   test_expect_code 129 git difftool -h >output &&
+   grep ^usage: output &&
# create a ceiling directory to prevent Git from finding a repo
mkdir -p not/repo &&
-   ceiling="$PWD/not" &&
-   lines=$(cd not/repo &&
-   GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h |
-   grep ^usage: | wc -l) &&
-   test "$lines" -eq 1 &&
-   rmdir -p not/repo
+   test_when_finished rm -r not &&
+   test_expect_code 129 \
+   env GIT_CEILING_DIRECTORIES="$(pwd)/not" \
+   git -C not/repo difftool -h >output &&
+   grep ^usage: output
 '
 
 # Create a file on master and change it on branch
-- 
2.12.0.rc0.228.g6c028b8e94

Re: [PATCH v2 0/7] completion bash: add more options and commands

[Johannes Sixt]

Am 06.02.2017 um 22:29 schrieb Junio C Hamano:

cornelius.w...@tngtech.com writes:


From: Cornelius Weig 

This is the re-roll of patch series 
<2017015724.19360-1-cornelius.w...@tngtech.com>.

This patch series adds all long-options that are mentioned in the synopsis of
the man-page for the respective git-command. There are only a few exceptions,
as discussed in the above thread. For example, no unsafe options should be
completed.
Furthermore, the patches add subommand option completion for git-submodule and
git-remote.


Reviewers, do these look good now?


My concerns have been addressed. The patches look good from a cursory read.

-- Hannes

subtree merging fails

[Stavros Liaskos]

Following the instructions here:
https://git-scm.com/book/en/v1/Git-Tools-Subtree-Merging
will lead to an error.

In particular, if the subtree is merged and then updated, this command
that is supposed to update the local subtree fails with a fatal:
refusing to merge unrelated histories error.

$ git merge --squash -s subtree --no-commit rack_branch

A workaround could be using the --allow-unrelated-histories option

$ git merge --squash --allow-unrelated-histories -s subtree
--no-commit rack_branch

But this completely destroys my project by pushing the subtree
contents into a completely irrelevant directory in my project (no in
the subtree).

Any ideas??

https://github.com/git/git-scm.com/issues/896#issuecomment-277587626