git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/8] clone: set fetch.bundleURI if appropriate

From
Victoria Dye <vdye@github.com>
Date
Jan 19, 2023, 19:42 UTC
Message-ID
<61a830f0-a8d8-5a8a-e952-db213d571352@github.com>
In-Reply-To
<d9c6f50e4f218267c1e8da060ce5b190dc8a709c.1673037405.git.gitgitgadget@gmail.com>
Derrick Stolee via GitGitGadget wrote:
Show 16 quoted lines
> diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
> index cd65d236b43..4f796218aab 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 `<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].

Since it's not mentioned from this or any other user-facing documentation (AFAICT), could you note that this value is set automatically by 'git clone' iff '--bundle-uri' is specified *and* 'bundle.heuristic' is set for the initially downloaded bundle list?

It would also be nice to make note of that behavior in the documentation of the '--bundle-uri' option in 'Documentation/git-clone.txt', since command documentation in general seems to be more popular/visible to users than config docs.

Show 19 quoted lines
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 5453ba5277f..5370617664d 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1248,12 +1248,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	 * data from the --bundle-uri option.
>  	 */
>  	if (bundle_uri) {
> +		int has_heuristic = 0;
> +
>  		/* At this point, we need the_repository to match the cloned repo. */
>  		if (repo_init(the_repository, git_dir, work_tree))
>  			warning(_("failed to initialize the repo, skipping bundle URI"));
> -		else if (fetch_bundle_uri(the_repository, bundle_uri))
> +		else if (fetch_bundle_uri(the_repository, bundle_uri, &has_heuristic))
>  			warning(_("failed to fetch objects from bundle URI '%s'"),
>  				bundle_uri);
> +		else if (has_heuristic)
> +			git_config_set_gently("fetch.bundleuri", bundle_uri);

If the heuristic is anything other than "none", this config value is set in the repository-scoped config file. Makes sense!

Show 16 quoted lines
>  	}
>  
>  	strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
> diff --git a/bundle-uri.c b/bundle-uri.c
> index b30c85ba6f2..1dbbbb980eb 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c
> @@ -594,9 +594,10 @@ static int fetch_bundle_list_in_config_format(struct repository *r,
>  	 * it advertises are expected to be bundles, not nested lists.
>  	 * We can drop 'global_list' and 'depth'.
>  	 */
> -	if (list_from_bundle.heuristic == BUNDLE_HEURISTIC_CREATIONTOKEN)
> +	if (list_from_bundle.heuristic == BUNDLE_HEURISTIC_CREATIONTOKEN) {
>  		result = fetch_bundles_by_token(r, &list_from_bundle);
> -	else if ((result = download_bundle_list(r, &list_from_bundle,
> +		global_list->heuristic = BUNDLE_HEURISTIC_CREATIONTOKEN;

If the 'heuristic' field already existed and was being used to apply bundles, why wasn't 'global_list->heuristic' already being set? Before this patch, was the 'global_list->heuristic' field not accurately reflecting the heuristic type of a given bundle list?

If so, I think it'd make sense to move this section to patch 4 [1], since that's when the heuristic is first applied to the bundle list.

[1] https://lore.kernel.org/git/57c0174d3752fb61a05e0653de9d3057616ed16a.1673037405.git.gitgitgadget@gmail.com/
Show 40 quoted lines
> diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
> index d7461ec907e..8ff560425ee 100755
> --- a/t/t5558-clone-bundle-uri.sh
> +++ b/t/t5558-clone-bundle-uri.sh
> @@ -435,6 +435,39 @@ test_expect_success 'clone bundle list (http, creationToken)' '
>  	test_bundle_downloaded bundle-2.bundle trace-clone.txt
>  '
>  
> +test_expect_success 'http clone with bundle.heuristic creates fetch.bundleURI' '
> +	test_when_finished rm -rf fetch-http-4 trace*.txt &&
> +
> +	cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
> +	[bundle]
> +		version = 1
> +		mode = all
> +		heuristic = creationToken
> +
> +	[bundle "bundle-1"]
> +		uri = bundle-1.bundle
> +		creationToken = 1
> +	EOF
> +
> +	GIT_TRACE2_EVENT="$(pwd)/trace-clone.txt" \
> +	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 &&
> +
> +	# The clone should copy two files: the list and bundle-1.
> +	test_bundle_downloaded bundle-list trace-clone.txt &&
> +	test_bundle_downloaded bundle-1.bundle trace-clone.txt &&
> +
> +	# 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
> +'

This test looks good - it verifies the config update, bundle download, and unbundle all work as intended.

> +
>  # Do not add tests here unless they use the HTTP server, as they will
>  # not run unless the HTTP dependencies exist.
>  
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 12 of 79 in “Bundle URIs V: creationToken heuristic for incremental fetches”
  1. 0/8 Bundle URIs V: creationToken heuristic for incremental fetchesDerrick Stolee via GitGitGadget, Jan 6, 2023
  2. 2/8 bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee via GitGitGadget, Jan 6, 2023
  3. Junio C HamanoJan 9, 2023
  4. Derrick StoleeJan 9, 2023
  5. Victoria DyeJan 17, 2023
  6. 1/8 t5558: add tests for creationToken heuristicDerrick Stolee via GitGitGadget, Jan 6, 2023
  7. Victoria DyeJan 17, 2023
  8. Derrick StoleeJan 17, 2023
  9. 6/8 bundle-uri: drop bundle.flag from design docDerrick Stolee via GitGitGadget, Jan 6, 2023
  10. Victoria DyeJan 19, 2023
  11. 5/8 clone: set fetch.bundleURI if appropriateDerrick Stolee via GitGitGadget, Jan 6, 2023
  12. Victoria DyeJan 19, 2023
  13. Derrick StoleeJan 20, 2023
  14. 7/8 fetch: fetch from an external bundle URIDerrick Stolee via GitGitGadget, Jan 6, 2023
  15. Victoria DyeJan 19, 2023
  16. Derrick StoleeJan 20, 2023
  17. 4/8 bundle-uri: download in creationToken orderDerrick Stolee via GitGitGadget, Jan 6, 2023
  18. Junio C HamanoJan 9, 2023
  19. Derrick StoleeJan 9, 2023
  20. Victoria DyeJan 19, 2023
  21. Derrick StoleeJan 20, 2023
  22. 8/8 bundle-uri: store fetch.bundleCreationTokenDerrick Stolee via GitGitGadget, Jan 6, 2023
  23. Victoria DyeJan 19, 2023
  24. Derrick StoleeJan 20, 2023
  25. 3/8 bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee via GitGitGadget, Jan 6, 2023
  26. Junio C HamanoJan 9, 2023
  27. Derrick StoleeJan 9, 2023
  28. Victoria DyeJan 17, 2023
  29. 00/10 Bundle URIs V: creationToken heuristic for incremental fetchesDerrick Stolee via GitGitGadget, Jan 23, 2023
  30. 01/10 bundle: optionally skip reachability walkDerrick Stolee via GitGitGadget, Jan 23, 2023
  31. Junio C HamanoJan 23, 2023
  32. Derrick StoleeJan 23, 2023
  33. Junio C HamanoJan 23, 2023
  34. Junio C HamanoJan 23, 2023
  35. Derrick StoleeJan 24, 2023
  36. 01/11 bundle: test unbundling with incomplete historyDerrick Stolee, Jan 24, 2023
  37. Junio C HamanoJan 24, 2023
  38. 02/11 bundle: verify using connected()Derrick Stolee, Jan 24, 2023
  39. Junio C HamanoJan 24, 2023
  40. Derrick StoleeJan 24, 2023
  41. Junio C HamanoJan 24, 2023
  42. Junio C HamanoJan 24, 2023
  43. Junio C HamanoJan 23, 2023
  44. 02/10 t5558: add tests for creationToken heuristicDerrick Stolee via GitGitGadget, Jan 23, 2023
  45. Victoria DyeJan 27, 2023
  46. 04/10 bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee via GitGitGadget, Jan 23, 2023
  47. 03/10 bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee via GitGitGadget, Jan 23, 2023
  48. 05/10 bundle-uri: download in creationToken orderDerrick Stolee via GitGitGadget, Jan 23, 2023
  49. Victoria DyeJan 27, 2023
  50. Junio C HamanoJan 27, 2023
  51. Derrick StoleeJan 30, 2023
  52. Junio C HamanoJan 30, 2023
  53. Derrick StoleeJan 30, 2023
  54. 06/10 clone: set fetch.bundleURI if appropriateDerrick Stolee via GitGitGadget, Jan 23, 2023
  55. 07/10 bundle-uri: drop bundle.flag from design docDerrick Stolee via GitGitGadget, Jan 23, 2023
  56. 09/10 bundle-uri: store fetch.bundleCreationTokenDerrick Stolee via GitGitGadget, Jan 23, 2023
  57. 10/10 bundle-uri: test missing bundles with heuristicDerrick Stolee via GitGitGadget, Jan 23, 2023
  58. Victoria DyeJan 27, 2023
  59. Derrick StoleeJan 30, 2023
  60. 08/10 fetch: fetch from an external bundle URIDerrick Stolee via GitGitGadget, Jan 23, 2023
  61. Victoria DyeJan 27, 2023
  62. Victoria DyeJan 27, 2023
  63. 00/11 Bundle URIs V: creationToken heuristic for incremental fetchesDerrick Stolee via GitGitGadget, Jan 31, 2023
  64. 01/11 bundle: test unbundling with incomplete historyDerrick Stolee via GitGitGadget, Jan 31, 2023
  65. 02/11 bundle: verify using check_connected()Derrick Stolee via GitGitGadget, Jan 31, 2023
  66. Junio C HamanoJan 31, 2023
  67. Derrick StoleeJan 31, 2023
  68. Junio C HamanoJan 31, 2023
  69. 03/11 t5558: add tests for creationToken heuristicDerrick Stolee via GitGitGadget, Jan 31, 2023
  70. 04/11 bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee via GitGitGadget, Jan 31, 2023
  71. 05/11 bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee via GitGitGadget, Jan 31, 2023
  72. Junio C HamanoJan 31, 2023
  73. 07/11 clone: set fetch.bundleURI if appropriateDerrick Stolee via GitGitGadget, Jan 31, 2023
  74. 06/11 bundle-uri: download in creationToken orderDerrick Stolee via GitGitGadget, Jan 31, 2023
  75. 08/11 bundle-uri: drop bundle.flag from design docDerrick Stolee via GitGitGadget, Jan 31, 2023
  76. 09/11 fetch: fetch from an external bundle URIDerrick Stolee via GitGitGadget, Jan 31, 2023
  77. 11/11 bundle-uri: test missing bundles with heuristicDerrick Stolee via GitGitGadget, Jan 31, 2023
  78. 10/11 bundle-uri: store fetch.bundleCreationTokenDerrick Stolee via GitGitGadget, Jan 31, 2023
  79. Junio C HamanoJan 31, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.