* Re: [PATCH v1 4/4] maintenance: use XDG config if it exists
From: Eric Sunshine @ 2023-10-23 11:39 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Kristoffer Haugsbakk, git, stolee
In-Reply-To: <ZTZDsIcrB0zwHlFR@tanuki>
On Mon, Oct 23, 2023 at 5:58 AM Patrick Steinhardt <ps@pks.im> wrote:
> On Wed, Oct 18, 2023 at 10:28:41PM +0200, Kristoffer Haugsbakk wrote:
> > `git maintenance register` registers the repository in the user's global
> > config. `$XDG_CONFIG_HOME/git/config` is supposed to be used if
> > `~/.gitconfig` does not exist. However, this command creates a
> > `~/.gitconfig` file and writes to that one even though the XDG variant
> > exists.
> >
> > This used to work correctly until 50a044f1e4 (gc: replace config
> > subprocesses with API calls, 2022-09-27), when the command started calling
> > the config API instead of git-config(1).
> >
> > Also change `unregister` accordingly.
> >
> > Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> > ---
> > +test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
> > + XDG_CONFIG_HOME=.config &&
> > + test_when_finished rm -r "$XDG_CONFIG_HOME"/git/config &&
> > + export "XDG_CONFIG_HOME" &&
>
> Also, I think we need to unset this variable at the end of this test as
> tests don't run in a subshell. [...]
Yup, well spotted. Almost the entire body of this test should be in a
subshell to ensure that the environment variable does not live beyond
the end of this test. But test_when_finished() can't be used in a
subshell, so a little care is needed:
test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
test_when_finished rm -r .config/git/config &&
(
XDG_CONFIG_HOME=.config &&
...
)
'
> > + mkdir -p "$XDG_CONFIG_HOME"/git &&
> > + touch "$XDG_CONFIG_HOME"/git/config &&
If the timestamp of the file is not significant, then we use `>` to
create it rather than `touch`:
>"$XDG_CONFIG_HOME"/git/config &&
> > + git maintenance register &&
> > + git config --file="$XDG_CONFIG_HOME"/git/config --get maintenance.repo >actual &&
> > + pwd >expect &&
> > + test_cmp expect actual
> > +'
^ permalink raw reply
* [PATCH v2 2/2] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-23 11:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Junio C Hamano, Taylor Blau, Jeff King
In-Reply-To: <cover.1698060036.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6787 bytes --]
Commit graphs can become stale and contain references to commits that do
not exist in the object database anymore. Theoretically, this can lead
to a scenario where we are able to successfully look up any such commit
via the commit graph even though such a lookup would fail if done via
the object database directly.
As the commit graph is mostly intended as a sort of cache to speed up
parsing of commits we do not want to have diverging behaviour in a
repository with and a repository without commit graphs, no matter
whether they are stale or not. As commits are otherwise immutable, the
only thing that we really need to care about is thus the presence or
absence of a commit.
To address potentially stale commit data that may exist in the graph,
our `lookup_commit_in_graph()` function will check for the commit's
existence in both the commit graph, but also in the object database. So
even if we were able to look up the commit's data in the graph, we would
still pretend as if the commit didn't exist if it is missing in the
object database.
We don't have the same safety net in `parse_commit_in_graph_one()`
though. This function is mostly used internally in "commit-graph.c"
itself to validate the commit graph, and this usage is fine. We do
expose its functionality via `parse_commit_in_graph()` though, which
gets called by `repo_parse_commit_internal()`, and that function is in
turn used in many places in our codebase.
For all I can see this function is never used to directly turn an object
ID into a commit object without additional safety checks before or after
this lookup. What it is being used for though is to walk history via the
parent chain of commits. So when commits in the parent chain of a graph
walk are missing it is possible that we wouldn't notice if that missing
commit was part of the commit graph. Thus, a query like `git rev-parse
HEAD~2` can succeed even if the intermittent commit is missing.
It's unclear whether there are additional ways in which such stale
commit graphs can lead to problems. In any case, it feels like this is a
bigger bug waiting to happen when we gain additional direct or indirect
callers of `repo_parse_commit_internal()`. So let's fix the inconsistent
behaviour by checking for object existence via the object database, as
well.
This check of course comes with a performance penalty. The following
benchmarks have been executed in a clone of linux.git with stable tags
added:
Benchmark 1: git -c core.commitGraph=true rev-list --topo-order --all (git = master)
Time (mean ± σ): 2.913 s ± 0.018 s [User: 2.363 s, System: 0.548 s]
Range (min … max): 2.894 s … 2.950 s 10 runs
Benchmark 2: git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
Time (mean ± σ): 3.834 s ± 0.052 s [User: 3.276 s, System: 0.556 s]
Range (min … max): 3.780 s … 3.961 s 10 runs
Benchmark 3: git -c core.commitGraph=false rev-list --topo-order --all (git = master)
Time (mean ± σ): 13.841 s ± 0.084 s [User: 13.152 s, System: 0.687 s]
Range (min … max): 13.714 s … 13.995 s 10 runs
Benchmark 4: git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
Time (mean ± σ): 13.762 s ± 0.116 s [User: 13.094 s, System: 0.667 s]
Range (min … max): 13.645 s … 14.038 s 10 runs
Summary
git -c core.commitGraph=true rev-list --topo-order --all (git = master) ran
1.32 ± 0.02 times faster than git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
4.72 ± 0.05 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
4.75 ± 0.04 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = master)
We look at a ~30% regression in general, but in general we're still a
whole lot faster than without the commit graph. To counteract this, the
new check can be turned off with the `GIT_COMMIT_GRAPH_PARANOIA` envvar.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
commit.c | 16 +++++++++++++++-
t/t5318-commit-graph.sh | 27 +++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index b3223478bc..7399e90212 100644
--- a/commit.c
+++ b/commit.c
@@ -28,6 +28,7 @@
#include "shallow.h"
#include "tree.h"
#include "hook.h"
+#include "parse.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
@@ -572,8 +573,21 @@ int repo_parse_commit_internal(struct repository *r,
return -1;
if (item->object.parsed)
return 0;
- if (use_commit_graph && parse_commit_in_graph(r, item))
+ if (use_commit_graph && parse_commit_in_graph(r, item)) {
+ static int object_paranoia = -1;
+
+ if (object_paranoia == -1)
+ object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
+
+ if (object_paranoia && !has_object(r, &item->object.oid, 0)) {
+ unparse_commit(r, &item->object.oid);
+ return quiet_on_missing ? -1 :
+ error(_("commit %s exists in commit-graph but not in the object database"),
+ oid_to_hex(&item->object.oid));
+ }
+
return 0;
+ }
if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
return quiet_on_missing ? -1 :
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index c0cc454538..79467d7926 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -842,4 +842,31 @@ test_expect_success 'stale commit cannot be parsed when given directly' '
)
'
+test_expect_success 'stale commit cannot be parsed when traversing graph' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit A &&
+ test_commit B &&
+ test_commit C &&
+ git commit-graph write --reachable &&
+
+ # Corrupt the repository by deleting the intermittent commit
+ # object. Commands should notice that this object is absent and
+ # thus that the repository is corrupt even if the commit graph
+ # exists.
+ oid=$(git rev-parse B) &&
+ rm .git/objects/"$(test_oid_to_path "$oid")" &&
+
+ # 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 &&
+ # ... but fail when we are paranoid.
+ test_must_fail git rev-parse HEAD~2 2>error &&
+ grep "error: commit $oid exists in commit-graph but not in the object database" error
+ )
+'
+
test_done
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 1/2] commit-graph: introduce envvar to disable commit existence checks
From: Patrick Steinhardt @ 2023-10-23 11:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Junio C Hamano, Taylor Blau, Jeff King
In-Reply-To: <cover.1698060036.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5776 bytes --]
Our `lookup_commit_in_graph()` helper tries to look up commits from the
commit graph and, if it doesn't exist there, falls back to parsing it
from the object database instead. This is intended to speed up the
lookup of any such commit that exists in the database. There is an edge
case though where the commit exists in the graph, but not in the object
database. To avoid returning such stale commits the helper function thus
double checks that any such commit parsed from the graph also exists in
the object database. This makes the function safe to use even when
commit graphs aren't updated regularly.
We're about to introduce the same pattern into other parts of our code
base though, namely `repo_parse_commit_internal()`. Here the extra
sanity check is a bit of a tougher sell: `lookup_commit_in_graph()` was
a newly introduced helper, and as such there was no performance hit by
adding this sanity check. If we added `repo_parse_commit_internal()`
with that sanity check right from the beginning as well, this would
probably never have been an issue to begin with. But by retrofitting it
with this sanity check now we do add a performance regression to
preexisting code, and thus there is a desire to avoid this or at least
give an escape hatch.
In practice, there is no inherent reason why either of those functions
should have the sanity check whereas the other one does not: either both
of them are able to detect this issue or none of them should be. This
also means that the default of whether we do the check should likely be
the same for both. To err on the side of caution, we thus rather want to
make `repo_parse_commit_internal()` stricter than to loosen the checks
that we already have in `lookup_commit_in_graph()`.
The escape hatch is added in the form of a new GIT_COMMIT_GRAPH_PARANOIA
environment variable that mirrors GIT_REF_PARANOIA. If enabled, which is
the default, we will double check that commits looked up in the commit
graph via `lookup_commit_in_graph()` also exist in the object database.
This same check will also be added in `repo_parse_commit_internal()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git.txt | 9 +++++++++
commit-graph.c | 6 +++++-
commit-graph.h | 6 ++++++
t/t5318-commit-graph.sh | 21 +++++++++++++++++++++
4 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 11228956cd..22c2b537aa 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -911,6 +911,15 @@ for full details.
should not normally need to set this to `0`, but it may be
useful when trying to salvage data from a corrupted repository.
+`GIT_COMMIT_GRAPH_PARANOIA`::
+ If this Boolean environment variable is set to false, ignore the
+ case where commits exist in the commit graph but not in the
+ object database. Normally, Git will check whether commits loaded
+ from the commit graph exist in the object database to avoid
+ issues with stale commit graphs, but this check comes with a
+ performance penalty. The default is `1` (i.e., be paranoid about
+ stale commits in the commit graph).
+
`GIT_ALLOW_PROTOCOL`::
If set to a colon-separated list of protocols, behave as if
`protocol.allow` is set to `never`, and each of the listed
diff --git a/commit-graph.c b/commit-graph.c
index fd2f700b2e..12ec31902e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -939,14 +939,18 @@ int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,
struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id)
{
+ static int object_paranoia = -1;
struct commit *commit;
uint32_t pos;
+ if (object_paranoia == -1)
+ object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
+
if (!prepare_commit_graph(repo))
return NULL;
if (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos))
return NULL;
- if (!has_object(repo, id, 0))
+ if (object_paranoia && !has_object(repo, id, 0))
return NULL;
commit = lookup_commit(repo, id);
diff --git a/commit-graph.h b/commit-graph.h
index 20ada7e891..bd4289620c 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -8,6 +8,12 @@
#define GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE "GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE"
#define GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS "GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS"
+/*
+ * This environment variable controls whether commits looked up via the
+ * commit graph will be double checked to exist in the object database.
+ */
+#define GIT_COMMIT_GRAPH_PARANOIA "GIT_COMMIT_GRAPH_PARANOIA"
+
/*
* This method is only used to enhance coverage of the commit-graph
* feature in the test suite with the GIT_TEST_COMMIT_GRAPH and
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index ba65f17dd9..c0cc454538 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -821,4 +821,25 @@ test_expect_success 'overflow during generation version upgrade' '
)
'
+test_expect_success 'stale commit cannot be parsed when given directly' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit A &&
+ test_commit B &&
+ git commit-graph write --reachable &&
+
+ oid=$(git rev-parse B) &&
+ rm .git/objects/"$(test_oid_to_path "$oid")" &&
+
+ # 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 &&
+ # ... 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_done
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 0/2] commit-graph: detect commits missing in ODB
From: Patrick Steinhardt @ 2023-10-23 11:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Junio C Hamano, Taylor Blau, Jeff King
[-- Attachment #1: Type: text/plain, Size: 6482 bytes --]
Hi,
this is version 2 of my patch series to more readily detect commits
parsed from the commit graph which are missing in the object database.
I've split this out into a separate thread as version 1 was sent in
reply to a different series to extend git-rev-list(1)'s `--missing`
option so that I don't continue to hijack this thread.
Changes compared to v1:
- I've added a preparatory patch that introduced a new
GIT_COMMIT_GRAPH_PARANOIA environment variable as suggested by
Peff. This envvar is retrofitted to the preexisting check in
`lookup_commit_in_graph()` so that the behaviour for this sanity
check is consistent.
- `repo_parse_commit_internal()` now also honors this new envvar.
- I've amended the commit message in v2 to include the benchmark
that demonstrates the performance regression.
- We now un-parse the commit when parsing it via the commit graph
succeeded, but it doesn't exist in the ODB.
Thanks for all the feedback and discussion around this.
Patrick
[1]: <b0bf576c51a706367a758b8e30eca37edb9c2734.1697200576.git.ps@pks.im>
Patrick Steinhardt (2):
commit-graph: introduce envvar to disable commit existence checks
commit: detect commits that exist in commit-graph but not in the ODB
Documentation/git.txt | 9 ++++++++
commit-graph.c | 6 +++++-
commit-graph.h | 6 ++++++
commit.c | 16 +++++++++++++-
t/t5318-commit-graph.sh | 48 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 83 insertions(+), 2 deletions(-)
Range-diff against v1:
-: ---------- > 1: a89c435528 commit-graph: introduce envvar to disable commit existence checks
1: 6ec1e340f8 ! 2: 0476d48555 commit: detect commits that exist in commit-graph but not in the ODB
@@ Commit message
behaviour by checking for object existence via the object database, as
well.
+ This check of course comes with a performance penalty. The following
+ benchmarks have been executed in a clone of linux.git with stable tags
+ added:
+
+ Benchmark 1: git -c core.commitGraph=true rev-list --topo-order --all (git = master)
+ Time (mean ± σ): 2.913 s ± 0.018 s [User: 2.363 s, System: 0.548 s]
+ Range (min … max): 2.894 s … 2.950 s 10 runs
+
+ Benchmark 2: git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
+ Time (mean ± σ): 3.834 s ± 0.052 s [User: 3.276 s, System: 0.556 s]
+ Range (min … max): 3.780 s … 3.961 s 10 runs
+
+ Benchmark 3: git -c core.commitGraph=false rev-list --topo-order --all (git = master)
+ Time (mean ± σ): 13.841 s ± 0.084 s [User: 13.152 s, System: 0.687 s]
+ Range (min … max): 13.714 s … 13.995 s 10 runs
+
+ Benchmark 4: git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
+ Time (mean ± σ): 13.762 s ± 0.116 s [User: 13.094 s, System: 0.667 s]
+ Range (min … max): 13.645 s … 14.038 s 10 runs
+
+ Summary
+ git -c core.commitGraph=true rev-list --topo-order --all (git = master) ran
+ 1.32 ± 0.02 times faster than git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
+ 4.72 ± 0.05 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
+ 4.75 ± 0.04 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = master)
+
+ We look at a ~30% regression in general, but in general we're still a
+ whole lot faster than without the commit graph. To counteract this, the
+ new check can be turned off with the `GIT_COMMIT_GRAPH_PARANOIA` envvar.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## commit.c ##
+@@
+ #include "shallow.h"
+ #include "tree.h"
+ #include "hook.h"
++#include "parse.h"
+
+ static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
+
@@ commit.c: int repo_parse_commit_internal(struct repository *r,
return -1;
if (item->object.parsed)
return 0;
- if (use_commit_graph && parse_commit_in_graph(r, item))
+ if (use_commit_graph && parse_commit_in_graph(r, item)) {
-+ if (!has_object(r, &item->object.oid, 0))
++ static int object_paranoia = -1;
++
++ if (object_paranoia == -1)
++ object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
++
++ if (object_paranoia && !has_object(r, &item->object.oid, 0)) {
++ unparse_commit(r, &item->object.oid);
+ return quiet_on_missing ? -1 :
+ error(_("commit %s exists in commit-graph but not in the object database"),
+ oid_to_hex(&item->object.oid));
++ }
++
return 0;
+ }
@@ commit.c: int repo_parse_commit_internal(struct repository *r,
return quiet_on_missing ? -1 :
## t/t5318-commit-graph.sh ##
-@@ t/t5318-commit-graph.sh: test_expect_success 'overflow during generation version upgrade' '
+@@ t/t5318-commit-graph.sh: test_expect_success 'stale commit cannot be parsed when given directly' '
)
'
-+test_expect_success 'commit exists in commit-graph but not in object database' '
++test_expect_success 'stale commit cannot be parsed when traversing graph' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
@@ t/t5318-commit-graph.sh: test_expect_success 'overflow during generation version
+ oid=$(git rev-parse B) &&
+ rm .git/objects/"$(test_oid_to_path "$oid")" &&
+
++ # 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 &&
++ # ... but fail when we are paranoid.
+ test_must_fail 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
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-23 10:52 UTC (permalink / raw)
To: Dragan Simic; +Cc: Jacob Stopak, git
In-Reply-To: <58a6a25a7b2eb82c21d9b87143033cef@manjaro.org>
On Sun, Oct 22, 2023 at 02:55:05PM +0200, Dragan Simic wrote:
>Oh, that's awesome and I'm really happy to be wrong with my broad
>classification of VCS users. However, I still need to be convinced
>further, and I'd assign your example as an exception to the rules,
>
i don't see myself as exceptional at all in that regard.
in fact, your second user group seems like unicorns, and the first like
a disparaging attitude from an elitist. in reality, users lie on a
spectrum of willingness to engage with the details of the tools they
use, and that willingness is circumstantial. a tool that is forthcoming
with information has a higher chance of being actively engaged.
> especially because you migrated to git from another VCS,
>
that isn't all that uncommon, esp. in the older cohorts. and unless git
achieves a total monopoly, it will remain a regular occurence.
but i don't see how that advances your argument anyway.
> which you liked,
>
i didn't say that.
> and because you use the command line a lot.
>
in my experience, this isn't uncommon for users of "discrete" vcs'es at
all, even if they aren't too interested in the details. they just copy
"magic incantations" from stackoverflow, etc. - disgusting, right? ;-)
>After using git for a while, I can firmly say that git is awesome, but
>that it also is a total overkill for many projects that need a VCS, for
>which choosing Subversion would be a much batter choice. Why, you'll
>ask? Because Subversion is many times simpler, and because many
>projects actually don't need a distributed VCS.
>
that line of reasoning seems mostly bogus to me. every project can
benefit from using a dvcs - reviewing and polishing the history prior to
publication leads to higher quality (primarily of the history, but such
polishing often also transpires to the code itself), so encouraging it
is a useful default (of course interested users can just use git-svn,
but that's a bigger step than just having a closer look at what was
shoved in their faces).
git is indeed pretty much by definition many times harder than svn,
simply because it splits the process of submission into three stages.
however, this is not a _useful_ definition, as everyone can remember to
use `git commit -a && git push` instead of `svn commit`. the real
complexity comes from all the interesting things one can do because of
the split stages, but that's optional (well, until you need to pull and
you get a merge conflict - unlike with svn, git leaves the repo in a
"weird" state, and the poor communication of that was in fact the major
source of frustration for me when i started).
>I also ask myself why would I use git-gui or any other GUI utility? To
>me, clicking on something that represents a file is often simply wrong.
>
that makes you an outlier. most people find point-and-click interaction
rather intuitive and significantly more efficient than encoding their
intent into character sequences.
also, i said "add -p". selecting hunks (and even single lines) plays in
a wholly different league than whole files.
>Though, I understand that many people prefer GUI utilities and I
>respect that, everyone is free to do anything, but I also expect others
>to respect my own preferences.
>
where did anyone even suggest disrespecting your preferences?
you should however consider whether your preferences are a good default
for the wider audience, even within the context of the command line.
i for one think that it would be a perfectly valid experiment to go
all-in and beyond with jacob's proposal - _and make it the default_
(when the output is a tty). more advanced users who feel annoyed would
be expected to opt out of it via configuration, as they are for the
advice messages. because it's really the same idea, only thought bigger.
regards
^ permalink raw reply
* malformed commit messages for git version 2.42.0 when "commit.verbose true"
From: matthieu @ 2023-10-23 10:36 UTC (permalink / raw)
To: git
Hello,
I recently set the option `commit.verbose` to `true` for git version 2.42.0, and I really like it.
However, most of the time, commits are malformed if I don’t remove the diff after the marker
`# ------------------------ >8 ------------------------`
I would expect that anything after the marker is automatically removed by git, or could it be that some pre-commit script see the raw commit message before it is cleaned-up by git itself?
Best,
Matthieu
^ permalink raw reply
* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-23 10:15 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Karthik Nayak, Taylor Blau
In-Reply-To: <20231020100024.GA2194074@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]
On Fri, Oct 20, 2023 at 06:00:24AM -0400, Jeff King wrote:
> On Thu, Oct 19, 2023 at 10:16:56AM -0700, Junio C Hamano wrote:
>
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > There's another way to handle this, which is to conditionally enable the
> > > object existence check. This would be less of a performance hit compared
> > > to disabling commit graphs altogether with `--missing`, but still ensure
> > > that repository corruption was detected. Second, it would not regress
> > > performance for all preexisting users of `repo_parse_commit_gently()`.
> >
> > The above was what I meant to suggest when you demonstrated that the
> > code with additional check is still much more performant than
> > running without the commit-graph optimization, yet has observable
> > impact on performance for normal codepaths that do not need the
> > extra carefulness.
> >
> > But I wasn't sure if it is easy to plumb the "do we want to double
> > check? in other words, are we running something like --missing that
> > care the correctness a bit more than usual cases?" bit down from the
> > caller, because this check is so deep in the callchain.
>
> I wonder if we would want a "be extra careful" flag that is read from
> the environment? That is largely how GIT_REF_PARANOIA works, and then
> particular operations set it (though actually it is the default these
> days, so they no longer do so explicitly).
>
> I guess that is really like a global variable but even more gross. ;)
> But it is nice that it can cross process boundaries, because "how
> careful do we want to be" may be in the eye of the caller, especially
> for plumbing commands. E.g., even without --missing, you may want
> "rev-list" to be extra careful about checking the existence of commits.
> The caller in check_connected() could arguably turn that on by default
> (the way we used to turn on GIT_REF_PARANOIA for pruning repacks before
> it became the default).
Good point indeed, I also started to ask myself whether we'd want to
have a config option. An environment variable `GIT_OBJECT_PARANOIA`
would work equally well though and be less "official".
What I like about this idea is that it would also allow us to easily
unify the behaviour of `lookup_commit_in_graph()` and the new sanity
check in `repo_parse_commit_gently()` that I'm introducing here. I'd
personally think that the default for any such toggle should be `true`,
also to keep the current behaviour of `lookup_commit_in_graph()`. But
changing it would be easy enough.
I'll send a v2 of this series with such a toggle.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] fetch: no redundant error message for atomic fetch
From: Patrick Steinhardt @ 2023-10-23 10:07 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <CANYiYbEJ_mHdsPM3-huDPFktSWFhrpoz7Cvf000JSfZM2cco9w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]
On Mon, Oct 23, 2023 at 05:16:20PM +0800, Jiang Xin wrote:
> On Mon, Oct 23, 2023 at 4:27 PM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Thu, Oct 19, 2023 at 10:34:33PM +0800, Jiang Xin wrote:
> > > @@ -1775,10 +1775,8 @@ static int do_fetch(struct transport *transport,
> > > }
> > >
> > > cleanup:
> > > - if (retcode && transaction) {
> > > - ref_transaction_abort(transaction, &err);
> > > + if (retcode && transaction && ref_transaction_abort(transaction, &err))
> > > error("%s", err.buf);
> > > - }
> >
> > Right. We already call `error()` in all cases where `err` was populated
> > before we `goto cleanup;`, so calling it unconditionally a second time
> > here is wrong.
> >
> > That being said, `ref_transaction_abort()` will end up calling the
> > respective backend's implementation of `transaction_abort`, and for the
> > files backend it actually ignores `err` completely. So if the abort
> > fails, we would still end up calling `error()` with an empty string.
>
> The transaction_abort implementations of the two builtin refs backends
> will not use "err“ because they never fail (always return 0). Some one
> may want to implement their own refs backend which may use the "err"
> variable in their "transaction_abort". So follow the pattern as
> update-ref.c and files-backend.c to call ref_transaction_abort() is
> safe.
>
> > Furthermore, it can happen that `transaction_commit` fails, writes to
> > the buffer and then prints the error. If the abort now fails as well, we
> > would end up printing the error message twice.
>
> The abort never fails so error message from transaction_commit() will
> not reach the code.
With that reasoning we could get rid of the error handling of abort
completely as it's known not to fail. But only because it does not fail
right now doesn't mean that it won't in the future, as the infra for it
to fail is all in place. And in case it ever does the current code will
run into the bug I described.
So in my opinion, we should either refactor the code to clarify that
this cannot fail indeed. Or do the right thing and handle the error case
correctly, which right now we don't.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v1 4/4] maintenance: use XDG config if it exists
From: Patrick Steinhardt @ 2023-10-23 9:58 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, stolee
In-Reply-To: <1e2376a4b998b5b182cc5f72afc7282134bcdf2c.1697660181.git.code@khaugsbakk.name>
[-- Attachment #1: Type: text/plain, Size: 4194 bytes --]
On Wed, Oct 18, 2023 at 10:28:41PM +0200, Kristoffer Haugsbakk wrote:
>
> `git maintenance register` registers the repository in the user's global
> config. `$XDG_CONFIG_HOME/git/config` is supposed to be used if
> `~/.gitconfig` does not exist. However, this command creates a
> `~/.gitconfig` file and writes to that one even though the XDG variant
> exists.
>
> This used to work correctly until 50a044f1e4 (gc: replace config
> subprocesses with API calls, 2022-09-27), when the command started calling
> the config API instead of git-config(1).
>
> Also change `unregister` accordingly.
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
> builtin/gc.c | 23 +++++------------------
> t/t7900-maintenance.sh | 21 +++++++++++++++++++++
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 17fc031f63a..7b780f2ab38 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1526,19 +1526,12 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
>
> if (!found) {
> int rc;
> - char *user_config = NULL, *xdg_config = NULL;
>
> - if (!config_file) {
> - git_global_config_paths(&user_config, &xdg_config);
> - config_file = user_config;
> - if (!user_config)
> - die(_("$HOME not set"));
> - }
> + if (!config_file)
> + config_file = git_global_config();
> rc = git_config_set_multivar_in_file_gently(
> config_file, "maintenance.repo", maintpath,
> CONFIG_REGEX_NONE, 0);
> - free(user_config);
> - free(xdg_config);
Don't we have to free `config_file` now?
> if (rc)
> die(_("unable to add '%s' value of '%s'"),
> @@ -1595,18 +1588,12 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
>
> if (found) {
> int rc;
> - char *user_config = NULL, *xdg_config = NULL;
> - if (!config_file) {
> - git_global_config_paths(&user_config, &xdg_config);
> - config_file = user_config;
> - if (!user_config)
> - die(_("$HOME not set"));
> - }
> +
> + if (!config_file)
> + config_file = git_global_config();
> rc = git_config_set_multivar_in_file_gently(
> config_file, key, NULL, maintpath,
> CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE);
> - free(user_config);
> - free(xdg_config);
Same here.
> if (rc &&
> (!force || rc == CONFIG_NOTHING_SET))
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 487e326b3fa..a11e6c61520 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -67,6 +67,27 @@ test_expect_success 'maintenance.auto config option' '
> test_subcommand ! git maintenance run --auto --quiet <false
> '
>
> +test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
> + XDG_CONFIG_HOME=.config &&
> + test_when_finished rm -r "$XDG_CONFIG_HOME"/git/config &&
> + export "XDG_CONFIG_HOME" &&
Style: there is no need to quote here.
Also, I think we need to unset this variable at the end of this test as
tests don't run in a subshell. In theory, we should also be able to
leave this variable unset completely as it will be derived from HOME
automatically anyway.
> + mkdir -p "$XDG_CONFIG_HOME"/git &&
> + touch "$XDG_CONFIG_HOME"/git/config &&
> + git maintenance register &&
> + git config --file="$XDG_CONFIG_HOME"/git/config --get maintenance.repo >actual &&
> + pwd >expect &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'register does not need XDG_CONFIG_HOME config to exist' '
> + test_when_finished git maintenance unregister &&
> + test_path_is_missing "$XDG_CONFIG_HOME"/git/config &&
> + git maintenance register &&
> + git config --global --get maintenance.repo >actual &&
> + pwd >expect &&
> + test_cmp expect actual
> +'
> +
As you also change the code of `git maintenance unregister`, should we
test its behaviour, as well?
Patrick
> test_expect_success 'maintenance.<task>.enabled' '
> git config maintenance.gc.enabled false &&
> git config maintenance.commit-graph.enabled true &&
> --
> 2.42.0.2.g879ad04204
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v1 3/4] config: factor out global config file retrieval
From: Patrick Steinhardt @ 2023-10-23 9:58 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, stolee
In-Reply-To: <147c767443c35b3b4a5516bf40557f41bb201078.1697660181.git.code@khaugsbakk.name>
[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]
On Wed, Oct 18, 2023 at 10:28:40PM +0200, Kristoffer Haugsbakk wrote:
>
> Factor out code that retrieves the global config file so that we can use
> it in `gc.c` as well.
>
> Use the old name from the previous commit since this function acts
> functionally the same as `git_system_config` but for “global”.
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
> builtin/config.c | 25 ++-----------------------
> config.c | 24 ++++++++++++++++++++++++
> config.h | 1 +
> 3 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index 6fff2655816..df06b766fad 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -708,30 +708,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> }
>
> if (use_global_config) {
> - char *user_config, *xdg_config;
> -
> - git_global_config_paths(&user_config, &xdg_config);
> - if (!user_config)
> - /*
> - * It is unknown if HOME/.gitconfig exists, so
> - * we do not know if we should write to XDG
> - * location; error out even if XDG_CONFIG_HOME
> - * is set and points at a sane location.
> - */
> - die(_("$HOME not set"));
> -
> + given_config_source.file = git_global_config();
> given_config_source.scope = CONFIG_SCOPE_GLOBAL;
> -
> - if (access_or_warn(user_config, R_OK, 0) &&
> - xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
> - given_config_source.file = xdg_config;
> - free(user_config);
> - } else {
> - given_config_source.file = user_config;
> - free(xdg_config);
> - }
> - }
> - else if (use_system_config) {
> + } else if (use_system_config) {
> given_config_source.file = git_system_config();
> given_config_source.scope = CONFIG_SCOPE_SYSTEM;
> } else if (use_local_config) {
> diff --git a/config.c b/config.c
> index d2cdda96edd..2ff766c56ff 100644
> --- a/config.c
> +++ b/config.c
> @@ -2111,6 +2111,30 @@ char *git_system_config(void)
> return system_config;
> }
>
> +char *git_global_config(void)
> +{
> + char *user_config, *xdg_config;
> +
> + git_global_config_paths(&user_config, &xdg_config);
> + if (!user_config)
> + /*
> + * It is unknown if HOME/.gitconfig exists, so
> + * we do not know if we should write to XDG
Nit: we don't know about the intent of the caller, so they may not want
to write to the file but only read it.
> + * location; error out even if XDG_CONFIG_HOME
> + * is set and points at a sane location.
> + */
> + die(_("$HOME not set"));
Is it sensible to `die()` here in this new function that behaves more
like a library function? I imagine it would be more sensible to indicate
the error to the user and let them handle it accordingly.
Patrick
> +
> + if (access_or_warn(user_config, R_OK, 0) && xdg_config &&
> + !access_or_warn(xdg_config, R_OK, 0)) {
> + free(user_config);
> + return xdg_config;
> + } else {
> + free(xdg_config);
> + return user_config;
> + }
> +}
> +
> void git_global_config_paths(char **user_out, char **xdg_out)
> {
> char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
> diff --git a/config.h b/config.h
> index 9f04de8ee3e..5cf961b548d 100644
> --- a/config.h
> +++ b/config.h
> @@ -394,6 +394,7 @@ int config_error_nonbool(const char *);
> #endif
>
> char *git_system_config(void);
> +char *git_global_config(void);
> void git_global_config_paths(char **user, char **xdg);
>
> int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
> --
> 2.42.0.2.g879ad04204
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/7] merge-ort: implement support for packing objects together
From: Patrick Steinhardt @ 2023-10-23 9:19 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano
In-Reply-To: <cover.1697736516.git.me@ttaylorr.com>
[-- Attachment #1: Type: text/plain, Size: 13253 bytes --]
On Thu, Oct 19, 2023 at 01:28:38PM -0400, Taylor Blau wrote:
> (Rebased onto the current tip of 'master', which is 813d9a9188 (The
> nineteenth batch, 2023-10-18) at the time of writing).
>
> This series implements support for a new merge-tree option,
> `--write-pack`, which causes any newly-written objects to be packed
> together instead of being stored individually as loose.
>
> I intentionally broke this off from the existing thread, since I
> accidentally rerolled mine and Jonathan Tan's Bloom v2 series into it,
> causing some confusion.
>
> This is a new round that is significantly simplified thanks to
> another very helpful suggestion[1] from Junio. By factoring out a common
> "deflate object to pack" that takes an abstract bulk_checkin_source as a
> parameter, all of the earlier refactorings can be dropped since we
> retain only a single caller instead of multiple.
>
> This resulted in a rather satisfying range-diff (included below, as
> usual), and a similarly satisfying inter-diff:
>
> $ git diff --stat tb/ort-bulk-checkin.v3..
> bulk-checkin.c | 203 ++++++++++++++++---------------------------------
> 1 file changed, 64 insertions(+), 139 deletions(-)
>
> Beyond that, the changes since last time can be viewed in the range-diff
> below. Thanks in advance for any review!
>
> [1]: https://lore.kernel.org/git/xmqq34y7plj4.fsf@gitster.g/
A single question regarding an `assert()` from my side. Other than that
the series looks good to me, thanks.
Patrick
> Taylor Blau (7):
> bulk-checkin: extract abstract `bulk_checkin_source`
> bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
> bulk-checkin: refactor deflate routine to accept a
> `bulk_checkin_source`
> bulk-checkin: implement `SOURCE_INCORE` mode for `bulk_checkin_source`
> bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
> bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
> builtin/merge-tree.c: implement support for `--write-pack`
>
> Documentation/git-merge-tree.txt | 4 +
> builtin/merge-tree.c | 5 +
> bulk-checkin.c | 161 ++++++++++++++++++++++++++-----
> bulk-checkin.h | 8 ++
> merge-ort.c | 42 ++++++--
> merge-recursive.h | 1 +
> t/t4301-merge-tree-write-tree.sh | 93 ++++++++++++++++++
> 7 files changed, 280 insertions(+), 34 deletions(-)
>
> Range-diff against v3:
> 1: 2dffa45183 < -: ---------- bulk-checkin: factor out `format_object_header_hash()`
> 2: 7a10dc794a < -: ---------- bulk-checkin: factor out `prepare_checkpoint()`
> 3: 20c32d2178 < -: ---------- bulk-checkin: factor out `truncate_checkpoint()`
> 4: 893051d0b7 < -: ---------- bulk-checkin: factor out `finalize_checkpoint()`
> 5: da52ec8380 ! 1: 97bb6e9f59 bulk-checkin: extract abstract `bulk_checkin_source`
> @@ bulk-checkin.c: static int stream_blob_to_pack(struct bulk_checkin_packfile *sta
> if (*already_hashed_to < offset) {
> size_t hsize = offset - *already_hashed_to;
> @@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> - git_hash_ctx ctx;
> + unsigned header_len;
> struct hashfile_checkpoint checkpoint = {0};
> struct pack_idx_entry *idx = NULL;
> + struct bulk_checkin_source source = {
> @@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *st
> seekback = lseek(fd, 0, SEEK_CUR);
> if (seekback == (off_t) -1)
> @@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> - while (1) {
> - prepare_checkpoint(state, &checkpoint, idx, flags);
> + crc32_begin(state->f);
> + }
> if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
> - fd, size, path, flags))
> + &source, flags))
> break;
> - truncate_checkpoint(state, &checkpoint, idx);
> + /*
> + * Writing this object to the current pack will make
> +@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> + hashfile_truncate(state->f, &checkpoint);
> + state->offset = checkpoint.offset;
> + flush_bulk_checkin_packfile(state);
> - if (lseek(fd, seekback, SEEK_SET) == (off_t) -1)
> + if (bulk_checkin_source_seek_to(&source, seekback) == (off_t)-1)
> return error("cannot seek back");
> }
> - finalize_checkpoint(state, &ctx, &checkpoint, idx, result_oid);
> + the_hash_algo->final_oid_fn(result_oid, &ctx);
> 7: 04ec74e357 ! 2: 9d633df339 bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
> @@ bulk-checkin.c: static int stream_blob_to_pack(struct bulk_checkin_packfile *sta
> s.avail_out = sizeof(obuf) - hdrlen;
>
> @@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> -
> - while (1) {
> - prepare_checkpoint(state, &checkpoint, idx, flags);
> + idx->offset = state->offset;
> + crc32_begin(state->f);
> + }
> - if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
> - &source, flags))
> + if (!stream_obj_to_pack(state, &ctx, &already_hashed_to,
> + &source, OBJ_BLOB, flags))
> break;
> - truncate_checkpoint(state, &checkpoint, idx);
> - if (bulk_checkin_source_seek_to(&source, seekback) == (off_t)-1)
> + /*
> + * Writing this object to the current pack will make
> -: ---------- > 3: d5bbd7810e bulk-checkin: refactor deflate routine to accept a `bulk_checkin_source`
> 6: 4e9bac5bc1 = 4: e427fe6ad3 bulk-checkin: implement `SOURCE_INCORE` mode for `bulk_checkin_source`
> 8: 8667b76365 ! 5: 48095afe80 bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
> @@ Commit message
> objects individually as loose.
>
> Similar to the existing `index_blob_bulk_checkin()` function, the
> - entrypoint delegates to `deflate_blob_to_pack_incore()`, which is
> - responsible for formatting the pack header and then deflating the
> - contents into the pack. The latter is accomplished by calling
> - deflate_obj_contents_to_pack_incore(), which takes advantage of the
> - earlier refactorings and is responsible for writing the object to the
> - pack and handling any overage from pack.packSizeLimit.
> -
> - The bulk of the new functionality is implemented in the function
> - `stream_obj_to_pack()`, which can handle streaming objects from memory
> - to the bulk-checkin pack as a result of the earlier refactoring.
> + entrypoint delegates to `deflate_obj_to_pack_incore()`. That function in
> + turn delegates to deflate_obj_to_pack(), which is responsible for
> + formatting the pack header and then deflating the contents into the
> + pack.
>
> Consistent with the rest of the bulk-checkin mechanism, there are no
> direct tests here. In future commits when we expose this new
> @@ Commit message
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
>
> ## bulk-checkin.c ##
> -@@ bulk-checkin.c: static void finalize_checkpoint(struct bulk_checkin_packfile *state,
> - }
> +@@ bulk-checkin.c: static int deflate_obj_to_pack(struct bulk_checkin_packfile *state,
> + return 0;
> }
>
> -+static int deflate_obj_contents_to_pack_incore(struct bulk_checkin_packfile *state,
> -+ git_hash_ctx *ctx,
> -+ struct hashfile_checkpoint *checkpoint,
> -+ struct object_id *result_oid,
> -+ const void *buf, size_t size,
> -+ enum object_type type,
> -+ const char *path, unsigned flags)
> ++static int deflate_obj_to_pack_incore(struct bulk_checkin_packfile *state,
> ++ struct object_id *result_oid,
> ++ const void *buf, size_t size,
> ++ const char *path, enum object_type type,
> ++ unsigned flags)
> +{
> -+ struct pack_idx_entry *idx = NULL;
> -+ off_t already_hashed_to = 0;
> + struct bulk_checkin_source source = {
> + .type = SOURCE_INCORE,
> + .buf = buf,
> @@ bulk-checkin.c: static void finalize_checkpoint(struct bulk_checkin_packfile *st
> + .path = path,
> + };
> +
> -+ /* Note: idx is non-NULL when we are writing */
> -+ if (flags & HASH_WRITE_OBJECT)
> -+ CALLOC_ARRAY(idx, 1);
> -+
> -+ while (1) {
> -+ prepare_checkpoint(state, checkpoint, idx, flags);
> -+
> -+ if (!stream_obj_to_pack(state, ctx, &already_hashed_to, &source,
> -+ type, flags))
> -+ break;
> -+ truncate_checkpoint(state, checkpoint, idx);
> -+ bulk_checkin_source_seek_to(&source, 0);
> -+ }
> -+
> -+ finalize_checkpoint(state, ctx, checkpoint, idx, result_oid);
> -+
> -+ return 0;
> -+}
> -+
> -+static int deflate_blob_to_pack_incore(struct bulk_checkin_packfile *state,
> -+ struct object_id *result_oid,
> -+ const void *buf, size_t size,
> -+ const char *path, unsigned flags)
> -+{
> -+ git_hash_ctx ctx;
> -+ struct hashfile_checkpoint checkpoint = {0};
> -+
> -+ format_object_header_hash(the_hash_algo, &ctx, &checkpoint, OBJ_BLOB,
> -+ size);
> -+
> -+ return deflate_obj_contents_to_pack_incore(state, &ctx, &checkpoint,
> -+ result_oid, buf, size,
> -+ OBJ_BLOB, path, flags);
> ++ return deflate_obj_to_pack(state, result_oid, &source, type, 0, flags);
> +}
> +
> static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> @@ bulk-checkin.c: int index_blob_bulk_checkin(struct object_id *oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags)
> +{
> -+ int status = deflate_blob_to_pack_incore(&bulk_checkin_packfile, oid,
> -+ buf, size, path, flags);
> ++ int status = deflate_obj_to_pack_incore(&bulk_checkin_packfile, oid,
> ++ buf, size, path, OBJ_BLOB,
> ++ flags);
> + if (!odb_transaction_nesting)
> + flush_bulk_checkin_packfile(&bulk_checkin_packfile);
> + return status;
> 9: cba043ef14 ! 6: 60568f9281 bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
> @@ Commit message
> machinery will have enough to keep track of the converted object's hash
> in order to update the compatibility mapping.
>
> - Within `deflate_tree_to_pack_incore()`, the changes should be limited
> - to something like:
> + Within some thin wrapper around `deflate_obj_to_pack_incore()` (perhaps
> + `deflate_tree_to_pack_incore()`), the changes should be limited to
> + something like:
>
> struct strbuf converted = STRBUF_INIT;
> if (the_repository->compat_hash_algo) {
> @@ Commit message
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
>
> ## bulk-checkin.c ##
> -@@ bulk-checkin.c: static int deflate_blob_to_pack_incore(struct bulk_checkin_packfile *state,
> - OBJ_BLOB, path, flags);
> - }
> -
> -+static int deflate_tree_to_pack_incore(struct bulk_checkin_packfile *state,
> -+ struct object_id *result_oid,
> -+ const void *buf, size_t size,
> -+ const char *path, unsigned flags)
> -+{
> -+ git_hash_ctx ctx;
> -+ struct hashfile_checkpoint checkpoint = {0};
> -+
> -+ format_object_header_hash(the_hash_algo, &ctx, &checkpoint, OBJ_TREE,
> -+ size);
> -+
> -+ return deflate_obj_contents_to_pack_incore(state, &ctx, &checkpoint,
> -+ result_oid, buf, size,
> -+ OBJ_TREE, path, flags);
> -+}
> -+
> - static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
> - struct object_id *result_oid,
> - int fd, size_t size,
> @@ bulk-checkin.c: int index_blob_bulk_checkin_incore(struct object_id *oid,
> return status;
> }
> @@ bulk-checkin.c: int index_blob_bulk_checkin_incore(struct object_id *oid,
> + const void *buf, size_t size,
> + const char *path, unsigned flags)
> +{
> -+ int status = deflate_tree_to_pack_incore(&bulk_checkin_packfile, oid,
> -+ buf, size, path, flags);
> ++ int status = deflate_obj_to_pack_incore(&bulk_checkin_packfile, oid,
> ++ buf, size, path, OBJ_TREE,
> ++ flags);
> + if (!odb_transaction_nesting)
> + flush_bulk_checkin_packfile(&bulk_checkin_packfile);
> + return status;
> 10: ae70508037 = 7: b9be9df122 builtin/merge-tree.c: implement support for `--write-pack`
> --
> 2.42.0.405.g86fe3250c2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 4/7] bulk-checkin: implement `SOURCE_INCORE` mode for `bulk_checkin_source`
From: Patrick Steinhardt @ 2023-10-23 9:19 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano
In-Reply-To: <e427fe6ad383cc238c13f313dc9f11eab37a3840.1697736516.git.me@ttaylorr.com>
[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]
On Thu, Oct 19, 2023 at 01:28:51PM -0400, Taylor Blau wrote:
> Continue to prepare for streaming an object's contents directly from
> memory by teaching `bulk_checkin_source` how to perform reads and seeks
> based on an address in memory.
>
> Unlike file descriptors, which manage their own offset internally, we
> have to keep track of how many bytes we've read out of the buffer, and
> make sure we don't read past the end of the buffer.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> bulk-checkin.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index 28bc8d5ab4..60361b3e2e 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -141,11 +141,15 @@ static int already_written(struct bulk_checkin_packfile *state, struct object_id
> }
>
> struct bulk_checkin_source {
> - enum { SOURCE_FILE } type;
> + enum { SOURCE_FILE, SOURCE_INCORE } type;
>
> /* SOURCE_FILE fields */
> int fd;
>
> + /* SOURCE_INCORE fields */
> + const void *buf;
> + size_t read;
> +
> /* common fields */
> size_t size;
> const char *path;
> @@ -157,6 +161,11 @@ static off_t bulk_checkin_source_seek_to(struct bulk_checkin_source *source,
> switch (source->type) {
> case SOURCE_FILE:
> return lseek(source->fd, offset, SEEK_SET);
> + case SOURCE_INCORE:
> + if (!(0 <= offset && offset < source->size))
> + return (off_t)-1;
> + source->read = offset;
> + return source->read;
> default:
> BUG("unknown bulk-checkin source: %d", source->type);
> }
> @@ -168,6 +177,13 @@ static ssize_t bulk_checkin_source_read(struct bulk_checkin_source *source,
> switch (source->type) {
> case SOURCE_FILE:
> return read_in_full(source->fd, buf, nr);
> + case SOURCE_INCORE:
> + assert(source->read <= source->size);
Is there any guideline around when to use `assert()` vs `BUG()`? I think
that this assertion here is quite critical, because when it does not
hold we can end up performing out-of-bounds reads and writes. But as
asserts are typically missing in non-debug builds, this safeguard would
not do anything for our end users, right?
Patrick
> + if (nr > source->size - source->read)
> + nr = source->size - source->read;
> + memcpy(buf, (unsigned char *)source->buf + source->read, nr);
> + source->read += nr;
> + return nr;
> default:
> BUG("unknown bulk-checkin source: %d", source->type);
> }
> --
> 2.42.0.405.g86fe3250c2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] fetch: no redundant error message for atomic fetch
From: Jiang Xin @ 2023-10-23 9:16 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <ZTYue-3gAS1aGXNa@tanuki>
On Mon, Oct 23, 2023 at 4:27 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Oct 19, 2023 at 10:34:33PM +0800, Jiang Xin wrote:
> > @@ -1775,10 +1775,8 @@ static int do_fetch(struct transport *transport,
> > }
> >
> > cleanup:
> > - if (retcode && transaction) {
> > - ref_transaction_abort(transaction, &err);
> > + if (retcode && transaction && ref_transaction_abort(transaction, &err))
> > error("%s", err.buf);
> > - }
>
> Right. We already call `error()` in all cases where `err` was populated
> before we `goto cleanup;`, so calling it unconditionally a second time
> here is wrong.
>
> That being said, `ref_transaction_abort()` will end up calling the
> respective backend's implementation of `transaction_abort`, and for the
> files backend it actually ignores `err` completely. So if the abort
> fails, we would still end up calling `error()` with an empty string.
The transaction_abort implementations of the two builtin refs backends
will not use "err“ because they never fail (always return 0). Some one
may want to implement their own refs backend which may use the "err"
variable in their "transaction_abort". So follow the pattern as
update-ref.c and files-backend.c to call ref_transaction_abort() is
safe.
> Furthermore, it can happen that `transaction_commit` fails, writes to
> the buffer and then prints the error. If the abort now fails as well, we
> would end up printing the error message twice.
The abort never fails so error message from transaction_commit() will
not reach the code.
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH 2/2] fetch: no redundant error message for atomic fetch
From: Patrick Steinhardt @ 2023-10-23 8:27 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]
On Thu, Oct 19, 2023 at 10:34:33PM +0800, Jiang Xin wrote:
> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> If an error occurs during an atomic fetch, a redundant error message
> will appear at the end of do_fetch(). It was introduced in b3a804663c
> (fetch: make `--atomic` flag cover backfilling of tags, 2022-02-17).
>
> Instead of displaying the error message unconditionally, the final error
> output should follow the pattern in update-ref.c and files-backend.c as
> follows:
>
> if (ref_transaction_abort(transaction, &error))
> error("abort: %s", error.buf);
>
> This will fix the test case "fetch porcelain output (atomic)" in t5574.
>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
> builtin/fetch.c | 4 +---
> t/t5574-fetch-output.sh | 2 +-
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index fd134ba74d..01a573cf8d 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1775,10 +1775,8 @@ static int do_fetch(struct transport *transport,
> }
>
> cleanup:
> - if (retcode && transaction) {
> - ref_transaction_abort(transaction, &err);
> + if (retcode && transaction && ref_transaction_abort(transaction, &err))
> error("%s", err.buf);
> - }
Right. We already call `error()` in all cases where `err` was populated
before we `goto cleanup;`, so calling it unconditionally a second time
here is wrong.
That being said, `ref_transaction_abort()` will end up calling the
respective backend's implementation of `transaction_abort`, and for the
files backend it actually ignores `err` completely. So if the abort
fails, we would still end up calling `error()` with an empty string.
Furthermore, it can happen that `transaction_commit` fails, writes to
the buffer and then prints the error. If the abort now fails as well, we
would end up printing the error message twice.
I wonder whether we should fix this by unifying all calls to `error()`
to only happen in the cleanup block, and only iff the buffer length is
non-zero?
Patrick
> display_state_release(&display_state);
> close_fetch_head(&fetch_head);
> diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh
> index 1397101629..3c72fc693f 100755
> --- a/t/t5574-fetch-output.sh
> +++ b/t/t5574-fetch-output.sh
> @@ -97,7 +97,7 @@ do
> opt=
> ;;
> esac
> - test_expect_failure "fetch porcelain output ${opt:+(atomic)}" '
> + test_expect_success "fetch porcelain output ${opt:+(atomic)}" '
> test_when_finished "rm -rf porcelain" &&
>
> # Clone and pre-seed the repositories. We fetch references into two
> --
> 2.42.0.411.g813d9a9188
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
From: Patrick Steinhardt @ 2023-10-23 7:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Sunshine, cousteau via GitGitGadget, git, Javier Mora
In-Reply-To: <xmqqa5sap44i.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]
On Sun, Oct 22, 2023 at 05:35:41PM -0700, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >> The description of the `git bisect run` command syntax at the beginning
> >> of the manpage is `git bisect run <cmd>...`, which isn't quite clear
> >> about what `<cmd>` is or what the `...` mean; one could think that it is
> >> the whole (quoted) command line with all arguments in a single string,
> >> or that it supports multiple commands, or that it doesn't accept
> >> commands with arguments at all.
> >>
> >> Change to `git bisect run <cmd> [<arg>...]` to clarify the syntax.
> >
> > Okay, makes sense.
> >
> >> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> >> ---
> >> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> >> @@ -26,7 +26,7 @@ on the subcommand:
> >> - git bisect run <cmd>...
> >> + git bisect run <cmd> [<arg>...]
> >
> > The output of `git bisect -h` suffers the same problem. Perhaps this
> > patch can fix that, as well?
>
> Good eyes.
>
> Not a new problem and obviously can be left outside of this simple
> update, but I wonder if we should eventually move these into the
> proper SYNOPSIS section. Other multi-modal commands like "git
> checkout", "git rebase", etc. do list different forms all in the
> SYNOPSIS section.
>
> I also thought at least some commands we know the "-h" output and
> SYNOPSIS match, we had tests to ensure they do not drift apart. We
> would probably want to cover more subcommands with t0450.
>
> Thanks.
If we don't want them to drift apart I wonder whether we could instead
generate the synopsis from the output of `-h`? This reduces duplication
at the cost of a more complex build process for our manpages.
Not saying that this is necessarily a good idea, just throwing it out
there.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Sebastian Thiel @ 2023-10-23 7:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren, Josh Triplett, git
In-Reply-To: <xmqqmswjsv8c.fsf@gitster.g>
On 16 Oct 2023, at 8:02, Sebastian Thiel wrote:
> I don't know if this time will be different as I can only offer to implement
> the syntax adjustment, whatever that might be (possibly after validating
> the candidate against a corpus of repositories), along with the update
> to `git clean` so it leaves precious files alone by default and a new flag
> to also remove precious files.
I am happy to announce this feature can now be contributed in full by me once
you give it a go. This would mean that the entirety of `git` would become
aware of precious files over time.
To my mind, and probably out of ignorance, it seems that once the syntax is
decided on it's possible for the implementation to start. From there I could
use Elijah's analysis to know which parts of git to make aware of precious files
in addition to `git clean`.
I am definitely looking forward to hearing from you :).
^ permalink raw reply
* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
From: Junio C Hamano @ 2023-10-23 0:35 UTC (permalink / raw)
To: Eric Sunshine; +Cc: cousteau via GitGitGadget, git, Javier Mora
In-Reply-To: <CAPig+cS4J-L44a-fjQ=2bXxRj6e1qdQK8705K3NPqmTsWXBQsw@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> The description of the `git bisect run` command syntax at the beginning
>> of the manpage is `git bisect run <cmd>...`, which isn't quite clear
>> about what `<cmd>` is or what the `...` mean; one could think that it is
>> the whole (quoted) command line with all arguments in a single string,
>> or that it supports multiple commands, or that it doesn't accept
>> commands with arguments at all.
>>
>> Change to `git bisect run <cmd> [<arg>...]` to clarify the syntax.
>
> Okay, makes sense.
>
>> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
>> ---
>> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
>> @@ -26,7 +26,7 @@ on the subcommand:
>> - git bisect run <cmd>...
>> + git bisect run <cmd> [<arg>...]
>
> The output of `git bisect -h` suffers the same problem. Perhaps this
> patch can fix that, as well?
Good eyes.
Not a new problem and obviously can be left outside of this simple
update, but I wonder if we should eventually move these into the
proper SYNOPSIS section. Other multi-modal commands like "git
checkout", "git rebase", etc. do list different forms all in the
SYNOPSIS section.
I also thought at least some commands we know the "-h" output and
SYNOPSIS match, we had tests to ensure they do not drift apart. We
would probably want to cover more subcommands with t0450.
Thanks.
^ permalink raw reply
* Re: [PATCH 0/7] log: decorate pseudorefs and other refs
From: Junio C Hamano @ 2023-10-23 0:20 UTC (permalink / raw)
To: Andy Koppe; +Cc: git
In-Reply-To: <fe3abed8-6be0-4d77-9057-79c9b7c0795c@gmail.com>
Andy Koppe <andy.koppe@gmail.com> writes:
>> [2/7] is a trivial readability improvement. It obviously should be
>> left outside the scope of this series, but we should notice
>> the same pattern in similar color tables (e.g., wt-status.c
>> has one, diff.c has another) and perform the same clean-up as
>> a #leftoverbits item.
>
> Okay, I've removed that commit in v2. (I should have mentioned in the
> commit message that it was triggered by the inconsistency with the
> immediately following color_decorate_slots array, which uses
> designated initializers.)
Sorry, that is not what I meant. [2/7] as a preliminary clean-up to
work in the same area does make very much sense. What I meant to be
"outside the scope" was to make similar fixes to other color tables
that this series does not care about.
>> .ref = "refs/", the implementation of the search must know
>> that it is a fallback position (i.e. if it found a match with
>> the fallback .ref = "refs/" , unless it looked at all other
>> entries that could begin with "refs/" and are more specific,
>> it needs to keep going).
>
> Fair points. I've rewritten things to not touch the ref_namespace array.
Well, the namespace_info mechanism still may be a good place to have
the necessary information; it may be that the current implementation
detail of how a given ref is classified to one of the namespaces is
too limiting---it essentially allows the string match with the .ref
member. But we can imagine that it could be extended a bit, e.g.
struct ref_namespace_info {
char *ref;
int (*membership)(const char *, const struct ref_namespace_info *);
... other members ...;
};
where the .membership member is used in add_ref_decoration() to
determine the membership of a given "refname" to the namespace "i"
perhaps like so:
struct ref_namespace_info *info = &ref_namespace[i];
if (!info->decoration)
continue;
+ if (info->membership) {
+ if (info->membership(refname, info)) {
+ deco_type = info->decoration;
+ break;
+ }
+ } else if (info->exact) {
- if (info->exact) {
if (!strcmp(refname, info->ref)) {
deco_type = info_decoration;
break;
}
Then you can arrange the pseudoref class to use .membership function
perhaps like this:
static int pseudoref_namespace_membership(
const char *refname, const struct ref_namespace_info *info UNUSED
)
{
return is_pseudoref(refname);
}
and make them all into a single class.
What I called a bad design was to reuse the namespace_info code
without extending it to suit our needs.
This comment will probably affect everything below.
>> [6/7] This is pretty straight-forward, assuming that the existing
>> is_pseudoref_syntax() function does the right thing. I am
>> not sure about that, though. A refname with '-' is allowed
>> to be called a pseudoref???
>> Also, not a fault of this patch, but the "_syntax" in its
>> name is totally unnecessary, I would think. At first glance,
>> I suspected that the excuse to append _syntax may have been
>> to signal the fact that the helper function does not check if
>> there actually is such a ref, but examining a few helpers
>> defined nearby tells us that such an excuse does not make
>> sense:
>
> I've dropped the use of that function from the change, checking
> against the actual pseudoref names instead.
>
>> [7/7] Allowing pseudorefs to optionally used when decorating might
>> be a good idea, but I do not think it is particularly a good
>> design decision to enable it by default.
>
> Okay!
>
>> Each of them forming a separate "namespace" also looks like a
>> poor design, as being able to group multiple things into one
>> family and treat them the same way is the primary point of
>> "namespace", I would think.
>
> Fair enough, although the array already contains HEAD and refs/stash
> as singletons.
But these deserve to be singletons, don't they? There is no other
thing that behaves like HEAD; there is no other thing that behaves
like stash; and they do not behave like each other.
Having said that, I do not think it makes much sense to decorate a
commit off of refs/stash, as the true richeness of the stash is not
in its history but in its reflog, which the decoration code does not
dig into. But obviously it is not a part of the topic we are
discussing (unless, of course, we are not "adding" new decoration
sources and colors, but we are improving the decoration sources and
colors by adding new useful ones while retiring existing useless
ones).
Thanks.
^ permalink raw reply
* What's cooking in git.git (Oct 2023, #08; Sun, 22)
From: Junio C Hamano @ 2023-10-22 22:17 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).
There are too many topics marked with "Needs review" label. I can
review some of them myself, but obviously not all. For now I will
refrain from adding more topics to 'seen' that are not reviewed at
all.
I'll be away from the keyboard for most of the later half of the
week, and the updates to my tree will be slower than usual.
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/
--------------------------------------------------
[Graduated to 'master']
* ak/pretty-decorate-more-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-12 at 3cbb4c2268)
+ pretty: fix ref filtering for %(decorate) formats
Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
not auto-initialize the decoration subsystem, which has been
corrected.
source: <20231008202307.1568477-1-andy.koppe@gmail.com>
* ps/rewritten-is-per-worktree-doc (2023-10-10) 1 commit
(merged to 'next' on 2023-10-12 at 501b960e8c)
+ doc/git-worktree: mention "refs/rewritten" as per-worktree refs
Doc update.
source: <985ac850eb6e60ae76601acc8bfbcd56f99348b4.1696935657.git.ps@pks.im>
* ty/merge-tree-strategy-options (2023-10-11) 2 commits
(merged to 'next' on 2023-10-12 at 9b873601df)
+ merge: introduce {copy|clear}_merge_options()
(merged to 'next' on 2023-09-29 at aa65b54416)
+ merge-tree: add -X strategy option
"git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.
source: <pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com>
* vd/loose-ref-iteration-optimization (2023-10-09) 4 commits
(merged to 'next' on 2023-10-12 at 99e2f83855)
+ files-backend.c: avoid stat in 'loose_fill_ref_dir'
+ dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
+ dir.[ch]: expose 'get_dtype'
+ ref-cache.c: fix prefix matching in ref iteration
The code to iterate over loose references have been optimized to
reduce the number of lstat() system calls.
source: <pull.1594.v2.git.1696888736.gitgitgadget@gmail.com>
--------------------------------------------------
[New Topics]
* jc/am-doc-whitespace-action-fix (2023-10-18) 1 commit
(merged to 'next' on 2023-10-20 at 9200d39c08)
+ am: align placeholder for --whitespace option with apply
Docfix.
Will merge to 'master'.
source: <xmqqwmvjzeqd.fsf@gitster.g>
* da/t7601-style-fix (2023-10-18) 1 commit
(merged to 'next' on 2023-10-20 at 8e7326458c)
+ t7601: use "test_path_is_file" etc. instead of "test -f"
Coding style update.
Will merge to 'master'.
source: <20231018124538.68549-2-anonolitunya@gmail.com>
* 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.
Needs review.
source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
* mm/p4-symlink-with-lfs (2023-10-19) 1 commit
(merged to 'next' on 2023-10-20 at 9c05ce7e85)
+ git-p4 shouldn't attempt to store symlinks in LFS
"git p4" tried to store symlinks to LFS when told, but has been
fixed not to do so, because it does not make sense.
Will merge to 'master'.
source: <20231019002558.867830-1-mmcclain@noprivs.com>
* jk/send-email-fix-addresses-from-composed-messages (2023-10-20) 3 commits
(merged to 'next' on 2023-10-22 at 43221cc3a4)
+ send-email: handle to/cc/bcc from --compose message
+ Revert "send-email: extract email-parsing code into a subroutine"
+ doc/send-email: mention handling of "reply-to" with --compose
The codepath to handle recipient addresses `git send-email
--compose` learns from the user was completely broken, which has
been corrected.
Will merge to 'master'.
source: <20231020100343.GA2194322@coredump.intra.peff.net>
* ms/doc-push-fix (2023-10-20) 1 commit
(merged to 'next' on 2023-10-22 at 7ce3cef56b)
+ git-push doc: more visibility for -q option
Docfix.
Will merge to 'master'.
source: <20231020184627.14336-1-msuchanek@suse.de>
* ob/rebase-cleanup (2023-10-20) 3 commits
(merged to 'next' on 2023-10-22 at 05e14ca4fc)
+ rebase: move parse_opt_keep_empty() down
+ rebase: handle --strategy via imply_merge() as well
+ rebase: simplify code related to imply_merge()
Code clean-up.
Will merge to 'master'.
source: <20231020093654.922890-1-oswald.buddenhagen@gmx.de>
* wx/merge-ort-comment-typofix (2023-10-21) 1 commit
(merged to 'next' on 2023-10-22 at ad1e33883a)
+ merge-ort.c: fix typo 'neeed' to 'needed'
Typofix.
Will merge to 'master'.
source: <pull.1592.v3.git.git.1697942768555.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]
* jc/commit-new-underscore-index-fix (2023-10-17) 1 commit
(merged to 'next' on 2023-10-22 at 0e4787303d)
+ commit: do not use cryptic "new_index" in end-user facing messages
Message fix.
Will merge to 'master'.
source: <xmqqo7gwvd8c.fsf_-_@gitster.g>
* 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.
Needs review.
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/pathspec-error-wo-repository-fix (2023-10-20) 1 commit
(merged to 'next' on 2023-10-22 at 4f77af1e40)
+ grep: die gracefully when outside repository
The pathspec code carelessly dereferenced NULL while emitting an
error message, which has been corrected.
Will merge to 'master'.
source: <5c8ef6bec1c99e0fae7ada903885a8e77f8137f9.1697819838.git.code@khaugsbakk.name>
* 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.
Needs review.
source: <cover.1697319294.git.code@khaugsbakk.name>
* ni/die-message-fix-for-git-add (2023-10-17) 1 commit
(merged to 'next' on 2023-10-22 at f46c5dfd63)
+ builtin/add.c: clean up die() messages
Message updates.
Will merge to 'master'.
source: <20231017113946.747-1-naomi.ibeh69@gmail.com>
* ps/git-repack-doc-fixes (2023-10-16) 2 commits
(merged to 'next' on 2023-10-22 at df64849f26)
+ doc/git-repack: don't mention nonexistent "--unpacked" option
+ doc/git-repack: fix syntax for `-g` shorthand option
Doc updates.
Will merge to 'master'.
source: <cover.1697440686.git.ps@pks.im>
* tb/merge-tree-write-pack (2023-10-19) 7 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: implement `SOURCE_INCORE` mode for `bulk_checkin_source`
- bulk-checkin: refactor deflate routine to accept a `bulk_checkin_source`
- 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.
Will merge to 'next'?
source: <cover.1697736516.git.me@ttaylorr.com>
* tb/format-pack-doc-update (2023-10-12) 2 commits
- Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
- Documentation/gitformat-pack.txt: fix typo
Doc update.
Expecting a reroll.
cf. <xmqq5y3b4id2.fsf@gitster.g>
source: <cover.1697144959.git.me@ttaylorr.com>
* ps/do-not-trust-commit-graph-blindly-for-existence (2023-10-13) 1 commit
- commit: detect commits that exist in commit-graph but not in the ODB
(this branch is used by kn/rev-list-missing-fix.)
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.
Comments?
source: <b0bf576c51a706367a758b8e30eca37edb9c2734.1697200576.git.ps@pks.im>
* tb/pair-chunk-expect-size (2023-10-14) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- midx: read `OIDF` 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 `OIDF` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
(this branch uses jk/chunk-bounds.)
Code clean-up for jk/chunk-bounds topic.
Comments?
source: <45cac29403e63483951f7766c6da3c022c68d9f0.1697225110.git.me@ttaylorr.com>
source: <cover.1697225110.git.me@ttaylorr.com>
* jc/fail-stash-to-store-non-stash (2023-10-11) 1 commit
(merged to 'next' on 2023-10-16 at e26db57315)
+ stash: be careful what we store
Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.
Will merge to 'master'.
source: <xmqqbkd4lwj0.fsf_-_@gitster.g>
* bc/racy-4gb-files (2023-10-13) 2 commits
(merged to 'next' on 2023-10-16 at c60962dfee)
+ Prevent git from rehashing 4GiB files
+ t: add a test helper to truncate files
The index file has room only for 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 multiple
of 4GiB. This is mistaken for a racily clean path. Avoid it by
storing a bogus sd_size value instead for such files.
Will merge to 'master'.
source: <20231012160930.330618-1-sandals@crustytoothpaste.net>
* jc/grep-f-relative-to-cwd (2023-10-12) 1 commit
- grep: -f <path> is relative to $cwd
"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.
Needs review.
source: <xmqqedhzg37z.fsf@gitster.g>
* 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>
* en/docfixes (2023-10-09) 25 commits
(merged to 'next' on 2023-10-17 at 1e3cdeb427)
+ documentation: add missing parenthesis
+ documentation: add missing quotes
+ documentation: add missing fullstops
+ documentation: add some commas where they are helpful
+ documentation: fix whitespace issues
+ documentation: fix capitalization
+ documentation: fix punctuation
+ documentation: use clearer prepositions
+ documentation: add missing hyphens
+ documentation: remove unnecessary hyphens
+ documentation: add missing article
+ documentation: fix choice of article
+ documentation: whitespace is already generally plural
+ documentation: fix singular vs. plural
+ documentation: fix verb vs. noun
+ documentation: fix adjective vs. noun
+ documentation: fix verb tense
+ documentation: employ consistent verb tense for a list
+ documentation: fix subject/verb agreement
+ documentation: remove extraneous words
+ documentation: add missing words
+ documentation: fix apostrophe usage
+ documentation: fix typos
+ documentation: fix small error
+ documentation: wording improvements
Documentation typo and grammo fixes.
Will merge to 'master'.
source: <pull.1595.git.1696747527.gitgitgadget@gmail.com>
* kn/rev-list-missing-fix (2023-10-22) 4 commits
- rev-list: add commit object support in `--missing` option
- rev-list: move `show_commit()` to the bottom
- revision: rename bit to `do_not_die_on_missing_objects`
- Merge branch 'ps/do-not-trust-commit-graph-blindly-for-existence' into kn/rev-list-missing-fix
(this branch uses ps/do-not-trust-commit-graph-blindly-for-existence.)
"git rev-list --missing" did not work for missing commit objects,
which has been corrected.
Comments?
source: <20231019121024.194317-1-karthik.188@gmail.com>
* jk/chunk-bounds (2023-10-14) 21 commits
(merged to 'next' on 2023-10-16 at 68c9e37980)
+ t5319: make corrupted large-offset test more robust
(merged to 'next' on 2023-10-10 at 21139603ce)
+ chunk-format: drop pair_chunk_unsafe()
+ commit-graph: detect out-of-order BIDX offsets
+ commit-graph: check bounds when accessing BIDX chunk
+ commit-graph: check bounds when accessing BDAT chunk
+ commit-graph: bounds-check generation overflow chunk
+ commit-graph: check size of generations chunk
+ commit-graph: bounds-check base graphs chunk
+ commit-graph: detect out-of-bounds extra-edges pointers
+ commit-graph: check size of commit data chunk
+ midx: check size of revindex chunk
+ midx: bounds-check large offset chunk
+ midx: check size of object offset chunk
+ midx: enforce chunk alignment on reading
+ midx: check size of pack names chunk
+ commit-graph: check consistency of fanout table
+ midx: check size of oid lookup chunk
+ commit-graph: check size of oid fanout chunk
+ midx: stop ignoring malformed oid fanout chunk
+ t: add library for munging chunk-format files
+ chunk-format: note that pair_chunk() is unsafe
(this branch is used by tb/pair-chunk-expect-size.)
The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.
Will merge to 'master'.
source: <20231009205544.GA3281950@coredump.intra.peff.net>
* sn/typo-grammo-phraso-fixes (2023-10-05) 5 commits
(merged to 'next' on 2023-10-18 at 575d767f9a)
+ t/README: fix multi-prerequisite example
+ doc/gitk: s/sticked/stuck/
+ git-jump: admit to passing merge mode args to ls-files
+ doc/diff-options: improve wording of the log.diffMerges mention
+ doc: fix some typos, grammar and wording issues
Many typos, ungrammatical sentences and wrong phrasing have been
fixed.
Will merge to 'master'.
source: <20231003082107.3002173-1-stepnem@smrk.net>
* so/diff-merges-dd (2023-10-09) 3 commits
(merged to 'next' on 2023-10-16 at 71b5e29625)
+ completion: complete '--dd'
+ diff-merges: introduce '--dd' option
+ diff-merges: improve --diff-merges documentation
"git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".
Will merge to 'master'.
source: <20231009160535.236523-1-sorganov@gmail.com>
* jc/update-list-references-to-lore (2023-10-06) 1 commit
(merged to 'next' on 2023-10-19 at 83a721a137)
+ doc: update list archive reference to use lore.kernel.org
Doc update.
Will merge to 'master'.
source: <xmqq7cnz741s.fsf@gitster.g>
* cc/git-replay (2023-10-10) 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.
Needs (hopefully final and quick) review.
source: <20231010123847.2777056-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-19) 7 commits
- log: show pseudorefs in decorations
- refs: exempt pseudoref patterns from prefixing
- log: add color.decorate.ref option for other refs
- refs: separate decoration type from default filter
- log: add color.decorate.symbol config option
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231003205442.22963-1-andy.koppe@gmail.com>
* jc/attr-tree-config (2023-10-13) 2 commits
(merged to 'next' on 2023-10-19 at 202dc1c453)
+ attr: add attr.tree for setting the treeish to read attributes from
+ attr: read attributes from HEAD when bare repo
The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.
Will merge to 'master'.
source: <pull.1577.v5.git.git.1697218770.gitgitgadget@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.
Will merge to 'next'?
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>
* rs/parse-options-value-int (2023-09-18) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
Not ready yet.
cf. <4014e490-c6c1-453d-b0ed-645220e3e614@web.de>
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* js/doc-unit-tests (2023-10-16) 5 commits
- fixup! ci: run unit tests in CI
- fixup! unit tests: add TAP unit test framework
- 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.
Expecting a (hopefully final and minor) reroll.
cf. <20231016134421.21659-1-phillip.wood123@gmail.com>
source: <cover.1696889529.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-10-19) 7 commits
- 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.
Needs review.
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/doc-unit-tests-fixup (2023-10-11) 1 commit
. SQUASH???
(this branch uses js/doc-unit-tests and js/doc-unit-tests-with-cmake.)
Quick fix for jc/doc-unit-tests topic to unbreak CI running on 'seen'.
Superseded by a fixup! in the base series.
source: <xmqqwmvskf8t.fsf@gitster.g>
^ permalink raw reply
* Re: [PATCH 0/7] log: decorate pseudorefs and other refs
From: Andy Koppe @ 2023-10-22 21:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq1qdnseed.fsf@gitster.g>
On 22/10/2023 01:13, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> writes:
>> This series is to replace the 'decorate: add color.decorate.symbols
>> config option' patch proposed at:
>> https://lore.kernel.org/git/20231003205442.22963-1-andy.koppe@gmail.com
>
> If that is the case, it probably would have been nicer to mark the
> series as [PATCH v2].
Thanks, I wasn't sure about that due to the change in title and increase
in scope. I shall err towards version-bumping in any future such cases.
> Also, can you make messages [1/7]..[7/7] replies to [0/7] when you
> send them out? It seems that all 8 of them (including the cover
> letter) are replies to the previous round, which looked a bit
> unusual.
Not quite sure how that happened, but I think my mistake was passing
--in-reply-to to git-format-patch instead of git-send-email.
> [2/7] is a trivial readability improvement. It obviously should be
> left outside the scope of this series, but we should notice
> the same pattern in similar color tables (e.g., wt-status.c
> has one, diff.c has another) and perform the same clean-up as
> a #leftoverbits item.
Okay, I've removed that commit in v2. (I should have mentioned in the
commit message that it was triggered by the inconsistency with the
immediately following color_decorate_slots array, which uses designated
initializers.)
> [4/7] The name of new member .include added to ref_namespace_info
> will not be understood by anybody unless they are too deeply
> obsessed by decoration mechansim. As the namespace_info
> covers far wider interest, so a name that *shouts* that it is
> about decoration filter must be used to be understood by
> readers of the code
Agreed.
> [5/7] I am not sure if "other refs" should be an item in the
> namespace_info array. If it is truly "catch-all", then
> shouldn't the refs in other namespaces without their own
> decoration (e.g. ones in refs/notes/ and refs/prefetch/) be
> colored in the same way as this new class?
They would, because add_ref_decoration() skips ref_namespace entries
without a decoration type, so they would fall through to "refs/" and
pick up the DECORATION_REF type.
> And if so, having
> it as an independent element that sits next to these other
> classes smells like a strange design. >
> Another more worrying thing is that existing .ref members are
> designed to never overlap with each other, but this one
> obviously does. When a caller with a ref (or a pseudoref)
> asks "which namespace does this one belong to", does the
> existing code still do the right thing with this new element?
> Without it, because there was no overlap, an implementation
> can randomly search in the namespace_info table and stop at
> the first hit, but now with the overlapping and widely open
> .ref = "refs/", the implementation of the search must know
> that it is a fallback position (i.e. if it found a match with
> the fallback .ref = "refs/" , unless it looked at all other
> entries that could begin with "refs/" and are more specific,
> it needs to keep going).
Fair points. I've rewritten things to not touch the ref_namespace array.
> [6/7] This is pretty straight-forward, assuming that the existing
> is_pseudoref_syntax() function does the right thing. I am
> not sure about that, though. A refname with '-' is allowed
> to be called a pseudoref???
>
> Also, not a fault of this patch, but the "_syntax" in its
> name is totally unnecessary, I would think. At first glance,
> I suspected that the excuse to append _syntax may have been
> to signal the fact that the helper function does not check if
> there actually is such a ref, but examining a few helpers
> defined nearby tells us that such an excuse does not make
> sense:
I've dropped the use of that function from the change, checking against
the actual pseudoref names instead.
> [7/7] Allowing pseudorefs to optionally used when decorating might
> be a good idea, but I do not think it is particularly a good
> design decision to enable it by default.
Okay!
> Each of them forming a separate "namespace" also looks like a
> poor design, as being able to group multiple things into one
> family and treat them the same way is the primary point of
> "namespace", I would think.
Fair enough, although the array already contains HEAD and refs/stash as
singletons. I had vacillated about shoe-horning the pseudorefs in there,
and was swayed by having a single place to define which (pseudo)refs
should be included in decorations by default. That motivation goes away
with all the pseudorefs off by default.
I've rewritten things to handle the pseudorefs separately from the
ref_namespace array, with iteration functions similar to the ones used
for HEAD and proper refs.
> You do not want to say "I want
> to decorate off of ORIG_HEAD and FETCH_HEAD"; instead you
> would want to say "I want to decorate off of any pseudoref".
They can now all be enabled with --clear-decorations or
log.initialDecorationSet=all, or be controlled individually with the
other filter options.
Thank you very much for the review!
Andy
^ permalink raw reply
* [PATCH v2 6/6] log: add color.decorate.pseudoref config variable
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>
Add the ability to show pseudorefs in log decorations, and add config
variable color.decorate.pseudoref to determine their color. They will
not be shown unless the default decoration filtering is overridden with
the relevant log options such as --clear-decorations or
log.initialDecorationSet.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 4 +++-
commit.h | 1 +
log-tree.c | 24 +++++++++++++++++++
..._--decorate=full_--clear-decorations_--all | 4 ++--
...f.log_--decorate_--clear-decorations_--all | 4 ++--
t/t4202-log.sh | 21 +++++++++-------
t/t4207-log-decoration-colors.sh | 13 +++++++---
7 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 005a2bdb03..7af7d65f76 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -92,6 +92,8 @@ color.decorate.<slot>::
the stash ref
`ref`;;
any other refs (not shown by default)
+`pseudoref`;;
+ pseudorefs such as ORIG_HEAD or MERGE_HEAD (not shown by default)
`grafted`;;
grafted and replaced commits
`symbol`;;
@@ -99,7 +101,7 @@ color.decorate.<slot>::
--
+
(Variable `log.initialDecorationSet` or linkgit:git-log[1] option
-`--clear-decorations` can be used to show all refs.)
+`--clear-decorations` can be used to show all refs and pseudorefs.)
color.grep::
When set to `always`, always highlight matches. When `false` (or
diff --git a/commit.h b/commit.h
index f6b2125fc4..44dd3ce19b 100644
--- a/commit.h
+++ b/commit.h
@@ -56,6 +56,7 @@ enum decoration_type {
DECORATION_REF_STASH,
DECORATION_REF,
DECORATION_REF_HEAD,
+ DECORATION_REF_PSEUDO,
DECORATION_GRAFTED,
DECORATION_SYMBOL,
};
diff --git a/log-tree.c b/log-tree.c
index 36558f3008..4091b55532 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
GIT_COLOR_BOLD_MAGENTA, /* REF_STASH */
GIT_COLOR_BOLD_MAGENTA, /* REF */
GIT_COLOR_BOLD_CYAN, /* REF_HEAD */
+ GIT_COLOR_BOLD_BLUE, /* REF_PSEUDO */
GIT_COLOR_BOLD_BLUE, /* GRAFTED */
GIT_COLOR_NIL, /* SYMBOL */
};
@@ -52,6 +53,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_STASH] = "stash",
[DECORATION_REF] = "ref",
[DECORATION_REF_HEAD] = "HEAD",
+ [DECORATION_REF_PSEUDO] = "pseudoref",
[DECORATION_GRAFTED] = "grafted",
[DECORATION_SYMBOL] = "symbol",
};
@@ -208,6 +210,27 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
return 0;
}
+static int add_pseudoref_decoration(const char *refname,
+ const struct object_id *oid,
+ int flags UNUSED,
+ void *cb_data)
+{
+ struct object *obj;
+ enum object_type objtype;
+ struct decoration_filter *filter = (struct decoration_filter *)cb_data;
+
+ if (filter && !ref_filter_match(refname, filter))
+ return 0;
+
+ objtype = oid_object_info(the_repository, oid, NULL);
+ if (objtype < 0)
+ return 0;
+
+ obj = lookup_object_by_type(the_repository, oid, objtype);
+ add_name_decoration(DECORATION_REF_PSEUDO, refname, obj);
+ return 0;
+}
+
static int add_graft_decoration(const struct commit_graft *graft,
void *cb_data UNUSED)
{
@@ -236,6 +259,7 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)
decoration_loaded = 1;
decoration_flags = flags;
for_each_ref(add_ref_decoration, filter);
+ for_each_pseudoref(add_pseudoref_decoration, filter);
head_ref(add_ref_decoration, filter);
for_each_commit_graft(add_graft_decoration, filter);
}
diff --git a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
index 1c030a6554..7d16978e7f 100644
--- a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
+++ b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
@@ -33,13 +33,13 @@ Date: Mon Jun 26 00:04:00 2006 +0000
Merge branch 'side'
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (FETCH_HEAD, refs/heads/side)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:03:00 2006 +0000
Side
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:02:00 2006 +0000
diff --git a/t/t4013/diff.log_--decorate_--clear-decorations_--all b/t/t4013/diff.log_--decorate_--clear-decorations_--all
index 88be82cce3..4f9be50ce0 100644
--- a/t/t4013/diff.log_--decorate_--clear-decorations_--all
+++ b/t/t4013/diff.log_--decorate_--clear-decorations_--all
@@ -33,13 +33,13 @@ Date: Mon Jun 26 00:04:00 2006 +0000
Merge branch 'side'
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (FETCH_HEAD, side)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:03:00 2006 +0000
Side
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:02:00 2006 +0000
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index af4a123cd2..b14da62e3e 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -927,7 +927,7 @@ test_expect_success 'multiple decorate-refs' '
test_expect_success 'decorate-refs-exclude with glob' '
cat >expect.decorate <<-\EOF &&
Merge-tag-reach (HEAD -> main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: seventh)
octopus-b (tag: octopus-b)
octopus-a (tag: octopus-a)
@@ -944,7 +944,7 @@ test_expect_success 'decorate-refs-exclude with glob' '
test_expect_success 'decorate-refs-exclude without globs' '
cat >expect.decorate <<-\EOF &&
Merge-tag-reach (HEAD -> main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: seventh)
octopus-b (tag: octopus-b, octopus-b)
octopus-a (tag: octopus-a, octopus-a)
@@ -961,7 +961,7 @@ test_expect_success 'decorate-refs-exclude without globs' '
test_expect_success 'multiple decorate-refs-exclude' '
cat >expect.decorate <<-\EOF &&
Merge-tag-reach (HEAD -> main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: seventh)
octopus-b (tag: octopus-b)
octopus-a (tag: octopus-a)
@@ -1022,10 +1022,12 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
EOF
git log -n6 --decorate=short --pretty="tformat:%f%d" \
--decorate-refs-exclude="*octopus*" \
+ --decorate-refs-exclude="ORIG_HEAD" \
--simplify-by-decoration >actual &&
test_cmp expect.decorate actual &&
- git -c log.excludeDecoration="*octopus*" log \
- -n6 --decorate=short --pretty="tformat:%f%d" \
+ git -c log.excludeDecoration="*octopus*" \
+ -c log.excludeDecoration="ORIG_HEAD" \
+ log -n6 --decorate=short --pretty="tformat:%f%d" \
--simplify-by-decoration >actual &&
test_cmp expect.decorate actual
'
@@ -1067,9 +1069,10 @@ test_expect_success 'decorate-refs and simplify-by-decoration without output' '
test_cmp expect actual
'
-test_expect_success 'decorate-refs-exclude HEAD' '
+test_expect_success 'decorate-refs-exclude HEAD ORIG_HEAD' '
git log --decorate=full --oneline \
- --decorate-refs-exclude="HEAD" >actual &&
+ --decorate-refs-exclude="HEAD" \
+ --decorate-refs-exclude="ORIG_HEAD" >actual &&
! grep HEAD actual
'
@@ -1107,7 +1110,7 @@ test_expect_success '--clear-decorations overrides defaults' '
cat >expect.all <<-\EOF &&
Merge-tag-reach (HEAD -> refs/heads/main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: refs/tags/seventh)
octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
@@ -1139,7 +1142,7 @@ test_expect_success '--clear-decorations clears previous exclusions' '
cat >expect.all <<-\EOF &&
Merge-tag-reach (HEAD -> refs/heads/main)
reach (tag: refs/tags/reach, refs/heads/reach)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
seventh (tag: refs/tags/seventh)
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 4b51e34f8b..0b32e0bb8e 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
git config color.decorate.tag "reverse bold yellow" &&
git config color.decorate.stash magenta &&
git config color.decorate.ref blue &&
+ git config color.decorate.pseudoref "bold cyan" &&
git config color.decorate.grafted black &&
git config color.decorate.symbol white &&
git config color.decorate.HEAD cyan &&
@@ -30,6 +31,7 @@ test_expect_success setup '
c_tag="<BOLD;REVERSE;YELLOW>" &&
c_stash="<MAGENTA>" &&
c_ref="<BLUE>" &&
+ c_pseudoref="<BOLD;CYAN>" &&
c_HEAD="<CYAN>" &&
c_grafted="<BLACK>" &&
c_symbol="<WHITE>" &&
@@ -46,7 +48,10 @@ test_expect_success setup '
test_commit B &&
git tag v1.0 &&
echo >>A.t &&
- git stash save Changes to A.t
+ git stash save Changes to A.t &&
+ git reset other/main &&
+ git reset ORIG_HEAD &&
+ git revert --no-commit @~
'
cmp_filtered_decorations () {
@@ -63,17 +68,19 @@ ${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_pseudoref}ORIG_HEAD${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_symbol}, ${c_reset}\
${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_pseudoref}REVERT_HEAD${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
EOF
- git log --first-parent --no-abbrev --decorate --clear-decorations \
- --oneline --color=always --all >actual &&
+ git log --first-parent --no-abbrev --decorate --color=always \
+ --decorate-refs-exclude=FETCH_HEAD --oneline --all >actual &&
cmp_filtered_decorations
'
--
2.42.GIT
^ permalink raw reply related
* [PATCH v2 5/6] refs: exempt pseudorefs from pattern prefixing
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>
In normalize_glob_ref(), don't prefix pseudorefs with "refs/", thereby
implementing a NEEDSWORK from b877e617e6e5.
This is in preparation for showing pseudorefs in log decorations, as
they are not matched as intended in decoration filters otherwise. The
function is only used in load_ref_decorations().
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
refs.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index aa7e4c02c5..fbd15a8cff 100644
--- a/refs.c
+++ b/refs.c
@@ -565,13 +565,16 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
if (prefix)
strbuf_addstr(&normalized_pattern, prefix);
- else if (!starts_with(pattern, "refs/") &&
- strcmp(pattern, "HEAD"))
- strbuf_addstr(&normalized_pattern, "refs/");
- /*
- * NEEDSWORK: Special case other symrefs such as REBASE_HEAD,
- * MERGE_HEAD, etc.
- */
+ else if (!starts_with(pattern, "refs/") && strcmp(pattern, "HEAD")) {
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pseudorefs); i++)
+ if (!strcmp(pattern, pseudorefs[i]))
+ break;
+
+ if (i == ARRAY_SIZE(pseudorefs))
+ strbuf_addstr(&normalized_pattern, "refs/");
+ }
strbuf_addstr(&normalized_pattern, pattern);
strbuf_strip_suffix(&normalized_pattern, "/");
--
2.42.GIT
^ permalink raw reply related
* [PATCH v2 4/6] refs: add pseudorefs array and iteration functions
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>
Define const array 'pseudorefs' with the names of the pseudorefs that
are documented in gitrevisions.1, and add functions for_each_pseudoref()
and refs_for_each_pseudoref() for iterating over them.
The functions process the pseudorefs in the same way as head_ref() and
refs_head_ref() process HEAD, invoking an each_ref_fn callback on each
pseudoref that exists.
This is in preparation for adding pseudorefs to log decorations.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
refs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
refs.h | 5 +++++
2 files changed, 47 insertions(+)
diff --git a/refs.c b/refs.c
index fcae5dddc6..aa7e4c02c5 100644
--- a/refs.c
+++ b/refs.c
@@ -65,6 +65,21 @@ static unsigned char refname_disposition[256] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
};
+/*
+ * List of documented pseudorefs. This needs to be kept in sync with the list
+ * in Documentation/revisions.txt.
+ */
+static const char *const pseudorefs[] = {
+ "FETCH_HEAD",
+ "ORIG_HEAD",
+ "MERGE_HEAD",
+ "REBASE_HEAD",
+ "CHERRY_PICK_HEAD",
+ "REVERT_HEAD",
+ "BISECT_HEAD",
+ "AUTO_MERGE",
+};
+
struct ref_namespace_info ref_namespace[] = {
[NAMESPACE_HEAD] = {
.ref = "HEAD",
@@ -1549,6 +1564,33 @@ int head_ref(each_ref_fn fn, void *cb_data)
return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
}
+int refs_for_each_pseudoref(struct ref_store *refs,
+ each_ref_fn fn, void *cb_data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pseudorefs); i++) {
+ struct object_id oid;
+ int flag;
+
+ if (refs_resolve_ref_unsafe(refs, pseudorefs[i],
+ RESOLVE_REF_READING, &oid, &flag)) {
+ int ret = fn(pseudorefs[i], &oid, flag, cb_data);
+
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int for_each_pseudoref(each_ref_fn fn, void *cb_data)
+{
+ return refs_for_each_pseudoref(get_main_ref_store(the_repository),
+ fn, cb_data);
+}
+
struct ref_iterator *refs_ref_iterator_begin(
struct ref_store *refs,
const char *prefix,
diff --git a/refs.h b/refs.h
index 23211a5ea1..7b55cced31 100644
--- a/refs.h
+++ b/refs.h
@@ -320,6 +320,8 @@ typedef int each_repo_ref_fn(struct repository *r,
*/
int refs_head_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
+int refs_for_each_pseudoref(struct ref_store *refs,
+ each_ref_fn fn, void *cb_data);
int refs_for_each_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -334,6 +336,9 @@ int refs_for_each_remote_ref(struct ref_store *refs,
/* just iterates the head ref. */
int head_ref(each_ref_fn fn, void *cb_data);
+/* iterates pseudorefs. */
+int for_each_pseudoref(each_ref_fn fn, void *cb_data);
+
/* iterates all refs. */
int for_each_ref(each_ref_fn fn, void *cb_data);
--
2.42.GIT
^ permalink raw reply related
* [PATCH v2 3/6] log: add color.decorate.ref config variable
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>
Refs other than branches, remote-tracking branches, tags and the stash
do not appear in log decorations by default, but they can be shown by
using decoration filter options such as --clear-decorations or
log.initialDecorationSet. However, they would appear without color.
Add config variable color.decorate.ref for such refs, defaulting to bold
magenta, which is the same as refs/stash.
Document the new variable on the git-config page and amend
t4207-log-decoration-colors.sh to test it.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 5 +++++
commit.h | 1 +
log-tree.c | 4 +++-
t/t4207-log-decoration-colors.sh | 9 +++++++--
4 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index cc0a881125..005a2bdb03 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -90,11 +90,16 @@ color.decorate.<slot>::
lightweight and annotated tags
`stash`;;
the stash ref
+`ref`;;
+ any other refs (not shown by default)
`grafted`;;
grafted and replaced commits
`symbol`;;
punctuation symbols surrounding the other elements
--
++
+(Variable `log.initialDecorationSet` or linkgit:git-log[1] option
+`--clear-decorations` can be used to show all refs.)
color.grep::
When set to `always`, always highlight matches. When `false` (or
diff --git a/commit.h b/commit.h
index cb13e4d5ba..f6b2125fc4 100644
--- a/commit.h
+++ b/commit.h
@@ -54,6 +54,7 @@ enum decoration_type {
DECORATION_REF_REMOTE,
DECORATION_REF_TAG,
DECORATION_REF_STASH,
+ DECORATION_REF,
DECORATION_REF_HEAD,
DECORATION_GRAFTED,
DECORATION_SYMBOL,
diff --git a/log-tree.c b/log-tree.c
index 5ad168458e..36558f3008 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -39,6 +39,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
GIT_COLOR_BOLD_RED, /* REF_REMOTE */
GIT_COLOR_BOLD_YELLOW, /* REF_TAG */
GIT_COLOR_BOLD_MAGENTA, /* REF_STASH */
+ GIT_COLOR_BOLD_MAGENTA, /* REF */
GIT_COLOR_BOLD_CYAN, /* REF_HEAD */
GIT_COLOR_BOLD_BLUE, /* GRAFTED */
GIT_COLOR_NIL, /* SYMBOL */
@@ -49,6 +50,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_REMOTE] = "remoteBranch",
[DECORATION_REF_TAG] = "tag",
[DECORATION_REF_STASH] = "stash",
+ [DECORATION_REF] = "ref",
[DECORATION_REF_HEAD] = "HEAD",
[DECORATION_GRAFTED] = "grafted",
[DECORATION_SYMBOL] = "symbol",
@@ -151,7 +153,7 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
int i;
struct object *obj;
enum object_type objtype;
- enum decoration_type deco_type = DECORATION_NONE;
+ enum decoration_type deco_type = DECORATION_REF;
struct decoration_filter *filter = (struct decoration_filter *)cb_data;
const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index f4173b6114..4b51e34f8b 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -17,6 +17,7 @@ test_expect_success setup '
git config color.decorate.remoteBranch red &&
git config color.decorate.tag "reverse bold yellow" &&
git config color.decorate.stash magenta &&
+ git config color.decorate.ref blue &&
git config color.decorate.grafted black &&
git config color.decorate.symbol white &&
git config color.decorate.HEAD cyan &&
@@ -28,11 +29,13 @@ test_expect_success setup '
c_remoteBranch="<RED>" &&
c_tag="<BOLD;REVERSE;YELLOW>" &&
c_stash="<MAGENTA>" &&
+ c_ref="<BLUE>" &&
c_HEAD="<CYAN>" &&
c_grafted="<BLACK>" &&
c_symbol="<WHITE>" &&
test_commit A &&
+ git update-ref refs/foo A &&
git clone . other &&
(
cd other &&
@@ -65,10 +68,12 @@ ${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
+${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
EOF
- git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
+ git log --first-parent --no-abbrev --decorate --clear-decorations \
+ --oneline --color=always --all >actual &&
cmp_filtered_decorations
'
--
2.42.GIT
^ permalink raw reply related
* [PATCH v2 2/6] log: add color.decorate.symbol config variable
From: Andy Koppe @ 2023-10-22 21:44 UTC (permalink / raw)
To: git; +Cc: gitster, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>
Add new color.decorate.symbol config variable for determining the
color of the prefix, suffix, separator and pointer symbols used in
log --decorate output and related log format placeholders, to allow
them to be colored differently from commit hashes.
For backward compatibility, fall back to the commit hash color that can
be specified with the color.diff.commit variable if the new variable is
not provided.
Add the variable to the color.decorate.<slot> documentation.
Amend t4207-log-decoration-colors.sh to test it. Put ${c_reset} elements
in the expected output at the end of lines for consistency.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 2 ++
commit.h | 1 +
log-tree.c | 15 ++++++---
t/t4207-log-decoration-colors.sh | 58 +++++++++++++++++---------------
4 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 3453703f9b..cc0a881125 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -92,6 +92,8 @@ color.decorate.<slot>::
the stash ref
`grafted`;;
grafted and replaced commits
+`symbol`;;
+ punctuation symbols surrounding the other elements
--
color.grep::
diff --git a/commit.h b/commit.h
index 28928833c5..cb13e4d5ba 100644
--- a/commit.h
+++ b/commit.h
@@ -56,6 +56,7 @@ enum decoration_type {
DECORATION_REF_STASH,
DECORATION_REF_HEAD,
DECORATION_GRAFTED,
+ DECORATION_SYMBOL,
};
void add_name_decoration(enum decoration_type type, const char *name, struct object *obj);
diff --git a/log-tree.c b/log-tree.c
index 504da6b519..5ad168458e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
GIT_COLOR_BOLD_MAGENTA, /* REF_STASH */
GIT_COLOR_BOLD_CYAN, /* REF_HEAD */
GIT_COLOR_BOLD_BLUE, /* GRAFTED */
+ GIT_COLOR_NIL, /* SYMBOL */
};
static const char *color_decorate_slots[] = {
@@ -50,6 +51,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_STASH] = "stash",
[DECORATION_REF_HEAD] = "HEAD",
[DECORATION_GRAFTED] = "grafted",
+ [DECORATION_SYMBOL] = "symbol",
};
static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
@@ -312,7 +314,7 @@ void format_decorations(struct strbuf *sb,
{
const struct name_decoration *decoration;
const struct name_decoration *current_and_HEAD;
- const char *color_commit, *color_reset;
+ const char *color_symbol, *color_reset;
const char *prefix = " (";
const char *suffix = ")";
@@ -337,7 +339,10 @@ void format_decorations(struct strbuf *sb,
tag = opts->tag;
}
- color_commit = diff_get_color(use_color, DIFF_COMMIT);
+ color_symbol = decorate_get_color(use_color, DECORATION_SYMBOL);
+ if (color_is_nil(color_symbol))
+ color_symbol = diff_get_color(use_color, DIFF_COMMIT);
+
color_reset = decorate_get_color(use_color, DECORATION_NONE);
current_and_HEAD = current_pointed_by_HEAD(decoration);
@@ -352,7 +357,7 @@ void format_decorations(struct strbuf *sb,
decorate_get_color(use_color, decoration->type);
if (*prefix) {
- strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, color_symbol);
strbuf_addstr(sb, prefix);
strbuf_addstr(sb, color_reset);
}
@@ -369,7 +374,7 @@ void format_decorations(struct strbuf *sb,
if (current_and_HEAD &&
decoration->type == DECORATION_REF_HEAD) {
- strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, color_symbol);
strbuf_addstr(sb, pointer);
strbuf_addstr(sb, color_reset);
strbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));
@@ -382,7 +387,7 @@ void format_decorations(struct strbuf *sb,
decoration = decoration->next;
}
if (*suffix) {
- strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, color_symbol);
strbuf_addstr(sb, suffix);
strbuf_addstr(sb, color_reset);
}
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 21986a866d..f4173b6114 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
git config color.decorate.tag "reverse bold yellow" &&
git config color.decorate.stash magenta &&
git config color.decorate.grafted black &&
+ git config color.decorate.symbol white &&
git config color.decorate.HEAD cyan &&
c_reset="<RESET>" &&
@@ -29,6 +30,7 @@ test_expect_success setup '
c_stash="<MAGENTA>" &&
c_HEAD="<CYAN>" &&
c_grafted="<BLACK>" &&
+ c_symbol="<WHITE>" &&
test_commit A &&
git clone . other &&
@@ -53,17 +55,17 @@ cmp_filtered_decorations () {
# to this test since it does not contain any decoration, hence --first-parent
test_expect_success 'commit decorations colored correctly' '
cat >expect <<-EOF &&
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_commit}, \
-${c_reset}${c_remoteBranch}other/main${c_reset}${c_commit})${c_reset} A1
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_stash}refs/stash${c_reset}${c_commit})${c_reset} On main: Changes to A.t
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_symbol}, ${c_reset}\
+${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
EOF
git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
@@ -78,14 +80,14 @@ test_expect_success 'test coloring with replace-objects' '
git replace HEAD~1 HEAD~2 &&
cat >expect <<-EOF &&
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_commit})${c_reset} D
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}C${c_reset}${c_commit}, \
-${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_symbol})${c_reset} D
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}C${c_reset}${c_symbol}, ${c_reset}\
+${c_grafted}replaced${c_reset}${c_symbol})${c_reset} B
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
EOF
git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
@@ -104,15 +106,15 @@ test_expect_success 'test coloring with grafted commit' '
git replace --graft HEAD HEAD~2 &&
cat >expect <<-EOF &&
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_commit}, \
-${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_symbol}, ${c_reset}\
+${c_grafted}replaced${c_reset}${c_symbol})${c_reset} D
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
EOF
git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
--
2.42.GIT
^ 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