On 2021.01.23 18:34, Jonathan Tan wrote:
> This is the next step in teaching fetch-pack to pass its index-pack
> arguments when processing packfiles referenced by URIs.
>
> The "--keep" in fetch-pack.c will be replaced with a full message in a
> subsequent commit.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> Documentation/git-http-fetch.txt | 9 ++++++--
> fetch-pack.c | 1 +
> http-fetch.c | 35 +++++++++++++++++++++++++++-----
> t/t5550-http-fetch-dumb.sh | 3 ++-
> 4 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-http-fetch.txt b/Documentation/git-http-fetch.txt
> index 4deb4893f5..aa171088e8 100644
> --- a/Documentation/git-http-fetch.txt
> +++ b/Documentation/git-http-fetch.txt
> @@ -41,11 +41,16 @@ commit-id::
> <commit-id>['\t'<filename-as-in--w>]
>
> --packfile=<hash>::
> - Instead of a commit id on the command line (which is not expected in
> + For internal use only. Instead of a commit id on the command line (which is not expected in
> this case), 'git http-fetch' fetches the packfile directly at the given
> URL and uses index-pack to generate corresponding .idx and .keep files.
> The hash is used to determine the name of the temporary file and is
> - arbitrary. The output of index-pack is printed to stdout.
> + arbitrary. The output of index-pack is printed to stdout. Requires
> + --index-pack-args.
> +
> +--index-pack-args=<args>::
> + For internal use only. The command to run on the contents of the
> + downloaded pack. Arguments are URL-encoded separated by spaces.
I'm a bit skeptical of using URL encoding to work around embedded
spaces. I believe in Emily's config-based hooks series, she wrote an
argument parser to pull repeated arguments into a strvec, could you do
something like that here?
I'm sympathetic to the idea that since this is an internal-only flag, we
can be a bit weird with the argument format, though.
> --recover::
> Verify that everything reachable from target is fetched. Used after
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 876f90c759..274ae602f7 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1645,6 +1645,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
> strvec_pushf(&cmd.args, "--packfile=%.*s",
> (int) the_hash_algo->hexsz,
> packfile_uris.items[i].string);
> + strvec_push(&cmd.args, "--index-pack-args=index-pack --stdin --keep");
> strvec_push(&cmd.args, uri);
> cmd.git_cmd = 1;
> cmd.no_stdin = 1;
> diff --git a/http-fetch.c b/http-fetch.c
> index 2d1d9d054f..12feb84e71 100644
> --- a/http-fetch.c
> +++ b/http-fetch.c
> @@ -3,6 +3,7 @@
> #include "exec-cmd.h"
> #include "http.h"
> #include "walker.h"
> +#include "strvec.h"
>
> static const char http_fetch_usage[] = "git http-fetch "
> "[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin | --packfile=hash | commit-id] url";
> @@ -43,11 +44,9 @@ static int fetch_using_walker(const char *raw_url, int get_verbosely,
> return rc;
> }
>
> -static const char *index_pack_args[] =
> - {"index-pack", "--stdin", "--keep", NULL};
> -
> static void fetch_single_packfile(struct object_id *packfile_hash,
> - const char *url) {
> + const char *url,
> + const char **index_pack_args) {
> struct http_pack_request *preq;
> struct slot_results results;
> int ret;
> @@ -90,6 +89,7 @@ int cmd_main(int argc, const char **argv)
> int packfile = 0;
> int nongit;
> struct object_id packfile_hash;
> + const char *index_pack_args = NULL;
>
> setup_git_directory_gently(&nongit);
>
> @@ -116,6 +116,8 @@ int cmd_main(int argc, const char **argv)
> packfile = 1;
> if (parse_oid_hex(p, &packfile_hash, &end) || *end)
> die(_("argument to --packfile must be a valid hash (got '%s')"), p);
> + } else if (skip_prefix(argv[arg], "--index-pack-args=", &p)) {
> + index_pack_args = p;
> }
> arg++;
> }
> @@ -128,10 +130,33 @@ int cmd_main(int argc, const char **argv)
> git_config(git_default_config, NULL);
>
> if (packfile) {
> - fetch_single_packfile(&packfile_hash, argv[arg]);
> + struct strvec encoded = STRVEC_INIT;
> + char **raw;
> + int i;
> +
> + if (!index_pack_args)
> + die(_("--packfile requires --index-pack-args"));
> +
> + strvec_split(&encoded, index_pack_args);
> +
> + CALLOC_ARRAY(raw, encoded.nr + 1);
> + for (i = 0; i < encoded.nr; i++)
> + raw[i] = url_percent_decode(encoded.v[i]);
> +
> + fetch_single_packfile(&packfile_hash, argv[arg],
> + (const char **) raw);
> +
> + for (i = 0; i < encoded.nr; i++)
> + free(raw[i]);
> + free(raw);
> + strvec_clear(&encoded);
> +
> return 0;
> }
>
> + if (index_pack_args)
> + die(_("--index-pack-args can only be used with --packfile"));
> +
> if (commits_on_stdin) {
> commits = walker_targets_stdin(&commit_id, &write_ref);
> } else {
> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
> index 483578b2d7..af90e7efed 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -224,7 +224,8 @@ test_expect_success 'http-fetch --packfile' '
>
> git init packfileclient &&
> p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && ls objects/pack/pack-*.pack) &&
> - git -C packfileclient http-fetch --packfile=$ARBITRARY "$HTTPD_URL"/dumb/repo_pack.git/$p >out &&
> + git -C packfileclient http-fetch --packfile=$ARBITRARY \
> + --index-pack-args="index-pack --stdin --keep" "$HTTPD_URL"/dumb/repo_pack.git/$p >out &&
>
> grep "^keep.[0-9a-f]\{16,\}$" out &&
> cut -c6- out >packhash &&
> --
> 2.30.0.280.ga3ce27912f-goog
>
