Subject: Re: Problems with t6011

From: Junio C Hamano <[email-masked]>
Date: Tue, 06 May 2025 14:18:33 -0700

Torsten Bögershausen <[email-masked]> writes:

> Hej Patrick,
> in case you have a second:
> the mv command here needs a "-f" to overwrite
> read-only files:
>
> --- a/t/t6011-rev-list-with-bad-commit.sh
> +++ b/t/t6011-rev-list-with-bad-commit.sh
> @@ -39,7 +39,7 @@ test_expect_success 'corrupt second commit object' '
>         for p in .git/objects/pack/*.pack
>         do
>                 sed "s/second commit/socond commit/" "$p" >"$p.munged" &&
> -               mv "$p.munged" "$p" ||
> +               mv -f "$p.munged" "$p" ||
>                 return 1

Looking at the remainder of cdbdc6bf (t: refactor tests depending on
Perl substitution operator, 2025-04-03), the commit that introduced
these lines, it seems that the prevailing pattern was:

	chmod +w "$packfile" &&
	perl -pe "regexp" "$packfile" >"$packfile.munged" &&
	mv "packfile.munged" $packfile"

but the original for this loop used "perl -i.bak -pe" that dealt
with read-only input just fine, wihtout the need for a separate
"mv".

So the lack of "-f" indeed is a bug in that "refactor" commit.

I would be more worried about using "sed" on clearly non-text files,
which technically is undefined operation, and I strongly suspect
that it was the reason why we used Perl to munge files that are
clearly binary, like the packfiles, in the first place.