doc: git-push: clarify <refspec> section #1973
Open
+102
−96
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Right now the rules for when a `git push` is allowed are buried at the bottom of the description of `<refspec>`. Put them in their own section so that we can reference them from `--force` and give some context for why they exist. Having the "PUSH RULES" section also lets us be a little bit more specific with the rule in `--force`: we can just focus on the rule for pushing for a branch (which is likely the one that's most relevant) and leave the details about what happens when you push to a tag or a ref that isn't a branch to the later section. Signed-off-by: Julia Evans <[email protected]>
From user feedback, there was a request for examples, as well as a comment that one person found "If git push [<repository>] without any <refspec> argument is set to update some ref at the destination with <src> with remote.<repository>.push configuration variable..." impossible to understand. To make the section easier to navigate, create a list of every possible refspec form, with examples for each form as well as 2 forms which were previously missing (patterns and negative refspecs). Made a few changes to use more familiar language, but ultimately refspecs are a pretty advanced feature so I've mostly left the terminology alone. Signed-off-by: Julia Evans <[email protected]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
Documentation/git-push.adoc
Outdated
| * Other ambiguity resolutions might be added in the future, but for | ||
| now any other cases will error out with an error indicating what we | ||
| tried, and depending on the `advice.pushUnqualifiedRefname` | ||
| configuration (see linkgit:git-config[1]) suggest what refs/ |
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Julia Evans via GitGitGadget" <[email protected]> writes:
> --force::
> - Usually, the command refuses to update a remote ref that is
> - not an ancestor of the local ref used to overwrite it.
> - Also, when `--force-with-lease` option is used, the command refuses
> - to update a remote ref whose current value does not match
> - what is expected.
> + Usually, `git push` will refuse to update a branch that is not an
> + ancestor of the local branch or commit being pushed.
I read this as "there are two conditions, and satisifying only one of
them is sufficient for the push to be allowed. (1) the local branch
is a decendant of the remote branch being updated, or (2) the commit
we push to update the remote branch is a descendant of the remote
branch being updated".
But of course that is not what you wanted to say. (1) would mean
$ git reset origin/foo && git push origin anything:foo
would allow us to push literally anything to overwrite origin's foo
branch.
I think
"... not an ancestor of the commit being pushed to update it."
would be a way to avoid such confusion.
The problem the original description has is the phrase "the local
ref used to overwrite it". It wasn't as commonly done to push a
specific commit that may not necessarily at the tip of the branch
back when this paragraph was written.
> -This flag disables these checks, and can cause the remote repository
> -to lose commits; use it with care.
> +This flag disables that check, the other safety checks in PUSH RULES
> +below, and the checks in --force-with-lease. It can cause the remote
> +repository to lose commits; use it with care.
OK.
> +PUSH RULES
> +----------
> +
> +As a safety feature, the `git push` command only allows certain kinds of
> +updates to prevent you from accidentally losing data on the remote.
> +
> +Because branches and tags are intended to be used differently, the
> +safety rules for pushing to a branch are different from the rules
> +for pushing to a tag. In the following rules "update" means any
> +modifications except deletes. Deletions are always allowed, except when
> +forbidden by configuration or hooks.
One important operation is omitted. "update" does not include
"create" in the following, no? Obviously since refs/tags/ would
never take any "update" (unless forced), if "create" were thrown
into the same category as "update", you cannot push a new tag out.
So, next to "Deletions are always allowed", shouldn't we describe
what rules apply to creations? I presume that they are also always
allowed?
> +1. If the push destination is a **branch** (`refs/heads/*`): only
> + fast-forward updates are allowed: the destination must be an ancestor
> + of the source commit. The source must be a commit.
Reads very well and also correct.
The first colon might be acceptable (I find it a bit odd, though).
The second colon is very weird. ": only" -> ", only" & "allowed:
the" -> "allowed. The", perhaps?
> +2. If the push destination is a **tag** (`refs/tags/*`): all updates will
> + be rejected. The source can be any object
> + (since commits, trees and blobs can be tagged).
Again, I might prefer ":" -> ",". I cannot decide which I prefer
between "all updates will be rejected" and "by default no updates
are allowed". Either should be OK, so let's take what has already
been written.
The second sentence is not wrong per-se, and I can see that this was
inherited from the original, but gives me a strange aftertaste. When
you list object types in the context of "tag" and have only commit,
tree, and blob, a little voice in the back of my head asks "oh, what
happend to tags?". It is made a bit worse with the phrase "can be
tagged", as it typically means either (1) to create an annotated or
signed tag object, or (2) to create a ref in refs/tags/ hierarchy
locally, but usually you do not think of pushing to refs/tags/
hierarchy as "tagging that object remotely".
I think the untold assumption here is that refs/tags/foo at the
remote is being updated most of the time from refs/tags/foo we have
locally, and "any kind of object can be tagged" is trying to say
that refs/tags/foo we have locally can be an object of any type, as
the act of creating a ref "refs/tags/foo" and pointing it directly
at an object is "to create a light-weight tag" for the object.
Since we can have not just tags but any kind of object locally
(because any object "can be tagged"), a push can ask object of any
kind to be pushed to refs/tags/* hierarchy. But it is an awkward
concept to explain.
Would side-stepping what exactly "tagging a thing" means, and
phrasing it like this
The source is not limited to an annotated or signed tag object,
but can be a commit, a tree or even a blob.
work better, I wonder?
> +3. If the push destination is not a branch or tag:
Here, I do understand and support the colon, so I'd equally support
the first colon of the previous 2 sections for consistency.
> + * If the source is a tree or blob object, any updates will be rejected
OK, so this is the same rule as the refs/tags/ hierarchy.
> + * If the source is a tag or commit object, any fast-forward update
> + 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.
> +
> +You can override these rules by passing `--force` or by adding the
> +optional leading `+` to a refspec. The only exception to this is that no
> +amount of forcing will make a branch accept a non-commit object.
> +
> +Hooks and configuration can also override or amend these rules,
> +see e.g. `receive.denyNonFastForwards` and `receive.denyDeletes`
> +in linkgit:git-config[1] and `pre-receive` and `update` in
> +linkgit:githooks[5].
Very well written. refs/heads/ not taking any non-commit, and
receiver side hooks that may reject a push, are not something
"--force" can override. The mention to "the only exception" above
sounded as if it forgot to mention the latter.
Looking mostly good. Thanks.There was a problem hiding this comment.
On the Git mailing list, "Julia Evans" wrote (reply to this):
>> --force::
>> - Usually, the command refuses to update a remote ref that is
>> - not an ancestor of the local ref used to overwrite it.
>> - Also, when `--force-with-lease` option is used, the command refuses
>> - to update a remote ref whose current value does not match
>> - what is expected.
>> + Usually, `git push` will refuse to update a branch that is not an
>> + ancestor of the local branch or commit being pushed.
>
> I read this as "there are two conditions, and satisifying only one of
> them is sufficient for the push to be allowed. (1) the local branch
> is a decendant of the remote branch being updated, or (2) the commit
> we push to update the remote branch is a descendant of the remote
> branch being updated".
>
> But of course that is not what you wanted to say. (1) would mean
>
> $ git reset origin/foo && git push origin anything:foo
>
> would allow us to push literally anything to overwrite origin's foo
> branch.
>
> I think
>
> "... not an ancestor of the commit being pushed to update it."
>
> would be a way to avoid such confusion.
I like this, will use that phrasing.
>
>> +PUSH RULES
>> +----------
>> +
>> +As a safety feature, the `git push` command only allows certain kinds of
>> +updates to prevent you from accidentally losing data on the remote.
>> +
>> +Because branches and tags are intended to be used differently, the
>> +safety rules for pushing to a branch are different from the rules
>> +for pushing to a tag. In the following rules "update" means any
>> +modifications except deletes. Deletions are always allowed, except when
>> +forbidden by configuration or hooks.
>
> One important operation is omitted. "update" does not include
> "create" in the following, no? Obviously since refs/tags/ would
> never take any "update" (unless forced), if "create" were thrown
> into the same category as "update", you cannot push a new tag out.
>
> So, next to "Deletions are always allowed", shouldn't we describe
> what rules apply to creations? I presume that they are also always
> allowed?
> The first colon might be acceptable (I find it a bit odd, though).
> The second colon is very weird. ": only" -> ", only" & "allowed:
> the" -> "allowed. The", perhaps?
I agree the second colon is weird, will fix it. (probably ", which means the
destination must be an ancestor of the source commit")
>
>> +2. If the push destination is a **tag** (`refs/tags/*`): all updates will
>> + be rejected. The source can be any object
>> + (since commits, trees and blobs can be tagged).
>
> Again, I might prefer ":" -> ",". I cannot decide which I prefer
> between "all updates will be rejected" and "by default no updates
> are allowed". Either should be OK, so let's take what has already
> been written.
>
> The second sentence is not wrong per-se, and I can see that this was
> inherited from the original, but gives me a strange aftertaste. When
> you list object types in the context of "tag" and have only commit,
> tree, and blob, a little voice in the back of my head asks "oh, what
> happend to tags?". It is made a bit worse with the phrase "can be
> tagged", as it typically means either (1) to create an annotated or
> signed tag object, or (2) to create a ref in refs/tags/ hierarchy
> locally, but usually you do not think of pushing to refs/tags/
> hierarchy as "tagging that object remotely".
>
> I think the untold assumption here is that refs/tags/foo at the
> remote is being updated most of the time from refs/tags/foo we have
> locally, and "any kind of object can be tagged" is trying to say
> that refs/tags/foo we have locally can be an object of any type, as
> the act of creating a ref "refs/tags/foo" and pointing it directly
> at an object is "to create a light-weight tag" for the object.
> Since we can have not just tags but any kind of object locally
> (because any object "can be tagged"), a push can ask object of any
> kind to be pushed to refs/tags/* hierarchy. But it is an awkward
> concept to explain.
>
> Would side-stepping what exactly "tagging a thing" means, and
> phrasing it like this
>
> The source is not limited to an annotated or signed tag object,
> but can be a commit, a tree or even a blob.
>
> work better, I wonder?
I think it depends on whether the reader is more familiar with lightweight
tags or annotated tags. The only kind of tag I've ever used personally is a
lightweight tag, so the sentence
"The source is not limited to an annotated or signed tag object,"
feels confusing to me ("why would it be limited to an annotated tag?
I'm not even totally sure what an annotated tag is!")
But I can see that it would be different if the reader is more familiar
with annotated tags. Will think about this.
Maybe we can just say "the source can be any object" to just be clear that there
are no restrictions without trying to educate the reader about the nature of
Git tags.
>> +3. If the push destination is not a branch or tag:
>
> Here, I do understand and support the colon, so I'd equally support
> the first colon of the previous 2 sections for consistency.
>
>> + * If the source is a tree or blob object, any updates will be rejected
>
> OK, so this is the same rule as the refs/tags/ hierarchy.
>
>> + * If the source is a tag or commit object, any fast-forward update
>> + 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.
>> +
>> +You can override these rules by passing `--force` or by adding the
>> +optional leading `+` to a refspec. The only exception to this is that no
>> +amount of forcing will make a branch accept a non-commit object.
>> +
>> +Hooks and configuration can also override or amend these rules,
>> +see e.g. `receive.denyNonFastForwards` and `receive.denyDeletes`
>> +in linkgit:git-config[1] and `pre-receive` and `update` in
>> +linkgit:githooks[5].
>
> Very well written. refs/heads/ not taking any non-commit, and
> receiver side hooks that may reject a push, are not something
> "--force" can override. The mention to "the only exception" above
> sounded as if it forgot to mention the latter.
Will mention the receiver side hooks there, I didn't think of that.
> Looking mostly good. Thanks.
|
This patch series was integrated into seen via git@1ba0be5. |
|
This patch series was integrated into seen via git@1ca6afb. |
|
This branch is now known as |
|
This patch series was integrated into seen via git@501e1f7. |
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): On 2025-09-17 at 21:33:33, Julia Evans via GitGitGadget wrote:
> This is a continuation of the changes to git push, from
> https://lore.kernel.org/git/[email protected]/
> . These changes to the refspec section got kind of big so I'm moving them
> into a separate topic.
>
> Since the last review, the main change is to move the rules for pushing out
> of the section and into their own section ("PUSH RULES") so that it can be
> easily referenced from other places in the man page.
>
> I don't love the nested list in PUSH RULES but the sentence starting with
> "If the source is a tag or commit object..." is really a tough one to read,
> it's not going to be relevant to the vast majority of people, and I think
> keeping it contained inside a bullet point will make it much easier to skip
> over to get to later information which is more likely to be relevant to
> folks.
>
> Other changes:
>
> * removed "+:<dst> is optional.", from Junio's review
> * kept "+ is optional and does the same thing as --force", since now the
> push rules are in their own section.
> * fixed the fully expanded refspec form (main:refs/heads/main =>
> refs/heads/main:refs/heads/main)
> * switched from a numbered list to an unordered list, from Junio's review.
> I think the numbered list looks a lot nicer in the terminal output, but
> it's true that there isn't any order. I briefly attempted to understand
> how AsciiDoc's nroff (?) generation works to see if it's possible to make
> unordered lists indent with fewer spaces (2 instead of 4) but I was left
> feeling that nroff/troff/etc are not for mere mortals like me to
> understand.
I have used groff for many years to write letters and address envelopes
(as well as write a few manual pages and design some awards) and I still
don't understand much of it, so I understand how you feel.
In this case we have two possible implementations, AsciiDoc and
Asciidoctor, but both use the man macros. I think if there were some
way to control the value generated as the argument to the `.RS` macro,
then that would be what you want, but I have no idea how to adjust that
in either one. Personally, I would just leave it as it is, since I
believe 4 is the traditional value used with the man macros, even if
typographically or aesthetically it might be suboptimal.
Having said all that, I think this documentation is much improved and
easier to understand, so thank you for that. I don't have specific
comments on the patches (although I see that Junio has suggested some
clarifying comments), but I do deeply appreciate your efforts to improve
the documentation (and, outside of the list, to clarify difficult
technical topics in general).
--
brian m. carlson (they/them)
Toronto, Ontario, CA |
|
User |
|
On the Git mailing list, Jeff King wrote (reply to this): On Fri, Sep 19, 2025 at 12:39:12AM +0000, brian m. carlson wrote:
> In this case we have two possible implementations, AsciiDoc and
> Asciidoctor, but both use the man macros. I think if there were some
> way to control the value generated as the argument to the `.RS` macro,
> then that would be what you want, but I have no idea how to adjust that
> in either one. Personally, I would just leave it as it is, since I
> believe 4 is the traditional value used with the man macros, even if
> typographically or aesthetically it might be suboptimal.
I think both implementations will just generate XML via our Makefile,
and ultimately it is DocBook which will convert the <orderedlist> into
actual roff. So something like:
diff --git a/Documentation/manpage-normal.xsl b/Documentation/manpage-normal.xsl
index beb5ff8ec2..b494fbb5df 100644
--- a/Documentation/manpage-normal.xsl
+++ b/Documentation/manpage-normal.xsl
@@ -11,4 +11,6 @@
<!-- unset maximum length of title -->
<xsl:param name="man.th.title.max.length"/>
+<xsl:param name="man.indent.width" select="2"/>
+
</xsl:stylesheet>
would affect that process. And I think there might even be a specific
list-indent variable, but I didn't dig very far. I agree it's probably
not worth going too far into the rabbit hole of manpage styling. The
parameter docs are here:
https://docbook.sourceforge.net/release/xsl/1.78.1/doc/manpages/indent.html
I believe Asciidocttor _can_ generate roff directly, but we don't use it
that way. I don't think it would make sense to do so unless we are ready
to drop AsciiDoc support entirely (since keeping them as close together
as possible reduces the maintenance burden).
-Peff |
|
User |
|
This patch series was integrated into seen via git@b336ea6. |
|
There was a status update in the "Cooking" section about the branch Doc updates. source: <[email protected]> |
|
This patch series was integrated into seen via git@e125bf0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a continuation of the changes to
git push, from https://lore.kernel.org/git/[email protected]/ . These changes to the refspec section got kind of big so I'm moving them into a separate topic.Since the last review, the main change is to move the rules for pushing out of the section and into their own section ("PUSH RULES") so that it can be easily referenced from other places in the man page.
I don't love the nested list in PUSH RULES but the sentence starting with "If the source is a tag or commit object..." is really a tough one to read, it's not going to be relevant to the vast majority of people, and I think keeping it contained inside a bullet point will make it much easier to skip over to get to later information which is more likely to be relevant to folks.
Other changes:
:<dst>is optional.", from Junio's review+is optional and does the same thing as--force", since now the push rules are in their own section.main:refs/heads/main=>refs/heads/main:refs/heads/main)cc: "brian m. carlson" [email protected]
cc: Jeff King [email protected]