All of lore.kernel.org
 help / color / mirror / Atom feed
From: "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Michael Haggerty" <mhagger@alum.mit.edu>,
	"Christian Couder" <christian.couder@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"ZheNing Hu" <adlternative@gmail.com>
Subject: [PATCH v2 0/2] [RFC] push: allow delete one level ref
Date: Sat, 04 Feb 2023 16:48:15 +0000	[thread overview]
Message-ID: <pull.1465.v2.git.1675529298.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1465.git.1673951562.gitgitgadget@gmail.com>

This might be an odd request: allow git push to delete one level refs like
"ref/foo".

Some users on my side often push "refs/for/master" to the remote for code
review, but due to a user's typo, "refs/master" is pushed to the remote.

Pushing a one level ref like "refs/foo" should not have been successful, but
since my server side directly updated the ref during the proc-receive-hook
phase of git receive-pack, "refs/foo" was mistakenly left at on the server.

But for some reasons, users cannot delete this special branch through "git
push -d".

First, I executed git update-ref --stdin inside my proc-receive-hook to
delete the branch. As a result, update-ref reported an error: "cannot lock
ref 'refs/foo': reference already exists".

So I tried GIT_TRACKET_PACKET to debug and found that git push did not seem
to pass the correct ref old-oid, which is why update-ref reported an error.

"18:13:41.214872 pkt-line.c:80           packet: receive-pack< 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 refs/foo\0 report-status-v2 side-band-64k object-format=sha1 agent=git/2.39.0.227.g262c45b6a1"


Tracing it back, the check_ref() in the git push link didn't record the
old-oid for the remote "refs/foo".

At the same time, the server update() process will reject the one level ref
by default and return a "funny refname" error.

It is worth mentioning that since I deleted the branch, the error message
returned here is "refusing to create funny ref 'refs/foo' remotely", which
is also worth fixing.

So this patch series do:

v1.

 1. fix funny refname error message from "create" to "update".
 2. let server allow delete one level ref.
 3. let client pass correct one level ref old-oid.

v2.

 1. fix code style.

ZheNing Hu (2):
  receive-pack: fix funny ref error messsage
  [RFC] push: allow delete one level ref

 builtin/receive-pack.c |  6 ++++--
 connect.c              |  3 ++-
 t/t5516-fetch-push.sh  | 12 ++++++++++++
 3 files changed, 18 insertions(+), 3 deletions(-)


base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1465%2Fadlternative%2Fzh%2Fdelete-one-level-ref-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1465/adlternative/zh/delete-one-level-ref-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1465

Range-diff vs v1:

 1:  214a2b662e3 = 1:  214a2b662e3 receive-pack: fix funny ref error messsage
 2:  605b95bf8ab ! 2:  966cb49c388 [RFC] push: allow delete one level ref
     @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh
       	/* only refs/... are allowed */
      -	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
      +	if (!starts_with(name, "refs/") ||
     -+	    check_refname_format(name + 5,
     -+				 is_null_oid(new_oid) ?
     ++	    check_refname_format(name + 5, is_null_oid(new_oid) ?
      +				 REFNAME_ALLOW_ONELEVEL : 0)) {
       		rp_error("refusing to update funny ref '%s' remotely", name);
       		ret = "funny refname";
     @@ connect.c: static int check_ref(const char *name, unsigned int flags)
       
       	/* REF_NORMAL means that we don't want the magic fake tag refs */
      -	if ((flags & REF_NORMAL) && check_refname_format(name, 0))
     -+	if ((flags & REF_NORMAL) && check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
     ++	if ((flags & REF_NORMAL) && check_refname_format(name,
     ++							 REFNAME_ALLOW_ONELEVEL))
       		return 0;
       
       	/* REF_HEADS means that we want regular branch heads */
     @@ t/t5516-fetch-push.sh: test_expect_success 'push --delete refuses empty string'
       
      +test_expect_success 'push --delete onelevel refspecs' '
      +	mk_test testrepo heads/main &&
     -+	(
     -+		cd testrepo &&
     -+		git update-ref refs/onelevel refs/heads/main
     -+	) &&
     ++	git -C testrepo update-ref refs/onelevel refs/heads/main &&
      +	git push testrepo --delete refs/onelevel &&
     -+	(
     -+		cd testrepo &&
     -+		test_must_fail git rev-parse --verify refs/onelevel
     -+	)
     ++	test_must_fail git -C testrepo rev-parse --verify refs/onelevel
      +'
      +
       test_expect_success 'warn on push to HEAD of non-bare repository' '

-- 
gitgitgadget

  parent reply	other threads:[~2023-02-04 16:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-17 10:32 [PATCH 0/2] [RFC] push: allow delete one level ref ZheNing Hu via GitGitGadget
2023-01-17 10:32 ` [PATCH 1/2] receive-pack: fix funny ref error messsage ZheNing Hu via GitGitGadget
2023-01-17 10:32 ` [PATCH 2/2] [RFC] push: allow delete one level ref ZheNing Hu via GitGitGadget
2023-01-17 13:14   ` Ævar Arnfjörð Bjarmason
2023-01-19 17:39     ` ZheNing Hu
2023-02-04 16:48 ` ZheNing Hu via GitGitGadget [this message]
2023-02-04 16:48   ` [PATCH v2 1/2] receive-pack: fix funny ref error messsage ZheNing Hu via GitGitGadget
2023-02-04 16:48   ` [PATCH v2 2/2] [RFC] push: allow delete one level ref ZheNing Hu via GitGitGadget
2023-02-06 22:13     ` Junio C Hamano
2023-02-24  6:02       ` ZheNing Hu
2023-02-24 17:12         ` Junio C Hamano
2023-02-25 14:11           ` ZheNing Hu
2023-02-27  1:57   ` [PATCH v3 0/2] " ZheNing Hu via GitGitGadget
2023-02-27  1:57     ` [PATCH v3 1/2] receive-pack: fix funny ref error messsage ZheNing Hu via GitGitGadget
2023-02-27 16:07       ` Junio C Hamano
2023-03-01  9:31         ` ZheNing Hu
2023-02-27  1:57     ` [PATCH v3 2/2] [RFC] push: allow delete single-level ref ZheNing Hu via GitGitGadget
2023-03-01 10:20     ` [PATCH v4 0/2] [RFC] push: allow delete one level ref ZheNing Hu via GitGitGadget
2023-03-01 10:20       ` [PATCH v4 1/2] receive-pack: fix funny ref error messsage ZheNing Hu via GitGitGadget
2023-03-01 10:20       ` [PATCH v4 2/2] push: allow delete single-level ref ZheNing Hu via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1465.v2.git.1675529298.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=adlternative@gmail.com \
    --cc=avarab@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.