[PATCH 0/4] First steps towards partial clone submodules

@@ -99,6 +94,11 @@ static int promisor_remote_config(const char *var, const char *value, void *data
 	size_t namelen;
 	const char *subkey;

+	if (!strcmp(var, "extensions.partialclone")) {
+		repository_format_partial_clone = xstrdup(value);

@@ -235,9 +244,11 @@ int promisor_remote_get_direct(struct repository *repo,
 	if (oid_nr == 0)
 		return 0;

-	promisor_remote_init();
+	promisor_remote_init(repo);

-	for (r = promisors; r; r = r->next) {
+	if (repo != the_repository)
+		BUG("only the_repository is supported for now");

Instead of using global variables to store promisor remote information,
store this config in struct repository instead, and add
repository-agnostic non-static functions corresponding to the existing
non-static functions that only work on the_repository.

The actual lazy-fetching of missing objects currently does not work on
repositories other than the_repository, and will still not work after
this commit, so add a BUG message explaining this. A subsequent commit
will remove this limitation.

Signed-off-by: Jonathan Tan <redacted>
---
 promisor-remote.c | 101 +++++++++++++++++++++++++---------------------
 promisor-remote.h |  20 +++++++--
 repository.h      |   4 ++
 3 files changed, 77 insertions(+), 48 deletions(-)

diff --git a/promisor-remote.c b/promisor-remote.c
index bfe8eee5f2..5819d2cf28 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c

@@ -5,7 +5,11 @@
 #include "transport.h"
 #include "strvec.h"

-static char *repository_format_partial_clone;
+struct promisor_remote_config {
+       char *repository_format_partial_clone;
+       struct promisor_remote *promisors;
+       struct promisor_remote **promisors_tail;
+};

 static int fetch_objects(const char *remote_name,
                         const struct object_id *oids,

@@ -37,10 +41,8 @@ static int fetch_objects(const char *remote_name,
        return finish_command(&child) ? -1 : 0;
 }

-static struct promisor_remote *promisors;
-static struct promisor_remote **promisors_tail = &promisors;
-
-static struct promisor_remote *promisor_remote_new(const char *remote_name)
+static struct promisor_remote *promisor_remote_new(struct promisor_remote_config *config,
+                                                  const char *remote_name)
 {
        struct promisor_remote *r;

@@ -52,18 +54,19 @@ static struct promisor_remote *promisor_remote_new(const char *remote_name)

        FLEX_ALLOC_STR(r, name, remote_name);

-       *promisors_tail = r;
-       promisors_tail = &r->next;
+       *config->promisors_tail = r;
+       config->promisors_tail = &r->next;

        return r;
 }

-static struct promisor_remote *promisor_remote_lookup(const char *remote_name,
+static struct promisor_remote *promisor_remote_lookup(struct promisor_remote_config *config,
+                                                     const char *remote_name,
                                                      struct promisor_remote **previous)
 {
        struct promisor_remote *r, *p;

-       for (p = NULL, r = promisors; r; p = r, r = r->next)
+       for (p = NULL, r = config->promisors; r; p = r, r = r->next)
                if (!strcmp(r->name, remote_name)) {
                        if (previous)
                                *previous = p;

@@ -73,7 +76,8 @@ static struct promisor_remote *promisor_remote_lookup(const char *remote_name,
        return NULL;
 }

-static void promisor_remote_move_to_tail(struct promisor_remote *r,
+static void promisor_remote_move_to_tail(struct promisor_remote_config *config,
+                                        struct promisor_remote *r,
                                         struct promisor_remote *previous)
 {
        if (r->next == NULL)

@@ -82,20 +86,21 @@ static void promisor_remote_move_to_tail(struct promisor_remote *r,
        if (previous)
                previous->next = r->next;
        else
-               promisors = r->next ? r->next : r;
+               config->promisors = r->next ? r->next : r;
        r->next = NULL;
-       *promisors_tail = r;
-       promisors_tail = &r->next;
+       *config->promisors_tail = r;
+       config->promisors_tail = &r->next;
 }

 static int promisor_remote_config(const char *var, const char *value, void *data)
 {
+       struct promisor_remote_config *config = data;
        const char *name;
        size_t namelen;
        const char *subkey;

        if (!strcmp(var, "extensions.partialclone")) {
-               repository_format_partial_clone = xstrdup(value);
+               config->repository_format_partial_clone = xstrdup(value);
                return 0;
        }

@@ -110,8 +115,8 @@ static int promisor_remote_config(const char *var, const char *value, void *data

                remote_name = xmemdupz(name, namelen);

-               if (!promisor_remote_lookup(remote_name, NULL))
-                       promisor_remote_new(remote_name);
+               if (!promisor_remote_lookup(config, remote_name, NULL))
+                       promisor_remote_new(config, remote_name);

                free(remote_name);
                return 0;

@@ -120,9 +125,9 @@ static int promisor_remote_config(const char *var, const char *value, void *data
                struct promisor_remote *r;
                char *remote_name = xmemdupz(name, namelen);

-               r = promisor_remote_lookup(remote_name, NULL);
+               r = promisor_remote_lookup(config, remote_name, NULL);
                if (!r)
-                       r = promisor_remote_new(remote_name);
+                       r = promisor_remote_new(config, remote_name);

                free(remote_name);

@@ -135,59 +140,63 @@ static int promisor_remote_config(const char *var, const char *value, void *data
        return 0;
 }

-static int initialized;
-
-static void promisor_remote_init(void)
+static void promisor_remote_init(struct repository *r)
 {
-       if (initialized)
+       struct promisor_remote_config *config;
+
+       if (r->promisor_remote_config)
                return;
-       initialized = 1;
+       config = r->promisor_remote_config =
+               xcalloc(sizeof(*r->promisor_remote_config), 1);
+       config->promisors_tail = &config->promisors;

-       git_config(promisor_remote_config, NULL);
+       git_config(promisor_remote_config, config);

-       if (repository_format_partial_clone) {
+       if (config->repository_format_partial_clone) {
                struct promisor_remote *o, *previous;

-               o = promisor_remote_lookup(repository_format_partial_clone,
+               o = promisor_remote_lookup(config,
+                                          config->repository_format_partial_clone,
                                           &previous);
                if (o)
-                       promisor_remote_move_to_tail(o, previous);
+                       promisor_remote_move_to_tail(config, o, previous);
                else
-                       promisor_remote_new(repository_format_partial_clone);
+                       promisor_remote_new(config, config->repository_format_partial_clone);
        }
 }

-static void promisor_remote_clear(void)
+static void promisor_remote_clear(struct promisor_remote_config *config)
 {
-       while (promisors) {
-               struct promisor_remote *r = promisors;
-               promisors = promisors->next;
+       while (config->promisors) {
+               struct promisor_remote *r = config->promisors;
+               config->promisors = config->promisors->next;
                free(r);
        }

-       promisors_tail = &promisors;
+       config->promisors_tail = &config->promisors;
 }

-void promisor_remote_reinit(void)
+void repo_promisor_remote_reinit(struct repository *r)
 {
-       initialized = 0;
-       promisor_remote_clear();
-       promisor_remote_init();
+       promisor_remote_clear(r->promisor_remote_config);
+       FREE_AND_NULL(r->promisor_remote_config);
+       promisor_remote_init(r);
 }

-struct promisor_remote *promisor_remote_find(const char *remote_name)
+struct promisor_remote *repo_promisor_remote_find(struct repository *r,
+                                                 const char *remote_name)
 {
-       promisor_remote_init();
+       promisor_remote_init(r);

        if (!remote_name)
-               return promisors;
+               return r->promisor_remote_config->promisors;

-       return promisor_remote_lookup(remote_name, NULL);
+       return promisor_remote_lookup(r->promisor_remote_config, remote_name, NULL);
 }

-int has_promisor_remote(void)
+int repo_has_promisor_remote(struct repository *r)
 {
-       return !!promisor_remote_find(NULL);
+       return !!repo_promisor_remote_find(r, NULL);
 }

 static int remove_fetched_oids(struct repository *repo,

@@ -235,9 +244,11 @@ int promisor_remote_get_direct(struct repository *repo,
        if (oid_nr == 0)
                return 0;

-       promisor_remote_init();
+       promisor_remote_init(repo);

-       for (r = promisors; r; r = r->next) {
+       if (repo != the_repository)
+               BUG("only the_repository is supported for now");
+       for (r = repo->promisor_remote_config->promisors; r; r = r->next) {
                if (fetch_objects(r->name, remaining_oids, remaining_nr) < 0) {
                        if (remaining_nr == 1)
                                continue;

diff --git a/promisor-remote.h b/promisor-remote.h
index 687210ab87..5390d3e7bf 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h

@@ -17,9 +17,23 @@ struct promisor_remote {
        const char name[FLEX_ARRAY];
 };

-void promisor_remote_reinit(void);
-struct promisor_remote *promisor_remote_find(const char *remote_name);
-int has_promisor_remote(void);
+void repo_promisor_remote_reinit(struct repository *r);
+static inline void promisor_remote_reinit(void)
+{
+       repo_promisor_remote_reinit(the_repository);
+}
+
+struct promisor_remote *repo_promisor_remote_find(struct repository *r, const char *remote_name);
+static inline struct promisor_remote *promisor_remote_find(const char *remote_name)
+{
+       return repo_promisor_remote_find(the_repository, remote_name);
+}
+
+int repo_has_promisor_remote(struct repository *r);
+static inline int has_promisor_remote(void)
+{
+       return repo_has_promisor_remote(the_repository);
+}

 /*
  * Fetches all requested objects from all promisor remotes, trying them one at

diff --git a/repository.h b/repository.h
index a45f7520fd..fc06c154e2 100644
--- a/repository.h
+++ b/repository.h

@@ -10,6 +10,7 @@ struct lock_file;
 struct pathspec;
 struct raw_object_store;
 struct submodule_cache;
+struct promisor_remote_config;

 enum untracked_cache_setting {
        UNTRACKED_CACHE_UNSET = -1,

@@ -139,6 +140,9 @@ struct repository {
        /* True if commit-graph has been disabled within this process. */
        int commit_graph_disabled;

+       /* Configurations related to promisor remotes. */
+       struct promisor_remote_config *promisor_remote_config;
+
        /* Configurations */

        /* Indicate if a repository has a different 'commondir' from 'gitdir' */
--

2.32.0.rc0.204.g9fa02ecfa5-goog

diff --git a/t/helper/test-partial-clone.c b/t/helper/test-partial-clone.c
new file mode 100644
index 0000000000..e7bc7eb21f
--- /dev/null
+++ b/t/helper/test-partial-clone.c

@@ -0,0 +1,34 @@
+#include "cache.h"
+#include "test-tool.h"
+#include "repository.h"
+#include "object-store.h"
+
+static void object_info(const char *gitdir, const char *oid_hex)
+{
+	struct repository r;
+	struct object_id oid;
+	unsigned long size;
+	struct object_info oi = {.sizep = &size};
+	const char *p;
+
+	if (repo_init(&r, gitdir, NULL))
+		die("could not init repo");
+	if (parse_oid_hex(oid_hex, &oid, &p))
+		die("could not parse oid");
+	if (oid_object_info_extended(&r, &oid, &oi, 0))
+		die("could not obtain object info");
+	printf("%d\n", (int) size);
+}

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 584a039b85..e804d267e6 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh

@@ -604,6 +604,30 @@ test_expect_success 'do not fetch when checking existence of tree we construct o
 	git -C repo cherry-pick side1
 '

+test_expect_success 'lazy-fetch when accessing object not in the_repository' '
+	rm -rf full partial.git &&
+	test_create_repo full &&
+	printf 12345 >full/file.txt &&
+	git -C full add file.txt &&
+	git -C full commit -m "first commit" &&

This is one step towards supporting partial clone submodules.

Even after this patch, we will still lack partial clone submodules
support, primarily because a lot of Git code that accesses submodule
objects does so by adding their object stores as alternates, meaning
that any lazy fetches that would occur in the submodule would be done
based on the config of the superproject, not of the submodule. This also
prevents testing of the functionality in this patch by user-facing
commands. So for now, test this mechanism using a test helper.

Signed-off-by: Jonathan Tan <redacted>
---
 Makefile                      |  1 +
 object-file.c                 |  7 ++-----
 promisor-remote.c             | 14 +++++++++-----
 t/helper/test-partial-clone.c | 34 ++++++++++++++++++++++++++++++++++
 t/helper/test-tool.c          |  1 +
 t/helper/test-tool.h          |  1 +
 t/t0410-partial-clone.sh      | 24 ++++++++++++++++++++++++
 7 files changed, 72 insertions(+), 10 deletions(-)
 create mode 100644 t/helper/test-partial-clone.c

diff --git a/Makefile b/Makefile
index c3565fc0f8..f6653bcd5e 100644
--- a/Makefile
+++ b/Makefile

@@ -725,6 +725,7 @@ TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
 TEST_BUILTINS_OBJS += test-parse-options.o
 TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
+TEST_BUILTINS_OBJS += test-partial-clone.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-pcre2-config.o
 TEST_BUILTINS_OBJS += test-pkt-line.o

diff --git a/object-file.c b/object-file.c
index f233b440b2..ebf273e9e7 100644
--- a/object-file.c
+++ b/object-file.c

@@ -1570,15 +1570,12 @@ static int do_oid_object_info_extended(struct repository *r,
                }

                /* Check if it is a missing object */
-               if (fetch_if_missing && has_promisor_remote() &&
-                   !already_retried && r == the_repository &&
+               if (fetch_if_missing && repo_has_promisor_remote(r) &&
+                   !already_retried &&

                         */
                        promisor_remote_get_direct(r, real, 1);
                        already_retried = 1;

diff --git a/promisor-remote.c b/promisor-remote.c
index 5819d2cf28..1601f05d79 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c

@@ -11,7 +11,8 @@ struct promisor_remote_config {
        struct promisor_remote **promisors_tail;
 };

-static int fetch_objects(const char *remote_name,
+static int fetch_objects(struct repository *repo,
+                        const char *remote_name,
                         const struct object_id *oids,
                         int oid_nr)
 {

@@ -21,6 +22,11 @@ static int fetch_objects(const char *remote_name,

        child.git_cmd = 1;
        child.in = -1;
+       if (repo != the_repository) {
+               prepare_other_repo_env(&child.env_array);
+               strvec_pushf(&child.env_array, "%s=%s", GIT_DIR_ENVIRONMENT,
+                            repo->gitdir);
+       }
        strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop",
                     "fetch", remote_name, "--no-tags",
                     "--no-write-fetch-head", "--recurse-submodules=no",

@@ -150,7 +156,7 @@ static void promisor_remote_init(struct repository *r)
                xcalloc(sizeof(*r->promisor_remote_config), 1);
        config->promisors_tail = &config->promisors;

-       git_config(promisor_remote_config, config);
+       repo_config(r, promisor_remote_config, config);

        if (config->repository_format_partial_clone) {
                struct promisor_remote *o, *previous;

@@ -246,10 +252,8 @@ int promisor_remote_get_direct(struct repository *repo,

        promisor_remote_init(repo);

-       if (repo != the_repository)
-               BUG("only the_repository is supported for now");
        for (r = repo->promisor_remote_config->promisors; r; r = r->next) {
-               if (fetch_objects(r->name, remaining_oids, remaining_nr) < 0) {
+               if (fetch_objects(repo, r->name, remaining_oids, remaining_nr) < 0) {
                        if (remaining_nr == 1)
                                continue;
                        remaining_nr = remove_fetched_oids(repo, &remaining_oids,

diff --git a/t/helper/test-partial-clone.c b/t/helper/test-partial-clone.c
new file mode 100644
index 0000000000..e7bc7eb21f
--- /dev/null
+++ b/t/helper/test-partial-clone.c

@@ -0,0 +1,34 @@
+#include "cache.h"
+#include "test-tool.h"
+#include "repository.h"
+#include "object-store.h"
+
+static void object_info(const char *gitdir, const char *oid_hex)
+{
+       struct repository r;
+       struct object_id oid;
+       unsigned long size;
+       struct object_info oi = {.sizep = &size};
+       const char *p;
+
+       if (repo_init(&r, gitdir, NULL))
+               die("could not init repo");
+       if (parse_oid_hex(oid_hex, &oid, &p))
+               die("could not parse oid");
+       if (oid_object_info_extended(&r, &oid, &oi, 0))
+               die("could not obtain object info");
+       printf("%d\n", (int) size);
+}
+
+int cmd__partial_clone(int argc, const char **argv)
+{
+       if (argc < 4)
+               die("too few arguments");
+
+       if (!strcmp(argv[1], "object-info"))
+               object_info(argv[2], argv[3]);
+       else
+               die("invalid argument '%s'", argv[1]);
+
+       return 0;
+}

diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index c5bd0c6d4c..b21e8f1519 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c

@@ -46,6 +46,7 @@ static struct test_cmd cmds[] = {
        { "online-cpus", cmd__online_cpus },
        { "parse-options", cmd__parse_options },
        { "parse-pathspec-file", cmd__parse_pathspec_file },
+       { "partial-clone", cmd__partial_clone },
        { "path-utils", cmd__path_utils },
        { "pcre2-config", cmd__pcre2_config },
        { "pkt-line", cmd__pkt_line },

diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index e8069a3b22..f845ced4b3 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h

@@ -35,6 +35,7 @@ int cmd__oidmap(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
 int cmd__parse_pathspec_file(int argc, const char** argv);
+int cmd__partial_clone(int argc, const char **argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__pcre2_config(int argc, const char **argv);
 int cmd__pkt_line(int argc, const char **argv);

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 584a039b85..e804d267e6 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh

@@ -604,6 +604,30 @@ test_expect_success 'do not fetch when checking existence of tree we construct o
        git -C repo cherry-pick side1
 '

+test_expect_success 'lazy-fetch when accessing object not in the_repository' '
+       rm -rf full partial.git &&
+       test_create_repo full &&
+       printf 12345 >full/file.txt &&
+       git -C full add file.txt &&
+       git -C full commit -m "first commit" &&
+
+       test_config -C full uploadpack.allowfilter 1 &&
+       test_config -C full uploadpack.allowanysha1inwant 1 &&
+       git clone --filter=blob:none --bare "file://$(pwd)/full" partial.git &&
+       FILE_HASH=$(git hash-object --stdin <full/file.txt) &&
+
+       # Sanity check that the file is missing
+       git -C partial.git rev-list --objects --missing=print HEAD >out &&
+       grep "[?]$FILE_HASH" out &&
+
+       OUT=$(test-tool partial-clone object-info partial.git "$FILE_HASH") &&
+       test "$OUT" -eq 5 &&
+
+       # Sanity check that the file is now present
+       git -C partial.git rev-list --objects --missing=print HEAD >out &&
+       ! grep "[?]$FILE_HASH" out
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd

--

2.32.0.rc0.204.g9fa02ecfa5-goog