From 6a3c9d36bfe807c7c0176c0fc727773cbb60748e 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 | 10 +- bundle-uri.c | 13 ++- bundle-uri.h | 11 ++- t/t5558-fetch-bundle-uri.sh | 166 +++++++++++++++++++++++++++++++++ 6 files changed, 209 insertions(+), 5 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 8a32241ef3c49e..7efbbfc25f09f8 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2089,11 +2089,15 @@ 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; + int bundle_uri_from_config; struct string_list list = STRING_LIST_INIT_DUP; struct remote *remote = NULL; int result = 0; int prune_tags_ok = 1; + bundle_uri_from_config = !bundle_uri && + !git_config_get_string_tmp("fetch.bundleuri", &bundle_uri); + packet_trace_identity("fetch"); /* Record the command line for the reflog */ @@ -2175,8 +2179,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) write_fetch_head = 0; if (bundle_uri) { - result = fetch_bundle_uri(the_repository, bundle_uri); - goto cleanup; + result = fetch_bundle_uri(the_repository, bundle_uri, NULL); + + if (!bundle_uri_from_config) + goto cleanup; } if (all) { diff --git a/bundle-uri.c b/bundle-uri.c index 9fafa724ffc3c4..cdf2847f12087f 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(pkey, "flag")) { + if (!strcmp("forFetch", value)) + list->flags |= BUNDLE_FLAG_FOR_FETCH; + return 0; + } + if (!strcmp(pkey, "heuristic")) { int i; for (i = 0; i < BUNDLE_HEURISTIC_COUNT; i++) { @@ -564,6 +570,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; } @@ -693,7 +701,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; @@ -714,6 +723,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 22c14700c59d96..1c4017f0a19359 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; /** @@ -122,8 +127,12 @@ int parse_bundle_list_in_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-fetch-bundle-uri.sh b/t/t5558-fetch-bundle-uri.sh index 23cded235fd104..b518b80785d2e0 100755 --- a/t/t5558-fetch-bundle-uri.sh +++ b/t/t5558-fetch-bundle-uri.sh @@ -157,6 +157,89 @@ test_expect_success 'fetch 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 = fetch-from/bundle-1.bundle + creationToken = 1 + EOF + + git clone --no-local --single-branch --branch=base \ + --bundle-uri="file://$(pwd)/bundle-list" \ + fetch-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 = fetch-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 = fetch-from/bundle-3.bundle + creationToken = 3 + + [bundle "bundle-4"] + uri = fetch-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 @@ -279,6 +362,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.