git push tags

git push tags

[Angelo Borsotti]

Hello,

git push tag updates silently the specified tag. E.g.

git init --bare release.git
git clone release.git integrator
cd integrator
git branch -avv
touch f1; git add f1; git commit -m A
git tag v1
git push origin tag v1
touch f2; git add f2; git commit -m B
git tag -f v1
git push origin tag v1

the second git push updates the tag in the remote repository. This is
somehow counterintuitive because tags normally do not move (unless
forced to that), and is not documented.
This is also harmful because it allows to change silently something
(tags) that normally must not change.

-Angelo Borsotti

Re: git push tags

[Drew Northup]

On Thu, Oct 25, 2012 at 2:58 AM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
> Hello,
>
> git push tag updates silently the specified tag. E.g.
>
> git init --bare release.git
> git clone release.git integrator
> cd integrator
> git branch -avv
> touch f1; git add f1; git commit -m A
> git tag v1
> git push origin tag v1
> touch f2; git add f2; git commit -m B
> git tag -f v1
> git push origin tag v1
>
> the second git push updates the tag in the remote repository. This is
> somehow counterintuitive because tags normally do not move (unless
> forced to that), and is not documented.

You specified "-f" (force) and it did exactly what you asked. That is
fully documented (git help tag).

> This is also harmful because it allows to change silently something
> (tags) that normally must not change.

Tags have many uses. Some of those uses are harmed when tags change
and some aren't. That's a philosophical argument and Git is a computer
program, not a philosopher. It is not the job of the machine to
prevent the user from shooting himself in the foot when he clearly
expressed an interest in doing so.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Angelo Borsotti]

Hi Drew,

>
> You specified "-f" (force) and it did exactly what you asked. That is
> fully documented (git help tag).
>

Yes, it is, and I used it to show that there is a need to specify
explicitly the intent to change a tag, that without such an indication
would not be changed.

>Tags have many uses. Some of those uses are harmed when tags change
and some aren't. That's a philosophical argument

I agree, but in this case the computer does not provide any means to
implement the same strategy on tags as it does instead on local
repositories. Why I must force a change on a tag in the local
repository and instead I can change it without any forcing in a remote
one? Are remote repositories less protected than the local ones? I
think that to be consistent, the same strategy should be used on all
repositories, i.e. rejecting changes on tags by default, unless they
are forced.

-Angelo


> --
> -Drew Northup
> --------------------------------------------------------------
> "As opposed to vegetable or mineral error?"
> -John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Drew Northup]

On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
---At 13:19 on Oct 25, 2012, Drew Northup wrote: [added for clarity]
>>Tags have many uses. Some of those uses are harmed when tags change
> and some aren't. That's a philosophical argument
>
> I agree, but in this case the computer does not provide any means to
> implement the same strategy on tags as it does instead on local
> repositories. Why I must force a change on a tag in the local
> repository and instead I can change it without any forcing in a remote
> one?

Changing the tag in the local repository is a tag modification
operation. Pushing that change to a remote repository DOES NOT execute
"git tag...." in the remote. Plain and simple the two are different
operations.

> Are remote repositories less protected than the local ones? I
> think that to be consistent, the same strategy should be used on all
> repositories, i.e. rejecting changes on tags by default, unless they
> are forced.

So here we come to the core argument. Is sounds to me like you want
changes to remote tags to work differently from push updates to ALL
other references. The required change, if I'm not mistaken, would be
for tags to not permit fast-forward updates while all other references
would be pushed normally. From my brief and un-enlightened look at the
push code I can't see that being as easy as it sounds.

In any case, I think your complaint stems from thinking that "git tag"
is the operation being performed on the remote when in fact it is not.
Given the mayhem that changing this may involve I'm not going to claim
it to be a good idea.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Angelo Borsotti]

Hi Drew,

>
> Changing the tag in the local repository is a tag modification
> operation. Pushing that change to a remote repository DOES NOT execute
> "git tag...." in the remote. Plain and simple the two are different
> operations.
>

They are different for what concerns the implementation. They are not
necessarily so for what concerns their semantics, and the most
straightforward is to apply to the remote repository the changes done
on the local one -- the changes that can legally done on it -- and
changing a tag is not one allowed (unless forced).
Obviously, the semantics of git-push is different, and then needs to
be described clearly.
Note that some (probably most) of the operations that are disallowed
on the local repo are also disallowed by git-push, like, e.g. deleting
the current branch. But the user cannot tell what is disallowed and
what not if the man page does not state it.

> So here we come to the core argument. Is sounds to me like you want
> changes to remote tags to work differently from push updates to ALL
> other references. The required change, if I'm not mistaken, would be
> for tags to not permit fast-forward updates while all other references
> would be pushed normally. From my brief and un-enlightened look at the
> push code I can't see that being as easy as it sounds.
>

No, I was hoping that git-push refused to change tags at all, unless
forced (e.g. prefixing them with +), as it is on a local repository.

-Angelo

Re: git push tags

[Drew Northup]

On Fri, Oct 26, 2012 at 2:42 AM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
> Hi Drew,
>
------Adding for clarity: On Thurs, Oct 25, 2012 at 17:16 EDT, Drew
Northup wrote:
>>
>> Changing the tag in the local repository is a tag modification
>> operation. Pushing that change to a remote repository DOES NOT execute
>> "git tag...." in the remote. Plain and simple the two are different
>> operations.
>>
>
> They are different for what concerns the implementation. They are not
> necessarily so for what concerns their semantics, and the most
> straightforward is to apply to the remote repository the changes done
> on the local one -- the changes that can legally done on it -- and
> changing a tag is not one allowed (unless forced).

So you want the rules of "git tag" to be forced upon "git push." I
heard (read) that already. By what means do you intend to enforce
that? Please go look at the code of "git push" (builtin/push.c) before
coming back with an answer.

> Obviously, the semantics of git-push is different, and then needs to
> be described clearly.
> Note that some (probably most) of the operations that are disallowed
> on the local repo are also disallowed by git-push, like, e.g. deleting
> the current branch. But the user cannot tell what is disallowed and
> what not if the man page does not state it.

