Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
- From
Jeff King <peff@peff.net>
- Date
- Dec 1, 2021, 19:26 UTC
- Message-ID
- <YafMS6qI+6t6SOtg@coredump.intra.peff.net>
- In-Reply-To
- <xmqqh7bs177v.fsf@gitster.g>
On Wed, Dec 01, 2021 at 11:00:04AM -0800, Junio C Hamano wrote:
Show 5 quoted lines
> > 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.
Show 5 quoted lines
> 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