Re: [PATCH 5/8] clone: set fetch.bundleURI if appropriate
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jan 20, 2023, 15:42 UTC
- Message-ID
- <75d25afd-f941-3090-5cac-f1a4ca7d9827@github.com>
- In-Reply-To
- <61a830f0-a8d8-5a8a-e952-db213d571352@github.com>
On 1/19/2023 2:42 PM, Victoria Dye wrote:
Show 22 quoted lines
> Derrick Stolee via GitGitGadget wrote: >> 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?
Can do.
> 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.
Yes. I also thought that I had updated this documentation to not refer to 'git fetch --bundle-uri', which doesn't exist anymore since an earlier RFC version. I'll be sure to update that, too.
Show 24 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/Can do.
Thanks, -Stolee