* 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
* [PATCH 7/9] ref-filter.c: filter & format refs in the same callback
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 'filter_and_format_refs()' to try to perform ref filtering &
formatting in a single ref iteration, without an intermediate 'struct
ref_array'. This can only be done if no operations need to be performed on a
pre-filtered array; specifically, if the refs are
- filtered on reachability,
- sorted, or
- formatted with ahead-behind information
they cannot be filtered & formatted in the same iteration. In that case,
fall back on the current filter-then-sort-then-format flow.
This optimization substantially improves memory usage due to no longer
storing a ref array in memory. In some cases, it also dramatically reduces
runtime (e.g. 'git for-each-ref --no-sort --count=1', which no longer loads
all refs into a 'struct ref_array' to printing only the first ref).
Signed-off-by: Victoria Dye <vdye@github.com>
---
ref-filter.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 74 insertions(+), 6 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index ff00ab4b8d8..384cf1595ff 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2863,6 +2863,44 @@ static void free_array_item(struct ref_array_item *item)
free(item);
}
+struct ref_filter_and_format_cbdata {
+ struct ref_filter *filter;
+ struct ref_format *format;
+
+ struct ref_filter_and_format_internal {
+ int count;
+ } internal;
+};
+
+static int filter_and_format_one(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+{
+ struct ref_filter_and_format_cbdata *ref_cbdata = cb_data;
+ struct ref_array_item *ref;
+ struct strbuf output = STRBUF_INIT, err = STRBUF_INIT;
+
+ ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter);
+ if (!ref)
+ return 0;
+
+ if (format_ref_array_item(ref, ref_cbdata->format, &output, &err))
+ die("%s", err.buf);
+
+ if (output.len || !ref_cbdata->format->array_opts.omit_empty) {
+ fwrite(output.buf, 1, output.len, stdout);
+ putchar('\n');
+ }
+
+ strbuf_release(&output);
+ strbuf_release(&err);
+ free_array_item(ref);
+
+ if (ref_cbdata->format->array_opts.max_count &&
+ ++ref_cbdata->internal.count >= ref_cbdata->format->array_opts.max_count)
+ return -1;
+
+ return 0;
+}
+
/* Free all memory allocated for ref_array */
void ref_array_clear(struct ref_array *array)
{
@@ -3046,16 +3084,46 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
return ret;
}
+static inline int can_do_iterative_format(struct ref_filter *filter,
+ struct ref_sorting *sorting,
+ struct ref_format *format)
+{
+ /*
+ * Refs can be filtered and formatted in the same iteration as long
+ * as we aren't filtering on reachability, sorting the results, or
+ * including ahead-behind information in the formatted output.
+ */
+ return !(filter->reachable_from ||
+ filter->unreachable_from ||
+ sorting ||
+ format->bases.nr);
+}
+
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);
+ if (can_do_iterative_format(filter, sorting, format)) {
+ int save_commit_buffer_orig;
+ struct ref_filter_and_format_cbdata ref_cbdata = {
+ .filter = filter,
+ .format = format,
+ };
+
+ save_commit_buffer_orig = save_commit_buffer;
+ save_commit_buffer = 0;
+
+ do_filter_refs(filter, type, filter_and_format_one, &ref_cbdata);
+
+ save_commit_buffer = save_commit_buffer_orig;
+ } else {
+ 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)
--
gitgitgadget
^ permalink raw reply related
* [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:26 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 a boolean flag '--full-deref' that, when enabled, fills '%(*fieldname)'
format fields using the fully peeled target of tag objects, rather than the
immediate target.
In other builtins ('rev-parse', 'show-ref'), "dereferencing" tags typically
means peeling them down to their non-tag target. Unlike these commands,
'for-each-ref' dereferences only one "level" of tags in '*' format fields
(like "%(*objectname)"). For most annotated tags, one level of dereferencing
is enough, since most tags point to commits or trees. However, nested tags
(annotated tags whose target is another annotated tag) dereferenced once
will point to their target tag, different a full peel to e.g. a commit.
Currently, if a user wants to filter & format refs and include information
about the fully dereferenced tag, they can do so with something like
'cat-file --batch-check':
git for-each-ref --format="%(objectname)^{} %(refname)" <pattern> |
git cat-file --batch-check="%(objectname) %(rest)"
But the combination of commands is inefficient. So, to improve the
efficiency of this use case, add a '--full-deref' option that causes
'for-each-ref' to fully dereference tags when formatting with '*' fields.
Signed-off-by: Victoria Dye <vdye@github.com>
---
Documentation/git-for-each-ref.txt | 9 ++++++++
builtin/for-each-ref.c | 2 ++
ref-filter.c | 26 ++++++++++++++---------
ref-filter.h | 1 +
t/t6300-for-each-ref.sh | 34 ++++++++++++++++++++++++++++++
5 files changed, 62 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 407f624fbaa..2714a87088e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,6 +11,7 @@ SYNOPSIS
'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
[(--sort=<key>)...] [--format=<format>]
[ --stdin | <pattern>... ]
+ [--full-deref]
[--points-at=<object>]
[--merged[=<object>]] [--no-merged[=<object>]]
[--contains[=<object>]] [--no-contains[=<object>]]
@@ -77,6 +78,14 @@ OPTIONS
the specified host language. This is meant to produce
a scriptlet that can directly be `eval`ed.
+--full-deref::
+ Populate dereferenced format fields (indicated with an asterisk (`*`)
+ prefix before the fieldname) with information about the fully-peeled
+ target object of a tag ref, rather than its immediate target object.
+ This only affects the output for nested annotated tags, where the tag's
+ immediate target is another tag but its fully-peeled target is another
+ object type (e.g. a commit).
+
--points-at=<object>::
Only list refs which points at the given object.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1c19cd5bd34..7a2127a3bc4 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -43,6 +43,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
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_BOOL(0, "full-deref", &format.full_deref,
+ N_("fully dereference tags to populate '*' format fields")),
OPT_REF_FILTER_EXCLUDE(&filter),
OPT_REF_SORT(&sorting_options),
OPT_CALLBACK(0, "points-at", &filter.points_at,
diff --git a/ref-filter.c b/ref-filter.c
index 384cf1595ff..a66ac7921b1 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -237,7 +237,14 @@ static struct used_atom {
char *head;
} u;
} *used_atom;
-static int used_atom_cnt, need_tagged, need_symref;
+static int used_atom_cnt, need_symref;
+
+enum tag_dereference_mode {
+ NO_DEREF = 0,
+ DEREF_ONE,
+ DEREF_ALL
+};
+static enum tag_dereference_mode need_tagged;
/*
* Expand string, append it to strbuf *sb, then return error code ret.
@@ -1066,8 +1073,8 @@ static int parse_ref_filter_atom(struct ref_format *format,
memset(&used_atom[at].u, 0, sizeof(used_atom[at].u));
if (valid_atom[i].parser && valid_atom[i].parser(format, &used_atom[at], arg, err))
return -1;
- if (*atom == '*')
- need_tagged = 1;
+ if (*atom == '*' && !need_tagged)
+ need_tagged = format->full_deref ? DEREF_ALL : DEREF_ONE;
if (i == ATOM_SYMREF)
need_symref = 1;
return at;
@@ -2511,14 +2518,13 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
* If it is a tag object, see if we use a value that derefs
* the object, and if we do grab the object it refers to.
*/
- oi_deref.oid = *get_tagged_oid((struct tag *)obj);
+ if (need_tagged == DEREF_ALL) {
+ if (peel_iterated_oid(&obj->oid, &oi_deref.oid))
+ die("bad tag");
+ } else {
+ oi_deref.oid = *get_tagged_oid((struct tag *)obj);
+ }
- /*
- * NEEDSWORK: This derefs tag only once, which
- * is good to deal with chains of trust, but
- * is not consistent with what deref_tag() does
- * which peels the onion to the core.
- */
return get_object(ref, 1, &obj, &oi_deref, err);
}
diff --git a/ref-filter.h b/ref-filter.h
index 0ce5af58ab3..0caa39ecee5 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -92,6 +92,7 @@ struct ref_format {
const char *rest;
int quote_style;
int use_color;
+ int full_deref;
/* Internal state to ref-filter */
int need_color_reset_at_eol;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 0613e5e3623..3c2af785cdb 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -1839,6 +1839,40 @@ test_expect_success 'git for-each-ref with non-existing refs' '
test_must_be_empty actual
'
+test_expect_success 'git for-each-ref with nested tags' '
+ git tag -am "Normal tag" nested/base HEAD &&
+ git tag -am "Nested tag" nested/nest1 refs/tags/nested/base &&
+ git tag -am "Double nested tag" nested/nest2 refs/tags/nested/nest1 &&
+
+ head_oid="$(git rev-parse HEAD)" &&
+ base_tag_oid="$(git rev-parse refs/tags/nested/base)" &&
+ nest1_tag_oid="$(git rev-parse refs/tags/nested/nest1)" &&
+ nest2_tag_oid="$(git rev-parse refs/tags/nested/nest2)" &&
+
+ # Without full dereference
+ cat >expect <<-EOF &&
+ refs/tags/nested/base $base_tag_oid tag $head_oid commit
+ refs/tags/nested/nest1 $nest1_tag_oid tag $base_tag_oid tag
+ refs/tags/nested/nest2 $nest2_tag_oid tag $nest1_tag_oid tag
+ EOF
+
+ git for-each-ref --format="%(refname) %(objectname) %(objecttype) %(*objectname) %(*objecttype)" \
+ refs/tags/nested/ >actual &&
+ test_cmp expect actual &&
+
+ # With full dereference
+ cat >expect <<-EOF &&
+ refs/tags/nested/base $base_tag_oid tag $head_oid commit
+ refs/tags/nested/nest1 $nest1_tag_oid tag $head_oid commit
+ refs/tags/nested/nest2 $nest2_tag_oid tag $head_oid commit
+ EOF
+
+ git for-each-ref --full-deref \
+ --format="%(refname) %(objectname) %(objecttype) %(*objectname) %(*objecttype)" \
+ refs/tags/nested/ >actual &&
+ test_cmp expect actual
+'
+
GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
--
gitgitgadget
^ permalink raw reply related
* [PATCH 9/9] t/perf: add perf tests for for-each-ref
From: Victoria Dye via GitGitGadget @ 2023-11-07 1:26 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 performance tests for 'for-each-ref'. The tests exercise different
combinations of filters/formats/options, as well as the overall performance
of 'git for-each-ref | git cat-file --batch-check' to demonstrate the
performance difference vs. 'git for-each-ref --full-deref'.
All tests are run against a repository with 40k loose refs - 10k commits,
each having a unique:
- branch
- custom ref (refs/custom/special_*)
- annotated tag pointing at the commit
- annotated tag pointing at the other annotated tag (i.e., a nested tag)
After those tests are finished, the refs are packed with 'pack-refs --all'
and the same tests are rerun.
Signed-off-by: Victoria Dye <vdye@github.com>
---
t/perf/p6300-for-each-ref.sh | 87 ++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
create mode 100755 t/perf/p6300-for-each-ref.sh
diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
new file mode 100755
index 00000000000..172fd68a4e9
--- /dev/null
+++ b/t/perf/p6300-for-each-ref.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+
+test_description='performance of for-each-ref'
+. ./perf-lib.sh
+
+test_perf_fresh_repo
+
+ref_count_per_type=10000
+test_iteration_count=10
+
+test_expect_success "setup" '
+ test_commit_bulk $(( 1 + $ref_count_per_type )) &&
+
+ # Create refs
+ test_seq $ref_count_per_type |
+ sed "s,.*,update refs/heads/branch_& HEAD~&\nupdate refs/custom/special_& HEAD~&," |
+ git update-ref --stdin &&
+
+ # Create annotated tags
+ for i in $(test_seq $ref_count_per_type)
+ do
+ # Base tags
+ echo "tag tag_$i" &&
+ echo "mark :$i" &&
+ echo "from HEAD~$i" &&
+ printf "tagger %s <%s> %s\n" \
+ "$GIT_COMMITTER_NAME" \
+ "$GIT_COMMITTER_EMAIL" \
+ "$GIT_COMMITTER_DATE" &&
+ echo "data <<EOF" &&
+ echo "tag $i" &&
+ echo "EOF" &&
+
+ # Nested tags
+ echo "tag nested_$i" &&
+ echo "from :$i" &&
+ printf "tagger %s <%s> %s\n" \
+ "$GIT_COMMITTER_NAME" \
+ "$GIT_COMMITTER_EMAIL" \
+ "$GIT_COMMITTER_DATE" &&
+ echo "data <<EOF" &&
+ echo "nested tag $i" &&
+ echo "EOF" || return 1
+ done | git fast-import
+'
+
+test_for_each_ref () {
+ title="for-each-ref"
+ if test $# -gt 0; then
+ title="$title ($1)"
+ shift
+ fi
+ args="$@"
+
+ test_perf "$title" "
+ for i in \$(test_seq $test_iteration_count); do
+ git for-each-ref $args >/dev/null
+ done
+ "
+}
+
+run_tests () {
+ test_for_each_ref "$1"
+ test_for_each_ref "$1, no sort" --no-sort
+ test_for_each_ref "$1, tags" refs/tags/
+ test_for_each_ref "$1, tags, no sort" --no-sort refs/tags/
+ test_for_each_ref "$1, tags, shallow deref" '--format="%(refname) %(objectname) %(*objectname)"' refs/tags/
+ test_for_each_ref "$1, tags, shallow deref, no sort" --no-sort '--format="%(refname) %(objectname) %(*objectname)"' refs/tags/
+ test_for_each_ref "$1, tags, full deref" --full-deref '--format="%(refname) %(objectname) %(*objectname)"' refs/tags/
+ test_for_each_ref "$1, tags, full deref, no sort" --no-sort --full-deref '--format="%(refname) %(objectname) %(*objectname)"' refs/tags/
+
+ test_perf "for-each-ref ($1, tags) + cat-file --batch-check (full deref)" "
+ for i in \$(test_seq $test_iteration_count); do
+ git for-each-ref --format='%(objectname)^{} %(refname) %(objectname)' refs/tags/ | \
+ git cat-file --batch-check='%(objectname) %(rest)' >/dev/null
+ done
+ "
+}
+
+run_tests "loose"
+
+test_expect_success 'pack refs' '
+ git pack-refs --all
+'
+run_tests "packed"
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 0/2] RelNotes: minor changes in 2.43.0 draft
From: Junio C Hamano @ 2023-11-07 1:32 UTC (permalink / raw)
To: Todd Zullinger; +Cc: git
In-Reply-To: <20231103141759.864875-1-tmz@pobox.com>
Todd Zullinger <tmz@pobox.com> writes:
> These are minor changes from a quick reading of the 2.43.0
> release notes draft.
>
> Todd Zullinger (2):
> RelNotes: minor typo fixes in 2.43.0 draft
> RelNotes: improve wording of credential helper notes
>
> Documentation/RelNotes/2.43.0.txt | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Thanks. Applied.
^ permalink raw reply
* Re: [PATCH 0/2] revision: exclude all packed objects with `--unpacked`
From: Junio C Hamano @ 2023-11-07 1:42 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King
In-Reply-To: <cover.1699311386.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> 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.
Yeah, it does sound like a straight bugfix to me.
>
> [^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
^ permalink raw reply
* Re: [PATCH 2/2] pack-bitmap: drop --unpacked non-commit objects from results
From: Junio C Hamano @ 2023-11-07 2:20 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King
In-Reply-To: <7492dc699052a392d2fb394e1dcfabebac82ded0.1699311386.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
The same comment to the commit title applies here.
> .... 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.
;-)
Very nice. Since we are omitting any object that appears in some
packfile, this produces an expected result even when some of these
objects also appear in loose form.
> +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
> +'
OK.
^ permalink raw reply
* Re: [PATCH 0/4] Memory leak fixes
From: Junio C Hamano @ 2023-11-07 2:20 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git
In-Reply-To: <20231106173205.GD10414@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 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).
Thanks, both.
^ permalink raw reply
* Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results
From: Junio C Hamano @ 2023-11-07 2:21 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King
In-Reply-To: <d3992c98aaa54c3635c249a15d919aa1177324d8.1699311386.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> Subject: Re: [PATCH 1/2] list-objects: drop --unpacked non-commit objects from results
I thought the purpose of this change is to make sure that we drop
packed objects from results when --unpacked is given? This makes
it sound as if we are dropping unpacked ones instead? I dunno.
> 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.
Strictly speaking, as long as all the objects that are not in packs
are shown, "print the object IDs that are not in packs" is satisfied.
With this fix, perhaps we would want to tighten the explanation a
little bit while we are at it. Perhaps
print the object names but exclude those that are in packs
or something along that line?
> 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);
> }
The implementation is as straight-forward as it can get. Very
pleasing.
> 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
^ permalink raw reply
* Re: [PATCH 0/9] for-each-ref optimizations & usability improvements
From: Junio C Hamano @ 2023-11-07 2:36 UTC (permalink / raw)
To: Victoria Dye via GitGitGadget; +Cc: git, Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 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).
We can read it changes the behaviour and what the current behaviour
is, but I presume that the untold new behaviour with --no-sort is to
show the output in an unspecified order of implementation's
convenience? I think it makes quite a lot of sense if that is what
is done.
> 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').
OK.
> 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.
OK. I was wondering if you are going threaded implementation, until
I read into 6th line ;-)
> 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.
Makes quite a lot of sense.
> * I'm not attached to '--full-deref' as a name - if someone has an idea for
> a more descriptive name, please suggest it!
Another candidate verb may be "to peel", and I have no strong
opinion between it and "to dereference". But I have a mild aversion
to an abbreviation that is not strongly established.
> 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):
Nice.
^ permalink raw reply
* Re: [PATCH v6 00/14] Introduce new `git replay` command
From: Elijah Newren @ 2023-11-07 2:43 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <20231102135151.843758-1-christian.couder@gmail.com>
Hi,
Looking good, just one comment on one small hunk...
On Thu, Nov 2, 2023 at 6:52 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
[...]
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> -
> strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
>
> ++ /*
> ++ * TODO: For now, let's warn when we see an option that we are
> ++ * going to override after setup_revisions() below. In the
> ++ * future we might want to either die() or allow them if we
> ++ * think they could be useful though.
> ++ */
> ++ for (i = 0; i < argc; i++) {
> ++ if (!strcmp(argv[i], "--reverse") || !strcmp(argv[i], "--date-order") ||
> ++ !strcmp(argv[i], "--topo-order") || !strcmp(argv[i], "--author-date-order") ||
> ++ !strcmp(argv[i], "--full-history"))
> ++ warning(_("option '%s' will be overridden"), argv[i]);
> ++ }
> ++
Two things:
1) Not sure it makes sense to throw a warning with --topo-order or
--full-history, since they would result in a value matching what we
would be setting anyway.
2) This seems like an inefficient way to provide this warning; could
we avoid parsing the arguments for an extra time? Perhaps instead
a) set the desired values here, before setup_revisions()
b) after setup_revisions, check whether these values differ from the
desired values, if so throw a warning.
c) set the desired values, again
?
^ permalink raw reply
* Re: [PATCH] diff: implement config.diff.renames=copies-harder
From: Elijah Newren @ 2023-11-07 2:45 UTC (permalink / raw)
To: Sam James via GitGitGadget; +Cc: git, Sam James
In-Reply-To: <pull.1606.git.1699010701704.gitgitgadget@gmail.com>
Hi,
On Fri, Nov 3, 2023 at 4:25 AM Sam James via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Sam James <sam@gentoo.org>
>
> This patch adds a config value for 'diff.renames' called 'copies-harder'
> which make it so '-C -C' is in effect always passed for 'git log -p',
> 'git diff', etc.
>
> This allows specifying that 'git log -p', 'git diff', etc should always act
> as if '-C --find-copies-harder' was passed.
>
> I've found this especially useful for certain types of repository (like
> Gentoo's ebuild repositories) because files are often copies of a previous
> version.
These must be very small repositories? --find-copies-harder is really
expensive...
But, if you are willing to pay the price, the idea of making this a
configuration item makes sense.
> Signed-off-by: Sam James <sam@gentoo.org>
> ---
> diff: implement config.diff.renames=copies-harder
>
> This patch adds a config value for 'diff.renames' called 'copies-harder'
> which make it so '-C -C' is in effect always passed for 'git log -p',
> 'git diff', etc.
>
> This allows specifying that 'git log -p', 'git diff', etc should always
> act as if '-C --find-copies-harder' was passed.
>
> I've found this especially useful for certain types of repository (like
> Gentoo's ebuild repositories) because files are often copies of a
> previous version.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1606%2Fthesamesam%2Fconfig-copies-harder-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1606/thesamesam/config-copies-harder-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1606
>
> Documentation/config/diff.txt | 3 ++-
> Documentation/config/status.txt | 3 ++-
> diff.c | 12 +++++++++---
> diff.h | 1 +
> diffcore-rename.c | 4 ++--
> merge-ort.c | 2 +-
> merge-recursive.c | 2 +-
> 7 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index bd5ae0c3378..d2ff3c62d41 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -131,7 +131,8 @@ diff.renames::
> Whether and how Git detects renames. If set to "false",
> rename detection is disabled. If set to "true", basic rename
> detection is enabled. If set to "copies" or "copy", Git will
> - detect copies, as well. Defaults to true. Note that this
> + detect copies, as well. If set to "copies-harder", Git will try harder
> + to detect copies. Defaults to true. Note that this
"try harder to detect copies" feels like an unhelpful explanation. I
understand that a lengthy explanation (like the one found under the
`--find-copies-harder` option in git-diff) may not be wanted here
since we are trying to describe things succinctly, but could we at
least reference the `--find-copies-harder` option so that people know
where to go to get a more detailed explanation?
> affects only 'git diff' Porcelain like linkgit:git-diff[1] and
> linkgit:git-log[1], and not lower level commands such as
> linkgit:git-diff-files[1].
> diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
> index 2ff8237f8fc..7ca7a4becd7 100644
> --- a/Documentation/config/status.txt
> +++ b/Documentation/config/status.txt
> @@ -33,7 +33,8 @@ status.renames::
> Whether and how Git detects renames in linkgit:git-status[1] and
> linkgit:git-commit[1] . If set to "false", rename detection is
> disabled. If set to "true", basic rename detection is enabled.
> - If set to "copies" or "copy", Git will detect copies, as well.
> + If set to "copies" or "copy", Git will detect copies, as well. If
> + set to "copies-harder", Git will try harder to detect copies.
Same here.
> Defaults to the value of diff.renames.
>
> status.showStash::
> diff --git a/diff.c b/diff.c
> index 2c602df10a3..0ca906611f5 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -206,8 +206,11 @@ int git_config_rename(const char *var, const char *value)
> {
> if (!value)
> return DIFF_DETECT_RENAME;
> + if (!strcasecmp(value, "copies-harder"))
> + return DIFF_DETECT_COPY_HARDER;
> if (!strcasecmp(value, "copies") || !strcasecmp(value, "copy"))
> - return DIFF_DETECT_COPY;
> + return DIFF_DETECT_COPY;
> +
As per CodingGuidelines:
"""
- Fixing style violations while working on a real change as a
preparatory clean-up step is good, but otherwise avoid useless code
churn for the sake of conforming to the style.
"""
So, the fixing of extra space and the extra blank line should be
placed in a separate patch.
> return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
> }
>
> @@ -4832,8 +4835,11 @@ void diff_setup_done(struct diff_options *options)
> else
> options->flags.diff_from_contents = 0;
>
> - if (options->flags.find_copies_harder)
> + /* Just fold this in as it makes the patch-to-git smaller */
> + if (options->flags.find_copies_harder || options->detect_rename == DIFF_DETECT_COPY_HARDER) {
As per CodingGuidelines, this line is too long and should be split
across two lines at the `||`.
> + options->flags.find_copies_harder = 1;
> options->detect_rename = DIFF_DETECT_COPY;
> + }
>
> if (!options->flags.relative_name)
> options->prefix = NULL;
> @@ -5264,7 +5270,7 @@ static int diff_opt_find_copies(const struct option *opt,
> if (*arg != 0)
> return error(_("invalid argument to %s"), opt->long_name);
>
> - if (options->detect_rename == DIFF_DETECT_COPY)
> + if (options->detect_rename == DIFF_DETECT_COPY || options->detect_rename == DIFF_DETECT_COPY_HARDER)
Also too long.
> options->flags.find_copies_harder = 1;
> else
> options->detect_rename = DIFF_DETECT_COPY;
> diff --git a/diff.h b/diff.h
> index 66bd8aeb293..b29e5b777f8 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -555,6 +555,7 @@ int git_config_rename(const char *var, const char *value);
>
> #define DIFF_DETECT_RENAME 1
> #define DIFF_DETECT_COPY 2
> +#define DIFF_DETECT_COPY_HARDER 3
>
> #define DIFF_PICKAXE_ALL 1
> #define DIFF_PICKAXE_REGEX 2
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 5a6e2bcac71..856291d66f2 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -299,7 +299,7 @@ static int find_identical_files(struct hashmap *srcs,
> }
> /* Give higher scores to sources that haven't been used already */
> score = !source->rename_used;
> - if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY)
> + if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY && options->detect_rename != DIFF_DETECT_COPY_HARDER)
This line should also be split.
> continue;
> score += basename_same(source, target);
> if (score > best_score) {
> @@ -1405,7 +1405,7 @@ void diffcore_rename_extended(struct diff_options *options,
> trace2_region_enter("diff", "setup", options->repo);
> info.setup = 0;
> assert(!dir_rename_count || strmap_empty(dir_rename_count));
> - want_copies = (detect_rename == DIFF_DETECT_COPY);
> + want_copies = (detect_rename == DIFF_DETECT_COPY || detect_rename == DIFF_DETECT_COPY_HARDER);
and so should this one.
> if (dirs_removed && (break_idx || want_copies))
> BUG("dirs_removed incompatible with break/copy detection");
> if (break_idx && relevant_sources)
> diff --git a/merge-ort.c b/merge-ort.c
> index 6491070d965..77498354652 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -4782,7 +4782,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
> * sanity check them anyway.
> */
> assert(opt->detect_renames >= -1 &&
> - opt->detect_renames <= DIFF_DETECT_COPY);
> + opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
> assert(opt->verbosity >= 0 && opt->verbosity <= 5);
> assert(opt->buffer_output <= 2);
> assert(opt->obuf.len == 0);
> diff --git a/merge-recursive.c b/merge-recursive.c
> index e3beb0801b1..d52dd536606 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -3708,7 +3708,7 @@ static int merge_start(struct merge_options *opt, struct tree *head)
> assert(opt->branch1 && opt->branch2);
>
> assert(opt->detect_renames >= -1 &&
> - opt->detect_renames <= DIFF_DETECT_COPY);
> + opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
> assert(opt->detect_directory_renames >= MERGE_DIRECTORY_RENAMES_NONE &&
> opt->detect_directory_renames <= MERGE_DIRECTORY_RENAMES_TRUE);
> assert(opt->rename_limit >= -1);
>
> base-commit: 692be87cbba55e8488f805d236f2ad50483bd7d5
> --
> gitgitgadget
The overall patch makes sense and looks good, modulo some minor
stylistic things that need cleaning up.
^ permalink raw reply
* Re: [PATCH 0/9] for-each-ref optimizations & usability improvements
From: Victoria Dye @ 2023-11-07 2:48 UTC (permalink / raw)
To: Junio C Hamano, Victoria Dye via GitGitGadget; +Cc: git
In-Reply-To: <xmqqo7g69tmf.fsf@gitster.g>
Junio C Hamano wrote:
> "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> 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).
>
> We can read it changes the behaviour and what the current behaviour
> is, but I presume that the untold new behaviour with --no-sort is to
> show the output in an unspecified order of implementation's
> convenience? I think it makes quite a lot of sense if that is what
> is done.
Ah sorry, I over-edited my cover letter and accidentally removed the
explanation of what this patch does! Yes - the new behavior is that
'--no-sort' (assuming there are no subsequent --sort=<something> options)
will completely skip sorting the filtered refs.
>> * I'm not attached to '--full-deref' as a name - if someone has an idea for
>> a more descriptive name, please suggest it!
>
> Another candidate verb may be "to peel", and I have no strong
> opinion between it and "to dereference". But I have a mild aversion
> to an abbreviation that is not strongly established.
>
Makes sense. I got the "deref" abbreviation for 'update-ref --no-deref', but
'show-ref' has a "--dereference" option and protocol v2's "ls-refs" includes
a "peel" arg. "Dereference" is the term already used in the 'for-each-ref'
documentation, though, so if no one comes in with an especially strong
opinion on this I'll change the option to '--full-dereference'. Thanks!
^ permalink raw reply
* Re: [PATCH 0/9] for-each-ref optimizations & usability improvements
From: Junio C Hamano @ 2023-11-07 3:04 UTC (permalink / raw)
To: Victoria Dye; +Cc: Victoria Dye via GitGitGadget, git
In-Reply-To: <dbcbcf0e-aeee-4bb9-9e39-e2e85194d083@github.com>
Victoria Dye <vdye@github.com> writes:
> Ah sorry, I over-edited my cover letter and accidentally removed the
> explanation of what this patch does! Yes - the new behavior is that
> '--no-sort' (assuming there are no subsequent --sort=<something> options)
> will completely skip sorting the filtered refs.
Makes sense.
And the way to countermand "--no-sort" that appears earlier on the
command line to revert to the default sort order is "--sort" that
uses "refname" as the sort key, which is also nice.
^ 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