* Re: [PATCH v2 03/11] submodule deinit: use most reliable url
From: Brandon Williams @ 2017-03-09 18:15 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGZ79kZG+Y+AtYYxeE5hmsNxfwmNp1h7aKAem=JD3yiKB6STPw@mail.gmail.com>
On 03/08, Stefan Beller wrote:
> On Wed, Mar 8, 2017 at 5:23 PM, Brandon Williams <bmwill@google.com> wrote:
> > The user could have configured the submodule to have a different URL
> > from the one in the superproject's config. To account for this read
> > what the submodule has configured for remote.origin.url and use that
> > instead.
>
> When reading this commit message, I first thought this is an unrelated
> bug fix. However it is not unrelated. In a later patch you introduce a mode
> where submodule.<name>.url may not exist in .git/config any more,
> (there may be a .existence flag instead?) such that url="", which is
> obviously a bad UI:
So the later patch doesn't actually omit submodule.<name>.url but we
could end up with this if you init and then deinit without actually
cloning the submodule. So maybe the best bet is to drop this patch from
the series and we can include one like it in a follow up which uses a
helper function that you suggest.
>
> Submodule '$name' (<empty string>) unregistered for path '$displaypath'"
>
> Like the first patch of this series, we could have a helper function
> "git submodule--helper url <name>" that figures out how to get the URL:
> 1) Look into that GIT_DIR
> 2) if non-existent check .git/config for the URL or
> 3) fall back to .gitmodules?
>
> Instead of having such a helper, we directly look into that first option
> hoping it exists, however it doesn't have to:
>
> git submodule init <ps>
> # no command in between
> git submodule deinit <ps>
> # submodule was not cloned yet, but we still test positive for
> > # Remove the .git/config entries (unless the user already did it)
> > if test -n "$(git config --get-regexp submodule."$name\.")"
>
> I am not sure if there is an easy way out here.
>
> Thinking about the name for such a url helper lookup, I wonder if
> we want to have
>
> git submodule--helper show-url <name>
>
> as potentially we end up having this issue for a lot
> of different things (e.g. submodule.<name>.shallow = (true,false),
> or in case the submodule is cloned you can give the actual depth
> as an integral number), so maybe we'd want to introduce one
> layer of indirection
>
> git submodule--helper ask-property \
> (is-active/URL/depth/size/..) <name>
>
> So with that said, I wonder if we also want to ease up:
>
> GIT_DIR="$(git rev-parse --git-path modules/$name
>
> to be
>
> GIT_DIR=$(git submodule--helper show-git-dir $name)
>
> > then
> > # Remove the whole section so we have a clean state when
> > # the user later decides to init this submodule again
> > - url=$(git config submodule."$name".url)
> > + url=$(GIT_DIR="$(git rev-parse --git-path modules/$name)" git config remote.origin.url)
>
> In the submodule helper we have get_default_remote(), which we do not
> have in shell
> (which we seemed to have in shell?), so maybe it's worth not using origin here,
> although I guess it is rare that the original remote is named other than origin.
>
> > git config --remove-section submodule."$name" 2>/dev/null &&
> > say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
> > fi
> > --
> > 2.12.0.246.ga2ecc84866-goog
> >
>
> Thanks,
> Stefan
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 02/10] pack-objects: add --partial-by-size=n --partial-special
From: Jeff Hostetler @ 2017-03-09 18:26 UTC (permalink / raw)
To: Jeff King, Jeff Hostetler; +Cc: git, gitster, markbt, benpeart, jonathantanmy
In-Reply-To: <20170309073117.g3br5btsfwntcdpe@sigill.intra.peff.net>
On 3/9/2017 2:31 AM, Jeff King wrote:
> On Wed, Mar 08, 2017 at 05:37:57PM +0000, Jeff Hostetler wrote:
>
>> From: Jeff Hostetler <git@jeffhostetler.com>
>>
>> Teach pack-objects to omit blobs from the generated packfile.
>>
>> When the --partial-by-size=n[kmg] argument is used, only blobs
>> smaller than the requested size are included. When n is zero,
>> no blobs are included.
>>
>> When the --partial-special argument is used, git special files,
>> such as ".gitattributes" and ".gitignores" are included.
>>
>> When both are given, the union of two are included.
>
> I understand why one would want to do:
>
> --partial-by-size=100 --partial-special
>
> and get the union. The first one restricts, and the second one adds back
> in. But I don't understand why "--partial-special" by itself makes any
> sense. Wouldn't we already be including all blobs, and it would be a
> noop?
My thought was that the "--partial-special" when used by itself
would *only* give you the .git* files (and if we had something
like a .gitsparse/ directory, everything under it). The client
could then do a "special" clone -- mainly to get the sparse checkout
templates under .gitsparse/ and then come back for a sparse fetch
using one of them. Somewhat of a chicken-n-egg problem, unless the
user knows the template names in advance.
>
>
> Also, I was thinking a bit on Junio's comment elsewhere on whether
> read_object_list_from_stdin() should do the same limiting. I think the
> answer is "probably not", because whoever is generating that object list
> can cull the set. You could do it today with something like:
>
> git rev-list --objects HEAD |
> git cat-file --batch-check='%(objectsize) %(objecttype) %(objectname) %(rest)' |
> perl -lne 's/^(\d+) (\S+) //; print if $2 ne "blob" || $1 < 100' |
> git pack-objects
>
> But if we are going to add this --partial-by-size for the pack-objects
> traversal, shouldn't we just add it to rev-list? Then:
>
> git rev-list --objects --partial-by-size=100 --partial-special |
> git pack-objects
>
> works, and you should get it in the pack-objects basically for free (I
> think you'd have to allow through the "--partial" arguments on stdin,
> and make sure the rev-list implementation is done via
> traverse_commit_list).
>
> As a bonus, I suspect it would make the --partial-special path-handling
> easier, because you'd see each tree entry rather than the fully
> constructed path (so no more monkeying around with "/").
Interesting. Let me give that a try and see what it looks like.
>
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index 7e052bb..2df2f49 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -77,6 +77,10 @@ static unsigned long cache_max_small_delta_size = 1000;
>>
>> static unsigned long window_memory_limit = 0;
>>
>> +static signed long partial_by_size = -1;
>
> I would have expected this to be an off_t, though I think
> OPT_MAGNITUDE() forces you into "unsigned long". I guess it is nothing
> new for Git; we use "unsigned long" for single object sizes elsewhere,
> so systems with a 32-bit long are out of luck anyway until we fix that.
>
> The signed "long" here is unfortunate, as it limits us to 2G on such
> systems. Maybe it is not worth worrying too much about. The "big object"
> threshold is usually around 500MB. I think the failure behavior is not
> great, though (asking for "3G" would go negative and effectively be
> ignored).
>
> I think handling all cases would involve swapping out OPT_MAGNITUDE()
> for a special callback that writes the "yes, the user set this" bit in a
> separate variable.
Yeah, there is a bit of confusion there. I used OPT_MAGNITUDE in
one place (for the argument checking), but couldn't in another place.
And I tried to pass the original string across the wire for sanity.
And I had to fight with the types a little. It would probably be
simpler to replace that with a custom handler (or a uint64_t version
of magnitude) that would do the right thing and then use that numeric
value elsewhere.
Thanks,
Jeff
^ permalink raw reply
* Re: [PATCH 04/10] upload-pack: add partial (sparse) fetch
From: Jeff Hostetler @ 2017-03-09 18:34 UTC (permalink / raw)
To: Jeff King, Jeff Hostetler; +Cc: git, gitster, markbt, benpeart, jonathantanmy
In-Reply-To: <20170309074849.ktl5vqbzkiwwwbob@sigill.intra.peff.net>
On 3/9/2017 2:48 AM, Jeff King wrote:
> On Wed, Mar 08, 2017 at 05:37:59PM +0000, Jeff Hostetler wrote:
>
>> diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
>> index c59ac99..0032729 100644
>> --- a/Documentation/technical/pack-protocol.txt
>> +++ b/Documentation/technical/pack-protocol.txt
>> @@ -212,6 +212,7 @@ out of what the server said it could do with the first 'want' line.
>> upload-request = want-list
>> *shallow-line
>> *1depth-request
>> + *partial
>> flush-pkt
>>
>> want-list = first-want
>> @@ -223,10 +224,15 @@ out of what the server said it could do with the first 'want' line.
>> PKT-LINE("deepen-since" SP timestamp) /
>> PKT-LINE("deepen-not" SP ref)
>>
>> + partial = PKT-LINE("partial-by-size" SP magnitude) /
>> + PKT-LINE("partial-special)
>> +
>
> I probably would have added this as a capability coming back from the
> client, since it only makes sense to send once (the same way we ask for
> other features like include-tag or ofs-delta). I guess it's six of one,
> half a dozen of the other, though.
True. I wanted the size argument. And later want to add
a sparse-file. It seemed like they better belonged in a
PKT-LINE than in the capability header.
>
> I notice that you require the client to request the "partial" capability
> _and_ to send these commands. I'm not sure what the client capability
> response is helping. The server has said "I can do this" and the client
> either asks for it or not.
Yeah, I wasn't sure if that was necessary or not.
It looked like there were other fields where the
client advised that it wanted to used a capability
that the server had previously advertised. If we
don't need it, that's fine.
>
>> + if (skip_prefix(line, "partial-by-size ", &arg)) {
>> + unsigned long s;
>> + if (!client_requested_partial_capability)
>> + die("git upload-pack: 'partial-by-size' option requires 'partial' capability");
>> + if (!git_parse_ulong(arg, &s))
>> + die("git upload-pack: invalid partial-by-size value: %s", line);
>> + strbuf_addstr(&partial_by_size, "--partial-by-size=");
>> + strbuf_addstr(&partial_by_size, arg);
>> + have_partial_by_size = 1;
>> + continue;
>
> So we parse it here for validation, but then pass the original string on
> to be parsed again by pack-objects. I think I'd rather see us use the
> result of our parse here, just to avoid any bugs where the parsing isn't
> identical (and there is such a bug currently due to the signed/unsigned
> thing I mentioned).
>
> I also wonder whether the magnitude suffixes are worth exposing across
> the wire. Anybody touching the list of units in git_parse_ulong() would
> probably be surprised that the protocol is dependent on them (not that I
> expect us to really take any away, but it just seems like an unnecessary
> protocol complication).
Yeah, I'll change this as I described in an earlier sub-thread.
Jeff
^ permalink raw reply
* Re: [PATCH 06/10] rev-list: add --allow-partial option to relax connectivity checks
From: Jeff Hostetler @ 2017-03-09 18:38 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Jeff Hostetler, git, markbt, benpeart,
jonathantanmy
In-Reply-To: <20170309075642.jy5o353ann524k7f@sigill.intra.peff.net>
On 3/9/2017 2:56 AM, Jeff King wrote:
> On Wed, Mar 08, 2017 at 03:10:54PM -0500, Jeff Hostetler wrote:
>
>>> Even though I do very much like the basic "high level" premise to
>>> omit often useless large blobs that are buried deep in the history
>>> we would not necessarily need from the initial cloning and
>>> subsequent fetches, I find it somewhat disturbing that the code
>>> "Assume"s that any missing blob is due to an previous partial clone.
>>> Adding this option smells like telling the users that they are not
>>> supposed to run "git fsck" because a partially cloned repository is
>>> inherently a corrupt repository.
>>>
>>> Can't we do a bit better? If we want to make the world safer again,
>>> what additional complexity is required to allow us to tell the
>>> "missing by design" and "corrupt repository" apart?
>>
>> I'm open to suggestions here. It would be nice to extend the
>> fetch-pack/upload-pack protocol to return a list of the SHAa
>> (and maybe the sizes) of the omitted blobs, so that a partial
>> clone or fetch would still be able to be integrity checked.
>
> Yeah, the early external-odb patches did this. It lets you do a more
> accurate fsck, and it also helps diff avoid faulting in large-object
> cases (because we can mark them as binary for "free" by comparing the
> size to big_file_threshold).
>
> So I think it makes a lot of sense in the large-blob case, where
> transmitting a type/size/sha1 tuple is way more efficient than sending
> the blob itself. But it's less clear for "sparse" cases where just
> enumerating the set of blobs may be prohibitively large.
>
> I have a feeling that the "sparse" thing needs to be handled separately
> from "partial". IOW, the client needs to tell the server "I'm only
> interested in the path foo/bar, so just send that". Then you don't find
> out about the types and sizes outside of that path, but you don't need
> to; the sparse path is stored locally and fsck knows to avoid looking
> into it.
>
> -Peff
>
That makes sense. I'd like to get both concepts (by-size/special vs
sparse-file) in, but they don't really overlap that much (internally).
So I could see doing this in 2 separate efforts.
Thanks,
Jeff
^ permalink raw reply
* Re: [PATCH] repack: Add options to preserve and prune old pack files
From: Martin Fick @ 2017-03-09 18:45 UTC (permalink / raw)
To: jmelvin; +Cc: Junio C Hamano, git, nasserg, peff, sbeller
In-Reply-To: <1d816bbb08b228ece9a74ffcdfb7a5b1@codeaurora.org>
On Thursday, March 09, 2017 10:50:21 AM
jmelvin@codeaurora.org wrote:
> On 2017-03-07 13:33, Junio C Hamano wrote:
> > James Melvin <jmelvin@codeaurora.org> writes:
> >> These options are designed to prevent stale file handle
> >> exceptions during git operations which can happen on
> >> users of NFS repos when repacking is done on them. The
> >> strategy is to preserve old pack files around until
> >> the next repack with the hopes that they will become
> >> unreferenced by then and not cause any exceptions to
> >> running processes when they are finally deleted
> >> (pruned).
> >
> > I find it a very sensible strategy to work around NFS,
> > but it does not explain why the directory the old ones
> > are moved to need to be configurable. It feels to me
> > that a boolean that causes the old ones renamed
> > s/^pack-/^old-&/ in the same directory (instead of
> > pruning them right away) would risk less chances of
> > mistakes (e.g. making "preserved" subdirectory on a
> > separate device mounted there in a hope to reduce disk
> > usage of the primary repository, which may defeat the
> > whole point of moving the still-active file around
> > instead of removing them).
>
> Moving the preserved pack files to a separate directory
> only helped make the pack directory cleaner, but I agree
> that having the old* pack files in the same directory is
> a better approach as it would ensure that it's still on
> the same mounted device. I'll update the logic to reflect
> that.
>
> As for the naming convention of the preserved pack files,
> there is already some logic to remove "old-" files in
> repack. Currently this is the naming convention I have
> for them:
>
> pack-<sha1>.old-<ext>
> pack-7412ee739b8a20941aa1c2fd03abcc7336b330ba.old-pack
>
> One advantage of that is the extension is no longer an
> expected one, differentiating it from current pack files.
>
> That said, if that is not a concern, I could prefix them
> with "preserved" instead of "old" to differentiate them
> from the other logic that cleans up "old-*". What are
> your thoughts on that?
>
> preserved-<sha1>.<ext>
> preserved-7412ee739b8a20941aa1c2fd03abcc7336b330ba.pack
Some other proposals so that the preserved files do not get
returned by naive finds based on their extensions,
preserved-<sha1>.<ext>-preserved
preserved-7412ee739b8a20941aa1c2fd03abcc7336b330ba.pack-
preserved
or:
preserved-<sha1>.preserved-<ext>
preserved-7412ee739b8a20941aa1c2fd03abcc7336b330ba.preserved-
pack
or maybe even just:
preserved-<ext>-<sha1>
preserved-pack-7412ee739b8a20941aa1c2fd03abcc7336b330ba
-Martin
--
The Qualcomm Innovation Center, Inc. is a member of Code
Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* [PATCH] t2027: avoid using pipes
From: Prathamesh Chavan @ 2017-03-09 19:03 UTC (permalink / raw)
To: git; +Cc: christian.couder, jdl, Prathamesh
From: Prathamesh <pc44800@gmail.com>
Whenever a git command is present in the upstream of a pipe, its failure
gets masked by piping and hence it should be avoided for testing the
upstream git command. By writing out the output of the git command to
a file, we can test the exit codes of both the commands as a failure exit
code in any command is able to stop the && chain.
Signed-off-by: Prathamesh <pc44800@gmail.com>
---
t/t2027-worktree-list.sh | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 848da5f36..d8b3907e0 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -31,7 +31,8 @@ test_expect_success '"list" all worktrees from main' '
test_when_finished "rm -rf here && git worktree prune" &&
git worktree add --detach here master &&
echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git worktree list | sed "s/ */ /g" >actual &&
+ git worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -40,7 +41,8 @@ test_expect_success '"list" all worktrees from linked' '
test_when_finished "rm -rf here && git worktree prune" &&
git worktree add --detach here master &&
echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C here worktree list | sed "s/ */ /g" >actual &&
+ git -C here worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -73,7 +75,8 @@ test_expect_success '"list" all worktrees from bare main' '
git -C bare1 worktree add --detach ../there master &&
echo "$(pwd)/bare1 (bare)" >expect &&
echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C bare1 worktree list | sed "s/ */ /g" >actual &&
+ git -C bare1 worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -96,7 +99,8 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
git -C bare1 worktree add --detach ../there master &&
echo "$(pwd)/bare1 (bare)" >expect &&
echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C there worktree list | sed "s/ */ /g" >actual &&
+ git -C there worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -118,9 +122,9 @@ test_expect_success 'broken main worktree still at the top' '
cd linked &&
echo "worktree $(pwd)" >expected &&
echo "ref: .broken" >../.git/HEAD &&
- git worktree list --porcelain | head -n 3 >actual &&
+ git worktree list --porcelain >out && head -n 3 out >actual &&
test_cmp ../expected actual &&
- git worktree list | head -n 1 >actual.2 &&
+ git worktree list >out && head -n 1 out >actual.2 &&
grep -F "(error)" actual.2
)
'
@@ -134,7 +138,7 @@ test_expect_success 'linked worktrees are sorted' '
test_commit new &&
git worktree add ../first &&
git worktree add ../second &&
- git worktree list --porcelain | grep ^worktree >actual
+ git worktree list --porcelain >out && grep ^worktree out >actual
) &&
cat >expected <<-EOF &&
worktree $(pwd)/sorted/main
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 04/10] upload-pack: add partial (sparse) fetch
From: Jeff King @ 2017-03-09 19:09 UTC (permalink / raw)
To: Jeff Hostetler
Cc: Jeff Hostetler, git, gitster, markbt, benpeart, jonathantanmy
In-Reply-To: <ce170fec-846b-fc51-221d-c005b4a42b97@jeffhostetler.com>
On Thu, Mar 09, 2017 at 01:34:32PM -0500, Jeff Hostetler wrote:
> > > + partial = PKT-LINE("partial-by-size" SP magnitude) /
> > > + PKT-LINE("partial-special)
> > > +
> >
> > I probably would have added this as a capability coming back from the
> > client, since it only makes sense to send once (the same way we ask for
> > other features like include-tag or ofs-delta). I guess it's six of one,
> > half a dozen of the other, though.
>
> True. I wanted the size argument. And later want to add
> a sparse-file. It seemed like they better belonged in a
> PKT-LINE than in the capability header.
Yeah, at some point we will run out of room in the capabilities
response. :) If the sparse information might be arbitrarily long (e.g.,
a list of pathspecs) then it probably is better for it to get split into
its own pktline (or even a series of pktlines).
-Peff
^ permalink raw reply
* Re: RFC v3: Another proposed hash function transition plan
From: Shawn Pearce @ 2017-03-09 19:14 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Linus Torvalds, Git Mailing List, Stefan Beller, bmwill,
Jonathan Tan, Jeff King, David Lang, brian m. carlson
In-Reply-To: <20170307001709.GC26789@aiede.mtv.corp.google.com>
On Mon, Mar 6, 2017 at 4:17 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Linus Torvalds wrote:
>> On Fri, Mar 3, 2017 at 5:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> This document is still in flux but I thought it best to send it out
>>> early to start getting feedback.
>>
>> This actually looks very reasonable if you can implement it cleanly
>> enough.
>
> Thanks for the kind words on what had quite a few flaws still. Here's
> a new draft. I think the next version will be a patch against
> Documentation/technical/.
FWIW, I like this approach.
> Alongside the packfile, a sha3 repository stores a bidirectional
> mapping between sha3 and sha1 object names. The mapping is generated
> locally and can be verified using "git fsck". Object lookups use this
> mapping to allow naming objects using either their sha1 and sha3 names
> interchangeably.
I saw some discussion about using LevelDB for this mapping table. I
think any existing database may be overkill.
For packs, you may be able to simplify by having only one file
(pack-*.msha1) that maps SHA-1 to pack offset; idx v2. The CRC32 table
in v2 is unnecessary, but you need the 64 bit offset support.
SHA-1 to SHA-3: lookup SHA-1 in .msha1, reverse .idx, find offset to
read the SHA-3.
SHA-3 to SHA-1: lookup SHA-3 in .idx, and reverse the .msha1 file to
translate offset to SHA-1.
For loose objects, the loose object directories should have only
O(4000) entries before auto gc is strongly encouraging
packing/pruning. With 256 shards, each given directory has O(16) loose
objects in it. When writing a SHA-3 loose object, Git could also
append a line "$sha3 $sha1\n" to objects/${first_byte}/sha1, which
GC/prune rewrites to remove entries. With O(16) objects in a
directory, these files should only have O(16) entries in them.
SHA-3 to SHA-1: open objects/${sha3_first_byte}/sha1 and scan until a
match is found.
SHA-1 to SHA-3: brute force read 256 files. Callers performing this
mapping may load all 256 files into a table in memory.
^ permalink raw reply
* [PATCH] t2027: avoid using pipes
From: Prathamesh Chavan @ 2017-03-09 19:18 UTC (permalink / raw)
To: christian.couder; +Cc: jdl, git, pc44800
In-Reply-To: <CAP8UFD19njU30HODYvp1pddpZaVSVGgn7whcTa2rdjMPe-vzYQ@mail.gmail.com>
From: Prathamesh <pc44800@gmail.com>
Whenever a git command is present in the upstream of a pipe, its failure
gets masked by piping and hence it should be avoided for testing the
upstream git command. By writing out the output of the git command to
a file, we can test the exit codes of both the commands as a failure exit
code in any command is able to stop the && chain.
Signed-off-by: Prathamesh <pc44800@gmail.com>
---
t/t2027-worktree-list.sh | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 848da5f36..d8b3907e0 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -31,7 +31,8 @@ test_expect_success '"list" all worktrees from main' '
test_when_finished "rm -rf here && git worktree prune" &&
git worktree add --detach here master &&
echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git worktree list | sed "s/ */ /g" >actual &&
+ git worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -40,7 +41,8 @@ test_expect_success '"list" all worktrees from linked' '
test_when_finished "rm -rf here && git worktree prune" &&
git worktree add --detach here master &&
echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C here worktree list | sed "s/ */ /g" >actual &&
+ git -C here worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -73,7 +75,8 @@ test_expect_success '"list" all worktrees from bare main' '
git -C bare1 worktree add --detach ../there master &&
echo "$(pwd)/bare1 (bare)" >expect &&
echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C bare1 worktree list | sed "s/ */ /g" >actual &&
+ git -C bare1 worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -96,7 +99,8 @@ test_expect_success '"list" all worktrees from linked with a bare main' '
git -C bare1 worktree add --detach ../there master &&
echo "$(pwd)/bare1 (bare)" >expect &&
echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
- git -C there worktree list | sed "s/ */ /g" >actual &&
+ git -C there worktree list >out &&
+ sed "s/ */ /g" <out >actual &&
test_cmp expect actual
'
@@ -118,9 +122,9 @@ test_expect_success 'broken main worktree still at the top' '
cd linked &&
echo "worktree $(pwd)" >expected &&
echo "ref: .broken" >../.git/HEAD &&
- git worktree list --porcelain | head -n 3 >actual &&
+ git worktree list --porcelain >out && head -n 3 out >actual &&
test_cmp ../expected actual &&
- git worktree list | head -n 1 >actual.2 &&
+ git worktree list >out && head -n 1 out >actual.2 &&
grep -F "(error)" actual.2
)
'
@@ -134,7 +138,7 @@ test_expect_success 'linked worktrees are sorted' '
test_commit new &&
git worktree add ../first &&
git worktree add ../second &&
- git worktree list --porcelain | grep ^worktree >actual
+ git worktree list --porcelain >out && grep ^worktree out >actual
) &&
cat >expected <<-EOF &&
worktree $(pwd)/sorted/main
--
2.11.0
^ permalink raw reply related
* [RFC PATCH] help: add optional instructions for reporting bugs
From: Stefan Beller @ 2017-03-09 19:30 UTC (permalink / raw)
To: git; +Cc: Stefan Beller
Git is distributed in various ways by various organizations. The Git
community prefers to have bugs reported on the mailing list, whereas
other organizations may rather want to have filed a bug in a bug tracker
or such. The point of contact is different by organization as well.
When reporting bugs, users will usually look at the output of
'git --version' at one point to write a quality bug report.
So that is a good spot to provide additional information to the user
about e.g. additional the organizational quirks how to report a bug.
As the output of 'git --version' is parsed by scripts as well,
we only want to present this information to users, which is why
we only give the output to TTYs.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Not sure if this is a good idea at all, hence RFC.
Thanks,
Stefan
help.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/help.c b/help.c
index bc6cd19cf3..d4c1fbe5cb 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,12 @@
#include "version.h"
#include "refs.h"
+#ifdef GIT_BUG_REPORT_HELP
+const char git_bug_reporting_string[] = GIT_BUG_REPORT_HELP;
+#else
+const char git_bug_reporting_string[] = "To report bugs send a plain text email to git@vger.kernel.org";
+#endif
+
void add_cmdname(struct cmdnames *cmds, const char *name, int len)
{
struct cmdname *ent;
@@ -435,6 +441,8 @@ int cmd_version(int argc, const char **argv, const char *prefix)
/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
}
}
+ if (isatty(1))
+ puts(git_bug_reporting_string);
return 0;
}
--
2.12.0.190.g6e60aba09d.dirty
^ permalink raw reply related
* [gsoc] regarding microproject
From: sourav mondal @ 2017-03-09 19:49 UTC (permalink / raw)
To: git
hello,
I want "add more built in patterns for userdiff " as my microproject. Hopefully I'll add some patterns for well known languages.
thanks & regards,
sourav
^ permalink raw reply
* [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref
From: Ævar Arnfjörð Bjarmason @ 2017-03-09 20:02 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Lars Hjemli, Jeff King, Christian Couder,
Ævar Arnfjörð Bjarmason
In-Reply-To: <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>
Change the tag, branch & for-each-ref commands to have a --no-contains
option in addition to their longstanding --contains options.
The use-case I have for this is to find the last-good rollout tag
given a known-bad <commit>. Right now, given a hypothetically bad
commit v2.10.1-3-gcf5c7253e0, you can find which git version to revert
to with this hacky two-liner:
(./git tag -l 'v[0-9]*'; ./git tag -l 'v[0-9]*' --contains v2.10.1-3-gcf5c7253e0) \
|sort|uniq -c|grep -E '^ *1 '|awk '{print $2}' | tail -n 10
But with the --no-contains option you can now get the exact same
output with:
./git tag -l 'v[0-9]*' --no-contains v2.10.1-3-gcf5c7253e0|sort|tail -n 10
The filtering machinery is generic between the tag, branch &
for-each-ref commands, so once I'd implemented it for tag it was
trivial to add support for this to the other two.
A practical use for this with "branch" is e.g. finding branches which
diverged between 2.8 & 2.10:
./git branch --contains v2.8.0 --no-contains v2.10.0
The "describe" command also has a --contains option, but its semantics
are unrelated to what tag/branch/for-each-ref use --contains for. I
don't see how a --no-contains option for "describe" would make any
sense, other than being exactly equivalent to not supplying --contains
at all, which would be confusing at best.
I'm adding a --without option to "tag" as an alias for --no-contains
for consistency with --with and --contains. Since we don't even
document --with anymore (or test it). The --with option is
undocumented, and possibly the only user of it is Junio[1]. But it's
trivial to support, so let's do that.
Where I'm changing existing documentation lines I'm mainly word
wrapping at 75 columns to be consistent with the existing style. The
changes to Documentation/ are much smaller with: git show --word-diff,
same for the minor change to git-completion.bash.
Most of the test changes I've made are just doing the inverse of the
existing --contains tests, with this change --no-contains for tag,
branch & for-each-ref is just as well tested as the existing
--contains option.
In addition to those tests I've added tests for --contains in
combination with --no-contains for all three commands, and a test for
"tag" which asserts that --no-contains won't find tree/blob tags,
which is slightly unintuitive, but consistent with how --contains
works.
When --contains and --no-contains are provided to "tag" we disable the
optimized code to find tags added in v1.7.2-rc1-1-gffc4b8012d. That
code changes flags on commit objects as an optimization, and thus
can't be called twice.
Jeff King has a WIP patch to fix that[2], but rather than try to
incorporate that into this already big patch I'm just disabling the
optimized codepath. Using both --contains and --no-contains will most
likely be rare, and we can get an initial correct version working &
optimize it later.
It's possible that the --merge & --no-merge codepaths for "tag" have a
similar bug, but as of writing I can't produce any evidence of that
via a brute-force test script[3].
1. <xmqqefy71iej.fsf@gitster.mtv.corp.google.com>
2. <20170309125132.tubwxtneffok4nrc@sigill.intra.peff.net>
3. <CACBZZX4W7brFe5ecTvQPMmf4X5_zH+dw3cB5xeVg+2hWYps0Ug@mail.gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Changes since v1:
* Now git-for-each-ref has --no-contains
* Doc fixes, happy with the NOTES section in git-branch.txt now.
* Now tag --contains works with --no-contains, but falls back on
the much slower pre-1.8 era code pending efforts to make
commit_contains() re-callable.
* Now `git tag --no-contains HEAD hi` dies, just like `git tag
--contains HEAD hi` does.
* Fixed up my shitty C in ref-filter.c as Peff suggested.
* Tests for --contains combined with --no-contains for all the
commands.
* Rewrote the very verbose t7004-tag.sh tag test Peff complied
about.
Documentation/git-branch.txt | 24 ++++---
Documentation/git-for-each-ref.txt | 6 +-
Documentation/git-tag.txt | 6 +-
builtin/branch.c | 4 +-
builtin/for-each-ref.c | 1 +
builtin/tag.c | 15 +++-
contrib/completion/git-completion.bash | 9 +--
parse-options.h | 4 +-
ref-filter.c | 16 +++--
ref-filter.h | 1 +
t/t3201-branch-contains.sh | 51 +++++++++++++-
t/t6302-for-each-ref-filter.sh | 16 +++++
t/t7004-tag.sh | 123 ++++++++++++++++++++++++++++++++-
13 files changed, 249 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 092f1bcf9f..28a36a8a0a 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git branch' [--color[=<when>] | --no-color] [-r | -a]
[--list] [-v [--abbrev=<length> | --no-abbrev]]
[--column[=<options>] | --no-column]
- [(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
+ [(--merged | --no-merged | --contains | --no-contains) [<commit>]] [--sort=<key>]
[--points-at <object>] [--format=<format>] [<pattern>...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
@@ -35,11 +35,12 @@ as branch creation.
With `--contains`, shows only the branches that contain the named commit
(in other words, the branches whose tip commits are descendants of the
-named commit). With `--merged`, only branches merged into the named
-commit (i.e. the branches whose tip commits are reachable from the named
-commit) will be listed. With `--no-merged` only branches not merged into
-the named commit will be listed. If the <commit> argument is missing it
-defaults to `HEAD` (i.e. the tip of the current branch).
+named commit), `--no-contains` inverts it. With `--merged`, only branches
+merged into the named commit (i.e. the branches whose tip commits are
+reachable from the named commit) will be listed. With `--no-merged` only
+branches not merged into the named commit will be listed. If the <commit>
+argument is missing it defaults to `HEAD` (i.e. the tip of the current
+branch).
The command's second form creates a new branch head named <branchname>
which points to the current `HEAD`, or <start-point> if given.
@@ -213,6 +214,10 @@ start-point is either a local or remote-tracking branch.
Only list branches which contain the specified commit (HEAD
if not specified). Implies `--list`.
+--no-contains [<commit>]::
+ Only list branches which don't contain the specified commit
+ (HEAD if not specified). Implies `--list`.
+
--merged [<commit>]::
Only list branches whose tips are reachable from the
specified commit (HEAD if not specified). Implies `--list`.
@@ -296,13 +301,16 @@ If you are creating a branch that you want to checkout immediately, it is
easier to use the git checkout command with its `-b` option to create
a branch and check it out with a single command.
-The options `--contains`, `--merged` and `--no-merged` serve three related
-but different purposes:
+The options `--contains`, `--no-contains`, `--merged` and `--no-merged`
+serve four related but different purposes:
- `--contains <commit>` is used to find all branches which will need
special attention if <commit> were to be rebased or amended, since those
branches contain the specified <commit>.
+- `--no-contains <commit>` is the inverse of that, i.e. branches that don't
+ contain the specified <commit>.
+
- `--merged` is used to find all branches which can be safely deleted,
since those branches are fully contained by HEAD.
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 111e1be6f5..83b93c75a8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
[(--sort=<key>)...] [--format=<format>] [<pattern>...]
[--points-at <object>] [(--merged | --no-merged) [<object>]]
- [--contains [<object>]]
+ [(--contains | --no-contains) [<object>]]
DESCRIPTION
-----------
@@ -79,6 +79,10 @@ OPTIONS
Only list refs which contain the specified commit (HEAD if not
specified).
+--no-contains [<object>]::
+ Only list refs which don't contain the specified commit (HEAD
+ if not specified).
+
--ignore-case::
Sorting and filtering refs are case insensitive.
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 525737a5d8..4938496194 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
<tagname> [<commit> | <object>]
'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
+'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
'git tag' -v [--format=<format>] <tagname>...
@@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
Only list tags which contain the specified commit (HEAD if not
specified).
+--no-contains [<commit>]::
+ Only list tags which don't contain the specified commit (HEAD if
+ not secified).
+
--points-at <object>::
Only list tags of the given object.
diff --git a/builtin/branch.c b/builtin/branch.c
index 94f7de7fa5..e8d534604c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPT_SET_INT('r', "remotes", &filter.kind, N_("act on remote-tracking branches"),
FILTER_REFS_REMOTES),
OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
+ OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
+ OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
OPT__ABBREV(&filter.abbrev),
OPT_GROUP(N_("Specific git-branch actions:")),
@@ -604,7 +606,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
list = 1;
- if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
+ if (filter.with_commit || filter.no_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr)
list = 1;
if (!!delete + !!rename + !!new_upstream +
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index df41fa0350..b1ae2388e6 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
OPT_MERGED(&filter, N_("print only refs that are merged")),
OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+ OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")),
OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
OPT_END(),
};
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..d83674e3e6 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -53,7 +53,16 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
}
verify_ref_format(format);
- filter->with_commit_tag_algo = 1;
+ if (filter->with_commit && filter->no_commit)
+ /* Due to the way the contains_tag_algo() function
+ touches object.flags we can only call it once
+ per-process.
+
+ For now sacrifice performance for correctness when
+ both --contains and --no-contains are provided */
+ filter->with_commit_tag_algo = 0;
+ else
+ filter->with_commit_tag_algo = 1;
filter_refs(&array, filter, FILTER_REFS_TAGS);
ref_array_sort(sorting, &array);
@@ -424,7 +433,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_GROUP(N_("Tag listing options")),
OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
+ OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
+ OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
OPT_MERGED(&filter, N_("print only tags that are merged")),
OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
@@ -488,6 +499,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("-n option is only allowed with -l."));
if (filter.with_commit)
die(_("--contains option is only allowed with -l."));
+ if (filter.no_commit)
+ die(_("--no-contains option is only allowed with -l."));
if (filter.points_at.nr)
die(_("--points-at option is only allowed with -l."));
if (filter.merge_commit)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fc32286a43..fa3da49478 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1093,9 +1093,9 @@ _git_branch ()
--*)
__gitcomp "
--color --no-color --verbose --abbrev= --no-abbrev
- --track --no-track --contains --merged --no-merged
- --set-upstream-to= --edit-description --list
- --unset-upstream --delete --move --remotes
+ --track --no-track --contains --no-contains --merged
+ --no-merged --set-upstream-to= --edit-description
+ --list --unset-upstream --delete --move --remotes
--column --no-column --sort= --points-at
"
;;
@@ -2862,7 +2862,8 @@ _git_tag ()
__gitcomp "
--list --delete --verify --annotate --message --file
--sign --cleanup --local-user --force --column --sort=
- --contains --points-at --merged --no-merged --create-reflog
+ --contains --no-contains --points-at --merged
+ --no-merged --create-reflog
"
;;
esac
diff --git a/parse-options.h b/parse-options.h
index dcd8a0926c..0eac90b510 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,7 +258,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);
PARSE_OPT_LASTARG_DEFAULT | flag, \
parse_opt_commits, (intptr_t) "HEAD" \
}
-#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
#define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
+#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
#endif
diff --git a/ref-filter.c b/ref-filter.c
index 1ec0fb8391..b6dab75729 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1571,11 +1571,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
return contains_test(candidate, want);
}
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
+static int commit_contains(struct ref_filter *filter, struct commit_list *list, struct commit *commit)
{
if (filter->with_commit_tag_algo)
- return contains_tag_algo(commit, filter->with_commit);
- return is_descendant_of(commit, filter->with_commit);
+ return contains_tag_algo(commit, list);
+ return is_descendant_of(commit, list);
}
/*
@@ -1765,13 +1765,17 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
* obtain the commit using the 'oid' available and discard all
* non-commits early. The actual filtering is done later.
*/
- if (filter->merge_commit || filter->with_commit || filter->verbose) {
+ if (filter->merge_commit || filter->with_commit || filter->no_commit || filter->verbose) {
commit = lookup_commit_reference_gently(oid->hash, 1);
if (!commit)
return 0;
- /* We perform the filtering for the '--contains' option */
+ /* We perform the filtering for the '--contains' option... */
if (filter->with_commit &&
- !commit_contains(filter, commit))
+ !commit_contains(filter, filter->with_commit, commit))
+ return 0;
+ /* ...or for the `--no-contains' option */
+ if (filter->no_commit &&
+ commit_contains(filter, filter->no_commit, commit))
return 0;
}
diff --git a/ref-filter.h b/ref-filter.h
index 154e24c405..af85eb4592 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -53,6 +53,7 @@ struct ref_filter {
const char **name_patterns;
struct sha1_array points_at;
struct commit_list *with_commit;
+ struct commit_list *no_commit;
enum {
REF_FILTER_MERGED_NONE = 0,
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index 7f3ec47241..506415dbd3 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='branch --contains <commit>, --merged, and --no-merged'
+test_description='branch --contains <commit>, --no-contains <commit> --merged, and --no-merged'
. ./test-lib.sh
@@ -45,6 +45,22 @@ test_expect_success 'branch --contains master' '
'
+test_expect_success 'branch --no-contains=master' '
+
+ git branch --no-contains=master >actual &&
+ >expect &&
+ test_cmp expect actual
+
+'
+
+test_expect_success 'branch --no-contains master' '
+
+ git branch --no-contains master >actual &&
+ >expect &&
+ test_cmp expect actual
+
+'
+
test_expect_success 'branch --contains=side' '
git branch --contains=side >actual &&
@@ -55,6 +71,16 @@ test_expect_success 'branch --contains=side' '
'
+test_expect_success 'branch --no-contains=side' '
+
+ git branch --no-contains=side >actual &&
+ {
+ echo " master"
+ } >expect &&
+ test_cmp expect actual
+
+'
+
test_expect_success 'branch --contains with pattern implies --list' '
git branch --contains=master master >actual &&
@@ -65,6 +91,14 @@ test_expect_success 'branch --contains with pattern implies --list' '
'
+test_expect_success 'branch --no-contains with pattern implies --list' '
+
+ git branch --no-contains=master master >actual &&
+ >expect &&
+ test_cmp expect actual
+
+'
+
test_expect_success 'side: branch --merged' '
git branch --merged >actual &&
@@ -126,7 +160,9 @@ test_expect_success 'branch --no-merged with pattern implies --list' '
test_expect_success 'implicit --list conflicts with modification options' '
test_must_fail git branch --contains=master -d &&
- test_must_fail git branch --contains=master -m foo
+ test_must_fail git branch --contains=master -m foo &&
+ test_must_fail git branch --no-contains=master -d &&
+ test_must_fail git branch --no-contains=master -m foo
'
@@ -159,4 +195,15 @@ test_expect_success 'branch --merged with --verbose' '
test_i18ncmp expect actual
'
+test_expect_success 'branch --contains combined with --no-contains' '
+ git branch --contains zzz --no-contains topic >actual &&
+ cat >expect <<-\EOF &&
+ master
+ side
+ zzz
+ EOF
+ test_cmp expect actual
+
+'
+
test_done
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index a09a1a46ef..4902ba5f16 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -93,6 +93,22 @@ test_expect_success 'filtering with --contains' '
test_cmp expect actual
'
+test_expect_success 'filtering with --no-contains' '
+ cat >expect <<-\EOF &&
+ refs/tags/one
+ EOF
+ git for-each-ref --format="%(refname)" --no-contains=two >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'filtering with --contains and --no-contains' '
+ cat >expect <<-\EOF &&
+ refs/tags/two
+ EOF
+ git for-each-ref --format="%(refname)" --contains=two --no-contains=three >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '%(color) must fail' '
test_must_fail git for-each-ref --format="%(color)%(refname)"
'
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..8ad5719962 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1385,6 +1385,23 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
test_cmp expected actual
"
+# All the --contains tests above, but with --no-contains
+test_expect_success 'checking that first commit is not listed in any tag with --no-contains (hash)' "
+ >expected &&
+ git tag -l --no-contains $hash1 v* >actual &&
+ test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (tag)' "
+ git tag -l --no-contains v1.0 v* >actual &&
+ test_cmp expected actual
+"
+
+test_expect_success 'checking that first commit is in all tags (relative)' "
+ git tag -l --no-contains HEAD~2 v* >actual &&
+ test_cmp expected actual
+"
+
cat > expected <<EOF
v2.0
EOF
@@ -1394,6 +1411,17 @@ test_expect_success 'checking that second commit only has one tag' "
test_cmp expected actual
"
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+EOF
+
+test_expect_success 'inverse of the last test, with --no-contains' "
+ git tag -l --no-contains $hash2 v* >actual &&
+ test_cmp expected actual
+"
cat > expected <<EOF
EOF
@@ -1403,6 +1431,19 @@ test_expect_success 'checking that third commit has no tags' "
test_cmp expected actual
"
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'conversely --no-contains on the third commit lists all tags' "
+ git tag -l --no-contains $hash3 v* >actual &&
+ test_cmp expected actual
+"
+
# how about a simple merge?
test_expect_success 'creating simple branch' '
@@ -1424,6 +1465,19 @@ test_expect_success 'checking that branch head only has one tag' "
test_cmp expected actual
"
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+EOF
+
+test_expect_success 'checking that branch head with --no-contains lists all but one tag' "
+ git tag -l --no-contains $hash4 v* >actual &&
+ test_cmp expected actual
+"
+
test_expect_success 'merging original branch into this branch' '
git merge --strategy=ours master &&
git tag v4.0
@@ -1445,6 +1499,20 @@ v1.0.1
v1.1.3
v2.0
v3.0
+EOF
+
+test_expect_success 'checking that original branch head with --no-contains lists all but one tag now' "
+ git tag -l --no-contains $hash3 v* >actual &&
+ test_cmp expected actual
+"
+
+cat > expected <<EOF
+v0.2.1
+v1.0
+v1.0.1
+v1.1.3
+v2.0
+v3.0
v4.0
EOF
@@ -1453,6 +1521,12 @@ test_expect_success 'checking that initial commit is in all tags' "
test_cmp expected actual
"
+test_expect_success 'checking that initial commit is in all tags with --no-contains' "
+ >expected &&
+ git tag -l --no-contains $hash1 v* >actual &&
+ test_cmp expected actual
+"
+
# mixing modes and options:
test_expect_success 'mixing incompatibles modes and options is forbidden' '
@@ -1709,7 +1783,7 @@ run_with_limited_stack () {
test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
# we require ulimit, this excludes Windows
-test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
>expect &&
i=1 &&
while test $i -lt 8000
@@ -1725,7 +1799,9 @@ EOF"
git checkout master &&
git tag far-far-away HEAD^ &&
run_with_limited_stack git tag --contains HEAD >actual &&
- test_cmp expect actual
+ test_cmp expect actual &&
+ run_with_limited_stack git tag --no-contains HEAD >actual &&
+ test_line_count ">" 70 actual
'
test_expect_success '--format should list tags as per format given' '
@@ -1773,4 +1849,47 @@ test_expect_success 'ambiguous branch/tags not marked' '
test_cmp expect actual
'
+test_expect_success '--contains combined with --no-contains' '
+ (
+ git init no-contains &&
+ cd no-contains &&
+ test_commit v0.1 &&
+ test_commit v0.2 &&
+ test_commit v0.3 &&
+ test_commit v0.4 &&
+ test_commit v0.5 &&
+ cat >expected <<-\EOF &&
+ v0.2
+ v0.3
+ v0.4
+ EOF
+ git tag --contains v0.2 --no-contains v0.5 >actual &&
+ test_cmp expected actual
+ )
+'
+
+# As the docs say, list tags which contain a specified *commit*. We
+# don't recurse down to tags for trees or blobs pointed to by *those*
+# commits.
+test_expect_success 'Does --[no-]contains stop at commits? Yes!' '
+ cd no-contains &&
+ blob=$(git rev-parse v0.3:v0.3.t) &&
+ tree=$(git rev-parse v0.3^{tree}) &&
+ git tag tag-blob $blob &&
+ git tag tag-tree $tree &&
+ git tag --contains v0.3 >actual &&
+ cat >expected <<-\EOF &&
+ v0.3
+ v0.4
+ v0.5
+ EOF
+ test_cmp expected actual &&
+ git tag --no-contains v0.3 >actual &&
+ cat >expected <<-\EOF &&
+ v0.1
+ v0.2
+ EOF
+ test_cmp expected actual
+'
+
test_done
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] t2027: avoid using pipes
From: Christian Couder @ 2017-03-09 20:15 UTC (permalink / raw)
To: Prathamesh Chavan; +Cc: git, Jon Loeliger
In-Reply-To: <20170309190310.30589-1-pc44800@gmail.com>
On Thu, Mar 9, 2017 at 8:03 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> From: Prathamesh <pc44800@gmail.com>
>
> Whenever a git command is present in the upstream of a pipe, its failure
> gets masked by piping and hence it should be avoided for testing the
> upstream git command. By writing out the output of the git command to
> a file, we can test the exit codes of both the commands as a failure exit
> code in any command is able to stop the && chain.
>
> Signed-off-by: Prathamesh <pc44800@gmail.com>
> ---
When you post a new version of a patch or a patch series, could you do
the following:
- add v2 or v3, or ... after "PATCH" in the subject, so that we know
which version it is (see the mailing list archive and the format-patch
documentation to see how it should appear and how to do it)
- tell what you changed since the previous version, and maybe also why
you made those changes, if it has not already been explained or
discussed (you can do it after the "---" above and before the file
stats below, or in a separate email replying to the patch, or in the
cover letter of the patch series if you are sending a patch series)
> t/t2027-worktree-list.sh | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
> index 848da5f36..d8b3907e0 100755
> --- a/t/t2027-worktree-list.sh
> +++ b/t/t2027-worktree-list.sh
> @@ -31,7 +31,8 @@ test_expect_success '"list" all worktrees from main' '
> test_when_finished "rm -rf here && git worktree prune" &&
> git worktree add --detach here master &&
> echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
> - git worktree list | sed "s/ */ /g" >actual &&
> + git worktree list >out &&
> + sed "s/ */ /g" <out >actual &&
> test_cmp expect actual
> '
[...]
> @@ -118,9 +122,9 @@ test_expect_success 'broken main worktree still at the top' '
> cd linked &&
> echo "worktree $(pwd)" >expected &&
> echo "ref: .broken" >../.git/HEAD &&
> - git worktree list --porcelain | head -n 3 >actual &&
> + git worktree list --porcelain >out && head -n 3 out >actual &&
> test_cmp ../expected actual &&
> - git worktree list | head -n 1 >actual.2 &&
> + git worktree list >out && head -n 1 out >actual.2 &&
I think it would be better if the 'head' commands above and the 'grep'
command below were also on their own line.
> grep -F "(error)" actual.2
> )
> '
> @@ -134,7 +138,7 @@ test_expect_success 'linked worktrees are sorted' '
> test_commit new &&
> git worktree add ../first &&
> git worktree add ../second &&
> - git worktree list --porcelain | grep ^worktree >actual
> + git worktree list --porcelain >out && grep ^worktree out >actual
> ) &&
> cat >expected <<-EOF &&
> worktree $(pwd)/sorted/main
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH 00/10] RFC Partial Clone and Fetch
From: Jonathan Tan @ 2017-03-09 20:18 UTC (permalink / raw)
To: git, git; +Cc: jeffhost, peff, gitster, markbt, benpeart
In-Reply-To: <1488999039-37631-1-git-send-email-git@jeffhostetler.com>
Overall, this fetch/clone approach seems reasonable to me, except
perhaps some unanswered questions (some of which are also being
discussed elsewhere):
- does the server need to tell us of missing blobs?
- if yes, does the server need to tell us their file sizes?
- do we need to store the list of missing blobs somewhere (whether the
server told it to us or whether we inferred it from the fetched
trees)
The answers to this probably depend on the answers in "B. Issues
Backfilling Omitted Blobs" (especially the additional concepts I listed
below).
Also, do you have any plans to implement other functionality, e.g. "git
checkout" (which will allow fetches and clones to repositories with a
working directory)? (I don't know what the mailing list consensus would
be for the "acceptance criteria" for this patch set, but I would at
least include "checkout".)
On 03/08/2017 10:50 AM, git@jeffhostetler.com wrote:
> B. Issues Backfilling Omitted Blobs
> ===================================
>
> Ideally, if the client only does "--partial-by-profile" fetches, it
> should not need to fetch individual missing blobs, but we have to allow
> for it to handle the other commands and other unexpected issues.
>
> There are 3 orthogonal concepts here: when, how and where?
Another concept is "how to determine if a blob is really omitted" - do
we store a list somewhere or do we assume that all missing blobs are
purposely omitted (like in this patch set)?
Yet another concept is "whether to fetch" - for example, a checkout
should almost certainly fetch, but a rev-list used by a connectivity
check (like in patch 6 of this set) should not.
For example, for historical-blob-searching commands like "git log -S",
should we:
a) fetch everything missing (so users should use date-limiting
arguments)
b) fetch nothing missing
c) use the file size to automatically exclude big files, but fetch
everything else
For a) and b), we wouldn't need file size information for missing blobs,
but for c), we do. This might determine if we need file size information
in the fetch-pack/upload-pack protocol.
> C. New Blob-Fetch Protocol (2a)
> ===============================
>
> *TODO* A new pair of commands, such as fetch-blob-pack and upload-blob-pack,
> will be created to let the client request a batch of blobs and receive a
> packfile. A protocol similar to the fetch-pack/upload-pack will be spoken
> between them. (This avoids complicating the existing protocol and the work
> of enumerating the refs.) Upload-blob-pack will use pack-objects to build
> the packfile.
>
> It is also more efficient than requesting a single blob at a time using
> the existing fetch-pack/upload-pack mechanism (with the various allow
> unreachable options).
>
> *TODO* The new request protocol will be defined in the patch series.
> It will include: a list of the desired blob SHAs. Possibly also the commit
> SHA, branch name, and pathname of each blob (or whatever is necessary to let
> the server address the reachability concerns). Possibly also the last
> known SHA for each blob to allow for deltafication in the packfile.
Context (like the commit SHA-1) would help in reachability checks, but
I'm not sure if we can rely on that. It is true that I can't think of a
way that the client would dissociate a blob that is missing from its
tree or commit (because it would first need to "fault-in" that blob to
do its operation). But clients operating on non-contextual SHA-1s (e.g.
"git cat-file") and servers manipulating commits (so that the commit
SHA-1 that the client had in its context is no longer reachable) are not
uncommon, I think.
Having said that, it might be useful to include the context in the
protocol anyway as an optional "hint".
I'm not sure what you mean by "last known SHA for each blob".
(If we do store the file size of a blob somewhere, we could also store
some context there. I'm not sure how useful this is, though.)
> E. Unresolved Thoughts
> ======================
<snip>
> *TODO* The partial clone arguments should be recorded in ".git/info/"
> so that subsequent fetch commands can inherit them and rev-list/index-pack
> know to not complain by default.
>
> *TODO* Update GC like rev-list to not complain when there are missing blobs.
These 2 points would be part of "whether to fetch" above.
^ permalink raw reply
* Re: RFC v3: Another proposed hash function transition plan
From: Jonathan Nieder @ 2017-03-09 20:24 UTC (permalink / raw)
To: Shawn Pearce
Cc: Linus Torvalds, Git Mailing List, Stefan Beller, bmwill,
Jonathan Tan, Jeff King, David Lang, brian m. carlson
In-Reply-To: <CAJo=hJtoX9=AyLHHpUJS7fueV9ciZ_MNpnEPHUz8Whui6g9F0A@mail.gmail.com>
Hi,
Shawn Pearce wrote:
> On Mon, Mar 6, 2017 at 4:17 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Alongside the packfile, a sha3 repository stores a bidirectional
>> mapping between sha3 and sha1 object names. The mapping is generated
>> locally and can be verified using "git fsck". Object lookups use this
>> mapping to allow naming objects using either their sha1 and sha3 names
>> interchangeably.
>
> I saw some discussion about using LevelDB for this mapping table. I
> think any existing database may be overkill.
>
> For packs, you may be able to simplify by having only one file
> (pack-*.msha1) that maps SHA-1 to pack offset; idx v2. The CRC32 table
> in v2 is unnecessary, but you need the 64 bit offset support.
>
> SHA-1 to SHA-3: lookup SHA-1 in .msha1, reverse .idx, find offset to
> read the SHA-3.
> SHA-3 to SHA-1: lookup SHA-3 in .idx, and reverse the .msha1 file to
> translate offset to SHA-1.
Thanks for this suggestion. I was initially vaguely nervous about
lookup times in an idx-style file, but as you say, object reads from a
packfile already have to deal with this kind of lookup and work fine.
> For loose objects, the loose object directories should have only
> O(4000) entries before auto gc is strongly encouraging
> packing/pruning. With 256 shards, each given directory has O(16) loose
> objects in it. When writing a SHA-3 loose object, Git could also
> append a line "$sha3 $sha1\n" to objects/${first_byte}/sha1, which
> GC/prune rewrites to remove entries. With O(16) objects in a
> directory, these files should only have O(16) entries in them.
Insertion time is what worries me. When writing a small number of
objects using a command like "git commit", I don't want to have to
regenerate an entire idx file. I don't want to move the pain to
O(loose objects) work at read time, either --- some people disable
auto gc, and others have a large number of loose objects due to gc
ejecting unreachable objects.
But some kind of simplification along these lines should be possible.
I'll experiment.
Jonathan
^ permalink raw reply
* [PATCH v2 GSoC RFC] diff: allow "-" as a short-hand for "last branch"
From: mash @ 2017-03-09 20:26 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Vegard Nossum, Štěpán Němec,
Stefan Beller, Vedant Bassi, Prathamesh Chavan
In-Reply-To: <1clZj4-0006vN-9q@crossperf.com>
Just like "git merge -" is a short-hand for "git merge @{-1}" to
conveniently merge the previous branch, "git diff -" is a short-hand for
"git diff @{-1}" to conveniently diff against the previous branch.
Allow the usage of "-" in the dot dot notation to allow the use of
"git diff -..HEAD^" as a short-hand for "git diff @{-1}..HEAD^".
Signed-off-by: mash <mash+git@crossperf.com>
---
Add tests to confirm that passing in this short-hand from stdin works.
Handling the dash in sha1_name:get_sha1_basic is not an issue but git was
designed with the dash in mind for options not for this weird short-hand so as
long as there's no decision made that git should actually have this short-hand
everywhere it does not seem like a good idea to change anything in there
because it would probably have unwanted side-effects.
For example for now just handle_revision_arg was modified which is mainly used
by git diff but also used in builtin/pack-objects.c:get_object_list#2785 which
is terrible since this may have already introduced an unwanted the side-effect.
Bypassing the whatever starts with a dash is always an option filters is not a
nice thing to do either.
Overall I see the benefit of the dash short-hand when doing checkouts since
it's very similar to "cd -" and switching between two branches is something one
would commonly do but for every git command where executing it twice results
into the same action achieving the same result twice it seems like a not that
useful short-hand.
Example:
master# g co maint
maint# g co - # Switch to master
master# g co - # Switch to maint (different result)
maint# g d - # diff master..HEAD
maint# g d - # diff master..HEAD (same result - less useful)
This is obviously only my own option.
Because of what was stated above I've now marked this as open for discussion
since I'm currently not convinced that applying the patch is a good idea.
revision.c | 22 +++++++++++++++--
t/t4063-diff-last.sh | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 2 deletions(-)
create mode 100755 t/t4063-diff-last.sh
diff --git a/revision.c b/revision.c
index b37dbec..c331bd5 100644
--- a/revision.c
+++ b/revision.c
@@ -1439,6 +1439,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
const char *arg = arg_;
int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;
unsigned get_sha1_flags = 0;
+ static const char previous_branch[] = "@{-1}";
flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
@@ -1457,6 +1458,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
if (!*next)
next = head_by_default;
+ else if (!strcmp(next, "-"))
+ next = previous_branch;
if (dotdot == arg)
this = head_by_default;
if (this == head_by_default && next == head_by_default &&
@@ -1469,6 +1472,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
*dotdot = '.';
return -1;
}
+ } else if (!strcmp(this, "-")) {
+ this = previous_branch;
}
if (!get_sha1_committish(this, from_sha1) &&
!get_sha1_committish(next, sha1)) {
@@ -1568,6 +1573,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
if (revarg_opt & REVARG_COMMITTISH)
get_sha1_flags = GET_SHA1_COMMITTISH;
+ if (!strcmp(arg, "-"))
+ arg = previous_branch;
if (get_sha1_with_context(arg, get_sha1_flags, sha1, &oc))
return revs->ignore_missing ? 0 : -1;
if (!cant_be_filename)
@@ -1578,6 +1585,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
return 0;
}
+/*
+ * Check if the argument is supposed to be a revision argument instead of an
+ * option even though it starts with a dash.
+ */
+static int is_revision_arg(const char *arg)
+{
+ return *arg == '\0' || starts_with(arg, "..");
+}
+
struct cmdline_pathspec {
int alloc;
int nr;
@@ -1621,7 +1637,9 @@ static void read_revisions_from_stdin(struct rev_info *revs,
seen_dashdash = 1;
break;
}
- die("options not supported in --stdin mode");
+ if (!is_revision_arg(sb.buf + 1)) {
+ die("options not supported in --stdin mode");
+ }
}
if (handle_revision_arg(sb.buf, revs, 0,
REVARG_CANNOT_BE_FILENAME))
@@ -2205,7 +2223,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
read_from_stdin = 0;
for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
- if (*arg == '-') {
+ if (*arg == '-' && !is_revision_arg(arg + 1)) {
int opts;
opts = handle_revision_pseudo_opt(submodule,
diff --git a/t/t4063-diff-last.sh b/t/t4063-diff-last.sh
new file mode 100755
index 0000000..dc28f9d
--- /dev/null
+++ b/t/t4063-diff-last.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='diff against last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo hello >world &&
+ git add world &&
+ git commit -m initial &&
+ git branch other &&
+ echo "hello again" >>world &&
+ git add world &&
+ git commit -m second
+'
+
+test_expect_success '"diff -" does not work initially' '
+ test_must_fail git diff -
+'
+
+test_expect_success '"diff -" diffs against previous branch' '
+ git checkout other &&
+
+ cat <<-\EOF >expect &&
+ diff --git a/world b/world
+ index c66f159..ce01362 100644
+ --- a/world
+ +++ b/world
+ @@ -1,2 +1 @@
+ hello
+ -hello again
+ EOF
+
+ git diff - >out &&
+ test_cmp expect out
+'
+
+test_expect_success '"diff -" arguments from stdin' '
+ echo "-" | git diff --stdin >out &&
+ test_cmp expect out
+'
+
+test_expect_success '"diff -.." diffs against previous branch' '
+ git diff -.. >out &&
+ test_cmp expect out
+'
+
+test_expect_success '"diff -.." arguments from stdin' '
+ echo "-.." | git diff --stdin >out &&
+ test_cmp expect out
+'
+
+test_expect_success '"diff ..-" diffs inverted' '
+ cat <<-\EOF >expect &&
+ diff --git a/world b/world
+ index ce01362..c66f159 100644
+ --- a/world
+ +++ b/world
+ @@ -1 +1,2 @@
+ hello
+ +hello again
+ EOF
+
+ git diff ..- >out &&
+ test_cmp expect out
+'
+
+test_done
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref
From: Christian Couder @ 2017-03-09 20:31 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Lars Hjemli, Jeff King
In-Reply-To: <20170309200243.2203-1-avarab@gmail.com>
On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 525737a5d8..4938496194 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
> 'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
> <tagname> [<commit> | <object>]
> 'git tag' -d <tagname>...
> -'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
> +'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
> [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
> [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
> 'git tag' -v [--format=<format>] <tagname>...
> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
> Only list tags which contain the specified commit (HEAD if not
> specified).
>
> +--no-contains [<commit>]::
> + Only list tags which don't contain the specified commit (HEAD if
> + not secified).
s/secified/specified/
> +
> --points-at <object>::
> Only list tags of the given object.
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 94f7de7fa5..e8d534604c 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> OPT_SET_INT('r', "remotes", &filter.kind, N_("act on remote-tracking branches"),
> FILTER_REFS_REMOTES),
> OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
> + OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
> OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
> + OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
s/with_commit/no_commit/
^ permalink raw reply
* Git attributes are read from head branch instead of given branch when using git archive
From: Aalex Gabi @ 2017-03-09 21:02 UTC (permalink / raw)
To: git
Hello,
git archive --remote="URL" branchname > branchname.tar.gz is not
reading .gitattributes file from the given branch but from HEAD. More
specifically export-ignore lines in the target branch are ignored.
The remote is using git protocol and it's served by Gitlab. I use git
version 2.11.0.
Is this a git issue? In that case is this a bug or a known limitation?
Best regards,
Aalex Gabi
^ permalink raw reply
* [PATCH 0/2] bringing attributes to pathspecs
From: Brandon Williams @ 2017-03-09 21:07 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds
This small series extends the pathspec magic to allow users to specify
attributes that files must have in order for a pathspec to 'match' a file.
One potential use for this is to allow a repository to specify attributes for a
set of files. The user can then specify that attribute as a pathspec to
perform various operations only on that set of files. One simple example:
git ls-files -- ":(attr:text)"
can be used to list all of the files with the 'text' attribute.
Brandon Williams (2):
pathspec: allow querying for attributes
pathspec: allow escaped query values
Documentation/glossary-content.txt | 20 ++++
attr.c | 17 ++++
attr.h | 1 +
dir.c | 43 ++++++++-
pathspec.c | 165 ++++++++++++++++++++++++++++++--
pathspec.h | 16 +++-
t/t6135-pathspec-with-attrs.sh | 190 +++++++++++++++++++++++++++++++++++++
7 files changed, 442 insertions(+), 10 deletions(-)
create mode 100755 t/t6135-pathspec-with-attrs.sh
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply
* [PATCH 1/2] pathspec: allow querying for attributes
From: Brandon Williams @ 2017-03-09 21:07 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds
In-Reply-To: <20170309210756.105566-1-bmwill@google.com>
The pathspec mechanism is extended via the new
":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
requires paths to not just match the given pattern but also have the
specified attrs attached for them to be chosen.
Based on a patch by Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/glossary-content.txt | 20 ++++
attr.c | 17 ++++
attr.h | 1 +
dir.c | 43 ++++++++-
pathspec.c | 119 +++++++++++++++++++++++-
pathspec.h | 16 +++-
t/t6135-pathspec-with-attrs.sh | 181 +++++++++++++++++++++++++++++++++++++
7 files changed, 388 insertions(+), 9 deletions(-)
create mode 100755 t/t6135-pathspec-with-attrs.sh
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index fc9320e59..5c32d1905 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -384,6 +384,26 @@ full pathname may have special meaning:
+
Glob magic is incompatible with literal magic.
+attr;;
+After `attr:` comes a space separated list of "attribute
+requirements", all of which must be met in order for the
+path to be considered a match; this is in addition to the
+usual non-magic pathspec pattern matching.
++
+Each of the attribute requirements for the path takes one of
+these forms:
+
+- "`ATTR`" requires that the attribute `ATTR` must be set.
+
+- "`-ATTR`" requires that the attribute `ATTR` must be unset.
+
+- "`ATTR=VALUE`" requires that the attribute `ATTR` must be
+ set to the string `VALUE`.
+
+- "`!ATTR`" requires that the attribute `ATTR` must be
+ unspecified.
++
+
exclude;;
After a path matches any non-exclude pathspec, it will be run
through all exclude pathspec (magic signature: `!` or its
diff --git a/attr.c b/attr.c
index 5493bff22..7e2134471 100644
--- a/attr.c
+++ b/attr.c
@@ -603,6 +603,23 @@ struct attr_check *attr_check_initl(const char *one, ...)
return check;
}
+struct attr_check *attr_check_dup(const struct attr_check *check)
+{
+ struct attr_check *ret;
+
+ if (!check)
+ return NULL;
+
+ ret = attr_check_alloc();
+
+ ret->nr = check->nr;
+ ret->alloc = check->alloc;
+ ALLOC_ARRAY(ret->items, ret->nr);
+ COPY_ARRAY(ret->items, check->items, ret->nr);
+
+ return ret;
+}
+
struct attr_check_item *attr_check_append(struct attr_check *check,
const struct git_attr *attr)
{
diff --git a/attr.h b/attr.h
index 48ab3e1c2..442d464db 100644
--- a/attr.h
+++ b/attr.h
@@ -44,6 +44,7 @@ struct attr_check {
extern struct attr_check *attr_check_alloc(void);
extern struct attr_check *attr_check_initl(const char *, ...);
+extern struct attr_check *attr_check_dup(const struct attr_check *check);
extern struct attr_check_item *attr_check_append(struct attr_check *check,
const struct git_attr *attr);
diff --git a/dir.c b/dir.c
index 4541f9e14..2fe7acbcf 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,7 @@
*/
#include "cache.h"
#include "dir.h"
+#include "attr.h"
#include "refs.h"
#include "wildmatch.h"
#include "pathspec.h"
@@ -134,7 +135,8 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (n = 0; n < pathspec->nr; n++) {
size_t i = 0, len = 0, item_len;
@@ -209,6 +211,36 @@ int within_depth(const char *name, int namelen,
#define DO_MATCH_DIRECTORY (1<<1)
#define DO_MATCH_SUBMODULE (1<<2)
+static int match_attrs(const char *name, int namelen,
+ const struct pathspec_item *item)
+{
+ int i;
+
+ git_check_attr(name, item->attr_check);
+ for (i = 0; i < item->attr_match_nr; i++) {
+ const char *value;
+ int matched;
+ enum attr_match_mode match_mode;
+
+ value = item->attr_check->items[i].value;
+ match_mode = item->attr_match[i].match_mode;
+
+ if (ATTR_TRUE(value))
+ matched = (match_mode == MATCH_SET);
+ else if (ATTR_FALSE(value))
+ matched = (match_mode == MATCH_UNSET);
+ else if (ATTR_UNSET(value))
+ matched = (match_mode == MATCH_UNSPECIFIED);
+ else
+ matched = (match_mode == MATCH_VALUE &&
+ !strcmp(item->attr_match[i].value, value));
+ if (!matched)
+ return 0;
+ }
+
+ return 1;
+}
+
/*
* Does 'match' match the given name?
* A match is found if
@@ -261,6 +293,9 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
strncmp(item->match, name - prefix, item->prefix))
return 0;
+ if (item->attr_match_nr && !match_attrs(name, namelen, item))
+ return 0;
+
/* If the match was just the prefix, we matched */
if (!*match)
return MATCHED_RECURSIVELY;
@@ -339,7 +374,8 @@ static int do_match_pathspec(const struct pathspec *ps,
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
if (!ps->nr) {
if (!ps->recursive ||
@@ -1361,7 +1397,8 @@ static int simplify_away(const char *path, int pathlen,
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec->nr; i++) {
const struct pathspec_item *item = &pathspec->items[i];
diff --git a/pathspec.c b/pathspec.c
index b961f00c8..583ed5208 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "dir.h"
#include "pathspec.h"
+#include "attr.h"
/*
* Finds which of the given pathspecs match items in the index.
@@ -72,6 +73,7 @@ static struct pathspec_magic {
{ PATHSPEC_GLOB, '\0', "glob" },
{ PATHSPEC_ICASE, '\0', "icase" },
{ PATHSPEC_EXCLUDE, '!', "exclude" },
+ { PATHSPEC_ATTR, '\0', "attr" },
};
static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
@@ -87,6 +89,72 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)
+{
+ struct string_list_item *si;
+ struct string_list list = STRING_LIST_INIT_DUP;
+
+ if (item->attr_check)
+ die(_("Only one 'attr:' specification is allowed."));
+
+ if (!value || !strlen(value))
+ die(_("attr spec must not be empty"));
+
+ string_list_split(&list, value, ' ', -1);
+ string_list_remove_empty_items(&list, 0);
+
+ item->attr_check = attr_check_alloc();
+ ALLOC_GROW(item->attr_match,
+ item->attr_match_nr + list.nr,
+ item->attr_match_alloc);
+
+ for_each_string_list_item(si, &list) {
+ size_t attr_len;
+ char *attr_name;
+ const struct git_attr *a;
+
+ int j = item->attr_match_nr++;
+ const char *attr = si->string;
+ struct attr_match *am = &item->attr_match[j];
+
+ switch (*attr) {
+ case '!':
+ am->match_mode = MATCH_UNSPECIFIED;
+ attr++;
+ attr_len = strlen(attr);
+ break;
+ case '-':
+ am->match_mode = MATCH_UNSET;
+ attr++;
+ attr_len = strlen(attr);
+ break;
+ default:
+ attr_len = strcspn(attr, "=");
+ if (attr[attr_len] != '=')
+ am->match_mode = MATCH_SET;
+ else {
+ am->match_mode = MATCH_VALUE;
+ am->value = xstrdup(&attr[attr_len + 1]);
+ if (strchr(am->value, '\\'))
+ die(_("attr spec values must not contain backslashes"));
+ }
+ break;
+ }
+
+ attr_name = xmemdupz(attr, attr_len);
+ a = git_attr(attr_name);
+ if (!a)
+ die(_("invalid attribute name %s"), attr_name);
+
+ attr_check_append(item->attr_check, a);
+
+ free(attr_name);
+ }
+
+ string_list_clear(&list, 0);
+ return;
+}
+
static inline int get_literal_global(void)
{
static int literal = -1;
@@ -164,6 +232,7 @@ static int get_global_magic(int element_magic)
* returns the position in 'elem' after all magic has been parsed
*/
static const char *parse_long_magic(unsigned *magic, int *prefix_len,
+ struct pathspec_item *item,
const char *elem)
{
const char *pos;
@@ -189,6 +258,14 @@ static const char *parse_long_magic(unsigned *magic, int *prefix_len,
continue;
}
+ if (starts_with(pos, "attr:")) {
+ char *attr_body = xmemdupz(pos + 5, len - 5);
+ parse_pathspec_attr_match(item, attr_body);
+ *magic |= PATHSPEC_ATTR;
+ free(attr_body);
+ continue;
+ }
+
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, pos, len)) {
@@ -252,13 +329,14 @@ static const char *parse_short_magic(unsigned *magic, const char *elem)
}
static const char *parse_element_magic(unsigned *magic, int *prefix_len,
+ struct pathspec_item *item,
const char *elem)
{
if (elem[0] != ':' || get_literal_global())
return elem; /* nothing to do */
else if (elem[1] == '(')
/* longhand */
- return parse_long_magic(magic, prefix_len, elem);
+ return parse_long_magic(magic, prefix_len, item, elem);
else
/* shorthand */
return parse_short_magic(magic, elem);
@@ -335,12 +413,18 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
char *match;
int pathspec_prefix = -1;
+ item->attr_check = NULL;
+ item->attr_match = NULL;
+ item->attr_match_nr = 0;
+ item->attr_match_alloc = 0;
+
/* PATHSPEC_LITERAL_PATH ignores magic */
if (flags & PATHSPEC_LITERAL_PATH) {
magic = PATHSPEC_LITERAL;
} else {
copyfrom = parse_element_magic(&element_magic,
&pathspec_prefix,
+ item,
elt);
magic |= element_magic;
magic |= get_global_magic(element_magic);
@@ -544,6 +628,10 @@ void parse_pathspec(struct pathspec *pathspec,
if (item[i].nowildcard_len < item[i].len)
pathspec->has_wildcard = 1;
pathspec->magic |= item[i].magic;
+
+ if (item[i].attr_check &&
+ item[i].attr_check->nr != item[i].attr_match_nr)
+ die("BUG: should have same number of entries");
}
/*
@@ -565,26 +653,47 @@ void parse_pathspec(struct pathspec *pathspec,
void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
{
- int i;
+ int i, j;
*dst = *src;
ALLOC_ARRAY(dst->items, dst->nr);
COPY_ARRAY(dst->items, src->items, dst->nr);
for (i = 0; i < dst->nr; i++) {
- dst->items[i].match = xstrdup(src->items[i].match);
- dst->items[i].original = xstrdup(src->items[i].original);
+ struct pathspec_item *d = &dst->items[i];
+ struct pathspec_item *s = &src->items[i];
+
+ d->match = xstrdup(s->match);
+ d->original = xstrdup(s->original);
+
+ ALLOC_ARRAY(d->attr_match, d->attr_match_nr);
+ COPY_ARRAY(d->attr_match, s->attr_match, d->attr_match_nr);
+ for (j = 0; j < d->attr_match_nr; j++) {
+ const char *value = s->attr_match[j].value;
+ if (value)
+ d->attr_match[j].value = xstrdup(value);
+ }
+
+ d->attr_check = attr_check_dup(s->attr_check);
}
}
void clear_pathspec(struct pathspec *pathspec)
{
- int i;
+ int i, j;
for (i = 0; i < pathspec->nr; i++) {
free(pathspec->items[i].match);
free(pathspec->items[i].original);
+
+ for (j = 0; j < pathspec->items[j].attr_match_nr; j++)
+ free(pathspec->items[i].attr_match[j].value);
+ free(pathspec->items[i].attr_match);
+
+ if (pathspec->items[i].attr_check)
+ attr_check_free(pathspec->items[i].attr_check);
}
+
free(pathspec->items);
pathspec->items = NULL;
pathspec->nr = 0;
diff --git a/pathspec.h b/pathspec.h
index 49fd823dd..83625f006 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -8,13 +8,15 @@
#define PATHSPEC_GLOB (1<<3)
#define PATHSPEC_ICASE (1<<4)
#define PATHSPEC_EXCLUDE (1<<5)
+#define PATHSPEC_ATTR (1<<6)
#define PATHSPEC_ALL_MAGIC \
(PATHSPEC_FROMTOP | \
PATHSPEC_MAXDEPTH | \
PATHSPEC_LITERAL | \
PATHSPEC_GLOB | \
PATHSPEC_ICASE | \
- PATHSPEC_EXCLUDE)
+ PATHSPEC_EXCLUDE | \
+ PATHSPEC_ATTR)
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern satisfies GFNM_ONESTAR */
@@ -31,6 +33,18 @@ struct pathspec {
int len, prefix;
int nowildcard_len;
int flags;
+ int attr_match_nr;
+ int attr_match_alloc;
+ struct attr_match {
+ char *value;
+ enum attr_match_mode {
+ MATCH_SET,
+ MATCH_UNSET,
+ MATCH_VALUE,
+ MATCH_UNSPECIFIED
+ } match_mode;
+ } *attr_match;
+ struct attr_check *attr_check;
} *items;
};
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
new file mode 100755
index 000000000..b5e5a0607
--- /dev/null
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -0,0 +1,181 @@
+#!/bin/sh
+
+test_description='test labels in pathspecs'
+. ./test-lib.sh
+
+test_expect_success 'setup a tree' '
+ cat <<-EOF >expect &&
+ fileA
+ fileAB
+ fileAC
+ fileB
+ fileBC
+ fileC
+ fileNoLabel
+ fileSetLabel
+ fileUnsetLabel
+ fileValue
+ fileWrongLabel
+ sub/fileA
+ sub/fileAB
+ sub/fileAC
+ sub/fileB
+ sub/fileBC
+ sub/fileC
+ sub/fileNoLabel
+ sub/fileSetLabel
+ sub/fileUnsetLabel
+ sub/fileValue
+ sub/fileWrongLabel
+ EOF
+ mkdir sub &&
+ while read path
+ do
+ : >$path &&
+ git add $path || return 1
+ done <expect &&
+ git commit -m "initial commit" &&
+ git ls-files >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pathspec with no attr' '
+ test_must_fail git ls-files ":(attr:)"
+'
+
+test_expect_success 'pathspec with labels and non existent .gitattributes' '
+ git ls-files ":(attr:label)" >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'setup .gitattributes' '
+ cat <<-EOF >.gitattributes &&
+ fileA labelA
+ fileB labelB
+ fileC labelC
+ fileAB labelA labelB
+ fileAC labelA labelC
+ fileBC labelB labelC
+ fileUnsetLabel -label
+ fileSetLabel label
+ fileValue label=foo
+ fileWrongLabel label☺
+ EOF
+ git add .gitattributes &&
+ git commit -m "add attributes"
+'
+
+test_expect_success 'check specific set attr' '
+ cat <<-EOF >expect &&
+ fileSetLabel
+ sub/fileSetLabel
+ EOF
+ git ls-files ":(attr:label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check specific unset attr' '
+ cat <<-EOF >expect &&
+ fileUnsetLabel
+ sub/fileUnsetLabel
+ EOF
+ git ls-files ":(attr:-label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check specific value attr' '
+ cat <<-EOF >expect &&
+ fileValue
+ sub/fileValue
+ EOF
+ git ls-files ":(attr:label=foo)" >actual &&
+ test_cmp expect actual &&
+ git ls-files ":(attr:label=bar)" >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'check unspecified attr' '
+ cat <<-EOF >expect &&
+ .gitattributes
+ fileA
+ fileAB
+ fileAC
+ fileB
+ fileBC
+ fileC
+ fileNoLabel
+ fileWrongLabel
+ sub/fileA
+ sub/fileAB
+ sub/fileAC
+ sub/fileB
+ sub/fileBC
+ sub/fileC
+ sub/fileNoLabel
+ sub/fileWrongLabel
+ EOF
+ git ls-files ":(attr:!label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check multiple unspecified attr' '
+ cat <<-EOF >expect &&
+ .gitattributes
+ fileC
+ fileNoLabel
+ fileWrongLabel
+ sub/fileC
+ sub/fileNoLabel
+ sub/fileWrongLabel
+ EOF
+ git ls-files ":(attr:!labelB !labelA !label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check label with more labels but excluded path' '
+ cat <<-EOF >expect &&
+ fileAB
+ fileB
+ fileBC
+ EOF
+ git ls-files ":(attr:labelB)" ":(exclude)sub/" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check label excluding other labels' '
+ cat <<-EOF >expect &&
+ fileAB
+ fileB
+ fileBC
+ sub/fileAB
+ sub/fileB
+ EOF
+ git ls-files ":(attr:labelB)" ":(exclude,attr:labelC)sub/" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
+ test_must_fail git ls-files . ":(attr:labelB,attr:labelC)" 2>actual &&
+ test_i18ngrep "Only one" actual
+'
+
+test_expect_success 'fail if attr magic is used places not implemented' '
+ # The main purpose of this test is to check that we actually fail
+ # when you attempt to use attr magic in commands that do not implement
+ # attr magic. This test does not advocate git-add to stay that way,
+ # though, but git-add is convenient as it has its own internal pathspec
+ # parsing.
+ test_must_fail git add ":(attr:labelB)" 2>actual &&
+ test_i18ngrep "unsupported magic" actual
+'
+
+test_expect_success 'abort on giving invalid label on the command line' '
+ test_must_fail git ls-files . ":(attr:☺)"
+'
+
+test_expect_success 'abort on asking for wrong magic' '
+ test_must_fail git ls-files . ":(attr:-label=foo)" &&
+ test_must_fail git ls-files . ":(attr:!label=foo)"
+'
+
+test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* [PATCH 2/2] pathspec: allow escaped query values
From: Brandon Williams @ 2017-03-09 21:07 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds
In-Reply-To: <20170309210756.105566-1-bmwill@google.com>
In our own .gitattributes file we have attributes such as:
*.[ch] whitespace=indent,trail,space
When querying for attributes we want to be able to ask for the exact
value, i.e.
git ls-files :(attr:whitespace=indent,trail,space)
should work, but the commas are used in the attr magic to introduce
the next attr, such that this query currently fails with
fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'
This change allows escaping characters by a backslash, such that the query
git ls-files :(attr:whitespace=indent\,trail\,space)
will match all path that have the value "indent,trail,space" for the
whitespace attribute. To accomplish this, we need to modify two places.
First `parse_long_magic` needs to not stop early upon seeing a comma or
closing paren that is escaped. As a second step we need to remove any
escaping from the attr value.
Based on a patch by Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 52 ++++++++++++++++++++++++++++++++++++++----
t/t6135-pathspec-with-attrs.sh | 9 ++++++++
2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 583ed5208..f89f84a83 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -89,6 +89,51 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static size_t strcspn_escaped(const char *s, const char *stop)
+{
+ const char *i;
+
+ for (i = s; *i; i++) {
+ /* skip the escaped character */
+ if (i[0] == '\\' && i[1]) {
+ i++;
+ continue;
+ }
+
+ if (strchr(stop, *i))
+ break;
+ }
+ return i - s;
+}
+
+static inline int invalid_value_char(const char ch)
+{
+ if (isalnum(ch) || strchr(",-_", ch))
+ return 0;
+ return -1;
+}
+
+static char *attr_value_unescape(const char *value)
+{
+ const char *src;
+ char *dst, *ret;
+
+ ret = xmallocz(strlen(value));
+ for (src = value, dst = ret; *src; src++, dst++) {
+ if (*src == '\\') {
+ if (!src[1])
+ die(_("Escape character '\\' not allowed as "
+ "last character in attr value"));
+ src++;
+ }
+ if (invalid_value_char(*src))
+ die("cannot use '%c' for value matching", *src);
+ *dst = *src;
+ }
+ *dst = '\0';
+ return ret;
+}
+
static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)
{
struct string_list_item *si;
@@ -133,10 +178,9 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
if (attr[attr_len] != '=')
am->match_mode = MATCH_SET;
else {
+ const char *v = &attr[attr_len + 1];
am->match_mode = MATCH_VALUE;
- am->value = xstrdup(&attr[attr_len + 1]);
- if (strchr(am->value, '\\'))
- die(_("attr spec values must not contain backslashes"));
+ am->value = attr_value_unescape(v);
}
break;
}
@@ -239,7 +283,7 @@ static const char *parse_long_magic(unsigned *magic, int *prefix_len,
const char *nextat;
for (pos = elem + 2; *pos && *pos != ')'; pos = nextat) {
- size_t len = strcspn(pos, ",)");
+ size_t len = strcspn_escaped(pos, ",)");
int i;
if (pos[len] == ',')
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index b5e5a0607..585d17bad 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -178,4 +178,13 @@ test_expect_success 'abort on asking for wrong magic' '
test_must_fail git ls-files . ":(attr:!label=foo)"
'
+test_expect_success 'check attribute list' '
+ cat <<-EOF >>.gitattributes &&
+ * whitespace=indent,trail,space
+ EOF
+ git ls-files ":(attr:whitespace=indent\,trail\,space)" >actual &&
+ git ls-files >expect &&
+ test_cmp expect actual
+'
+
test_done
--
2.12.0.246.ga2ecc84866-goog
^ permalink raw reply related
* Re: [PATCH 0/2] bringing attributes to pathspecs
From: Stefan Beller @ 2017-03-09 21:22 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Duy Nguyen
In-Reply-To: <20170309210756.105566-1-bmwill@google.com>
On Thu, Mar 9, 2017 at 1:07 PM, Brandon Williams <bmwill@google.com> wrote:
> This small series extends the pathspec magic to allow users to specify
> attributes that files must have in order for a pathspec to 'match' a file.
>
> One potential use for this is to allow a repository to specify attributes for a
> set of files. The user can then specify that attribute as a pathspec to
> perform various operations only on that set of files. One simple example:
>
> git ls-files -- ":(attr:text)"
>
> can be used to list all of the files with the 'text' attribute.
Thanks for reviving these patches.
I have read through both of them and they look good to me.
Thanks,
Stefan
^ permalink raw reply
* [PATCH] gitk: blame older file in case of copied and renamed files
From: Max Kirillov @ 2017-03-09 21:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Max Kirillov, git
If file was renamed or copied, and in the same time edited, attempt to run
"Show origin of this line" or "Run gui blame on this line" would result in
error "fatal: no such path FILE in HASH". Reason is that it tried to use
the newer filename, while it should use the older one.
Since ctext_file_names in diff mode only used for parent commit
filenames, there is no need to split it to 2 lists, just change its
so that for diff mode it means older filename always.
Signed-off-by: Max Kirillov <max@max630.net>
---
gitk | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/gitk b/gitk
index d3e9e459c3..0b8b1a4e9f 100755
--- a/gitk
+++ b/gitk
@@ -8349,9 +8349,16 @@ proc setinlist {var i val} {
}
}
+proc add_ctext_file {fname} {
+ global ctext_file_names
+
+ set fname [encoding convertfrom $fname]
+ lset ctext_file_names end $fname
+}
+
proc makediffhdr {fname ids} {
global ctext curdiffstart treediffs diffencoding
- global ctext_file_names jump_to_here targetline diffline
+ global jump_to_here targetline diffline
set fname [encoding convertfrom $fname]
set diffencoding [get_path_encoding $fname]
@@ -8359,7 +8366,6 @@ proc makediffhdr {fname ids} {
if {$i >= 0} {
setinlist difffilestart $i $curdiffstart
}
- lset ctext_file_names end $fname
set l [expr {(78 - [string length $fname]) / 2}]
set pad [string range "----------------------------------------" 1 $l]
$ctext insert $curdiffstart "$pad $fname $pad" filesep
@@ -8454,6 +8460,7 @@ proc parseblobdiffline {ids line} {
set fname [string range $line 2 [expr {$i - 1}]]
}
}
+ add_ctext_file $fname
makediffhdr $fname $ids
} elseif {![string compare -length 16 "* Unmerged path " $line]} {
@@ -8492,6 +8499,7 @@ proc parseblobdiffline {ids line} {
lappend ctext_file_names ""
if {$currdiffsubmod != $fname} {
lappend ctext_file_lines $fname
+ add_ctext_file $fname
makediffhdr $fname $ids
set currdiffsubmod $fname
$ctext insert end "\n$line\n" filesep
@@ -8512,11 +8520,18 @@ proc parseblobdiffline {ids line} {
if {[string index $fname 0] eq "\""} {
set fname [lindex $fname 0]
}
+ add_ctext_file $fname
set fname [encoding convertfrom $fname]
set i [lsearch -exact $treediffs($ids) $fname]
if {$i >= 0} {
setinlist difffilestart $i $curdiffstart
}
+ } elseif {![string compare -length 10 $line "copy from "]} {
+ set fname [string range $line [expr 6 + [string first " from " $line] ] end]
+ if {[string index $fname 0] eq "\""} {
+ set fname [lindex $fname 0]
+ }
+ add_ctext_file $fname
} elseif {![string compare -length 10 $line "rename to "] ||
![string compare -length 8 $line "copy to "]} {
set fname [string range $line [expr 4 + [string first " to " $line] ] end]
--
2.11.0.1122.gc3fec58.dirty
^ permalink raw reply related
* Re: Git attributes are read from head branch instead of given branch when using git archive
From: Brandon Williams @ 2017-03-09 21:45 UTC (permalink / raw)
To: Aalex Gabi; +Cc: git
In-Reply-To: <CABRdPwCA6UR0kkZ2T8W4OYZ36oBwF9hw+SpJHXDE15CHyFiA8A@mail.gmail.com>
On 03/09, Aalex Gabi wrote:
> Hello,
>
> git archive --remote="URL" branchname > branchname.tar.gz is not
> reading .gitattributes file from the given branch but from HEAD. More
> specifically export-ignore lines in the target branch are ignored.
>
> The remote is using git protocol and it's served by Gitlab. I use git
> version 2.11.0.
>
> Is this a git issue? In that case is this a bug or a known limitation?
>
> Best regards,
> Aalex Gabi
Could you provide a little more context with the problem you are having?
When you say HEAD are you referring to the server's HEAD?
When testing this locally (with a relative URL) I can't seem to
reproduce what you are describing. If you could provide a way to
reproduce this I would be able to more easily diagnose the problem.
--
Brandon Williams
^ permalink raw reply
* Re: What's cooking in git.git (Mar 2017, #03; Wed, 8)
From: Johannes Schindelin @ 2017-03-09 22:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqvarjz5yv.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 8 Mar 2017, Junio C Hamano wrote:
> * jk/http-auth (2017-02-27) 2 commits
> (merged to 'next' on 2017-03-02 at 87f81b4395)
> + http: add an "auto" mode for http.emptyauth
> + http: restrict auth methods to what the server advertises
>
> Reduce authentication round-trip over HTTP when the server supports
> just a single authentication method.
Not only that. It fixes a bug where empty credentials were sent to a
server that did not declare to Negotiate (in which case empty credentials
are literally useless, and very likely incorrect).
Ciao,
Johannes
^ 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