git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 7/7] commit-graph.c: introduce '--[no-]check-oids'

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 15, 2020, 04:29 UTC
Message-ID
<20200415042930.GA11703@syl.local>
In-Reply-To
<1ff42f4c3d568dd25889d2808cda3edf38a36cb9.1586836700.git.me@ttaylorr.com>
Whoops. I sent the wrong version of this patch. It should be the below:
--- >8 ---
Subject: [PATCH] shallow.c: use 'reset_repository_shallow' when appropriate

In bd0b42aed3 (fetch-pack: do not take shallow lock unnecessarily, 2019-01-10), the author noted that 'is_repository_shallow' produces visible side-effect(s) by setting 'is_shallow' and 'shallow_stat'.

This is a problem for e.g., fetching with '--update-shallow' in a shallow repsoitory with 'fetch.writeCommitGraph' enabled, since the update to '.git/shallow' will cause Git to think that the repository *isn't* shallow when it is, thereby circumventing the commit-graph compatability check.

This causes problems in shallow repositories with at least shallow refs that have at least one ancestor (since the client won't have those object(s), and therefore can't take the reachability closure over commits to be written to the commit-graph).

Address this by introducing 'reset_repository_shallow()', and calling it when the shallow file is updated, forcing 'is_repository_shallow' to re-evaluate whether the repository is still shallow after fetching in the above scenario.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 commit.h     |  1 +
 fetch-pack.c |  1 +
 shallow.c    | 15 ++++++++-------
 3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/commit.h b/commit.h
index 008a0fa4a0..ee1ba139d4 100644
--- a/commit.h
+++ b/commit.h
@@ -251,6 +251,7 @@ int register_shallow(struct repository *r, const struct object_id *oid);
 int unregister_shallow(const struct object_id *oid);
 int for_each_commit_graft(each_commit_graft_fn, void *);
 int is_repository_shallow(struct repository *r);
+void reset_repository_shallow(struct repository *r);
 struct commit_list *get_shallow_commits(struct object_array *heads,
 					int depth, int shallow_flag, int not_shallow_flag);
 struct commit_list *get_shallow_commits_by_rev_list(
diff --git a/fetch-pack.c b/fetch-pack.c
index 1734a573b0..051902ef6d 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1630,6 +1630,7 @@ static void update_shallow(struct fetch_pack_args *args,
 		if (*alternate_shallow_file == '\0') { /* --unshallow */
 			unlink_or_warn(git_path_shallow(the_repository));
 			rollback_lock_file(&shallow_lock);
+			reset_repository_shallow(the_repository);
 		} else
 			commit_lock_file(&shallow_lock);
 		alternate_shallow_file = NULL;
diff --git a/shallow.c b/shallow.c
index 7fd04afed1..fac383dec9 100644
--- a/shallow.c
+++ b/shallow.c
@@ -40,13 +40,6 @@ int register_shallow(struct repository *r, const struct object_id *oid)

 int is_repository_shallow(struct repository *r)
 {
-	/*
-	 * NEEDSWORK: This function updates
-	 * r->parsed_objects->{is_shallow,shallow_stat} as a side effect but
-	 * there is no corresponding function to clear them when the shallow
-	 * file is updated.
-	 */
-
 	FILE *fp;
 	char buf[1024];
 	const char *path = r->parsed_objects->alternate_shallow_file;
@@ -79,6 +72,12 @@ int is_repository_shallow(struct repository *r)
 	return r->parsed_objects->is_shallow;
 }

+void reset_repository_shallow(struct repository *r)
+{
+	r->parsed_objects->is_shallow = -1;
+	stat_validity_clear(r->parsed_objects->shallow_stat);
+}
+
 /*
  * TODO: use "int" elemtype instead of "int *" when/if commit-slab
  * supports a "valid" flag.
@@ -362,6 +361,7 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
 		 * shallow file".
 		 */
 		*alternate_shallow_file = "";
+	reset_repository_shallow(the_repository);
 	strbuf_release(&sb);
 }

@@ -411,6 +411,7 @@ void prune_shallow(unsigned options)
 			die_errno("failed to write to %s",
 				  get_lock_file_path(&shallow_lock));
 		commit_lock_file(&shallow_lock);
+		reset_repository_shallow(the_repository);
 	} else {
 		unlink(git_path_shallow(the_repository));
 		rollback_lock_file(&shallow_lock);
--
2.26.0.106.g9fadedd637
Previous: Taylor BlauNext: Taylor Blau
Message 9 of 19 in “commit-graph: split strategies, '--[no-]check-oids'”
  1. 0/7 commit-graph: split strategies, '--[no-]check-oids'Taylor Blau, Apr 14, 2020
  2. 1/7 t/helper/test-read-graph.c: support commit-graph chainsTaylor Blau, Apr 14, 2020
  3. 2/7 builtin/commit-graph.c: support for '--split[=<strategy>]'Taylor Blau, Apr 14, 2020
  4. 3/7 builtin/commit-graph.c: introduce split strategy 'no-merge'Taylor Blau, Apr 14, 2020
  5. 4/7 builtin/commit-graph.c: introduce split strategy 'replace'Taylor Blau, Apr 14, 2020
  6. 5/7 oidset: introduce 'oidset_size'Taylor Blau, Apr 14, 2020
  7. 6/7 commit-graph.h: replace 'commit_hex' with 'commits'Taylor Blau, Apr 14, 2020
  8. 7/7 commit-graph.c: introduce '--[no-]check-oids'Taylor Blau, Apr 14, 2020
  9. Taylor BlauApr 15, 2020
  10. Taylor BlauApr 15, 2020
  11. SZEDER GáborApr 22, 2020
  12. Taylor BlauApr 22, 2020
  13. SZEDER GáborApr 24, 2020
  14. Taylor BlauMay 1, 2020
  15. Jeff KingMay 3, 2020
  16. Junio C HamanoMay 3, 2020
  17. Jeff KingMay 4, 2020
  18. Junio C HamanoMay 4, 2020
  19. Taylor BlauMay 4, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.