Re: [PATCH 5/8] push doc: correct lies about how push refspecs work

2018-05-07 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

>  +
>  The  is often the name of the branch you would want to push, but
> -it can be any arbitrary "SHA-1 expression", such as `master~4` or
> -`HEAD` (see linkgit:gitrevisions[7]).
> +it can be any arbitrary "SHA-1 expression" referring to a branch, such
> +as `master~4` or `HEAD` (see linkgit:gitrevisions[7]). It can also
> +refer to tag objects, trees or blobs if the  is outside of
> +`refs/heads/*`.

I think the addition of "referring to a branch" here is an opposite
of an improvement.  The fact  can name any object (if outside
the refs/heads/) or any commit (otherwise) is stressed with the
added "It can also ...", which is great, but neither "master~4" nor
"HEAD" refer to a branch (they refer to a commit in terms relative
to a branch and a (psuedo)ref, respectively).  And a "SHA-1 expression"
that uses branch tips as a starting point (e.g. master~4 is "start
at the tip of master and go backwards by 4 steps) is *not* special
here.  You can spell your  side as "v2.17.0^0" for example, and
it does not refer to any branch.

> @@ -74,12 +76,24 @@ without any `` on the command line.  Otherwise, 
> missing
>  `:` means to update the same ref as the ``.
>  +
>  The object referenced by  is used to update the  reference
> -on the remote side.  By default this is only allowed if  is not
> -a tag (annotated or lightweight), and then only if it can fast-forward
> -.  By having the optional leading `+`, you can tell Git to update
> -the  ref even if it is not allowed by default (e.g., it is not a
> -fast-forward.)  This does *not* attempt to merge  into .  See
> -EXAMPLES below for details.
> +on the remote side. Whether this is allowed depends on what where in

s/what where/where/, I think.

> +`refs/*` the  reference lives. The `refs/heads/*` namespace will
> +only accept commit objects, and then only they can be
> +fast-forwarded. ...

Nicely clarified.  Excellent.

> + The `refs/tags/*` namespace will accept any kind of
> +object, but there commit objects are known as lightweight tags, and
> +any changes to them and others types of objects will be
> +rejected. ...

with s/, but there commit objects are known as lightweght tags/ the
sentence does not change any meaning?  An early part of the paragraph
made readers anticipate that they hear rules for what can go where,
and "refs/tags/ ref that point at a commit is called lightweight tag",
while it is not an incorrect statement per-se, does not belong to
these "rules".  Unless the discussing of the rules immediately
follows involves (or becomes easier to read if we use the term)
"lightweight tags", it probably is better to drop it.

> +... Finally and most confusingly, it's possible to push any type
> +of object to any namespace outside of `refs/{tags,heads}/*`, but these
> +will be treated as branches, even in the case where a tag object is
> +pushed.

