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

From: Junio C Hamano <gitster@pobox.com>
Date: May 8, 2018, 05:14 UTC
Message-ID: <xmqqzi1alodz.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180429202100.32353-6-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 8 quoted lines

>  +
>  The <src> 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 <dst> is outside of
> +`refs/heads/*`.

I think the addition of "referring to a branch" here is an opposite of an improvement. The fact <src> 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 <src> side as "v2.17.0^0" for example, and it does not refer to any branch.

Show 11 quoted lines

> @@ -74,12 +76,24 @@ without any `<refspec>` on the command line.  Otherwise, missing
>  `:<dst>` means to update the same ref as the `<src>`.
>  +
>  The object referenced by <src> is used to update the <dst> reference
> -on the remote side.  By default this is only allowed if <dst> is not
> -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.
> +on the remote side. Whether this is allowed depends on what where in

s/what where/where/, I think.

> +`refs/*` the <dst> 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.

Show 5 quoted lines

> +By having the optional leading `+`, you can tell Git to update the
> +<dst> 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 <src> into
> +<dst>.  See EXAMPLES below for details.

Excellent.

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

Thanks.