* [PATCH 2/4] refs/files: use transactions to delete references
From: Patrick Steinhardt @ 2023-11-14 8:58 UTC (permalink / raw)
To: git; +Cc: David Turner, Han-Wen Nienhuys
In-Reply-To: <cover.1699951815.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7477 bytes --]
In the `files_delete_refs()` callback function of the files backend we
implement deletion of references. This is done in two steps:
1. We lock the packed-refs file and delete all references from it in
a single transaction.
2. We delete all loose references via separate calls to
`refs_delete_ref()`.
These steps essentially duplicate the logic around locking and deletion
order that we already have in the transactional interfaces, where we do
know to lock and evict references from the packed-refs file. Despite the
fact that we duplicate the logic, it's also less efficient than if we
used a single generic transaction:
- The transactional interface knows to skip locking of the packed
refs in case they don't contain any of the refs which are about to
be deleted.
- We end up creating N+1 separate reference transactions, one for
the packed-refs file and N for the individual loose references.
Refactor the code to instead delete references via a single transaction.
As we don't assert the expected old object ID this is equivalent to the
previous behaviour, and we already do the same in the packed-refs
backend.
Despite the fact that the result is simpler to reason about, this change
also results in improved performance. The following benchmarks have been
executed in linux.git:
```
$ hyperfine -n '{rev}, packed={packed} refcount={refcount}' \
-L packed true,false -L refcount 1,1000 -L rev master,pks-ref-store-generic-delete-refs \
--setup 'git -C /home/pks/Development/git switch --detach {rev} && make -C /home/pks/Development/git -j17' \
--prepare 'printf "create refs/heads/new-branch-%d HEAD\n" $(seq {refcount}) | git -C /home/pks/Reproduction/linux.git update-ref --stdin && if test {packed} = true; then git pack-refs --all; fi' \
--warmup=10 \
'/home/pks/Development/git/bin-wrappers/git -C /home/pks/Reproduction/linux.git branch -d new-branch-{1..{refcount}}'
Benchmark 1: master packed=true refcount=1
Time (mean ± σ): 7.8 ms ± 1.6 ms [User: 3.4 ms, System: 4.4 ms]
Range (min … max): 5.5 ms … 11.0 ms 120 runs
Benchmark 2: master packed=false refcount=1
Time (mean ± σ): 7.0 ms ± 1.1 ms [User: 3.2 ms, System: 3.8 ms]
Range (min … max): 5.7 ms … 9.8 ms 180 runs
Benchmark 3: master packed=true refcount=1000
Time (mean ± σ): 283.8 ms ± 5.2 ms [User: 45.7 ms, System: 231.5 ms]
Range (min … max): 276.7 ms … 291.6 ms 10 runs
Benchmark 4: master packed=false refcount=1000
Time (mean ± σ): 284.4 ms ± 5.3 ms [User: 44.2 ms, System: 233.6 ms]
Range (min … max): 277.1 ms … 293.3 ms 10 runs
Benchmark 5: generic-delete-refs packed=true refcount=1
Time (mean ± σ): 6.2 ms ± 1.8 ms [User: 2.3 ms, System: 3.9 ms]
Range (min … max): 4.1 ms … 12.2 ms 142 runs
Benchmark 6: generic-delete-refs packed=false refcount=1
Time (mean ± σ): 7.1 ms ± 1.4 ms [User: 2.8 ms, System: 4.3 ms]
Range (min … max): 4.2 ms … 10.8 ms 157 runs
Benchmark 7: generic-delete-refs packed=true refcount=1000
Time (mean ± σ): 198.9 ms ± 1.7 ms [User: 29.5 ms, System: 165.7 ms]
Range (min … max): 196.1 ms … 201.4 ms 10 runs
Benchmark 8: generic-delete-refs packed=false refcount=1000
Time (mean ± σ): 199.7 ms ± 7.8 ms [User: 32.2 ms, System: 163.2 ms]
Range (min … max): 193.8 ms … 220.7 ms 10 runs
Summary
generic-delete-refs packed=true refcount=1 ran
1.14 ± 0.37 times faster than master packed=false refcount=1
1.15 ± 0.40 times faster than generic-delete-refs packed=false refcount=1
1.26 ± 0.44 times faster than master packed=true refcount=1
32.24 ± 9.17 times faster than generic-delete-refs packed=true refcount=1000
32.36 ± 9.29 times faster than generic-delete-refs packed=false refcount=1000
46.00 ± 13.10 times faster than master packed=true refcount=1000
46.10 ± 13.13 times faster than master packed=false refcount=1000
```
Especially in the case where we have many references we can see a clear
performance speedup of nearly 30%.
This is in contrast to the stated objecive in a27dcf89b68 (refs: make
delete_refs() virtual, 2016-09-04), where the virtual `delete_refs()`
function was introduced with the intent to speed things up rather than
making things slower. So it seems like we have outlived the need for a
virtual function.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs/files-backend.c | 66 +++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 32 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index db5c0c7a72..778d3a96ba 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1268,49 +1268,51 @@ static int files_pack_refs(struct ref_store *ref_store,
static int files_delete_refs(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags)
{
- struct files_ref_store *refs =
- files_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
+ struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
- int i, result = 0;
+ struct string_list_item *item;
+ int ret = 0, failures = 0;
if (!refnames->nr)
return 0;
- if (packed_refs_lock(refs->packed_ref_store, 0, &err))
- goto error;
-
- if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) {
- packed_refs_unlock(refs->packed_ref_store);
- goto error;
+ /*
+ * Since we don't check the references' old_oids, the
+ * individual updates can't fail, so we can pack all of the
+ * updates into a single transaction.
+ */
+ transaction = ref_store_transaction_begin(ref_store, &err);
+ if (!transaction) {
+ ret = error("%s", err.buf);
+ goto out;
}
- packed_refs_unlock(refs->packed_ref_store);
-
- for (i = 0; i < refnames->nr; i++) {
- const char *refname = refnames->items[i].string;
-
- if (refs_delete_ref(&refs->base, msg, refname, NULL, flags))
- result |= error(_("could not remove reference %s"), refname);
+ for_each_string_list_item(item, refnames) {
+ ret = ref_transaction_delete(transaction, item->string,
+ NULL, flags, msg, &err);
+ if (ret) {
+ warning(_("could not delete reference %s: %s"),
+ item->string, err.buf);
+ strbuf_reset(&err);
+ failures = 1;
+ }
}
- strbuf_release(&err);
- return result;
-
-error:
- /*
- * If we failed to rewrite the packed-refs file, then it is
- * unsafe to try to remove loose refs, because doing so might
- * expose an obsolete packed value for a reference that might
- * even point at an object that has been garbage collected.
- */
- if (refnames->nr == 1)
- error(_("could not delete reference %s: %s"),
- refnames->items[0].string, err.buf);
- else
- error(_("could not delete references: %s"), err.buf);
+ ret = ref_transaction_commit(transaction, &err);
+ if (ret) {
+ if (refnames->nr == 1)
+ error(_("could not delete reference %s: %s"),
+ refnames->items[0].string, err.buf);
+ else
+ error(_("could not delete references: %s"), err.buf);
+ }
+out:
+ if (!ret && failures)
+ ret = -1;
+ ref_transaction_free(transaction);
strbuf_release(&err);
- return -1;
+ return ret;
}
/*
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 3/4] refs: deduplicate code to delete references
From: Patrick Steinhardt @ 2023-11-14 8:58 UTC (permalink / raw)
To: git; +Cc: David Turner, Han-Wen Nienhuys
In-Reply-To: <cover.1699951815.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5709 bytes --]
Both the files and the packed-refs reference backends now use the same
generic transactions-based code to delete references. Let's pull these
implementations up into `refs_delete_refs()` to deduplicate the code.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs.c | 48 ++++++++++++++++++++++++++++++++++++++++---
refs/files-backend.c | 46 +----------------------------------------
refs/packed-backend.c | 45 +---------------------------------------
3 files changed, 47 insertions(+), 92 deletions(-)
diff --git a/refs.c b/refs.c
index fcae5dddc6..16bfa21df7 100644
--- a/refs.c
+++ b/refs.c
@@ -2599,13 +2599,55 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,
int refs_delete_refs(struct ref_store *refs, const char *logmsg,
struct string_list *refnames, unsigned int flags)
{
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
+ struct string_list_item *item;
+ int ret = 0, failures = 0;
char *msg;
- int retval;
+
+ if (!refnames->nr)
+ return 0;
msg = normalize_reflog_message(logmsg);
- retval = refs->be->delete_refs(refs, msg, refnames, flags);
+
+ /*
+ * Since we don't check the references' old_oids, the
+ * individual updates can't fail, so we can pack all of the
+ * updates into a single transaction.
+ */
+ transaction = ref_store_transaction_begin(refs, &err);
+ if (!transaction) {
+ ret = error("%s", err.buf);
+ goto out;
+ }
+
+ for_each_string_list_item(item, refnames) {
+ ret = ref_transaction_delete(transaction, item->string,
+ NULL, flags, msg, &err);
+ if (ret) {
+ warning(_("could not delete reference %s: %s"),
+ item->string, err.buf);
+ strbuf_reset(&err);
+ failures = 1;
+ }
+ }
+
+ ret = ref_transaction_commit(transaction, &err);
+ if (ret) {
+ if (refnames->nr == 1)
+ error(_("could not delete reference %s: %s"),
+ refnames->items[0].string, err.buf);
+ else
+ error(_("could not delete references: %s"), err.buf);
+ }
+
+out:
+ if (!ret && failures)
+ ret = -1;
+ ref_transaction_free(transaction);
+ strbuf_release(&err);
free(msg);
- return retval;
+ return ret;
}
int delete_refs(const char *msg, struct string_list *refnames,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 778d3a96ba..8d28810e67 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1268,51 +1268,7 @@ static int files_pack_refs(struct ref_store *ref_store,
static int files_delete_refs(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags)
{
- struct ref_transaction *transaction;
- struct strbuf err = STRBUF_INIT;
- struct string_list_item *item;
- int ret = 0, failures = 0;
-
- if (!refnames->nr)
- return 0;
-
- /*
- * Since we don't check the references' old_oids, the
- * individual updates can't fail, so we can pack all of the
- * updates into a single transaction.
- */
- transaction = ref_store_transaction_begin(ref_store, &err);
- if (!transaction) {
- ret = error("%s", err.buf);
- goto out;
- }
-
- for_each_string_list_item(item, refnames) {
- ret = ref_transaction_delete(transaction, item->string,
- NULL, flags, msg, &err);
- if (ret) {
- warning(_("could not delete reference %s: %s"),
- item->string, err.buf);
- strbuf_reset(&err);
- failures = 1;
- }
- }
-
- ret = ref_transaction_commit(transaction, &err);
- if (ret) {
- if (refnames->nr == 1)
- error(_("could not delete reference %s: %s"),
- refnames->items[0].string, err.buf);
- else
- error(_("could not delete references: %s"), err.buf);
- }
-
-out:
- if (!ret && failures)
- ret = -1;
- ref_transaction_free(transaction);
- strbuf_release(&err);
- return ret;
+ return refs_delete_refs(ref_store, msg, refnames, flags);
}
/*
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 59c78d7941..1589577005 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1691,50 +1691,7 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED,
static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags)
{
- struct packed_ref_store *refs =
- packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
- struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction;
- struct string_list_item *item;
- int ret;
-
- (void)refs; /* We need the check above, but don't use the variable */
-
- if (!refnames->nr)
- return 0;
-
- /*
- * Since we don't check the references' old_oids, the
- * individual updates can't fail, so we can pack all of the
- * updates into a single transaction.
- */
-
- transaction = ref_store_transaction_begin(ref_store, &err);
- if (!transaction)
- return -1;
-
- for_each_string_list_item(item, refnames) {
- if (ref_transaction_delete(transaction, item->string, NULL,
- flags, msg, &err)) {
- warning(_("could not delete reference %s: %s"),
- item->string, err.buf);
- strbuf_reset(&err);
- }
- }
-
- ret = ref_transaction_commit(transaction, &err);
-
- if (ret) {
- if (refnames->nr == 1)
- error(_("could not delete reference %s: %s"),
- refnames->items[0].string, err.buf);
- else
- error(_("could not delete references: %s"), err.buf);
- }
-
- ref_transaction_free(transaction);
- strbuf_release(&err);
- return ret;
+ return refs_delete_refs(ref_store, msg, refnames, flags);
}
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 4/4] refs: remove `delete_refs` callback from backends
From: Patrick Steinhardt @ 2023-11-14 8:58 UTC (permalink / raw)
To: git; +Cc: David Turner, Han-Wen Nienhuys
In-Reply-To: <cover.1699951815.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]
Now that `refs_delete_refs` is implemented in a generic way via the ref
transaction interfaces there are no callers left that invoke the
`delete_refs` callback anymore. Remove it from all of our backends.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs/debug.c | 15 ---------------
refs/files-backend.c | 7 -------
refs/packed-backend.c | 7 -------
refs/refs-internal.h | 3 ---
4 files changed, 32 deletions(-)
diff --git a/refs/debug.c b/refs/debug.c
index b7ffc4ce67..83b7a0ba65 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -143,20 +143,6 @@ static int debug_create_symref(struct ref_store *ref_store,
return res;
}
-static int debug_delete_refs(struct ref_store *ref_store, const char *msg,
- struct string_list *refnames, unsigned int flags)
-{
- struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res =
- drefs->refs->be->delete_refs(drefs->refs, msg, refnames, flags);
- int i;
- trace_printf_key(&trace_refs, "delete_refs {\n");
- for (i = 0; i < refnames->nr; i++)
- trace_printf_key(&trace_refs, "%s\n", refnames->items[i].string);
- trace_printf_key(&trace_refs, "}: %d\n", res);
- return res;
-}
-
static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
const char *newref, const char *logmsg)
{
@@ -458,7 +444,6 @@ struct ref_storage_be refs_be_debug = {
.pack_refs = debug_pack_refs,
.create_symref = debug_create_symref,
- .delete_refs = debug_delete_refs,
.rename_ref = debug_rename_ref,
.copy_ref = debug_copy_ref,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8d28810e67..ad8b1d143f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1265,12 +1265,6 @@ static int files_pack_refs(struct ref_store *ref_store,
return 0;
}
-static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- struct string_list *refnames, unsigned int flags)
-{
- return refs_delete_refs(ref_store, msg, refnames, flags);
-}
-
/*
* People using contrib's git-new-workdir have .git/logs/refs ->
* /some/other/path/.git/logs/refs, and that may live on another device.
@@ -3258,7 +3252,6 @@ struct ref_storage_be refs_be_files = {
.pack_refs = files_pack_refs,
.create_symref = files_create_symref,
- .delete_refs = files_delete_refs,
.rename_ref = files_rename_ref,
.copy_ref = files_copy_ref,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 1589577005..b9fa097a29 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1688,12 +1688,6 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED,
return ref_transaction_commit(transaction, err);
}
-static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- struct string_list *refnames, unsigned int flags)
-{
- return refs_delete_refs(ref_store, msg, refnames, flags);
-}
-
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
struct pack_refs_opts *pack_opts UNUSED)
{
@@ -1722,7 +1716,6 @@ struct ref_storage_be refs_be_packed = {
.pack_refs = packed_pack_refs,
.create_symref = NULL,
- .delete_refs = packed_delete_refs,
.rename_ref = NULL,
.copy_ref = NULL,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 9db8aec4da..4af83bf9a5 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -553,8 +553,6 @@ typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
const char *refs_heads_master,
const char *logmsg);
-typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg,
- struct string_list *refnames, unsigned int flags);
typedef int rename_ref_fn(struct ref_store *ref_store,
const char *oldref, const char *newref,
const char *logmsg);
@@ -677,7 +675,6 @@ struct ref_storage_be {
pack_refs_fn *pack_refs;
create_symref_fn *create_symref;
- delete_refs_fn *delete_refs;
rename_ref_fn *rename_ref;
copy_ref_fn *copy_ref;
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: commit-graph paranoia performance, was Re: [ANNOUNCE] Git v2.43.0-rc1
From: Jeff King @ 2023-11-14 9:05 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, git
In-Reply-To: <ZVMz4iDWfC__H8Jp@tanuki>
On Tue, Nov 14, 2023 at 09:46:26AM +0100, Patrick Steinhardt wrote:
> > Should we default GIT_COMMIT_GRAPH_PARANOIA to "0"? Yes, some operations
> > might miss a breakage, but that is true of so much of Git. For day to
> > day commands we generally assume that the repository is not corrupted,
> > and avoid looking at any data we can. Other commands (like "commit-graph
> > verify", but maybe others) would probably want to be more careful
> > (either by checking this case explicitly, or by enabling the paranoia
> > flag themselves).
>
> I'd be fine with that as a follow-up change, yes. I agree that in
> general we shouldn't see this kind of corruption, and it's good that the
> behaviour can be toggled so easily now.
>
> I'm happy to write that patch if you don't plan to.
I hadn't started on it, so please feel free to go ahead.
-Peff
^ permalink raw reply
* Document git status -b --porcelain
From: Ondra Medek @ 2023-11-14 9:14 UTC (permalink / raw)
To: git
Hi,
I am using "git status -b --porcelain" and parsing output in a code.
However, the output is not documented for an empty repository or a
missing remote branch. (And maybe a detached head, too).
Doc for Porcelain Format Version 1
https://git-scm.com/docs/git-status#_porcelain_format_version_1
references doc for Short Format
https://git-scm.com/docs/git-status#_short_format
and it says:
> If -b is used the short-format status is preceded by a line
> ## branchname tracking info
However, when I have a fresh clone or an empty repository, "git status
-b --porcelain" results in:
## No commits yet on master...origin/master [gone]
So, it seems to me the format is something like "# [info1
]branchname...tracking[ info2]" and it should be documented what infos
may appear and when. Otherwise it's hard to write robust parsing of
such output.
Thanks
Ondra Medek
^ permalink raw reply
* Feature request: git status --branch-only
From: Ondra Medek @ 2023-11-14 10:16 UTC (permalink / raw)
To: git
Hello,
I am working on a tol which should fetch changes from a remote
repository on a user click. I want to limit fetch on the current
remote tracking branch (something like "origin/master"), but
surprisingly, it's hard to get it for all corner cases like a fresh
clone of an empty repository or detached head, etc. E.g see this SO
thread https://stackoverflow.com/questions/171550/find-out-which-remote-branch-a-local-branch-is-tracking/52896538
The most reliable way for me is to call
git status -b --no-ahead-behind --porcelain=v2
and parse the "# branch.upstream" line output. However, it is a bit
slow on large repos and yields unused output for me. So, I propose a
new switch "git status --branch-only" which would output branch status
only.
Note: workaround is to specify some non-existing directory, like
git status -b --no-ahead-behind --porcelain=v2 .this-dir-does-not-exists
Thanks
Ondra Medek
^ permalink raw reply
* [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Patrick Steinhardt @ 2023-11-14 10:23 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Karthik Nayak
[-- Attachment #1: Type: text/plain, Size: 6604 bytes --]
In 7a5d604443 (commit: detect commits that exist in commit-graph but not
in the ODB, 2023-10-31), we have introduced a new object existence check
into `repo_parse_commit_internal()` so that we do not parse commits via
the commit-graph that don't have a corresponding object in the object
database. This new check of course comes with a performance penalty,
which the commit put at around 30% for `git rev-list --topo-order`. But
there are in fact scenarios where the performance regression is even
higher. The following benchmark against linux.git with a fully-build
commit-graph:
Benchmark 1: git.v2.42.1 rev-list --count HEAD
Time (mean ± σ): 658.0 ms ± 5.2 ms [User: 613.5 ms, System: 44.4 ms]
Range (min … max): 650.2 ms … 666.0 ms 10 runs
Benchmark 2: git.v2.43.0-rc1 rev-list --count HEAD
Time (mean ± σ): 1.333 s ± 0.019 s [User: 1.263 s, System: 0.069 s]
Range (min … max): 1.302 s … 1.361 s 10 runs
Summary
git.v2.42.1 rev-list --count HEAD ran
2.03 ± 0.03 times faster than git.v2.43.0-rc1 rev-list --count HEAD
While it's a noble goal to ensure that results are the same regardless
of whether or not we have a potentially stale commit-graph, taking twice
as much time is a tough sell. Furthermore, we can generally assume that
the commit-graph will be updated by git-gc(1) or git-maintenance(1) as
required so that the case where the commit-graph is stale should not at
all be common.
With that in mind, default-disable GIT_COMMIT_GRAPH_PARANOIA and restore
the behaviour and thus performance previous to the mentioned commit. In
order to not be inconsistent, also disable this behaviour by default in
`lookup_commit_in_graph()`, where the object existence check has been
introduced right at its inception via f559d6d45e (revision: avoid
hitting packfiles when commits are in commit-graph, 2021-08-09).
This results in another speedup in commands that end up calling this
function, even though it's less pronounced compared to the above
benchmark. The following has been executed in linux.git with ~1.2
million references:
Benchmark 1: GIT_COMMIT_GRAPH_PARANOIA=true git rev-list --all --no-walk=unsorted
Time (mean ± σ): 2.947 s ± 0.003 s [User: 2.412 s, System: 0.534 s]
Range (min … max): 2.943 s … 2.949 s 3 runs
Benchmark 2: GIT_COMMIT_GRAPH_PARANOIA=false git rev-list --all --no-walk=unsorted
Time (mean ± σ): 2.724 s ± 0.030 s [User: 2.207 s, System: 0.514 s]
Range (min … max): 2.704 s … 2.759 s 3 runs
Summary
GIT_COMMIT_GRAPH_PARANOIA=false git rev-list --all --no-walk=unsorted ran
1.08 ± 0.01 times faster than GIT_COMMIT_GRAPH_PARANOIA=true git rev-list --all --no-walk=unsorted
So whereas 7a5d604443 initially introduced the logic to start doing an
object existence check in `repo_parse_commit_internal()` by default, the
updated logic will now instead cause `lookup_commit_in_graph()` to stop
doing the check by default. This behaviour continues to be tweakable by
the user via the GIT_COMMIT_GRAPH_PARANOIA environment variable.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git.txt | 6 +++---
commit-graph.c | 2 +-
commit.c | 2 +-
t/t5318-commit-graph.sh | 8 ++++----
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 2535a30194..6c19fd1d76 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -917,9 +917,9 @@ for full details.
avoid issues with stale commit-graphs that contain references to
already-deleted commits, but comes with a performance penalty.
+
-The default is "true", which enables the aforementioned behavior.
-Setting this to "false" disables the existence check. This can lead to
-a performance improvement at the cost of consistency.
+The default is "false", which disables the aforementioned behavior.
+Setting this to "true" enables the existence check so that stale commits
+will never be returned from the commit-graph at the cost of performance.
`GIT_ALLOW_PROTOCOL`::
If set to a colon-separated list of protocols, behave as if
diff --git a/commit-graph.c b/commit-graph.c
index ee66098e07..a712917356 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1029,7 +1029,7 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje
uint32_t pos;
if (commit_graph_paranoia == -1)
- commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
+ commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0);
if (!prepare_commit_graph(repo))
return NULL;
diff --git a/commit.c b/commit.c
index 8405d7c3fc..37956b836c 100644
--- a/commit.c
+++ b/commit.c
@@ -577,7 +577,7 @@ int repo_parse_commit_internal(struct repository *r,
static int commit_graph_paranoia = -1;
if (commit_graph_paranoia == -1)
- commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
+ commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0);
if (commit_graph_paranoia && !has_object(r, &item->object.oid, 0)) {
unparse_commit(r, &item->object.oid);
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index d4fc65e078..4c751a6871 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -909,10 +909,10 @@ test_expect_success 'stale commit cannot be parsed when given directly' '
# Verify that it is possible to read the commit from the
# commit graph when not being paranoid, ...
- GIT_COMMIT_GRAPH_PARANOIA=false git rev-list B &&
+ git rev-list B &&
# ... but parsing the commit when double checking that
# it actually exists in the object database should fail.
- test_must_fail git rev-list -1 B
+ test_must_fail env GIT_COMMIT_GRAPH_PARANOIA=true git rev-list -1 B
)
'
@@ -936,9 +936,9 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '
# Again, we should be able to parse the commit when not
# being paranoid about commit graph staleness...
- GIT_COMMIT_GRAPH_PARANOIA=false git rev-parse HEAD~2 &&
+ git rev-parse HEAD~2 &&
# ... but fail when we are paranoid.
- test_must_fail git rev-parse HEAD~2 2>error &&
+ test_must_fail env GIT_COMMIT_GRAPH_PARANOIA=true git rev-parse HEAD~2 2>error &&
grep "error: commit $oid exists in commit-graph but not in the object database" error
)
'
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Patrick Steinhardt @ 2023-11-14 10:35 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Karthik Nayak
In-Reply-To: <7e2d300c4af9a7853201121d66f982afa421bbba.1699957350.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 8759 bytes --]
On Tue, Nov 14, 2023 at 11:23:05AM +0100, Patrick Steinhardt wrote:
> In 7a5d604443 (commit: detect commits that exist in commit-graph but not
> in the ODB, 2023-10-31), we have introduced a new object existence check
> into `repo_parse_commit_internal()` so that we do not parse commits via
> the commit-graph that don't have a corresponding object in the object
> database. This new check of course comes with a performance penalty,
> which the commit put at around 30% for `git rev-list --topo-order`. But
> there are in fact scenarios where the performance regression is even
> higher. The following benchmark against linux.git with a fully-build
> commit-graph:
>
> Benchmark 1: git.v2.42.1 rev-list --count HEAD
> Time (mean ± σ): 658.0 ms ± 5.2 ms [User: 613.5 ms, System: 44.4 ms]
> Range (min … max): 650.2 ms … 666.0 ms 10 runs
>
> Benchmark 2: git.v2.43.0-rc1 rev-list --count HEAD
> Time (mean ± σ): 1.333 s ± 0.019 s [User: 1.263 s, System: 0.069 s]
> Range (min … max): 1.302 s … 1.361 s 10 runs
>
> Summary
> git.v2.42.1 rev-list --count HEAD ran
> 2.03 ± 0.03 times faster than git.v2.43.0-rc1 rev-list --count HEAD
>
> While it's a noble goal to ensure that results are the same regardless
> of whether or not we have a potentially stale commit-graph, taking twice
> as much time is a tough sell. Furthermore, we can generally assume that
> the commit-graph will be updated by git-gc(1) or git-maintenance(1) as
> required so that the case where the commit-graph is stale should not at
> all be common.
>
> With that in mind, default-disable GIT_COMMIT_GRAPH_PARANOIA and restore
> the behaviour and thus performance previous to the mentioned commit. In
> order to not be inconsistent, also disable this behaviour by default in
> `lookup_commit_in_graph()`, where the object existence check has been
> introduced right at its inception via f559d6d45e (revision: avoid
> hitting packfiles when commits are in commit-graph, 2021-08-09).
>
> This results in another speedup in commands that end up calling this
> function, even though it's less pronounced compared to the above
> benchmark. The following has been executed in linux.git with ~1.2
> million references:
>
> Benchmark 1: GIT_COMMIT_GRAPH_PARANOIA=true git rev-list --all --no-walk=unsorted
> Time (mean ± σ): 2.947 s ± 0.003 s [User: 2.412 s, System: 0.534 s]
> Range (min … max): 2.943 s … 2.949 s 3 runs
>
> Benchmark 2: GIT_COMMIT_GRAPH_PARANOIA=false git rev-list --all --no-walk=unsorted
> Time (mean ± σ): 2.724 s ± 0.030 s [User: 2.207 s, System: 0.514 s]
> Range (min … max): 2.704 s … 2.759 s 3 runs
>
> Summary
> GIT_COMMIT_GRAPH_PARANOIA=false git rev-list --all --no-walk=unsorted ran
> 1.08 ± 0.01 times faster than GIT_COMMIT_GRAPH_PARANOIA=true git rev-list --all --no-walk=unsorted
>
> So whereas 7a5d604443 initially introduced the logic to start doing an
> object existence check in `repo_parse_commit_internal()` by default, the
> updated logic will now instead cause `lookup_commit_in_graph()` to stop
> doing the check by default. This behaviour continues to be tweakable by
> the user via the GIT_COMMIT_GRAPH_PARANOIA environment variable.
>
> Reported-by: Jeff King <peff@peff.net>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
Gah, I forgot to run this with GIT_TEST_COMMIT_GRAPH=1 before sending
this patch. There are two test failures that this change introduces:
- t6022-rev-list-missing.sh, where we test for the `--missing=` option
of git-rev-list(1).
- t7700-repack.sh, where we also manually delete objects.
Both of these are expected failures: we knowingly corrupt the repository
and circumvent git-gc(1)/git-maintenance(1), thus no commit-graphs are
updated. If we stick with the new stance that repository corruption
should not require us to pessimize the common case, then we'd have to
squash in something like the below.
Patrick
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
index 40265a4f66..9e3f063d08 100755
--- a/t/t6022-rev-list-missing.sh
+++ b/t/t6022-rev-list-missing.sh
@@ -27,6 +27,12 @@ do
'
done
+# When running with GIT_TEST_COMMIT_GRAPH=true we write a commit-graph, but
+# don't update it before forcefully deleting the commit object. We thus enable
+# GIT_COMMIT_GRAPH_PARANOIA so that this case is detected with such a stale
+# commit-graph.
+export GIT_COMMIT_GRAPH_PARANOIA=true
+
for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
do
for action in "allow-any" "print"
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index d2975e6c93..ca8ad9c420 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -271,7 +271,7 @@ test_expect_success 'repacking fails when missing .pack actually means missing o
ls .git/objects/pack/*.pack >before-pack-dir &&
test_must_fail git fsck &&
- test_must_fail git repack --cruft -d 2>err &&
+ test_must_fail env GIT_COMMIT_GRAPH_PARANOIA=1 git repack --cruft -d 2>err &&
grep "bad object" err &&
# Before failing, the repack did not modify the
> ---
> Documentation/git.txt | 6 +++---
> commit-graph.c | 2 +-
> commit.c | 2 +-
> t/t5318-commit-graph.sh | 8 ++++----
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 2535a30194..6c19fd1d76 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -917,9 +917,9 @@ for full details.
> avoid issues with stale commit-graphs that contain references to
> already-deleted commits, but comes with a performance penalty.
> +
> -The default is "true", which enables the aforementioned behavior.
> -Setting this to "false" disables the existence check. This can lead to
> -a performance improvement at the cost of consistency.
> +The default is "false", which disables the aforementioned behavior.
> +Setting this to "true" enables the existence check so that stale commits
> +will never be returned from the commit-graph at the cost of performance.
>
> `GIT_ALLOW_PROTOCOL`::
> If set to a colon-separated list of protocols, behave as if
> diff --git a/commit-graph.c b/commit-graph.c
> index ee66098e07..a712917356 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1029,7 +1029,7 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje
> uint32_t pos;
>
> if (commit_graph_paranoia == -1)
> - commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
> + commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0);
>
> if (!prepare_commit_graph(repo))
> return NULL;
> diff --git a/commit.c b/commit.c
> index 8405d7c3fc..37956b836c 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -577,7 +577,7 @@ int repo_parse_commit_internal(struct repository *r,
> static int commit_graph_paranoia = -1;
>
> if (commit_graph_paranoia == -1)
> - commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
> + commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0);
>
> if (commit_graph_paranoia && !has_object(r, &item->object.oid, 0)) {
> unparse_commit(r, &item->object.oid);
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index d4fc65e078..4c751a6871 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -909,10 +909,10 @@ test_expect_success 'stale commit cannot be parsed when given directly' '
>
> # Verify that it is possible to read the commit from the
> # commit graph when not being paranoid, ...
> - GIT_COMMIT_GRAPH_PARANOIA=false git rev-list B &&
> + git rev-list B &&
> # ... but parsing the commit when double checking that
> # it actually exists in the object database should fail.
> - test_must_fail git rev-list -1 B
> + test_must_fail env GIT_COMMIT_GRAPH_PARANOIA=true git rev-list -1 B
> )
> '
>
> @@ -936,9 +936,9 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '
>
> # Again, we should be able to parse the commit when not
> # being paranoid about commit graph staleness...
> - GIT_COMMIT_GRAPH_PARANOIA=false git rev-parse HEAD~2 &&
> + git rev-parse HEAD~2 &&
> # ... but fail when we are paranoid.
> - test_must_fail git rev-parse HEAD~2 2>error &&
> + test_must_fail env GIT_COMMIT_GRAPH_PARANOIA=true git rev-parse HEAD~2 2>error &&
> grep "error: commit $oid exists in commit-graph but not in the object database" error
> )
> '
> --
> 2.42.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3] fuzz: add new oss-fuzz fuzzer for date.c / date.h
From: Arthur Chan via GitGitGadget @ 2023-11-14 10:53 UTC (permalink / raw)
To: git; +Cc: Jeff King, Arthur Chan, Arthur Chan
In-Reply-To: <pull.1612.v2.git.1699892568344.gitgitgadget@gmail.com>
From: Arthur Chan <arthur.chan@adalogics.com>
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
---
fuzz: add new oss-fuzz fuzzer for date.c / date.h
This patch is aimed to add a new oss-fuzz fuzzer to the oss-fuzz
directory for fuzzing date.c / date.h in the base directory.
The .gitignore of the oss-fuzz directory and the Makefile have been
modified to accommodate the new fuzzer fuzz-date.c.
Fixed the objects order in .gitignore and Makefiles and fixed some of
the logic and formatting for the fuzz-date.c fuzzer in v2.
Fixed the creation and memory allocation of the fuzzing str in v3. Also
fixed the tz type and sign-extended the data before passing to the tz
variable.
Comment: Yes, indeed. It is quite annoying to have that twice. Yes, the
tz should be considered as attacker controllable and thus negative
values should be considered. But it is tricky to fuzz it because the
date.c::gm_time_t() will call die() if the value is invalid and that
exit the fuzzer directly. OSS-Fuzz may consider it as an issue (or bug)
because the fuzzer exit "unexpectedly". I agree that if we consider the
tz as "attacker controllable, we should include negative values, but
since it will cause the fuzzer exit, I am not sure if it is the right
approach from the fuzzing perspective. Also, it is something that date.c
already take care of with the conditional checking, thus it may also be
worth to do some checking and exclude some invalid values before calling
date.c::show_date() but this may result in copying some conditional
checking code from date.c.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1612%2Farthurscchan%2Fnew-fuzzer-date-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1612/arthurscchan/new-fuzzer-date-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1612
Range-diff vs v2:
1: 2928e2b858d ! 1: 046bca32889 fuzz: add new oss-fuzz fuzzer for date.c / date.h
@@ oss-fuzz/fuzz-date.c (new)
+{
+ int local;
+ int num;
-+ uint16_t tz;
++ int tz;
+ char *str;
++ int8_t *tmp_data;
+ timestamp_t ts;
+ enum date_mode_type dmtype;
+ struct date_mode *dm;
@@ oss-fuzz/fuzz-date.c (new)
+ return 0;
+
+ local = !!(*data & 0x10);
-+ dmtype = (enum date_mode_type)(*data % DATE_UNIX);
-+ if (dmtype == DATE_STRFTIME)
-+ /*
-+ * Currently DATE_STRFTIME is not supported.
-+ */
-+ return 0;
++ num = *data % DATE_UNIX;
++ if (num >= DATE_STRFTIME)
++ num++;
++ dmtype = (enum date_mode_type)num;
+ data++;
+ size--;
+
-+ tz = *data++;
-+ tz = (tz << 8) | *data++;
-+ tz = (tz << 8) | *data++;
++ tmp_data = (int8_t*)data;
++ tz = *tmp_data++;
++ tz = (tz << 8) | *tmp_data++;
++ tz = (tz << 8) | *tmp_data++;
++ data += 3;
+ size -= 3;
+
-+ str = (char *)malloc(size + 1);
-+ if (!str)
-+ return 0;
-+ memcpy(str, data, size);
-+ str[size] = '\0';
++ str = xmemdupz(data, size);
+
+ ts = approxidate_careful(str, &num);
+ free(str);
+
+ dm = date_mode_from_type(dmtype);
+ dm->local = local;
-+ show_date(ts, (int16_t)tz, dm);
++ show_date(ts, tz, dm);
+
+ date_mode_release(dm);
+
Makefile | 1 +
oss-fuzz/.gitignore | 1 +
oss-fuzz/fuzz-date.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 oss-fuzz/fuzz-date.c
diff --git a/Makefile b/Makefile
index 03adcb5a480..4b875ef6ce1 100644
--- a/Makefile
+++ b/Makefile
@@ -750,6 +750,7 @@ SCRIPTS = $(SCRIPT_SH_GEN) \
ETAGS_TARGET = TAGS
FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o
+FUZZ_OBJS += oss-fuzz/fuzz-date.o
FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o
FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o
.PHONY: fuzz-objs
diff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore
index 9acb74412ef..5b954088254 100644
--- a/oss-fuzz/.gitignore
+++ b/oss-fuzz/.gitignore
@@ -1,3 +1,4 @@
fuzz-commit-graph
+fuzz-date
fuzz-pack-headers
fuzz-pack-idx
diff --git a/oss-fuzz/fuzz-date.c b/oss-fuzz/fuzz-date.c
new file mode 100644
index 00000000000..52bea5553a1
--- /dev/null
+++ b/oss-fuzz/fuzz-date.c
@@ -0,0 +1,53 @@
+#include "git-compat-util.h"
+#include "date.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+ int local;
+ int num;
+ int tz;
+ char *str;
+ int8_t *tmp_data;
+ timestamp_t ts;
+ enum date_mode_type dmtype;
+ struct date_mode *dm;
+
+ if (size <= 4)
+ /*
+ * we use the first byte to fuzz dmtype and local,
+ * then the next three bytes to fuzz tz offset,
+ * and the remainder (at least one byte) is fed
+ * as end-user input to approxidate_careful().
+ */
+ return 0;
+
+ local = !!(*data & 0x10);
+ num = *data % DATE_UNIX;
+ if (num >= DATE_STRFTIME)
+ num++;
+ dmtype = (enum date_mode_type)num;
+ data++;
+ size--;
+
+ tmp_data = (int8_t*)data;
+ tz = *tmp_data++;
+ tz = (tz << 8) | *tmp_data++;
+ tz = (tz << 8) | *tmp_data++;
+ data += 3;
+ size -= 3;
+
+ str = xmemdupz(data, size);
+
+ ts = approxidate_careful(str, &num);
+ free(str);
+
+ dm = date_mode_from_type(dmtype);
+ dm->local = local;
+ show_date(ts, tz, dm);
+
+ date_mode_release(dm);
+
+ return 0;
+}
base-commit: dadef801b365989099a9929e995589e455c51fed
--
gitgitgadget
^ permalink raw reply related
* Re: Feature request: git status --branch-only
From: Phillip Wood @ 2023-11-14 12:28 UTC (permalink / raw)
To: Ondra Medek, git
In-Reply-To: <CAJsoDaFX7YdncsTy7UsjxaM1GCKs36-H5RhJ6kzgBUFBJyoGZQ@mail.gmail.com>
Hi Ondra
On 14/11/2023 10:16, Ondra Medek wrote:
> Hello,
> I am working on a tol which should fetch changes from a remote
> repository on a user click. I want to limit fetch on the current
> remote tracking branch (something like "origin/master"), but
> surprisingly, it's hard to get it for all corner cases like a fresh
> clone of an empty repository or detached head, etc. E.g see this SO
> thread https://stackoverflow.com/questions/171550/find-out-which-remote-branch-a-local-branch-is-tracking/52896538
I think you can do this by calling
git symbolic-ref --quiet HEAD
to get the full refname of the current branch. If HEAD is detached it
will print nothing and exit with exit code 1. Then you can call
git for-each-ref --format="%(upstream:short)" $refname
to get the upstream branch
Best Wishes
Phillip
^ permalink raw reply
* Re: Feature request: git status --branch-only
From: Ondra Medek @ 2023-11-14 12:40 UTC (permalink / raw)
To: phillip.wood; +Cc: git
In-Reply-To: <47fa8400-1e5f-437f-84b8-50bb09580325@gmail.com>
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
git status -b --no-ahead-behind --porcelain=v2
outputs
# branch.oid (initial)
# branch.head master
# branch.upstream origin/master
I.e. it outputs a proper upstream branch.
Best regards
Ondra
Ondra Medek
On Tue, 14 Nov 2023 at 13:28, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ondra
>
> On 14/11/2023 10:16, Ondra Medek wrote:
> > Hello,
> > I am working on a tol which should fetch changes from a remote
> > repository on a user click. I want to limit fetch on the current
> > remote tracking branch (something like "origin/master"), but
> > surprisingly, it's hard to get it for all corner cases like a fresh
> > clone of an empty repository or detached head, etc. E.g see this SO
> > thread https://stackoverflow.com/questions/171550/find-out-which-remote-branch-a-local-branch-is-tracking/52896538
>
> I think you can do this by calling
>
> git symbolic-ref --quiet HEAD
>
> to get the full refname of the current branch. If HEAD is detached it
> will print nothing and exit with exit code 1. Then you can call
>
> git for-each-ref --format="%(upstream:short)" $refname
>
> to get the upstream branch
>
> Best Wishes
>
> Phillip
^ permalink raw reply
* Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Taylor Blau @ 2023-11-14 14:42 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Junio C Hamano, Karthik Nayak
In-Reply-To: <ZVNNXNRfrwc_0Sj3@tanuki>
On Tue, Nov 14, 2023 at 11:35:08AM +0100, Patrick Steinhardt wrote:
> Gah, I forgot to run this with GIT_TEST_COMMIT_GRAPH=1 before sending
> this patch. There are two test failures that this change introduces:
>
> - t6022-rev-list-missing.sh, where we test for the `--missing=` option
> of git-rev-list(1).
>
> - t7700-repack.sh, where we also manually delete objects.
>
> Both of these are expected failures: we knowingly corrupt the repository
> and circumvent git-gc(1)/git-maintenance(1), thus no commit-graphs are
> updated. If we stick with the new stance that repository corruption
> should not require us to pessimize the common case, then we'd have to
> squash in something like the below.
Good catch. Thanks for investigating and providing a thoughtful analysis
of why we should turn off GIT_COMMIT_GRAPH_PARANOIA by default. I agree
with your conclusion (with this follow-up patch squashed in, of course).
My hunch is that it would be easier for the maintainer to have a single
patch to queue instead of squashing this in themself. You may want to
send a "v2" to that effect.
Thanks,
Taylor
^ permalink raw reply
* Re: Feature request: git status --branch-only
From: Phillip Wood @ 2023-11-14 15:02 UTC (permalink / raw)
To: Ondra Medek, phillip.wood; +Cc: git
In-Reply-To: <CAJsoDaHX3t9bViq0F7gmJPD+PoE-ZqmJS5h=u-W900x9KEMmYA@mail.gmail.com>
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.
Best Wishes
Phillip
> git status -b --no-ahead-behind --porcelain=v2
>
> outputs
>
> # branch.oid (initial)
> # branch.head master
> # branch.upstream origin/master
>
> I.e. it outputs a proper upstream branch.
>
> Best regards
> Ondra
>
> Ondra Medek
>
>
> On Tue, 14 Nov 2023 at 13:28, Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Ondra
>>
>> On 14/11/2023 10:16, Ondra Medek wrote:
>>> Hello,
>>> I am working on a tol which should fetch changes from a remote
>>> repository on a user click. I want to limit fetch on the current
>>> remote tracking branch (something like "origin/master"), but
>>> surprisingly, it's hard to get it for all corner cases like a fresh
>>> clone of an empty repository or detached head, etc. E.g see this SO
>>> thread https://stackoverflow.com/questions/171550/find-out-which-remote-branch-a-local-branch-is-tracking/52896538
>>
>> I think you can do this by calling
>>
>> git symbolic-ref --quiet HEAD
>>
>> to get the full refname of the current branch. If HEAD is detached it
>> will print nothing and exit with exit code 1. Then you can call
>>
>> git for-each-ref --format="%(upstream:short)" $refname
>>
>> to get the upstream branch
>>
>> Best Wishes
>>
>> Phillip
>
^ permalink raw reply
* [PATCH] send-email: avoid duplicate specification warnings
From: Todd Zullinger @ 2023-11-14 16:38 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Jeff King,
Ondřej Pohořelský
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:
| +++ diff -u expect actual
| --- expect 2023-11-14 10:38:23.854346488 +0000
| +++ actual 2023-11-14 10:38:23.848346466 +0000
| @@ -1,2 +1,7 @@
| +Duplicate specification "no-chain-reply-to" for option "no-chain-reply-to"
| +Duplicate specification "to-cover|to-cover!" for option "to-cover"
| +Duplicate specification "cc-cover|cc-cover!" for option "cc-cover"
| +Duplicate specification "no-thread" for option "no-thread"
| +Duplicate specification "no-to-cover" for option "no-to-cover"
| fatal: longline.patch:35 is longer than 998 characters
| warning: no patches were sent
| error: last command exited with $?=1
| not ok 37 - reject long lines
Remove the duplicate option specs.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
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.
git-send-email.perl | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index cacdbd6bb2..13d9c47fe5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -491,7 +491,6 @@ sub config_regexp {
"bcc=s" => \@getopt_bcc,
"no-bcc" => \$no_bcc,
"chain-reply-to!" => \$chain_reply_to,
- "no-chain-reply-to" => sub {$chain_reply_to = 0},
"sendmail-cmd=s" => \$sendmail_cmd,
"smtp-server=s" => \$smtp_server,
"smtp-server-option=s" => \@smtp_server_options,
@@ -506,36 +505,27 @@ sub config_regexp {
"smtp-auth=s" => \$smtp_auth,
"no-smtp-auth" => sub {$smtp_auth = 'none'},
"annotate!" => \$annotate,
- "no-annotate" => sub {$annotate = 0},
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
"header-cmd=s" => \$header_cmd,
"no-header-cmd" => \$no_header_cmd,
"suppress-from!" => \$suppress_from,
- "no-suppress-from" => sub {$suppress_from = 0},
"suppress-cc=s" => \@suppress_cc,
- "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
- "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
- "cc-cover|cc-cover!" => \$cover_cc,
- "no-cc-cover" => sub {$cover_cc = 0},
- "to-cover|to-cover!" => \$cover_to,
- "no-to-cover" => sub {$cover_to = 0},
+ "signed-off-by-cc!" => \$signed_off_by_cc,
+ "cc-cover!" => \$cover_cc,
+ "to-cover!" => \$cover_to,
"confirm=s" => \$confirm,
"dry-run" => \$dry_run,
"envelope-sender=s" => \$envelope_sender,
"thread!" => \$thread,
- "no-thread" => sub {$thread = 0},
"validate!" => \$validate,
- "no-validate" => sub {$validate = 0},
"transfer-encoding=s" => \$target_xfer_encoding,
"format-patch!" => \$format_patch,
- "no-format-patch" => sub {$format_patch = 0},
"8bit-encoding=s" => \$auto_8bit_encoding,
"compose-encoding=s" => \$compose_encoding,
"force" => \$force,
"xmailer!" => \$use_xmailer,
- "no-xmailer" => sub {$use_xmailer = 0},
"batch-size=i" => \$batch_size,
"relogin-delay=i" => \$relogin_delay,
"git-completion-helper" => \$git_completion_helper,
^ permalink raw reply related
* Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Junio C Hamano @ 2023-11-14 16:48 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Karthik Nayak
In-Reply-To: <ZVNNXNRfrwc_0Sj3@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> Gah, I forgot to run this with GIT_TEST_COMMIT_GRAPH=1 before sending
> this patch. There are two test failures that this change introduces:
>
> - t6022-rev-list-missing.sh, where we test for the `--missing=` option
> of git-rev-list(1).
I would have expected you to enable the paranoia mode automatically
when this option is in effect.
> Both of these are expected failures: we knowingly corrupt the repository
> and circumvent git-gc(1)/git-maintenance(1), thus no commit-graphs are
> updated. If we stick with the new stance that repository corruption
> should not require us to pessimize the common case,...
Yeah, just like we try to be extra careful while running fsck,
because "--missing" is about finding these "corrupt" cases,
triggering the paranoia mode upon seeing the option would make
sense, no? It would fix the failure in 6022, right?
Thanks for working on this.
^ permalink raw reply
* Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Junio C Hamano @ 2023-11-14 16:51 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Karthik Nayak
In-Reply-To: <xmqq7cmkz3fi.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Gah, I forgot to run this with GIT_TEST_COMMIT_GRAPH=1 before sending
>> this patch. There are two test failures that this change introduces:
>>
>> - t6022-rev-list-missing.sh, where we test for the `--missing=` option
>> of git-rev-list(1).
>
> I would have expected you to enable the paranoia mode automatically
> when this option is in effect.
>
>> Both of these are expected failures: we knowingly corrupt the repository
>> and circumvent git-gc(1)/git-maintenance(1), thus no commit-graphs are
>> updated. If we stick with the new stance that repository corruption
>> should not require us to pessimize the common case,...
>
> Yeah, just like we try to be extra careful while running fsck,
> because "--missing" is about finding these "corrupt" cases,
> triggering the paranoia mode upon seeing the option would make
> sense, no? It would fix the failure in 6022, right?
>
> Thanks for working on this.
Just to make sure we do not miscommunicate, I do not think we want
to trigger the paranoia mode only in our tests. We want to be
paranoid to help real users who used "--missing" for their real use,
so enabling PARANOIA in the test script is a wrong approach. We
should enable it inside "rev-list --missing" codepath.
^ permalink raw reply
* Re: [PATCH v3] fuzz: add new oss-fuzz fuzzer for date.c / date.h
From: Junio C Hamano @ 2023-11-14 17:03 UTC (permalink / raw)
To: Arthur Chan via GitGitGadget; +Cc: git, Jeff King, Arthur Chan
In-Reply-To: <pull.1612.v3.git.1699959186146.gitgitgadget@gmail.com>
"Arthur Chan via GitGitGadget" <gitgitgadget@gmail.com> writes:
> ++ tmp_data = (int8_t*)data;
> ++ tz = *tmp_data++;
> ++ tz = (tz << 8) | *tmp_data++;
> ++ tz = (tz << 8) | *tmp_data++;
This has a funny skew towards negative number. Any time MSB of the
one of the three bytes is set, tz becomes negative. Worse, a byte
taken from *tmp_data that has its MSB on will _wipe_ what was read
in tz so far, because its higher order bits above 8th bit are sign
extended. If the incoming data is evenly distributed, 7/8 of the
time, you'd end up with a negative number in tz, no?
I think you can and should pick bytes with uint8_t pointer to avoid
sign extending individual bytes and sign extend the resulting number
at the end. Or if it is too cumbersome to do so, using "int16_t tz"
and filling it with two bytes from *data will sign extend itself
when we pass it to show_date() as a parameter of type "int", which
may be the easiest to code, I suspect.
Thanks.
^ permalink raw reply
* [ANNOUNCE] Git v2.43.0-rc2
From: Junio C Hamano @ 2023-11-14 17:25 UTC (permalink / raw)
To: git; +Cc: Linux Kernel, git-packagers
A release candidate Git v2.43.0-rc2 is now available for testing at
the usual places. It is comprised of 451 non-merge commits since
v2.42.0, contributed by 71 people, 17 of which are new faces [*].
The tarballs are found at:
https://www.kernel.org/pub/software/scm/git/testing/
The following public repositories all have a copy of the
'v2.43.0-rc2' tag and the 'master' branch that the tag points at:
url = https://git.kernel.org/pub/scm/git/git
url = https://kernel.googlesource.com/pub/scm/git/git
url = git://repo.or.cz/alt-git.git
url = https://github.com/gitster/git
New contributors whose contributions weren't in v2.42.0 are as follows.
Welcome to the Git development community!
Aditya Neelamraju, Alyssa Ross, Caleb Hill, Dorcas AnonoLitunya,
Dragan Simic, Isoken June Ibizugbe, Jan Alexander Steffens
(heftig), Javier Mora, ks1322 ks1322, Mark Ruvald Pedersen,
Matthew McClain, Naomi Ibe, Romain Chossart, Tang Yuyi, Vipul
Kumar, 王常新, and 谢致邦 (XIE Zhibang).
Returning contributors who helped this release are as follows.
Thanks for your continued support.
Ævar Arnfjörð Bjarmason, Andrei Rybak, Andy Koppe, Bagas
Sanjaya, Beat Bolli, brian m. carlson, Calvin Wan, Christian
Couder, Christian Hesse, Derrick Stolee, Drew DeVault, Elijah
Newren, Emily Shaffer, Eric W. Biederman, Eric Wong, Evan
Gates, Han Young, Hariom Verma, Jacob Abel, Jacob Stopak,
Jason Hatton, Jeff King, Johannes Schindelin, John Cai,
Josh Soref, Josip Sokcevic, Junio C Hamano, Karthik Nayak,
Kousik Sanagavarapu, Kristoffer Haugsbakk, Linus Arver, Mark
Levedahl, Martin Ågren, Martin Storsjö, M Hickford, Michael
Strawbridge, Michal Suchanek, Oswald Buddenhagen, Patrick
Steinhardt, Philippe Blain, Phillip Wood, Randall S. Becker,
René Scharfe, Robert Coup, Rubén Justo, Sergey Organov, Shuqi
Liang, Stefan Haller, Štěpán Němec, Taylor Blau, Teng Long,
Todd Zullinger, Victoria Dye, and Wesley Schwengle.
[*] We are counting not just the authorship contribution but issue
reporting, mentoring, helping and reviewing that are recorded in
the commit trailers.
----------------------------------------------------------------
Git v2.43 Release Notes (draft)
===============================
Backward Compatibility Notes
* The "--rfc" option of "git format-patch" used to be a valid way to
override an earlier "--subject-prefix=<something>" on the command
line and replace it with "[RFC PATCH]", but from this release, it
merely prefixes the string "RFC " in front of the given subject
prefix. If you are negatively affected by this change, please use
"--subject-prefix=PATCH --rfc" as a replacement.
* "git rev-list --stdin" learned to take non-revisions (like "--not")
recently from the standard input, but the way such a "--not" was
handled was quite confusing, which has been rethought. The updated
rule is that "--not" given from the command line only affects revs
given from the command line that comes but not revs read from the
standard input, and "--not" read from the standard input affects
revs given from the standard input and not revs given from the
command line.
UI, Workflows & Features
* A message written in olden time prevented a branch from getting
checked out saying it is already checked out elsewhere, but these
days, we treat a branch that is being bisected or rebased just like
a branch that is checked out and protect it. Rephrase the message
to say that the branch is in use.
* Hourly and other schedules of "git maintenance" jobs are randomly
distributed now.
* "git cmd -h" learned to signal which options can be negated by
listing such options like "--[no-]opt".
* The way authentication related data other than passwords (e.g.,
oauth token and password expiration data) are stored in libsecret
keyrings has been rethought.
* Update the libsecret and wincred credential helpers to correctly
match which credential to erase; they erased the wrong entry in
some cases.
* Git GUI updates.
* "git format-patch" learns a way to feed cover letter description,
that (1) can be used on detached HEAD where there is no branch
description available, and (2) also can override the branch
description if there is one.
* Use of --max-pack-size to allow multiple packfiles to be created is
now supported even when we are sending unreachable objects to cruft
packs.
* "git format-patch --rfc --subject-prefix=<foo>" used to ignore the
"--subject-prefix" option and used "[RFC PATCH]"; now we will add
"RFC" prefix to whatever subject prefix is specified.
* "git log --format" has been taught the %(decorate) placeholder.
* The default log message created by "git revert", when reverting a
commit that records a revert, has been tweaked, to encourage people
to describe complex "revert of revert of revert" situations better in
their own words.
* The command-line completion support (in contrib/) learned to
complete "git commit --trailer=" for possible trailer keys.
* "git update-index" learns "--show-index-version" to inspect
the index format version used by the on-disk index file.
* "git diff" learned diff.statNameWidth configuration variable, to
give the default width for the name part in the "--stat" output.
* "git range-diff --notes=foo" compared "log --notes=foo --notes" of
the two ranges, instead of using just the specified notes tree.
* The command line completion script (in contrib/) can be told to
complete aliases by including ": git <cmd> ;" in the alias to tell
it that the alias should be completed in a similar way to how "git <cmd>" is
completed. The parsing code for the alias has been loosened to
allow ';' without an extra space before it.
* "git for-each-ref" and friends learned to apply mailmap to
authorname and other fields.
* "git repack" machinery learns to pay attention to the "--filter="
option.
* "git repack" learned "--max-cruft-size" to prevent cruft packs from
growing without bounds.
* "git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.
* "git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".
* The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.
* "git merge-file" learns a mode to read three contents to be merged
from blob objects.
Performance, Internal Implementation, Development Support etc.
* "git check-attr" has been taught to work better with sparse-index.
* It may be tempting to leave the help text NULL for a command line
option that is either hidden or too obvious, but "git subcmd -h"
and "git subcmd --help-all" would have segfaulted if done so. Now
the help text is optional.
* Tests that are known to pass with LSan are now marked as such.
* Flaky "git p4" tests, as well as "git svn" tests, are now skipped
in the (rather expensive) sanitizer CI job.
* Tests with LSan from time to time seem to emit harmless messages
that make our tests unnecessarily flaky; we work around it by
filtering the uninteresting output.
* Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.
* The code to keep track of existing packs in the repository while
repacking has been refactored.
* The "streaming" interface used for bulk-checkin codepath has been
narrowed to take only blob objects for now, with no real loss of
functionality.
* GitHub CI workflow has learned to trigger Coverity check.
* Test coverage for trailers has been improved.
* The code to iterate over loose references has been optimized to
reduce the number of lstat() system calls.
* The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.
* Replace macos-12 used at GitHub CI with macos-13.
(merge 682a868f67 js/ci-use-macos-13 later to maint).
Fixes since v2.42
-----------------
* Overly long label names used in the sequencer machinery are now
chopped to fit under filesystem limitation.
* Scalar updates.
* Tweak GitHub Actions CI so that pushing the same commit to multiple
branch tips at the same time will not waste building and testing
the same thing twice.
* The commit-graph verification code that detects a mixture of zero and
non-zero generation numbers has been updated.
* "git diff -w --exit-code" with various options did not work
correctly, which is being addressed.
* transfer.unpackLimit ought to be used as a fallback, but overrode
fetch.unpackLimit and receive.unpackLimit instead.
* The use of API between two calls to require_clean_work_tree() from
the sequencer code has been cleaned up for consistency.
* "git diff --no-such-option" and other corner cases around the exit
status of the "diff" command have been corrected.
* "git for-each-ref --sort='contents:size'" sorts the refs according
to size numerically, giving a ref that points at a blob twelve-byte
(12) long before showing a blob hundred-byte (100) long.
* We now limit the depth of the tree objects and maximum length of
pathnames recorded in tree objects.
(merge 4d5693ba05 jk/tree-name-and-depth-limit later to maint).
* Various fixes to the behavior of "rebase -i" when the command got
interrupted by conflicting changes.
* References from a description of the `--patch` option in various
manual pages have been simplified and improved.
* "git grep -e A --no-or -e B" is accepted, even though the negation
of "or" did not mean anything, which has been tightened.
* The completion script (in contrib/) has been taught to treat the
"-t" option to "git checkout" and "git switch" just like the
"--track" option, to complete remote-tracking branches.
* "git diff --no-index -R <(one) <(two)" did not work correctly,
which has been corrected.
* Update "git maintenance" timers' implementation based on systemd
timers to work with WSL.
* "git diff --cached" codepath did not fill the necessary stat
information for a file when fsmonitor knows it is clean and ended
up behaving as if it is not clean, which has been corrected.
* Clarify how "alias.foo = : git cmd ; aliased-command-string" should be
spelled with necessary whitespace around punctuation marks to
work.
* HTTP Header redaction code has been adjusted for a newer version of
cURL library that shows its traces differently from earlier
versions.
* An error message given by "git send-email" when given a malformed
address did not give correct information, which has been corrected.
* UBSan options were not propagated through the test framework to git
run via the httpd, unlike ASan options, which has been corrected.
* "checkout --merge -- path" and "update-index --unresolve path" did
not resurrect conflicted state that was resolved to remove path,
but now they do.
(merge 5bdedac3c7 jc/unresolve-removal later to maint).
* The display width table for unicode characters has been updated for
Unicode 15.1
(merge 872976c37e bb/unicode-width-table-15 later to maint).
* Update mailmap entry for Derrick.
(merge 6e5457d8c7 ds/mailmap-entry-update later to maint).
* In .gitmodules files, submodules are keyed by their names, and the
path to the submodule whose name is $name is specified by the
submodule.$name.path variable. There were a few codepaths that
mixed the name and path up when consulting the submodule database,
which have been corrected. It took long for these bugs to be found
as the name of a submodule initially is the same as its path, and
the problem does not surface until it is moved to a different path,
which apparently happens very rarely.
* "git diff --merge-base X other args..." insisted that X must be a
commit and errored out when given an annotated tag that peels to a
commit, but we only need it to be a committish. This has been
corrected.
(merge 4adceb5a29 ar/diff-index-merge-base-fix later to maint).
* Fix "git merge-tree" to stop segfaulting when the --attr-source
option is used.
(merge e95bafc52f jc/merge-ort-attr-index-fix later to maint).
* Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
not auto-initialize the decoration subsystem, which has been
corrected.
* Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.
(merge d9b6634589 jc/fail-stash-to-store-non-stash later to maint).
* The index file has room only for the lower 32-bit of the file size in
the cached stat information, which means cached stat information
will have 0 in its sd_size member for a file whose size is a multiple
of 4GiB. This is mistaken for a racily clean path. Avoid it by
storing a bogus sd_size value instead for such files.
(merge 5143ac07b1 bc/racy-4gb-files later to maint).
* "git p4" tried to store symlinks to LFS when told, but has been
fixed not to do so, because it does not make sense.
(merge 10c89a02b0 mm/p4-symlink-with-lfs later to maint).
* The codepath to handle recipient addresses `git send-email
--compose` learns from the user was completely broken, which has
been corrected.
(merge 3ec6167567 jk/send-email-fix-addresses-from-composed-messages later to maint).
* "cd sub && git grep -f patterns" tried to read "patterns" file at
the top level of the working tree; it has been corrected to read
"sub/patterns" instead.
* "git reflog expire --single-worktree" has been broken for the past
20 months or so, which has been corrected.
* "git send-email" did not have certain pieces of data computed yet
when it tried to validate the outgoing messages and its recipient
addresses, which has been sorted out.
* "git bugreport" learned to complain when it received a command line
argument that it will not use.
* The codepath to traverse the commit-graph learned to notice that a
commit is missing (e.g., corrupt repository lost an object), even
though it knows something about the commit (like its parents) from
what is in commit-graph.
(merge 7a5d604443 ps/do-not-trust-commit-graph-blindly-for-existence later to maint).
* "git rev-list --missing" did not work for missing commit objects,
which has been corrected.
* "git rev-list --unpacked --objects" failed to exclude packed
non-commit objects, which has been corrected.
(merge 7b3c8e9f38 tb/rev-list-unpacked-fix later to maint).
* Other code cleanup, docfix, build fix, etc.
(merge c2c349a15c xz/commit-title-soft-limit-doc later to maint).
(merge 1bd809938a tb/format-pack-doc-update later to maint).
(merge 8f81532599 an/clang-format-typofix later to maint).
(merge 3ca86adc2d la/strvec-header-fix later to maint).
(merge 6789275d37 jc/test-i18ngrep later to maint).
(merge 9972cd6004 ps/leakfixes later to maint).
----------------------------------------------------------------
Changes since v2.42.0 are as follows:
Aditya Neelamraju (1):
clang-format: fix typo in comment
Alyssa Ross (1):
diff: fix --merge-base with annotated tags
Andrei Rybak (1):
SubmittingPatches: call gitk's command "Copy commit reference"
Andy Koppe (8):
pretty-formats: enclose options in angle brackets
decorate: refactor format_decorations()
decorate: avoid some unnecessary color overhead
decorate: color each token separately
pretty: add %(decorate[:<options>]) format
pretty: add pointer and tag options to %(decorate)
decorate: use commit color for HEAD arrow
pretty: fix ref filtering for %(decorate) formats
Beat Bolli (1):
unicode: update the width tables to Unicode 15.1
Caleb Hill (1):
git-clean doc: fix "without do cleaning" typo
Calvin Wan (4):
hex-ll: separate out non-hash-algo functions
wrapper: reduce scope of remove_or_warn()
config: correct bad boolean env value error message
parse: separate out parsing functions from config.h
Christian Couder (9):
pack-objects: allow `--filter` without `--stdout`
t/helper: add 'find-pack' test-tool
repack: refactor finishing pack-objects command
repack: refactor finding pack prefix
pack-bitmap-write: rebuild using new bitmap when remapping
repack: add `--filter=<filter-spec>` option
gc: add `gc.repackFilter` config option
repack: implement `--filter-to` for storing filtered out objects
gc: add `gc.repackFilterTo` config option
Christian Hesse (2):
t/lib-gpg: forcibly run a trustdb update
t/t6300: drop magic filtering
Derrick Stolee (13):
upload-pack: fix race condition in error messages
maintenance: add get_random_minute()
maintenance: use random minute in launchctl scheduler
maintenance: use random minute in Windows scheduler
maintenance: use random minute in cron scheduler
maintenance: swap method locations
maintenance: use random minute in systemd scheduler
maintenance: fix systemd schedule overlaps
maintenance: update schedule before config
scalar: add --[no-]src option
setup: add discover_git_directory_reason()
scalar reconfigure: help users remove buggy repos
mailmap: change primary address for Derrick Stolee
Dorcas AnonoLitunya (1):
t7601: use "test_path_is_file" etc. instead of "test -f"
Dragan Simic (2):
diff --stat: add config option to limit filename width
diff --stat: set the width defaults in a helper function
Drew DeVault (1):
format-patch: --rfc honors what --subject-prefix sets
Elijah Newren (26):
documentation: wording improvements
documentation: fix small error
documentation: fix typos
documentation: fix apostrophe usage
documentation: add missing words
documentation: remove extraneous words
documentation: fix subject/verb agreement
documentation: employ consistent verb tense for a list
documentation: fix verb tense
documentation: fix adjective vs. noun
documentation: fix verb vs. noun
documentation: fix singular vs. plural
documentation: whitespace is already generally plural
documentation: fix choice of article
documentation: add missing article
documentation: remove unnecessary hyphens
documentation: add missing hyphens
documentation: use clearer prepositions
documentation: fix punctuation
documentation: fix capitalization
documentation: fix whitespace issues
documentation: add some commas where they are helpful
documentation: add missing fullstops
documentation: add missing quotes
documentation: add missing parenthesis
RelNotes: minor wording fixes in 2.43.0 release notes
Emily Shaffer (2):
t0091-bugreport: stop using i18ngrep
bugreport: reject positional arguments
Eric W. Biederman (1):
bulk-checkin: only support blobs in index_bulk_checkin
Eric Wong (1):
treewide: fix various bugs w/ OpenSSL 3+ EVP API
Evan Gates (1):
git-config: fix misworded --type=path explanation
Han Young (1):
show doc: redirect user to git log manual instead of git diff-tree
Isoken June Ibizugbe (1):
builtin/branch.c: adjust error messages to coding guidelines
Jacob Abel (1):
builtin/worktree.c: fix typo in "forgot fetch" msg
Jacob Stopak (1):
Include gettext.h in MyFirstContribution tutorial
Jan Alexander Steffens (heftig) (6):
submodule--helper: use submodule_from_path in set-{url,branch}
submodule--helper: return error from set-url when modifying failed
t7419: actually test the branch switching
t7419, t7420: use test_cmp_config instead of grepping .gitmodules
t7419: test that we correctly handle renamed submodules
t7420: test that we correctly handle renamed submodules
Jason Hatton (1):
Prevent git from rehashing 4GiB files
Javier Mora (2):
git-status.txt: fix minor asciidoc format issue
doc/git-bisect: clarify `git bisect run` syntax
Jeff King (114):
hashmap: use expected signatures for comparison functions
diff-files: avoid negative exit value
diff: show usage for unknown builtin_diff_files() options
diff: die when failing to read index in git-diff builtin
diff: drop useless return from run_diff_{files,index} functions
diff: drop useless return values in git-diff helpers
diff: drop useless "status" parameter from diff_result_code()
commit-graph: verify swapped zero/non-zero generation cases
test-lib: ignore uninteresting LSan output
sequencer: use repository parameter in short_commit_name()
sequencer: mark repository argument as unused
ref-filter: mark unused parameters in parser callbacks
pack-bitmap: mark unused parameters in show_object callback
worktree: mark unused parameters in each_ref_fn callback
commit-graph: mark unused data parameters in generation callbacks
ls-tree: mark unused parameter in callback
stash: mark unused parameter in diff callback
trace2: mark unused us_elapsed_absolute parameters
trace2: mark unused config callback parameter
test-trace2: mark unused argv/argc parameters
grep: mark unused parameter in output function
add-interactive: mark unused callback parameters
negotiator/noop: mark unused callback parameters
worktree: mark unused parameters in noop repair callback
imap-send: mark unused parameters with NO_OPENSSL
grep: mark unused parmaeters in pcre fallbacks
credential: mark unused parameter in urlmatch callback
fetch: mark unused parameter in ref_transaction callback
bundle-uri: mark unused parameters in callbacks
gc: mark unused descriptors in scheduler callbacks
update-ref: mark unused parameter in parser callbacks
ci: allow branch selection through "vars"
ci: deprecate ci/config/allow-ref script
merge: make xopts a strvec
merge: simplify parsing of "-n" option
format-patch: use OPT_STRING_LIST for to/cc options
tree-walk: reduce stack size for recursive functions
tree-walk: drop MAX_TRAVERSE_TREES macro
tree-walk: rename "error" variable
fsck: detect very large tree pathnames
add core.maxTreeDepth config
traverse_trees(): respect max_allowed_tree_depth
read_tree(): respect max_allowed_tree_depth
list-objects: respect max_allowed_tree_depth
tree-diff: respect max_allowed_tree_depth
lower core.maxTreeDepth default to 2048
checkout-index: delay automatic setting of to_tempfile
parse-options: prefer opt->value to globals in callbacks
parse-options: mark unused "opt" parameter in callbacks
merge: do not pass unused opt->value parameter
parse-options: add more BUG_ON() annotations
interpret-trailers: mark unused "unset" parameters in option callbacks
parse-options: mark unused parameters in noop callback
merge-ort: drop custom err() function
merge-ort: stop passing "opt" to read_oid_strbuf()
merge-ort: drop unused parameters from detect_and_process_renames()
merge-ort: drop unused "opt" parameter from merge_check_renames_reusable()
http: factor out matching of curl http/2 trace lines
http: update curl http/2 info matching for curl 8.3.0
merge-ort: lowercase a few error messages
fsmonitor: prefer repo_git_path() to git_pathdup()
fsmonitor/win32: drop unused parameters
fsmonitor: mark some maybe-unused parameters
fsmonitor/win32: mark unused parameter in fsm_os__incompatible()
fsmonitor: mark unused parameters in stub functions
fsmonitor/darwin: mark unused parameters in system callback
fsmonitor: mark unused hashmap callback parameters
run-command: mark unused parameters in start_bg_wait callbacks
test-lib: set UBSAN_OPTIONS to match ASan
commit-graph: factor out chain opening function
commit-graph: check mixed generation validation when loading chain file
t5324: harmonize sha1/sha256 graph chain corruption
commit-graph: detect read errors when verifying graph chain
commit-graph: tighten chain size check
commit-graph: report incomplete chains during verification
t6700: mark test as leak-free
commit-reach: free temporary list in get_octopus_merge_bases()
merge: free result of repo_get_merge_bases()
commit-graph: move slab-clearing to close_commit_graph()
commit-graph: free all elements of graph chain
commit-graph: delay base_graph assignment in add_graph_to_chain()
commit-graph: free graph struct that was not added to chain
commit-graph: free write-context entries before overwriting
commit-graph: free write-context base_graph_name during cleanup
commit-graph: clear oidset after finishing write
decorate: add clear_decoration() function
revision: clear decoration structs during release_revisions()
daemon: free listen_addr before returning
repack: free existing_cruft array after use
chunk-format: note that pair_chunk() is unsafe
t: add library for munging chunk-format files
midx: stop ignoring malformed oid fanout chunk
commit-graph: check size of oid fanout chunk
midx: check size of oid lookup chunk
commit-graph: check consistency of fanout table
midx: check size of pack names chunk
midx: enforce chunk alignment on reading
midx: check size of object offset chunk
midx: bounds-check large offset chunk
midx: check size of revindex chunk
commit-graph: check size of commit data chunk
commit-graph: detect out-of-bounds extra-edges pointers
commit-graph: bounds-check base graphs chunk
commit-graph: check size of generations chunk
commit-graph: bounds-check generation overflow chunk
commit-graph: check bounds when accessing BDAT chunk
commit-graph: check bounds when accessing BIDX chunk
commit-graph: detect out-of-order BIDX offsets
chunk-format: drop pair_chunk_unsafe()
t5319: make corrupted large-offset test more robust
doc/send-email: mention handling of "reply-to" with --compose
Revert "send-email: extract email-parsing code into a subroutine"
send-email: handle to/cc/bcc from --compose message
t: avoid perl's pack/unpack "Q" specifier
Johannes Schindelin (19):
windows: ignore empty `PATH` elements
is_Cygwin: avoid `exec`ing anything
Move is_<platform> functions to the beginning
Move the `_which` function (almost) to the top
Work around Tcl's default `PATH` lookup
rebase: allow overriding the maximal length of the generated labels
ci: avoid building from the same commit in parallel
ci(linux-asan-ubsan): let's save some time
var: avoid a segmentation fault when `HOME` is unset
completion(switch/checkout): treat --track and -t the same
maintenance(systemd): support the Windows Subsystem for Linux
ci: add a GitHub workflow to submit Coverity scans
coverity: cache the Coverity Build Tool
coverity: allow overriding the Coverity project
coverity: support building on Windows
coverity: allow running on macOS
coverity: detect and report when the token or project is incorrect
max_tree_depth: lower it for MSVC to avoid stack overflows
ci: upgrade to using macos-13
John Cai (3):
merge-ort: initialize repo in index state
attr: read attributes from HEAD when bare repo
attr: add attr.tree for setting the treeish to read attributes from
Josh Soref (1):
Documentation/git-status: add missing line breaks
Josip Sokcevic (1):
diff-lib: fix check_removed when fsmonitor is on
Junio C Hamano (56):
update-index: do not read HEAD and MERGE_HEAD unconditionally
resolve-undo: allow resurrecting conflicted state that resolved to deletion
update-index: use unmerge_index_entry() to support removal
update-index: remove stale fallback code for "--unresolve"
checkout/restore: refuse unmerging paths unless checking out of the index
checkout/restore: add basic tests for --merge
checkout: allow "checkout -m path" to unmerge removed paths
mv: fix error for moving directory to another
diff: move the fallback "--exit-code" code down
diff: mode-only change should be noticed by "--patch -w --exit-code"
diff: teach "--stat -w --exit-code" to notice differences
t4040: remove test that succeeded for a wrong reason
pretty-formats: define "literal formatting code"
diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()
diff: the -w option breaks --exit-code for --raw and other output modes
transfer.unpackLimit: fetch/receive.unpackLimit takes precedence
Start the 2.43 cycle
The second batch for 2.43
The extra batch to update credenthal helpers
The third batch
The fourth batch
The fifth batch
The sixth batch
The seventh batch
update-index doc: v4 is OK with JGit and libgit2
update-index: add --show-index-version
test-tool: retire "index-version"
The eighth batch
The ninth batch
The tenth batch
The eleventh batch
completion: loosen and document the requirement around completing alias
The twelfth batch
The thirteenth batch
The fourteenth batch
The fifteenth batch
doc: update list archive reference to use lore.kernel.org
The sixteenth batch
merge: introduce {copy|clear}_merge_options()
stash: be careful what we store
grep: -f <path> is relative to $cwd
The seventeenth batch
The eighteenth batch
commit: do not use cryptic "new_index" in end-user facing messages
The nineteenth batch
am: align placeholder for --whitespace option with apply
The twentieth batch
The twenty-first batch
The twenty-second batch
test framework: further deprecate test_i18ngrep
Git 2.42.1
tests: teach callers of test_i18ngrep to use test_grep
A bit more before -rc1
Prepare for -rc1
Git 2.43-rc1
Git 2.43-rc2
Karthik Nayak (3):
revision: rename bit to `do_not_die_on_missing_objects`
rev-list: move `show_commit()` to the bottom
rev-list: add commit object support in `--missing` option
Kousik Sanagavarapu (4):
ref-filter: sort numerically when ":size" is used
t/t6300: cleanup test_atom
t/t6300: introduce test_bad_atom
ref-filter: add mailmap support
Kristoffer Haugsbakk (2):
range-diff: treat notes like `log`
grep: die gracefully when outside repository
Linus Arver (17):
trailer tests: make test cases self-contained
trailer test description: this tests --where=after, not --where=before
trailer: add tests to check defaulting behavior with --no-* flags
trailer doc: narrow down scope of --where and related flags
trailer: trailer location is a place, not an action
trailer --no-divider help: describe usual "---" meaning
trailer --parse help: expose aliased options
trailer --only-input: prefer "configuration variables" over "rules"
trailer --parse docs: add explanation for its usefulness
trailer --unfold help: prefer "reformat" over "join"
trailer doc: emphasize the effect of configuration variables
trailer doc: separator within key suppresses default separator
trailer doc: <token> is a <key> or <keyAlias>, not both
trailer: separate public from internal portion of trailer_iterator
trailer: split process_input_file into separate pieces
trailer: split process_command_line_args into separate functions
strvec: drop unnecessary include of hex.h
M Hickford (3):
credential/libsecret: store new attributes
credential/libsecret: erase matching creds only
credential/wincred: erase matching creds only
Mark Levedahl (6):
git gui Makefile - remove Cygwin modifications
git-gui - remove obsolete Cygwin specific code
git-gui - use cygstart to browse on Cygwin
git-gui - use mkshortcut on Cygwin
git-gui - re-enable use of hook scripts
git-gui - use git-hook, honor core.hooksPath
Mark Ruvald Pedersen (1):
sequencer: truncate labels to accommodate loose refs
Martin Ågren (1):
git-merge-file doc: drop "-file" from argument placeholders
Matthew McClain (1):
git-p4 shouldn't attempt to store symlinks in LFS
Michael Strawbridge (1):
send-email: move validation code below process_address_list
Michal Suchanek (1):
git-push doc: more visibility for -q option
Naomi Ibe (1):
builtin/add.c: clean up die() messages
Oswald Buddenhagen (16):
t/lib-rebase: set_fake_editor(): fix recognition of reset's short command
t/lib-rebase: set_fake_editor(): handle FAKE_LINES more consistently
sequencer: simplify allocation of result array in todo_list_rearrange_squash()
t/lib-rebase: improve documentation of set_fake_editor()
t9001: fix indentation in test_no_confirm()
format-patch: add --description-file option
sequencer: rectify empty hint in call of require_clean_work_tree()
sequencer: beautify subject of reverts of reverts
git-revert.txt: add discussion
sequencer: fix error message on failure to copy SQUASH_MSG
t3404-rebase-interactive.sh: fix typos in title of a rewording test
sequencer: remove unreachable exit condition in pick_commits()
am: fix error message in parse_opt_show_current_patch()
rebase: simplify code related to imply_merge()
rebase: handle --strategy via imply_merge() as well
rebase: move parse_opt_keep_empty() down
Patrick Steinhardt (23):
upload-pack: fix exit code when denying fetch of unreachable object ID
revision: make pseudo-opt flags read via stdin behave consistently
doc/git-worktree: mention "refs/rewritten" as per-worktree refs
doc/git-repack: fix syntax for `-g` shorthand option
doc/git-repack: don't mention nonexistent "--unpacked" option
commit-graph: introduce envvar to disable commit existence checks
commit: detect commits that exist in commit-graph but not in the ODB
builtin/show-ref: convert pattern to a local variable
builtin/show-ref: split up different subcommands
builtin/show-ref: fix leaking string buffer
builtin/show-ref: fix dead code when passing patterns
builtin/show-ref: refactor `--exclude-existing` options
builtin/show-ref: stop using global variable to count matches
builtin/show-ref: stop using global vars for `show_one()`
builtin/show-ref: refactor options for patterns subcommand
builtin/show-ref: ensure mutual exclusiveness of subcommands
builtin/show-ref: explicitly spell out different modes in synopsis
builtin/show-ref: add new mode to check for reference existence
t: use git-show-ref(1) to check for ref existence
test-bloom: stop setting up Git directory twice
shallow: fix memory leak when registering shallow roots
setup: refactor `upgrade_repository_format()` to have common exit
setup: fix leaking repository format
Philippe Blain (3):
completion: commit: complete configured trailer tokens
completion: commit: complete trailers tokens more robustly
completion: improve doc for complex aliases
Phillip Wood (7):
rebase -i: move unlink() calls
rebase -i: remove patch file after conflict resolution
sequencer: use rebase_path_message()
sequencer: factor out part of pick_commits()
rebase: fix rewritten list for failed pick
rebase --continue: refuse to commit after failed command
rebase -i: fix adding failed command to the todo list
René Scharfe (18):
subtree: disallow --no-{help,quiet,debug,branch,message}
t1502, docs: disallow --no-help
t1502: move optionspec help output to a file
t1502: test option negation
parse-options: show negatability of options in short help
parse-options: factor out usage_indent() and usage_padding()
parse-options: no --[no-]no-...
parse-options: simplify usage_padding()
parse-options: allow omitting option help text
name-rev: use OPT_HIDDEN_BOOL for --peel-tag
grep: use OPT_INTEGER_F for --max-depth
grep: reject --no-or
diff --no-index: fix -R with stdin
parse-options: drop unused parse_opt_ctx_t member
parse-options: make CMDMODE errors more precise
am: simplify --show-current-patch handling
am, rebase: fix arghelp syntax of --empty
reflog: fix expire --single-worktree
Robert Coup (1):
upload-pack: add tracing for fetches
Rubén Justo (2):
branch: error message deleting a branch in use
branch: error message checking out a branch in use
Sergey Organov (4):
doc/diff-options: fix link to generating patch section
diff-merges: improve --diff-merges documentation
diff-merges: introduce '--dd' option
completion: complete '--dd'
Shuqi Liang (3):
t1092: add tests for 'git check-attr'
attr.c: read attributes in a sparse directory
check-attr: integrate with sparse-index
Tang Yuyi (1):
merge-tree: add -X strategy option
Taylor Blau (28):
repack: move `pack_geometry` struct to the stack
commit-graph: introduce `commit_graph_generation_from_graph()`
t/t5318-commit-graph.sh: test generation zero transitions during fsck
commit-graph: avoid repeated mixed generation number warnings
leak tests: mark a handful of tests as leak-free
leak tests: mark t3321-notes-stripspace.sh as leak-free
leak tests: mark t5583-push-branches.sh as leak-free
builtin/pack-objects.c: remove unnecessary strbuf_reset()
builtin/pack-objects.c: support `--max-pack-size` with `--cruft`
Documentation/gitformat-pack.txt: remove multi-cruft packs alternative
Documentation/gitformat-pack.txt: drop mixed version section
builtin/repack.c: extract structure to store existing packs
builtin/repack.c: extract marking packs for deletion
builtin/repack.c: extract redundant pack cleanup for --geometric
builtin/repack.c: extract redundant pack cleanup for existing packs
builtin/repack.c: extract `has_existing_non_kept_packs()`
builtin/repack.c: store existing cruft packs separately
builtin/repack.c: avoid directly inspecting "util"
builtin/repack.c: extract common cruft pack loop
git-send-email.perl: avoid printing undef when validating addresses
t7700: split cruft-related tests to t7704
builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
builtin/repack.c: implement support for `--max-cruft-size`
builtin/repack.c: avoid making cruft packs preferred
Documentation/gitformat-pack.txt: fix typo
Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
list-objects: drop --unpacked non-commit objects from results
pack-bitmap: drop --unpacked non-commit objects from results
Todd Zullinger (2):
RelNotes: minor typo fixes in 2.43.0 draft
RelNotes: improve wording of credential helper notes
Victoria Dye (4):
ref-cache.c: fix prefix matching in ref iteration
dir.[ch]: expose 'get_dtype'
dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
files-backend.c: avoid stat in 'loose_fill_ref_dir'
Vipul Kumar (1):
git-gui: Fix a typo in README
Wesley Schwengle (2):
git-push.txt: fix grammar
git-svn: drop FakeTerm hack
brian m. carlson (2):
t: add a test helper to truncate files
merge-file: add an option to process object IDs
Ævar Arnfjörð Bjarmason (1):
Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4
Štěpán Němec (6):
doc: fix some typos, grammar and wording issues
doc/diff-options: improve wording of the log.diffMerges mention
git-jump: admit to passing merge mode args to ls-files
doc/gitk: s/sticked/stuck/
t/README: fix multi-prerequisite example
doc/cat-file: make synopsis and description less confusing
王常新 (1):
merge-ort.c: fix typo 'neeed' to 'needed'
谢致邦 (XIE Zhibang) (2):
doc: correct the 50 characters soft limit
doc: correct the 50 characters soft limit (+)
^ permalink raw reply
* Re: [PATCH] send-email: avoid duplicate specification warnings
From: Junio C Hamano @ 2023-11-14 17:32 UTC (permalink / raw)
To: Todd Zullinger
Cc: git, Ævar Arnfjörð Bjarmason, Jeff King,
Ondřej Pohořelský
In-Reply-To: <20231114163826.207267-1-tmz@pobox.com>
Todd Zullinger <tmz@pobox.com> writes:
> 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:
>
> | +++ diff -u expect actual
> | --- expect 2023-11-14 10:38:23.854346488 +0000
> | +++ actual 2023-11-14 10:38:23.848346466 +0000
> | @@ -1,2 +1,7 @@
> | +Duplicate specification "no-chain-reply-to" for option "no-chain-reply-to"
> | +Duplicate specification "to-cover|to-cover!" for option "to-cover"
> | +Duplicate specification "cc-cover|cc-cover!" for option "cc-cover"
> | +Duplicate specification "no-thread" for option "no-thread"
> | +Duplicate specification "no-to-cover" for option "no-to-cover"
> | fatal: longline.patch:35 is longer than 998 characters
> | warning: no patches were sent
> | error: last command exited with $?=1
> | not ok 37 - reject long lines
>
> Remove the duplicate option specs.
As long as these manual implementation of "no-" are doing true
opposite of the positive one, it should be sufficient to remove
them, so I'd prefer to see you explicitly say that you did audit
them all to make sure.
For example,
> "annotate!" => \$annotate,
> - "no-annotate" => sub {$annotate = 0},
this is an example of good pair. With the former, "--no-annotate"
and "--annotate" result in $annotate set to false and true, and the
latter attempts to set $annotate to false upon "--no-annotate", so
the net result of removing the latter should be a no-op.
> "suppress-from!" => \$suppress_from,
> - "no-suppress-from" => sub {$suppress_from = 0},
Ditto.
As it is very late at night here, I didn't do a though job to scan
and validate all of them (some did not have their positive
counterparts in the context), though. Thanks for woking on this.
^ permalink raw reply
* What's cooking in git.git (Nov 2023, #06; Tue, 14)
From: Junio C Hamano @ 2023-11-14 18:00 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Git 2.43-rc2 has been tagged.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[New Topics]
* js/ci-discard-prove-state (2023-11-14) 1 commit
(merged to 'next' on 2023-11-14 at fade3ba143)
+ ci: avoid running the test suite _twice_
(this branch uses ps/ci-gitlab.)
The way CI testing used "prove" could lead to running the test
suite twice needlessly, which has been corrected.
Will cook in 'next'.
source: <pull.1613.git.1699894837844.gitgitgadget@gmail.com>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* jk/chunk-bounds-more (2023-11-09) 9 commits
(merged to 'next' on 2023-11-13 at 3df4b18bea)
+ commit-graph: mark chunk error messages for translation
+ commit-graph: drop verify_commit_graph_lite()
+ commit-graph: check order while reading fanout chunk
+ commit-graph: use fanout value for graph size
+ commit-graph: abort as soon as we see a bogus chunk
+ commit-graph: clarify missing-chunk error messages
+ commit-graph: drop redundant call to "lite" verification
+ midx: check consistency of fanout table
+ commit-graph: handle overflow in chunk_size checks
(this branch is used by tb/pair-chunk-expect.)
Code clean-up for jk/chunk-bounds topic.
Will cook in 'next'.
source: <20231109070310.GA2697602@coredump.intra.peff.net>
* ps/httpd-tests-on-nixos (2023-11-11) 3 commits
(merged to 'next' on 2023-11-13 at 81bd6f5334)
+ t9164: fix inability to find basename(1) in Subversion hooks
+ t/lib-httpd: stop using legacy crypt(3) for authentication
+ t/lib-httpd: dynamically detect httpd and modules path
Portability tweak.
Will cook in 'next'.
source: <cover.1699596457.git.ps@pks.im>
* ss/format-patch-use-encode-headers-for-cover-letter (2023-11-10) 1 commit
(merged to 'next' on 2023-11-14 at 1a4bd59e15)
+ format-patch: fix ignored encode_email_headers for cover letter
"git format-patch --encode-email-headers" ignored the option when
preparing the cover letter, which has been corrected.
Will cook in 'next'.
source: <20231109111950.387219-1-contact@emersion.fr>
* ps/ban-a-or-o-operator-with-test (2023-11-11) 4 commits
(merged to 'next' on 2023-11-14 at d84471baab)
+ Makefile: stop using `test -o` when unlinking duplicate executables
+ contrib/subtree: convert subtree type check to use case statement
+ contrib/subtree: stop using `-o` to test for number of args
+ global: convert trivial usages of `test <expr> -a/-o <expr>`
Test and shell scripts clean-up.
Will cook in 'next'.
source: <cover.1699609940.git.ps@pks.im>
* vd/glossary-dereference-peel (2023-11-14) 1 commit
- glossary: add definitions for dereference & peel
"To dereference" and "to peel" were sometimes used in in-code
comments and documentation but without description in the glossary.
Will merge to 'next'.
source: <pull.1610.v2.git.1699917471769.gitgitgadget@gmail.com>
* ak/rebase-autosquash (2023-11-13) 4 commits
- rebase: rewrite --(no-)autosquash documentation
- rebase: test autosquash with and without -i
- rebase: support --autosquash without -i
- rebase: fully ignore rebase.autoSquash without -i
"git rebase --autosquash" is now enabled for non-interactive rebase,
but it is still incompatible with the apply backend.
Expecting a (hopefully small and final) reroll.
cf. <058eb5d9-35d9-4452-8d33-d9cfebb94347@gmail.com>
source: <20231111132720.78877-1-andy.koppe@gmail.com>
* vd/for-each-ref-unsorted-optimization (2023-11-07) 9 commits
- t/perf: add perf tests for for-each-ref
- for-each-ref: add option to fully dereference tags
- ref-filter.c: filter & format refs in the same callback
- ref-filter.c: refactor to create common helper functions
- ref-filter.h: add functions for filter/format & format-only
- ref-filter.h: move contains caches into filter
- ref-filter.h: add max_count and omit_empty to ref_format
- for-each-ref: clarify interaction of --omit-empty & --count
- ref-filter.c: really don't sort when using --no-sort
"git for-each-ref --no-sort" still sorted the refs alphabetically
which paid non-trivial cost. It has been redefined to show output
in an unspecified order, to allow certain optimizations to take
advantage of.
Expecting a reroll.
cf. <dbcbcf0e-aeee-4bb9-9e39-e2e85194d083@github.com>
source: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
* jw/git-add-attr-pathspec (2023-11-04) 1 commit
(merged to 'next' on 2023-11-13 at b61be94e4d)
+ attr: enable attr pathspec magic for git-add and git-stash
"git add" and "git stash" learned to support the ":(attr:...)"
magic pathspec.
Will cook in 'next'.
source: <20231103163449.1578841-1-jojwang@google.com>
* ps/ci-gitlab (2023-11-09) 8 commits
(merged to 'next' on 2023-11-10 at ea7ed67945)
+ ci: add support for GitLab CI
+ ci: install test dependencies for linux-musl
+ ci: squelch warnings when testing with unusable Git repo
+ ci: unify setup of some environment variables
+ ci: split out logic to set up failed test artifacts
+ ci: group installation of Docker dependencies
+ ci: make grouping setup more generic
+ ci: reorder definitions for grouping functions
(this branch is used by js/ci-discard-prove-state.)
Add support for GitLab CI.
Will cook in 'next'.
source: <cover.1699514143.git.ps@pks.im>
* ps/ref-tests-update (2023-11-03) 10 commits
(merged to 'next' on 2023-11-13 at dc26e55d6f)
+ t: mark several tests that assume the files backend with REFFILES
+ t7900: assert the absence of refs via git-for-each-ref(1)
+ t7300: assert exact states of repo
+ t4207: delete replace references via git-update-ref(1)
+ t1450: convert tests to remove worktrees via git-worktree(1)
+ t: convert tests to not access reflog via the filesystem
+ t: convert tests to not access symrefs via the filesystem
+ t: convert tests to not write references via the filesystem
+ t: allow skipping expected object ID in `ref-store update-ref`
+ Merge branch 'ps/show-ref' into ps/ref-tests-update
Update ref-related tests.
Will cook in 'next'.
source: <cover.1698914571.git.ps@pks.im>
* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
- fetch: no redundant error message for atomic fetch
- t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Expecting an update.
cf. <ZTjQIrCgSANAT8wR@tanuki>
source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
Instead of auto-generating a filename that is already in use for
output and fail the command, `git bugreport` learned to fuzz the
filename to avoid collisions with existing files.
Expecting a reroll.
cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
Test clean-up.
Perhaps discard?
cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
source: <cover.1697319294.git.code@khaugsbakk.name>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
(this branch uses jk/chunk-bounds-more.)
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1697653929.git.me@ttaylorr.com>
* cc/git-replay (2023-11-03) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
Comments?
source: <20231102135151.843758-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-10-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Comments?
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* js/doc-unit-tests (2023-11-10) 3 commits
(merged to 'next' on 2023-11-10 at 7d00ffd06b)
+ ci: run unit tests in CI
+ unit tests: add TAP unit test framework
+ unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Will cook in 'next'.
source: <cover.1699555664.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-11-10) 7 commits
(merged to 'next' on 2023-11-10 at b4503c9c8c)
+ cmake: handle also unit tests
+ cmake: use test names instead of full paths
+ cmake: fix typo in variable name
+ artifacts-tar: when including `.dll` files, don't forget the unit-tests
+ unit-tests: do show relative file paths
+ unit-tests: do not mistake `.pdb` files for being executable
+ cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Will cook in 'next'.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/strbuf-comment-line-char (2023-11-01) 4 commits
. strbuf: move env-using functions to environment.c
. strbuf: make add_lines() public
. strbuf_add_commented_lines(): drop the comment_line_char parameter
. strbuf_commented_addf(): drop the comment_line_char parameter
Code simplification that goes directly against a past libification
topic. It is hard to judge because the "libification" is done
piecewise without seemingly clear design principle.
Will discard.
source: <cover.1698791220.git.jonathantanmy@google.com>
^ permalink raw reply
* Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Jeff King @ 2023-11-14 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, git, Karthik Nayak
In-Reply-To: <xmqqzfzgxops.fsf@gitster.g>
On Wed, Nov 15, 2023 at 01:51:43AM +0900, Junio C Hamano wrote:
> >> Both of these are expected failures: we knowingly corrupt the repository
> >> and circumvent git-gc(1)/git-maintenance(1), thus no commit-graphs are
> >> updated. If we stick with the new stance that repository corruption
> >> should not require us to pessimize the common case,...
> >
> > Yeah, just like we try to be extra careful while running fsck,
> > because "--missing" is about finding these "corrupt" cases,
> > triggering the paranoia mode upon seeing the option would make
> > sense, no? It would fix the failure in 6022, right?
> >
> > Thanks for working on this.
>
> Just to make sure we do not miscommunicate, I do not think we want
> to trigger the paranoia mode only in our tests. We want to be
> paranoid to help real users who used "--missing" for their real use,
> so enabling PARANOIA in the test script is a wrong approach. We
> should enable it inside "rev-list --missing" codepath.
Yeah. Just like we auto-enabled GIT_REF_PARANOIA for git-gc, etc, I
think we should do the same here.
As we are closing in on the v2.43 release, there's one thing I'm not
sure about regarding release planning. Are these test cases that _used_
to detect the corruption, but now don't? I.e., I would have expected
that disabling GIT_COMMIT_GRAPH_PARANOIA would take us back to the same
state as v2.42. But I think it doesn't because of the hunk in e04838ea82
(commit-graph: introduce envvar to disable commit existence checks,
2023-10-31) that makes the has_object() call conditional (and now
defaults to off).
What I'm getting as it that I think we have three options for v2.43:
1. Ship what has been in the release candidates, which has a known
performance regression (though the severity is up for debate).
2. Flip the default to "0" (i.e., Patrick's patch in this thread). We
know that loosens some cases versus 2.42, which may be considered a
regression.
3. Sort it out before the release. We're getting pretty close to do
a lot new work there, but I think the changes should be small-ish.
The nuclear option is ejecting the topic and re-doing it in the
next cycle.
I don't have a really strong preference between the three.
-Peff
^ permalink raw reply
* Re: Feature request: git status --branch-only
From: Ondra Medek @ 2023-11-14 19:44 UTC (permalink / raw)
To: phillip.wood; +Cc: git
In-Reply-To: <00033c86-dbd7-4c88-bfbd-8f6766cd66c9@gmail.com>
Hi Phillip,
even "[gone]" would not be much of help for me. I would need something
like "origin/master [gone]" i.e. what "git status -b --porcelain"
prints. (Note, I've written about "git status -b --porcelain=v2"
before because v2 is better documented and parseable.)
Regards
Ondra Medek
On Tue, 14 Nov 2023 at 16:02, Phillip Wood <phillip.wood123@gmail.com> 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.
>
> Best Wishes
>
> Phillip
>
>
> > git status -b --no-ahead-behind --porcelain=v2
> >
> > outputs
> >
> > # branch.oid (initial)
> > # branch.head master
> > # branch.upstream origin/master
> >
> > I.e. it outputs a proper upstream branch.
> >
> > Best regards
> > Ondra
> >
> > Ondra Medek
> >
> >
> > On Tue, 14 Nov 2023 at 13:28, Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>
> >> Hi Ondra
> >>
> >> On 14/11/2023 10:16, Ondra Medek wrote:
> >>> Hello,
> >>> I am working on a tol which should fetch changes from a remote
> >>> repository on a user click. I want to limit fetch on the current
> >>> remote tracking branch (something like "origin/master"), but
> >>> surprisingly, it's hard to get it for all corner cases like a fresh
> >>> clone of an empty repository or detached head, etc. E.g see this SO
> >>> thread https://stackoverflow.com/questions/171550/find-out-which-remote-branch-a-local-branch-is-tracking/52896538
> >>
> >> I think you can do this by calling
> >>
> >> git symbolic-ref --quiet HEAD
> >>
> >> to get the full refname of the current branch. If HEAD is detached it
> >> will print nothing and exit with exit code 1. Then you can call
> >>
> >> git for-each-ref --format="%(upstream:short)" $refname
> >>
> >> to get the upstream branch
> >>
> >> Best Wishes
> >>
> >> Phillip
> >
^ permalink raw reply
* [PATCH v2 01/10] ref-filter.c: really don't sort when using --no-sort
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>
When '--no-sort' is passed to 'for-each-ref', 'tag', and 'branch', the
printed refs are still sorted by ascending refname. Change the handling of
sort options in these commands so that '--no-sort' to truly disables
sorting.
'--no-sort' does not disable sorting in these commands is because their
option parsing does not distinguish between "the absence of '--sort'"
(and/or values for tag.sort & branch.sort) and '--no-sort'. Both result in
an empty 'sorting_options' string list, which is parsed by
'ref_sorting_options()' to create the 'struct ref_sorting *' for the
command. If the string list is empty, 'ref_sorting_options()' interprets
that as "the absence of '--sort'" and returns the default ref sorting
structure (equivalent to "refname" sort).
To handle '--no-sort' properly while preserving the "refname" sort in the
"absence of --sort'" case, first explicitly add "refname" to the string list
*before* parsing options. This alone 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.
Now that the string list is populated by default, '--no-sort' is the only
way to empty the 'sorting_options' string list. Update
'ref_sorting_options()' to return a NULL 'struct ref_sorting *' if the
string list is empty, and add a condition to 'ref_array_sort()' to skip the
sort altogether if the sort structure is NULL. Note that other functions
using 'struct ref_sorting *' do not need any changes because they already
ignore NULL values.
Finally, remove the condition around sorting in 'ls-remote', since it's no
longer necessary. Unlike 'for-each-ref' et. al., it does *not* do any
sorting by default. This default is preserved by simply leaving its sort key
string list empty before parsing options; if no additional sort keys are
set, 'struct ref_sorting *' is NULL and sorting is skipped.
Signed-off-by: Victoria Dye <vdye@github.com>
---
builtin/branch.c | 6 ++++
builtin/for-each-ref.c | 3 ++
builtin/ls-remote.c | 11 +++----
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, 153 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..b416602b4d3 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];
@@ -157,6 +153,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
status = 0; /* we found something */
}
+ ref_sorting_release(sorting);
ref_array_clear(&ref_array);
if (transport_disconnect(transport))
status = 1;
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 v2 00/10] for-each-ref optimizations & usability improvements
From: Victoria Dye via GitGitGadget @ 2023-11-14 19:53 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Øystein Walle, Kristoffer Haugsbakk,
Victoria Dye
In-Reply-To: <pull.1609.git.1699320361.gitgitgadget@gmail.com>
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). Now, '--no-sort'
completely disables sorting (unless subsequent '--sort' options are
provided).
Patches 2-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 updates the 'for-each-ref' documentation, making the '--format'
description a bit less jumbled and more clearly explaining the '*' prefix
(to be updated in the next patch)
Patch 9 changes the dereferencing done by the '*' format prefix from a
single dereference to a recursive peel. See [1] + replies for the discussion
that led to this approach (as opposed to a command line option or new format
specifier).
[1] https://lore.kernel.org/git/ZUoWWo7IEKsiSx-C@tanuki/
Finally, patch 10 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 HEAD
----------------------------------------------------------------------------
6300.2: (loose) 4.68(0.98+3.64)
6300.3: (loose, no sort) 4.65(0.91+3.67)
6300.4: (loose, --count=1) 4.50(0.84+3.60)
6300.5: (loose, --count=1, no sort) 4.24(0.46+3.71)
6300.6: (loose, tags) 2.41(0.45+1.93)
6300.7: (loose, tags, no sort) 2.33(0.48+1.83)
6300.8: (loose, tags, dereferenced) 3.65(1.66+1.95)
6300.9: (loose, tags, dereferenced, no sort) 3.48(1.59+1.87)
6300.10: for-each-ref + cat-file (loose, tags) 4.48(2.27+2.22)
6300.12: (packed) 0.90(0.68+0.18)
6300.13: (packed, no sort) 0.71(0.55+0.06)
6300.14: (packed, --count=1) 0.77(0.52+0.16)
6300.15: (packed, --count=1, no sort) 0.03(0.01+0.02)
6300.16: (packed, tags) 0.45(0.33+0.10)
6300.17: (packed, tags, no sort) 0.39(0.33+0.03)
6300.18: (packed, tags, dereferenced) 1.83(1.67+0.10)
6300.19: (packed, tags, dereferenced, no sort) 1.42(1.28+0.08)
6300.20: for-each-ref + cat-file (packed, tags) 2.36(2.11+0.29)
* Victoria
Changes since V1
================
* Restructured commit message of patch 1 for better readability
* Re-added 'ref_sorting_release(sorting)' to 'ls-remote'
* Dropped patch 2 so we don't commit to behavior we don't want in
'for-each-ref --omit-empty --count'
* Split patch 6 into one that renames 'ref_filter_handler()' to
'filter_one()' and another that creates helper functions from existing
code
* Added/updated code comments in patch 7, changed ref iteration "break"
return value from -1 to 1
* Added a patch to reword 'for-each-ref' documentation in anticipation of
updating the description of what '*' does in the format
* Removed command-line option '--full-deref' for peeling tags in '*' format
fields in favor of simply cutting over from the current single
dereference to recursive dereference in all cases. Updated tests to match
new behavior.
* Added the '--count=1' tests back to p6300 (I must have unintentionally
removed them before submitting V1)
Victoria Dye (10):
ref-filter.c: really don't sort when using --no-sort
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: rename 'ref_filter_handler()' to 'filter_one()'
ref-filter.c: refactor to create common helper functions
ref-filter.c: filter & format refs in the same callback
for-each-ref: clean up documentation of --format
ref-filter.c: use peeled tag for '*' format fields
t/perf: add perf tests for for-each-ref
Documentation/git-for-each-ref.txt | 23 +--
builtin/branch.c | 42 +++--
builtin/for-each-ref.c | 39 +----
builtin/ls-remote.c | 11 +-
builtin/tag.c | 32 +---
ref-filter.c | 272 ++++++++++++++++++++---------
ref-filter.h | 25 +++
t/perf/p6300-for-each-ref.sh | 87 +++++++++
t/t3200-branch.sh | 68 +++++++-
t/t6300-for-each-ref.sh | 43 +++++
t/t6302-for-each-ref-filter.sh | 4 +-
t/t7004-tag.sh | 45 +++++
12 files changed, 517 insertions(+), 174 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-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1609/vdye/vdye/for-each-ref-optimizations-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1609
Range-diff vs v1:
1: dea8d7d1e86 ! 1: 074da1ff3e8 ref-filter.c: really don't sort when using --no-sort
@@ Metadata
## Commit message ##
ref-filter.c: really don't sort when using --no-sort
- 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.
+ When '--no-sort' is passed to 'for-each-ref', 'tag', and 'branch', the
+ printed refs are still sorted by ascending refname. Change the handling of
+ sort options in these commands so that '--no-sort' to truly disables
+ sorting.
+
+ '--no-sort' does not disable sorting in these commands is because their
+ option parsing does not distinguish between "the absence of '--sort'"
+ (and/or values for tag.sort & branch.sort) and '--no-sort'. Both result in
+ an empty 'sorting_options' string list, which is parsed by
+ 'ref_sorting_options()' to create the 'struct ref_sorting *' for the
+ command. If the string list is empty, 'ref_sorting_options()' interprets
+ that as "the absence of '--sort'" and returns the default ref sorting
+ structure (equivalent to "refname" sort).
- 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 handle '--no-sort' properly while preserving the "refname" sort in the
+ "absence of --sort'" case, first explicitly add "refname" to the string list
+ *before* parsing options. This alone 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.
- 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.
+ Now that the string list is populated by default, '--no-sort' is the only
+ way to empty the 'sorting_options' string list. Update
+ 'ref_sorting_options()' to return a NULL 'struct ref_sorting *' if the
+ string list is empty, and add a condition to 'ref_array_sort()' to skip the
+ sort altogether if the sort structure is NULL. Note that other functions
+ using 'struct ref_sorting *' do not need any changes because they already
+ ignore NULL values.
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()'.
+ longer necessary. Unlike 'for-each-ref' et. al., it does *not* do any
+ sorting by default. This default is preserved by simply leaving its sort key
+ string list empty before parsing options; if no additional sort keys are
+ set, 'struct ref_sorting *' is NULL and sorting is skipped.
Signed-off-by: Victoria Dye <vdye@github.com>
@@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *
for (i = 0; i < ref_array.nr; i++) {
const struct ref_array_item *ref = ref_array.items[i];
+@@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char *prefix)
+ status = 0; /* we found something */
+ }
+
++ ref_sorting_release(sorting);
+ ref_array_clear(&ref_array);
+ if (transport_disconnect(transport))
+ status = 1;
## builtin/tag.c ##
@@ builtin/tag.c: int cmd_tag(int argc, const char **argv, const char *prefix)
2: 88eba4146cd < -: ----------- for-each-ref: clarify interaction of --omit-empty & --count
3: 2e2f9738205 = 2: adac101bc60 ref-filter.h: add max_count and omit_empty to ref_format
4: 6c66445ee31 ! 3: f44c4b42c93 ref-filter.h: move contains caches into filter
@@ Commit message
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
+ 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'.
5: f5be57eea7d = 4: 187b1d6610f ref-filter.h: add functions for filter/format & format-only
-: ----------- > 5: 040d291ca45 ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'
6: 8c77452e5dd ! 6: 633c0c74c2e ref-filter.c: refactor to create common helper functions
@@ Commit message
ref-filter.c: refactor to create common helper functions
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'.
+ 'filter_refs()' into new helper functions:
- The changes are as follows:
+ * 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()'.
- * 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.
+ 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: static int filter_ref_kind(struct ref_filter *filter, const char *
- * 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)
+static struct ref_array_item *apply_ref_filter(const char *refname, const struct object_id *oid,
+ int flag, struct ref_filter *filter)
{
@@ ref-filter.c: static int filter_ref_kind(struct ref_filter *filter, const char *
/*
* A merge filter is applied on refs pointing to commits. Hence
-@@ ref-filter.c: static int ref_filter_handler(const char *refname, const struct object_id *oid,
+@@ ref-filter.c: 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)
@@ ref-filter.c: static int ref_filter_handler(const char *refname, const struct ob
}
/*
-@@ ref-filter.c: static int ref_filter_handler(const char *refname, const struct object_id *oid,
+@@ ref-filter.c: 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-filter.c: int filter_refs(struct ref_array *array, struct ref_filter *filter
* 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);
+ 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/", 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/", ref_filter_handler, &ref_cbdata);
+- 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, ref_filter_handler, &ref_cbdata);
+- 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(ref_filter_handler, &ref_cbdata);
+- head_ref(filter_one, &ref_cbdata);
+ head_ref(fn, cb_data);
}
7: 84db440896c ! 7: 91a77c1a834 ref-filter.c: filter & format refs in the same callback
@@ ref-filter.c: static void free_array_item(struct ref_array_item *item)
+ 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 1;
+
+ return 0;
+}
@@ ref-filter.c: int filter_refs(struct ref_array *array, struct ref_filter *filter
+ 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.
++ * 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 ||
-: ----------- > 8: 8eb2fc2950c for-each-ref: clean up documentation of --format
8: 352b5c42ac3 ! 9: 48254d8e161 for-each-ref: add option to fully dereference tags
@@ Metadata
Author: Victoria Dye <vdye@github.com>
## Commit message ##
- for-each-ref: add option to fully dereference tags
+ ref-filter.c: use peeled tag for '*' format fields
- 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.
+ 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 the fully dereferenced tag, they can do so with something like
+ 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
- efficiency of this use case, add a '--full-deref' option that causes
- 'for-each-ref' to fully dereference tags when formatting with '*' fields.
+ 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 ##
-@@ Documentation/git-for-each-ref.txt: 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>]]
-@@ Documentation/git-for-each-ref.txt: OPTIONS
- the specified host language. This is meant to produce
- a scriptlet that can directly be `eval`ed.
+@@ Documentation/git-for-each-ref.txt: from the `committer` or `tagger` fields depending on the object type.
+ These are intended for working on a mix of annotated and lightweight tags.
-+--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.
+ 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.
-
- ## builtin/for-each-ref.c ##
-@@ builtin/for-each-ref.c: 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,
+ Fields that have name-email-date tuple as its value (`author`,
+ `committer`, and `tagger`) can be suffixed with `name`, `email`,
## ref-filter.c ##
-@@ ref-filter.c: 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.
-@@ ref-filter.c: 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;
@@ ref-filter.c: 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.
+ 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 == DEREF_ALL) {
-+ if (peel_iterated_oid(&obj->oid, &oi_deref.oid))
-+ die("bad tag");
-+ } else {
-+ 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
@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbu
}
- ## ref-filter.h ##
-@@ ref-filter.h: 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;
-
## t/t6300-for-each-ref.sh ##
@@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref with non-existing refs' '
test_must_be_empty actual
@@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref with non-existing
+ 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 \
++ git for-each-ref \
+ --format="%(refname) %(objectname) %(objecttype) %(*objectname) %(*objecttype)" \
+ refs/tags/nested/ >actual &&
+ test_cmp expect actual
@@ t/t6300-for-each-ref.sh: test_expect_success 'git for-each-ref with non-existing
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)"
+
+ ## t/t6302-for-each-ref-filter.sh ##
+@@ t/t6302-for-each-ref-filter.sh: 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
9: a409d773057 ! 10: d51d073aa4a t/perf: add perf tests for for-each-ref
@@ Commit message
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'.
+ 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:
@@ t/perf/p6300-for-each-ref.sh (new)
+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, 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_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 (full deref)" "
++ 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
--
gitgitgadget
^ permalink raw reply
* [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