diff mbox series

[v3,1/2] receive-pack: fix funny ref error messsage

Message ID 857d2435caf6211cd5a1baa6c9d77311ce7ba745.1677463022.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series push: allow delete one level ref | expand

Commit Message

ZheNing Hu Feb. 27, 2023, 1:57 a.m. UTC
From: ZheNing Hu <adlternative@gmail.com>

When the user deletes the remote one level branch through
"git push origin -d refs/foo", remote will return an error:
"refusing to create funny ref 'refs/foo' remotely", here we
are not creating "refs/foo" instead wants to delete it, so a
better error description here would be: "refusing to update
funny ref 'refs/foo' remotely".

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 builtin/receive-pack.c | 2 +-
 t/t5516-fetch-push.sh  | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Junio C Hamano Feb. 27, 2023, 4:07 p.m. UTC | #1
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: ZheNing Hu <adlternative@gmail.com>
>
> When the user deletes the remote one level branch through
> "git push origin -d refs/foo", remote will return an error:
> "refusing to create funny ref 'refs/foo' remotely", here we
> are not creating "refs/foo" instead wants to delete it, so a
> better error description here would be: "refusing to update
> funny ref 'refs/foo' remotely".

OK, update() works on each ref affected, not just the ones that are
updated, but also created and deleted.  The updated wording may
probably be better.


>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>  builtin/receive-pack.c | 2 +-
>  t/t5516-fetch-push.sh  | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> ...
> +test_expect_success 'push with onelevel ref' '
> +	mk_test testrepo heads/main &&
> +	test_must_fail git push testrepo HEAD:refs/onelevel
> +'
> +

I am not sure what relevance this new test has to the proposed
update to the message, though.

Thanks.
ZheNing Hu March 1, 2023, 9:31 a.m. UTC | #2
Junio C Hamano <gitster@pobox.com> 于2023年2月28日周二 00:07写道:
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > When the user deletes the remote one level branch through
> > "git push origin -d refs/foo", remote will return an error:
> > "refusing to create funny ref 'refs/foo' remotely", here we
> > are not creating "refs/foo" instead wants to delete it, so a
> > better error description here would be: "refusing to update
> > funny ref 'refs/foo' remotely".
>
> OK, update() works on each ref affected, not just the ones that are
> updated, but also created and deleted.  The updated wording may
> probably be better.
>
>
> >
> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> > ---
> >  builtin/receive-pack.c | 2 +-
> >  t/t5516-fetch-push.sh  | 5 +++++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > ...
> > +test_expect_success 'push with onelevel ref' '
> > +     mk_test testrepo heads/main &&
> > +     test_must_fail git push testrepo HEAD:refs/onelevel
> > +'
> > +
>
> I am not sure what relevance this new test has to the proposed
> update to the message, though.
>

Ah, I should have incorporated this part of the test into another
patch test.

> Thanks.
diff mbox series

Patch

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index cd5c7a28eff..c24616a3ac6 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1464,7 +1464,7 @@  static const char *update(struct command *cmd, struct shallow_info *si)
 
 	/* only refs/... are allowed */
 	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
-		rp_error("refusing to create funny ref '%s' remotely", name);
+		rp_error("refusing to update funny ref '%s' remotely", name);
 		ret = "funny refname";
 		goto out;
 	}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 98a27a2948b..f37861efc40 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -401,6 +401,11 @@  test_expect_success 'push with ambiguity' '
 
 '
 
+test_expect_success 'push with onelevel ref' '
+	mk_test testrepo heads/main &&
+	test_must_fail git push testrepo HEAD:refs/onelevel
+'
+
 test_expect_success 'push with colon-less refspec (1)' '
 
 	mk_test testrepo heads/frotz tags/frotz &&