Re: [PATCH v4 4/6] push doc: correct lies about how push refspecs work
On Fri, Aug 31, 2018 at 6:24 PM Junio C Hamano wrote: > > Ævar Arnfjörð Bjarmason writes: > > > On Thu, Aug 30 2018, Ævar Arnfjörð Bjarmason wrote: > > > > [Notes to self] > > ... > > > > Later below this we say: > > > > Pushing an empty allows you to delete the ref from the > > remote repository. > > > > Which, perhaps given the discussion of deletions as updates, should be > > mentioned earlier in some way, i.e. should we just say above all these > > rules that by "update" we mean non-deletions? > > You raised good points. The rule that applies to deletion is quite > different from the one for update, we want to make sure readers know > updates and deletions are different. As the rule for deletion is a > lot simpler (i.e. you can always delete unless a configuration or > pre-receive says otherwise), perhaps it would be sufficient to give > the rules for deletion upfront in one section, and then start the > section(s) for update with a phrase like "rules for accepting > updates are follows" after that. Yeah, that was the plan. I'll do that.
Re: [PATCH v4 4/6] push doc: correct lies about how push refspecs work
Ævar Arnfjörð Bjarmason writes: > On Thu, Aug 30 2018, Ævar Arnfjörð Bjarmason wrote: > > [Notes to self] > ... > > Later below this we say: > > Pushing an empty allows you to delete the ref from the > remote repository. > > Which, perhaps given the discussion of deletions as updates, should be > mentioned earlier in some way, i.e. should we just say above all these > rules that by "update" we mean non-deletions? You raised good points. The rule that applies to deletion is quite different from the one for update, we want to make sure readers know updates and deletions are different. As the rule for deletion is a lot simpler (i.e. you can always delete unless a configuration or pre-receive says otherwise), perhaps it would be sufficient to give the rules for deletion upfront in one section, and then start the section(s) for update with a phrase like "rules for accepting updates are follows" after that.
Re: [PATCH v4 4/6] push doc: correct lies about how push refspecs work
On Thu, Aug 30 2018, Ævar Arnfjörð Bjarmason wrote:
[Notes to self]
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 55277a9781..0f03d36f1e 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -74,12 +74,41 @@ 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 as described in detail below. Any
> +such update does *not* attempt to merge into . See EXAMPLES
> +below for details.
> ++
> +The `refs/heads/*` namespace will only accept commit objects, and 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
> +be rejected.
> ++
Both of these should carve out some mention for the "deletion" aspect of
"updates". I.e. you don't need --force to delete.
> +It's possible to push any type of object to any namespace outside of
> +`refs/{tags,heads}/*`. In the case of tags and commits, these will be
> +treated as if they were the commits inside `refs/heads/*` for the
> +purposes of whether the update is allowed.
> ++
> +I.e. a fast-forward of commits and tags outside `refs/{tags,heads}/*`
> +is allowed, even in cases where what's being fast-forwarded is not a
> +commit, but a tag object which happens to point to a new commit which
> +is a fast-forward of the commit the last tag (or commit) it's
> +replacing. Replacing a tag with an entirely different tag is also
> +allowed, if it points to the same commit, as well as pushing a peeled
> +tag, i.e. pushing the commit that existing tag object points to, or a
> +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.
> ++
> +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 ` means the same as `refs/tags/:refs/tags/`.
> +
Later below this we say:
Pushing an empty allows you to delete the ref from the
remote repository.
Which, perhaps given the discussion of deletions as updates, should be
mentioned earlier in some way, i.e. should we just say above all these
rules that by "update" we mean non-deletions?
Re: [PATCH v4 4/6] push doc: correct lies about how push refspecs work
Ævar Arnfjörð Bjarmason writes:
> +on the remote side. Whether this is allowed depends on where in
> +`refs/*` the reference lives as described in detail below. Any
> +such update does *not* attempt to merge into . See EXAMPLES
> +below for details.
> ++
> +The `refs/heads/*` namespace will only accept commit objects, and 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
> +be rejected.
> ++
> +It's possible to push any type of object to any namespace outside of
> +`refs/{tags,heads}/*`. In the case of tags and commits, these will be
> +treated as if they were the commits inside `refs/heads/*` for the
> +purposes of whether the update is allowed.
> ++
> +I.e. a fast-forward of commits and tags outside `refs/{tags,heads}/*`
> +is allowed, even in cases where what's being fast-forwarded is not a
> +commit, but a tag object which happens to point to a new commit which
> +is a fast-forward of the commit the last tag (or commit) it's
> +replacing. Replacing a tag with an entirely different tag is also
> +allowed, if it points to the same commit, as well as pushing a peeled
> +tag, i.e. pushing the commit that existing tag object points to, or a
> +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.
> ++
> +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.
This, while some may find it overly long, is quite clear, compared
to the current text and to the previous rounds of this patch, and I
found it very much readable.
[PATCH v4 4/6] 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 | 41 +-
Documentation/gitrevisions.txt | 7 +++---
2 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 55277a9781..0f03d36f1e 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -74,12 +74,41 @@ 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 as described in detail below. Any
+such update does *not* attempt to merge into . See EXAMPLES
+below for details.
++
+The `refs/heads/*` namespace will only accept commit objects, and 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
+be rejected.
++
+It's possible to push any type of object to any namespace outside of
+`refs/{tags,heads}/*`. In the case of tags and commits, these will be
+treated as if they were the commits inside `refs/heads/*` for the
+purposes of whether the update is allowed.
++
+I.e. a fast-forward of commits and tags outside `refs/{tags,heads}/*`
+is allowed, even in cases where what's being fast-forwarded is not a
+commit, but a tag object which happens to point to a new commit which
+is a fast-forward of the commit the last tag (or commit) it's
+replacing. Replacing a tag with an entirely different tag is also
+allowed, if it points to the same commit, as well as pushing a peeled
+tag, i.e. pushing the commit that existing tag object points to, or a
+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.
++
+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 ` 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.19.0.rc1.350.ge57e33dbd1
