Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
> 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@xxxxxxxxxx>
> ---
> 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 ...
> @@ -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.
> 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.
