Re: [PATCH v2 4/9] http-fetch: support fetching packfiles by URL
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 11, 2020, 01:30 UTC
- Message-ID
- <xmqqo8pqcrtq.fsf@gitster.c.googlers.com>
- In-Reply-To
- <a4a7ed512b251fb16f0b2400fac8c0c5cc14aa75.1591821067.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 15 quoted lines
> Teach http-fetch the ability to download packfiles directly, given a > URL, and to verify them. > > The http_pack_request suite has been augmented with a function that > takes a URL directly. With this function, the hash is only used to > determine the name of the temporary file. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > Documentation/git-http-fetch.txt | 9 ++++- > http-fetch.c | 63 +++++++++++++++++++++++++++----- > http.c | 28 ++++++++++---- > http.h | 11 ++++++ > t/t5550-http-fetch-dumb.sh | 30 +++++++++++++++ > 5 files changed, 123 insertions(+), 18 deletions(-)
This step certainly got easier to read thanks to the introduction of 3/9 but ...
Show 40 quoted lines
> @@ -68,25 +103,33 @@ int cmd_main(int argc, const char **argv)
> get_recover = 1;
> } else if (!strcmp(argv[arg], "--stdin")) {
> commits_on_stdin = 1;
> + } else if (skip_prefix(argv[arg], "--packfile=", &p)) {
> + const char *end;
> +
> + packfile = 1;
> + if (parse_oid_hex(p, &packfile_hash, &end) || *end)
> + die(_("argument to --packfile must be a valid hash (got '%s')"), p);
> }
> arg++;
> }
> - if (argc != arg + 2 - commits_on_stdin)
> + if (argc != arg + 2 - (commits_on_stdin || packfile))
> usage(http_fetch_usage);
> - if (commits_on_stdin) {
> - commits = walker_targets_stdin(&commit_id, &write_ref);
> - } else {
> - commit_id = (char **) &argv[arg++];
> - commits = 1;
> - }
>
> setup_git_directory();
>
> git_config(git_default_config, NULL);
>
> - if (!argv[arg])
> - BUG("must have one arg remaining");
> + if (packfile) {
> + fetch_single_packfile(&packfile_hash, argv[arg]);
> + return 0;
> + }
>
> + if (commits_on_stdin) {
> + commits = walker_targets_stdin(&commit_id, &write_ref);
> + } else {
> + commit_id = (char **) &argv[arg++];
> + commits = 1;
> + }... it would have been even less taxing to the readers' minds if the code movement to run setup-git-directory and git-config before the computation of the walker target done here is *not* part of this step. Perhaps that can be done between 2/9 and 3/9, or as part of 3/9? The point is to reduce the mental load required to understand the step that does *new* things, like this step.
Show 16 quoted lines
> diff --git a/http.h b/http.h
> index bbc6b070f1..dc49c60165 100644
> --- a/http.h
> +++ b/http.h
> @@ -216,6 +216,15 @@ int http_get_info_packs(const char *base_url,
>
> struct http_pack_request {
> char *url;
> +
> + /*
> + * If this is true, finish_http_pack_request() will pass "--keep" to
> + * index-pack, resulting in the creation of a keep file, and will not
> + * suppress its stdout (that is, the "keep\t<hash>\n" line will be
> + * printed to stdout).
> + */
> + unsigned generate_keep : 1;OK.