* Re: [PATCH v2 2/2] grep: use '/' delimiter for paths
From: Junio C Hamano @ 2017-02-07 20:37 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Brandon Williams, git, Jeff King
In-Reply-To: <xmqq8tphzr41.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> 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/&y/;
> 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/.
^ permalink raw reply
* [PATCH/RFC v2] WIP: log: allow "-" as a short-hand for "previous branch"
From: Siddharth Kannan @ 2017-02-07 20:51 UTC (permalink / raw)
To: git
Cc: gitster, pranit.bauva, Matthieu.Moy, peff, pclouds, sandals,
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 <kannan.siddharth12@gmail.com>
---
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 0000000..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 actual.last_empty
+'
+
+test_expect_success 'symmetric revision range should work when both ends are given' '
+ git checkout testing-2 &&
+ git checkout master &&
+ git log -...testing-1 >expect &&
+ git log testing-2...testing-1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'asymmetric revision range should work when both ends are given' '
+ git checkout testing-2 &&
+ git checkout master &&
+ git log -..testing-1 >expect &&
+ git log testing-2..testing-1 >actual &&
+ test_cmp expect actual
+'
+test_done
--
2.1.4
^ permalink raw reply related
* Re: Git clonebundles
From: Junio C Hamano @ 2017-02-07 20:54 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Christian Couder, Shawn Pearce, Stefan Saasen, Git Mailing List
In-Reply-To: <alpine.DEB.2.20.1702071303370.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.
^ permalink raw reply
* git push - 401 unauthorized
From: Alessio Rocchi @ 2017-02-07 20:47 UTC (permalink / raw)
To: git
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
^ permalink raw reply
* Re: [RFC] Add support for downloading blobs on demand
From: Jakub Narębski @ 2017-02-07 21:56 UTC (permalink / raw)
To: Ben Peart, 'Christian Couder'
Cc: 'Jeff King', 'git', 'Johannes Schindelin',
Ben Peart
In-Reply-To: <002701d2816e$f4682fa0$dd388ee0$@gmail.com>
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
* [PATCH] dir: avoid allocation in fill_directory()
From: René Scharfe @ 2017-02-07 22:04 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git List, Duy Nguyen
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 <l.s.r@web.de>
---
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
^ permalink raw reply related
* Re: [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-02-07 22:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Johannes Schindelin, Brandon Williams, Johannes Sixt,
Git List
In-Reply-To: <xmqqd1f1pxqc.fsf@gitster.mtv.corp.google.com>
Am 01.02.2017 um 19:33 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> 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é
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Junio C Hamano @ 2017-02-07 22:30 UTC (permalink / raw)
To: René Scharfe
Cc: Jeff King, Johannes Schindelin, Brandon Williams, Johannes Sixt,
Git List
In-Reply-To: <12e7db44-ff69-a38e-322a-6b5fc5f1fc29@web.de>
René Scharfe <l.s.r@web.de> 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.
^ permalink raw reply
* Re: [RFC] mailmap.blob overrides default .mailmap
From: Cornelius Weig @ 2017-02-07 22:45 UTC (permalink / raw)
To: Jeff King, Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <20170207192801.qoncjaqjpn3axpyn@sigill.intra.peff.net>
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:peff@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 'peff@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.
^ permalink raw reply
* 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
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