From 1e1442f12316f981f7051e341a2c91dec754dc1e Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 23 Jun 2022 21:55:07 -0400 Subject: [PATCH] clone: set fetch.bundleURI if bundle.forFetch is advertised TODO: change forFetch local to a 'flags' value. NEEDSWORK: we are downloading too many bundles. Signed-off-by: Derrick Stolee --- Documentation/config/fetch.txt | 8 ++ builtin/clone.c | 6 +- builtin/fetch.c | 5 + bundle-uri.c | 13 ++- bundle-uri.h | 11 ++- t/t5558-clone-bundle-uri.sh | 166 +++++++++++++++++++++++++++++++++ 6 files changed, 206 insertions(+), 3 deletions(-) diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index cd65d236b43ffc..4f796218aab361 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -96,3 +96,11 @@ fetch.writeCommitGraph:: merge and the write may take longer. Having an updated commit-graph file helps performance of many Git commands, including `git merge-base`, `git push -f`, and `git log --graph`. Defaults to false. + +fetch.bundleURI:: + This value stores a URI for fetching Git object data from a bundle URI + before performing an incremental fetch from the origin Git server. If + the value is `` then running `git fetch ` is equivalent to + first running `git fetch --bundle-uri=` immediately before + `git fetch `. See details of the `--bundle-uri` option in + linkgit:git-fetch[1]. diff --git a/builtin/clone.c b/builtin/clone.c index 78eb5ae16f34d2..570ce3aff82cf0 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1245,11 +1245,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix) * data from the --bundle-uri option. */ if (bundle_uri) { + enum bundle_list_flags flags = 0; + /* At this point, we need the_repository to match the cloned repo. */ repo_init(the_repository, git_dir, work_tree); - if (fetch_bundle_uri(the_repository, bundle_uri)) + if (fetch_bundle_uri(the_repository, bundle_uri, &flags)) warning(_("failed to fetch objects from bundle URI '%s'"), bundle_uri); + else if (flags & BUNDLE_FLAG_FOR_FETCH) + git_config_set_gently("fetch.bundleuri", bundle_uri); } strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD"); diff --git a/builtin/fetch.c b/builtin/fetch.c index fc5cecb48356ff..9a7d71e9ee7475 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -29,6 +29,7 @@ #include "commit-graph.h" #include "shallow.h" #include "worktree.h" +#include "bundle-uri.h" #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000) @@ -2084,6 +2085,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv, int cmd_fetch(int argc, const char **argv, const char *prefix) { int i; + const char *bundle_uri; struct string_list list = STRING_LIST_INIT_DUP; struct remote *remote = NULL; int result = 0; @@ -2169,6 +2171,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) if (dry_run) write_fetch_head = 0; + if (!git_config_get_string_tmp("fetch.bundleuri", &bundle_uri)) + result = fetch_bundle_uri(the_repository, bundle_uri, NULL); + if (all) { if (argc == 1) die(_("fetch --all does not take a repository argument")); diff --git a/bundle-uri.c b/bundle-uri.c index 2b998a3f8f94e6..0976973e6e5a12 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -151,6 +151,12 @@ static int bundle_list_update(const char *key, const char *value, return 0; } + if (!strcmp(subkey, "flag")) { + if (!strcmp("forFetch", value)) + list->flags |= BUNDLE_FLAG_FOR_FETCH; + return 0; + } + if (!strcmp(subkey, "heuristic")) { int i; for (i = 0; i < BUNDLE_HEURISTIC_COUNT; i++) { @@ -572,6 +578,8 @@ static int fetch_bundle_list_in_config_format(struct repository *r, goto cleanup; cleanup: + /* Copy results from the local list to the global list. */ + global_list->flags |= list_from_bundle.flags; clear_bundle_list(&list_from_bundle); return result; } @@ -697,7 +705,8 @@ static int unlink_bundle(struct remote_bundle_info *info, void *data) return 0; } -int fetch_bundle_uri(struct repository *r, const char *uri) +int fetch_bundle_uri(struct repository *r, const char *uri, + enum bundle_list_flags *flags) { int result; struct bundle_list list; @@ -717,6 +726,8 @@ int fetch_bundle_uri(struct repository *r, const char *uri) result = unbundle_all_bundles(r, &list); cleanup: + if (flags) + *flags = list.flags; for_all_bundles_in_list(&list, unlink_bundle, NULL); clear_bundle_list(&list); clear_remote_bundle_info(&bundle, NULL); diff --git a/bundle-uri.h b/bundle-uri.h index 7a9b67e3b0b8fc..901da1cdfd0f54 100644 --- a/bundle-uri.h +++ b/bundle-uri.h @@ -68,6 +68,10 @@ enum bundle_list_heuristic { BUNDLE_HEURISTIC_COUNT, }; +enum bundle_list_flags { + BUNDLE_FLAG_FOR_FETCH = (1 << 0), +}; + /** * A bundle_list contains an unordered set of remote_bundle_info structs, * as well as information about the bundle listing, such as version and @@ -76,6 +80,7 @@ enum bundle_list_heuristic { struct bundle_list { int version; enum bundle_list_mode mode; + enum bundle_list_flags flags; struct hashmap bundles; /** @@ -121,8 +126,12 @@ int bundle_uri_parse_config_format(const char *uri, * based on that information. * * Returns non-zero if no bundle information is found at the given 'uri'. + * + * The enum at 'flags' (if non-NULL) is populated with the flags found at + * the bundle URI. */ -int fetch_bundle_uri(struct repository *r, const char *uri); +int fetch_bundle_uri(struct repository *r, const char *uri, + enum bundle_list_flags *flags); /** * Given a bundle list that was already advertised (likely by the diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh index e97cd4f8835e6f..f3a2e0cba0e6d9 100755 --- a/t/t5558-clone-bundle-uri.sh +++ b/t/t5558-clone-bundle-uri.sh @@ -137,6 +137,89 @@ test_expect_success 'clone bundle list (file, creationToken)' ' done ' +test_expect_success 'clone with bundle.flag=forFetch creates fetch.bundleURI' ' + test_when_finished rm -rf fetch-file-3 trace1.txt trace2.txt && + cat >bundle-list <<-EOF && + [bundle] + version = 1 + mode = all + heuristic = creationToken + flag = forFetch + + [bundle "bundle-1"] + uri = clone-from/bundle-1.bundle + creationToken = 1 + EOF + + git clone --no-local --single-branch --branch=base \ + --bundle-uri="file://$(pwd)/bundle-list" \ + clone-from fetch-file-3 && + + test_cmp_config -C fetch-file-3 "file://$(pwd)/bundle-list" fetch.bundleuri && + + # only received base ref from bundle-1 + git -C fetch-file-3 for-each-ref --format="%(refname)" "refs/bundles/*" >refs && + cat >expect <<-\EOF && + refs/bundles/base + EOF + test_cmp expect refs && + + cat >>bundle-list <<-EOF && + [bundle "bundle-2"] + uri = clone-from/bundle-2.bundle + creationToken = 2 + EOF + + GIT_TRACE2_EVENT="$(pwd)/trace1.txt" \ + git -C fetch-file-3 fetch origin --no-tags \ + refs/heads/left:refs/heads/left && + + # This fetch should copy two files: the list and bundle-2. + grep "region_enter.*bundle-uri.*copy_file" trace1.txt >copies && + + # NEEDSWORK: This should only copy two files, not three. + test_line_count = 3 copies && + + # received left from bundle-2 + git -C fetch-file-3 for-each-ref --format="%(refname)" "refs/bundles/*" >refs && + cat >expect <<-\EOF && + refs/bundles/base + refs/bundles/left + EOF + test_cmp expect refs && + + cat >>bundle-list <<-EOF && + [bundle "bundle-3"] + uri = clone-from/bundle-3.bundle + creationToken = 3 + + [bundle "bundle-4"] + uri = clone-from/bundle-4.bundle + creationToken = 4 + EOF + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ + git -C fetch-file-3 fetch origin --no-tags \ + refs/heads/right:refs/heads/right && + + # This fetch should copy three files: the list, bundle-4, and bundle-3. + grep "region_enter.*bundle-uri.*copy_file" trace2.txt >copies && + + # NEEDSWORK: this is re-downloading all the bundles! + test_line_count = 5 copies && + + # received right and merge from bundle-3 and bundle-4. + git -C fetch-file-3 for-each-ref --format="%(refname)" "refs/bundles/*" >refs && + + # NEEDS WORK: why not refs/bundle/merge ??? + cat >expect <<-\EOF && + refs/bundles/base + refs/bundles/left + refs/bundles/right + EOF + test_cmp expect refs +' + ######################################################################### # HTTP tests begin here @@ -248,6 +331,89 @@ test_expect_success 'fetch bundle list (http, creationToken, incremental)' ' test_cmp expect refs ' +test_expect_success 'http clone with bundle.flag=forFetch creates fetch.bundleURI' ' + test_when_finished rm -rf fetch-http-4 trace1.txt trace2.txt && + cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF && + [bundle] + version = 1 + mode = all + heuristic = creationToken + flag = forFetch + + [bundle "bundle-1"] + uri = bundle-1.bundle + creationToken = 1 + EOF + + git clone --single-branch --branch=base \ + --bundle-uri="$HTTPD_URL/bundle-list" \ + "$HTTPD_URL/smart/fetch.git" fetch-http-4 && + + test_cmp_config -C fetch-http-4 "$HTTPD_URL/bundle-list" fetch.bundleuri && + + # only received base ref from bundle-1 + git -C fetch-http-4 for-each-ref --format="%(refname)" "refs/bundles/*" >refs && + cat >expect <<-\EOF && + refs/bundles/base + EOF + test_cmp expect refs && + + cat >>"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF && + [bundle "bundle-2"] + uri = bundle-2.bundle + creationToken = 2 + EOF + + GIT_TRACE2_EVENT="$(pwd)/trace1.txt" \ + git -C fetch-http-4 fetch origin --no-tags \ + refs/heads/left:refs/heads/left && + + # This fetch should copy two files: the list and bundle-2. + grep "region_enter.*bundle-uri.*download_https_uri_to_file" trace1.txt >copies && + + # NEEDSWORK: Currently downloading three copies instead of two! + test_line_count = 3 copies && + + # received left from bundle-2 + git -C fetch-http-4 for-each-ref --format="%(refname)" "refs/bundles/*" >refs && + cat >expect <<-\EOF && + refs/bundles/base + refs/bundles/left + EOF + test_cmp expect refs && + + cat >>"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF && + [bundle "bundle-3"] + uri = bundle-3.bundle + creationToken = 3 + + [bundle "bundle-4"] + uri = bundle-4.bundle + creationToken = 4 + EOF + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ + git -C fetch-http-4 fetch origin --no-tags \ + refs/heads/right:refs/heads/right && + + # This fetch should copy three files: the list, bundle-4, and bundle-3. + grep "region_enter.*bundle-uri.*download_https_uri_to_file" trace2.txt >copies && + + # NEEDSWORK: this is re-downloading everything! + test_line_count = 5 copies && + + # received right and merge from bundle-3 and bundle-4. + git -C fetch-http-4 for-each-ref --format="%(refname)" "refs/bundles/*" >refs && + + # NEEDSWORK: Why not refs/bundles/merge? + cat >expect <<-\EOF && + refs/bundles/base + refs/bundles/left + refs/bundles/right + EOF + test_cmp expect refs +' + # Do not add tests here unless they use the HTTP server, as they will # not run unless the HTTP dependencies exist.