Re: [PATCH 11/15] fetch --prune: prune only based on explicit refspecs
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 24, 2013, 21:11 UTC
- Message-ID
- <xmqqiowmml0y.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1382543448-2586-12-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> ... > Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Everything in the proposed log message made sense to me.
Show 10 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt > index d4d93c9..83c1700 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2086,7 +2086,7 @@ remote.<name>.vcs:: > remote.<name>.prune:: > When set to true, fetching from this remote by default will also > remove any remote-tracking branches which no longer exist on the > - remote (as if the `--prune` option was give on the command line). > + remote (as if the `--prune` option was given on the command line).
Shouldn't we stop saying "branches" here?
Show 11 quoted lines
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 0e6d2ac..5d12219 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -41,8 +41,14 @@ ifndef::git-pull[] > > -p:: > --prune:: > - After fetching, remove any remote-tracking branches which > - no longer exist on the remote. > + After fetching, remove any remote-tracking branches that
Likewise. This is a lot more important than the one in remote.<name>.prune documentation, as the next sentence "Tags are not subject to ..." implies that they fall into the same category as what gets pruned here, i.e. "remote-tracking branches" in the above sentence, but nobody calls refs/tags/v1.0.0 a "remote-tracking branch" even if it came from your 'origin'.
> + no longer exist on the remote. Tags are not subject to > + pruning in the usual case that they are fetched because of the > + --tags option or remote.<name>.tagopt.
We should mention the most usual case tags are fetched, before mentioning the case the unusual option "--tags" was used from the command line or .tagopt configuration was used. Namely, when the tags are automatically followed.
Show 5 quoted lines
> + However, if tags are > + fetched due to an explicit refspec (either on the command line > + or in the remote configuration, for example if the remote was > + cloned with the --mirror option), then they are also subject > + to pruning.
Very good.
Show 9 quoted lines
> @@ -63,7 +69,10 @@ ifndef::git-pull[] > --tags:: > This is a short-hand requesting that all tags be fetched from > the remote in addition to whatever else is being fetched. It > - is similar to using the refspec `refs/tags/*:refs/tags/*`. > + is similar to using the refspec `refs/tags/*:refs/tags/*`, > + except that it doesn't subject tags to pruning, regardless of > + a --prune option or the configuration settings of fetch.prune > + or remote.<name>.prune.
Using --tags is not similar to using refs/tags/*:refs/tags/* after the previous patch already; "git fetch origin --tags" and "git fetch origin refs/tags/*:refs/tags/*" are vastly different and that was the whole point of the previous step. And that "calling something not so similar similar" needs to be fixed further here to clarify that they are not similar in yet another way.
We should just lose "It is similar to using" from 10/15 and start over, perhaps? Add the first paragraph of the below in 10/15 and add the rest in 11/15, or something.
--tags::
Request that all tags be fetched from the remote
under the same name (i.e. `refs/tags/X` is created in
our repository by copying their `refs/tags/X`), in
addition to whatever is fetched by the same `git
fetch` command without this option on the command
line.
+
When `refs/tags/*` hierarchy from the remote is copied only
because this option was given, they are not subject to be
pruned when `--prune` option (or configuration variables
like `fetch.prune` or `remote.<name>.prune`) is in effect.That would make it clear that they are subject to pruning when --mirror or an explicit refspec refs/tags/*:refs/tags/* is given, as tags are not fetched "only because of --tags" in such cases.
Show 51 quoted lines
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 7edb1ea..47b63a7 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -829,38 +829,17 @@ static int do_fetch(struct transport *transport,
> goto cleanup;
> }
> if (prune) {
> - struct refspec *prune_refspecs;
> - int prune_refspec_count;
> -
> + /*
> + * We only prune based on refspecs specified
> + * explicitly (via command line or configuration); we
> + * don't care whether --tags was specified.
> + */
> if (ref_count) {
> - prune_refspecs = refs;
> - prune_refspec_count = ref_count;
> - } else {
> - prune_refspecs = transport->remote->fetch;
> - prune_refspec_count = transport->remote->fetch_refspec_nr;
> - }
> -
> - if (tags == TAGS_SET) {
> - /*
> - * --tags was specified. Pretend that the user also
> - * gave us the canonical tags refspec
> - */
> - const char *tags_str = "refs/tags/*:refs/tags/*";
> - struct refspec *tags_refspec, *refspec;
> -
> - /* Copy the refspec and add the tags to it */
> - refspec = xcalloc(prune_refspec_count + 1, sizeof(*refspec));
> - tags_refspec = parse_fetch_refspec(1, &tags_str);
> - memcpy(refspec, prune_refspecs, prune_refspec_count * sizeof(*refspec));
> - memcpy(&refspec[prune_refspec_count], tags_refspec, sizeof(*refspec));
> -
> - prune_refs(refspec, prune_refspec_count + 1, ref_map);
> -
> - /* The rest of the strings belong to fetch_one */
> - free_refspec(1, tags_refspec);
> - free(refspec);
> + prune_refs(refs, ref_count, ref_map);
> } else {
> - prune_refs(prune_refspecs, prune_refspec_count, ref_map);
> + prune_refs(transport->remote->fetch,
> + transport->remote->fetch_refspec_nr,
> + ref_map);
> }
> }Nice.