Re: [PATCH 1/2] t1450: robustify `remove_object()`

From: Taylor Blau <me@ttaylorr.com>
Date: Feb 10, 2021, 23:20 UTC
Message-ID: <YCRqOVS/iXp/d6Rc@nand.local>
In-Reply-To: <xmqqtuqj1wzg.fsf@gitster.c.googlers.com>

On Wed, Feb 10, 2021 at 12:36:19PM -0800, Junio C Hamano wrote:

Show 17 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > -test_expect_success 'setup: helpers for corruption tests' '
> > -	sha1_file() {
> > -		remainder=${1#??} &&
> > -		firsttwo=${1%$remainder} &&
> > -		echo ".git/objects/$firsttwo/$remainder"
> > -	} &&
> > +sha1_file () {
> > +	git rev-parse --git-path objects/$(test_oid_to_path "$1")
> > +}
>
> Yeah, back when 90cf590f (fsck: optionally show more helpful info
> for broken links, 2016-07-17) originally introduced this pattern,
> we didn't have nicely abstracted helper, but now we do, and there
> is no reason not to use it.  Nice.

This has nothing to do with this series, but I do notice a number of other uses of test_oid_to_path that are doing this exact thing. In fact, many of them don't use "git rev-parse --git-path", which would be better.

I wonder if it's worth a clean-up on top to consolidate all of those "combine the loose object path of the object with xyz OID and the $GIT_DIR/objects directory".

In either case -- and I think I'm pretty clearly being pedantic at this point -- do you suppose it's worthwhile to rename sha1_file to something that doesn't have sha1 in it?

Thanks, Taylor