Re: [PATCH v2 4/9] http-fetch: support fetching packfiles by URL
From: Junio C Hamano <hidden>
Date: 2020-06-11 01:30:17
Jonathan Tan [off-list ref] 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 <redacted> --- 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 ...
quoted hunk
@@ -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.
quoted hunk
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.