If you think something is inadequately documented then do please
submit a patch proposing the changes you would like to see. Even if it
isn't quite right the first time you can get help from interested
people on the list getting it accepted. If nothing else you'll get
some constructive discussion going.

(As for deleting the current branch, you can't really do that on a
proper bare remote anyway as there is no such thing as a "current
branch" in that context.)

>> So here we come to the core argument. Is sounds to me like you want
>> changes to remote tags to work differently from push updates to ALL
>> other references. The required change, if I'm not mistaken, would be
>> for tags to not permit fast-forward updates while all other references
>> would be pushed normally. From my brief and un-enlightened look at the
>> push code I can't see that being as easy as it sounds.
>>
>
> No, I was hoping that git-push refused to change tags at all, unless
> forced (e.g. prefixing them with +), as it is on a local repository.

I'm not sure how you get to negate my statement and then restate it
and think that you are saying something different. Please go look at
the the source code. Tags are just a class of refs that we set aside
for human consumption while also providing them with meaning that the
computer can understand.

What are refs then? Refs are REFERENCES to specific objects (at this
point I know only of commits being _referenced_ that way) by their sha
hashes (literally, that's what the file contains) in the repository.
That's it. Git push/pull doesn't handle them any differently may they
be heads or tags. (Remote heads, aka tracking branches, are references
to objects in the repository that do not exist in a local branch. It
doesn't look like we push those, but I could be wrong.) What Git does
do is allow you the flexibility of pushing refs selectively (with some
caveats). Once tags are loaded into the queue of references to be
pushed they are handled like any other kind of reference.

If you think there would be a benefit to making the reference pushing
code "type aware" then do please look into the code and submit a
patch. I am not going to waste any more time arguing about how it
"should be" if you cannot go to the trouble of reading the code and
coming up with a solution that implements what you would like to see
happen. Frankly, if all you can come up with is pseudocode you still
will at least have a chance of working with the people here on this
list to to write up something that implements the behavior you would
like to see. The first step to that is understanding how it actually
works now.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Chris Rorvick]

On Fri, Oct 26, 2012 at 8:37 AM, Drew Northup <n1xim.email@gmail.com> wrote:
> (As for deleting the current branch, you can't really do that on a
> proper bare remote anyway as there is no such thing as a "current
> branch" in that context.)

Really?  When I clone a bare repository I see a HEAD, and Git doesn't
want me to delete it with a push from a remote repo.  So is this not a
"proper" bare repository?

Re: git push tags

[Drew Northup]

On Fri, Oct 26, 2012 at 9:59 AM, Chris Rorvick <chris@rorvick.com> wrote:
> On Fri, Oct 26, 2012 at 8:37 AM, Drew Northup <n1xim.email@gmail.com> wrote:
>> (As for deleting the current branch, you can't really do that on a
>> proper bare remote anyway as there is no such thing as a "current
>> branch" in that context.)
>
> Really?  When I clone a bare repository I see a HEAD, and Git doesn't
> want me to delete it with a push from a remote repo.  So is this not a
> "proper" bare repository?

Chris,
If there's no working directory in the remote repository is there a
"current branch" in the _remote_ repository? (I am not talking about
the tracking branch here. I also presume that attempting to delete the
one and only branch is somewhat nonsensical.)

In any case, this is a different mental model than the one Angelo
seems (to me) to be arguing.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Chris Rorvick]

On Fri, Oct 26, 2012 at 9:13 AM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Fri, Oct 26, 2012 at 9:59 AM, Chris Rorvick <chris@rorvick.com> wrote:
>> On Fri, Oct 26, 2012 at 8:37 AM, Drew Northup <n1xim.email@gmail.com> wrote:
>>> (As for deleting the current branch, you can't really do that on a
>>> proper bare remote anyway as there is no such thing as a "current
>>> branch" in that context.)
>>
>> Really?  When I clone a bare repository I see a HEAD, and Git doesn't
>> want me to delete it with a push from a remote repo.  So is this not a
>> "proper" bare repository?
>
> Chris,
> If there's no working directory in the remote repository is there a
> "current branch" in the _remote_ repository?

Yes, I believe it is the default branch when the repository is cloned.

Re: git push tags

[Angelo Borsotti]

Hi Drew,

git is an open source, community project, which means that it benefits
from all the contributions of many people, and they are not restricted
to patches.
If the only one suggestions that were taken into account were patches
sent by people that had the time to study the sources and propose
changes, then we would miss the opportunity of taking many good things
that the community generates, like new ideas for example.

By the way, I already browsed the code, but have seen that there is
not only push.c to understand, but a dozen or more of other sources.
This is why I am not yet able to propose patches.
But I am using git all day, and often come to unexpected or
undocumented behaviors, and want to share my findings in case they
could serve to improve git. This means to me to spend time in
formulating them the best I can to make others understand what I have
found, and that is a contribution too.

Here is a proposed change to the git-push manpage:

- section: "DESCRIPTION", first paragraph ("Updates remote .."), add at the end:

   "Remote references (branches and tags) that do not exist are
created. The ones that exist are updated except when + is not
specified and they are not fast-forward updates."

-Angelo

Re: git push tags

[Kacper Kornet]

On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
> <angelo.borsotti@gmail.com> wrote:
> > Are remote repositories less protected than the local ones? I
> > think that to be consistent, the same strategy should be used on all
> > repositories, i.e. rejecting changes on tags by default, unless they
> > are forced.

> So here we come to the core argument. Is sounds to me like you want
> changes to remote tags to work differently from push updates to ALL
> other references. The required change, if I'm not mistaken, would be
> for tags to not permit fast-forward updates while all other references
> would be pushed normally. From my brief and un-enlightened look at the
> push code I can't see that being as easy as it sounds.

I think the patch below obtains the requested behaviour:

diff --git a/remote.c b/remote.c
index 04fd9ea..7fcb51e 100644
--- a/remote.c
+++ b/remote.c
@@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			!ref->deletion &&
 			!is_null_sha1(ref->old_sha1) &&
 			(!has_sha1_file(ref->old_sha1)
-			  || !ref_newer(ref->new_sha1, ref->old_sha1));
+			  || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
 
 		if (ref->nonfastforward && !ref->force && !force_update) {
 			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;

-- 
  Kacper Kornet

Re: git push tags

[Drew Northup]

On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
>> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
>> <angelo.borsotti@gmail.com> wrote:
>> > Are remote repositories less protected than the local ones? I
>> > think that to be consistent, the same strategy should be used on all
>> > repositories, i.e. rejecting changes on tags by default, unless they
>> > are forced.
>
>> So here we come to the core argument. Is sounds to me like you want
>> changes to remote tags to work differently from push updates to ALL
>> other references. The required change, if I'm not mistaken, would be
>> for tags to not permit fast-forward updates while all other references
>> would be pushed normally. From my brief and un-enlightened look at the
>> push code I can't see that being as easy as it sounds.
>
> I think the patch below obtains the requested behaviour:
>
> diff --git a/remote.c b/remote.c
> index 04fd9ea..7fcb51e 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>                         !ref->deletion &&
>                         !is_null_sha1(ref->old_sha1) &&
>                         (!has_sha1_file(ref->old_sha1)
> -                         || !ref_newer(ref->new_sha1, ref->old_sha1));
> +                         || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
>
>                 if (ref->nonfastforward && !ref->force && !force_update) {
>                         ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
>
> --
>   Kacper Kornet

Kacper,
I obviously didn't dig deep enough. In any case, I presume that's what
he's asking for. I can't remember if git is still forcing tags to
always be located in "refs/tags" however. I didn't think it was.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Kacper Kornet]

On Fri, Oct 26, 2012 at 02:07:09PM -0400, Drew Northup wrote:
> On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> > On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
> >> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
> >> <angelo.borsotti@gmail.com> wrote:
> >> > Are remote repositories less protected than the local ones? I
> >> > think that to be consistent, the same strategy should be used on all
> >> > repositories, i.e. rejecting changes on tags by default, unless they
> >> > are forced.

> >> So here we come to the core argument. Is sounds to me like you want
> >> changes to remote tags to work differently from push updates to ALL
> >> other references. The required change, if I'm not mistaken, would be
> >> for tags to not permit fast-forward updates while all other references
> >> would be pushed normally. From my brief and un-enlightened look at the
> >> push code I can't see that being as easy as it sounds.

> > I think the patch below obtains the requested behaviour:

> > diff --git a/remote.c b/remote.c
> > index 04fd9ea..7fcb51e 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
> >                         !ref->deletion &&
> >                         !is_null_sha1(ref->old_sha1) &&
> >                         (!has_sha1_file(ref->old_sha1)
> > -                         || !ref_newer(ref->new_sha1, ref->old_sha1));
> > +                         || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));

> >                 if (ref->nonfastforward && !ref->force && !force_update) {
> >                         ref->status = REF_STATUS_REJECT_NONFASTFORWARD;

> > --
> >   Kacper Kornet

> Kacper,
> I obviously didn't dig deep enough. In any case, I presume that's what
> he's asking for. I can't remember if git is still forcing tags to
> always be located in "refs/tags" however. I didn't think it was.

I have based my assumption about location of tags on gitglossary:

tag
 A ref under refs/tags/ namespace that points to an object of an
 arbitrary type (typically a tag points to either a tag or a commit
 object).

-- 
  Kacper

Re: git push tags

[Angelo Borsotti]

Hello

Drew,

I made some further tests on git-push to see if it handled branches
and tags in the same way, and have discovered the following
differences:

    - git push origin --delete master
      remote: error: By default, deleting the current branch is denied

    - git push origin --delete vx            (where vx is a tag)
      ... accepted

This is consistent with what is done on the local repo: deleting the
current branch is disallowed, but deleting a tag is allowed (even when
HEAD points to it).
That means that git-push does not handle branches and tags exactly the same.

Kacper

thank you for the patch. To keep downward compatibility, the denial to
update tags should perhaps be enabled with some option.

-Angelo

Re: git push tags

[Kacper Kornet]

On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
> Hello

> Drew,

> I made some further tests on git-push to see if it handled branches
> and tags in the same way, and have discovered the following
> differences:

>     - git push origin --delete master
>       remote: error: By default, deleting the current branch is denied

>     - git push origin --delete vx            (where vx is a tag)
>       ... accepted

> This is consistent with what is done on the local repo: deleting the
> current branch is disallowed, but deleting a tag is allowed (even when
> HEAD points to it).
> That means that git-push does not handle branches and tags exactly the same.

> Kacper

> thank you for the patch. To keep downward compatibility, the denial to
> update tags should perhaps be enabled with some option.

You are probably right. The proper submission should also contain a
test. I have sent a crude patch to show that the behaviour asked by you
is possible to obtain.

I will try to prepare a formal submission patch, but I can't say how
long it will take me. So if you want to do it by yourself feel free.

-- 
  Kacper Kornet

Re: git push tags

[Drew Northup]

On Fri, Oct 26, 2012 at 3:00 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
>> Hello
>
>> Drew,
....
>> Kacper
>
>> thank you for the patch. To keep downward compatibility, the denial to
>> update tags should perhaps be enabled with some option.
>
> You are probably right. The proper submission should also contain a
> test. I have sent a crude patch to show that the behaviour asked by you
> is possible to obtain.
>
> I will try to prepare a formal submission patch, but I can't say how
> long it will take me. So if you want to do it by yourself feel free.

Kacper,
I have been rebuilding my local so that generating a proper patch for
the manpage change part of the set won't make a horrible mess over
here.

It looks to me like the changeset requires the following:
Kacper's patch, some manpage changes, a test.

Angelo, may we use your original mail as the source for the cover page?

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Johannes Sixt]

Am 25.10.2012 08:58, schrieb Angelo Borsotti:
> Hello,
> 
> git push tag updates silently the specified tag. E.g.
> 
> git init --bare release.git
> git clone release.git integrator
> cd integrator
> git branch -avv
> touch f1; git add f1; git commit -m A
> git tag v1
> git push origin tag v1
> touch f2; git add f2; git commit -m B
> git tag -f v1
> git push origin tag v1
> 
> the second git push updates the tag in the remote repository. This is
> somehow counterintuitive because tags normally do not move (unless
> forced to that), and is not documented.

Tags are refs, just like branches. "Tags don't move" is just a
convention, and git doesn't even respect it (except possibly in one
place[1]). You can't reseat tags unless you use -f, which is exactly the
same with branches, which you can't reseat unless you use -f.

[1] By default, git fetch does not fetch tags that it already has.

> This is also harmful because it allows to change silently something
> (tags) that normally must not change.

You asked git push to push a tag, and the tag was pushed. What's wrong?

-- Hannes

Re: git push tags

[Chris Rorvick]

On Sun, Oct 28, 2012 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Tags are refs, just like branches. "Tags don't move" is just a
> convention, and git doesn't even respect it (except possibly in one
> place[1]). You can't reseat tags unless you use -f, which is exactly the
> same with branches, which you can't reseat unless you use -f.
>
> [1] By default, git fetch does not fetch tags that it already has.

Also, git checkout <tag> puts you on a detached HEAD.  This seems
pretty significant with regard to Git respecting a "tags don't move"
convention.

Chris Rorvick

Re: git push tags

[Philip Oakley]

From: "Chris Rorvick" <chris@rorvick.com> Sent: Sunday, October 28, 2012 
7:59 PM
> On Sun, Oct 28, 2012 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Tags are refs, just like branches. "Tags don't move" is just a
>> convention, and git doesn't even respect it (except possibly in one
>> place[1]). You can't reseat tags unless you use -f, which is exactly 
>> the
>> same with branches, which you can't reseat unless you use -f.
>>
>> [1] By default, git fetch does not fetch tags that it already has.
>
> Also, git checkout <tag> puts you on a detached HEAD.  This seems
> pretty significant with regard to Git respecting a "tags don't move"
> convention.

Surely the convention is the other way around. That is, it is branches 
that are _expected_ to move, hence unless you are checkout a branch 
(movable) you will be on a detached head at a fixed place/sha1 [aka not 
on a branch].

The checking out of a tag action doesn't make it more or less 
significant.

I think Angelo's original post should be reviewed to see if the issue 
can now be restated in a manner that shows up the implied conflict.

If I read it right it was where two users can tag two different commits 
with the same tag name [e.g. 'Release_V3.3'] and the last person to push 
wins, so anyone in the team can change what is to be the released 
version!

Philip

>

Re: git push tags

[Drew Northup]

On Sun, Oct 28, 2012 at 5:49 PM, Philip Oakley <philipoakley@iee.org> wrote:

> If I read it right it was where two users can tag two different commits with
> the same tag name [e.g. 'Release_V3.3'] and the last person to push wins, so
> anyone in the team can change what is to be the released version!

Philip,
Please look at Kacper's patch and Angelo's response to it. He seems to
be asking that tags not be permitted to be pushed as if doing so were
a "fast-forward" update.

This weekend I was, in part, trying to figure out what the correct CC
list for that patch would be, what the documentation change would be,
what changes would need to be made to the advice, what test would need
to be included, and so on to build a proper patch bundle. All of that
while tring to keep the house from "falling in" (I've been doing some
cleaning) and prepare for the Northeastern USA Coast to be doused and
blasted by Sandy in about a day and a half. If we decide to continue
in the path that Kacper and I have stumbled upon (with Angelo's
prodding) I'd appreciate a little help putting all of this together to
mesh with the aforementioned patch. (Heck, if there's somebody better
than me to take this over I'd be game for that too...)

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Chris Rorvick]

On Sun, Oct 28, 2012 at 4:49 PM, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Chris Rorvick" <chris@rorvick.com> Sent: Sunday, October 28, 2012
> 7:59 PM
>
>> On Sun, Oct 28, 2012 at 1:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>>
>>> Tags are refs, just like branches. "Tags don't move" is just a
>>> convention, and git doesn't even respect it (except possibly in one
>>> place[1]). You can't reseat tags unless you use -f, which is exactly the
>>> same with branches, which you can't reseat unless you use -f.
>>>
>>> [1] By default, git fetch does not fetch tags that it already has.
>>
>>
>> Also, git checkout <tag> puts you on a detached HEAD.  This seems
>> pretty significant with regard to Git respecting a "tags don't move"
>> convention.
>
>
> Surely the convention is the other way around. That is, it is branches that
> are _expected_ to move, hence unless you are checkout a branch (movable) you
> will be on a detached head at a fixed place/sha1 [aka not on a branch].
>
> The checking out of a tag action doesn't make it more or less significant.

