All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>, chriscool@tuxfamily.org
Subject: [PATCH 2/2] fetch: do not override partial clone filter
Date: Mon, 21 Sep 2020 20:03:57 -0700	[thread overview]
Message-ID: <070a717aabbac31ae44567dbe54a325029f9931f.1600743698.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1600743698.git.jonathantanmy@google.com>

When a fetch with the --filter argument is made, the configured default
filter is set even if one already exists. This change was made in
5e46139376 ("builtin/fetch: remove unique promisor remote limitation",
2019-06-25) - in particular, changing from:

 * If this is the FIRST partial-fetch request, we enable partial
 * on this repo and remember the given filter-spec as the default
 * for subsequent fetches to this remote.

to:

 * If this is a partial-fetch request, we enable partial on
 * this repo if not already enabled and remember the given
 * filter-spec as the default for subsequent fetches to this
 * remote.

(The given filter-spec is "remembered" even if there is already an
existing one.)

This is problematic whenever a lazy fetch is made, because lazy fetches
are made using "git fetch --filter=blob:none", but this will also happen
if the user invokes "git fetch --filter=<filter>" manually. Therefore,
restore the behavior prior to 5e46139376, which writes a filter-spec
only if the current fetch request is the first partial-fetch one (for
that remote).

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 builtin/fetch.c               |  2 +-
 list-objects-filter-options.c | 10 +++++++++-
 t/t5601-clone.sh              |  5 +++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index a6d3268661..752148eec5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1677,7 +1677,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * If this is a partial-fetch request, we enable partial on
 	 * this repo if not already enabled and remember the given
 	 * filter-spec as the default for subsequent fetches to this
-	 * remote.
+	 * remote if there is currently no default filter-spec.
 	 */
 	if (filter_options.choice) {
 		partial_clone_register(remote->name, &filter_options);
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index b66314560a..defd3dfd10 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -344,11 +344,19 @@ void partial_clone_register(
 	const char *remote,
 	struct list_objects_filter_options *filter_options)
 {
+	struct promisor_remote *promisor_remote;
 	char *cfg_name;
 	char *filter_name;
 
 	/* Check if it is already registered */
-	if (!promisor_remote_find(remote)) {
+	if ((promisor_remote = promisor_remote_find(remote))) {
+		if (promisor_remote->partial_clone_filter)
+			/*
+			 * Remote is already registered and a filter is already
+			 * set, so we don't need to do anything here.
+			 */
+			return;
+	} else {
 		if (upgrade_repository_format(1) < 0)
 			die(_("unable to upgrade repository format to support partial clone"));
 
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 15fb64c18d..7c5cf5a2ec 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -672,6 +672,11 @@ test_expect_success 'partial clone with -o' '
 	git clone -o blah --filter=blob:limit=0 "file://$(pwd)/server" client
 '
 
+test_expect_success 'ensure that filter is written to config' '
+	FILTER=$(git -C client config --get remote.blah.partialclonefilter) &&
+	test "$FILTER" == "blob:limit=0"
+'
+
 test_expect_success 'partial clone: warn if server does not support object filtering' '
 	rm -rf server client &&
 	test_create_repo server &&
-- 
2.28.0.681.g6f77f65b4e-goog


  parent reply	other threads:[~2020-09-22  3:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-22  3:03 [PATCH 0/2] Fix overriding of partial clone filter when lazy-fetching Jonathan Tan
2020-09-22  3:03 ` [PATCH 1/2] promisor-remote: remove unused variable Jonathan Tan
2020-09-22  3:03 ` Jonathan Tan [this message]
2020-09-22  5:46   ` [PATCH 2/2] fetch: do not override partial clone filter Junio C Hamano
2020-09-22 11:35     ` Derrick Stolee
2020-09-22 15:51       ` Junio C Hamano
2020-09-22  5:45 ` [PATCH 0/2] Fix overriding of partial clone filter when lazy-fetching Junio C Hamano
2020-09-28 22:26 ` [PATCH v2 " Jonathan Tan
2020-09-28 22:26   ` [PATCH v2 1/2] promisor-remote: remove unused variable Jonathan Tan
2020-09-28 22:26   ` [PATCH v2 2/2] fetch: do not override partial clone filter Jonathan Tan
2020-09-28 23:43     ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=070a717aabbac31ae44567dbe54a325029f9931f.1600743698.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.