From bdf4276c91623cf57efefd2ada7fb3e0709e2230 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 8 Oct 2018 11:09:23 -0700 Subject: transport: drop refnames from for_each_alternate_ref None of the current callers use the refname parameter we pass to their callbacks. In theory somebody _could_ do so, but it's actually quite weird if you think about it: it's a ref in somebody else's repository. So the name has no meaning locally, and in fact there may be duplicates if there are multiple alternates. The users of this interface really only care about seeing some ref tips, since that promises that the alternate has the full commit graph reachable from there. So let's keep the information we pass back to the bare minimum. Signed-off-by: Jeff King Signed-off-by: Taylor Blau Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c17ce94..edbc76f 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -280,8 +280,7 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid, return 0; } -static void show_one_alternate_ref(const char *refname, - const struct object_id *oid, +static void show_one_alternate_ref(const struct object_id *oid, void *data) { struct oidset *seen = data; diff --git a/fetch-pack.c b/fetch-pack.c index 88a078e..c5aaedf 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -76,8 +76,7 @@ struct alternate_object_cache { size_t nr, alloc; }; -static void cache_one_alternate(const char *refname, - const struct object_id *oid, +static void cache_one_alternate(const struct object_id *oid, void *vcache) { struct alternate_object_cache *cache = vcache; diff --git a/transport.c b/transport.c index 06ffea2..1ae2973 100644 --- a/transport.c +++ b/transport.c @@ -1336,7 +1336,7 @@ static void read_alternate_refs(const char *path, cmd.git_cmd = 1; argv_array_pushf(&cmd.args, "--git-dir=%s", path); argv_array_push(&cmd.args, "for-each-ref"); - argv_array_push(&cmd.args, "--format=%(objectname) %(refname)"); + argv_array_push(&cmd.args, "--format=%(objectname)"); cmd.env = local_repo_env; cmd.out = -1; @@ -1348,13 +1348,13 @@ static void read_alternate_refs(const char *path, struct object_id oid; if (get_oid_hex(line.buf, &oid) || - line.buf[GIT_SHA1_HEXSZ] != ' ') { + line.buf[GIT_SHA1_HEXSZ]) { warning(_("invalid line while parsing alternate refs: %s"), line.buf); break; } - cb(line.buf + GIT_SHA1_HEXSZ + 1, &oid, data); + cb(&oid, data); } fclose(fh); diff --git a/transport.h b/transport.h index 01e717c..9baeca2 100644 --- a/transport.h +++ b/transport.h @@ -261,6 +261,6 @@ int transport_refs_pushed(struct ref *ref); void transport_print_push_status(const char *dest, struct ref *refs, int verbose, int porcelain, unsigned int *reject_reasons); -typedef void alternate_ref_fn(const char *refname, const struct object_id *oid, void *); +typedef void alternate_ref_fn(const struct object_id *oid, void *); extern void for_each_alternate_ref(alternate_ref_fn, void *); #endif -- cgit v0.10.2-6-g49f6 From 1e5f31d444436cfab1b1aef3026ffbd925f5a818 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 8 Oct 2018 11:09:26 -0700 Subject: transport.c: extract 'fill_alternate_refs_command' To list alternate references, 'read_alternate_refs' creates a child process running 'git for-each-ref' in the alternate's Git directory. Prepare to run other commands besides 'git for-each-ref' by introducing and moving the relevant code from 'read_alternate_refs' to 'fill_alternate_refs_command'. Signed-off-by: Taylor Blau Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/transport.c b/transport.c index 1ae2973..25d8ea7 100644 --- a/transport.c +++ b/transport.c @@ -1325,6 +1325,17 @@ literal_copy: return xstrdup(url); } +static void fill_alternate_refs_command(struct child_process *cmd, + const char *repo_path) +{ + cmd->git_cmd = 1; + argv_array_pushf(&cmd->args, "--git-dir=%s", repo_path); + argv_array_push(&cmd->args, "for-each-ref"); + argv_array_push(&cmd->args, "--format=%(objectname)"); + cmd->env = local_repo_env; + cmd->out = -1; +} + static void read_alternate_refs(const char *path, alternate_ref_fn *cb, void *data) @@ -1333,12 +1344,7 @@ static void read_alternate_refs(const char *path, struct strbuf line = STRBUF_INIT; FILE *fh; - cmd.git_cmd = 1; - argv_array_pushf(&cmd.args, "--git-dir=%s", path); - argv_array_push(&cmd.args, "for-each-ref"); - argv_array_push(&cmd.args, "--format=%(objectname)"); - cmd.env = local_repo_env; - cmd.out = -1; + fill_alternate_refs_command(&cmd, path); if (start_command(&cmd)) return; -- cgit v0.10.2-6-g49f6 From 89284c1d6ca03480a93f9d95a2460a09390f3c40 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 8 Oct 2018 11:09:28 -0700 Subject: transport.c: introduce core.alternateRefsCommand When in a repository containing one or more alternates, Git would sometimes like to list references from those alternates. For example, 'git receive-pack' lists the "tips" pointed to by references in those alternates as special ".have" references. Listing ".have" references is designed to make pushing changes from upstream to a fork a lightweight operation, by advertising to the pusher that the fork already has the objects (via its alternate). Thus, the client can avoid sending them. However, when the alternate (upstream, in the previous example) has a pathologically large number of references, the initial advertisement is too expensive. In fact, it can dominate any such optimization where the pusher avoids sending certain objects. Introduce "core.alternateRefsCommand" in order to provide a facility to limit or filter alternate references. This can be used, for example, to filter out references the alternate does not wish to send (for space concerns, or otherwise) during the initial advertisement. Let the repository that has alternates configure this command to avoid trusting the alternate to provide us a safe command to run in the shell. To find the alternate, pass its absolute path as the first argument. Signed-off-by: Taylor Blau Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/Documentation/config.txt b/Documentation/config.txt index eb66a11..00224de 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -616,6 +616,17 @@ core.preferSymlinkRefs:: This is sometimes needed to work with old scripts that expect HEAD to be a symbolic link. +core.alternateRefsCommand:: + When advertising tips of available history from an alternate, use the shell to + execute the specified command instead of linkgit:git-for-each-ref[1]. The + first argument is the absolute path of the alternate. Output must contain one + hex object id per line (i.e., the same as produce by `git for-each-ref + --format='%(objectname)'`). ++ +Note that you cannot generally put `git for-each-ref` directly into the config +value, as it does not take a repository path as an argument (but you can wrap +the command above in a shell script). + core.bare:: If true this repository is assumed to be 'bare' and has no working directory associated with it. If this is the case a diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh new file mode 100755 index 0000000..49d0fe4 --- /dev/null +++ b/t/t5410-receive-pack-alternates.sh @@ -0,0 +1,33 @@ +#!/bin/sh + +test_description='git receive-pack with alternate ref filtering' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit base && + git clone -s --bare . fork && + git checkout -b public/branch master && + test_commit public && + git checkout -b private/branch master && + test_commit private +' + +extract_haves () { + depacketize | perl -lne '/^(\S+) \.have/ and print $1' +} + +test_expect_success 'with core.alternateRefsCommand' ' + write_script fork/alternate-refs <<-\EOF && + git --git-dir="$1" for-each-ref \ + --format="%(objectname)" \ + refs/heads/public/ + EOF + test_config -C fork core.alternateRefsCommand alternate-refs && + git rev-parse public/branch >expect && + printf "0000" | git receive-pack fork >actual && + extract_haves actual.haves && + test_cmp expect actual.haves +' + +test_done diff --git a/transport.c b/transport.c index 25d8ea7..19baec7 100644 --- a/transport.c +++ b/transport.c @@ -1328,10 +1328,21 @@ literal_copy: static void fill_alternate_refs_command(struct child_process *cmd, const char *repo_path) { - cmd->git_cmd = 1; - argv_array_pushf(&cmd->args, "--git-dir=%s", repo_path); - argv_array_push(&cmd->args, "for-each-ref"); - argv_array_push(&cmd->args, "--format=%(objectname)"); + const char *value; + + if (!git_config_get_value("core.alternateRefsCommand", &value)) { + cmd->use_shell = 1; + + argv_array_push(&cmd->args, value); + argv_array_push(&cmd->args, repo_path); + } else { + cmd->git_cmd = 1; + + argv_array_pushf(&cmd->args, "--git-dir=%s", repo_path); + argv_array_push(&cmd->args, "for-each-ref"); + argv_array_push(&cmd->args, "--format=%(objectname)"); + } + cmd->env = local_repo_env; cmd->out = -1; } -- cgit v0.10.2-6-g49f6 From 40f327faf543ccaeb03aa0362f3fed7438c2c615 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 8 Oct 2018 11:09:30 -0700 Subject: transport.c: introduce core.alternateRefsPrefixes The recently-introduced "core.alternateRefsCommand" allows callers to specify with high flexibility the tips that they wish to advertise from alternates. This flexibility comes at the cost of some inconvenience when the caller only wishes to limit the advertisement to one or more prefixes. For example, to advertise only tags, a caller using 'core.alternateRefsCommand' would have to do: $ git config core.alternateRefsCommand ' \ f() { git -C "$1" for-each-ref \ refs/tags --format="%(objectname)" }; f "$@"' The above is cumbersome to write, so let's introduce a "core.alternateRefsPrefixes" to address this common case. Instead, the caller can run: $ git config core.alternateRefsPrefixes 'refs/tags' Which will behave identically to the longer example using "core.alternateRefsCommand". Since the value of "core.alternateRefsPrefixes" is appended to 'git for-each-ref' and then executed, include a "--" before taking the configured value to avoid misinterpreting arguments as flags to 'git for-each-ref'. In the case that the caller wishes to specify multiple prefixes, they may separate them by whitespace. If "core.alternateRefsCommand" is set, it will take precedence over "core.alternateRefsPrefixes". Signed-off-by: Taylor Blau Acked-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/Documentation/config.txt b/Documentation/config.txt index 00224de..3486f9f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -627,6 +627,13 @@ Note that you cannot generally put `git for-each-ref` directly into the config value, as it does not take a repository path as an argument (but you can wrap the command above in a shell script). +core.alternateRefsPrefixes:: + When listing references from an alternate, list only references that begin + with the given prefix. Prefixes match as if they were given as arguments to + linkgit:git-for-each-ref[1]. To list multiple prefixes, separate them with + whitespace. If `core.alternateRefsCommand` is set, setting + `core.alternateRefsPrefixes` has no effect. + core.bare:: If true this repository is assumed to be 'bare' and has no working directory associated with it. If this is the case a diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh index 49d0fe4..457c20c 100755 --- a/t/t5410-receive-pack-alternates.sh +++ b/t/t5410-receive-pack-alternates.sh @@ -30,4 +30,12 @@ test_expect_success 'with core.alternateRefsCommand' ' test_cmp expect actual.haves ' +test_expect_success 'with core.alternateRefsPrefixes' ' + test_config -C fork core.alternateRefsPrefixes "refs/heads/private" && + git rev-parse private/branch >expect && + printf "0000" | git receive-pack fork >actual && + extract_haves actual.haves && + test_cmp expect actual.haves +' + test_done diff --git a/transport.c b/transport.c index 19baec7..90a3350 100644 --- a/transport.c +++ b/transport.c @@ -1341,6 +1341,11 @@ static void fill_alternate_refs_command(struct child_process *cmd, argv_array_pushf(&cmd->args, "--git-dir=%s", repo_path); argv_array_push(&cmd->args, "for-each-ref"); argv_array_push(&cmd->args, "--format=%(objectname)"); + + if (!git_config_get_value("core.alternateRefsPrefixes", &value)) { + argv_array_push(&cmd->args, "--"); + argv_array_split(&cmd->args, value); + } } cmd->env = local_repo_env; -- cgit v0.10.2-6-g49f6