Ah, that makes sense.  Thanks.  So it sounds like the solution should
be something more general than checking if the prefix is "refs/tags"
if I understand your point correctly.

Also, setting the status to REF_STATUS_REJECT_NONFASTFORWARD even when
it would be a fast-forward (does that terminology apply to non-branch
refs?) is confusing.  It's already confusing that a failure to push a
tag tells me I need to fetch and merge in the changes.

Chris

Re: git push tags

[Angelo Borsotti]

Hi,

Pro Git, By Scott Chacon says:

2.6

   "Like most VCSs, Git has the ability to tag specific points in
history as being important.
Generally, people use this functionality to mark release points (v1.0,
and so on)."

2.6.2:

   "A [lightweight] tag is very much like a branch that does not change ..."

Clearly, tags are not the same as branches. They are there for a
different purpose. If they were exactly the same as branches, there
would be no need for them.
Of course, they share some behaviors with branches, and there are
several commands that handle them in the same way, but the key point
is that they do not change, or at least they do not do that by
default. The ability to force changes of tags is there only to correct
tags that might have set tags to the wrong points by mistake.
So, what I am telling is that this property must be preserved
consistently across all commands, including git-push.

-Angelo

Re: git push tags

[Angelo Borsotti]

Hi,

to let the owner of a remote repository (one on which git-push
deposits objects) disallow
others to change tags, a key on its config file could be used.
An option on git-push, or environment variable, or key in config file
of the repo from which git-push takes objects do not help in enforcing
the policy not to update tags in the remote repo.

