{"thread":{"id":"63635","subject":"build: sed portability fixes","startedAt":"2025-06-12T17:05:00Z","lastAt":"2025-06-13T16:07:20Z","messageCount":3,"participants":["Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"520179","messageId":"xmqqplf8evr9.fsf@gitster.g","threadId":"63635","inReplyTo":null,"subject":"build: sed portability fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T17:04:58Z","receivedAt":"2025-06-12T17:05:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Recently generating the version-def.h file and the config-list.h\nfile have been updated, which broke versions of \"sed\" that do not\nwant to be fed a file that ends with an incomplete line, and/or that\ndo not understand the more recent \"-E\" option to use extended\nregular expression.\n\nFix them in response to a build-failure reported on Solaris boxes.\n\ncf. https://lore.kernel.org/git/09f954b8-d9c3-418f-ad4b-9cb9b063f4ae@comstyle.com/\n\nReported-by: Brad Smith \nReviewed-by: Collin Funk \nSigned-off-by: Junio C Hamano \n---\n\n * This time with a proposed log message. I may fast-track it down\n to 'master' before the release. I personally am undecided, and I\n do know that I hate the style of this particular sed script and\n am tempted to fix it before committing, but I'll refrain from\n doing so before the release.\n\n GIT-VERSION-GEN | 2 +-\n generate-configlist.sh | 8 ++++----\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex 1047b8d11d..ad3aa59045 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -82,7 +82,7 @@ read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trail\n $(echo \"$GIT_VERSION\" 0 0 0 0 | tr '.a-zA-Z-' ' ')\n EOF\n \n-REPLACED=$(printf \"%s\" \"$INPUT\" | sed -e \"s|@GIT_VERSION@|$GIT_VERSION|\" \\\n+REPLACED=$(printf \"%s\\n\" \"$INPUT\" | sed -e \"s|@GIT_VERSION@|$GIT_VERSION|\" \\\n \t-e \"s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|\" \\\n \t-e \"s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|\" \\\n \t-e \"s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|\" \\\ndiff --git a/generate-configlist.sh b/generate-configlist.sh\nindex 9d2ad6165d..75c39ade20 100755\n--- a/generate-configlist.sh\n+++ b/generate-configlist.sh\n@@ -13,16 +13,16 @@ print_config_list () {\n \tcat < * This time with a proposed log message. I may fast-track it down\n> to 'master' before the release. I personally am undecided, and I\n> do know that I hate the style of this particular sed script and\n> am tempted to fix it before committing, but I'll refrain from\n> doing so before the release.\n\nThe newline-less input is in v2.49.0 already, but the use of \"sed -E\" is\nnew in the 2.50 cycle. So it probably is worth addressing before the\nrelease. In which case I tried to give the patch a very careful read to\navoid any brown paper bags.\n\n> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> index 1047b8d11d..ad3aa59045 100755\n> --- a/GIT-VERSION-GEN\n> +++ b/GIT-VERSION-GEN\n> @@ -82,7 +82,7 @@ read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trail\n> $(echo \"$GIT_VERSION\" 0 0 0 0 | tr '.a-zA-Z-' ' ')\n> EOF\n> \n> -REPLACED=$(printf \"%s\" \"$INPUT\" | sed -e \"s|@GIT_VERSION@|$GIT_VERSION|\" \\\n> +REPLACED=$(printf \"%s\\n\" \"$INPUT\" | sed -e \"s|@GIT_VERSION@|$GIT_VERSION|\" \\\n> \t-e \"s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|\" \\\n> \t-e \"s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|\" \\\n> \t-e \"s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|\" \\\n\nOK, makes sense since we now stick the content into the INPUT variable.\nThat sometimes comes from a file, but we get it via process substitution\nwith $(cat), so the shell will strip off the trailing newline there. So\nwe can unconditionally add it back here. Goo.d\n\n> diff --git a/generate-configlist.sh b/generate-configlist.sh\n> index 9d2ad6165d..75c39ade20 100755\n> --- a/generate-configlist.sh\n> +++ b/generate-configlist.sh\n> @@ -13,16 +13,16 @@ print_config_list () {\n> \tcat < static const char *config_name_list[] = {\n> EOF\n> -\tsed -E '\n> -/^`?[a-zA-Z].*\\..*`?::$/ {\n> +\tsed -e '\n> +\t/^`*[a-zA-Z].*\\..*`*::$/ {\n\nOK, this is just replacing the use of \"?\" with \"*\". I think it is OK to\nbe loose here, as we are parsing our own config docs. And if somebody\ndid write\n\n ```foo.bar```::\n\nit is probably OK to parse that anyway. ;)\n\n> -d' \\\n> +\td' \\\n> \t \"$SOURCE_DIR\"/Documentation/*config.adoc \\\n> -\t \"$SOURCE_DIR\"/Documentation/config/*.adoc|\n> +\t \"$SOURCE_DIR\"/Documentation/config/*.adoc |\n\nAnd then this (plus the indentation above) is just non-semantic\nwhitespace tidying.\n\nSo the whole thing looks good to me.\n\n-Peff\n"},{"id":"520226","messageId":"xmqqa56baamh.fsf@gitster.g","threadId":"63635","inReplyTo":"20250613105932.GA1995623@coredump.intra.peff.net","subject":"Re: build: sed portability fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-13T16:07:18Z","receivedAt":"2025-06-13T16:07:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Jun 12, 2025 at 10:04:58AM -0700, Junio C Hamano wrote:\n>\n>> * This time with a proposed log message. I may fast-track it down\n>> to 'master' before the release. I personally am undecided, and I\n>> do know that I hate the style of this particular sed script and\n>> am tempted to fix it before committing, but I'll refrain from\n>> doing so before the release.\n>\n> The newline-less input is in v2.49.0 already, but the use of \"sed -E\" is\n> new in the 2.50 cycle. So it probably is worth addressing before the\n> release. In which case I tried to give the patch a very careful read to\n> avoid any brown paper bags.\n\nYeah, I could split them into two patches, but I do not think it is\nworth keeping one half broken for the sake of being bug-to-bug\ncompatible with a previous version in this case.\n\n>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n>> index 1047b8d11d..ad3aa59045 100755\n>> --- a/GIT-VERSION-GEN\n>> +++ b/GIT-VERSION-GEN\n>> @@ -82,7 +82,7 @@ read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trail\n>> $(echo \"$GIT_VERSION\" 0 0 0 0 | tr '.a-zA-Z-' ' ')\n>> EOF\n>> \n>> -REPLACED=$(printf \"%s\" \"$INPUT\" | sed -e \"s|@GIT_VERSION@|$GIT_VERSION|\" \\\n>> +REPLACED=$(printf \"%s\\n\" \"$INPUT\" | sed -e \"s|@GIT_VERSION@|$GIT_VERSION|\" \\\n>> \t-e \"s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|\" \\\n>> \t-e \"s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|\" \\\n>> \t-e \"s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|\" \\\n>\n> OK, makes sense since we now stick the content into the INPUT variable.\n> That sometimes comes from a file, but we get it via process substitution\n> with $(cat), so the shell will strip off the trailing newline there. So\n> we can unconditionally add it back here. Goo.d\n\nYup, the commit that introduced this sprinkled the same 'printf\n\"%s\\n\" used as a better/safer version of echo, feeding a pipe'\npattern and this I think is merely a typo.\n\n>> diff --git a/generate-configlist.sh b/generate-configlist.sh\n>> index 9d2ad6165d..75c39ade20 100755\n>> --- a/generate-configlist.sh\n>> +++ b/generate-configlist.sh\n>> @@ -13,16 +13,16 @@ print_config_list () {\n>> \tcat <> static const char *config_name_list[] = {\n>> EOF\n>> -\tsed -E '\n>> -/^`?[a-zA-Z].*\\..*`?::$/ {\n>> +\tsed -e '\n>> +\t/^`*[a-zA-Z].*\\..*`*::$/ {\n>\n> OK, this is just replacing the use of \"?\" with \"*\". I think it is OK to\n> be loose here, as we are parsing our own config docs.\n\nExactly. Similar reasoning appears in the precursor of this patch,\ni.e. https://lore.kernel.org/git/xmqqo6utfvxu.fsf@gitster.g/\n\n> And if somebody\n> did write\n>\n> ```foo.bar```::\n>\n> it is probably OK to parse that anyway. ;)\n\nprobably ;-).\n\n>> -d' \\\n>> +\td' \\\n>> \t \"$SOURCE_DIR\"/Documentation/*config.adoc \\\n>> -\t \"$SOURCE_DIR\"/Documentation/config/*.adoc|\n>> +\t \"$SOURCE_DIR\"/Documentation/config/*.adoc |\n>\n> And then this (plus the indentation above) is just non-semantic\n> whitespace tidying.\n>\n> So the whole thing looks good to me.\n\nThanks.\n\nWill fast-track.\n"}]}