Skip to content

Commit

Permalink
clone: set fetch.bundleURI if bundle.forFetch is advertised
Browse files Browse the repository at this point in the history
TODO: change forFetch local to a 'flags' value.

NEEDSWORK: we are downloading too many bundles.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
  • Loading branch information
derrickstolee committed Jul 25, 2022
1 parent aadfe7d commit e5b55ad
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 3 deletions.
8 changes: 8 additions & 0 deletions Documentation/config/fetch.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<uri>` then running `git fetch <args>` is equivalent to
first running `git fetch --bundle-uri=<uri>` immediately before
`git fetch <args>`. See details of the `--bundle-uri` option in
linkgit:git-fetch[1].
6 changes: 5 additions & 1 deletion builtin/clone.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
5 changes: 5 additions & 0 deletions builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
Expand Down
13 changes: 12 additions & 1 deletion bundle-uri.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down Expand Up @@ -562,6 +568,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;
}
Expand Down Expand Up @@ -686,7 +694,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;
Expand All @@ -707,6 +716,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);
Expand Down
11 changes: 10 additions & 1 deletion bundle-uri.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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
Expand Down
166 changes: 166 additions & 0 deletions t/t5558-clone-bundle-uri.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.

Expand Down

0 comments on commit e5b55ad

Please sign in to comment.