{"thread":{"id":"63630","subject":"Solaris sed","startedAt":"2025-06-12T03:23:41Z","lastAt":"2025-06-13T20:31:00Z","messageCount":15,"participants":["Brad Smith","Collin Funk","Junio C Hamano","Eli Schwartz","Eric Sunshine","Paul Smith","Jean-Noël AVILA"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"520149","messageId":"09f954b8-d9c3-418f-ad4b-9cb9b063f4ae@comstyle.com","threadId":"63630","inReplyTo":null,"subject":"Solaris sed","fromName":"Brad Smith","fromEmail":"brad@comstyle.com","sentAt":"2025-06-12T03:23:38Z","receivedAt":"2025-06-12T03:23:41Z","isPatch":false,"sender":{"key":"brad@comstyle.com","avatar":"https://avatars.githubusercontent.com/u/1129902?v=4"},"body":"Building on Solaris I noticed the following two issues with Solaris sed.\n\n     GEN version-def.h\nsed: Missing newline at end of file standard input.\n\n     GEN config-list.h\nsed: illegal option -- E\nUsage:  sed [-n] script [file...]\n         sed [-n] [-e script]...[-f script_file]...[file...]\n\n\nhttps://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019\n\nThe second issue being introduced fairly recently. Not sure what would be\nappropriate fixes. Just pointing them out if someone has an suggestions for\nfixes.\n\n"},{"id":"520151","messageId":"87bjqteicd.fsf@gmail.com","threadId":"63630","inReplyTo":"09f954b8-d9c3-418f-ad4b-9cb9b063f4ae@comstyle.com","subject":"Re: Solaris sed","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-12T03:42:26Z","receivedAt":"2025-06-12T03:42:28Z","isPatch":false,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Hi Brad,\n\nBrad Smith <brad@comstyle.com> writes:\n\n> Building on Solaris I noticed the following two issues with Solaris sed.\n>\n>     GEN version-def.h\n> sed: Missing newline at end of file standard input.\n>\n>     GEN config-list.h\n> sed: illegal option -- E\n> Usage:  sed [-n] script [file...]\n>         sed [-n] [-e script]...[-f script_file]...[file...]\n>\n>\n> https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019\n>\n> The second issue being introduced fairly recently. Not sure what would be\n> appropriate fixes. Just pointing them out if someone has an suggestions for\n> fixes.\n\nI noticed these as well, but just ignored them since it seems to build\nfine.\n\nThe first one seems like just a warning? Probably something to do with\nPOSIX defining a \"Text File\" as \"A file that contains characters\norganized into zero or more lines\" where a line is \"A sequence of zero\nor more non- <newline> characters plus a terminating <newline>\ncharacter.\"\n\nThe second is more tricky. The '-E' option to use EREs was not added to\nthe specification for 'sed' until POSIX.1-2024 [1]. Maybe the script\ncould check for the 'gsed' command? All of the (few) Solaris machines I\nuse will have many GNU programs installed like that.\n\nCollin\n\n[1] https://pubs.opengroup.org/onlinepubs/9799919799/utilities/sed.html\n"},{"id":"520154","messageId":"f2082cde-7eb9-4927-a01c-e6fb3b355d13@comstyle.com","threadId":"63630","inReplyTo":"87bjqteicd.fsf@gmail.com","subject":"Re: Solaris sed","fromName":"Brad Smith","fromEmail":"brad@comstyle.com","sentAt":"2025-06-12T03:49:30Z","receivedAt":"2025-06-12T03:49:32Z","isPatch":false,"sender":{"key":"brad@comstyle.com","avatar":"https://avatars.githubusercontent.com/u/1129902?v=4"},"body":"On 2025-06-11 11:42 p.m., Collin Funk wrote:\n> Hi Brad,\n>\n> Brad Smith <brad@comstyle.com> writes:\n>\n>> Building on Solaris I noticed the following two issues with Solaris sed.\n>>\n>>      GEN version-def.h\n>> sed: Missing newline at end of file standard input.\n>>\n>>      GEN config-list.h\n>> sed: illegal option -- E\n>> Usage:  sed [-n] script [file...]\n>>          sed [-n] [-e script]...[-f script_file]...[file...]\n>>\n>>\n>> https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019\n>>\n>> The second issue being introduced fairly recently. Not sure what would be\n>> appropriate fixes. Just pointing them out if someone has an suggestions for\n>> fixes.\n> I noticed these as well, but just ignored them since it seems to build\n> fine.\n>\n> The first one seems like just a warning? Probably something to do with\n> POSIX defining a \"Text File\" as \"A file that contains characters\n> organized into zero or more lines\" where a line is \"A sequence of zero\n> or more non- <newline> characters plus a terminating <newline>\n> character.\"\nIt looks as if it is just a warning to me. I wasn't worrying about that \none as much\nas I was the second issue.\n> The second is more tricky. The '-E' option to use EREs was not added to\n> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script\n> could check for the 'gsed' command? All of the (few) Solaris machines I\n> use will have many GNU programs installed like that.\nI can't comment on that especially as the build bits support pretty old \nreleases and\nI have no idea how long Sun / Oracle have been shipping GNU bits like \nthis. I do not\nbelieve this has always been a thing.\n> Collin\n>\n> [1] https://pubs.opengroup.org/onlinepubs/9799919799/utilities/sed.html\n>\n"},{"id":"520156","messageId":"xmqqo6utfvxu.fsf@gitster.g","threadId":"63630","inReplyTo":"09f954b8-d9c3-418f-ad4b-9cb9b063f4ae@comstyle.com","subject":"Re: Solaris sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T04:03:25Z","receivedAt":"2025-06-12T04:03:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brad Smith <brad@comstyle.com> writes:\n\n> Building on Solaris I noticed the following two issues with Solaris sed.\n>\n>     GEN version-def.h\n> sed: Missing newline at end of file standard input.\n\nPerhaps it is this input line it is complaining about.  sed works on\ntext files, and a file that ends in incomplete line was not quite\ntext.\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>     GEN config-list.h\n> sed: illegal option -- E\n> Usage:  sed [-n] script [file...]\n>         sed [-n] [-e script]...[-f script_file]...[file...]\n\nThis is a bit trickier but should be doable.  It does not like the\n-E option to use ERE (as opposed to BRE) for pattern matching used\nin generate-configlist.sh script.\n\n\tsed -E '\n\t/^`?[a-zA-Z].*\\..*`?::$/ {\n\t/deprecated/d;\n\ts/::$//;\n\ts/`//g;\n\ts/^.*$/\t\"&\",/;\n\tp;};\n\td'\n\nI think the only problematic one is the first address, whose BRE\nequivalent I think is\n\n\t/^`\\{0,1\\}[a-zA-Z].*\\..*`\\{0,1\\}::$/\n\nIn practice, I suspect \\{0,1\\} is unnecessarily strict and using\nsomething looser like\n\n\t/^`*[a-zA-Z].*\\..*`*::$/\n\nmay be sufficient.  Replace the address expression associated with\nthe {editing command} and drop \"-E\", and use \"-e\" for readability,\nperhaps?\n\nTotally untested patch follows.\n\n GIT-VERSION-GEN        | 2 +-\n generate-configlist.sh | 8 ++++----\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN\nindex 208e91a17f..de989657fb 100755\n--- c/GIT-VERSION-GEN\n+++ w/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 c/generate-configlist.sh w/generate-configlist.sh\nindex 9d2ad6165d..75c39ade20 100755\n--- c/generate-configlist.sh\n+++ w/generate-configlist.sh\n@@ -13,16 +13,16 @@ print_config_list () {\n \tcat <<EOF\n static const char *config_name_list[] = {\n EOF\n-\tsed -E '\n-/^`?[a-zA-Z].*\\..*`?::$/ {\n+\tsed -e '\n+\t/^`*[a-zA-Z].*\\..*`*::$/ {\n \t/deprecated/d;\n \ts/::$//;\n \ts/`//g;\n \ts/^.*$/\t\"&\",/;\n \tp;};\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 \tsort\n \tcat <<EOF\n \tNULL,\n"},{"id":"520157","messageId":"caaa5d54-d32d-40b3-9bf3-0f322e7c4316@comstyle.com","threadId":"63630","inReplyTo":"xmqqo6utfvxu.fsf@gitster.g","subject":"Re: Solaris sed","fromName":"Brad Smith","fromEmail":"brad@comstyle.com","sentAt":"2025-06-12T04:13:25Z","receivedAt":"2025-06-12T04:13:28Z","isPatch":false,"sender":{"key":"brad@comstyle.com","avatar":"https://avatars.githubusercontent.com/u/1129902?v=4"},"body":"On 2025-06-12 12:03 a.m., Junio C Hamano wrote:\n> Brad Smith <brad@comstyle.com> writes:\n>\n>> Building on Solaris I noticed the following two issues with Solaris sed.\n>>\n>>      GEN version-def.h\n>> sed: Missing newline at end of file standard input.\n> Perhaps it is this input line it is complaining about.  sed works on\n> text files, and a file that ends in incomplete line was not quite\n> text.\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>>      GEN config-list.h\n>> sed: illegal option -- E\n>> Usage:  sed [-n] script [file...]\n>>          sed [-n] [-e script]...[-f script_file]...[file...]\n> This is a bit trickier but should be doable.  It does not like the\n> -E option to use ERE (as opposed to BRE) for pattern matching used\n> in generate-configlist.sh script.\n>\n> \tsed -E '\n> \t/^`?[a-zA-Z].*\\..*`?::$/ {\n> \t/deprecated/d;\n> \ts/::$//;\n> \ts/`//g;\n> \ts/^.*$/\t\"&\",/;\n> \tp;};\n> \td'\n>\n> I think the only problematic one is the first address, whose BRE\n> equivalent I think is\n>\n> \t/^`\\{0,1\\}[a-zA-Z].*\\..*`\\{0,1\\}::$/\n>\n> In practice, I suspect \\{0,1\\} is unnecessarily strict and using\n> something looser like\n>\n> \t/^`*[a-zA-Z].*\\..*`*::$/\n>\n> may be sufficient.  Replace the address expression associated with\n> the {editing command} and drop \"-E\", and use \"-e\" for readability,\n> perhaps?\n>\n> Totally untested patch follows.\n>\n>   GIT-VERSION-GEN        | 2 +-\n>   generate-configlist.sh | 8 ++++----\n>   2 files changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN\n> index 208e91a17f..de989657fb 100755\n> --- c/GIT-VERSION-GEN\n> +++ w/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> diff --git c/generate-configlist.sh w/generate-configlist.sh\n> index 9d2ad6165d..75c39ade20 100755\n> --- c/generate-configlist.sh\n> +++ w/generate-configlist.sh\n> @@ -13,16 +13,16 @@ print_config_list () {\n>   \tcat <<EOF\n>   static const char *config_name_list[] = {\n>   EOF\n> -\tsed -E '\n> -/^`?[a-zA-Z].*\\..*`?::$/ {\n> +\tsed -e '\n> +\t/^`*[a-zA-Z].*\\..*`*::$/ {\n>   \t/deprecated/d;\n>   \ts/::$//;\n>   \ts/`//g;\n>   \ts/^.*$/\t\"&\",/;\n>   \tp;};\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>   \tsort\n>   \tcat <<EOF\n>   \tNULL,\n\n\nNo errors or warnings after this is applied.\n\n"},{"id":"520158","messageId":"ed3d9c32-5de8-4653-be75-d2b5c89340e0@gentoo.org","threadId":"63630","inReplyTo":"f2082cde-7eb9-4927-a01c-e6fb3b355d13@comstyle.com","subject":"Re: Solaris sed","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-06-12T04:16:23Z","receivedAt":"2025-06-12T04:16:27Z","isPatch":false,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 6/11/25 11:49 PM, Brad Smith wrote:\n> On 2025-06-11 11:42 p.m., Collin Funk wrote:\n>> Hi Brad,\n>>\n>> Brad Smith <brad@comstyle.com> writes:\n>>\n>>> Building on Solaris I noticed the following two issues with Solaris sed.\n>>>\n>>>      GEN version-def.h\n>>> sed: Missing newline at end of file standard input.\n>>>\n>>>      GEN config-list.h\n>>> sed: illegal option -- E\n>>> Usage:  sed [-n] script [file...]\n>>>          sed [-n] [-e script]...[-f script_file]...[file...]\n>>>\n>>>\n>>> https://github.com/git/git/commit/\n>>> e1b81f54da80267edee2cb8fd0d0f75f03023019\n>>>\n>>> The second issue being introduced fairly recently. Not sure what\n>>> would be\n>>> appropriate fixes. Just pointing them out if someone has an\n>>> suggestions for\n>>> fixes.\n>> I noticed these as well, but just ignored them since it seems to build\n>> fine.\n>>\n>> The first one seems like just a warning? Probably something to do with\n>> POSIX defining a \"Text File\" as \"A file that contains characters\n>> organized into zero or more lines\" where a line is \"A sequence of zero\n>> or more non- <newline> characters plus a terminating <newline>\n>> character.\"\n> It looks as if it is just a warning to me. I wasn't worrying about that\n> one as much\n> as I was the second issue.\n>> The second is more tricky. The '-E' option to use EREs was not added to\n>> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script\n>> could check for the 'gsed' command? All of the (few) Solaris machines I\n>> use will have many GNU programs installed like that.\n> I can't comment on that especially as the build bits support pretty old\n> releases and\n> I have no idea how long Sun / Oracle have been shipping GNU bits like\n> this. I do not\n> believe this has always been a thing.\n\n\nThe Solaris box I have a shell on, has gsed installed as a purely\noptional third-party addon from a third-party package feed. As far as I\nknow, Solaris never did nor plans to ship \"GNU bits like this\".\n\nOf course, the Git project *could* declare users must first build GNU\nsed, then build Git. Or only build on boxes where the admin is a GNU\nenthusiast. But that option seems unlikely and unattractive...\n\n\n-- \nEli Schwartz\n"},{"id":"520159","messageId":"874iwlegmg.fsf@gmail.com","threadId":"63630","inReplyTo":"caaa5d54-d32d-40b3-9bf3-0f322e7c4316@comstyle.com","subject":"Re: Solaris sed","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-12T04:19:35Z","receivedAt":"2025-06-12T04:19:37Z","isPatch":false,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Brad Smith <brad@comstyle.com> writes:\n\n>> Totally untested patch follows.\n>>\n>>   GIT-VERSION-GEN        | 2 +-\n>>   generate-configlist.sh | 8 ++++----\n>>   2 files changed, 5 insertions(+), 5 deletions(-)\n>>\n>> diff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN\n>> index 208e91a17f..de989657fb 100755\n>> --- c/GIT-VERSION-GEN\n>> +++ w/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>>   -REPLACED=$(printf \"%s\" \"$INPUT\" | sed -e\n>> \"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>> diff --git c/generate-configlist.sh w/generate-configlist.sh\n>> index 9d2ad6165d..75c39ade20 100755\n>> --- c/generate-configlist.sh\n>> +++ w/generate-configlist.sh\n>> @@ -13,16 +13,16 @@ print_config_list () {\n>>   \tcat <<EOF\n>>   static const char *config_name_list[] = {\n>>   EOF\n>> -\tsed -E '\n>> -/^`?[a-zA-Z].*\\..*`?::$/ {\n>> +\tsed -e '\n>> +\t/^`*[a-zA-Z].*\\..*`*::$/ {\n>>   \t/deprecated/d;\n>>   \ts/::$//;\n>>   \ts/`//g;\n>>   \ts/^.*$/\t\"&\",/;\n>>   \tp;};\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>>   \tsort\n>>   \tcat <<EOF\n>>   \tNULL,\n>\n>\n> No errors or warnings after this is applied.\n\nLikewise.\n\nI checked on my Linux machine and both files are the same before and\nafter the patch. Before the patch on Solaris 10, the following is\ngenerated:\n\n    /* Automatically generated by generate-configlist.sh */\n    \n    \n    static const char *config_name_list[] = {\n            NULL,\n    };\n\nAfter the patch the output on Solaris is the same as on Linux.\n\nSo the patch is perfect.\n\nReviewed-by: Collin Funk <collin.funk1@gmail.com>\n\nCollin\n"},{"id":"520160","messageId":"87v7p1d1rf.fsf@gmail.com","threadId":"63630","inReplyTo":"ed3d9c32-5de8-4653-be75-d2b5c89340e0@gentoo.org","subject":"Re: Solaris sed","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-12T04:25:56Z","receivedAt":"2025-06-12T04:25:58Z","isPatch":false,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Eli Schwartz <eschwartz@gentoo.org> writes:\n\n>>> The second is more tricky. The '-E' option to use EREs was not added to\n>>> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script\n>>> could check for the 'gsed' command? All of the (few) Solaris machines I\n>>> use will have many GNU programs installed like that.\n>> I can't comment on that especially as the build bits support pretty old\n>> releases and\n>> I have no idea how long Sun / Oracle have been shipping GNU bits like\n>> this. I do not\n>> believe this has always been a thing.\n>\n>\n> The Solaris box I have a shell on, has gsed installed as a purely\n> optional third-party addon from a third-party package feed. As far as I\n> know, Solaris never did nor plans to ship \"GNU bits like this\".\n\nYes, sorry for not being clear. It is not installed by default. On the\ncompile farm machines I have access to it is always installed by the\nmaintainer. Or on VMs I use, I always download it. I figured that is\npretty common.\n\n> Of course, the Git project *could* declare users must first build GNU\n> sed, then build Git. Or only build on boxes where the admin is a GNU\n> enthusiast. But that option seems unlikely and unattractive...\n\nPerhaps I am too mean to Solaris... Their 'date' command made me give a\nsimilar recommendation before. Anyways, Junio wrote a patch that avoids\nus forcing GNU tools on them.\n\nCollin\n"},{"id":"520161","messageId":"e63d1ef3-6bd9-4720-95ea-16c800f549c1@comstyle.com","threadId":"63630","inReplyTo":"ed3d9c32-5de8-4653-be75-d2b5c89340e0@gentoo.org","subject":"Re: Solaris sed","fromName":"Brad Smith","fromEmail":"brad@comstyle.com","sentAt":"2025-06-12T04:26:07Z","receivedAt":"2025-06-12T04:26:09Z","isPatch":false,"sender":{"key":"brad@comstyle.com","avatar":"https://avatars.githubusercontent.com/u/1129902?v=4"},"body":"On 2025-06-12 12:16 a.m., Eli Schwartz wrote:\n> On 6/11/25 11:49 PM, Brad Smith wrote:\n>> On 2025-06-11 11:42 p.m., Collin Funk wrote:\n>>> Hi Brad,\n>>>\n>>> Brad Smith <brad@comstyle.com> writes:\n>>>\n>>>> Building on Solaris I noticed the following two issues with Solaris sed.\n>>>>\n>>>>       GEN version-def.h\n>>>> sed: Missing newline at end of file standard input.\n>>>>\n>>>>       GEN config-list.h\n>>>> sed: illegal option -- E\n>>>> Usage:  sed [-n] script [file...]\n>>>>           sed [-n] [-e script]...[-f script_file]...[file...]\n>>>>\n>>>>\n>>>> https://github.com/git/git/commit/\n>>>> e1b81f54da80267edee2cb8fd0d0f75f03023019\n>>>>\n>>>> The second issue being introduced fairly recently. Not sure what\n>>>> would be\n>>>> appropriate fixes. Just pointing them out if someone has an\n>>>> suggestions for\n>>>> fixes.\n>>> I noticed these as well, but just ignored them since it seems to build\n>>> fine.\n>>>\n>>> The first one seems like just a warning? Probably something to do with\n>>> POSIX defining a \"Text File\" as \"A file that contains characters\n>>> organized into zero or more lines\" where a line is \"A sequence of zero\n>>> or more non- <newline> characters plus a terminating <newline>\n>>> character.\"\n>> It looks as if it is just a warning to me. I wasn't worrying about that\n>> one as much\n>> as I was the second issue.\n>>> The second is more tricky. The '-E' option to use EREs was not added to\n>>> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script\n>>> could check for the 'gsed' command? All of the (few) Solaris machines I\n>>> use will have many GNU programs installed like that.\n>> I can't comment on that especially as the build bits support pretty old\n>> releases and\n>> I have no idea how long Sun / Oracle have been shipping GNU bits like\n>> this. I do not\n>> believe this has always been a thing.\n>\n> The Solaris box I have a shell on, has gsed installed as a purely\n> optional third-party addon from a third-party package feed. As far as I\n> know, Solaris never did nor plans to ship \"GNU bits like this\".\n>\n> Of course, the Git project *could* declare users must first build GNU\n> sed, then build Git. Or only build on boxes where the admin is a GNU\n> enthusiast. But that option seems unlikely and unattractive...\n\nTo clarify what I meant. Solaris 11 from the looks of it includes GNU \nsed with\nthe base OS. That was not the case with 10 and older.\n\nThe documentation for 11.4 for example mentions both versions of sed.\n\nhttps://docs.oracle.com/cd/E88353_01/html/E37839/sed-1.html\nhttps://docs.oracle.com/cd/E88353_01/html/E37839/sed-1g.html\n"},{"id":"520165","messageId":"CAPig+cROcMt1crKjvqcetFNGdE4ywmD1+NO+q+MnDzctx8ewag@mail.gmail.com","threadId":"63630","inReplyTo":"xmqqo6utfvxu.fsf@gitster.g","subject":"Re: Solaris sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-06-12T05:50:48Z","receivedAt":"2025-06-12T05:51:01Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 12, 2025 at 12:05 AM Junio C Hamano <gitster@pobox.com> wrote:\n> Brad Smith <brad@comstyle.com> writes:\n> > Building on Solaris I noticed the following two issues with Solaris sed.\n> >     GEN version-def.h\n> > sed: Missing newline at end of file standard input.\n>\n> Perhaps it is this input line it is complaining about.  sed works on\n> text files, and a file that ends in incomplete line was not quite\n> text.\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>         -e \"s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|\" \\\n>         -e \"s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|\" \\\n>         -e \"s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|\" \\\n\nIt's curious that this is using:\n\n    printf \"%s\" \"$foo\"`\n\nin the first place. Had it used the simpler:\n\n    echo \"$foo\"\n\nthis sort of problem (forgetting the \"\\n\") would never have occurred.\n\nIn fact, it seems that f6a2efdc9b (GIT-VERSION-GEN: allow running\nwithout input and output files, 2025-01-22), which introduced this\nproblem, also introduced a few similar cases in which the `printf\n\"%s\\n\"` idiom was employed when a simple `echo` would have sufficed.\n"},{"id":"520169","messageId":"b2d23be73902c8433295e2a5f30b051d044e227c.camel@mad-scientist.net","threadId":"63630","inReplyTo":"CAPig+cROcMt1crKjvqcetFNGdE4ywmD1+NO+q+MnDzctx8ewag@mail.gmail.com","subject":"Re: Solaris sed","fromName":"Paul Smith","fromEmail":"paul@mad-scientist.net","sentAt":"2025-06-12T13:35:54Z","receivedAt":"2025-06-12T13:36:03Z","isPatch":false,"sender":{"key":"paul@mad-scientist.net","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Thu, 2025-06-12 at 01:50 -0400, Eric Sunshine wrote:\n> Had it used the simpler:\n> \n>     echo \"$foo\"\n> \n> this sort of problem (forgetting the \"\\n\") would never have occurred.\n\nJust be aware that echo is not well-standardized: many versions of echo\naccept extra options or treat certain chars specially.  So, printf\n(which IS well-standardized) is always safer unless you are 100% sure\nthat the text on the echo command line is simple: cannot start with a\n\"-\", doesn't contain special chars like backslash, etc.\n\nFor portability I (personally) always prefer printf unless I know\nexactly what the text contains (like showing a static string).\n"},{"id":"520174","messageId":"CAPig+cREA6YdMgbZ59eGnU8SRWmfNR8bGGvLfTQEpS4PqKm9mg@mail.gmail.com","threadId":"63630","inReplyTo":"b2d23be73902c8433295e2a5f30b051d044e227c.camel@mad-scientist.net","subject":"Re: Solaris sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-06-12T16:40:29Z","receivedAt":"2025-06-12T16:40:42Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jun 12, 2025 at 9:44 AM Paul Smith <paul@mad-scientist.net> wrote:\n> On Thu, 2025-06-12 at 01:50 -0400, Eric Sunshine wrote:\n> > Had it used the simpler:\n> >\n> >     echo \"$foo\"\n> >\n> > this sort of problem (forgetting the \"\\n\") would never have occurred.\n>\n> Just be aware that echo is not well-standardized: many versions of echo\n> accept extra options or treat certain chars specially.  So, printf\n> (which IS well-standardized) is always safer unless you are 100% sure\n> that the text on the echo command line is simple: cannot start with a\n> \"-\", doesn't contain special chars like backslash, etc.\n>\n> For portability I (personally) always prefer printf unless I know\n> exactly what the text contains (like showing a static string).\n\nYup, you're right. I always do the same when I can't trust the\nargument to be `echo`-safe, but apparently I wasn't thinking of that\ncase when I wrote the email. Thanks for the dose of sanity.\n"},{"id":"520237","messageId":"5895400.DvuYhMxLoT@cayenne","threadId":"63630","inReplyTo":"874iwlegmg.fsf@gmail.com","subject":"Re: Solaris sed","fromName":"Jean-Noël AVILA","fromEmail":"jn.avila@free.fr","sentAt":"2025-06-13T20:13:48Z","receivedAt":"2025-06-13T20:14:06Z","isPatch":false,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"On Thursday, 12 June 2025 06:19:35 CEST Collin Funk wrote:\n> Brad Smith <brad@comstyle.com> writes:\n\n> > No errors or warnings after this is applied.\n> \n> Likewise.\n> \n> I checked on my Linux machine and both files are the same before and\n> after the patch. Before the patch on Solaris 10, the following is\n> generated:\n> \n>     /* Automatically generated by generate-configlist.sh */\n> \n> \n>     static const char *config_name_list[] = {\n>             NULL,\n>     };\n> \n> After the patch the output on Solaris is the same as on Linux.\n> \n> So the patch is perfect.\n> \n> Reviewed-by: Collin Funk <collin.funk1@gmail.com>\n> \n> Collin\n\nHello, \n\nWould it be possible to set up some kind of CI to check for compatibility with \nsuch systems. This is the second time I introduced regressions without even \nknowing it, and it would be really great to catch them before borking a \nrelease process.\n\nThanks,\n\nJN\n\n\n\n"},{"id":"520238","messageId":"CAPig+cSu7+fxveULiB1vDbcy6Cnia_5isVVy+RCO+HGAyr8uvg@mail.gmail.com","threadId":"63630","inReplyTo":"5895400.DvuYhMxLoT@cayenne","subject":"Re: Solaris sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-06-13T20:23:16Z","receivedAt":"2025-06-13T20:23:28Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 13, 2025 at 4:15 PM Jean-Noël AVILA <jn.avila@free.fr> wrote:\n> Would it be possible to set up some kind of CI to check for compatibility with\n> such systems. This is the second time I introduced regressions without even\n> knowing it, and it would be really great to catch them before borking a\n> release process.\n\nHad this been in a test script, it would have been caught by\nt/check-non-portable-shell.sh. We may want to apply the check to\nbuild-related scripts, as well. For instance, it would have caught the\n-E problem:\n\n    % ./t/check-non-portable-shell.pl generate-*.sh\n    generate-configlist.sh:16: error: sed option not portable (use\nonly -n, -e, -f): sed -E '\n    %\n\nYou can, of course, run check-non-portable-shell.pl manually after\nediting a script, but perhaps this check could be enabled by a\n(hopefully) minor tweak to the main Git Makefile?\n"},{"id":"520239","messageId":"87ikkzs7su.fsf@gmail.com","threadId":"63630","inReplyTo":"5895400.DvuYhMxLoT@cayenne","subject":"Re: Solaris sed","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-13T20:30:57Z","receivedAt":"2025-06-13T20:31:00Z","isPatch":false,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Jean-Noël AVILA <jn.avila@free.fr> writes:\n\n> Would it be possible to set up some kind of CI to check for compatibility with \n> such systems. This is the second time I introduced regressions without even \n> knowing it, and it would be really great to catch them before borking a \n> release process.\n\nI'm sure that Solaris packagers are used to patching stuff like this. I\nwouldn't feel guilty about it. It is difficult to remember all these\nportability quirks.\n\nWith GitHub actions you can add Oracle Solaris and OmniOS (based on\nillumos, which was based on OpenSolaris) using vmactions [1]. That might\nhelp catch some stuff.\n\nIn this case, the build still works even with the broken sed commands.\nNot sure if the tests would have caught it though.\n\nCollin\n\n[1] https://github.com/vmactions\n"}]}