* Re: [PATCH 2/4] shallow: fix memory leak when registering shallow roots
From: Jeff King @ 2023-11-06 17:21 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <137c150e358a8248200e0c7174a79358adb92e45.1699267422.git.ps@pks.im>
On Mon, Nov 06, 2023 at 11:45:57AM +0100, Patrick Steinhardt wrote:
> --- a/shallow.c
> +++ b/shallow.c
> @@ -38,8 +38,10 @@ int register_shallow(struct repository *r, const struct object_id *oid)
>
> oidcpy(&graft->oid, oid);
> graft->nr_parent = -1;
> - if (commit && commit->object.parsed)
> + if (commit && commit->object.parsed) {
> + free_commit_list(commit->parents);
> commit->parents = NULL;
> + }
> return register_commit_graft(r, graft, 0);
> }
Good catch. When I've dealt with leaks around commit_lists in the past,
one gotcha is that sometimes we've saved a pointer to the list
elsewhere. But in this case it looks pretty clear that the parent list
just goes away. So this patch is doing the right thing.
-Peff
^ permalink raw reply
* Re: Bug: magic-less pathspecs that start with ":" not processed as expected.
From: Jeff King @ 2023-11-06 17:31 UTC (permalink / raw)
To: Joanna Wang; +Cc: git
In-Reply-To: <CAMmZTi-JJ6=Uw_+r50hKMYf34D0NtqXhaA=f+11+wAQBjqA7_g@mail.gmail.com>
On Mon, Nov 06, 2023 at 10:54:03AM -0500, Joanna Wang wrote:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> What did you expect to happen? (Expected behavior)
> `git stash push -- :file` where `:` is part of the filename, should search
> for files named ":file"
>
> What happened instead? (Actual behavior)
> The match string used to find files is "file" without the ":".
>
> What's different between what you expected and what actually happened?
> file named ":file" is not found.
>
> Anything else you want to add:
> I believe this is due to parse_short_magic() not handling the case where
> ":" is part of the file name rather than a prefix for pathspec magic.
>
> I could not find any documentation that says ":" is a reserved
> character to indicate pathspec magic symbols MUST follow or that
> ":file" is not a valid file name.
I think this is the correct behavior according to the documentation.
From "git help glossary", the entry on "pathspec" says:
A pathspec that begins with a colon : has special meaning. In the
short form, the leading colon : is followed by zero or more "magic
signature" letters (which optionally is terminated by another colon
:), and the remainder is the pattern to match against the path.
So ":file" just has zero magic signature letters. I think you want:
:::file
which of course is rather ugly, but then so is your filename. ;)
Although longer, probably this is more readable:
:(literal):file
And if you're working with a lot of such files (or you are scripting and
know you have a set of filenames), then your best bet is probably
turning on literal mode, like:
git --literal-pathspecs ls-files -- :file
-Peff
PS It took me a while to figure out where we document pathspec syntax. I
wonder if a "gitpathspecs" manpage would make sense, like we have
"gitrevisions".
^ permalink raw reply
* Re: [PATCH 0/4] Memory leak fixes
From: Jeff King @ 2023-11-06 17:32 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <cover.1699267422.git.ps@pks.im>
On Mon, Nov 06, 2023 at 11:45:48AM +0100, Patrick Steinhardt wrote:
> this patch series fixes some memory leaks. All of these leaks have been
> found while working on the reftable backend.
All four look good to me (and the refactoring in 3/4 is very cleanly
done).
-Peff
^ permalink raw reply
* git refusing to merge branches when pulling using a refspec
From: Santiago García Pimentel @ 2023-11-06 17:36 UTC (permalink / raw)
To: git
Hello,
I'm dealing with a small automation (CI) to synchronise some specific branches between two git repositories.
I need to sync a branch with other using a user-given refspec. e.g.
$ git pull origin "refs/heads/branchOrigin:refs/heads/branchDestination”.
(I’ll have a list of refspecs, but so far Im trying to make it work with one)
When the branch can be fast-forwarded there is no problem, but I cannot manage to make it work when it cant.
I just. get the message "[rejected] branchOrigin -> branchDestination (non-fast forward)."
I've tried the following:
- adding a git config with pull.rebase=false (I also tried true)
- adding --no-rebase to the pull command (I also tried —rebase)
In any cases the result is the same
The issue is, It does work if I just pass the branch names. e.g.
git pull origin branchOrigin --no-edit
merges the branch just fine
Im not sure why git refuses to merge the branches when I pass a full refspec. is this intentional? is there some other incantation I need to use to make this work?
Thank you!
Santiago García Pimentel.
santiago@garciapimentel.com
^ permalink raw reply
* first-class conflicts?
From: Sandra Snan @ 2023-11-06 21:17 UTC (permalink / raw)
To: git
Is this feature from jj also a good idea for git?
https://martinvonz.github.io/jj/v0.11.0/conflicts/
^ permalink raw reply
* Re: first-class conflicts?
From: Dragan Simic @ 2023-11-06 22:01 UTC (permalink / raw)
To: Sandra Snan; +Cc: git
In-Reply-To: <87cywmintp.fsf@ellen.idiomdrottning.org>
On 2023-11-06 22:17, Sandra Snan wrote:
> Is this feature from jj also a good idea for git?
> https://martinvonz.github.io/jj/v0.11.0/conflicts/
Hmm, that's quite interesting, but frankly it makes little sense to me.
See, the source code in a repository should always be in a compileable
or runnable state, in each and every commit, so going against that rule
wouldn't make much sense. Just think about various CI/CD tools that
also expect the same.
^ permalink raw reply
* Re: first-class conflicts?
From: Sandra Snan @ 2023-11-06 22:34 UTC (permalink / raw)
To: git
In-Reply-To: <ef30a484525157579c64249a396f10ae@manjaro.org>
[-- Attachment #1: Type: text/plain, Size: 169 bytes --]
I've sometimes merged stuff in and almost not notice that I had a conflict
in there and in those cases the code wasn't compilable even though I was
using vanilla git.
^ permalink raw reply
* RE: first-class conflicts?
From: rsbecker @ 2023-11-06 22:34 UTC (permalink / raw)
To: 'Dragan Simic', 'Sandra Snan'; +Cc: git
In-Reply-To: <ef30a484525157579c64249a396f10ae@manjaro.org>
On November 6, 2023 5:01 PM, Dragan Simic wrote:
>On 2023-11-06 22:17, Sandra Snan wrote:
>> Is this feature from jj also a good idea for git?
>> https://martinvonz.github.io/jj/v0.11.0/conflicts/
>
>Hmm, that's quite interesting, but frankly it makes little sense to me.
>See, the source code in a repository should always be in a compileable or
runnable
>state, in each and every commit, so going against that rule wouldn't make
much
>sense. Just think about various CI/CD tools that also expect the same.
It seems to me, perhaps naively, that the longer a conflict persists in a
repository, the greater the potential for chaotic results. There are,
notably, at least two fundamental types of conflicts:
1. Content conflict, where a point in a file is modified in two (or n)
branches being combined, is what git tries to ensure never happens. The
longer such a conflict exists in a file, the greater the variance from a
buildable or consistent state will persist and will likely be increasingly
harder to resolve.
2. Semantic conflicts, where unrelated modification points cause
incompatibilities are much harder to resolve and quantify - many are, in
fact, undetectable from a computational standpoint (as in detecting general
semantic conflicts is an uncomputable problem). The longer those persist,
partly when they are missed by pull requests/code reviews, the more
persistent a defect can become.
3. I am avoiding matters such as code optimization conflicts which are
outside the scope of the proposal.
In either case, storing conflicts in the integration branches of a
repository is, in my view, a bad thing that eventually can make the
repository unsustainable. I will concede that keeping conflicts around in
non-integration branches may have intellectual value for recording research
and development progress.
This is just my opinion.
Randall
--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.
^ permalink raw reply
* Re: RE: first-class conflicts?
From: Sandra Snan @ 2023-11-06 22:45 UTC (permalink / raw)
To: git, Dragan Simic, rsbecker
In-Reply-To: <002901da1101$7d39a420$77acec60$@nexbridge.com>
[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]
Randall, thank you for that.
I did mean of the first type, pure content conflicts (just like the examples
on that jj page).
I just have sometimes wish git could be a little more aware of them beyond
just storing them with ASCII art in the files themselves (and alerting /
warning when they happen but I often can't properly see those warnings flash
by so I end up having to search for the conflict markers manually). So if
conflicts are a thing that *can* happen, it'd be better if vc could know
about them which would make some of the rebases simpler as in jj. That doesn't
mean we wanna adopt the jj workflow of deliberately checking in conflicts
(not even locally), just be able to deal with them better if it does happen.
I dunno… and I've really appreciated the naysayers so far, helps me sort
out my thoughts in this. I personally really prefer the vanilla "explicit
staging" workflow (with magit) over jj, got, gitless etc. I'm more scared
of overcommitting by mistake than undercommitting. But this one feature
seemed to me that it might be really good: just having the vc be aware of
the conflicts it has created.
^ permalink raw reply
* [PATCH 0/2] revision: exclude all packed objects with `--unpacked`
From: Taylor Blau @ 2023-11-06 22:56 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
While working on my longer series to enable verbatim pack reuse across
multiple packs[^1], I noticed a couple of oddities with the `--unpacked`
rev-walk flag.
While it does exclude packed commits, it does not exclude (all) packed
trees/blobs/annotated tags. This problem exists in the pack-bitmap
machinery, too, which will over-count queries like:
$ git rev-list --use-bitmap-index --all --unpacked --objects
, etc.
The fix is relatively straightforward, split across two patches that
Peff and I worked on together earlier today.
This is technically a backwards-incompatible change, but the existing
behavior is broken and does not match the documented behavior, so I
think in this case we are OK to change --unpacked to faithfully
implement its documentation.
[^1]: Which, I'm very excited to say, is working :-).
Taylor Blau (2):
list-objects: drop --unpacked non-commit objects from results
pack-bitmap: drop --unpacked non-commit objects from results
list-objects.c | 3 +++
pack-bitmap.c | 27 +++++++++++++++++++++++++++
t/t6000-rev-list-misc.sh | 13 +++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++
t/t6115-rev-list-du.sh | 7 +++++++
5 files changed, 63 insertions(+)
base-commit: bc5204569f7db44d22477485afd52ea410d83743
--
2.43.0.rc0.2.gef6b2154a3
^ permalink raw reply
* [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results
From: Taylor Blau @ 2023-11-06 22:56 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699311386.git.me@ttaylorr.com>
In git-rev-list(1), we describe the `--unpacked` option as:
Only useful with `--objects`; print the object IDs that are not in
packs.
This is true of commits, which we discard via get_commit_action(), but
not of the objects they reach. So if we ask for an --objects traversal
with --unpacked, we may get arbitrarily many objects which are indeed
packed.
I am nearly certain this behavior dates back to the introduction of
`--unpacked` via 12d2a18780 ("git rev-list --unpacked" shows only
unpacked commits, 2005-07-03), but I couldn't get that revision of Git
to compile for me. At least as early as v2.0.0 this has been subtly
broken:
$ git.compile --version
git version 2.0.0
$ git.compile rev-list --objects --all --unpacked
72791fe96c93f9ec5c311b8bc966ab349b3b5bbe
05713d991c18bbeef7e154f99660005311b5004d v1.0
153ed8b7719c6f5a68ce7ffc43133e95a6ac0fdb
8e4020bb5a8d8c873b25de15933e75cc0fc275df one
9200b628cf9dc883a85a7abc8d6e6730baee589c two
3e6b46e1b7e3b91acce99f6a823104c28aae0b58 unpacked.t
There, only the first, third, and sixth entries are loose, with the
remaining set of objects belonging to at least one pack.
The implications for this are relatively benign: bare 'git repack'
invocations which invoke pack-objects with --unpacked are impacted, and
at worst we'll store a few extra objects that should have been excluded.
Arguably changing this behavior is a backwards-incompatible change,
since it alters the set of objects emitted from rev-list queries with
`--objects` and `--unpacked`. But I argue that this change is still
sensible, since the existing implementation deviates from
clearly-written documentation.
The fix here is straightforward: avoid showing any non-commit objects
which are contained in packs by discarding them within list-objects.c,
before they are shown to the user. Note that similar treatment for
`list-objects.c::show_commit()` is not needed, since that case is
already handled by `revision.c::get_commit_action()`.
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
list-objects.c | 3 +++
t/t6000-rev-list-misc.sh | 13 +++++++++++++
2 files changed, 16 insertions(+)
diff --git a/list-objects.c b/list-objects.c
index c25c72b32c..c8a5fb998e 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -39,6 +39,9 @@ static void show_object(struct traversal_context *ctx,
{
if (!ctx->show_object)
return;
+ if (ctx->revs->unpacked && has_object_pack(&object->oid))
+ return;
+
ctx->show_object(object, name, ctx->show_data);
}
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 12def7bcbf..6289a2e8b0 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -169,4 +169,17 @@ test_expect_success 'rev-list --count --objects' '
test_line_count = $count actual
'
+test_expect_success 'rev-list --unpacked' '
+ git repack -ad &&
+ test_commit unpacked &&
+
+ git rev-list --objects --no-object-names unpacked^.. >expect.raw &&
+ sort expect.raw >expect &&
+
+ git rev-list --all --objects --unpacked --no-object-names >actual.raw &&
+ sort actual.raw >actual &&
+
+ test_cmp expect actual
+'
+
test_done
--
2.43.0.rc0.2.gef6b2154a3
^ permalink raw reply related
* [PATCH 2/2] pack-bitmap: drop --unpacked non-commit objects from results
From: Taylor Blau @ 2023-11-06 22:56 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699311386.git.me@ttaylorr.com>
When performing revision queries with `--objects` and
`--use-bitmap-index`, the output may incorrectly contain objects which
are packed, even when the `--unpacked` option is given. This affects
traversals, but also other querying operations, like `--count`,
`--disk-usage`, etc.
Like in the previous commit, the fix is to exclude those objects from
the result set before they are shown to the user (or, in this case,
before the bitmap containing the result of the traversal is enumerated
and its objects listed).
This is performed by a new function in pack-bitmap.c, called
`filter_packed_objects_from_bitmap()`. Note that we do not have to
inspect individual bits in the result bitmap, since we know that the
first N (where N is the number of objects in the bitmap's pack/MIDX)
bits correspond to objects which packed by definition.
In other words, for an object to have a bitmap position (not in the
extended index), it must appear in either the bitmap's pack or one of
the packs in its MIDX.
This presents an appealing optimization to us, which is that we can
simply memset() the corresponding number of `eword_t`'s to zero,
provided that we handle any objects which spill into the next word (but
don't occupy all 64 bits of the word itself).
We only have to handle objects in the bitmap's extended index. These
objects may (or may not) appear in one or more pack(s). Since these
objects are known to not appear in either the bitmap's MIDX or pack,
they may be stored as loose, appear in other pack(s), or both.
Before returning a bitmap containing the result of the traversal back to
the caller, drop any bits from the extended index which appear in one or
more packs. This implements the correct behavior for rev-list operations
which use the bitmap index to compute their result.
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap.c | 27 +++++++++++++++++++++++++++
t/t6113-rev-list-bitmap-filters.sh | 13 +++++++++++++
t/t6115-rev-list-du.sh | 7 +++++++
3 files changed, 47 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index ca8319b87c..0260890341 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1666,6 +1666,30 @@ static int can_filter_bitmap(struct list_objects_filter_options *filter)
return !filter_bitmap(NULL, NULL, NULL, filter);
}
+
+static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
+ struct bitmap *result)
+{
+ struct eindex *eindex = &bitmap_git->ext_index;
+ uint32_t objects_nr;
+ size_t i, pos;
+
+ objects_nr = bitmap_num_objects(bitmap_git);
+ pos = objects_nr / BITS_IN_EWORD;
+
+ if (pos > result->word_alloc)
+ pos = result->word_alloc;
+
+ memset(result->words, 0x00, sizeof(eword_t) * pos);
+ for (i = pos * BITS_IN_EWORD; i < objects_nr; i++)
+ bitmap_unset(result, i);
+
+ for (i = 0; i < eindex->count; ++i) {
+ if (has_object_pack(&eindex->objects[i]->oid))
+ bitmap_unset(result, objects_nr + i);
+ }
+}
+
struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
int filter_provided_objects)
{
@@ -1788,6 +1812,9 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
wants_bitmap,
&revs->filter);
+ if (revs->unpacked)
+ filter_packed_objects_from_bitmap(bitmap_git, wants_bitmap);
+
bitmap_git->result = wants_bitmap;
bitmap_git->haves = haves_bitmap;
diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
index 4d8e09167e..86c70521f1 100755
--- a/t/t6113-rev-list-bitmap-filters.sh
+++ b/t/t6113-rev-list-bitmap-filters.sh
@@ -141,4 +141,17 @@ test_expect_success 'combine filter with --filter-provided-objects' '
done <objects
'
+test_expect_success 'bitmap traversal with --unpacked' '
+ git repack -adb &&
+ test_commit unpacked &&
+
+ git rev-list --objects --no-object-names unpacked^.. >expect.raw &&
+ sort expect.raw >expect &&
+
+ git rev-list --use-bitmap-index --objects --all --unpacked >actual.raw &&
+ sort actual.raw >actual &&
+
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t6115-rev-list-du.sh b/t/t6115-rev-list-du.sh
index d59111dede..c0cfda62fa 100755
--- a/t/t6115-rev-list-du.sh
+++ b/t/t6115-rev-list-du.sh
@@ -48,6 +48,13 @@ check_du HEAD
check_du --objects HEAD
check_du --objects HEAD^..HEAD
+test_expect_success 'setup for --unpacked tests' '
+ git repack -adb &&
+ test_commit unpacked
+'
+
+check_du --all --objects --unpacked
+
# As mentioned above, don't use hardcode sizes as actual size, but use the
# output from git cat-file.
test_expect_success 'rev-list --disk-usage=human' '
--
2.43.0.rc0.2.gef6b2154a3
^ permalink raw reply related
* Re: Regression: git send-email Message-Id: numbering doesn't start at 1 any more
From: Junio C Hamano @ 2023-11-06 23:06 UTC (permalink / raw)
To: Uwe Kleine-König, Michael Strawbridge
Cc: git, Douglas Anderson, entwicklung
In-Reply-To: <20231106153214.s5abourejkuiwk64@pengutronix.de>
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> Hello,
>
> Since commit 3ece9bf0f9e24909b090cf348d89e8920bd4f82f I experience that
> the generated Message-Ids don't start at ....-1-... any more. I have:
>
> $ git send-email w/*
> ...
> Subject: [PATCH 0/5] watchdog: Drop platform_driver_probe() and convert to platform remove callback returning void (part II)
> Date: Mon, 6 Nov 2023 16:10:04 +0100
> Message-ID: <20231106151003.3844134-7-u.kleine-koenig@pengutronix.de>
> ...
>
> So the cover letter is sent with Message-Id: ...-7-...
The above is consistent with the fact that a 5-patch series with a
cover letter consists of 6 messages. Dry-run uses message numbers
1-6 and forgets to reset the counter, so the next message becomes 7.
As you identified, the fix in 3ece9bf0 (send-email: clear the
$message_id after validation, 2023-05-17) for the fallout from an
even earlier change to process each message twice still had left an
observable side effect subject to the Hyrum's law, it seems.
> +my ($message_id_stamp, $message_id_serial);
> if ($validate) {
> # FIFOs can only be read once, exclude them from validation.
> my @real_files = ();
> @@ -821,6 +822,7 @@ sub is_format_patch_arg {
> }
> delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
> delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
> + $message_id_serial = 0;
> }
This fix looks quite logical to me, but even with this, the side
effects of the earlier "read message twice" persists in end-user
observable form, don't they? IIRC, when sending out an N message
series, we start from the timestamp as of N seconds ago and give
each message the Date: header that increments by 1 second, which
would mean the validator will see Date: that is different from what
will actually be sent out, and more importantly, the messages sent
out for real will have timestamps from the future, negating the
point of starting from N seconds ago in the first place. Your
script may not have been paying attention to it and only noticed the
difference in id_serial, but somebody else would complain the
difference coming from calling gen_header more than once for each
messages since a8022c5f (send-email: expose header information to
git-send-email's sendemail-validate hook, 2023-04-19).
So, I dunno. Michael, what do you think? It appears to me that a
more fundamental fix to the fallout from a8022c5f might be needed
(e.g., we still let gen_header run while validating, but once
validation is done, save the headers that validator saw and use them
without calling gen_header again when we send the messages out, or
something), if we truly want to be regression free.
By the way, out of curiosity, earlier you said your script looks at
the Message-IDs and counts the number of messages. How does it do
that? Does it read the output of send-email and pass the messages
to MTA for sending out for real?
Thanks.
> @files = handle_backup_files(@files);
> @@ -1181,7 +1183,6 @@ sub validate_address_list {
>
> # We'll setup a template for the message id, using the "from" address:
>
> -my ($message_id_stamp, $message_id_serial);
> sub make_message_id {
> my $uniq;
> if (!defined $message_id_stamp) {
>
> But I guess this could be done prettier by someone who is fluent in
> Perl.
>
> Best regards
> Uwe
^ permalink raw reply
* Re: [PATCH v5 0/5] merge-ort: implement support for packing objects together
From: Junio C Hamano @ 2023-11-06 23:19 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Taylor Blau, git, Elijah Newren, Eric W. Biederman, Jeff King,
Patrick Steinhardt
In-Reply-To: <0ac32374-7d52-8f0c-8583-110de678291e@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> What I have in mind is more along Elijah's suggestion at the Contributor
> Summit to use the `tmp_objdir*()` machinery. But instead of discarding the
> temporary object database, the contained objects would be repacked and the
> `.pack`, (maybe `.rev`) and the `.idx` file would then be moved (in that
> order) before discarding the temporary object database.
That may be more involved but does indeed sound like an approach
more generally applicable. Back when the bulk-checkin machinery was
invented, I envisioned that we would be adding annotations to
various codepaths so that object creation machinery can say "now we
are plugged, in anticipation for creating many objects at once" and
"now the flood of new object creation is done, time to wrap up" for
that kind of optimization.
The callsites to {begin,end}_odb_transaction() functions haven't
grown beyond the original "add" and "update-index" (because the user
can add the entire working tree worth of files to the object
database), "unpack-objects" (because a fetch can bring in many
objects), and "cache-tree" (because a tree creation can cascade up
to create many objects), but I agree "merge" and "replay" are prime
candidates to benefit from the optimization of the same kind (so is
"fast-import"). They are about creating many objects at once, and
give us an opportunity for such an optimization.
^ permalink raw reply
* Re: Bug: magic-less pathspecs that start with ":" not processed as expected.
From: Junio C Hamano @ 2023-11-06 23:25 UTC (permalink / raw)
To: Joanna Wang; +Cc: git
In-Reply-To: <CAMmZTi-JJ6=Uw_+r50hKMYf34D0NtqXhaA=f+11+wAQBjqA7_g@mail.gmail.com>
Joanna Wang <jojwang@google.com> writes:
> I believe this is due to parse_short_magic() not handling the case where
> ":" is part of the file name rather than a prefix for pathspec magic.
Also, ":<path>" is a notation for the object name sitting at <path>
in the index, so giving it from the command line is ambiguous, too.
I thought that ./:<path> would be a common way to talk about such a
path to git commands?
$ date >./:now.txt
$ git add ./:now.txt
$ git rm ./:now.txt
I do not offhand remember where this is documented, but I agree that
we need to make sure that we give help to users on things like this
("how do I add a file whose name begins with a dash" is another
question I think we hear from time to time).
Thanks.
^ permalink raw reply
* Re: Bug: magic-less pathspecs that start with ":" not processed as expected.
From: Junio C Hamano @ 2023-11-06 23:29 UTC (permalink / raw)
To: Jeff King; +Cc: Joanna Wang, git
In-Reply-To: <20231106173133.GC10414@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> PS It took me a while to figure out where we document pathspec syntax. I
> wonder if a "gitpathspecs" manpage would make sense, like we have
> "gitrevisions".
Yeah, I came to the same conclusion (should have saved time by
scanning the mailing list before I started writing my response) and
wondered where we wrote it down. The description you found in the
glossary, as far as I recall, is the authoritative one and looks
readable, but I agree it is not as discoverable as it should be.
A simpler and more readable workaround than ":::file" is "./:file"
by the way ;-)
^ permalink raw reply
* Re: git refusing to merge branches when pulling using a refspec
From: Junio C Hamano @ 2023-11-06 23:38 UTC (permalink / raw)
To: Santiago García Pimentel; +Cc: git
In-Reply-To: <FC0625CD-D736-412C-A4C5-04F9F41FFEEA@garciapimentel.com>
Santiago García Pimentel <santiago@garciapimentel.com> writes:
> Hello,
>
> I'm dealing with a small automation (CI) to synchronise some specific branches between two git repositories.
>
> I need to sync a branch with other using a user-given refspec. e.g.
>
> $ git pull origin "refs/heads/branchOrigin:refs/heads/branchDestination”.
>
> (I’ll have a list of refspecs, but so far Im trying to make it work with one)
>
> When the branch can be fast-forwarded there is no problem, but I cannot manage to make it work when it cant.
>
> I just. get the message "[rejected] branchOrigin -> branchDestination (non-fast forward)."
With the "pull" command above, you are doing two logically
independent things. Do you really need to do both?
* git pull "<remote>" "<src>:<dst>" first does a "git fetch" to
locally update <dst> with the commit that is pointed at by <src>
at the <remote> repository.
* then, into the currently checked out HEAD, the <src> taken from
<remote> is merged into.
If you do not need to update <dst> locally, don't give :<dst> part
on the command line.
If you do need to update <dst> locally and safely, then thank that
you got the [rejected] message. Because the <src> was updated at
the <remote> side that is not based on what you have at <dst>
locally, you may be losint commits from your local <dst> if you let
the first stage of the "git pull" go through, and that is what the
failing command is about.
If you do need to update <dst> locally but you do not have anything
valuable on <dst> locally (in other words, <dst> is used only to
keep track of <src> at <remote>, and if <remote> rewinds the history
of their <src> and loses some commits, you want to lose these commits
the same way from your <dst>), then add "+" before the refspec, i.e.
git pull "<remote>" "+<src>:<dst>"
^ permalink raw reply
* Re: Explanation of `eol` attribute doesn't seem correct for Windows machines
From: Hans Meiser @ 2023-11-07 0:23 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git@vger.kernel.org
In-Reply-To: <20231106162125.GA31375@tb-raspi4>
> > And what happens when a file is added to the index with `core.autocrlf=input` on Windows machines?
> Do you have a .gitattributes file ? Or not ?
> Is the file a new one, or does it exist ?
Exactly all these questions/cases should be explained by the documentation, I suppose.
-------
From: Torsten Bögershausen <tboegi@web.de>
Sent: Monday, November 6, 2023 17:21
To: Hans Meiser <brille1@hotmail.com>
Cc: git@vger.kernel.org <git@vger.kernel.org>
Subject: Re: Explanation of `eol` attribute doesn't seem correct for Windows machines
On Mon, Nov 06, 2023 at 03:11:54PM +0000, Hans Meiser wrote:
> https://git-scm.com/docs/gitattributes#_eol
>
> claims that:
>
> ---
> Set to string value "crlf"
>
> This setting converts the file’s line endings in the working directory to CRLF when the file is checked out.
> Set to string value "lf"
>
> This setting uses the same line endings in the working directory as in the index when the file is checked out.
> ---
>
> I don't think this explanation is sufficient for files stored on Windows machines.
> A file might be stored with CRLF in the index.
> What happens then?
>This setting uses the same line endings in the working directory
>as in the index when the file is checked out.
If you have a file commited with CRLF into the index, it will have CRLF in the working tree.
If the file had been commited with LF into the index, it will have LF in the working tree.
That is what the documentation tries to say, I think.
However, the above discussion is only valid, when the "text" attribute is defined.
Otherwise the eol attribute has no effect.
So you need something like
* text=auto
and/or
*.txt text
in your .gitattributes file
> And what happens when a file is added to the index with `core.autocrlf=input` on Windows machines?
Do you have a .gitattributes file ? Or not ?
Is the file a new one, or does it exist ?
In any way,
git ls-files --eol
may help to find out, what is in the index and what is in the workingtree.
^ permalink raw reply
* [PATCH 0/9] for-each-ref optimizations & usability improvements
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye
This series is a bit of an informal follow-up to [1], adding some more
substantial optimizations and usability fixes around ref
filtering/formatting. Some of the changes here affect user-facing behavior,
some are internal-only, but they're all interdependent enough to warrant
putting them together in one series.
[1]
https://lore.kernel.org/git/pull.1594.v2.git.1696888736.gitgitgadget@gmail.com/
Patch 1 changes the behavior of the '--no-sort' option in 'for-each-ref',
'tag', and 'branch'. Currently, it just removes previous sort keys and, if
no further keys are specified, falls back on ascending refname sort (which,
IMO, makes the name '--no-sort' somewhat misleading).
Patch 2 updates the 'for-each-ref' docs to clearly state what happens if you
use '--omit-empty' and '--count' together. I based the explanation on what
the current behavior is (i.e., refs omitted with '--omit-empty' do count
towards the total limited by '--count').
Patches 3-7 incrementally refactor various parts of the ref
filtering/formatting workflows in order to create a
'filter_and_format_refs()' function. If certain conditions are met (sorting
disabled, no reachability filtering or ahead-behind formatting), ref
filtering & formatting is done within a single 'for_each_fullref_in'
callback. Especially in large repositories, this makes a huge difference in
memory usage & runtime for certain usages of 'for-each-ref', since it's no
longer writing everything to a 'struct ref_array' then repeatedly whittling
down/updating its contents.
Patch 8 introduces a new option to 'for-each-ref' called '--full-deref'.
When provided, any format fields for the dereferenced value of a tag (e.g.
"%(*objectname)") will be populated with the fully peeled target of the tag;
right now, those fields are populated with the immediate target of a tag
(which can be another tag). This avoids the need to pipe 'for-each-ref'
results to 'cat-file --batch-check' to get fully-peeled tag information. It
also benefits from the 'filter_and_format_refs()' single-iteration
optimization, since 'peel_iterated_oid()' may be able to read the
pre-computed peeled OID from a packed ref. A couple notes on this one:
* I went with a command line option for '--full-deref' rather than another
format specifier (like ** instead of *) because it seems unlikely that a
user is going to want to perform a shallow dereference and a full
dereference in the same 'for-each-ref'. There's also a NEEDSWORK going
all the way back to the introduction of 'for-each-ref' in 9f613ddd21c
(Add git-for-each-ref: helper for language bindings, 2006-09-15) that (to
me) implies different dereferencing behavior corresponds to different use
cases/user needs.
* I'm not attached to '--full-deref' as a name - if someone has an idea for
a more descriptive name, please suggest it!
Finally, patch 9 adds performance tests for 'for-each-ref', showing the
effects of optimizations made throughout the series. Here are some sample
results from my Ubuntu VM (test names shortened for space):
Test this branch
----------------------------------------------------------------------------
6300.2: (loose) 4.78(0.89+3.82)
6300.3: (loose, no sort) 4.51(0.86+3.58)
6300.4: (loose, --count=1) 4.70(0.90+3.73)
6300.5: (loose, --count=1, no sort) 4.35(0.58+3.73)
6300.6: (loose, tags) 2.45(0.44+1.95)
6300.7: (loose, tags, no sort) 2.38(0.44+1.90)
6300.8: (loose, tags, shallow deref) 3.33(1.27+1.99)
6300.9: (loose, tags, shallow deref, no sort) 3.29(1.29+1.93)
6300.10: (loose, tags, full deref) 3.76(1.69+1.99)
6300.11: (loose, tags, full deref, no sort) 3.73(1.71+1.94)
6300.12: for-each-ref + cat-file (loose, tags) 4.25(2.16+2.17)
6300.14: (packed) 0.61(0.50+0.09)
6300.15: (packed, no sort) 0.46(0.40+0.04)
6300.16: (packed, --count=1) 0.59(0.44+0.13)
6300.17: (packed, --count=1, no sort) 0.02(0.01+0.01)
6300.18: (packed, tags) 0.28(0.18+0.09)
6300.19: (packed, tags, no sort) 0.29(0.24+0.03)
6300.20: (packed, tags, shallow deref) 1.20(1.03+0.13)
6300.21: (packed, tags, shallow deref, no sort) 1.13(0.99+0.08)
6300.22: (packed, tags, full deref) 1.57(1.45+0.11)
6300.23: (packed, tags, full deref, no sort) 1.07(1.01+0.05)
6300.24: for-each-ref + cat-file (packed, tags) 2.01(1.81+0.33)
* Victoria
Victoria Dye (9):
ref-filter.c: really don't sort when using --no-sort
for-each-ref: clarify interaction of --omit-empty & --count
ref-filter.h: add max_count and omit_empty to ref_format
ref-filter.h: move contains caches into filter
ref-filter.h: add functions for filter/format & format-only
ref-filter.c: refactor to create common helper functions
ref-filter.c: filter & format refs in the same callback
for-each-ref: add option to fully dereference tags
t/perf: add perf tests for for-each-ref
Documentation/git-for-each-ref.txt | 12 +-
builtin/branch.c | 42 +++--
builtin/for-each-ref.c | 41 ++---
builtin/ls-remote.c | 10 +-
builtin/tag.c | 32 +---
ref-filter.c | 277 ++++++++++++++++++++---------
ref-filter.h | 26 +++
t/perf/p6300-for-each-ref.sh | 87 +++++++++
t/t3200-branch.sh | 68 ++++++-
t/t6300-for-each-ref.sh | 55 ++++++
t/t7004-tag.sh | 45 +++++
11 files changed, 532 insertions(+), 163 deletions(-)
create mode 100755 t/perf/p6300-for-each-ref.sh
base-commit: bc5204569f7db44d22477485afd52ea410d83743
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1609%2Fvdye%2Fvdye%2Ffor-each-ref-optimizations-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1609/vdye/vdye/for-each-ref-optimizations-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1609
--
gitgitgadget
^ permalink raw reply
* [PATCH 2/9] for-each-ref: clarify interaction of --omit-empty & --count
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Update the 'for-each-ref' builtin documentation to clarify that refs
"omitted" by --omit-empty are still counted toward the limit specified by
--count. The use of the term "omit" would otherwise be somewhat ambiguous
and could incorrectly be construed as excluding empty refs entirely (i.e.
not counting them towards the total ref count).
Signed-off-by: Victoria Dye <vdye@github.com>
---
Documentation/git-for-each-ref.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index e86d5700ddf..407f624fbaa 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,7 +101,8 @@ OPTIONS
--omit-empty::
Do not print a newline after formatted refs where the format expands
- to the empty string.
+ to the empty string. Although omitted refs are not shown in the output,
+ they still count toward the total limited by `--count`.
--exclude=<pattern>::
If one or more patterns are given, only refs which do not match
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/9] ref-filter.c: really don't sort when using --no-sort
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Update 'ref_sorting_options()' to return a NULL 'struct ref_sorting *' if
the string list provided to it is empty, rather than returning the default
refname sort structure. Also update 'ref_array_sort()' to explicitly skip
sorting if its 'struct ref_sorting *' arg is NULL. Other functions using
'struct ref_sorting *' do not need any changes because they already properly
ignore NULL values.
The goal of this change is to have the '--no-sort' option truly disable
sorting in commands like 'for-each-ref, 'tag', and 'branch'. Right now,
'--no-sort' will still trigger refname sorting by default in 'for-each-ref',
'tag', and 'branch'.
To match existing behavior as closely as possible, explicitly add "refname"
to the list of sort keys in 'for-each-ref', 'tag', and 'branch' before
parsing options (if no config-based sort keys are set). This ensures that
sorting will only be fully disabled if '--no-sort' is provided as an option;
otherwise, "refname" sorting will remain the default. Note: this also means
that even when sort keys are provided on the command line, "refname" will be
the final sort key in the sorting structure. This doesn't actually change
any behavior, since 'compare_refs()' already falls back on comparing
refnames if two refs are equal w.r.t all other sort keys.
Finally, remove the condition around sorting in 'ls-remote', since it's no
longer necessary. Unlike 'for-each-ref' et. al., it does *not* set any sort
keys by default. The default empty list of sort keys will produce a NULL
'struct ref_sorting *', which causes the sorting to be skipped in
'ref_array_sort()'.
Signed-off-by: Victoria Dye <vdye@github.com>
---
builtin/branch.c | 6 ++++
builtin/for-each-ref.c | 3 ++
builtin/ls-remote.c | 10 ++----
builtin/tag.c | 6 ++++
ref-filter.c | 19 ++----------
t/t3200-branch.sh | 68 +++++++++++++++++++++++++++++++++++++++--
t/t6300-for-each-ref.sh | 21 +++++++++++++
t/t7004-tag.sh | 45 +++++++++++++++++++++++++++
8 files changed, 152 insertions(+), 26 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index e7ee9bd0f15..d67738bbcaa 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -767,7 +767,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_branch_usage, options);
+ /*
+ * Try to set sort keys from config. If config does not set any,
+ * fall back on default (refname) sorting.
+ */
git_config(git_branch_config, &sorting_options);
+ if (!sorting_options.nr)
+ string_list_append(&sorting_options, "refname");
track = git_branch_track;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 350bfa6e811..93b370f550b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -67,6 +67,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
git_config(git_default_config, NULL);
+ /* Set default (refname) sorting */
+ string_list_append(&sorting_options, "refname");
+
parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);
if (maxcount < 0) {
error("invalid --count argument: `%d'", maxcount);
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index fc765754305..436249b720c 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -58,6 +58,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
struct transport *transport;
const struct ref *ref;
struct ref_array ref_array;
+ struct ref_sorting *sorting;
struct string_list sorting_options = STRING_LIST_INIT_DUP;
struct option options[] = {
@@ -141,13 +142,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
item->symref = xstrdup_or_null(ref->symref);
}
- if (sorting_options.nr) {
- struct ref_sorting *sorting;
-
- sorting = ref_sorting_options(&sorting_options);
- ref_array_sort(sorting, &ref_array);
- ref_sorting_release(sorting);
- }
+ sorting = ref_sorting_options(&sorting_options);
+ ref_array_sort(sorting, &ref_array);
for (i = 0; i < ref_array.nr; i++) {
const struct ref_array_item *ref = ref_array.items[i];
diff --git a/builtin/tag.c b/builtin/tag.c
index 3918eacbb57..64f3196cd4c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -501,7 +501,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
setup_ref_filter_porcelain_msg();
+ /*
+ * Try to set sort keys from config. If config does not set any,
+ * fall back on default (refname) sorting.
+ */
git_config(git_tag_config, &sorting_options);
+ if (!sorting_options.nr)
+ string_list_append(&sorting_options, "refname");
memset(&opt, 0, sizeof(opt));
filter.lines = -1;
diff --git a/ref-filter.c b/ref-filter.c
index e4d3510e28e..7250089b7c6 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3142,7 +3142,8 @@ void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting,
void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
{
- QSORT_S(array->items, array->nr, compare_refs, sorting);
+ if (sorting)
+ QSORT_S(array->items, array->nr, compare_refs, sorting);
}
static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)
@@ -3248,18 +3249,6 @@ static int parse_sorting_atom(const char *atom)
return res;
}
-/* If no sorting option is given, use refname to sort as default */
-static struct ref_sorting *ref_default_sorting(void)
-{
- static const char cstr_name[] = "refname";
-
- struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
-
- sorting->next = NULL;
- sorting->atom = parse_sorting_atom(cstr_name);
- return sorting;
-}
-
static void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)
{
struct ref_sorting *s;
@@ -3283,9 +3272,7 @@ struct ref_sorting *ref_sorting_options(struct string_list *options)
struct string_list_item *item;
struct ref_sorting *sorting = NULL, **tail = &sorting;
- if (!options->nr) {
- sorting = ref_default_sorting();
- } else {
+ if (options->nr) {
for_each_string_list_item(item, options)
parse_ref_sorting(tail, item->string);
}
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 3182abde27f..9918ba05dec 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1570,9 +1570,10 @@ test_expect_success 'tracking with unexpected .fetch refspec' '
test_expect_success 'configured committerdate sort' '
git init -b main sort &&
+ test_config -C sort branch.sort "committerdate" &&
+
(
cd sort &&
- git config branch.sort committerdate &&
test_commit initial &&
git checkout -b a &&
test_commit a &&
@@ -1592,9 +1593,10 @@ test_expect_success 'configured committerdate sort' '
'
test_expect_success 'option override configured sort' '
+ test_config -C sort branch.sort "committerdate" &&
+
(
cd sort &&
- git config branch.sort committerdate &&
git branch --sort=refname >actual &&
cat >expect <<-\EOF &&
a
@@ -1606,10 +1608,70 @@ test_expect_success 'option override configured sort' '
)
'
+test_expect_success '--no-sort cancels config sort keys' '
+ test_config -C sort branch.sort "-refname" &&
+
+ (
+ cd sort &&
+
+ # objecttype is identical for all of them, so sort falls back on
+ # default (ascending refname)
+ git branch \
+ --no-sort \
+ --sort="objecttype" >actual &&
+ cat >expect <<-\EOF &&
+ a
+ * b
+ c
+ main
+ EOF
+ test_cmp expect actual
+ )
+
+'
+
+test_expect_success '--no-sort cancels command line sort keys' '
+ (
+ cd sort &&
+
+ # objecttype is identical for all of them, so sort falls back on
+ # default (ascending refname)
+ git branch \
+ --sort="-refname" \
+ --no-sort \
+ --sort="objecttype" >actual &&
+ cat >expect <<-\EOF &&
+ a
+ * b
+ c
+ main
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--no-sort without subsequent --sort prints expected branches' '
+ (
+ cd sort &&
+
+ # Sort the results with `sort` for a consistent comparison
+ # against expected
+ git branch --no-sort | sort >actual &&
+ cat >expect <<-\EOF &&
+ a
+ c
+ main
+ * b
+ EOF
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'invalid sort parameter in configuration' '
+ test_config -C sort branch.sort "v:notvalid" &&
+
(
cd sort &&
- git config branch.sort "v:notvalid" &&
# this works in the "listing" mode, so bad sort key
# is a dying offence.
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 00a060df0b5..0613e5e3623 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -1335,6 +1335,27 @@ test_expect_success '--no-sort cancels the previous sort keys' '
test_cmp expected actual
'
+test_expect_success '--no-sort without subsequent --sort prints expected refs' '
+ cat >expected <<-\EOF &&
+ refs/tags/multi-ref1-100000-user1
+ refs/tags/multi-ref1-100000-user2
+ refs/tags/multi-ref1-200000-user1
+ refs/tags/multi-ref1-200000-user2
+ refs/tags/multi-ref2-100000-user1
+ refs/tags/multi-ref2-100000-user2
+ refs/tags/multi-ref2-200000-user1
+ refs/tags/multi-ref2-200000-user2
+ EOF
+
+ # Sort the results with `sort` for a consistent comparison against
+ # expected
+ git for-each-ref \
+ --format="%(refname)" \
+ --no-sort \
+ "refs/tags/multi-*" | sort >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
test_when_finished "git checkout main" &&
git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e689db42929..b41a47eb943 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1862,6 +1862,51 @@ test_expect_success 'option override configured sort' '
test_cmp expect actual
'
+test_expect_success '--no-sort cancels config sort keys' '
+ test_config tag.sort "-refname" &&
+
+ # objecttype is identical for all of them, so sort falls back on
+ # default (ascending refname)
+ git tag -l \
+ --no-sort \
+ --sort="objecttype" \
+ "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.3
+ foo1.6
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--no-sort cancels command line sort keys' '
+ # objecttype is identical for all of them, so sort falls back on
+ # default (ascending refname)
+ git tag -l \
+ --sort="-refname" \
+ --no-sort \
+ --sort="objecttype" \
+ "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.3
+ foo1.6
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--no-sort without subsequent --sort prints expected tags' '
+ # Sort the results with `sort` for a consistent comparison against
+ # expected
+ git tag -l --no-sort "foo*" | sort >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.3
+ foo1.6
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'invalid sort parameter on command line' '
test_must_fail git tag -l --sort=notvalid "foo*" >actual
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/9] ref-filter.h: add max_count and omit_empty to ref_format
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Add an internal 'array_opts' struct to 'struct ref_format' containing
formatting options that pertain to the formatting of an entire ref array:
'max_count' and 'omit_empty'. These values are specified by the '--count'
and '--omit-empty' options, respectively, to 'for-each-ref'/'tag'/'branch'.
Storing these values in the 'ref_format' will simplify the consolidation of
ref array formatting logic across builtins in later patches.
Signed-off-by: Victoria Dye <vdye@github.com>
---
builtin/branch.c | 5 ++---
builtin/for-each-ref.c | 21 +++++++++++----------
builtin/tag.c | 5 ++---
ref-filter.h | 5 +++++
4 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index d67738bbcaa..5a1ec1cd04f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -45,7 +45,6 @@ static const char *head;
static struct object_id head_oid;
static int recurse_submodules = 0;
static int submodule_propagate_branches = 0;
-static int omit_empty = 0;
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
@@ -480,7 +479,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
string_list_append(output, out.buf);
} else {
fwrite(out.buf, 1, out.len, stdout);
- if (out.len || !omit_empty)
+ if (out.len || !format->array_opts.omit_empty)
putchar('\n');
}
}
@@ -737,7 +736,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
- OPT_BOOL(0, "omit-empty", &omit_empty,
+ OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty,
N_("do not output a newline after empty formatted refs")),
OPT_BIT('c', "copy", ©, N_("copy a branch and its reflog"), 1),
OPT_BIT('C', NULL, ©, N_("copy a branch, even if target exists"), 2),
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 93b370f550b..881c3ee055f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -19,10 +19,10 @@ static char const * const for_each_ref_usage[] = {
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
- int i;
+ int i, total;
struct ref_sorting *sorting;
struct string_list sorting_options = STRING_LIST_INIT_DUP;
- int maxcount = 0, icase = 0, omit_empty = 0;
+ int icase = 0;
struct ref_array array;
struct ref_filter filter = REF_FILTER_INIT;
struct ref_format format = REF_FORMAT_INIT;
@@ -40,11 +40,11 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
N_("quote placeholders suitably for python"), QUOTE_PYTHON),
OPT_BIT(0 , "tcl", &format.quote_style,
N_("quote placeholders suitably for Tcl"), QUOTE_TCL),
- OPT_BOOL(0, "omit-empty", &omit_empty,
+ OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty,
N_("do not output a newline after empty formatted refs")),
OPT_GROUP(""),
- OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
+ OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")),
OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")),
OPT__COLOR(&format.use_color, N_("respect format colors")),
OPT_REF_FILTER_EXCLUDE(&filter),
@@ -71,8 +71,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
string_list_append(&sorting_options, "refname");
parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);
- if (maxcount < 0) {
- error("invalid --count argument: `%d'", maxcount);
+ if (format.array_opts.max_count < 0) {
+ error("invalid --count argument: `%d'", format.array_opts.max_count);
usage_with_options(for_each_ref_usage, opts);
}
if (HAS_MULTI_BITS(format.quote_style)) {
@@ -109,15 +109,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
ref_array_sort(sorting, &array);
- if (!maxcount || array.nr < maxcount)
- maxcount = array.nr;
- for (i = 0; i < maxcount; i++) {
+ total = format.array_opts.max_count;
+ if (!total || array.nr < total)
+ total = array.nr;
+ for (i = 0; i < total; i++) {
strbuf_reset(&err);
strbuf_reset(&output);
if (format_ref_array_item(array.items[i], &format, &output, &err))
die("%s", err.buf);
fwrite(output.buf, 1, output.len, stdout);
- if (output.len || !omit_empty)
+ if (output.len || !format.array_opts.omit_empty)
putchar('\n');
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 64f3196cd4c..2d599245d48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -44,7 +44,6 @@ static const char * const git_tag_usage[] = {
static unsigned int colopts;
static int force_sign_annotate;
static int config_sign_tag = -1; /* unspecified */
-static int omit_empty = 0;
static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
struct ref_format *format)
@@ -83,7 +82,7 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
if (format_ref_array_item(array.items[i], format, &output, &err))
die("%s", err.buf);
fwrite(output.buf, 1, output.len, stdout);
- if (output.len || !omit_empty)
+ if (output.len || !format->array_opts.omit_empty)
putchar('\n');
}
@@ -481,7 +480,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
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_BOOL(0, "omit-empty", &omit_empty,
+ OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty,
N_("do not output a newline after empty formatted refs")),
OPT_REF_SORT(&sorting_options),
{
diff --git a/ref-filter.h b/ref-filter.h
index 1524bc463a5..d87d61238b7 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -92,6 +92,11 @@ struct ref_format {
/* List of bases for ahead-behind counts. */
struct string_list bases;
+
+ struct {
+ int max_count;
+ int omit_empty;
+ } array_opts;
};
#define REF_FILTER_INIT { \
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/9] ref-filter.h: move contains caches into filter
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Move the 'contains_cache' and 'no_contains_cache' used in filter_refs into
an 'internal' struct of the 'struct ref_filter'. In later patches, the
'struct ref_filter *' will be a common data structure across multiple
filtering functions. These caches are part of the common functionality the
filter struct will support, so they are updated to be internally accessible
wherever the filter is used.
The design used here is mirrors what was introduced in 576de3d956
(unpack_trees: start splitting internal fields from public API, 2023-02-27)
for 'unpack_trees_options'.
Signed-off-by: Victoria Dye <vdye@github.com>
---
ref-filter.c | 14 ++++++--------
ref-filter.h | 6 ++++++
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 7250089b7c6..5129b6986c9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2764,8 +2764,6 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
struct ref_filter_cbdata {
struct ref_array *array;
struct ref_filter *filter;
- struct contains_cache contains_cache;
- struct contains_cache no_contains_cache;
};
/*
@@ -2816,11 +2814,11 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
return 0;
/* We perform the filtering for the '--contains' option... */
if (filter->with_commit &&
- !commit_contains(filter, commit, filter->with_commit, &ref_cbdata->contains_cache))
+ !commit_contains(filter, commit, filter->with_commit, &filter->internal.contains_cache))
return 0;
/* ...or for the `--no-contains' option */
if (filter->no_commit &&
- commit_contains(filter, commit, filter->no_commit, &ref_cbdata->no_contains_cache))
+ commit_contains(filter, commit, filter->no_commit, &filter->internal.no_contains_cache))
return 0;
}
@@ -2989,8 +2987,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
save_commit_buffer_orig = save_commit_buffer;
save_commit_buffer = 0;
- init_contains_cache(&ref_cbdata.contains_cache);
- init_contains_cache(&ref_cbdata.no_contains_cache);
+ init_contains_cache(&filter->internal.contains_cache);
+ init_contains_cache(&filter->internal.no_contains_cache);
/* Simple per-ref filtering */
if (!filter->kind)
@@ -3014,8 +3012,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
head_ref(ref_filter_handler, &ref_cbdata);
}
- clear_contains_cache(&ref_cbdata.contains_cache);
- clear_contains_cache(&ref_cbdata.no_contains_cache);
+ clear_contains_cache(&filter->internal.contains_cache);
+ clear_contains_cache(&filter->internal.no_contains_cache);
/* Filters that need revision walking */
reach_filter(array, &filter->reachable_from, INCLUDE_REACHED);
diff --git a/ref-filter.h b/ref-filter.h
index d87d61238b7..0db3ff52889 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -7,6 +7,7 @@
#include "commit.h"
#include "string-list.h"
#include "strvec.h"
+#include "commit-reach.h"
/* Quoting styles */
#define QUOTE_NONE 0
@@ -75,6 +76,11 @@ struct ref_filter {
lines;
int abbrev,
verbose;
+
+ struct {
+ struct contains_cache contains_cache;
+ struct contains_cache no_contains_cache;
+ } internal;
};
struct ref_format {
--
gitgitgadget
^ permalink raw reply related
* [PATCH 5/9] ref-filter.h: add functions for filter/format & format-only
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Add two new public methods to 'ref-filter.h':
* 'print_formatted_ref_array()' which, given a format specification & array
of ref items, formats and prints the items to stdout.
* 'filter_and_format_refs()' which combines 'filter_refs()',
'ref_array_sort()', and 'print_formatted_ref_array()' into a single
function.
This consolidates much of the code used to filter and format refs in
'builtin/for-each-ref.c', 'builtin/tag.c', and 'builtin/branch.c', reducing
duplication and simplifying the future changes needed to optimize the filter
& format process.
Signed-off-by: Victoria Dye <vdye@github.com>
---
builtin/branch.c | 33 +++++++++++++++++----------------
builtin/for-each-ref.c | 27 +--------------------------
builtin/tag.c | 23 +----------------------
ref-filter.c | 35 +++++++++++++++++++++++++++++++++++
ref-filter.h | 14 ++++++++++++++
5 files changed, 68 insertions(+), 64 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 5a1ec1cd04f..2ed59f16f1c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -437,8 +437,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
{
int i;
struct ref_array array;
- struct strbuf out = STRBUF_INIT;
- struct strbuf err = STRBUF_INIT;
int maxwidth = 0;
const char *remote_prefix = "";
char *to_free = NULL;
@@ -468,24 +466,27 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
filter_ahead_behind(the_repository, format, &array);
ref_array_sort(sorting, &array);
- for (i = 0; i < array.nr; i++) {
- strbuf_reset(&err);
- strbuf_reset(&out);
- if (format_ref_array_item(array.items[i], format, &out, &err))
- die("%s", err.buf);
- if (column_active(colopts)) {
- assert(!filter->verbose && "--column and --verbose are incompatible");
- /* format to a string_list to let print_columns() do its job */
+ if (column_active(colopts)) {
+ struct strbuf out = STRBUF_INIT, err = STRBUF_INIT;
+
+ assert(!filter->verbose && "--column and --verbose are incompatible");
+
+ for (i = 0; i < array.nr; i++) {
+ strbuf_reset(&err);
+ strbuf_reset(&out);
+ if (format_ref_array_item(array.items[i], format, &out, &err))
+ die("%s", err.buf);
+
+ /* format to a string_list to let print_columns() do its job */
string_list_append(output, out.buf);
- } else {
- fwrite(out.buf, 1, out.len, stdout);
- if (out.len || !format->array_opts.omit_empty)
- putchar('\n');
}
+
+ strbuf_release(&err);
+ strbuf_release(&out);
+ } else {
+ print_formatted_ref_array(&array, format);
}
- strbuf_release(&err);
- strbuf_release(&out);
ref_array_clear(&array);
free(to_free);
}
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 881c3ee055f..1c19cd5bd34 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -19,15 +19,11 @@ static char const * const for_each_ref_usage[] = {
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
- int i, total;
struct ref_sorting *sorting;
struct string_list sorting_options = STRING_LIST_INIT_DUP;
int icase = 0;
- struct ref_array array;
struct ref_filter filter = REF_FILTER_INIT;
struct ref_format format = REF_FORMAT_INIT;
- struct strbuf output = STRBUF_INIT;
- struct strbuf err = STRBUF_INIT;
int from_stdin = 0;
struct strvec vec = STRVEC_INIT;
@@ -61,8 +57,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
OPT_END(),
};
- memset(&array, 0, sizeof(array));
-
format.format = "%(objectname) %(objecttype)\t%(refname)";
git_config(git_default_config, NULL);
@@ -104,27 +98,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
}
filter.match_as_path = 1;
- filter_refs(&array, &filter, FILTER_REFS_ALL);
- filter_ahead_behind(the_repository, &format, &array);
-
- ref_array_sort(sorting, &array);
-
- total = format.array_opts.max_count;
- if (!total || array.nr < total)
- total = array.nr;
- for (i = 0; i < total; i++) {
- strbuf_reset(&err);
- strbuf_reset(&output);
- if (format_ref_array_item(array.items[i], &format, &output, &err))
- die("%s", err.buf);
- fwrite(output.buf, 1, output.len, stdout);
- if (output.len || !format.array_opts.omit_empty)
- putchar('\n');
- }
+ filter_and_format_refs(&filter, FILTER_REFS_ALL, sorting, &format);
- strbuf_release(&err);
- strbuf_release(&output);
- ref_array_clear(&array);
ref_filter_clear(&filter);
ref_sorting_release(sorting);
strvec_clear(&vec);
diff --git a/builtin/tag.c b/builtin/tag.c
index 2d599245d48..2528d499dd8 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -48,13 +48,7 @@ static int config_sign_tag = -1; /* unspecified */
static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
struct ref_format *format)
{
- struct ref_array array;
- struct strbuf output = STRBUF_INIT;
- struct strbuf err = STRBUF_INIT;
char *to_free = NULL;
- int i;
-
- memset(&array, 0, sizeof(array));
if (filter->lines == -1)
filter->lines = 0;
@@ -72,23 +66,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
if (verify_ref_format(format))
die(_("unable to parse format string"));
filter->with_commit_tag_algo = 1;
- filter_refs(&array, filter, FILTER_REFS_TAGS);
- filter_ahead_behind(the_repository, format, &array);
- ref_array_sort(sorting, &array);
-
- for (i = 0; i < array.nr; i++) {
- strbuf_reset(&output);
- strbuf_reset(&err);
- if (format_ref_array_item(array.items[i], format, &output, &err))
- die("%s", err.buf);
- fwrite(output.buf, 1, output.len, stdout);
- if (output.len || !format->array_opts.omit_empty)
- putchar('\n');
- }
+ filter_and_format_refs(filter, FILTER_REFS_TAGS, sorting, format);
- strbuf_release(&err);
- strbuf_release(&output);
- ref_array_clear(&array);
free(to_free);
return 0;
diff --git a/ref-filter.c b/ref-filter.c
index 5129b6986c9..8992fbf45b1 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3023,6 +3023,18 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
return ret;
}
+void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
+ struct ref_sorting *sorting,
+ struct ref_format *format)
+{
+ struct ref_array array = { 0 };
+ filter_refs(&array, filter, type);
+ filter_ahead_behind(the_repository, format, &array);
+ ref_array_sort(sorting, &array);
+ print_formatted_ref_array(&array, format);
+ ref_array_clear(&array);
+}
+
static int compare_detached_head(struct ref_array_item *a, struct ref_array_item *b)
{
if (!(a->kind ^ b->kind))
@@ -3212,6 +3224,29 @@ int format_ref_array_item(struct ref_array_item *info,
return 0;
}
+void print_formatted_ref_array(struct ref_array *array, struct ref_format *format)
+{
+ int total;
+ struct strbuf output = STRBUF_INIT, err = STRBUF_INIT;
+
+ total = format->array_opts.max_count;
+ if (!total || array->nr < total)
+ total = array->nr;
+ for (int i = 0; i < total; i++) {
+ strbuf_reset(&err);
+ strbuf_reset(&output);
+ if (format_ref_array_item(array->items[i], format, &output, &err))
+ die("%s", err.buf);
+ if (output.len || !format->array_opts.omit_empty) {
+ fwrite(output.buf, 1, output.len, stdout);
+ putchar('\n');
+ }
+ }
+
+ strbuf_release(&err);
+ strbuf_release(&output);
+}
+
void pretty_print_ref(const char *name, const struct object_id *oid,
struct ref_format *format)
{
diff --git a/ref-filter.h b/ref-filter.h
index 0db3ff52889..0ce5af58ab3 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -137,6 +137,14 @@ struct ref_format {
* filtered refs in the ref_array structure.
*/
int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type);
+/*
+ * Filter refs using the given ref_filter and type, sort the contents
+ * according to the given ref_sorting, format the filtered refs with the
+ * given ref_format, and print them to stdout.
+ */
+void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
+ struct ref_sorting *sorting,
+ struct ref_format *format);
/* Clear all memory allocated to ref_array */
void ref_array_clear(struct ref_array *array);
/* Used to verify if the given format is correct and to parse out the used atoms */
@@ -161,6 +169,12 @@ char *get_head_description(void);
/* Set up translated strings in the output. */
void setup_ref_filter_porcelain_msg(void);
+/*
+ * Print up to maxcount ref_array elements to stdout using the given
+ * ref_format.
+ */
+void print_formatted_ref_array(struct ref_array *array, struct ref_format *format);
+
/*
* Print a single ref, outside of any ref-filter. Note that the
* name must be a fully qualified refname.
--
gitgitgadget
^ permalink raw reply related
* [PATCH 6/9] ref-filter.c: refactor to create common helper functions
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:25 UTC (permalink / raw)
To: git; +Cc: Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Factor out parts of 'ref_array_push()', 'ref_filter_handler()', and
'filter_refs()' into new helper functions ('ref_array_append()',
'apply_ref_filter()', and 'do_filter_refs()' respectively), as well as
rename 'ref_filter_handler()' to 'filter_one()'. In this and later
patches, these helpers will be used by new ref-filter API functions. This
patch does not result in any user-facing behavior changes or changes to
callers outside of 'ref-filter.c'.
The changes are as follows:
* The logic to grow a 'struct ref_array' and append a given 'struct
ref_array_item *' to it is extracted from 'ref_array_push()' into
'ref_array_append()'.
* 'ref_filter_handler()' is renamed to 'filter_one()' to more clearly
distinguish it from other ref filtering callbacks that will be added in
later patches. The "*_one()" naming convention is common throughout the
codebase for iteration callbacks.
* The code to filter a given ref by refname & object ID then create a new
'struct ref_array_item' is moved out of 'filter_one()' and into
'apply_ref_filter()'. 'apply_ref_filter()' returns either NULL (if the ref
does not match the given filter) or a 'struct ref_array_item *' created
with 'new_ref_array_item()'; 'filter_one()' appends that item to
its ref array with 'ref_array_append()'.
* The filter pre-processing, contains cache creation, and ref iteration of
'filter_refs()' is extracted into 'do_filter_refs()'. 'do_filter_refs()'
takes its ref iterator function & callback data as an input from the
caller, setting it up to be used with additional filtering callbacks in
later patches.
Signed-off-by: Victoria Dye <vdye@github.com>
---
ref-filter.c | 115 ++++++++++++++++++++++++++++++---------------------
1 file changed, 69 insertions(+), 46 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 8992fbf45b1..ff00ab4b8d8 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2716,15 +2716,18 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
return ref;
}
+static void ref_array_append(struct ref_array *array, struct ref_array_item *ref)
+{
+ ALLOC_GROW(array->items, array->nr + 1, array->alloc);
+ array->items[array->nr++] = ref;
+}
+
struct ref_array_item *ref_array_push(struct ref_array *array,
const char *refname,
const struct object_id *oid)
{
struct ref_array_item *ref = new_ref_array_item(refname, oid);
-
- ALLOC_GROW(array->items, array->nr + 1, array->alloc);
- array->items[array->nr++] = ref;
-
+ ref_array_append(array, ref);
return ref;
}
@@ -2761,46 +2764,36 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
return ref_kind_from_refname(refname);
}
-struct ref_filter_cbdata {
- struct ref_array *array;
- struct ref_filter *filter;
-};
-
-/*
- * A call-back given to for_each_ref(). Filter refs and keep them for
- * later object processing.
- */
-static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static struct ref_array_item *apply_ref_filter(const char *refname, const struct object_id *oid,
+ int flag, struct ref_filter *filter)
{
- struct ref_filter_cbdata *ref_cbdata = cb_data;
- struct ref_filter *filter = ref_cbdata->filter;
struct ref_array_item *ref;
struct commit *commit = NULL;
unsigned int kind;
if (flag & REF_BAD_NAME) {
warning(_("ignoring ref with broken name %s"), refname);
- return 0;
+ return NULL;
}
if (flag & REF_ISBROKEN) {
warning(_("ignoring broken ref %s"), refname);
- return 0;
+ return NULL;
}
/* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */
kind = filter_ref_kind(filter, refname);
if (!(kind & filter->kind))
- return 0;
+ return NULL;
if (!filter_pattern_match(filter, refname))
- return 0;
+ return NULL;
if (filter_exclude_match(filter, refname))
- return 0;
+ return NULL;
if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname))
- return 0;
+ return NULL;
/*
* A merge filter is applied on refs pointing to commits. Hence
@@ -2811,15 +2804,15 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
filter->with_commit || filter->no_commit || filter->verbose) {
commit = lookup_commit_reference_gently(the_repository, oid, 1);
if (!commit)
- return 0;
+ return NULL;
/* We perform the filtering for the '--contains' option... */
if (filter->with_commit &&
!commit_contains(filter, commit, filter->with_commit, &filter->internal.contains_cache))
- return 0;
+ return NULL;
/* ...or for the `--no-contains' option */
if (filter->no_commit &&
commit_contains(filter, commit, filter->no_commit, &filter->internal.no_contains_cache))
- return 0;
+ return NULL;
}
/*
@@ -2827,11 +2820,32 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
* to do its job and the resulting list may yet to be pruned
* by maxcount logic.
*/
- ref = ref_array_push(ref_cbdata->array, refname, oid);
+ ref = new_ref_array_item(refname, oid);
ref->commit = commit;
ref->flag = flag;
ref->kind = kind;
+ return ref;
+}
+
+struct ref_filter_cbdata {
+ struct ref_array *array;
+ struct ref_filter *filter;
+};
+
+/*
+ * A call-back given to for_each_ref(). Filter refs and keep them for
+ * later object processing.
+ */
+static int filter_one(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+{
+ struct ref_filter_cbdata *ref_cbdata = cb_data;
+ struct ref_array_item *ref;
+
+ ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter);
+ if (ref)
+ ref_array_append(ref_cbdata->array, ref);
+
return 0;
}
@@ -2967,26 +2981,12 @@ void filter_ahead_behind(struct repository *r,
free(commits);
}
-/*
- * API for filtering a set of refs. Based on the type of refs the user
- * has requested, we iterate through those refs and apply filters
- * as per the given ref_filter structure and finally store the
- * filtered refs in the ref_array structure.
- */
-int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type)
+static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref_fn fn, void *cb_data)
{
- struct ref_filter_cbdata ref_cbdata;
- int save_commit_buffer_orig;
int ret = 0;
- ref_cbdata.array = array;
- ref_cbdata.filter = filter;
-
filter->kind = type & FILTER_REFS_KIND_MASK;
- save_commit_buffer_orig = save_commit_buffer;
- save_commit_buffer = 0;
-
init_contains_cache(&filter->internal.contains_cache);
init_contains_cache(&filter->internal.no_contains_cache);
@@ -3001,20 +3001,43 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
* of filter_ref_kind().
*/
if (filter->kind == FILTER_REFS_BRANCHES)
- ret = for_each_fullref_in("refs/heads/", ref_filter_handler, &ref_cbdata);
+ ret = for_each_fullref_in("refs/heads/", fn, cb_data);
else if (filter->kind == FILTER_REFS_REMOTES)
- ret = for_each_fullref_in("refs/remotes/", ref_filter_handler, &ref_cbdata);
+ ret = for_each_fullref_in("refs/remotes/", fn, cb_data);
else if (filter->kind == FILTER_REFS_TAGS)
- ret = for_each_fullref_in("refs/tags/", ref_filter_handler, &ref_cbdata);
+ ret = for_each_fullref_in("refs/tags/", fn, cb_data);
else if (filter->kind & FILTER_REFS_ALL)
- ret = for_each_fullref_in_pattern(filter, ref_filter_handler, &ref_cbdata);
+ ret = for_each_fullref_in_pattern(filter, fn, cb_data);
if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
- head_ref(ref_filter_handler, &ref_cbdata);
+ head_ref(fn, cb_data);
}
clear_contains_cache(&filter->internal.contains_cache);
clear_contains_cache(&filter->internal.no_contains_cache);
+ return ret;
+}
+
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type)
+{
+ struct ref_filter_cbdata ref_cbdata;
+ int save_commit_buffer_orig;
+ int ret = 0;
+
+ ref_cbdata.array = array;
+ ref_cbdata.filter = filter;
+
+ save_commit_buffer_orig = save_commit_buffer;
+ save_commit_buffer = 0;
+
+ ret = do_filter_refs(filter, type, filter_one, &ref_cbdata);
+
/* Filters that need revision walking */
reach_filter(array, &filter->reachable_from, INCLUDE_REACHED);
reach_filter(array, &filter->unreachable_from, EXCLUDE_REACHED);
--
gitgitgadget
^ permalink raw reply related
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