-Angelo

Re: git push tags

[Michael Haggerty]

I agree with you that it is too easy to create chaos by changing tags in
a published repository and that git should do more to prevent this from
happening without explicit user forcing.  The fact that git internally
handles tags similarly to other references is IMO an excuse for the
current behavior, but not a justification.

On 10/29/2012 09:12 AM, Angelo Borsotti wrote:
> to let the owner of a remote repository (one on which git-push
> deposits objects) disallow
> others to change tags, a key on its config file could be used.
> An option on git-push, or environment variable, or key in config file
> of the repo from which git-push takes objects do not help in enforcing
> the policy not to update tags in the remote repo.

If your remote repository is managed using gitolite, you can institute
restrictions on changing tags via the gitolite config.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: git push tags

[Kacper Kornet]

On Mon, Oct 29, 2012 at 09:12:52AM +0100, Angelo Borsotti wrote:
> Hi,

> to let the owner of a remote repository (one on which git-push
> deposits objects) disallow
> others to change tags, a key on its config file could be used.
> An option on git-push, or environment variable, or key in config file
> of the repo from which git-push takes objects do not help in enforcing
> the policy not to update tags in the remote repo.

It think these are two separate issues:

1. Enforcing policy that the server should not accept changes in
existing tags. This can be easily done with hooks.

2. Perform operation: push my tag if and only if it doesn't overwrite
tag which already exists on the server. I think currently it is not possible
with git.

-- 
  Kacper

Re: git push tags

[Jeff King]

On Mon, Oct 29, 2012 at 10:58:07AM +0100, Michael Haggerty wrote:

> I agree with you that it is too easy to create chaos by changing tags in
> a published repository and that git should do more to prevent this from
> happening without explicit user forcing.  The fact that git internally
> handles tags similarly to other references is IMO an excuse for the
> current behavior, but not a justification.

I would have expected git to at least complain about updating an
annotated tag with another annotated tag. But it actually uses the same
fast-forward rule, just on the pointed-to commits. So a fast-forward
annotated re-tag will throw away the old tag object completely. Which
seems a bit crazy to me.

It seems like a no-brainer to me that annotated tags should not replace
each other without a force, no matter where in the refs hierarchy they
go.

For lightweight tags, I think it's more gray. They are just pointers
into history. Some projects may use them to tag immutable official
versions, but I also see them used as shared bookmarks. Requiring "-f"
may make the latter use more annoying. On the other hand, bookmark tags
tend not to be pushed, or if they are, it is part of a mirror-like
backup which should be forcing all updates anyway.

-Peff

Re: git push tags

[Drew Northup]

On Mon, Oct 29, 2012 at 6:38 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 10:58:07AM +0100, Michael Haggerty wrote:
>
>> I agree with you that it is too easy to create chaos by changing tags in
>> a published repository and that git should do more to prevent this from
>> happening without explicit user forcing.  The fact that git internally
>> handles tags similarly to other references is IMO an excuse for the
>> current behavior, but not a justification.
>
> I would have expected git to at least complain about updating an
> annotated tag with another annotated tag. But it actually uses the same
> fast-forward rule, just on the pointed-to commits. So a fast-forward
> annotated re-tag will throw away the old tag object completely. Which
> seems a bit crazy to me.
>
> It seems like a no-brainer to me that annotated tags should not replace
> each other without a force, no matter where in the refs hierarchy they
> go.
>
> For lightweight tags, I think it's more gray. They are just pointers
> into history. Some projects may use them to tag immutable official
> versions, but I also see them used as shared bookmarks. Requiring "-f"
> may make the latter use more annoying. On the other hand, bookmark tags
> tend not to be pushed, or if they are, it is part of a mirror-like
> backup which should be forcing all updates anyway.

Would that be an endorsement of continuing to build a patch set
including the snippet that Kacper posted earlier (1) in response to my
comment about not being sure how complicated all of this would be or
not?

[1] http://article.gmane.org/gmane.comp.version-control.git/208473

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Angelo Borsotti]

Hi Drew,

sure. That is a good starting point. I would suggest to block tag
updates of existing tags if a dedicated option is present in the
config of the remote repo, like, e.g. pushOverwriteTags.

-Angelo

Re: git push tags

[Jeff King]

On Mon, Oct 29, 2012 at 07:21:52AM -0400, Drew Northup wrote:

> > I would have expected git to at least complain about updating an
> > annotated tag with another annotated tag. But it actually uses the same
> > fast-forward rule, just on the pointed-to commits. So a fast-forward
> > annotated re-tag will throw away the old tag object completely. Which
> > seems a bit crazy to me.
> >
> > It seems like a no-brainer to me that annotated tags should not replace
> > each other without a force, no matter where in the refs hierarchy they
> > go.
> >
> > For lightweight tags, I think it's more gray. They are just pointers
> > into history. Some projects may use them to tag immutable official
> > versions, but I also see them used as shared bookmarks. Requiring "-f"
> > may make the latter use more annoying. On the other hand, bookmark tags
> > tend not to be pushed, or if they are, it is part of a mirror-like
> > backup which should be forcing all updates anyway.
> 
> Would that be an endorsement of continuing to build a patch set
> including the snippet that Kacper posted earlier (1) in response to my
> comment about not being sure how complicated all of this would be or
> not?

That patch just blocks non-forced updates to refs/tags/. I think a saner
start would be to disallow updating non-commit objects without a force.
We already do so for blobs and trees because they are not (and cannot
be) fast forwards. The fact that annotated tags are checked for
fast-forward seems to me to be a case of "it happens to work that way"
and not anything planned. Since such a push drops the reference to the
old version of the tag, it should probably require a force.

Then on top of that we can talk about what lightweight tags should do.
I'm not sure. Following the regular fast-forward rules makes some sense
to me, because you are never losing objects. But there may be
complications with updating tags in general because of fetch's rules,
and we would be better off preventing people from accidentally doing so.
I think a careful review of fetch's tag rules would be in order before
making any decision there.

-Peff

Re: git push tags

[Drew Northup]

