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

2 messages, 2 authors, 2021-02-10 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-02-10 20:37:04

"Johannes Schindelin via GitGitGadget" [off-list ref]
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.
-	remove_object() {
-		rm "$(sha1_file "$1")"
-	}
-'
+remove_object() {
Just like you did for the other one, let's insert SP before () for
consistency here.
+	rm "$(sha1_file "$1")"
+}
 
 test_expect_success 'object with bad sha1' '
 	sha=$(echo blob | git hash-object -w --stdin) &&
Nicely done.  

Reviewed-by: Junio C Hamano <redacted>

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

From: Taylor Blau <hidden>
Date: 2021-02-10 23:21:45

On Wed, Feb 10, 2021 at 12:36:19PM -0800, Junio C Hamano wrote:
"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:
quoted
-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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help