Re: Problems with t6011

From: Junio C Hamano <gitster@pobox.com>
Date: May 6, 2025, 21:18 UTC
Message-ID: <xmqqa57ph1zq.fsf@gitster.g>
In-Reply-To: <71fcb24d-55e3-40bb-9368-5b47aa180993@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

Show 14 quoted lines

> 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.