From 79543e760d0219063ab21f687e7538c3e89b2e99 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:00:41 +0100 Subject: [PATCH 1/7] setup: extract function to create the refdb We're about to let callers skip creation of the reference database when calling `init_db()`. Extract the logic into a standalone function so that it becomes easier to do this refactoring. While at it, expand the comment that explains why we always create the "refs/" directory. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- setup.c | 103 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 38 deletions(-) diff --git a/setup.c b/setup.c index fc592dc6dd5bf3..865cfe6743e2dd 100644 --- a/setup.c +++ b/setup.c @@ -1885,6 +1885,68 @@ void initialize_repository_version(int hash_algo, int reinit) git_config_set_gently("extensions.objectformat", NULL); } +static int is_reinit(void) +{ + struct strbuf buf = STRBUF_INIT; + char junk[2]; + int ret; + + git_path_buf(&buf, "HEAD"); + ret = !access(buf.buf, R_OK) || readlink(buf.buf, junk, sizeof(junk) - 1) != -1; + strbuf_release(&buf); + return ret; +} + +static void create_reference_database(const char *initial_branch, int quiet) +{ + struct strbuf err = STRBUF_INIT; + int reinit = is_reinit(); + + /* + * We need to create a "refs" dir in any case so that older versions of + * Git can tell that this is a repository. This serves two main purposes: + * + * - Clients will know to stop walking the parent-directory chain when + * detecting the Git repository. Otherwise they may end up detecting + * a Git repository in a parent directory instead. + * + * - Instead of failing to detect a repository with unknown reference + * format altogether, old clients will print an error saying that + * they do not understand the reference format extension. + */ + safe_create_dir(git_path("refs"), 1); + adjust_shared_perm(git_path("refs")); + + if (refs_init_db(&err)) + die("failed to set up refs db: %s", err.buf); + + /* + * Point the HEAD symref to the initial branch with if HEAD does + * not yet exist. + */ + if (!reinit) { + char *ref; + + if (!initial_branch) + initial_branch = git_default_branch_name(quiet); + + ref = xstrfmt("refs/heads/%s", initial_branch); + if (check_refname_format(ref, 0) < 0) + die(_("invalid initial branch name: '%s'"), + initial_branch); + + if (create_symref("HEAD", ref, NULL) < 0) + exit(1); + free(ref); + } + + if (reinit && initial_branch) + warning(_("re-init: ignored --initial-branch=%s"), + initial_branch); + + strbuf_release(&err); +} + static int create_default_files(const char *template_path, const char *original_git_dir, const char *initial_branch, @@ -1896,10 +1958,8 @@ static int create_default_files(const char *template_path, struct stat st1; struct strbuf buf = STRBUF_INIT; char *path; - char junk[2]; int reinit; int filemode; - struct strbuf err = STRBUF_INIT; const char *init_template_dir = NULL; const char *work_tree = get_git_work_tree(); @@ -1919,6 +1979,8 @@ static int create_default_files(const char *template_path, reset_shared_repository(); git_config(git_default_config, NULL); + reinit = is_reinit(); + /* * We must make sure command-line options continue to override any * values we might have just re-read from the config. @@ -1962,39 +2024,7 @@ static int create_default_files(const char *template_path, adjust_shared_perm(get_git_dir()); } - /* - * We need to create a "refs" dir in any case so that older - * versions of git can tell that this is a repository. - */ - safe_create_dir(git_path("refs"), 1); - adjust_shared_perm(git_path("refs")); - - if (refs_init_db(&err)) - die("failed to set up refs db: %s", err.buf); - - /* - * Point the HEAD symref to the initial branch with if HEAD does - * not yet exist. - */ - path = git_path_buf(&buf, "HEAD"); - reinit = (!access(path, R_OK) - || readlink(path, junk, sizeof(junk)-1) != -1); - if (!reinit) { - char *ref; - - if (!initial_branch) - initial_branch = git_default_branch_name(quiet); - - ref = xstrfmt("refs/heads/%s", initial_branch); - if (check_refname_format(ref, 0) < 0) - die(_("invalid initial branch name: '%s'"), - initial_branch); - - if (create_symref("HEAD", ref, NULL) < 0) - exit(1); - free(ref); - } - + create_reference_database(initial_branch, quiet); initialize_repository_version(fmt->hash_algo, 0); /* Check filemode trustability */ @@ -2158,9 +2188,6 @@ int init_db(const char *git_dir, const char *real_git_dir, prev_bare_repository, init_shared_repository, flags & INIT_DB_QUIET); - if (reinit && initial_branch) - warning(_("re-init: ignored --initial-branch=%s"), - initial_branch); create_object_directory(); From 56cd0334f7664feffcd8ed8f8f0d3929374c69e0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:00:46 +0100 Subject: [PATCH 2/7] setup: allow skipping creation of the refdb Allow callers to skip creation of the reference database via a new flag `INIT_DB_SKIP_REFDB`, which is required for git-clone(1) so that we can create it at a later point once the object format has been discovered from the remote repository. Note that we also uplift the call to `create_reference_database()` into `init_db()`, which makes it easier to handle the new flag for us. This changes the order in which we do initialization so that we now set up the Git configuration before we create the reference database. In practice this move should not result in any change in behaviour. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- setup.c | 13 +++++-------- setup.h | 5 +++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/setup.c b/setup.c index 865cfe6743e2dd..d6a1c59b7bd915 100644 --- a/setup.c +++ b/setup.c @@ -1949,11 +1949,9 @@ static void create_reference_database(const char *initial_branch, int quiet) static int create_default_files(const char *template_path, const char *original_git_dir, - const char *initial_branch, const struct repository_format *fmt, int prev_bare_repository, - int init_shared_repository, - int quiet) + int init_shared_repository) { struct stat st1; struct strbuf buf = STRBUF_INIT; @@ -2024,7 +2022,6 @@ static int create_default_files(const char *template_path, adjust_shared_perm(get_git_dir()); } - create_reference_database(initial_branch, quiet); initialize_repository_version(fmt->hash_algo, 0); /* Check filemode trustability */ @@ -2184,11 +2181,11 @@ int init_db(const char *git_dir, const char *real_git_dir, validate_hash_algorithm(&repo_fmt, hash); reinit = create_default_files(template_dir, original_git_dir, - initial_branch, &repo_fmt, - prev_bare_repository, - init_shared_repository, - flags & INIT_DB_QUIET); + &repo_fmt, prev_bare_repository, + init_shared_repository); + if (!(flags & INIT_DB_SKIP_REFDB)) + create_reference_database(initial_branch, flags & INIT_DB_QUIET); create_object_directory(); if (get_shared_repository()) { diff --git a/setup.h b/setup.h index b48cf1c43b5a55..cbf538286bfefc 100644 --- a/setup.h +++ b/setup.h @@ -169,8 +169,9 @@ int verify_repository_format(const struct repository_format *format, */ void check_repository_format(struct repository_format *fmt); -#define INIT_DB_QUIET 0x0001 -#define INIT_DB_EXIST_OK 0x0002 +#define INIT_DB_QUIET (1 << 0) +#define INIT_DB_EXIST_OK (1 << 1) +#define INIT_DB_SKIP_REFDB (1 << 2) int init_db(const char *git_dir, const char *real_git_dir, const char *template_dir, int hash_algo, From bab2283ec69f46a5c181f6d8131ded6da4d449c5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:00:50 +0100 Subject: [PATCH 3/7] remote-curl: rediscover repository when fetching refs The reftable format encodes the hash function used by the repository inside of its tables. The reftable backend thus needs to be initialized with the correct hash function right from the start, or otherwise we may end up writing tables with the wrong hash function. But git-clone(1) initializes the reference database before learning about the hash function used by the remote repository, which has never been a problem with the reffiles backend. To fix this, we'll have to change git-clone(1) to be more careful and only create the reference backend once it learned about the remote hash function. This creates a problem for git-remote-curl(1), which will then be spawned at a time where the repository is not yet fully-initialized. Consequentially, git-remote-curl(1) will fail to detect the repository, which eventually causes it to error out once it is asked to fetch remote objects. We can address this issue by trying to re-discover the Git repository in case none was detected at startup time. With this change, the clone will look as following: 1. git-clone(1) sets up the initial repository, excluding the reference database. 2. git-clone(1) spawns git-remote-curl(1), which will be unable to detect the repository due to a missing "HEAD". 3. git-clone(1) asks git-remote-curl(1) to list remote references. This works just fine as this step does not require a local repository 4. git-clone(1) creates the reference database as it has now learned about the hash function. 5. git-clone(1) asks git-remote-curl(1) to fetch the remote packfile. The latter notices that it doesn't have a repository available, but it now knows to try and re-discover it. If the re-discovery succeeds in the last step we can continue with the clone. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- remote-curl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index ef05752ca5738e..fc29757b654c93 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -1564,8 +1564,11 @@ int cmd_main(int argc, const char **argv) if (buf.len == 0) break; if (starts_with(buf.buf, "fetch ")) { - if (nongit) - die(_("remote-curl: fetch attempted without a local repo")); + if (nongit) { + setup_git_directory_gently(&nongit); + if (nongit) + die(_("remote-curl: fetch attempted without a local repo")); + } parse_fetch(&buf); } else if (!strcmp(buf.buf, "list") || starts_with(buf.buf, "list ")) { From 91590293298c132e427851616e660e781596c0d8 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:00:54 +0100 Subject: [PATCH 4/7] builtin/clone: fix bundle URIs with mismatching object formats We create the reference database in git-clone(1) quite early before connecting to the remote repository. Given that we do not yet know about the object format that the remote repository uses at that point in time the consequence is that the refdb may be initialized with the wrong object format. This is not a problem in the context of the files backend as we do not encode the object format anywhere, and furthermore the only reference that we write between initializing the refdb and learning about the object format is the "HEAD" symref. It will become a problem though once we land the reftable backend, which indeed does require to know about the proper object format at the time of creation. We thus need to rearrange the logic in git-clone(1) so that we only initialize the refdb once we have learned about the actual object format. As a first step, move listing of remote references to happen earlier, which also allow us to set up the hash algorithm of the repository earlier now. While we aim to execute this logic as late as possible until after most of the setup has happened already, detection of the object format and thus later the setup of the reference database must happen before any other logic that may spawn Git commands or otherwise these Git commands may not recognize the repository as such. The first Git step where we expect the repository to be fully initalized is when we fetch bundles via bundle URIs. Funny enough, the comments there also state that "the_repository must match the cloned repo", which is indeed not necessarily the case for the hash algorithm right now. So in practice it is the right thing to detect the remote's object format before downloading bundle URIs anyway, and not doing so causes clones with bundle URIs to fail when the local default object format does not match the remote repository's format. Unfortunately though, this creates a new issue: downloading bundles may take a long time, so if we list refs beforehand they might've grown stale meanwhile. It is not clear how to solve this issue except for a second reference listing though after we have downloaded the bundles, which may be an expensive thing to do. Arguably though, it's preferable to have a staleness issue compared to being unable to clone a repository altogether. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 48 ++++++++++++++++++------------------- t/t5558-clone-bundle-uri.sh | 18 ++++++++++++++ 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index c6357af949895a..d1886508819813 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1266,6 +1266,26 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (transport->smart_options && !deepen && !filter_options.choice) transport->smart_options->check_self_contained_and_connected = 1; + strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD"); + refspec_ref_prefixes(&remote->fetch, + &transport_ls_refs_options.ref_prefixes); + if (option_branch) + expand_ref_prefix(&transport_ls_refs_options.ref_prefixes, + option_branch); + if (!option_no_tags) + strvec_push(&transport_ls_refs_options.ref_prefixes, + "refs/tags/"); + + refs = transport_get_remote_refs(transport, &transport_ls_refs_options); + + /* + * Now that we know what algorithm the remote side is using, let's set + * ours to the same thing. + */ + hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport)); + initialize_repository_version(hash_algo, 1); + repo_set_hash_algo(the_repository, hash_algo); + /* * Before fetching from the remote, download and install bundle * data from the --bundle-uri option. @@ -1281,24 +1301,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) bundle_uri); else if (has_heuristic) git_config_set_gently("fetch.bundleuri", bundle_uri); - } - - strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD"); - refspec_ref_prefixes(&remote->fetch, - &transport_ls_refs_options.ref_prefixes); - if (option_branch) - expand_ref_prefix(&transport_ls_refs_options.ref_prefixes, - option_branch); - if (!option_no_tags) - strvec_push(&transport_ls_refs_options.ref_prefixes, - "refs/tags/"); - - refs = transport_get_remote_refs(transport, &transport_ls_refs_options); - - if (refs) - mapped_refs = wanted_peer_refs(refs, &remote->fetch); - - if (!bundle_uri) { + } else { /* * Populate transport->got_remote_bundle_uri and * transport->bundle_uri. We might get nothing. @@ -1319,13 +1322,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } } - /* - * Now that we know what algorithm the remote side is using, - * let's set ours to the same thing. - */ - hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport)); - initialize_repository_version(hash_algo, 1); - repo_set_hash_algo(the_repository, hash_algo); + if (refs) + mapped_refs = wanted_peer_refs(refs, &remote->fetch); if (mapped_refs) { /* diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh index 996a08e90c9c2c..1ca5f745e73bd3 100755 --- a/t/t5558-clone-bundle-uri.sh +++ b/t/t5558-clone-bundle-uri.sh @@ -33,6 +33,15 @@ test_expect_success 'clone with path bundle' ' test_cmp expect actual ' +test_expect_success 'clone with path bundle and non-default hash' ' + test_when_finished "rm -rf clone-path-non-default-hash" && + GIT_DEFAULT_HASH=sha256 git clone --bundle-uri="clone-from/B.bundle" \ + clone-from clone-path-non-default-hash && + git -C clone-path-non-default-hash rev-parse refs/bundles/topic >actual && + git -C clone-from rev-parse topic >expect && + test_cmp expect actual +' + test_expect_success 'clone with file:// bundle' ' git clone --bundle-uri="file://$(pwd)/clone-from/B.bundle" \ clone-from clone-file && @@ -284,6 +293,15 @@ test_expect_success 'clone HTTP bundle' ' test_config -C clone-http log.excludedecoration refs/bundle/ ' +test_expect_success 'clone HTTP bundle with non-default hash' ' + test_when_finished "rm -rf clone-http-non-default-hash" && + GIT_DEFAULT_HASH=sha256 git clone --bundle-uri="$HTTPD_URL/B.bundle" \ + "$HTTPD_URL/smart/fetch.git" clone-http-non-default-hash && + git -C clone-http-non-default-hash rev-parse refs/bundles/topic >actual && + git -C clone-from rev-parse topic >expect && + test_cmp expect actual +' + test_expect_success 'clone bundle list (HTTP, no heuristic)' ' test_when_finished rm -f trace*.txt && From 360822a3471f779c761694b11f30ff738c6851e9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:00:59 +0100 Subject: [PATCH 5/7] builtin/clone: set up sparse checkout later When asked to do a sparse checkout, then git-clone(1) will spawn `git sparse-checkout set` to set up the configuration accordingly. This requires a proper Git repository or otherwise the command will fail. But as we are about to move creation of the reference database to a later point, this prerequisite will not hold anymore. Move the logic to a later point in time where we know to have created the reference database already. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index d1886508819813..9c60923f312546 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1185,9 +1185,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); - if (option_sparse_checkout && git_sparse_checkout_init(dir)) - return 1; - remote = remote_get(remote_name); refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix, @@ -1426,6 +1423,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) dissociate_from_references(); } + if (option_sparse_checkout && git_sparse_checkout_init(dir)) + return 1; + junk_mode = JUNK_LEAVE_REPO; err = checkout(submodule_progress, filter_submodules); From 3c8f60c6416bdfcc690ff5d56d80350a89ed92b5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:01:03 +0100 Subject: [PATCH 6/7] builtin/clone: skip reading HEAD when retrieving remote After we have set up the remote configuration in git-clone(1) we'll call `remote_get()` to read the remote from the on-disk configuration. But next to reading the on-disk configuration, `remote_get()` will also cause us to try and read the repository's HEAD reference so that we can figure out the current branch. Besides being pointless in git-clone(1) because we're operating in an empty repository anyway, this will also break once we move creation of the reference database to a later point in time. Refactor the code to introduce a new `remote_get_early()` function that will skip reading the HEAD reference to address this issue. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 2 +- remote.c | 26 ++++++++++++++++---------- remote.h | 1 + 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 9c60923f312546..06966c5d4c25c5 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1185,7 +1185,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); - remote = remote_get(remote_name); + remote = remote_get_early(remote_name); refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix, branch_top.buf); diff --git a/remote.c b/remote.c index abb24822beb2a1..051d0a64a0b962 100644 --- a/remote.c +++ b/remote.c @@ -509,7 +509,7 @@ static void alias_all_urls(struct remote_state *remote_state) } } -static void read_config(struct repository *repo) +static void read_config(struct repository *repo, int early) { int flag; @@ -518,7 +518,7 @@ static void read_config(struct repository *repo) repo->remote_state->initialized = 1; repo->remote_state->current_branch = NULL; - if (startup_info->have_repository) { + if (startup_info->have_repository && !early) { const char *head_ref = refs_resolve_ref_unsafe( get_main_ref_store(repo), "HEAD", 0, NULL, &flag); if (head_ref && (flag & REF_ISSYMREF) && @@ -561,7 +561,7 @@ static const char *remotes_remote_for_branch(struct remote_state *remote_state, const char *remote_for_branch(struct branch *branch, int *explicit) { - read_config(the_repository); + read_config(the_repository, 0); die_on_missing_branch(the_repository, branch); return remotes_remote_for_branch(the_repository->remote_state, branch, @@ -587,7 +587,7 @@ remotes_pushremote_for_branch(struct remote_state *remote_state, const char *pushremote_for_branch(struct branch *branch, int *explicit) { - read_config(the_repository); + read_config(the_repository, 0); die_on_missing_branch(the_repository, branch); return remotes_pushremote_for_branch(the_repository->remote_state, @@ -599,7 +599,7 @@ static struct remote *remotes_remote_get(struct remote_state *remote_state, const char *remote_ref_for_branch(struct branch *branch, int for_push) { - read_config(the_repository); + read_config(the_repository, 0); die_on_missing_branch(the_repository, branch); if (branch) { @@ -709,7 +709,13 @@ remotes_remote_get(struct remote_state *remote_state, const char *name) struct remote *remote_get(const char *name) { - read_config(the_repository); + read_config(the_repository, 0); + return remotes_remote_get(the_repository->remote_state, name); +} + +struct remote *remote_get_early(const char *name) +{ + read_config(the_repository, 1); return remotes_remote_get(the_repository->remote_state, name); } @@ -722,7 +728,7 @@ remotes_pushremote_get(struct remote_state *remote_state, const char *name) struct remote *pushremote_get(const char *name) { - read_config(the_repository); + read_config(the_repository, 0); return remotes_pushremote_get(the_repository->remote_state, name); } @@ -738,7 +744,7 @@ int remote_is_configured(struct remote *remote, int in_repo) int for_each_remote(each_remote_fn fn, void *priv) { int i, result = 0; - read_config(the_repository); + read_config(the_repository, 0); for (i = 0; i < the_repository->remote_state->remotes_nr && !result; i++) { struct remote *remote = @@ -1831,7 +1837,7 @@ struct branch *branch_get(const char *name) { struct branch *ret; - read_config(the_repository); + read_config(the_repository, 0); if (!name || !*name || !strcmp(name, "HEAD")) ret = the_repository->remote_state->current_branch; else @@ -1973,7 +1979,7 @@ static const char *branch_get_push_1(struct remote_state *remote_state, const char *branch_get_push(struct branch *branch, struct strbuf *err) { - read_config(the_repository); + read_config(the_repository, 0); die_on_missing_branch(the_repository, branch); if (!branch) diff --git a/remote.h b/remote.h index cdc8b1db42c4a8..79353ba2265768 100644 --- a/remote.h +++ b/remote.h @@ -118,6 +118,7 @@ struct remote { * and configuration. */ struct remote *remote_get(const char *name); +struct remote *remote_get_early(const char *name); struct remote *pushremote_get(const char *name); int remote_is_configured(struct remote *remote, int in_repo); From 18c9cb7524c13ca88ee334a707f281c2e80d5fdf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 12 Dec 2023 08:01:07 +0100 Subject: [PATCH 7/7] builtin/clone: create the refdb with the correct object format We're currently creating the reference database with a potentially incorrect object format when the remote repository's object format is different from the local default object format. This works just fine for now because the files backend never records the object format anywhere. But this logic will fail with any new reference backend that encodes this information in some form either on-disk or in-memory. The preceding commits have reshuffled code in git-clone(1) so that there is no code path that will access the reference database before we have detected the remote's object format. With these refactorings we can now defer initialization of the reference database until after we have learned the remote's object format and thus initialize it with the correct format from the get-go. These refactorings are required to make git-clone(1) work with the upcoming reftable backend when cloning repositories with the SHA256 object format. This change breaks a test in "t5550-http-fetch-dumb.sh" when cloning an empty repository with `GIT_TEST_DEFAULT_HASH=sha256`. The test expects the resulting hash format of the empty cloned repository to match the default hash, but now we always end up with a sha1 repository. The problem is that for dumb HTTP fetches, we have no easy way to figure out the remote's hash function except for deriving it based on the hash length of refs in `info/refs`. But as the remote repository is empty we cannot rely on this detection mechanism. Before the change in this commit we already initialized the repository with the default hash function and then left it as-is. With this patch we always use the hash function detected via the remote, where we fall back to "sha1" in case we cannot detect it. Neither the old nor the new behaviour are correct as we second-guess the remote hash function in both cases. But given that this is a rather unlikely edge case (we use the dumb HTTP protocol, the remote repository uses SHA256 and the remote repository is empty), let's simply adapt the test to assert the new behaviour. If we want to properly address this edge case in the future we will have to extend the dumb HTTP protocol so that we can properly detect the hash function for empty repositories. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 9 ++++++++- setup.c | 2 +- setup.h | 1 + t/t5550-http-fetch-dumb.sh | 4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 06966c5d4c25c5..fd052b8b543729 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1097,8 +1097,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } } + /* + * Initialize the repository, but skip initializing the reference + * database. We do not yet know about the object format of the + * repository, and reference backends may persist that information into + * their on-disk data structures. + */ init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL, - do_not_override_repo_unix_permissions, INIT_DB_QUIET); + do_not_override_repo_unix_permissions, INIT_DB_QUIET | INIT_DB_SKIP_REFDB); if (real_git_dir) { free((char *)git_dir); @@ -1282,6 +1288,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport)); initialize_repository_version(hash_algo, 1); repo_set_hash_algo(the_repository, hash_algo); + create_reference_database(NULL, 1); /* * Before fetching from the remote, download and install bundle diff --git a/setup.c b/setup.c index d6a1c59b7bd915..155fe13f701c7a 100644 --- a/setup.c +++ b/setup.c @@ -1897,7 +1897,7 @@ static int is_reinit(void) return ret; } -static void create_reference_database(const char *initial_branch, int quiet) +void create_reference_database(const char *initial_branch, int quiet) { struct strbuf err = STRBUF_INIT; int reinit = is_reinit(); diff --git a/setup.h b/setup.h index cbf538286bfefc..3f0f17c351cbc5 100644 --- a/setup.h +++ b/setup.h @@ -178,6 +178,7 @@ int init_db(const char *git_dir, const char *real_git_dir, const char *initial_branch, int init_shared_repository, unsigned int flags); void initialize_repository_version(int hash_algo, int reinit); +void create_reference_database(const char *initial_branch, int quiet); /* * NOTE NOTE NOTE!! diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index e444b30bf61568..4c3b32785d580f 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -66,11 +66,11 @@ test_expect_success 'create empty remote repository' ' setup_post_update_server_info_hook "$HTTPD_DOCUMENT_ROOT_PATH/empty.git" ' -test_expect_success 'empty dumb HTTP repository has default hash algorithm' ' +test_expect_success 'empty dumb HTTP repository falls back to SHA1' ' test_when_finished "rm -fr clone-empty" && git clone $HTTPD_URL/dumb/empty.git clone-empty && git -C clone-empty rev-parse --show-object-format >empty-format && - test "$(cat empty-format)" = "$(test_oid algo)" + test "$(cat empty-format)" = sha1 ' setup_askpass_helper