git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Han-Wen Nienhuys <hanwen@google.com>,
	Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>
Subject: Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Date: Wed, 1 Dec 2021 14:26:03 -0500	[thread overview]
Message-ID: <YafMS6qI+6t6SOtg@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqh7bs177v.fsf@gitster.g>

On Wed, Dec 01, 2021 at 11:00:04AM -0800, Junio C Hamano wrote:

> > The test helper takes the flag as an argument, in decimal. If you look
> > for 2048, you should find it.
> 
> Awful---when the symbolic constants change in the code, the test
> will silently break?

Agreed, this is quite nasty.

> It has been this way since 80f2a609 (t/helper: add test-ref-store to
> test ref-store functions, 2017-03-26), so it is nothing new, but at
> some point, we should do a better job.  Even if it is used only as a
> tool for testing, we shouldn't have to force developers to write in
> assembly ;-)

Sort of. The code to pass the flags was added then, but nobody was using
it until now.

At least for ref updates. Symref creation allowed "1" for force, but
that is a true boolean. It looks like some pack-refs calls pass "3" for
PACK_REFS_PRUNE|PACK_REFS_ALL.

So it may be considered a "new" issue in that sense.

> Perhaps when the dust settles after this series graduates to be a
> part of released version.  It may be a good bite-sized microproject
> material for aspiring new developers.

It is annoying to have to plumb through the names of the flags, but in
practice I don't think we need all (or even most) of them. And it's
equally annoying not to be able to grep for the flags. I already had a
hard enough time just grepping for the callers, and resorted to:

diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 3986665037..1b7a124a86 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -271,6 +271,8 @@ int cmd__ref_store(int argc, const char **argv)
 	const char *func;
 	struct command *cmd;
 
+	trace_argv_printf(argv, "trace: test-tool:");
+
 	setup_git_directory();
 
 	argv = get_store(argv + 1, &refs);

to find them.

-Peff

  reply	other threads:[~2021-12-01 19:26 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-25 15:55 [PATCH 0/2] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget
2021-11-25 15:56 ` [PATCH 1/2] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-11-25 15:56 ` [PATCH 2/2] refs: allow skipping OID verification Han-Wen Nienhuys via GitGitGadget
2021-11-26  7:32   ` Junio C Hamano
2021-11-29 18:49 ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget
2021-11-29 18:49   ` [PATCH v2 1/6] test-ref-store: plug memory leak in cmd_delete_refs Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:15     ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 2/6] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:17     ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 3/6] refs: allow skipping OID verification Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:28     ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:22     ` Junio C Hamano
2021-11-29 23:31     ` Junio C Hamano
2021-11-30 10:31       ` Han-Wen Nienhuys
2021-12-01 19:00         ` Junio C Hamano
2021-12-01 19:26           ` Jeff King [this message]
2021-12-02 16:40             ` Han-Wen Nienhuys
2021-12-02 19:05               ` Junio C Hamano
2021-11-29 18:49   ` [PATCH v2 5/6] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget
2021-11-29 18:49   ` [PATCH v2 6/6] t1430: create valid symrefs using test-helper Han-Wen Nienhuys via GitGitGadget
2021-11-29 23:12   ` [PATCH v2 0/6] Allow writing invalid OIDs into refs for testing purposes Junio C Hamano
2021-12-02 18:39   ` [PATCH v3 0/8] " Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 1/8] test-ref-store: remove force-create argument for create-reflog Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 2/8] test-ref-store: parse symbolic flag constants Han-Wen Nienhuys via GitGitGadget
2021-12-03  6:22       ` Jeff King
2021-12-02 18:39     ` [PATCH v3 3/8] test-ref-store: plug memory leak in cmd_delete_refs Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:39     ` [PATCH v3 5/8] refs: introduce REF_SKIP_OID_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:40     ` [PATCH v3 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:40     ` [PATCH v3 7/8] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget
2021-12-02 18:40     ` [PATCH v3 8/8] t1430: create valid symrefs using test-helper Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38     ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 1/8] test-ref-store: remove force-create argument for create-reflog Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 2/8] test-ref-store: parse symbolic flag constants Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 3/8] test-ref-store: plug memory leak in cmd_delete_refs Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 4/8] refs: update comment Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 5/8] refs: introduce REF_SKIP_OID_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 6/8] refs: introduce REF_SKIP_REFNAME_VERIFICATION flag Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 7/8] t1430: remove refs using test-tool Han-Wen Nienhuys via GitGitGadget
2021-12-07 13:38       ` [PATCH v4 8/8] t1430: create valid symrefs using test-helper Han-Wen Nienhuys via GitGitGadget
2021-12-07 21:18       ` [PATCH v4 0/8] Allow writing invalid OIDs into refs for testing purposes Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=YafMS6qI+6t6SOtg@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).