On Mon, Oct 29, 2012 at 7:35 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 07:21:52AM -0400, Drew Northup wrote:
>> > I would have expected git to at least complain about updating an
>> > annotated tag with another annotated tag. But it actually uses the same
>> > fast-forward rule, just on the pointed-to commits. So a fast-forward
>> > annotated re-tag will throw away the old tag object completely. Which
>> > seems a bit crazy to me.
>> >
>> > It seems like a no-brainer to me that annotated tags should not replace
>> > each other without a force, no matter where in the refs hierarchy they
>> > go.
>> >
>> > For lightweight tags, I think it's more gray. They are just pointers
>> > into history. Some projects may use them to tag immutable official
>> > versions, but I also see them used as shared bookmarks. Requiring "-f"
>> > may make the latter use more annoying. On the other hand, bookmark tags
>> > tend not to be pushed, or if they are, it is part of a mirror-like
>> > backup which should be forcing all updates anyway.
>>
>> Would that be an endorsement of continuing to build a patch set
>> including the snippet that Kacper posted earlier (1) in response to my
>> comment about not being sure how complicated all of this would be or
>> not?
>
> That patch just blocks non-forced updates to refs/tags/. I think a saner
> start would be to disallow updating non-commit objects without a force.
> We already do so for blobs and trees because they are not (and cannot
> be) fast forwards. The fact that annotated tags are checked for
> fast-forward seems to me to be a case of "it happens to work that way"
> and not anything planned. Since such a push drops the reference to the
> old version of the tag, it should probably require a force.
>
> Then on top of that we can talk about what lightweight tags should do.
> I'm not sure. Following the regular fast-forward rules makes some sense
> to me, because you are never losing objects. But there may be
> complications with updating tags in general because of fetch's rules,
> and we would be better off preventing people from accidentally doing so.
> I think a careful review of fetch's tag rules would be in order before
> making any decision there.
>
> -Peff

Thanks, I had the sinking suspicion that this was going to be more complicated.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: git push tags

[Angelo Borsotti]

Jeff,

> Then on top of that we can talk about what lightweight tags should do.
> I'm not sure.

If tags (even the lightweight ones) do not behave differently from
branches, then they are of no use, and the main difference is that
they do not move. So, I would suggest not to move them either.

-Angelo

Re: git push tags

[Kacper Kornet]

On Mon, Oct 29, 2012 at 07:35:00AM -0400, Jeff King wrote:
> On Mon, Oct 29, 2012 at 07:21:52AM -0400, Drew Northup wrote:

> > > I would have expected git to at least complain about updating an
> > > annotated tag with another annotated tag. But it actually uses the same
> > > fast-forward rule, just on the pointed-to commits. So a fast-forward
> > > annotated re-tag will throw away the old tag object completely. Which
> > > seems a bit crazy to me.

> > > It seems like a no-brainer to me that annotated tags should not replace
> > > each other without a force, no matter where in the refs hierarchy they
> > > go.

> > > For lightweight tags, I think it's more gray. They are just pointers
> > > into history. Some projects may use them to tag immutable official
> > > versions, but I also see them used as shared bookmarks. Requiring "-f"
> > > may make the latter use more annoying. On the other hand, bookmark tags
> > > tend not to be pushed, or if they are, it is part of a mirror-like
> > > backup which should be forcing all updates anyway.

> > Would that be an endorsement of continuing to build a patch set
> > including the snippet that Kacper posted earlier (1) in response to my
> > comment about not being sure how complicated all of this would be or
> > not?

> That patch just blocks non-forced updates to refs/tags/. I think a saner
> start would be to disallow updating non-commit objects without a force.
> We already do so for blobs and trees because they are not (and cannot
> be) fast forwards. The fact that annotated tags are checked for
> fast-forward seems to me to be a case of "it happens to work that way"
> and not anything planned. Since such a push drops the reference to the
> old version of the tag, it should probably require a force.

I'm not sure. Looking at 37fde87 ("Fix send-pack for non-commitish
tags.") I have an impression that Junio allowed for fast-forward pushes
of annotated tags on purpose. 

> Then on top of that we can talk about what lightweight tags should do.
> I'm not sure. Following the regular fast-forward rules makes some sense
> to me, because you are never losing objects. But there may be
> complications with updating tags in general because of fetch's rules,
> and we would be better off preventing people from accidentally doing so.
> I think a careful review of fetch's tag rules would be in order before
> making any decision there.

The problem with the current behaviour is, that one can never be 100% sure
that his push will not overwrite someone else tag.

-- 
  Kacper

Re: git push tags

[Jeff King]

On Mon, Oct 29, 2012 at 06:23:30PM +0100, Kacper Kornet wrote:

> > That patch just blocks non-forced updates to refs/tags/. I think a saner
> > start would be to disallow updating non-commit objects without a force.
> > We already do so for blobs and trees because they are not (and cannot
> > be) fast forwards. The fact that annotated tags are checked for
> > fast-forward seems to me to be a case of "it happens to work that way"
> > and not anything planned. Since such a push drops the reference to the
> > old version of the tag, it should probably require a force.
> 
> I'm not sure. Looking at 37fde87 ("Fix send-pack for non-commitish
> tags.") I have an impression that Junio allowed for fast-forward pushes
> of annotated tags on purpose.

Hmm. You're right, though I'm not sure I agree with the reasoning of
that commit. I'd certainly like to get Junio's input on the subject.

> > Then on top of that we can talk about what lightweight tags should do.
> > I'm not sure. Following the regular fast-forward rules makes some sense
> > to me, because you are never losing objects. But there may be
> > complications with updating tags in general because of fetch's rules,
> > and we would be better off preventing people from accidentally doing so.
> > I think a careful review of fetch's tag rules would be in order before
> > making any decision there.
> 
> The problem with the current behaviour is, that one can never be 100% sure
> that his push will not overwrite someone else tag.

Yes, although you do know that you are not throwing away history if you
do (because it must be a fast forward). Whereas if you have to use "-f"
to update a tag, then you have turned off all safety checks. So it is an
improvement for one case (creating a tag), but a regression for another
(updating an existing tag). I agree that the latter is probably less
common, but how much? If virtually nobody is doing it because git-fetch
makes the fetching side too difficult, then the regression is probably
not a big deal.

-Peff

Re: git push tags

[Chris Rorvick]

