On Wed, Nov 01, 2023 at 11:06:05AM +0900, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> >> Thanks, the range-diff here looks exactly as expected. Thanks for
> >> working on this, this version LGTM.
> >
> > OK, I'd like a version as incremental to v2 (since it already is in
> > 'next') that results in the same tree state as v3 then.
> >
> > Thanks for working on it, and reviewing it.
>
> In the meantime, here is a mechanically produced incremental I'll
> tentatively queue. Hopefully I did not screw up while generating
> it.
>
> Thanks.
Ah, sorry, didn't notice it was in 'next' already. Anyway, the diff
below looks good to me, thanks!
Patrick
> --- >8 ---
> From: Patrick Steinhardt <ps@xxxxxx>
> Date: Tue, 31 Oct 2023 08:16:09 +0100
> Subject: [PATCH] commit-graph: clarify GIT_COMMIT_GRAPH_PARANOIA documentation
>
> In response to reviews of the previous round that has already hit
> 'next', clarify the help text for GIT_COMMIT_GRAPH_PARANOIA and
> rename object_paranoia variable to commit_graph_paranoia for
> consistency.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> Documentation/git.txt | 15 ++++++++-------
> commit-graph.c | 8 ++++----
> commit.c | 8 ++++----
> t/t5318-commit-graph.sh | 2 +-
> 4 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 22c2b537aa..3bac24cf8a 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -912,13 +912,14 @@ for full details.
> 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).
> + When loading a commit object from the commit-graph, Git performs an
> + existence check on the object in the object database. This is done to
> + avoid issues with stale commit-graphs that contain references to
> + already-deleted commits, but comes with a performance penalty.
> ++
> +The default is "true", which enables the aforementioned behavior.
> +Setting this to "false" disables the existence check. This can lead to
> +a performance improvement at the cost of consistency.
>
> `GIT_ALLOW_PROTOCOL`::
> If set to a colon-separated list of protocols, behave as if
> diff --git a/commit-graph.c b/commit-graph.c
> index 376f59af73..b37fdcb214 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -907,18 +907,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;
> + static int commit_graph_paranoia = -1;
> struct commit *commit;
> uint32_t pos;
>
> - if (object_paranoia == -1)
> - object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
> + if (commit_graph_paranoia == -1)
> + commit_graph_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 (object_paranoia && !has_object(repo, id, 0))
> + if (commit_graph_paranoia && !has_object(repo, id, 0))
> return NULL;
>
> commit = lookup_commit(repo, id);
> diff --git a/commit.c b/commit.c
> index 7399e90212..8405d7c3fc 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -574,12 +574,12 @@ int repo_parse_commit_internal(struct repository *r,
> if (item->object.parsed)
> return 0;
> if (use_commit_graph && parse_commit_in_graph(r, item)) {
> - static int object_paranoia = -1;
> + static int commit_graph_paranoia = -1;
>
> - if (object_paranoia == -1)
> - object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
> + if (commit_graph_paranoia == -1)
> + commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
>
> - if (object_paranoia && !has_object(r, &item->object.oid, 0)) {
> + if (commit_graph_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"),
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index 55e3c7ec78..2c62b91ef9 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -847,7 +847,7 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '
> test_commit C &&
> git commit-graph write --reachable &&
>
> - # Corrupt the repository by deleting the intermittent commit
> + # Corrupt the repository by deleting the intermediate commit
> # object. Commands should notice that this object is absent and
> # thus that the repository is corrupt even if the commit graph
> # exists.
> --
> 2.42.0-530-g692be87cbb
>
Attachment:
signature.asc
Description: PGP signature
