* Re: git-daemon shallow checkout fail
From: Bob Proulx @ 2017-02-07 22:49 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CACsJy8D_X7u+kttu=ZD734u6nhR=wjMh0m99RBvm0_FOW74pWA@mail.gmail.com>
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
^ permalink raw reply
* The --name-only option for git log does not play nicely with --graph
From: Davide Del Vento @ 2017-02-07 23:11 UTC (permalink / raw)
To: git
`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
^ permalink raw reply
* Fwd: Possibly nicer pathspec syntax?
From: Linus Torvalds @ 2017-02-07 23:12 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CA+55aFyznf1k=iyiQx6KLj3okpid0-HexZWsVkxt7LqCdz+O5A@mail.gmail.com>
[ 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
^ permalink raw reply
* Re: Trying to use xfuncname without success.
From: René Scharfe @ 2017-02-07 23:18 UTC (permalink / raw)
To: Jack Adrian Zappa; +Cc: git-mailing-list
In-Reply-To: <CAKepmajNz7TP_Z6p_Wj17tOpiMOpKkvQOBvVthBkEiKabAppjg@mail.gmail.com>
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 ]*<Type[\\\t ]+Name=\"([^\"])\".*$"
The extra "\\" allow backslashes to be used for indentation as well as
between Type and Name, which is probably not what you want. And your
expression only matches single-char Name attributes. Try:
xfuncname = "^[\t ]*<Type[\t ]+Name=\"([^\"]+)\".*$"
René
^ permalink raw reply
* Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)
From: Jonathan Tan @ 2017-02-07 23:53 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, peff
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
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, &new_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, &new_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
^ permalink raw reply related
* Re: git/git-scm.com GH Issue Tracker
From: Samuel Lijin @ 2017-02-08 0:33 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20170206183429.sd7255b3ovvg746c@sigill.intra.peff.net>
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 <peff@peff.net> 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
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Junio C Hamano @ 2017-02-08 0:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFzkTZAb1vy3G5M_Nb1BeOhTiCGksUfLa+ZQtiU2x6Q=Fw@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> 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 <torvalds@linux-foundation.org> 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 ;-)
^ permalink raw reply
* Re: "disabling bitmap writing, as some objects are not being packed"?
From: David Turner @ 2017-02-08 1:03 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Git Mailing List, Jeff King
In-Reply-To: <CACsJy8ACy+Hv1Z3FgG-WFBozwWqmMuN-JnMWF-+rdpF0knFjqg@mail.gmail.com>
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?
^ permalink raw reply
* [PATCH] push options: fail properly in the stateless case
From: Stefan Beller @ 2017-02-08 1:09 UTC (permalink / raw)
To: gitster; +Cc: git, jrnieder, 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 <sbeller@google.com>
---
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 <c-string>::
+ 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
^ permalink raw reply related
* Re: Fwd: Possibly nicer pathspec syntax?
From: Linus Torvalds @ 2017-02-08 1:48 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
In-Reply-To: <CA+55aFzkTZAb1vy3G5M_Nb1BeOhTiCGksUfLa+ZQtiU2x6Q=Fw@mail.gmail.com>
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)
^ permalink raw reply related
* RE: [RFC] Add support for downloading blobs on demand
From: Ben Peart @ 2017-02-08 2:18 UTC (permalink / raw)
To: 'Jakub Narębski', 'Christian Couder'
Cc: 'Jeff King', 'git', 'Johannes Schindelin',
'Ben Peart'
In-Reply-To: <04cdd7ae-3349-470f-39c6-7f8723fdcae8@gmail.com>
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:jnareb@gmail.com]
> Sent: Tuesday, February 7, 2017 4:57 PM
> To: Ben Peart <peartben@gmail.com>; 'Christian Couder'
> <christian.couder@gmail.com>
> Cc: 'Jeff King' <peff@peff.net>; 'git' <git@vger.kernel.org>; 'Johannes
> Schindelin' <Johannes.Schindelin@gmx.de>; Ben Peart
> <benpeart@microsoft.com>
> 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
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Junio C Hamano @ 2017-02-08 2:42 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702071739060.17609@i7.lan>
Linus Torvalds <torvalds@linux-foundation.org> 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)
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Linus Torvalds @ 2017-02-08 2:49 UTC (permalink / raw)
To: Mike Hommey; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20170208024042.trmkjm4jnxidcflg@glandium.org>
On Tue, Feb 7, 2017 at 6:40 PM, Mike Hommey <mh@glandium.org> 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
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Linus Torvalds @ 2017-02-08 3:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqefz9xv0x.fsf@gitster.mtv.corp.google.com>
On Tue, Feb 7, 2017 at 6:42 PM, Junio C Hamano <gitster@pobox.com> 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
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Mike Hommey @ 2017-02-08 3:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CA+55aFxTwK=+oJT_zujKLWEho9CoL6u6LTLDEP+wzjFDx=JQyQ@mail.gmail.com>
On Tue, Feb 07, 2017 at 06:49:24PM -0800, Linus Torvalds wrote:
> On Tue, Feb 7, 2017 at 6:40 PM, Mike Hommey <mh@glandium.org> 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
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Mike Hommey @ 2017-02-08 2:40 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.LFD.2.20.1702071739060.17609@i7.lan>
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
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Junio C Hamano @ 2017-02-08 3:12 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFwPLtuPciN1o_03CwkKqFWgZd_br9Q14qyr7a7N7mxTeA@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> 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.
^ permalink raw reply
* Re: Fwd: Possibly nicer pathspec syntax?
From: Linus Torvalds @ 2017-02-08 3:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqa89xxtnd.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]
On Tue, Feb 7, 2017 at 7:12 PM, Junio C Hamano <gitster@pobox.com> 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
[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1293 bytes --]
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)
^ permalink raw reply related
* Re: Fwd: Possibly nicer pathspec syntax?
From: Junio C Hamano @ 2017-02-08 4:42 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFyAEaMKA+2oPJct4ffJ0-_z2vrYxmQ9yrkbxzB3Hk6WfQ@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> 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 ;-)
^ permalink raw reply
* [PATCH 1/2] pathspec magic: add '^' as alias for '!'
From: Linus Torvalds @ 2017-02-08 5:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
From: Linus Torvalds <torvalds@linux-foundation.org>
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 <torvalds@linux-foundation.org>
---
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
^ permalink raw reply related
* Re: Fwd: Possibly nicer pathspec syntax?
From: Linus Torvalds @ 2017-02-08 5:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqq4m05xph4.fsf@gitster.mtv.corp.google.com>
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
^ permalink raw reply
* [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
From: Linus Torvalds @ 2017-02-08 5:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
From: Linus Torvalds <torvalds@linux-foundation.org>
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 <torvalds@linux-foundation.org>
---
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
^ permalink raw reply related
* [PATCH v2] rev-list-options.txt: update --all about HEAD
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 6:06 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170207133850.14056-1-pclouds@gmail.com>
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 <pclouds@gmail.com>
---
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 '<commit>'.
+ Pretend as if all the refs in `refs/`, along with `HEAD`, are
+ listed on the command line as '<commit>'.
--branches[=<pattern>]::
Pretend as if all the refs in `refs/heads` are listed
--
2.11.0.157.gd943d85
^ permalink raw reply related
* Re: [PATCH] dir: avoid allocation in fill_directory()
From: Duy Nguyen @ 2017-02-08 6:22 UTC (permalink / raw)
To: René Scharfe; +Cc: Brandon Williams, Git List
In-Reply-To: <73ec9cc7-8a86-8ba9-90fd-a954fa031820@web.de>
On Wed, Feb 8, 2017 at 5:04 AM, René Scharfe <l.s.r@web.de> 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
^ permalink raw reply
* Re: Request re git status
From: Duy Nguyen @ 2017-02-08 6:13 UTC (permalink / raw)
To: Jacob Keller; +Cc: Samuel Lijin, Phil Hord, Ron Pero, Git
In-Reply-To: <CA+P7+xoZHOtURfbBbHHTpC3DsGxaGOVToqmW5wTg2EniRpL-Cg@mail.gmail.com>
On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller <jacob.keller@gmail.com> 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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox