* RE: [Potential Bug] Test t0301.34 hangs - Git v2.43.0-rc2
From: rsbecker @ 2023-11-14 22:18 UTC (permalink / raw)
To: 'Todd Zullinger'; +Cc: 'Junio C Hamano', git
In-Reply-To: <ZVPiQKhem7ew8o_8@pobox.com>
On Tuesday, November 14, 2023 4:10 PM, Todd Zullinger wrote:
>rsbecker@nexbridge.com wrote:
>> When running the full suite, I found that t0301.34 hangs on NonStop
>> x86 (Big Endian). No details at this point - will rerun this, but this
>> is a regression from rc1.
>
>FWIW, this test ran fine on Fedora's s390x architecture.
>That's little solace, I know, but may help rule out some potential causes.
>
> t0301-credential-cache.sh ..........................
> ...
> ok 34 - helper (cache) can forget user
> ...
> # passed all 44 test(s)
>
>The build log is available here (for a few weeks or so -- it was only a
test build):
>
>https://kojipkgs.fedoraproject.org//work/tasks/4976/109024976/build.log
Well... it looks transient. I reran the test with and without verbose in ksh
and bash without any difficulty. It might just be a timing issue.
I guess we can ignore this one for now unless it becomes persistent.
^ permalink raw reply
* Re: [PATCH v5 5/5] builtin/merge-tree.c: implement support for `--write-pack`
From: Jeff King @ 2023-11-14 22:04 UTC (permalink / raw)
To: Taylor Blau
Cc: Elijah Newren, git, Eric W. Biederman, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <ZVKkgpiFaOwwDcdw@nand.local>
On Mon, Nov 13, 2023 at 05:34:42PM -0500, Taylor Blau wrote:
> > It might not be too hard to just let in-process callers access the
> > objects we've written. A quick and dirty patch is below, which works
> > with the test you suggested (the test still fails because it finds a
> > conflict, but it gets past the "woah, I can't find that sha1" part).
>
> That's a very slick idea, and I think that this series has some legs to
> stand on now as a result.
I'm glad people seem to like it. ;)
I was mostly throwing it out there as an illustration, and I don't plan
on polishing it up further. But in case somebody else wants to work on
it, here are random extra thoughts on the topic:
- rather than teach packfile.c about bulk checkin, I think it might be
cleaner to let packed_git structs have a function pointer for
accessing the index (and if NULL, naturally we'd open the .idx file
in the usual way). And then bulk checkin could just register the
"fake" packed_git
- the flip side, though, is: would it be weird for other parts of the
code to ever see the fake bulk packed_git in the list? It doesn't
represent a real packfile the way the other ones do. So maybe it is
better to keep it separate
- I suspect this scheme may violate some bounds-checking of the
packfiles. For example, we haven't written the hashfile trailer yet in
the still-open packfile. But I think use_pack() assumes we always
have an extra the_hash_algo->rawsz bytes of padding at the end. I'm
not sure if that would ever cause us to accidentally read past the
end of the file.
- as you mentioned, an oidmap would be an obvious replacement for the
existing unsorted list
- the existing already_written() function faces the same problem. I
don't think anybody cared because we'd usually only bulk checkin a
few huge objects. But with --write-pack, we might write a lot of
such objects, and the linear scan in already_written() makes it
accidentally quadratic.
- speaking of already_written(): it calls repo_has_object_file() to
see if we can elide the write. It probably should be using
freshen_*_object() in the usual way. Again, this is an existing bug
but I suspect nobody noticed because the bulk checkin code path
hardly ever kicks in.
-Peff
^ permalink raw reply
* Re: [PATCH v5 5/5] builtin/merge-tree.c: implement support for `--write-pack`
From: Jeff King @ 2023-11-14 21:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: Taylor Blau, Elijah Newren, git, Eric W. Biederman,
Patrick Steinhardt
In-Reply-To: <xmqqpm0d1591.fsf@gitster.g>
On Tue, Nov 14, 2023 at 10:40:58AM +0900, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I posted an alternative in response to Elijah; the general idea being to
> > allow the usual object-lookup code to access the in-progress pack. That
> > would keep us limited to a single pack.
>
> If such a mechanism is done in a generic way, would we be able to
> simplify fast-import a lot, I wonder? IIRC, it had quite a lot of
> code to remember what it has written to its output to work around
> the exact issue your alternative tries to solve. In fact, maybe we
> could make fast-import a thin wrapper around the bulk checkin
> infrastructure?
I suspect that the implementation could be shared with fast-import. I'm
not sure it would save all that much code, though. There's a lot going
on in fast-import besides keeping track of which objects we wrote into a
pack. ;)
The bigger issue, though, is that fast-import does generate some deltas
and the bulk checkin code does not. And I'm not sure how the bulk
checkin interface would expose that API (you need the caller to say
"...and I suspect this object might be a good delta base").
-Peff
^ permalink raw reply
* Re: [PATCH v5 5/5] builtin/merge-tree.c: implement support for `--write-pack`
From: Jeff King @ 2023-11-14 21:53 UTC (permalink / raw)
To: Elijah Newren
Cc: Taylor Blau, git, Eric W. Biederman, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <CABPp-BEV8Mxu=4=TFN=0o9n+o69kKQfNZd_Rhr1swxqgKwd90g@mail.gmail.com>
On Mon, Nov 13, 2023 at 06:50:08PM -0800, Elijah Newren wrote:
> merge-tree is the same as far as hooks; I'd rather just not have them,
> but if we did, they'd be a post-operation hook.
>
> In both cases, that makes hooks not much of a sticking point.
>
> External merge drivers, however...
I suspect external merge drivers are OK, in the sense that they should
not be looking up arbitrary objects anyway. We should hand the driver
what it needs via tempfiles, etc.
That said, I'm also OK with the notion that "--write-pack" is optional,
and if some features don't work with it, that's OK. It's nice if we can
flag that explicitly (rather than having things break in weird and
subtle ways), but I think we might not even know about an implicit
assumption until somebody runs across it in practice.
-Peff
^ permalink raw reply
* [PATCH 1/1] attr: add builtin objectmode values support
From: Joanna Wang @ 2023-11-14 21:49 UTC (permalink / raw)
To: gitster; +Cc: git, jojwang, tboegi
In-Reply-To: <xmqqbkbx11x2.fsf@gitster.g>
Gives all paths builtin objectmode values based on the paths' modes
(one of 100644, 100755, 120000, 040000, 160000). Users may use
this feature to filter by file types. For example a pathspec such as
':(attr:builtin_objectmode=160000)' could filter for submodules without
needing to have `builtin_objectmode=160000` to be set in .gitattributes
for every submodule path.
These values are also reflected in `git check-attr` results.
If the git_attr_direction is set to GIT_ATTR_INDEX or GIT_ATTR_CHECKIN
and a path is not found in the index, the value will be unspecified.
This patch also reserves the builtin_* attribute namespace for objectmode
and any future builtin attributes. Any user defined attributes using this
reserved namespace will result in a warning. This is a breaking change for
any existing builtin_* attributes.
Pathspecs with some builtin_* attribute name (excluding builtin_objectmode)
will behave like any attribute where there are no user specified values.
Signed-off-by: Joanna Wang <jojwang@google.com>
---
>But the usual practice is *not* to cover all paths with explicit
> attribute definition, isn't it?
Got it, the breaking change is that we're giving mode a value when
user may expect no value.
>When checking things out of the index, the index should be the
>source of the truth. If something is not in the index, is there a
>point in falling back to the workint tree state to decide if the
>thing should be checked out of the index?
I don't think working tree objectmode fallback makes sense for
GIT_ATTR_CHECKOUT and GIT_ATTR_INDEX. Updated patch implements
without fallback.
> static struct git_attr mode_attr;
>
> if (!mode_attr)
> mode_attr = git_attr("mode");
Is there an idiomatic way to check if this git_attr (struct or pointer)
has been initialized? I could not find an anything for C but maybe I've
missed a common way to do this in the codebase.
I also addressed the style nits. Thanks for the review/fedback so far.
Documentation/gitattributes.txt | 14 +++++++
attr.c | 67 +++++++++++++++++++++++++++++++--
t/t0003-attributes.sh | 44 ++++++++++++++++++++++
t/t6135-pathspec-with-attrs.sh | 25 ++++++++++++
4 files changed, 147 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 8c1793c148..84bad3e741 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -100,6 +100,20 @@ for a path to `Unspecified` state. This can be done by listing
the name of the attribute prefixed with an exclamation point `!`.
+RESERVED BUILTIN_* ATTRIBUTES
+-----------------------------
+
+builtin_* is a reserved namespace for builtin attribute values. Any
+user defined attributes under this namespace will result in a warning.
+
+`builtin_objectmode`
+^^^^^^^^^^^^^^^^^^^^
+This attribute is for filtering files by their file bit modes (40000,
+120000, 160000, 100755, 100644). e.g. ':(attr:builtin_objectmode=160000)'.
+You may also check these values with `git check-attr builtin_objectmode -- <file>`.
+If the object is not in the index `git check-attr --cached` will return unspecified.
+
+
EFFECTS
-------
diff --git a/attr.c b/attr.c
index e62876dfd3..8493f2c5c0 100644
--- a/attr.c
+++ b/attr.c
@@ -17,6 +17,7 @@
#include "utf8.h"
#include "quote.h"
#include "read-cache-ll.h"
+#include "refs.h"
#include "revision.h"
#include "object-store-ll.h"
#include "setup.h"
@@ -183,6 +184,15 @@ static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check)
}
}
+/*
+ * Atribute name cannot begin with "builtin_" which
+ * is a reserved namespace for built in attributes values.
+ */
+static int attr_name_reserved(const char *name)
+{
+ return !strncmp(name, "builtin_", strlen("builtin_"));
+}
+
static int attr_name_valid(const char *name, size_t namelen)
{
/*
@@ -315,7 +325,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
cp++;
len--;
}
- if (!attr_name_valid(cp, len)) {
+ if (!attr_name_valid(cp, len) || attr_name_reserved(cp)) {
report_invalid_attr(cp, len, src, lineno);
return NULL;
}
@@ -379,7 +389,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
name += strlen(ATTRIBUTE_MACRO_PREFIX);
name += strspn(name, blank);
namelen = strcspn(name, blank);
- if (!attr_name_valid(name, namelen)) {
+ if (!attr_name_valid(name, namelen) || attr_name_reserved(name)) {
report_invalid_attr(name, namelen, src, lineno);
goto fail_return;
}
@@ -1240,6 +1250,57 @@ static struct object_id *default_attr_source(void)
return &attr_source;
}
+static const char *builtin_object_mode_attr(struct index_state *istate, const char *path)
+{
+ unsigned int mode;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (direction == GIT_ATTR_CHECKIN) {
+ struct object_id oid;
+ struct stat st;
+ if (lstat(path, &st))
+ die_errno(_("unable to stat '%s'"), path);
+ mode = canon_mode(st.st_mode);
+ if (S_ISDIR(mode)) {
+ /* `path` is either a directory or it is a submodule,
+ * in which case it is already indexed as submodule
+ * or it does not exist in the index yet and we need to
+ * check if we can resolve to a ref.
+ */
+ int pos = index_name_pos(istate, path, strlen(path));
+ if (pos >= 0) {
+ if (S_ISGITLINK(istate->cache[pos]->ce_mode))
+ mode = istate->cache[pos]->ce_mode;
+ } else if (resolve_gitlink_ref(path, "HEAD", &oid) == 0)
+ mode = S_IFGITLINK;
+ }
+ } else {
+ /*
+ * For GIT_ATTR_CHECKOUT and GIT_ATTR_INDEX we only check
+ * for mode in the index.
+ */
+ int pos = index_name_pos(istate, path, strlen(path));
+ if (pos >= 0)
+ mode = istate->cache[pos]->ce_mode;
+ else
+ return ATTR__UNSET;
+ }
+ strbuf_addf(&sb, "%06o", mode);
+ return sb.buf;
+
+}
+
+
+static const char *compute_builtin_attr(struct index_state *istate,
+ const char *path,
+ const struct git_attr *attr) {
+ const struct git_attr *object_mode_attr = git_attr("builtin_objectmode");
+
+ if (attr == object_mode_attr)
+ return builtin_object_mode_attr(istate, path);
+ return ATTR__UNSET;
+}
+
void git_check_attr(struct index_state *istate,
const char *path,
struct attr_check *check)
@@ -1253,7 +1314,7 @@ void git_check_attr(struct index_state *istate,
unsigned int n = check->items[i].attr->attr_nr;
const char *value = check->all_attrs[n].value;
if (value == ATTR__UNKNOWN)
- value = ATTR__UNSET;
+ value = compute_builtin_attr(istate, path, check->all_attrs[n].attr);
check->items[i].value = value;
}
}
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index aee2298f01..25aa3fbd05 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -19,6 +19,16 @@ attr_check () {
test_must_be_empty err
}
+attr_check_object_mode () {
+ path="$1" &&
+ expect="$2" &&
+ check_opts="$3" &&
+ git check-attr $check_opts builtin_objectmode -- "$path" >actual 2>err &&
+ echo "$path: builtin_objectmode: $expect" >expect &&
+ test_cmp expect actual
+ test_must_be_empty err
+}
+
attr_check_quote () {
path="$1" quoted_path="$2" expect="$3" &&
@@ -558,4 +568,38 @@ test_expect_success EXPENSIVE 'large attributes file ignored in index' '
test_cmp expect err
'
+test_expect_success 'native object mode attributes work' '
+ >exec && chmod +x exec && attr_check_object_mode exec 100755 &&
+ >normal && attr_check_object_mode normal 100644 &&
+ mkdir dir && attr_check_object_mode dir 040000 &&
+ >to_sym ln -s to_sym sym && attr_check_object_mode sym 120000
+'
+
+test_expect_success 'native object mode attributes work with --cached' '
+ >normal && attr_check_object_mode normal unspecified --cached &&
+ git add normal && attr_check_object_mode normal 100644 --cached
+'
+
+test_expect_success 'check object mode attributes work for submodules' '
+ mkdir sub &&
+ (
+ cd sub &&
+ git init &&
+ mv .git .real &&
+ echo "gitdir: .real" >.git &&
+ test_commit first
+ ) &&
+ attr_check_object_mode sub 160000 &&
+ attr_check_object_mode sub unspecified --cached &&
+ git add sub &&
+ attr_check_object_mode sub 160000 --cached
+'
+
+test_expect_success 'we do not allow user defined builtin_* attributes' '
+ echo "foo* builtin_foo" >.gitattributes &&
+ git add .gitattributes 2>actual &&
+ echo "builtin_foo is not a valid attribute name: .gitattributes:1" >expect &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index a9c1e4e0ec..b08a32ea68 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -295,4 +295,29 @@ test_expect_success 'reading from .gitattributes in a subdirectory (3)' '
test_cmp expect actual
'
+test_expect_success 'pathspec with builtin_objectmode attr can be used' '
+ >mode_exec_file_1 &&
+
+ git status -s ":(attr:builtin_objectmode=100644)mode_exec_*" >actual &&
+ echo ?? mode_exec_file_1 >expect &&
+ test_cmp expect actual &&
+
+ git add mode_exec_file_1 && chmod +x mode_exec_file_1 &&
+ git status -s ":(attr:builtin_objectmode=100755)mode_exec_*" >actual &&
+ echo AM mode_exec_file_1 >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'builtin_objectmode attr can be excluded' '
+ >mode_1_regular &&
+ >mode_1_exec && chmod +x mode_1_exec &&
+ git status -s ":(exclude,attr:builtin_objectmode=100644)" "mode_1_*" >actual &&
+ echo ?? mode_1_exec >expect &&
+ test_cmp expect actual &&
+
+ git status -s ":(exclude,attr:builtin_objectmode=100755)" "mode_1_*" >actual &&
+ echo ?? mode_1_regular >expect &&
+ test_cmp expect actual
+'
+
test_done
--
2.43.0.rc0.421.g78406f8d94-goog
^ permalink raw reply related
* [PATCH v5 3/3] rebase: rewrite --(no-)autosquash documentation
From: Andy Koppe @ 2023-11-14 21:43 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood, Andy Koppe
In-Reply-To: <20231114214339.10925-1-andy.koppe@gmail.com>
Rewrite the description of the rebase --(no-)autosquash options to try
to make it a bit clearer. Don't use "the '...'" to refer to part of a
commit message, mention how --interactive can be used to review the
todo list, and add a bit more detail on commit --squash/amend.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/git-rebase.txt | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 10548e715c..1dd6555f66 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -589,21 +589,27 @@ See also INCOMPATIBLE OPTIONS below.
--autosquash::
--no-autosquash::
- When the commit log message begins with "squash! ..." or "fixup! ..."
- or "amend! ...", and there is already a commit in the todo list that
- matches the same `...`, automatically modify the todo list of
- `rebase`, so that the commit marked for squashing comes right after
- the commit to be modified, and change the action of the moved commit
- from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
- matches the `...` if the commit subject matches, or if the `...` refers
- to the commit's hash. As a fall-back, partial matches of the commit
- subject work, too. The recommended way to create fixup/amend/squash
- commits is by using the `--fixup`, `--fixup=amend:` or `--fixup=reword:`
- and `--squash` options respectively of linkgit:git-commit[1].
+ Automatically squash commits with specially formatted messages into
+ previous commits being rebased. If a commit message starts with
+ "squash! ", "fixup! " or "amend! ", the remainder of the subject line
+ is taken as a commit specifier, which matches a previous commit if it
+ matches the subject line or the hash of that commit. If no commit
+ matches fully, matches of the specifier with the start of commit
+ subjects are considered.
+
-If the `--autosquash` option is enabled by default using the
-configuration variable `rebase.autoSquash`, this option can be
-used to override and disable this setting.
+In the rebase todo list, the actions of squash, fixup and amend commits are
+changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
+are moved right after the commit they modify. The `--interactive` option can
+be used to review and edit the todo list before proceeding.
++
+The recommended way to create commits with squash markers is by using the
+`--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
+linkgit:git-commit[1], which take the target commit as an argument and
+automatically fill in the subject line of the new commit from that.
++
+Settting configuration variable `rebase.autoSquash` to true enables
+auto-squashing by default for interactive rebase. The `--no-autosquash`
+option can be used to override that setting.
+
See also INCOMPATIBLE OPTIONS below.
--
2.43.0-rc2
^ permalink raw reply related
* [PATCH v5 2/3] rebase: support --autosquash without -i
From: Andy Koppe @ 2023-11-14 21:43 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood, Andy Koppe
In-Reply-To: <20231114214339.10925-1-andy.koppe@gmail.com>
The rebase --autosquash option is quietly ignored when used without
--interactive (apart from preventing preemptive fast-forwarding and
triggering conflicts with apply backend options).
Change that to support --autosquash without --interactive, by dropping
its restriction to REBASE_INTERACTIVE_EXCPLICIT mode. When used this
way, auto-squashing is done without opening the todo list editor.
Drop the -i requirement from the --autosquash description, and amend
t3415-rebase-autosquash.sh to test the option and the rebase.autoSquash
config variable with and without -i.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/git-rebase.txt | 2 +-
builtin/rebase.c | 4 +---
t/t3415-rebase-autosquash.sh | 38 ++++++++++++++++++++++++++----------
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index b4526ca246..10548e715c 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -592,7 +592,7 @@ See also INCOMPATIBLE OPTIONS below.
When the commit log message begins with "squash! ..." or "fixup! ..."
or "amend! ...", and there is already a commit in the todo list that
matches the same `...`, automatically modify the todo list of
- `rebase -i`, so that the commit marked for squashing comes right after
+ `rebase`, so that the commit marked for squashing comes right after
the commit to be modified, and change the action of the moved commit
from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
matches the `...` if the commit subject matches, or if the `...` refers
diff --git a/builtin/rebase.c b/builtin/rebase.c
index a73de7892b..9f8192e0a5 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -710,10 +710,8 @@ static int run_specific_rebase(struct rebase_options *opts)
if (opts->type == REBASE_MERGE) {
/* Run sequencer-based rebase */
setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
- if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+ if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT))
setenv("GIT_SEQUENCE_EDITOR", ":", 1);
- opts->autosquash = 0;
- }
if (opts->gpg_sign_opt) {
/* remove the leading "-S" */
char *tmp = xstrdup(opts->gpg_sign_opt + 2);
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index a364530d76..fcc40d6fe1 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -43,7 +43,7 @@ test_auto_fixup () {
git tag $1 &&
test_tick &&
- git rebase $2 -i HEAD^^^ &&
+ git rebase $2 HEAD^^^ &&
git log --oneline >actual &&
if test -n "$no_squash"
then
@@ -61,15 +61,24 @@ test_auto_fixup () {
}
test_expect_success 'auto fixup (option)' '
- test_auto_fixup final-fixup-option --autosquash
+ test_auto_fixup fixup-option --autosquash &&
+ test_auto_fixup fixup-option-i "--autosquash -i"
'
-test_expect_success 'auto fixup (config)' '
+test_expect_success 'auto fixup (config true)' '
git config rebase.autosquash true &&
- test_auto_fixup final-fixup-config-true &&
+ test_auto_fixup ! fixup-config-true &&
+ test_auto_fixup fixup-config-true-i -i &&
test_auto_fixup ! fixup-config-true-no --no-autosquash &&
+ test_auto_fixup ! fixup-config-true-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto fixup (config false)' '
git config rebase.autosquash false &&
- test_auto_fixup ! final-fixup-config-false
+ test_auto_fixup ! fixup-config-false &&
+ test_auto_fixup ! fixup-config-false-i -i &&
+ test_auto_fixup fixup-config-false-yes --autosquash &&
+ test_auto_fixup fixup-config-false-i-yes "-i --autosquash"
'
test_auto_squash () {
@@ -87,7 +96,7 @@ test_auto_squash () {
git commit -m "squash! first" -m "extra para for first" &&
git tag $1 &&
test_tick &&
- git rebase $2 -i HEAD^^^ &&
+ git rebase $2 HEAD^^^ &&
git log --oneline >actual &&
if test -n "$no_squash"
then
@@ -105,15 +114,24 @@ test_auto_squash () {
}
test_expect_success 'auto squash (option)' '
- test_auto_squash final-squash --autosquash
+ test_auto_squash squash-option --autosquash &&
+ test_auto_squash squash-option-i "--autosquash -i"
'
-test_expect_success 'auto squash (config)' '
+test_expect_success 'auto squash (config true)' '
git config rebase.autosquash true &&
- test_auto_squash final-squash-config-true &&
+ test_auto_squash ! squash-config-true &&
+ test_auto_squash squash-config-true-i -i &&
test_auto_squash ! squash-config-true-no --no-autosquash &&
+ test_auto_squash ! squash-config-true-i-no "-i --no-autosquash"
+'
+
+test_expect_success 'auto squash (config false)' '
git config rebase.autosquash false &&
- test_auto_squash ! final-squash-config-false
+ test_auto_squash ! squash-config-false &&
+ test_auto_squash ! squash-config-false-i -i &&
+ test_auto_squash squash-config-false-yes --autosquash &&
+ test_auto_squash squash-config-false-i-yes "-i --autosquash"
'
test_expect_success 'misspelled auto squash' '
--
2.43.0-rc2
^ permalink raw reply related
* [PATCH v5 1/3] rebase: fully ignore rebase.autoSquash without -i
From: Andy Koppe @ 2023-11-14 21:43 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood, Andy Koppe
In-Reply-To: <20231114214339.10925-1-andy.koppe@gmail.com>
Setting the rebase.autoSquash config variable to true implies a couple
of restrictions: it prevents preemptive fast-forwarding and it triggers
conflicts with apply backend options. However, it only actually results
in auto-squashing when combined with the --interactive (or -i) option,
due to code in run_specific_rebase() that disables auto-squashing unless
the REBASE_INTERACTIVE_EXPLICIT flag is set.
Doing autosquashing for rebase.autoSquash without --interactive would be
problematic in terms of backward compatibility, but conversely, there is
no need for the aforementioned restrictions without --interactive.
So drop the options.config_autosquash check from the conditions for
clearing allow_preemptive_ff, as the case where it is combined with
--interactive is already covered by the REBASE_INTERACTIVE_EXPLICIT
flag check above it.
Also drop the "apply options are incompatible with rebase.autoSquash"
error, because it is unreachable if it is restricted to --interactive,
as apply options already cause an error when used with --interactive.
Drop the tests for the error from t3422-rebase-incompatible-options.sh,
which has separate tests for the conflicts of --interactive with apply
options.
When neither --autosquash nor --no-autosquash are given, only set
options.autosquash to true if rebase.autosquash is combined with
--interactive.
Don't initialize options.config_autosquash to -1, as there is no need to
distinguish between rebase.autoSquash being unset or explicitly set to
false.
Finally, amend the rebase.autoSquash documentation to say it only
affects interactive mode.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/rebase.txt | 4 +++-
builtin/rebase.c | 13 ++++++-------
t/t3422-rebase-incompatible-options.sh | 12 ------------
3 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index 9c248accec..d59576dbb2 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -9,7 +9,9 @@ rebase.stat::
rebase. False by default.
rebase.autoSquash::
- If set to true enable `--autosquash` option by default.
+ If set to true, enable the `--autosquash` option of
+ linkgit:git-rebase[1] by default for interactive mode.
+ This can be overridden with the `--no-autosquash` option.
rebase.autoStash::
When set to true, automatically create a temporary stash entry
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 043c65dccd..a73de7892b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -149,7 +149,6 @@ struct rebase_options {
.reapply_cherry_picks = -1, \
.allow_empty_message = 1, \
.autosquash = -1, \
- .config_autosquash = -1, \
.rebase_merges = -1, \
.config_rebase_merges = -1, \
.update_refs = -1, \
@@ -1405,7 +1404,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
(options.action != ACTION_NONE) ||
(options.exec.nr > 0) ||
- (options.autosquash == -1 && options.config_autosquash == 1) ||
options.autosquash == 1) {
allow_preemptive_ff = 0;
}
@@ -1508,8 +1506,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (is_merge(&options))
die(_("apply options and merge options "
"cannot be used together"));
- else if (options.autosquash == -1 && options.config_autosquash == 1)
- die(_("apply options are incompatible with rebase.autoSquash. Consider adding --no-autosquash"));
else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
die(_("apply options are incompatible with rebase.rebaseMerges. Consider adding --no-rebase-merges"));
else if (options.update_refs == -1 && options.config_update_refs == 1)
@@ -1529,10 +1525,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
- if (options.autosquash == 1)
+ if (options.autosquash == 1) {
imply_merge(&options, "--autosquash");
- options.autosquash = (options.autosquash >= 0) ? options.autosquash :
- ((options.config_autosquash >= 0) ? options.config_autosquash : 0);
+ } else if (options.autosquash == -1) {
+ options.autosquash =
+ options.config_autosquash &&
+ (options.flags & REBASE_INTERACTIVE_EXPLICIT);
+ }
if (options.type == REBASE_UNSPECIFIED) {
if (!strcmp(options.default_backend, "merge"))
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 2eba00bdf5..b40f26250b 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -100,12 +100,6 @@ test_rebase_am_only () {
test_must_fail git rebase $opt --root A
"
- test_expect_success "$opt incompatible with rebase.autosquash" "
- git checkout B^0 &&
- test_must_fail git -c rebase.autosquash=true rebase $opt A 2>err &&
- grep -e --no-autosquash err
- "
-
test_expect_success "$opt incompatible with rebase.rebaseMerges" "
git checkout B^0 &&
test_must_fail git -c rebase.rebaseMerges=true rebase $opt A 2>err &&
@@ -118,12 +112,6 @@ test_rebase_am_only () {
grep -e --no-update-refs err
"
- test_expect_success "$opt okay with overridden rebase.autosquash" "
- test_when_finished \"git reset --hard B^0\" &&
- git checkout B^0 &&
- git -c rebase.autosquash=true rebase --no-autosquash $opt A
- "
-
test_expect_success "$opt okay with overridden rebase.rebaseMerges" "
test_when_finished \"git reset --hard B^0\" &&
git checkout B^0 &&
--
2.43.0-rc2
^ permalink raw reply related
* [PATCH v5 0/3] rebase: support --autosquash without -i
From: Andy Koppe @ 2023-11-14 21:43 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood, Andy Koppe
In-Reply-To: <20231111132720.78877-1-andy.koppe@gmail.com>
Make rebase --autosquash work without --interactive, but limit
rebase.autoSquash's effects to interactive mode, and improve testing
and documentation.
Changes from v4:
- Fix amend vs apply backend thinko in commit messages.
- Squash patch 3 for testing into patch 2 and improve the commit
message.
- No source changes.
Thanks again to Junio and Phillip for their reviews.
Andy Koppe (3):
rebase: fully ignore rebase.autoSquash without -i
rebase: support --autosquash without -i
rebase: rewrite --(no-)autosquash documentation
Documentation/config/rebase.txt | 4 ++-
Documentation/git-rebase.txt | 34 +++++++++++++----------
builtin/rebase.c | 17 +++++-------
t/t3415-rebase-autosquash.sh | 38 +++++++++++++++++++-------
t/t3422-rebase-incompatible-options.sh | 12 --------
5 files changed, 58 insertions(+), 47 deletions(-)
--
2.43.0-rc2
^ permalink raw reply
* Re: [PATCH] ci: avoid running the test suite _twice_
From: Josh Steadmon @ 2023-11-14 21:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Johannes Schindelin via GitGitGadget, Phillip Wood,
git, Johannes Schindelin
In-Reply-To: <xmqq4jhp438y.fsf@gitster.g>
On 2023.11.14 08:55, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I do have to wonder, though, as somebody who did not follow the
> > unit-test topic closely: why are the unit tests totally separate from
> > the rest of the suite? I would think we'd want them run from one or more
> > t/t*.sh scripts. That would make bugs like this impossible, but also:
> >
> > 1. They'd be run via "make test", so developers don't have to remember
> > to run them separately.
> >
> > 2. They can be run in parallel with all of the other tests when using
> > "prove -j", etc.
>
> Very good points. Josh?
In short, the last time I tried to add something to CI, it was not well
received, so I've been perhaps overly cautious in keeping the unit-tests
well-separated from other targets. But I can send a follow-up patch to
fold them into `make test`. Or would you prefer that I send a v11 of
js/doc-unit-tests instead?
^ permalink raw reply
* Re: [Potential Bug] Test t0301.34 hangs - Git v2.43.0-rc2
From: Todd Zullinger @ 2023-11-14 21:10 UTC (permalink / raw)
To: rsbecker; +Cc: 'Junio C Hamano', git
In-Reply-To: <053501da173c$ac5b4470$0511cd50$@nexbridge.com>
rsbecker@nexbridge.com wrote:
> When running the full suite, I found that t0301.34 hangs
> on NonStop x86 (Big Endian). No details at this point -
> will rerun this, but this is a regression from rc1.
FWIW, this test ran fine on Fedora's s390x architecture.
That's little solace, I know, but may help rule out some
potential causes.
t0301-credential-cache.sh ..........................
...
ok 34 - helper (cache) can forget user
...
# passed all 44 test(s)
The build log is available here (for a few weeks or so -- it
was only a test build):
https://kojipkgs.fedoraproject.org//work/tasks/4976/109024976/build.log
--
Todd
^ permalink raw reply
* [ANNOUNCE] Git for Windows 2.43.0-rc2
From: Johannes Schindelin @ 2023-11-14 21:05 UTC (permalink / raw)
To: git-for-windows, git, git-packagers; +Cc: Johannes Schindelin
Dear Git users,
I hereby announce that Git for Windows 2.43.0-rc2 is available from:
https://github.com/git-for-windows/git/releases/tag/v2.43.0-rc2.windows.1
Changes since Git for Windows v2.42.0(2) (August 30th 2023)
New Features
* Comes with Git v2.43.0-rc2.
* Comes with MSYS2 runtime v3.4.9.
* Comes with GNU TLS v3.8.1.
* When installing into a Windows setup with Mandatory Address Space
Layout Randomization (ASLR) enabled, which is incompatible with the
MSYS2 runtime powering Git Bash, SSH and some other programs
distributed with Git for Windows, the Git for Windows installer now
offers to add exceptions that will allow those programs to work as
expected.
* Comes with OpenSSH v9.5.P1.
* Comes with cURL v8.4.0.
* Comes with OpenSSL v3.1.4.
* Comes with Git Credential Manager v2.4.1.
* Comes with Bash v5.2.21.
* Comes with MinTTY v3.7.0.
Bug Fixes
* Symbolic links whose target is an absolute path without the drive
prefix accidentally had a drive prefix added when checked out,
rendering them "eternally modified". This bug has been fixed.
* Git for Windows's installer is no longer confused by global GIT_*
environment variables.
* The installer no longer claims that "fast-forward or merge" is the
default git pull behavior: The default behavior has changed in Git
a while ago, to "fast-forward only".
Git-2.43.0-rc2-64-bit.exe | 47eab02a2ef7ec969fc812918f16697c15199aa76cb42b4b352a882265017dd5
Git-2.43.0-rc2-32-bit.exe | 95618190eecde6e4a155dc45d3eb02e056fce67cb3510f44d513f9eba25e28f6
PortableGit-2.43.0-rc2-64-bit.7z.exe | 4a364e3b6b02357e316293223f620431dfabe665ea19beb6c2ae995effa6f48f
PortableGit-2.43.0-rc2-32-bit.7z.exe | 6c66cf90bfc634a93e4587f6b76e0c126a244d5f87421dc08eeb585bf7bef048
MinGit-2.43.0-rc2-64-bit.zip | 77720ce608791c4d55fcd80df1953f0b7e2889ed8683a5adce6133edd116d071
MinGit-2.43.0-rc2-32-bit.zip | fbb71e2a7092edbd5e3b3eb12d907b00c0f5757ab5d4b8ab76df3b882d2fdbc0
MinGit-2.43.0-rc2-busybox-64-bit.zip | c99191e111a37c7bc619827426f687925629a24a743e7d4dc64743301f96c07a
MinGit-2.43.0-rc2-busybox-32-bit.zip | 2da34c6d61dd481e6c33a317dccf464b8e9bffaf94e251823d780dba4c23d8cf
Git-2.43.0-rc2-64-bit.tar.bz2 | 3cd453298075b7da69fa14a095f1c9552c9d360c4a5946b1d5d1748243e9aaa1
Git-2.43.0-rc2-32-bit.tar.bz2 | d32f73e4027d416397d9382e017ddbf0b41d0ba38c78588f44723c1e8e7ceae4
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] send-email: avoid duplicate specification warnings
From: Todd Zullinger @ 2023-11-14 20:59 UTC (permalink / raw)
To: Jeff King
Cc: git, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <20231114200009.GD2092538@coredump.intra.peff.net>
Jeff King wrote:
> On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:
>> I've run this through the full test suite. I also compared the output of
>> --help to ensure it only differs in the removal of the "Duplicate
>> specification" warnings. I _think_ that's a good sign that no other changes
>> will result. But I would be grateful to anyone who can confirm or reject that
>> theory.
>
> I guess you meant "-h", not "--help", since the latter will just show
> the manpage. But isn't "-h" just dumping a static usage message we
> wrote, and not auto-generated by the code?
Yes to both. This is why I shouldn't submit patches within
a few hours of waking up.
> The changes look good to me (even after double-checking Junio's question
> that they are all appropriately matched with their "positive" sides).
Indeed. I need to go through them each to test that the
results match before and after. With the fallback to
passing options to format-patch, testing outside of a git
repo makes this rather convenient. If I've dropped an
option it will result in the "Cannot run git format-patch
from outside a repository" error. That's a good start to
ensure the changes don't cause any regressions.
I did notice that I mistakenly dropped --[no-]signed-off-cc.
I need to keep:
"signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
as is.
> This one is curious:
>
>> - "cc-cover|cc-cover!" => \$cover_cc,
>
> It was an alternate name for itself? I think somebody just misunderstood
> how the API was supposed to work. The "!" would applies to all names, if
> I understand correctly, so this really is doing nothing beyond just
> "cc-cover!", which is what your patch switches it to.
I wondered about those as well. Perhaps this is needed in
some older version of Getopt::Long? I'll try to look
through the history of the module to see if that's the case.
Since this isn't anything new with 2.43, it doesn't need to
be fixed with much urgency.
Thanks both,
--
Todd
^ permalink raw reply
* [Potential Bug] Test t0301.34 hangs - Git v2.43.0-rc2
From: rsbecker @ 2023-11-14 20:53 UTC (permalink / raw)
To: 'Junio C Hamano', git
In-Reply-To: <xmqqo7fwxn4s.fsf@gitster.g>
When running the full suite, I found that t0301.34 hangs on NonStop x86 (Big Endian). No details at this point - will rerun this, but this is a regression from rc1.
--Randall
^ permalink raw reply
* Re: Feature request: git status --branch-only
From: Jeff King @ 2023-11-14 20:18 UTC (permalink / raw)
To: phillip.wood; +Cc: Ondra Medek, git
In-Reply-To: <00033c86-dbd7-4c88-bfbd-8f6766cd66c9@gmail.com>
On Tue, Nov 14, 2023 at 03:02:04PM +0000, Phillip Wood wrote:
> Hi Ondra
>
> On 14/11/2023 12:40, Ondra Medek wrote:
> > Hi Phillip,
> >
> > it does not work for a fresh clone of an empty repository
> >
> > git for-each-ref --format="%(upstream:short)" refs/heads/master
> >
> > outputs nothing, while
>
> Oh dear, that's a shame. I wonder if it is a bug because the documentation
> says that
>
> --format="%(upstream:track)"
>
> should print "[gone]" whenever an unknown upstream ref is encountered but
> trying that on a clone of an empty repository gives no output.
I think it would print "gone" if the upstream branch went missing. But
in this case the actual local branch is missing. And for-each-ref will
not show an entry at all for a ref that does not exist. The
"refs/heads/master" on your command line is not a ref, but a pattern,
and that pattern does not match anything. So it's working as intended.
I think a more direct tool would be:
git rev-parse --symbolic-full-name master@{upstream}
That convinces branch_get_upstream() to return the value we want, but
sadly it seems to get lost somewhere in the resolution process, and we
spit out an error. Arguably that is a bug (with --symbolic or
--symbolic-full-name, I think it would be OK to resolve names even if
they don't point to something, but it's possible that would have other
unexpected side effects).
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: avoid duplicate specification warnings
From: Jeff King @ 2023-11-14 20:00 UTC (permalink / raw)
To: Todd Zullinger
Cc: git, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <20231114163826.207267-1-tmz@pobox.com>
On Tue, Nov 14, 2023 at 11:38:19AM -0500, Todd Zullinger wrote:
> With perl-Getopt-Long >= 2.55, a warning is issued for options which are
> specified more than once. In addition to causing users to see warnings,
> this results in test failures which compare the output. An example,
> from t9001-send-email.37:
This made me wonder if the warnings are new, or if the duplicated
auto-negated options are new. I.e., were the manual "--no-foo" option
specs doing something useful in the older versions (in which case we'd
need to do something more complicated)?
But I think the answer is no. We've explicitly marked these with "!" to
indicate that they're negatable. And certainly running with Getopt::Long
2.52 (from perl 5.36, which is the current in Debian unstable) seems to
support them.
It does make me wonder why some boolean options are not marked as
negatable (even if just to countermand an earlier option), but that is
outside the scope of your patch.
> I've run this through the full test suite. I also compared the output of
> --help to ensure it only differs in the removal of the "Duplicate
> specification" warnings. I _think_ that's a good sign that no other changes
> will result. But I would be grateful to anyone who can confirm or reject that
> theory.
I guess you meant "-h", not "--help", since the latter will just show
the manpage. But isn't "-h" just dumping a static usage message we
wrote, and not auto-generated by the code?
The changes look good to me (even after double-checking Junio's question
that they are all appropriately matched with their "positive" sides).
This one is curious:
> - "cc-cover|cc-cover!" => \$cover_cc,
It was an alternate name for itself? I think somebody just misunderstood
how the API was supposed to work. The "!" would applies to all names, if
I understand correctly, so this really is doing nothing beyond just
"cc-cover!", which is what your patch switches it to.
-Peff
^ permalink raw reply
* [PATCH v2 10/10] t/perf: add perf tests for for-each-ref
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.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' with "%(*fieldname)" format
specifiers.
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..fa7289c7522
--- /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, --count=1" --count=1
+ test_for_each_ref "$1, --count=1, no sort" --no-sort --count=1
+ 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, dereferenced" '--format="%(refname) %(objectname) %(*objectname)"' refs/tags/
+ test_for_each_ref "$1, tags, dereferenced, no sort" --no-sort '--format="%(refname) %(objectname) %(*objectname)"' refs/tags/
+
+ test_perf "for-each-ref ($1, tags) + cat-file --batch-check (dereferenced)" "
+ 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
* [PATCH v2 09/10] ref-filter.c: use peeled tag for '*' format fields
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
In most builtins ('rev-parse <revision>^{}', 'show-ref --dereference'),
"dereferencing" a tag refers to a recursive peel of the tag object. Unlike
these cases, the dereferencing prefix ('*') in 'for-each-ref' format
specifiers triggers only a single, non-recursive dereference of a given tag
object. For most annotated tags, a single dereference is all that is needed
to access the tag's associated commit or tree; "recursive" and
"non-recursive" dereferencing are functionally equivalent in these cases.
However, nested tags (annotated tags whose target is another annotated tag)
dereferenced once return another tag, where a recursive dereference would
return the commit or tree.
Currently, if a user wants to filter & format refs and include information
about a recursively-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
performance of this use case and align the defererencing behavior of
'for-each-ref' with that of other commands, update the ref formatting code
to use the peeled tag (from 'peel_iterated_oid()') to populate '*' fields
rather than the tag's immediate target object (from 'get_tagged_oid()').
Additionally, add a test to 't6300-for-each-ref' to verify new nested tag
behavior and update 't6302-for-each-ref-filter.sh' to print the correct
value for nested dereferenced fields.
Signed-off-by: Victoria Dye <vdye@github.com>
---
Documentation/git-for-each-ref.txt | 4 ++--
ref-filter.c | 13 ++++---------
t/t6300-for-each-ref.sh | 22 ++++++++++++++++++++++
t/t6302-for-each-ref-filter.sh | 4 ++--
4 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index b136c9fa908..be9543f6840 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -296,8 +296,8 @@ from the `committer` or `tagger` fields depending on the object type.
These are intended for working on a mix of annotated and lightweight tags.
For tag objects, a `fieldname` prefixed with an asterisk (`*`) expands to
-the `fieldname` value of object the tag points at, rather than that of the
-tag object itself.
+the `fieldname` value of the peeled object, rather than that of the tag
+object itself.
Fields that have name-email-date tuple as its value (`author`,
`committer`, and `tagger`) can be suffixed with `name`, `email`,
diff --git a/ref-filter.c b/ref-filter.c
index 48453db24f7..fdaabb5bb45 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2508,17 +2508,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
return 0;
/*
- * 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.
+ * If it is a tag object, see if we use the peeled value. If we do,
+ * grab the peeled OID.
*/
- oi_deref.oid = *get_tagged_oid((struct tag *)obj);
+ if (need_tagged && peel_iterated_oid(&obj->oid, &oi_deref.oid))
+ die("bad tag");
- /*
- * 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/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 0613e5e3623..54e22812598 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -1839,6 +1839,28 @@ 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)" &&
+
+ 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 \
+ --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)"
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index af223e44d67..82f3d1ea0f2 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -45,8 +45,8 @@ test_expect_success 'check signed tags with --points-at' '
sed -e "s/Z$//" >expect <<-\EOF &&
refs/heads/side Z
refs/tags/annotated-tag four
- refs/tags/doubly-annotated-tag An annotated tag
- refs/tags/doubly-signed-tag A signed tag
+ refs/tags/doubly-annotated-tag four
+ refs/tags/doubly-signed-tag four
refs/tags/four Z
refs/tags/signed-tag four
EOF
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 08/10] for-each-ref: clean up documentation of --format
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Move the description of the `*` prefix from the --format option
documentation to the part of the command documentation that deals with other
object type-specific modifiers. Also reorganize and reword the remaining
--format documentation so that the explanation of the default format doesn't
interrupt the details on format string interpolation.
Signed-off-by: Victoria Dye <vdye@github.com>
---
Documentation/git-for-each-ref.txt | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index e86d5700ddf..b136c9fa908 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -51,17 +51,14 @@ OPTIONS
key.
--format=<format>::
- A string that interpolates `%(fieldname)` from a ref being shown
- and the object it points at. If `fieldname`
- is prefixed with an asterisk (`*`) and the ref points
- at a tag object, use the value for the field in the object
- which the tag object refers to (instead of the field in the tag object).
- When unspecified, `<format>` defaults to
- `%(objectname) SPC %(objecttype) TAB %(refname)`.
- It also interpolates `%%` to `%`, and `%xx` where `xx`
- are hex digits interpolates to character with hex code
- `xx`; for example `%00` interpolates to `\0` (NUL),
- `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
+ A string that interpolates `%(fieldname)` from a ref being shown and
+ the object it points at. In addition, the string literal `%%`
+ renders as `%` and `%xx` - where `xx` are hex digits - renders as
+ the character with hex code `xx`. For example, `%00` interpolates to
+ `\0` (NUL), `%09` to `\t` (TAB), and `%0a` to `\n` (LF).
++
+When unspecified, `<format>` defaults to `%(objectname) SPC %(objecttype)
+TAB %(refname)`.
--color[=<when>]::
Respect any colors specified in the `--format` option. The
@@ -298,6 +295,10 @@ fields will correspond to the appropriate date or name-email-date tuple
from the `committer` or `tagger` fields depending on the object type.
These are intended for working on a mix of annotated and lightweight tags.
+For tag objects, a `fieldname` prefixed with an asterisk (`*`) expands to
+the `fieldname` value of object the tag points at, rather than that of the
+tag object itself.
+
Fields that have name-email-date tuple as its value (`author`,
`committer`, and `tagger`) can be suffixed with `name`, `email`,
and `date` to extract the named component. For email fields (`authoremail`,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 07/10] ref-filter.c: filter & format refs in the same callback
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.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 | 88 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 82 insertions(+), 6 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index ff00ab4b8d8..48453db24f7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2863,6 +2863,49 @@ 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);
+
+ /*
+ * Increment the running count of refs that match the filter. If
+ * max_count is set and we've reached the max, stop the ref
+ * iteration by returning a nonzero value.
+ */
+ 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 +3089,49 @@ 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)
+{
+ /*
+ * Filtering & formatting results within a single ref iteration
+ * callback is not compatible with options that require
+ * post-processing a filtered ref_array. These include:
+ * - filtering on reachability
+ * - sorting the filtered results
+ * - 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 v2 06/10] ref-filter.c: refactor to create common helper functions
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.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:
* Extract the code to grow a 'struct ref_array' and append a given 'struct
ref_array_item *' to it from 'ref_array_push()' into 'ref_array_append()'.
* Extract the code to filter a given ref by refname & object ID then create
a new 'struct ref_array_item *' from 'filter_one()' into
'apply_ref_filter()'.
* Extract the code for filter pre-processing, contains cache creation, and
ref iteration from 'filter_refs()' into 'do_filter_refs()'.
In 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'.
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 5186ee2687b..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 filter_one(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 filter_one(const char *refname, const struct object_id *oid, int flag
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 filter_one(const char *refname, const struct object_id *oid, int flag
* 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/", filter_one, &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/", filter_one, &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/", filter_one, &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, filter_one, &ref_cbdata);
+ ret = for_each_fullref_in_pattern(filter, fn, cb_data);
if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
- head_ref(filter_one, &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 v2 05/10] ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>
From: Victoria Dye <vdye@github.com>
Rename 'ref_filter_handler()' 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.
Signed-off-by: Victoria Dye <vdye@github.com>
---
ref-filter.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 8992fbf45b1..5186ee2687b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2770,7 +2770,7 @@ struct ref_filter_cbdata {
* 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 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_filter *filter = ref_cbdata->filter;
@@ -3001,15 +3001,15 @@ 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/", filter_one, &ref_cbdata);
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/", filter_one, &ref_cbdata);
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/", filter_one, &ref_cbdata);
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, filter_one, &ref_cbdata);
if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
- head_ref(ref_filter_handler, &ref_cbdata);
+ head_ref(filter_one, &ref_cbdata);
}
clear_contains_cache(&filter->internal.contains_cache);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 04/10] ref-filter.h: add functions for filter/format & format-only
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.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 v2 03/10] ref-filter.h: move contains caches into filter
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.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 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 v2 02/10] ref-filter.h: add max_count and omit_empty to ref_format
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye, Victoria Dye
In-Reply-To: <pull.1609.v2.git.1699991638.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
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