Re: [PATCH v6 0/8] push: update remote tags only with force

On Thu, Jan 17, 2013 at 12:59 AM, Junio C Hamano [off-list ref] wrote:

Chris Rorvick [off-list ref] writes:

quoted

On Wed, Jan 16, 2013 at 10:48 AM, Junio C Hamano [off-list ref] wrote:

quoted

It is fine when pushing into "refs/tags/" hierarchy.  It is *NOT*
OK if the type check does not satisfy this function.  In that case,
we do not actually see the existence of the destination as a
problem, but it is reported as such.  We are blocking because we do
not like the type of the new object or the type of the old object.
If the destination points at a commit, the push can succeed if the
user changes what object to push, so saying "you cannot push because
the destination already exists" is just wrong in such a case.

So the solution is to revert back to recommending a merge?

Of course not, because at that point you may not even have what you
were attempting to overwrite.  Nobody says it is even something you
could merge.

I was referring to your concern about rejecting based on type.  A push
causing a reference to move (for example) from a commit to a blob is
rejected as "already exists" with this patch.  You emphatically state
this is not OK and your solution is to revert back to behavior that
advises a merge.

Clearly the bug regarding an 'old' unknown to the client should be
fixed.  This is a obvious test case I should have covered and it's
unfortunate it made it into master.  But I don't understand why
is_forwardable() was misguided (maybe poorly named) nor why
ref_newer() is a better place to solve the issues it was addressing.

Chris