I sense a confused writer, not a confusing behaviour being described
here.  If refs/poo/* is "treated as branches", because of what you
earlier said, you shouldn't be able to push a tag object in the
first place.

If refs/poo/* is meant to be lawless land where anything goes, then
saying "will be treated as branches" does not help readers.

> +... That tag object will be overwritten by another tag object (or
> +commit!) without `--force` if the new tag happens to point to a commit
> +that's a fast-forward of the commit it replaces.

If I pretend that I didn't see the "treated as branches", I fully
agree with the above description and refs/poo/* being a world
governed by random rules, and I do not think I'd be too opposed to
change it to "anything goes".  I do not think I'd be too opposed to
change it to "nothing is allowed unless forced", either, though.

> +By having the optional leading `+`, you can tell Git to update the
> + ref even if it is not allowed by its respective namespace
> +clobbering rules (e.g., it is not a fast-forward. in the case of
> +`refs/heads/*` updates) This does *not* attempt to merge  into
> +.  See EXAMPLES below for details.

Excellent.  

s/leading `+`/& to a refspec (or using "--force" command line option)/

Thanks.



[PATCH 5/8] push doc: correct lies about how push refspecs work

2018-04-29 Thread Ævar Arnfjörð Bjarmason
There's complex rules governing whether a push is allowed to take
place depending on whether we're pushing to refs/heads/*, refs/tags/*
or refs/not-that/*. See is_branch() in refs.c, and the various
assertions in refs/files-backend.c. (e.g. "trying to write non-commit
object %s to branch '%s'").

This documentation has never been quite correct, but went downhill
after dbfeddb12e ("push: require force for refs under refs/tags/",
2012-11-29) when we started claiming that  couldn't be a tag
object, which is incorrect. After some of the logic in that patch was
changed in 256b9d70a4 ("push: fix "refs/tags/ hierarchy cannot be
updated without --force"", 2013-01-16) the docs weren't updated, and
we've had some version of documentation that confused whether 
was a tag or not with whether  would accept either an annotated
tag object or the commit it points to.

This makes the intro somewhat more verbose & complex, perhaps we
should have a shorter description here and split the full complexity
into a dedicated section. Very few users will find themselves needing
to e.g. push blobs or trees to refs/custom-namespace/* (or blobs or
trees at all), and that could be covered separately as an advanced
topic.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/git-push.txt | 30 ++
 Documentation/gitrevisions.txt |  7 ---
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 5b08302fc2..806c3d8c65 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -60,8 +60,10 @@ OPTIONS[[OPTIONS]]
by a colon `:`, followed by the destination ref .
 +
 The  is often the name of the branch you would want to push, but
-it can be any arbitrary "SHA-1 expression", such as `master~4` or
-`HEAD` (see linkgit:gitrevisions[7]).
+it can be any arbitrary "SHA-1 expression" referring to a branch, such
+as `master~4` or `HEAD` (see linkgit:gitrevisions[7]). It can also
+refer to tag objects, trees or blobs if the  is outside of
+`refs/heads/*`.
 +
 The  tells which ref on the remote side is updated with this
 push. Arbitrary expressions cannot be used here, an actual ref must
@@ -74,12 +76,24 @@ without any `` on the command line.  Otherwise, 
missing
 `:` means to update the same ref as the ``.
 +
 The object referenced by  is used to update the  reference
-on the remote side.  By default this is only allowed if  is not
-a tag (annotated or lightweight), and then only if it can fast-forward
-.  By having the optional leading `+`, you can tell Git to update
-the  ref even if it is not allowed by default (e.g., it is not a
-fast-forward.)  This does *not* attempt to merge  into .  See
-EXAMPLES below for details.
+on the remote side. Whether this is allowed depends on what where in
+`refs/*` the  reference lives. The `refs/heads/*` namespace will
+only accept commit objects, and then only they can be
+fast-forwarded. The `refs/tags/*` namespace will accept any kind of
+object, but there commit objects are known as lightweight tags, and
+any changes to them and others types of objects will be
+rejected. Finally and most confusingly, it's possible to push any type
+of object to any namespace outside of `refs/{tags,heads}/*`, but these
+will be treated as branches, even in the case where a tag object is
+pushed. That tag object will be overwritten by another tag object (or
+commit!) without `--force` if the new tag happens to point to a commit
+that's a fast-forward of the commit it replaces.
++
+By having the optional leading `+`, you can tell Git to update the
+ ref even if it is not allowed by its respective namespace
+clobbering rules (e.g., it is not a fast-forward. in the case of
+`refs/heads/*` updates) This does *not* attempt to merge  into
+.  See EXAMPLES below for details.
 +
 `tag ` means the same as `refs/tags/:refs/tags/`.
 +
diff --git a/Documentation/gitrevisions.txt b/Documentation/gitrevisions.txt
index 27dec5b91d..1b79cf1634 100644
--- a/Documentation/gitrevisions.txt
+++ b/Documentation/gitrevisions.txt
@@ -19,9 +19,10 @@ walk the revision graph (such as linkgit:git-log[1]), all 
commits which are
 reachable from that commit. For commands that walk the revision graph one can
 also specify a range of revisions explicitly.
 
-In addition, some Git commands (such as linkgit:git-show[1]) also take
-revision parameters which denote other objects than commits, e.g. blobs
-("files") or trees ("directories of files").
+In addition, some Git commands (such as linkgit:git-show[1] and
+linkgit:git-push[1]) can also take revision parameters which denote
+other objects than commits, e.g. blobs ("files") or trees
+("directories of files").
 
 include::revisions.txt[]
 
-- 
2.17.0.290.gded63e768a