Re: [PATCH] push: don't guess at qualifying remote refs on deletion
- From
jonsmirl@gmail.com <jonsmirl@gmail.com>
- Date
- Jul 3, 2012, 18:34 UTC
- Message-ID
- <CAKON4Oy0YBVTAhZPU=1B=yYY4t2O_uRWDW1zOMaC5iCb=kRQ2w@mail.gmail.com>
- In-Reply-To
- <20120703180439.GC3294@sigill.intra.peff.net>
On Tue, Jul 3, 2012 at 2:04 PM, Jeff King <peff@peff.net> wrote:
Show 31 quoted lines
> 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
Show 58 quoted lines
> 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
>-- Jon Smirl jonsmirl@gmail.com