{"thread":{"id":"63404","subject":"Problems with t6011","startedAt":"2025-05-06T12:32:19Z","lastAt":"2025-05-07T15:30:02Z","messageCount":5,"participants":["Torsten Bögershausen","Junio C Hamano","Patrick Steinhardt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"517332","messageId":"71fcb24d-55e3-40bb-9368-5b47aa180993@web.de","threadId":"63404","inReplyTo":null,"subject":"Problems with t6011","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2025-05-06T12:32:04Z","receivedAt":"2025-05-06T12:32:19Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Hej Patrick,\nin case you have a second:\nthe mv command here needs a \"-f\" to overwrite\nread-only files:\n\n--- a/t/t6011-rev-list-with-bad-commit.sh\n+++ b/t/t6011-rev-list-with-bad-commit.sh\n@@ -39,7 +39,7 @@ test_expect_success 'corrupt second commit object' '\n         for p in .git/objects/pack/*.pack\n         do\n                 sed \"s/second commit/socond commit/\" \"$p\" >\"$p.munged\" &&\n-               mv \"$p.munged\" \"$p\" ||\n+               mv -f \"$p.munged\" \"$p\" ||\n                 return 1\n"},{"id":"517393","messageId":"xmqqa57ph1zq.fsf@gitster.g","threadId":"63404","inReplyTo":"71fcb24d-55e3-40bb-9368-5b47aa180993@web.de","subject":"Re: Problems with t6011","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-06T21:18:33Z","receivedAt":"2025-05-06T21:18:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> Hej Patrick,\n> in case you have a second:\n> the mv command here needs a \"-f\" to overwrite\n> read-only files:\n>\n> --- a/t/t6011-rev-list-with-bad-commit.sh\n> +++ b/t/t6011-rev-list-with-bad-commit.sh\n> @@ -39,7 +39,7 @@ test_expect_success 'corrupt second commit object' '\n>         for p in .git/objects/pack/*.pack\n>         do\n>                 sed \"s/second commit/socond commit/\" \"$p\" >\"$p.munged\" &&\n> -               mv \"$p.munged\" \"$p\" ||\n> +               mv -f \"$p.munged\" \"$p\" ||\n>                 return 1\n\nLooking at the remainder of cdbdc6bf (t: refactor tests depending on\nPerl substitution operator, 2025-04-03), the commit that introduced\nthese lines, it seems that the prevailing pattern was:\n\n\tchmod +w \"$packfile\" &&\n\tperl -pe \"regexp\" \"$packfile\" >\"$packfile.munged\" &&\n\tmv \"packfile.munged\" $packfile\"\n\nbut the original for this loop used \"perl -i.bak -pe\" that dealt\nwith read-only input just fine, wihtout the need for a separate\n\"mv\".\n\nSo the lack of \"-f\" indeed is a bug in that \"refactor\" commit.\n\nI would be more worried about using \"sed\" on clearly non-text files,\nwhich technically is undefined operation, and I strongly suspect\nthat it was the reason why we used Perl to munge files that are\nclearly binary, like the packfiles, in the first place.\n\n\n"},{"id":"517400","messageId":"xmqq8qn9fj8o.fsf@gitster.g","threadId":"63404","inReplyTo":"71fcb24d-55e3-40bb-9368-5b47aa180993@web.de","subject":"[PATCH] t6011: fix misconversion from perl to sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-06T22:48:55Z","receivedAt":"2025-05-06T22:48:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"No, this is not about a quiz on regexp compatibility between Perl\nand sed.\n\nBack when cdbdc6bf (t: refactor tests depending on Perl substitution\noperator, 2025-04-03) rewrite many use of perl with sed, the general\npattern of the original scripts were\n\n    chmod +w some_read_only_file &&\n    perl -p -e \"regexp to munge\" some_read_only_file >some_tmp &&\n    mv some_tmp some_read_only_file\n\npersumably because the author new replacing some_read_only_file with\n\"mv\" at the last step would not work without \"mv -f\" in some\nenvironments (GNU does not seem to give any prompt when not running\ninteractively, which is what happens when running t/ scripts).\nReplacing perl with sed would be fine as long as sed with updated\nregexp does the equivalent munging.\n\nBut one place used to use a different construct in the original:\n\n    perl -i.bak -p -e \"regexp to munge\" some_read_only_file\n\nWith _no_ temporary file or \"mv\", \"perl -i\" allows you to replace a\nread-only file in place.\n\nWhen we replaced the use of \"perl\" with \"sed\" in the said commit,\nhowever, because \"sed -i\" is not portable, we rewrote that in-place\nreplacement to\n\n    sed \"regexp to munge\" some_read_only_file >some_tmp &&\n    mv some_tmp some_read_only_file\n\nAgain, unfortunately that does not work in some environment, without\n\"mv -f\".\n\nWe could run \"mv -f\" here, but we would then need to remove \"chmod\n+w\" and have them use \"mv -f\" instead at all places that were\ntouched cdbdc6bf (t: refactor tests depending on Perl substitution\noperator, 2025-04-03) to be consistent (and more concise).\n\nFor now, let's make it consistent in the other direction by mimick\nthe other places that made the target read-write before moving.\n\nSpeaking of portability, the outcome of using \"sed\" on non-text\nfiles is unspecified, so the entire exercise of cdbdc6bf may have\nneeded to be reverted if people still used ancient version of\n\"standard compliant\" sed that barfs on non-text files, but these\ndays we may be able to get away with \"BSDs and GNU seem OK with it\"\n;-)  But one fix at a time.\n\nReported-by: Torsten Bögershausen <tboegi@web.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n t/t6011-rev-list-with-bad-commit.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh\nindex b6f3344dbf..1dd1e50d21 100755\n--- a/t/t6011-rev-list-with-bad-commit.sh\n+++ b/t/t6011-rev-list-with-bad-commit.sh\n@@ -38,6 +38,7 @@ test_expect_success 'verify number of revisions' \\\n test_expect_success 'corrupt second commit object' '\n \tfor p in .git/objects/pack/*.pack\n \tdo\n+\t\tchmod +w \"$p\" &&\n \t\tsed \"s/second commit/socond commit/\" \"$p\" >\"$p.munged\" &&\n \t\tmv \"$p.munged\" \"$p\" ||\n \t\treturn 1\n-- \n2.49.0-615-gd1e3f1cce9\n\n\n"},{"id":"517440","messageId":"aBroM9wh7Oo0xMU2@pks.im","threadId":"63404","inReplyTo":"xmqq8qn9fj8o.fsf@gitster.g","subject":"Re: [PATCH] t6011: fix misconversion from perl to sed","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-07T04:57:23Z","receivedAt":"2025-05-07T04:57:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, May 06, 2025 at 03:48:55PM -0700, Junio C Hamano wrote:\n> No, this is not about a quiz on regexp compatibility between Perl\n> and sed.\n> \n> Back when cdbdc6bf (t: refactor tests depending on Perl substitution\n> operator, 2025-04-03) rewrite many use of perl with sed, the general\n\ns/rewrite many use/rewrote many uses/\n\n> pattern of the original scripts were\n> \n>     chmod +w some_read_only_file &&\n>     perl -p -e \"regexp to munge\" some_read_only_file >some_tmp &&\n>     mv some_tmp some_read_only_file\n> \n> persumably because the author new replacing some_read_only_file with\n\ns/new/knew?\n\n> diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh\n> index b6f3344dbf..1dd1e50d21 100755\n> --- a/t/t6011-rev-list-with-bad-commit.sh\n> +++ b/t/t6011-rev-list-with-bad-commit.sh\n> @@ -38,6 +38,7 @@ test_expect_success 'verify number of revisions' \\\n>  test_expect_success 'corrupt second commit object' '\n>  \tfor p in .git/objects/pack/*.pack\n>  \tdo\n> +\t\tchmod +w \"$p\" &&\n>  \t\tsed \"s/second commit/socond commit/\" \"$p\" >\"$p.munged\" &&\n>  \t\tmv \"$p.munged\" \"$p\" ||\n>  \t\treturn 1\n\nOk, the fix makes sense. Thanks!\n\nPatrick\n"},{"id":"517500","messageId":"xmqqr010e8w8.fsf@gitster.g","threadId":"63404","inReplyTo":"aBroM9wh7Oo0xMU2@pks.im","subject":"Re: [PATCH] t6011: fix misconversion from perl to sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-07T15:29:59Z","receivedAt":"2025-05-07T15:30:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Tue, May 06, 2025 at 03:48:55PM -0700, Junio C Hamano wrote:\n>> No, this is not about a quiz on regexp compatibility between Perl\n>> and sed.\n>> \n>> Back when cdbdc6bf (t: refactor tests depending on Perl substitution\n>> operator, 2025-04-03) rewrite many use of perl with sed, the general\n>\n> s/rewrite many use/rewrote many uses/\n>\n>> pattern of the original scripts were\n>> \n>>     chmod +w some_read_only_file &&\n>>     perl -p -e \"regexp to munge\" some_read_only_file >some_tmp &&\n>>     mv some_tmp some_read_only_file\n>> \n>> persumably because the author new replacing some_read_only_file with\n>\n> s/new/knew?\n>\n>> diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh\n>> index b6f3344dbf..1dd1e50d21 100755\n>> --- a/t/t6011-rev-list-with-bad-commit.sh\n>> +++ b/t/t6011-rev-list-with-bad-commit.sh\n>> @@ -38,6 +38,7 @@ test_expect_success 'verify number of revisions' \\\n>>  test_expect_success 'corrupt second commit object' '\n>>  \tfor p in .git/objects/pack/*.pack\n>>  \tdo\n>> +\t\tchmod +w \"$p\" &&\n>>  \t\tsed \"s/second commit/socond commit/\" \"$p\" >\"$p.munged\" &&\n>>  \t\tmv \"$p.munged\" \"$p\" ||\n>>  \t\treturn 1\n>\n> Ok, the fix makes sense. Thanks!\n>\n> Patrick\n\nThanks for typofixes and sanity checking.\n\n\n"}]}