Re: [PATCH 11/15] fetch --prune: prune only based on explicit refspecs

2013-10-28 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 On 10/24/2013 11:11 PM, Junio C Hamano wrote:
 ...
 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.

 I see your point.  What do you think about the following version, which
 is a bit more compact and refers the reader to --prune for the full story:

 -t::
 --tags::
   Fetch all tags from the remote (i.e., fetch remote tags
   `refs/tags/*` into local tags with the same name), in addition
   to whatever else would otherwise be fetched.  Using this
   option does not subject tags to pruning, even if --prune is
   used (though tags may be pruned anyway if they are also the
   destination of an explicit refspec; see '--prune').

I like the first sentence of yours better. The second sentence feels
somewhat iffy, though. --tags refs/tags/*:refs/tags/* will allow
tags to be pruned, so s/option does not/option alone does not/ needs
to be done to be precise, at least.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 11/15] fetch --prune: prune only based on explicit refspecs

2013-10-26 Thread Michael Haggerty
On 10/24/2013 11:11 PM, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 
 ...
 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 
 Everything in the proposed log message made sense to me.
 
 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?
 
 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'.

OK, I will change both of the above from remote-tracking branches to
remote-tracking references.

 +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.

OK, I will change this in the next draft.

 @@ -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.

I see your point.  What do you think about the following version, which
is a bit more compact and refers the reader to --prune for the full story:

-t::
--tags::
Fetch all tags from the remote (i.e., fetch remote tags
`refs/tags/*` into local tags with the same name), in addition
to whatever else would otherwise be fetched.  Using this
option does not subject tags to pruning, even if --prune is
used (though tags may be pruned anyway if they are also the
destination of an explicit refspec; see '--prune').

I also want to improve the description of tag auto-following in general.
 I will send a re-rolled patch series in the next couple of days.

Thanks for your prompt and helpful advice!

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 11/15] fetch --prune: prune only based on explicit refspecs

2013-10-24 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 ...
 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu

Everything in the proposed log message made sense to me.

 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?

 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.

 + 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.

 @@ -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.

 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) {
 -

[PATCH 11/15] fetch --prune: prune only based on explicit refspecs

2013-10-23 Thread Michael Haggerty
The old behavior of fetch --prune was to prune whatever was being
fetched.  In particular, fetch --prune --tags caused tags not only
to be fetched, but also to be pruned.  This is inappropriate because
there is only one tags namespace that is shared among the local
repository and all remotes.  Therefore, if the user defines a local
tag and then runs git fetch --prune --tags, then the local tag is
deleted.  Moreover, --prune and --tags can also be configured via
fetch.prune / remote.name.prune and remote.name.tagopt, making it
even less obvious that an invocation of git fetch could result in
tag lossage.

Since the command git remote update invokes git fetch, it had the
same problem.

The command git remote prune, on the other hand, disregarded the
setting of remote.name.tagopt, and so its behavior was inconsistent
with that of the other commands.

So the old behavior made it too easy to lose tags.  To fix this
problem, change fetch --prune to prune references based only on
refspecs specified explicitly by the user, either on the command line
or via remote.name.fetch.  Thus, tags are no longer made subject to
pruning by the --tags option or the remote.name.tagopt setting.

However, tags *are* still subject to pruning if they are fetched as
part of a refspec, and that is good.  For example:

* On the command line,

  git fetch --prune 'refs/tags/*:refs/tags/*'

  causes tags, and only tags, to be fetched and pruned, and is
  therefore a simple way for the user to get the equivalent of the old
  behavior of --prune --tag.

* For a remote that was configured with the --mirror option, the
  configuration is set to include

  [remote name]
  fetch = +refs/*:refs/*

  , which causes tags to be subject to pruning along with all other
  references.  This is the behavior that will typically be desired for
  a mirror.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 Documentation/config.txt|  2 +-
 Documentation/fetch-options.txt | 15 ---
 builtin/fetch.c | 39 +--
 t/t5510-fetch.sh| 10 +-
 4 files changed, 27 insertions(+), 39 deletions(-)

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).
Overrides `fetch.prune` settings, if any.
 
 remotes.group::
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
+   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.  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.
 endif::git-pull[]
 
 ifdef::git-pull[]
@@ -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.
 
 --recurse-submodules[=yes|on-demand|no]::
This option controls if and under what conditions new commits of
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;
-   }