On Mon, Oct 29, 2012 at 4:35 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 06:23:30PM +0100, Kacper Kornet wrote:
>
>> > That patch just blocks non-forced updates to refs/tags/. I think a saner
>> > start would be to disallow updating non-commit objects without a force.
>> > We already do so for blobs and trees because they are not (and cannot
>> > be) fast forwards. The fact that annotated tags are checked for
>> > fast-forward seems to me to be a case of "it happens to work that way"
>> > and not anything planned. Since such a push drops the reference to the
>> > old version of the tag, it should probably require a force.
>>
>> I'm not sure. Looking at 37fde87 ("Fix send-pack for non-commitish
>> tags.") I have an impression that Junio allowed for fast-forward pushes
>> of annotated tags on purpose.
>
> Hmm. You're right, though I'm not sure I agree with the reasoning of
> that commit. I'd certainly like to get Junio's input on the subject.
>
>> > Then on top of that we can talk about what lightweight tags should do.
>> > I'm not sure. Following the regular fast-forward rules makes some sense
>> > to me, because you are never losing objects. But there may be
>> > complications with updating tags in general because of fetch's rules,
>> > and we would be better off preventing people from accidentally doing so.
>> > I think a careful review of fetch's tag rules would be in order before
>> > making any decision there.
>>
>> The problem with the current behaviour is, that one can never be 100% sure
>> that his push will not overwrite someone else tag.
>
> Yes, although you do know that you are not throwing away history if you
> do (because it must be a fast forward). Whereas if you have to use "-f"
> to update a tag, then you have turned off all safety checks. So it is an
> improvement for one case (creating a tag), but a regression for another
> (updating an existing tag). I agree that the latter is probably less
> common, but how much? If virtually nobody is doing it because git-fetch
> makes the fetching side too difficult, then the regression is probably
> not a big deal.
>
> -Peff

This is probably a bit premature given there are still open questions,
but I was curious and decided to take a stab at this.

The change is to only allow fast-forward when both the old and new are
commits and the reference is not a lightweight tag.  All other
reference updates require --force.  I think this resolves the reported
issue and takes into account feedback on this thread.  This change
only broke one test and it was an expected failure given the change in
behavior (i.e., I needed to add a "-f" to update a tag in the remote.)

I wasn't sure how to handle provided feedback to the user when there
are multiple refs not pushed for different reasons.  But I think this
adds the plumbing for handling it correctly, whateverever that this.

It needs some work, but thought I'd throw it out for feedback to see
if it's at least in the right direction.

Chris

--- 8< ---
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..fabcea0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,6 +220,10 @@ static const char message_advice_checkout_pull_push[] =
 	   "(e.g. 'git pull') before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");

+static const char message_advice_ref_already_exists[] =
+	N_("Updates were rejected because a matching reference already exists in\n"
+	   "the remote.  Use git push -f if you really want to make this update.");
+
 static void advise_pull_before_push(void)
 {
 	if (!advice_push_non_ff_current || !advice_push_nonfastforward)
@@ -241,6 +245,11 @@ static void advise_checkout_pull_push(void)
 	advise(_(message_advice_checkout_pull_push));
 }

+static void advise_ref_already_exists(void)
+{
+	advise(_(message_advice_ref_already_exists));
+}
+
 static int push_with_options(struct transport *transport, int flags)
 {
 	int err;
@@ -277,6 +286,9 @@ static int push_with_options(struct transport
*transport, int flags)
 		else
 			advise_checkout_pull_push();
 		break;
+	case ALREADY_EXISTS:
+		advise_ref_already_exists();
+		break;
 	}

 	return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7d05064..f159ec3 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -202,6 +202,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "non-fast forward";
 			break;

+		case REF_STATUS_REJECT_ALREADY_EXISTS:
+			res = "error";
+			msg = "already exists";
+			break;
+
 		case REF_STATUS_REJECT_NODELETE:
 		case REF_STATUS_REMOTE_REJECT:
 			res = "error";
@@ -288,6 +293,7 @@ int send_pack(struct send_pack_args *args,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/cache.h b/cache.h
index a58df84..2d160a9 100644
--- a/cache.h
+++ b/cache.h
@@ -1002,11 +1002,14 @@ struct ref {
 	unsigned int force:1,
 		merge:1,
 		nonfastforward:1,
+		forwardable:1,
+		update:1,
 		deletion:1;
 	enum {
 		REF_STATUS_NONE = 0,
 		REF_STATUS_OK,
 		REF_STATUS_REJECT_NONFASTFORWARD,
+		REF_STATUS_REJECT_ALREADY_EXISTS,
 		REF_STATUS_REJECT_NODELETE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 04fd9ea..0d94888 100644
--- a/remote.c
+++ b/remote.c
@@ -1309,22 +1309,42 @@ void set_ref_status_for_push(struct ref
*remote_refs, int send_mirror,
 		 *     to overwrite it; you would not know what you are losing
 		 *     otherwise.
 		 *
-		 * (3) if both new and old are commit-ish, and new is a
-		 *     descendant of old, it is OK.
+		 * (3) if both new and old are commits, the reference is not
+		 *     a tag, and new is a descendant of old, it is OK.
 		 *
 		 * (4) regardless of all of the above, removing :B is
 		 *     always allowed.
 		 */

-		ref->nonfastforward =
+		ref->update =
 			!ref->deletion &&
-			!is_null_sha1(ref->old_sha1) &&
+			!is_null_sha1(ref->old_sha1);
+
+		ref->nonfastforward =
+			ref->update &&
 			(!has_sha1_file(ref->old_sha1)
 			  || !ref_newer(ref->new_sha1, ref->old_sha1));

-		if (ref->nonfastforward && !ref->force && !force_update) {
-			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
-			continue;
+		if (prefixcmp(ref->name, "refs/tags/")) {
+			struct object *old = parse_object(ref->old_sha1);
+			struct object *new = parse_object(ref->new_sha1);
+			ref->forwardable = (old && new &&
+			  old->type == OBJ_COMMIT && new->type == OBJ_COMMIT);
+		} else
+			ref->forwardable = 0;
+
+		if (!ref->force && !force_update) {
+			if (ref->forwardable) {
+				if (ref->nonfastforward) {
+					ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+					continue;
+				}
+			} else {
+				if (ref->update) {
+					ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
+					continue;
+				}
+			}
 		}
 	}
 }
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..cff559f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
 		git branch -D frotz
 	fi &&
 	git tag -f frotz &&
-	git push testrepo frotz &&
+	git push -f testrepo frotz &&
 	check_push_result $the_commit tags/frotz &&
 	check_push_result $the_first_commit heads/frotz

@@ -929,6 +929,34 @@ test_expect_success 'push into aliased refs
(inconsistent)' '
 	)
 '

+test_expect_success 'push tag requires --force to update remote tag' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git tag lw_tag &&
+		git tag -a -m "message 1" ann_tag &&
+		git push ../child2 lw_tag &&
+		git push ../child2 ann_tag &&
+		>file1 &&
+		git add file1 &&
+		git commit -m "file1" &&
+		git tag -f lw_tag &&
+		git tag -f -a -m "message 2" ann_tag &&
+		! git push ../child2 lw_tag &&
+		! git push ../child2 ann_tag &&
+		git push --force ../child2 lw_tag &&
+		git push --force ../child2 ann_tag &&
+		git tag -f lw_tag HEAD~ &&
+		git tag -f -a -m "message 3" ann_tag &&
+		! git push ../child2 lw_tag &&
+		! git push ../child2 ann_tag &&
+		git push --force ../child2 lw_tag &&
+		git push --force ../child2 ann_tag
+	)
+'
+
 test_expect_success 'push --porcelain' '
 	mk_empty &&
 	echo >.git/foo  "To testrepo" &&
diff --git a/transport-helper.c b/transport-helper.c
index cfe0988..ef9a6f8 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -643,6 +643,11 @@ static void push_update_ref_status(struct strbuf *buf,
 			free(msg);
 			msg = NULL;
 		}
+		else if (!strcmp(msg, "already exists")) {
+			status = REF_STATUS_REJECT_ALREADY_EXISTS;
+			free(msg);
+			msg = NULL;
+		}
 	}

 	if (*ref)
