On Tue, Apr 06, 2021 at 06:47:50PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> The prefetch task previously used the default refspec source plus a
> custom refspec destination to avoid colliding with remote refs:
>
> +refs/heads/*:refs/prefetch/<remote>/*
>
> However, some users customize their refspec to reduce how much data they
> download from specific remotes. This can involve restrictive patterns
> for fetching or negative patterns to avoid downloading some refs.
>
> Modify fetch_remote() to iterate over the remote's refspec list and
> translate that into the appropriate prefetch scenario. Specifically,
> re-parse the raw form of the refspec into a new 'struct refspec' and
> modify the 'dst' member to replace a leading "refs/" substring with
> "refs/prefetch/", or prepend "refs/prefetch/" to 'dst' otherwise.
> Negative refspecs do not have a 'dst' so they can be transferred to the
> 'git fetch' command unmodified.
>
> This prefix change provides the benefit of keeping whatever collisions
> may exist in the custom refspecs, if that is a desirable outcome.
>
> This changes the names of the refs that would be fetched by the default
> refspec. Instead of "refs/prefetch/<remote>/<branch>" they will now go
> to "refs/prefetch/remotes/<remote>/<branch>". While this is a change, it
> is not a seriously breaking one: these refs are intended to be hidden
> and not used.
>
> Update the documentation to be more generic about the destination refs.
> Do not mention custom refpecs explicitly, as that does not need to be
> highlighted in this documentation. The important part of placing refs in
> refs/prefetch remains.
>
> Reported-by: Tom Saeger <tom.saeger@xxxxxxxxxx>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> Documentation/git-maintenance.txt | 3 +--
> builtin/gc.c | 37 +++++++++++++++++++++++++-
> t/t7900-maintenance.sh | 43 ++++++++++++++++++++++++++-----
> 3 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt
> index 80ddd33ceba0..95a24264eb10 100644
> --- a/Documentation/git-maintenance.txt
> +++ b/Documentation/git-maintenance.txt
> @@ -94,8 +94,7 @@ prefetch::
> objects from all registered remotes. For each remote, a `git fetch`
> command is run. The refmap is custom to avoid updating local or remote
> branches (those in `refs/heads` or `refs/remotes`). Instead, the
> - remote refs are stored in `refs/prefetch/<remote>/`. Also, tags are
> - not updated.
> + refs are stored in `refs/prefetch/`. Also, tags are not updated.
> +
> This is done to avoid disrupting the remote-tracking branches. The end users
> expect these refs to stay unmoved unless they initiate a fetch. With prefetch
> diff --git a/builtin/gc.c b/builtin/gc.c
> index fa8128de9ae1..76f347dd6b11 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -32,6 +32,7 @@
> #include "remote.h"
> #include "object-store.h"
> #include "exec-cmd.h"
> +#include "refspec.h"
>
> #define FAILED_RUN "failed to run %s"
>
> @@ -877,6 +878,7 @@ static int fetch_remote(struct remote *remote, void *cbdata)
> {
> struct maintenance_run_opts *opts = cbdata;
> struct child_process child = CHILD_PROCESS_INIT;
> + int i;
>
> child.git_cmd = 1;
> strvec_pushl(&child.args, "fetch", remote->name, "--prune", "--no-tags",
> @@ -886,7 +888,40 @@ static int fetch_remote(struct remote *remote, void *cbdata)
> if (opts->quiet)
> strvec_push(&child.args, "--quiet");
>
> - strvec_pushf(&child.args, "+refs/heads/*:refs/prefetch/%s/*", remote->name);
> + for (i = 0; i < remote->fetch.nr; i++) {
> + struct refspec_item replace;
> + struct refspec_item *rsi = &remote->fetch.items[i];
> + struct strbuf new_dst = STRBUF_INIT;
> + size_t ignore_len = 0;
> + char *replace_string;
> +
> + if (rsi->negative) {
> + strvec_push(&child.args, remote->fetch.raw[i]);
> + continue;
> + }
> +
> + refspec_item_init(&replace, remote->fetch.raw[i], 1);
> +
> + /*
> + * If a refspec dst starts with "refs/" at the start,
> + * then we will replace "refs/" with "refs/prefetch/".
> + * Otherwise, we will prepend the dst string with
> + * "refs/prefetch/".
> + */
> + if (!strncmp(replace.dst, "refs/", 5))
> + ignore_len = 5;
> +
> + strbuf_addstr(&new_dst, "refs/prefetch/");
> + strbuf_addstr(&new_dst, replace.dst + ignore_len);
> + free(replace.dst);
> + replace.dst = strbuf_detach(&new_dst, NULL);
> +
> + replace_string = refspec_item_format(&replace);
> + strvec_push(&child.args, replace_string);
> + free(replace_string);
> +
> + refspec_item_clear(&replace);
> + }
>
> return !!run_command(&child);
> }
Junio brought up the point about configs which 'fetch' have no dst
https://lore.kernel.org/git/c06a198a-2043-27a2-cab3-3471190754cc@xxxxxxxxx/
[remote "submaintainer1"]
url = ... repository of submaintainer #1 ...
fetch = master
tagopt = --no-tags
This patch fixes segfault for config like above.
You might have ideas on a cleaner way to do this.
I did add `child_process_clear`.
--Tom
diff --git a/builtin/gc.c b/builtin/gc.c
index 76f347dd6b11..921266ee30a5 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -879,6 +879,7 @@ static int fetch_remote(struct remote *remote, void *cbdata)
struct maintenance_run_opts *opts = cbdata;
struct child_process child = CHILD_PROCESS_INIT;
int i;
+ int nargs;
child.git_cmd = 1;
strvec_pushl(&child.args, "fetch", remote->name, "--prune", "--no-tags",
@@ -888,6 +889,8 @@ static int fetch_remote(struct remote *remote, void *cbdata)
if (opts->quiet)
strvec_push(&child.args, "--quiet");
+ nargs = child.args.nr;
+
for (i = 0; i < remote->fetch.nr; i++) {
struct refspec_item replace;
struct refspec_item *rsi = &remote->fetch.items[i];
@@ -900,6 +903,10 @@ static int fetch_remote(struct remote *remote, void *cbdata)
continue;
}
+ if (!rsi->dst) {
+ continue;
+ }
+
refspec_item_init(&replace, remote->fetch.raw[i], 1);
/*
@@ -923,6 +930,12 @@ static int fetch_remote(struct remote *remote, void *cbdata)
refspec_item_clear(&replace);
}
+ /* skip remote if no refspecs to fetch */
+ if (child.args.nr - nargs <= 0) {
+ child_process_clear(&child);
+ return 0;
+ }
