Re: [PATCH v2 06/10] push doc: correct lies about how push refspecs work
On Thu, Aug 30, 2018 at 5:23 PM Junio C Hamano wrote:
>
> Ævar Arnfjörð Bjarmason writes:
>
> > I.e. the non-refs/{tags,heads}/* update logic treats all updates to
> > tags/commits as branch updates. We just look at the tag v2.18.0, see you
> > want to replace it with the commit v2.19.0-rc0^{} and see "oh, that's a
> > fast-forward".
>
> In my old message you are responding to, I asked what you meant by
> "will be treated as branches", and after seeing "as branch updates"
> above, I think I know what you want the phrase to mean, namely, that
> old-to-new transition requires new to be a descendant of old. But I
> think that is weaker than what other people (including me) thinks of
> rules to update refs/heads/* hierarchy (i.e. "branch update").
>
> You are allowing to store an object that is not a commit in
> refs/blah/my-tag in your example, so it clearly does not protect the
> ref with an extra rule that applies to "branches", namely, "it has
> to be a commit."
Indeed. This was all confusing. I've reworded in something I'll send
shortly, which should address this confusion.
> > Arguably that should be changed, but I won't do that in this series.
>
> OK.
Re: [PATCH v2 06/10] push doc: correct lies about how push refspecs work
Ævar Arnfjörð Bjarmason writes:
> I.e. the non-refs/{tags,heads}/* update logic treats all updates to
> tags/commits as branch updates. We just look at the tag v2.18.0, see you
> want to replace it with the commit v2.19.0-rc0^{} and see "oh, that's a
> fast-forward".
In my old message you are responding to, I asked what you meant by
"will be treated as branches", and after seeing "as branch updates"
above, I think I know what you want the phrase to mean, namely, that
old-to-new transition requires new to be a descendant of old. But I
think that is weaker than what other people (including me) thinks of
rules to update refs/heads/* hierarchy (i.e. "branch update").
You are allowing to store an object that is not a commit in
refs/blah/my-tag in your example, so it clearly does not protect the
ref with an extra rule that applies to "branches", namely, "it has
to be a commit."
> Arguably that should be changed, but I won't do that in this series.
OK.
Re: [PATCH v2 06/10] push doc: correct lies about how push refspecs work
On Tue, Jul 31 2018, Junio C Hamano wrote:
I'm finally getting to re-rolling this. Just some inline comments.
> Æ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 expression to a commit, 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/*`.
>
> "It can also refer to..." is a good addition, but do you really want
> to make it part of this series to change/deprecate "SHA-1 expression"
> (which would certainly involve discussion on "then what to call them
> instead, now we are trying to refrain from saying SHA-1?")?
I won't change that.
>> +on the remote side. Whether this is allowed depends on 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, and any changes to them and others types of objects will be
>> +rejected. Finally, it's possible to push any type of object to any
>> +namespace outside of `refs/{tags,heads}/*`,
>
> All sound correct.
>
>> but these will be treated
>> +as branches for the purposes of whether `--force` is required, even in
>> +the case where a tag object is pushed.
>
> I am not sure what "will be treated as branches" exactly means.
> Does it mean "as if they were in refs/heads/* hierarchy?" Or
> something else?
I'll clarify this. Have rewritten most of this.
>> 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.
>
> Yup, and that is something we want to fix with a later part of this
> series.
>
For what it's worth this is not at all what I'm fixing. The new docs
describe this better, but what I'm talking about here is that you can
push a tag like git.git's v2.18.0 to refs/blah/my-tag, then you can push
v2.19.0-rc0^{} to refs/blah/my-tag and it'll be allowed as a
fast-forward, and then v2.19.0-rc1 etc.
I.e. the non-refs/{tags,heads}/* update logic treats all updates to
tags/commits as branch updates. We just look at the tag v2.18.0, see you
want to replace it with the commit v2.19.0-rc0^{} and see "oh, that's a
fast-forward".
Arguably that should be changed, but I won't do that in this series.
>> +By having the optional leading `+` to a refspec (or using `--force`
>> +command line option) 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 gives an impression that with "--force" you can put non-commit
> inside refs/heads/* hierarchy. Is that correct (if so we probably
> would want to fix that behaviour)?
I'll fix the wording, but nope, luckily you can't do that.
>> +This
>> +does *not* attempt to merge into . See EXAMPLES below for
>> +details.
>
> That is not wrong per-se, but would normal people expect a merge to
> happen upon pushing on the other side, I wonder?
>
> Thanks for cleaning up our longstanding mess.
Will fix/reword.
Re: [PATCH v2 06/10] push doc: correct lies about how push refspecs work
Æ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 expression to a commit, 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/*`.
"It can also refer to..." is a good addition, but do you really want
to make it part of this series to change/deprecate "SHA-1 expression"
(which would certainly involve discussion on "then what to call them
instead, now we are trying to refrain from saying SHA-1?")?
> +on the remote side. Whether this is allowed depends on 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, and any changes to them and others types of objects will be
> +rejected. Finally, it's possible to push any type of object to any
> +namespace outside of `refs/{tags,heads}/*`,
All sound correct.
> but these will be treated
> +as branches for the purposes of whether `--force` is required, even in
> +the case where a tag object is pushed.
I am not sure what "will be treated as branches" exactly means.
Does it mean "as if they were in refs/heads/* hierarchy?" Or
something else?
> 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.
Yup, and that is something we want to fix with a later part of this
series.
> +By having the optional leading `+` to a refspec (or using `--force`
> +command line option) 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 gives an impression that with "--force" you can put non-commit
inside refs/heads/* hierarchy. Is that correct (if so we probably
would want to fix that behaviour)?
> +This
> +does *not* attempt to merge into . See EXAMPLES below for
> +details.
That is not wrong per-se, but would normal people expect a merge to
happen upon pushing on the other side, I wonder?
Thanks for cleaning up our longstanding mess.
[PATCH v2 06/10] push doc: correct lies about how push refspecs work
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 55277a9781..fe654482dc 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -60,8 +60,9 @@ 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 expression to a commit, 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 +75,25 @@ 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 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, and any changes to them and others types of objects will be
+rejected. Finally, it's possible to push any type of object to any
+namespace outside of `refs/{tags,heads}/*`, but these will be treated
+as branches for the purposes of whether `--force` is required, 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 `+` to a refspec (or using `--force`
+command line option) 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 1f6cceaefb..d407b7dee1 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.18.0.345.g5c9ce644c3
