Re: [PATCH] push: don't guess at qualifying remote refs on deletion

["jonsmirl@xxxxxxxxx" <jonsmirl@xxxxxxxxx>]

On Tue, Jul 3, 2012 at 2:04 PM, Jeff King <peff@xxxxxxxx> wrote:
> When we try to push a ref and the right-hand side of the
> refspec does not find a match, we try to create it. If it is
> not fully qualified, we try to guess where it would go in
> the refs hierarchy based on the left-hand source side. If
> the source side is not a ref, then we give up and give a
> long explanatory message.
>
> For deletions, however, this doesn't make any sense. We
> would never want to create on the remote side, and if an
> unqualified ref can't be matched, it is simply an error. The
> current code handles this already because the left-hand side
> is empty, and therefore does not give us a hint as to where
> the right-hand side should go, and we properly error out.
> Unfortunately, the error message is the long "we tried to
> qualify this, but the source side didn't let us guess"
> message, which is quite confusing.
>
> Instead, we can just be more succinct and say "we can't
> delete this because we couldn't find it". So before:
>
>   $ git push origin :bogus
>   error: unable to push to unqualified destination: bogus
>   The destination refspec neither matches an existing ref on the remote nor
>   begins with refs/, and we are unable to guess a prefix based on the source ref.
>   error: failed to push some refs to '$URL'
>
> and now:
>
>   $ git push origin :bogus
>   error: unable to delete 'bogus': remote ref does not exist
>   error: failed to push some refs to '$URL'

This error return would have made my mistake obvious.

Might want to add a paragraph to the doc saying this is how you delete
remote branches since it is not an obvious solution. I found it via
Google and a question asked on stackoverflow.com

> It is tempting to also catch a fully-qualified ref like
> "refs/heads/bogus" and generate the same error message.
> However, that currently does not error out at all, and
> instead gets sent to the remote side, which typically
> generates a warning:
>
>   $ git push origin:refs/heads/bogus
>   remote: warning: Deleting a non-existent ref.
>   To $URL
>    - [deleted]         bogus
>
> While it would be nice to catch this error early, a
> client-side error would mean aborting the push entirely and
> changing push's exit code. For example, right now you can
> do:
>
>   $ git push origin refs/heads/foo refs/heads/bar
>
> and end up in a state where "foo" and "bar" are deleted,
> whether both of them currently exist or not (and see an
> error only if we actually failed to contact the server).
> Generating an error would cause a regression for this use
> case.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> On Tue, Jul 03, 2012 at 07:42:07AM -0400, jonsmirl@xxxxxxxxx wrote:
>
>> I have the branch name wrong. It is fl.stgit not fl.stg.
>> But the error message sent me off in the wrong direction looking for an answer.
>
> I think this would help. I used "remote ref does not exist"
> because that is the simplest explanation for the user.
> However, given that we will try to push a fully qualified
> ref that does not exist, a more accurate message might
> "destination refspec did not match" or something similar.  I
> prefer the former, though, as it less arcane.
>
>  remote.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/remote.c b/remote.c
> index 6833538..04fd9ea 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1100,6 +1100,9 @@ static int match_explicit(struct ref *src, struct ref *dst,
>         case 0:
>                 if (!memcmp(dst_value, "refs/", 5))
>                         matched_dst = make_linked_ref(dst_value, dst_tail);
> +               else if (is_null_sha1(matched_src->new_sha1))
> +                       error("unable to delete '%s': remote ref does not exist",
> +                             dst_value);
>                 else if ((dst_guess = guess_ref(dst_value, matched_src)))
>                         matched_dst = make_linked_ref(dst_guess, dst_tail);
>                 else
> --
> 1.7.11.rc1.21.g3c8d91e
>



-- 
Jon Smirl
jonsmirl@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html