[PATCH] push: don't guess at qualifying remote refs on deletion
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'
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@peff.net>
---
On Tue, Jul 03, 2012 at 07:42:07AM -0400, jonsmirl@gmail.com 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
jonsmirl@gmail.com
