[PATCH v5 0/9] git fetch" should not clobber existing tags without --force

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

Addresses Junio's comments to v4, and I had a few fixes of my own. I
don't know if this range-diff is more or less readble than just
re-reading it, but here goes:

 1:  d05fd561f3 =  1:  d05fd561f3 fetch: change "branch" to "reference" in --force -h output
 -:  ---------- >  2:  28275baca2 push tests: make use of unused $1 in test description
 2:  013ecd83b3 !  3:  834501afdc push tests: correct quoting in interpolated string
    @@ -1,24 +1,11 @@
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    -    push tests: correct quoting in interpolated string
    +    push tests: use spaces in interpolated string
     
    -    The quoted -m'msg' option is passed as a string to another function,
    -    where due to interpolation it'll end up meaning the same as if we did
    -    just did -m'msg' here.
    -
    -    In [1] this was pointed out to me, but in submitting [2] the patches I
    -    missed this (since it was feedback on another patch I was holding
    -    off), so this logic error landed in 380efb65df ("push tests: assert
    -    re-pushing annotated tags", 2018-07-31).
    -
    -    Let's just remove the quotes, and use a string that doesn't need to be
    -    quoted (-mtag.message is a bit less confusing than -mmsg). I could try
    -    to chase after getting the quoting right here with multiple
    -    backslashes, but I don't think it's worth it, and it makes things much
    -    less readable.
    -
    -    1. https://public-inbox.org/git/xmqq4lgfcn5a.fsf@gitster-ct.c.googlers.com/
    -    2. https://public-inbox.org/git/20180813192249.27585-1-avarab@gmail.com/
    +    The quoted -m'msg' option would mean the same as -mmsg when passed
    +    through the test_force_push_tag helper. Let's instead use a string
    +    with spaces in it, to have a working example in case we need to pass
    +    other whitespace-delimited arguments to git-tag.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ -30,7 +17,7 @@
      
      test_force_push_tag "lightweight tag" "-f"
     -test_force_push_tag "annotated tag" "-f -a -m'msg'"
    -+test_force_push_tag "annotated tag" "-f -a -mtag.message"
    ++test_force_push_tag "annotated tag" "-f -a -m'tag message'"
      
      test_expect_success 'push --porcelain' '
      	mk_empty testrepo &&
 3:  2d216a7ef6 !  4:  5f85542bb2 fetch tests: add a test for clobbering tag behavior
    @@ -14,7 +14,7 @@
      +++ b/t/t5516-fetch-push.sh
     @@
      test_force_push_tag "lightweight tag" "-f"
    - test_force_push_tag "annotated tag" "-f -a -mtag.message"
    + test_force_push_tag "annotated tag" "-f -a -m'tag message'"
      
     +test_force_fetch_tag () {
     +	tag_type_description=$1
    @@ -38,7 +38,7 @@
     +}
     +
     +test_force_fetch_tag "lightweight tag" "-f"
    -+test_force_fetch_tag "annotated tag" "-f -a -mtag.message"
    ++test_force_fetch_tag "annotated tag" "-f -a -m'tag message'"
     +
      test_expect_success 'push --porcelain' '
      	mk_empty testrepo &&
 -:  ---------- >  5:  6906d5a84d push doc: remove confusing mention of remote merger
 -:  ---------- >  6:  a16a9c2d7f push doc: move mention of "tag <tag>" later in the prose
 4:  b751e80b00 !  7:  9f8785e01a push doc: correct lies about how push refspecs work
    @@ -38,18 +38,20 @@
     -a tag (annotated or lightweight), and then only if it can fast-forward
     -<dst>.  By having the optional leading `+`, you can tell Git to update
     -the <dst> ref even if it is not allowed by default (e.g., it is not a
    --fast-forward.)  This does *not* attempt to merge <src> into <dst>.  See
    --EXAMPLES below for details.
    +-fast-forward.).
    +-+
    +-Pushing an empty <src> allows you to delete the <dst> ref from
    +-the remote repository.
     +on the remote side. Whether this is allowed depends on where in
    -+`refs/*` the <dst> reference lives as described in detail below. Any
    -+such update does *not* attempt to merge <src> into <dst>. See EXAMPLES
    -+below for details.
    ++`refs/*` the <dst> reference lives as described in detail below, in
    ++those sections "update" means any modifications except deletes, which
    ++as noted after the next few sections are treated differently.
     ++
    -+The `refs/heads/*` namespace will only accept commit objects, and only
    -+if they can be fast-forwarded.
    ++The `refs/heads/*` namespace will only accept commit objects, and
    ++updates only if they can be fast-forwarded.
     ++
     +The `refs/tags/*` namespace will accept any kind of object (as
    -+commits, trees and blobs can be tagged), and any changes to them will
    ++commits, trees and blobs can be tagged), and any updates to them will
     +be rejected.
     ++
     +It's possible to push any type of object to any namespace outside of
    @@ -67,17 +69,26 @@
     +new tag object which an existing commit points to.
     ++
     +Tree and blob objects outside of `refs/{tags,heads}/*` will be treated
    -+the same way as if they were inside `refs/tags/*`, any modification of
    -+them will be rejected.
    ++the same way as if they were inside `refs/tags/*`, any update of them
    ++will be rejected.
     ++
     +All of the rules described above about what's not allowed as an update
     +can be overridden by adding an the optional leading `+` to a refspec
     +(or using `--force` command line option). The only exception to this
     +is that no amount of forcing will make the `refs/heads/*` namespace
    -+accept a non-commit object.
    - +
    - `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
    ++accept a non-commit object. Hooks and configuration can also override
    ++or amend these rules, see e.g. `receive.denyNonFastForwards` in
    ++linkgit:git-config[1] and`pre-receive` and `update` in
    ++linkgit:githooks[5].
    +++
    ++Pushing an empty <src> allows you to delete the <dst> ref from the
    ++remote repository. Deletions are always accepted without a leading `+`
    ++in the refspec (or `--force`), except when forbidden by configuration
    ++or hooks. See `receive.denyDeletes` in linkgit:git-config[1] and
    ++`pre-receive` and `update` in linkgit:githooks[5].
      +
    + The special refspec `:` (or `+:` to allow non-fast-forward updates)
    + directs Git to push "matching" branches: for every branch that exists on
     
      diff --git a/Documentation/gitrevisions.txt b/Documentation/gitrevisions.txt
      --- a/Documentation/gitrevisions.txt
 5:  b120051957 =  8:  3e90699b9f fetch: document local ref updates with/without --force
 6:  25df331fce !  9:  0e183b6f23 fetch: stop clobbering existing tags without --force
    @@ -66,13 +66,24 @@
     +Until Git version 2.20, and unlike when pushing with
     +linkgit:git-push[1], any updates to `refs/tags/*` would be accepted
     +without `+` in the refspec (or `--force`). The receiving promiscuously
    -+considered all tag updates from a remote to be forced fetches. Since
    -+Git version 2.20 updates to `refs/tags/*` work the same way as when
    -+pushing. I.e. any updates will be rejected without `+` in the refspec
    -+(or `--force`).
    ++considered all tag updates from a remote to be forced fetches.  Since
    ++Git version 2.20, fetching to update `refs/tags/*` work the same way
    ++as when pushing. I.e. any updates will be rejected without `+` in the
    ++refspec (or `--force`).
      +
      Unlike when pushing with linkgit:git-push[1], any updates outside of
      `refs/{tags,heads}/*` will be accepted without `+` in the refspec (or
    +@@
    + a commit for another commit that's doesn't have the previous commit as
    + an ancestor etc.
    + +
    ++Unlike when pushing with linkgit:git-push[1], there is no
    ++configuration which'll amend these rules, and nothing like a
    ++`pre-fetch` hook analogous to the `pre-receive` hook.
    +++
    + As with pushing with linkgit:git-push[1], all of the rules described
    + above about what's not allowed as an update can be overridden by
    + adding an the optional leading `+` to a refspec (or using `--force`
     
      diff --git a/builtin/fetch.c b/builtin/fetch.c
      --- a/builtin/fetch.c

Ævar Arnfjörð Bjarmason (9):
  fetch: change "branch" to "reference" in --force -h output
  push tests: make use of unused $1 in test description
  push tests: use spaces in interpolated string
  fetch tests: add a test for clobbering tag behavior
  push doc: remove confusing mention of remote merger
  push doc: move mention of "tag <tag>" later in the prose
  push doc: correct lies about how push refspecs work
  fetch: document local ref updates with/without --force
  fetch: stop clobbering existing tags without --force

 Documentation/fetch-options.txt    | 15 +++++---
 Documentation/git-push.txt         | 57 ++++++++++++++++++++++++------
 Documentation/gitrevisions.txt     |  7 ++--
 Documentation/pull-fetch-param.txt | 39 +++++++++++++++++---
 builtin/fetch.c                    | 20 +++++++----
 t/t5516-fetch-push.sh              | 29 +++++++++++++--
 t/t5612-clone-refspec.sh           |  4 +--
 7 files changed, 136 insertions(+), 35 deletions(-)

-- 
2.19.0.rc1.350.ge57e33dbd1