git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: dstolee@microsoft.com, gitster@pobox.com, martin.agren@gmail.com,
	peff@peff.net, szeder.dev@gmail.com
Subject: Re: [PATCH 7/7] commit-graph.c: introduce '--[no-]check-oids'
Date: Tue, 14 Apr 2020 22:29:30 -0600	[thread overview]
Message-ID: <20200415042930.GA11703@syl.local> (raw)
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

  reply	other threads:[~2020-04-15  4:29 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-14  4:03 [PATCH 0/7] commit-graph: split strategies, '--[no-]check-oids' Taylor Blau
2020-04-14  4:04 ` [PATCH 1/7] t/helper/test-read-graph.c: support commit-graph chains Taylor Blau
2020-04-14  4:04 ` [PATCH 2/7] builtin/commit-graph.c: support for '--split[=<strategy>]' Taylor Blau
2020-04-14  4:04 ` [PATCH 3/7] builtin/commit-graph.c: introduce split strategy 'no-merge' Taylor Blau
2020-04-14  4:04 ` [PATCH 4/7] builtin/commit-graph.c: introduce split strategy 'replace' Taylor Blau
2020-04-14  4:04 ` [PATCH 5/7] oidset: introduce 'oidset_size' Taylor Blau
2020-04-14  4:04 ` [PATCH 6/7] commit-graph.h: replace 'commit_hex' with 'commits' Taylor Blau
2020-04-14  4:04 ` [PATCH 7/7] commit-graph.c: introduce '--[no-]check-oids' Taylor Blau
2020-04-15  4:29   ` Taylor Blau [this message]
2020-04-15  4:31     ` Taylor Blau
2020-04-22 10:55       ` SZEDER Gábor
2020-04-22 23:39         ` Taylor Blau
2020-04-24 10:59           ` SZEDER Gábor
2020-05-01 22:38             ` Taylor Blau
2020-05-03  9:40               ` Jeff King
2020-05-03 16:55                 ` Junio C Hamano
2020-05-04 14:59                   ` Jeff King
2020-05-04 16:29                     ` Junio C Hamano
2020-05-04 22:16                       ` Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200415042930.GA11703@syl.local \
    --to=me@ttaylorr.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).