@@ -702,6 +707,7 @@ static int push_refs_with_push(struct transport *transport,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/transport.c b/transport.c
index 9932f40..d218884 100644
--- a/transport.c
+++ b/transport.c
@@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref,
int porcelain)
 		const char *msg;

 		strcpy(quickref, status_abbrev(ref->old_sha1));
-		if (ref->nonfastforward) {
+		if (ref->nonfastforward || (!ref->forwardable && ref->update)) {
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
@@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref,
const char *dest, int count, i
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 						 "non-fast-forward", porcelain);
 		break;
+	case REF_STATUS_REJECT_ALREADY_EXISTS:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+						 "already exists", porcelain);
+		break;
 	case REF_STATUS_REMOTE_REJECT:
 		print_ref_status('!', "[remote rejected]", ref,
 						 ref->deletion ? NULL : ref->peer_ref,
@@ -714,7 +718,7 @@ static int print_one_push_status(struct ref *ref,
const char *dest, int count, i
 }

 void transport_print_push_status(const char *dest, struct ref *refs,
-				  int verbose, int porcelain, int *nonfastforward)
+				  int verbose, int porcelain, int *willnotupdate)
 {
 	struct ref *ref;
 	int n = 0;
@@ -733,18 +737,21 @@ void transport_print_push_status(const char
*dest, struct ref *refs,
 		if (ref->status == REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);

-	*nonfastforward = 0;
+	*willnotupdate = 0;
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD &&
-		    *nonfastforward != NON_FF_HEAD) {
+		    *willnotupdate != NON_FF_HEAD) {
 			if (!strcmp(head, ref->name))
-				*nonfastforward = NON_FF_HEAD;
+				*willnotupdate = NON_FF_HEAD;
 			else
-				*nonfastforward = NON_FF_OTHER;
+				*willnotupdate = NON_FF_OTHER;
+		} else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS &&
+		    *willnotupdate == 0) {
+				*willnotupdate = ALREADY_EXISTS;
 		}
 	}
 }
@@ -1031,9 +1038,9 @@ static void die_with_unpushed_submodules(struct
string_list *needs_pushing)

 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
-		   int *nonfastforward)
+		   int *willnotupdate)
 {
-	*nonfastforward = 0;
+	*willnotupdate = 0;
 	transport_verify_remote_names(refspec_nr, refspec);

 	if (transport->push) {
@@ -1099,7 +1106,7 @@ int transport_push(struct transport *transport,
 		if (!quiet || err)
 			transport_print_push_status(transport->url, remote_refs,
 					verbose | porcelain, porcelain,
-					nonfastforward);
+					willnotupdate);

 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			set_upstreams(transport, remote_refs, pretend);
diff --git a/transport.h b/transport.h
index 3b21c4a..326271e 100644
--- a/transport.h
+++ b/transport.h
@@ -142,6 +142,7 @@ void transport_set_verbosity(struct transport
*transport, int verbosity,

 #define NON_FF_HEAD 1
 #define NON_FF_OTHER 2
+#define ALREADY_EXISTS 3
 int transport_push(struct transport *connection,
 		   int refspec_nr, const char **refspec, int flags,
 		   int * nonfastforward);
@@ -170,7 +171,7 @@ void transport_update_tracking_ref(struct remote
*remote, struct ref *ref, int v
 int transport_refs_pushed(struct ref *ref);

 void transport_print_push_status(const char *dest, struct ref *refs,
-		  int verbose, int porcelain, int *nonfastforward);
+		  int verbose, int porcelain, int *willnotupdate);

 typedef void alternate_ref_fn(const struct ref *, void *);
 extern void for_each_alternate_ref(alternate_ref_fn, void *);
-- 
1.8.0

Re: git push tags

[Chris Rorvick]

On Tue, Oct 30, 2012 at 1:34 PM, Angelo Borsotti
<angelo.borsotti@gmail.com> wrote:
> Hi Cris,
>
> I think a key in the config file of the remote repo is better than an
> option on git-push for what concerns security: it allows the owner of
> the remote repo to enforce the policy not to overwrite tags, which
> would not be possible if any user that has push access can --force
> tags.
>
> -Angelo

Hi Angelo,

Security is orthogonal to what this patch is attempting to resolve.
As Kacper pointed out, you can never be sure you're not going to
clobber an existing tag in the remote repo.  This patch attempts to
give git-push better (i.e., less surprising) semantics for tags.  In
other words, it's should will prevent mistakes, not provide any sort
of security.

So I don't think a config option is better or worse, it's just trying
to solve a different problem.

Thanks,

Chris