Message ID | 7f488eea6dbdd93ff1a97a2572de7ddb0d65b7e0.1618020225.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Maintenance: adapt custom refspecs | expand |
On 10/04/2021 03:03, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > The --prefetch option will be used by the 'prefetch' maintenance task > instead of sending refspecs explicitly across the command-line. The > intention is to modify the refspec to place all results in > refs/prefetch/ instead of anywhere else. > > Create helper method filter_prefetch_refspec() to modify a given refspec > to fit the rules expected of the prefetch task: > > * Negative refspecs are preserved. > * Refspecs without a destination are removed. > * Refspecs whose source starts with "refs/tags/" are removed. > * Other refspecs are placed within "refs/prefetch/". > > Finally, we add the 'force' option to ensure that prefetch refs are > replaced as necessary. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > Documentation/fetch-options.txt | 5 +++ > builtin/fetch.c | 56 +++++++++++++++++++++++++++++++ > t/t5582-fetch-negative-refspec.sh | 30 +++++++++++++++++ > 3 files changed, 91 insertions(+) > > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 07783deee309..9e7b4e189ce0 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -110,6 +110,11 @@ ifndef::git-pull[] > setting `fetch.writeCommitGraph`. > endif::git-pull[] > > +--prefetch:: > + Modify the configured refspec to place all refs into the > + `refs/prefetch/` namespace. See the `prefetch` task in > + linkgit:git-maintenance[1]. > + > -p:: > --prune:: > Before fetching, remove any remote-tracking references that no > diff --git a/builtin/fetch.c b/builtin/fetch.c > index 0b90de87c7a2..30856b442b79 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -48,6 +48,7 @@ enum { > static int fetch_prune_config = -1; /* unspecified */ > static int fetch_show_forced_updates = 1; > static uint64_t forced_updates_ms = 0; > +static int prefetch = 0; > static int prune = -1; /* unspecified */ > #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */ > > @@ -158,6 +159,8 @@ static struct option builtin_fetch_options[] = { > N_("do not fetch all tags (--no-tags)"), TAGS_UNSET), > OPT_INTEGER('j', "jobs", &max_jobs, > N_("number of submodules fetched in parallel")), > + OPT_BOOL(0, "prefetch", &prefetch, > + N_("modify the refspec to place all refs within refs/prefetch/")), > OPT_BOOL('p', "prune", &prune, > N_("prune remote-tracking branches no longer on remote")), > OPT_BOOL('P', "prune-tags", &prune_tags, > @@ -436,6 +439,55 @@ static void find_non_local_tags(const struct ref *refs, > oidset_clear(&fetch_oids); > } > > +static void filter_prefetch_refspec(struct refspec *rs) > +{ > + int i; > + > + if (!prefetch) > + return; > + > + for (i = 0; i < rs->nr; i++) { > + struct strbuf new_dst = STRBUF_INIT; > + char *old_dst; > + const char *sub = NULL; > + > + if (rs->items[i].negative) > + continue; > + if (!rs->items[i].dst || > + (rs->items[i].src && > + !strncmp(rs->items[i].src, "refs/tags/", 10))) { > + int j; > + > + free(rs->items[i].src); > + free(rs->items[i].dst); > + > + for (j = i + 1; j < rs->nr; j++) { > + rs->items[j - 1] = rs->items[j]; > + rs->raw[j - 1] = rs->raw[j]; > + } > + rs->nr--; Hmm, don't you need to do 'i--;' here? (Sorry in advance if this is nonsense, I am just skimming the patches without reading the whole series carefully). Maybe try a test which has an entry, which requires the 'prefetch' modification, that immediately follows a 'tag' or 'empty dst' entry. (I can't quite tell, just reading the email, whether that is covered by the tests below - so please just ignore me if it already works ;) ATB, Ramsay Jones > + continue; > + } > + > + old_dst = rs->items[i].dst; > + strbuf_addstr(&new_dst, "refs/prefetch/"); > + > + /* > + * If old_dst starts with "refs/", then place > + * sub after that prefix. Otherwise, start at > + * the beginning of the string. > + */ > + if (!skip_prefix(old_dst, "refs/", &sub)) > + sub = old_dst; > + strbuf_addstr(&new_dst, sub); > + > + rs->items[i].dst = strbuf_detach(&new_dst, NULL); > + rs->items[i].force = 1; > + > + free(old_dst); > + } > +} > + > static struct ref *get_ref_map(struct remote *remote, > const struct ref *remote_refs, > struct refspec *rs, > @@ -452,6 +504,10 @@ static struct ref *get_ref_map(struct remote *remote, > struct hashmap existing_refs; > int existing_refs_populated = 0; > > + filter_prefetch_refspec(rs); > + if (remote) > + filter_prefetch_refspec(&remote->fetch); > + > if (rs->nr) { > struct refspec *fetch_refspec; > > diff --git a/t/t5582-fetch-negative-refspec.sh b/t/t5582-fetch-negative-refspec.sh > index f34509727702..030e6f978c4e 100755 > --- a/t/t5582-fetch-negative-refspec.sh > +++ b/t/t5582-fetch-negative-refspec.sh > @@ -240,4 +240,34 @@ test_expect_success "push with matching +: and negative refspec" ' > git -C two push -v one > ' > > +test_expect_success '--prefetch correctly modifies refspecs' ' > + git -C one config --unset-all remote.origin.fetch && > + git -C one config --add remote.origin.fetch "refs/tags/*:refs/tags/*" && > + git -C one config --add remote.origin.fetch ^refs/heads/bogus/ignore && > + git -C one config --add remote.origin.fetch "refs/heads/bogus/*:bogus/*" && > + > + git tag -a -m never never-fetch-tag HEAD && > + > + git branch bogus/fetched HEAD~1 && > + git branch bogus/ignore HEAD && > + > + git -C one fetch --prefetch --no-tags && > + test_must_fail git -C one rev-parse never-fetch-tag && > + git -C one rev-parse refs/prefetch/bogus/fetched && > + test_must_fail git -C one rev-parse refs/prefetch/bogus/ignore && > + > + # correctly handle when refspec set becomes empty > + # after removing the refs/tags/* refspec. > + git -C one config --unset-all remote.origin.fetch && > + git -C one config --add remote.origin.fetch "refs/tags/*:refs/tags/*" && > + > + git -C one fetch --prefetch --no-tags && > + test_must_fail git -C one rev-parse never-fetch-tag && > + > + # The refspec for refs that are not fully qualified > + # are filtered multiple times. > + git -C one rev-parse refs/prefetch/bogus/fetched && > + test_must_fail git -C one rev-parse refs/prefetch/bogus/ignore > +' > + > test_done >
On 4/11/21 5:09 PM, Ramsay Jones wrote:> On 10/04/2021 03:03, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <dstolee@microsoft.com> >> + for (i = 0; i < rs->nr; i++) { >> + struct strbuf new_dst = STRBUF_INIT; >> + char *old_dst; >> + const char *sub = NULL; >> + >> + if (rs->items[i].negative) >> + continue; >> + if (!rs->items[i].dst || >> + (rs->items[i].src && >> + !strncmp(rs->items[i].src, "refs/tags/", 10))) { >> + int j; >> + >> + free(rs->items[i].src); >> + free(rs->items[i].dst); >> + >> + for (j = i + 1; j < rs->nr; j++) { >> + rs->items[j - 1] = rs->items[j]; >> + rs->raw[j - 1] = rs->raw[j]; >> + } >> + rs->nr--; > > Hmm, don't you need to do 'i--;' here? > > (Sorry in advance if this is nonsense, I am just skimming the > patches without reading the whole series carefully). > > Maybe try a test which has an entry, which requires the 'prefetch' > modification, that immediately follows a 'tag' or 'empty dst' entry. > (I can't quite tell, just reading the email, whether that is covered > by the tests below - so please just ignore me if it already works ;) You are absolutely right! Thanks. -Stolee
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 07783deee309..9e7b4e189ce0 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -110,6 +110,11 @@ ifndef::git-pull[] setting `fetch.writeCommitGraph`. endif::git-pull[] +--prefetch:: + Modify the configured refspec to place all refs into the + `refs/prefetch/` namespace. See the `prefetch` task in + linkgit:git-maintenance[1]. + -p:: --prune:: Before fetching, remove any remote-tracking references that no diff --git a/builtin/fetch.c b/builtin/fetch.c index 0b90de87c7a2..30856b442b79 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -48,6 +48,7 @@ enum { static int fetch_prune_config = -1; /* unspecified */ static int fetch_show_forced_updates = 1; static uint64_t forced_updates_ms = 0; +static int prefetch = 0; static int prune = -1; /* unspecified */ #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */ @@ -158,6 +159,8 @@ static struct option builtin_fetch_options[] = { N_("do not fetch all tags (--no-tags)"), TAGS_UNSET), OPT_INTEGER('j', "jobs", &max_jobs, N_("number of submodules fetched in parallel")), + OPT_BOOL(0, "prefetch", &prefetch, + N_("modify the refspec to place all refs within refs/prefetch/")), OPT_BOOL('p', "prune", &prune, N_("prune remote-tracking branches no longer on remote")), OPT_BOOL('P', "prune-tags", &prune_tags, @@ -436,6 +439,55 @@ static void find_non_local_tags(const struct ref *refs, oidset_clear(&fetch_oids); } +static void filter_prefetch_refspec(struct refspec *rs) +{ + int i; + + if (!prefetch) + return; + + for (i = 0; i < rs->nr; i++) { + struct strbuf new_dst = STRBUF_INIT; + char *old_dst; + const char *sub = NULL; + + if (rs->items[i].negative) + continue; + if (!rs->items[i].dst || + (rs->items[i].src && + !strncmp(rs->items[i].src, "refs/tags/", 10))) { + int j; + + free(rs->items[i].src); + free(rs->items[i].dst); + + for (j = i + 1; j < rs->nr; j++) { + rs->items[j - 1] = rs->items[j]; + rs->raw[j - 1] = rs->raw[j]; + } + rs->nr--; + continue; + } + + old_dst = rs->items[i].dst; + strbuf_addstr(&new_dst, "refs/prefetch/"); + + /* + * If old_dst starts with "refs/", then place + * sub after that prefix. Otherwise, start at + * the beginning of the string. + */ + if (!skip_prefix(old_dst, "refs/", &sub)) + sub = old_dst; + strbuf_addstr(&new_dst, sub); + + rs->items[i].dst = strbuf_detach(&new_dst, NULL); + rs->items[i].force = 1; + + free(old_dst); + } +} + static struct ref *get_ref_map(struct remote *remote, const struct ref *remote_refs, struct refspec *rs, @@ -452,6 +504,10 @@ static struct ref *get_ref_map(struct remote *remote, struct hashmap existing_refs; int existing_refs_populated = 0; + filter_prefetch_refspec(rs); + if (remote) + filter_prefetch_refspec(&remote->fetch); + if (rs->nr) { struct refspec *fetch_refspec; diff --git a/t/t5582-fetch-negative-refspec.sh b/t/t5582-fetch-negative-refspec.sh index f34509727702..030e6f978c4e 100755 --- a/t/t5582-fetch-negative-refspec.sh +++ b/t/t5582-fetch-negative-refspec.sh @@ -240,4 +240,34 @@ test_expect_success "push with matching +: and negative refspec" ' git -C two push -v one ' +test_expect_success '--prefetch correctly modifies refspecs' ' + git -C one config --unset-all remote.origin.fetch && + git -C one config --add remote.origin.fetch "refs/tags/*:refs/tags/*" && + git -C one config --add remote.origin.fetch ^refs/heads/bogus/ignore && + git -C one config --add remote.origin.fetch "refs/heads/bogus/*:bogus/*" && + + git tag -a -m never never-fetch-tag HEAD && + + git branch bogus/fetched HEAD~1 && + git branch bogus/ignore HEAD && + + git -C one fetch --prefetch --no-tags && + test_must_fail git -C one rev-parse never-fetch-tag && + git -C one rev-parse refs/prefetch/bogus/fetched && + test_must_fail git -C one rev-parse refs/prefetch/bogus/ignore && + + # correctly handle when refspec set becomes empty + # after removing the refs/tags/* refspec. + git -C one config --unset-all remote.origin.fetch && + git -C one config --add remote.origin.fetch "refs/tags/*:refs/tags/*" && + + git -C one fetch --prefetch --no-tags && + test_must_fail git -C one rev-parse never-fetch-tag && + + # The refspec for refs that are not fully qualified + # are filtered multiple times. + git -C one rev-parse refs/prefetch/bogus/fetched && + test_must_fail git -C one rev-parse refs/prefetch/bogus/ignore +' + test_done