* Solaris sed
@ 2025-06-12 3:23 Brad Smith
2025-06-12 3:42 ` Collin Funk
2025-06-12 4:03 ` Junio C Hamano
0 siblings, 2 replies; 16+ messages in thread
From: Brad Smith @ 2025-06-12 3:23 UTC (permalink / raw)
To: git
Building on Solaris I noticed the following two issues with Solaris sed.
GEN version-def.h
sed: Missing newline at end of file standard input.
GEN config-list.h
sed: illegal option -- E
Usage: sed [-n] script [file...]
sed [-n] [-e script]...[-f script_file]...[file...]
https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019
The second issue being introduced fairly recently. Not sure what would be
appropriate fixes. Just pointing them out if someone has an suggestions for
fixes.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 3:23 Solaris sed Brad Smith
@ 2025-06-12 3:42 ` Collin Funk
2025-06-12 3:49 ` Brad Smith
2025-06-12 4:03 ` Junio C Hamano
1 sibling, 1 reply; 16+ messages in thread
From: Collin Funk @ 2025-06-12 3:42 UTC (permalink / raw)
To: Brad Smith; +Cc: git
Hi Brad,
Brad Smith <brad@comstyle.com> writes:
> Building on Solaris I noticed the following two issues with Solaris sed.
>
> GEN version-def.h
> sed: Missing newline at end of file standard input.
>
> GEN config-list.h
> sed: illegal option -- E
> Usage: sed [-n] script [file...]
> sed [-n] [-e script]...[-f script_file]...[file...]
>
>
> https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019
>
> The second issue being introduced fairly recently. Not sure what would be
> appropriate fixes. Just pointing them out if someone has an suggestions for
> fixes.
I noticed these as well, but just ignored them since it seems to build
fine.
The first one seems like just a warning? Probably something to do with
POSIX defining a "Text File" as "A file that contains characters
organized into zero or more lines" where a line is "A sequence of zero
or more non- <newline> characters plus a terminating <newline>
character."
The second is more tricky. The '-E' option to use EREs was not added to
the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script
could check for the 'gsed' command? All of the (few) Solaris machines I
use will have many GNU programs installed like that.
Collin
[1] https://pubs.opengroup.org/onlinepubs/9799919799/utilities/sed.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 3:42 ` Collin Funk
@ 2025-06-12 3:49 ` Brad Smith
2025-06-12 4:16 ` Eli Schwartz
0 siblings, 1 reply; 16+ messages in thread
From: Brad Smith @ 2025-06-12 3:49 UTC (permalink / raw)
To: Collin Funk; +Cc: git
On 2025-06-11 11:42 p.m., Collin Funk wrote:
> Hi Brad,
>
> Brad Smith <brad@comstyle.com> writes:
>
>> Building on Solaris I noticed the following two issues with Solaris sed.
>>
>> GEN version-def.h
>> sed: Missing newline at end of file standard input.
>>
>> GEN config-list.h
>> sed: illegal option -- E
>> Usage: sed [-n] script [file...]
>> sed [-n] [-e script]...[-f script_file]...[file...]
>>
>>
>> https://github.com/git/git/commit/e1b81f54da80267edee2cb8fd0d0f75f03023019
>>
>> The second issue being introduced fairly recently. Not sure what would be
>> appropriate fixes. Just pointing them out if someone has an suggestions for
>> fixes.
> I noticed these as well, but just ignored them since it seems to build
> fine.
>
> The first one seems like just a warning? Probably something to do with
> POSIX defining a "Text File" as "A file that contains characters
> organized into zero or more lines" where a line is "A sequence of zero
> or more non- <newline> characters plus a terminating <newline>
> character."
It looks as if it is just a warning to me. I wasn't worrying about that
one as much
as I was the second issue.
> The second is more tricky. The '-E' option to use EREs was not added to
> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script
> could check for the 'gsed' command? All of the (few) Solaris machines I
> use will have many GNU programs installed like that.
I can't comment on that especially as the build bits support pretty old
releases and
I have no idea how long Sun / Oracle have been shipping GNU bits like
this. I do not
believe this has always been a thing.
> Collin
>
> [1] https://pubs.opengroup.org/onlinepubs/9799919799/utilities/sed.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 3:49 ` Brad Smith
@ 2025-06-12 4:16 ` Eli Schwartz
2025-06-12 4:25 ` Collin Funk
2025-06-12 4:26 ` Brad Smith
0 siblings, 2 replies; 16+ messages in thread
From: Eli Schwartz @ 2025-06-12 4:16 UTC (permalink / raw)
To: Brad Smith, Collin Funk; +Cc: git
[-- Attachment #1.1: Type: text/plain, Size: 2243 bytes --]
On 6/11/25 11:49 PM, Brad Smith wrote:
> On 2025-06-11 11:42 p.m., Collin Funk wrote:
>> Hi Brad,
>>
>> Brad Smith <brad@comstyle.com> writes:
>>
>>> Building on Solaris I noticed the following two issues with Solaris sed.
>>>
>>> GEN version-def.h
>>> sed: Missing newline at end of file standard input.
>>>
>>> GEN config-list.h
>>> sed: illegal option -- E
>>> Usage: sed [-n] script [file...]
>>> sed [-n] [-e script]...[-f script_file]...[file...]
>>>
>>>
>>> https://github.com/git/git/commit/
>>> e1b81f54da80267edee2cb8fd0d0f75f03023019
>>>
>>> The second issue being introduced fairly recently. Not sure what
>>> would be
>>> appropriate fixes. Just pointing them out if someone has an
>>> suggestions for
>>> fixes.
>> I noticed these as well, but just ignored them since it seems to build
>> fine.
>>
>> The first one seems like just a warning? Probably something to do with
>> POSIX defining a "Text File" as "A file that contains characters
>> organized into zero or more lines" where a line is "A sequence of zero
>> or more non- <newline> characters plus a terminating <newline>
>> character."
> It looks as if it is just a warning to me. I wasn't worrying about that
> one as much
> as I was the second issue.
>> The second is more tricky. The '-E' option to use EREs was not added to
>> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script
>> could check for the 'gsed' command? All of the (few) Solaris machines I
>> use will have many GNU programs installed like that.
> I can't comment on that especially as the build bits support pretty old
> releases and
> I have no idea how long Sun / Oracle have been shipping GNU bits like
> this. I do not
> believe this has always been a thing.
The Solaris box I have a shell on, has gsed installed as a purely
optional third-party addon from a third-party package feed. As far as I
know, Solaris never did nor plans to ship "GNU bits like this".
Of course, the Git project *could* declare users must first build GNU
sed, then build Git. Or only build on boxes where the admin is a GNU
enthusiast. But that option seems unlikely and unattractive...
--
Eli Schwartz
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 4:16 ` Eli Schwartz
@ 2025-06-12 4:25 ` Collin Funk
2025-06-12 4:26 ` Brad Smith
1 sibling, 0 replies; 16+ messages in thread
From: Collin Funk @ 2025-06-12 4:25 UTC (permalink / raw)
To: Eli Schwartz; +Cc: Brad Smith, git
Eli Schwartz <eschwartz@gentoo.org> writes:
>>> The second is more tricky. The '-E' option to use EREs was not added to
>>> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script
>>> could check for the 'gsed' command? All of the (few) Solaris machines I
>>> use will have many GNU programs installed like that.
>> I can't comment on that especially as the build bits support pretty old
>> releases and
>> I have no idea how long Sun / Oracle have been shipping GNU bits like
>> this. I do not
>> believe this has always been a thing.
>
>
> The Solaris box I have a shell on, has gsed installed as a purely
> optional third-party addon from a third-party package feed. As far as I
> know, Solaris never did nor plans to ship "GNU bits like this".
Yes, sorry for not being clear. It is not installed by default. On the
compile farm machines I have access to it is always installed by the
maintainer. Or on VMs I use, I always download it. I figured that is
pretty common.
> Of course, the Git project *could* declare users must first build GNU
> sed, then build Git. Or only build on boxes where the admin is a GNU
> enthusiast. But that option seems unlikely and unattractive...
Perhaps I am too mean to Solaris... Their 'date' command made me give a
similar recommendation before. Anyways, Junio wrote a patch that avoids
us forcing GNU tools on them.
Collin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 4:16 ` Eli Schwartz
2025-06-12 4:25 ` Collin Funk
@ 2025-06-12 4:26 ` Brad Smith
1 sibling, 0 replies; 16+ messages in thread
From: Brad Smith @ 2025-06-12 4:26 UTC (permalink / raw)
To: Eli Schwartz, Collin Funk; +Cc: git
On 2025-06-12 12:16 a.m., Eli Schwartz wrote:
> On 6/11/25 11:49 PM, Brad Smith wrote:
>> On 2025-06-11 11:42 p.m., Collin Funk wrote:
>>> Hi Brad,
>>>
>>> Brad Smith <brad@comstyle.com> writes:
>>>
>>>> Building on Solaris I noticed the following two issues with Solaris sed.
>>>>
>>>> GEN version-def.h
>>>> sed: Missing newline at end of file standard input.
>>>>
>>>> GEN config-list.h
>>>> sed: illegal option -- E
>>>> Usage: sed [-n] script [file...]
>>>> sed [-n] [-e script]...[-f script_file]...[file...]
>>>>
>>>>
>>>> https://github.com/git/git/commit/
>>>> e1b81f54da80267edee2cb8fd0d0f75f03023019
>>>>
>>>> The second issue being introduced fairly recently. Not sure what
>>>> would be
>>>> appropriate fixes. Just pointing them out if someone has an
>>>> suggestions for
>>>> fixes.
>>> I noticed these as well, but just ignored them since it seems to build
>>> fine.
>>>
>>> The first one seems like just a warning? Probably something to do with
>>> POSIX defining a "Text File" as "A file that contains characters
>>> organized into zero or more lines" where a line is "A sequence of zero
>>> or more non- <newline> characters plus a terminating <newline>
>>> character."
>> It looks as if it is just a warning to me. I wasn't worrying about that
>> one as much
>> as I was the second issue.
>>> The second is more tricky. The '-E' option to use EREs was not added to
>>> the specification for 'sed' until POSIX.1-2024 [1]. Maybe the script
>>> could check for the 'gsed' command? All of the (few) Solaris machines I
>>> use will have many GNU programs installed like that.
>> I can't comment on that especially as the build bits support pretty old
>> releases and
>> I have no idea how long Sun / Oracle have been shipping GNU bits like
>> this. I do not
>> believe this has always been a thing.
>
> The Solaris box I have a shell on, has gsed installed as a purely
> optional third-party addon from a third-party package feed. As far as I
> know, Solaris never did nor plans to ship "GNU bits like this".
>
> Of course, the Git project *could* declare users must first build GNU
> sed, then build Git. Or only build on boxes where the admin is a GNU
> enthusiast. But that option seems unlikely and unattractive...
To clarify what I meant. Solaris 11 from the looks of it includes GNU
sed with
the base OS. That was not the case with 10 and older.
The documentation for 11.4 for example mentions both versions of sed.
https://docs.oracle.com/cd/E88353_01/html/E37839/sed-1.html
https://docs.oracle.com/cd/E88353_01/html/E37839/sed-1g.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 3:23 Solaris sed Brad Smith
2025-06-12 3:42 ` Collin Funk
@ 2025-06-12 4:03 ` Junio C Hamano
2025-06-12 4:13 ` Brad Smith
2025-06-12 5:50 ` Eric Sunshine
1 sibling, 2 replies; 16+ messages in thread
From: Junio C Hamano @ 2025-06-12 4:03 UTC (permalink / raw)
To: Brad Smith; +Cc: git
Brad Smith <brad@comstyle.com> writes:
> Building on Solaris I noticed the following two issues with Solaris sed.
>
> GEN version-def.h
> sed: Missing newline at end of file standard input.
Perhaps it is this input line it is complaining about. sed works on
text files, and a file that ends in incomplete line was not quite
text.
-REPLACED=$(printf "%s" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
+REPLACED=$(printf "%s\n" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
-e "s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|" \
-e "s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|" \
-e "s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|" \
> GEN config-list.h
> sed: illegal option -- E
> Usage: sed [-n] script [file...]
> sed [-n] [-e script]...[-f script_file]...[file...]
This is a bit trickier but should be doable. It does not like the
-E option to use ERE (as opposed to BRE) for pattern matching used
in generate-configlist.sh script.
sed -E '
/^`?[a-zA-Z].*\..*`?::$/ {
/deprecated/d;
s/::$//;
s/`//g;
s/^.*$/ "&",/;
p;};
d'
I think the only problematic one is the first address, whose BRE
equivalent I think is
/^`\{0,1\}[a-zA-Z].*\..*`\{0,1\}::$/
In practice, I suspect \{0,1\} is unnecessarily strict and using
something looser like
/^`*[a-zA-Z].*\..*`*::$/
may be sufficient. Replace the address expression associated with
the {editing command} and drop "-E", and use "-e" for readability,
perhaps?
Totally untested patch follows.
GIT-VERSION-GEN | 2 +-
generate-configlist.sh | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN
index 208e91a17f..de989657fb 100755
--- c/GIT-VERSION-GEN
+++ w/GIT-VERSION-GEN
@@ -82,7 +82,7 @@ read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trail
$(echo "$GIT_VERSION" 0 0 0 0 | tr '.a-zA-Z-' ' ')
EOF
-REPLACED=$(printf "%s" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
+REPLACED=$(printf "%s\n" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
-e "s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|" \
-e "s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|" \
-e "s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|" \
diff --git c/generate-configlist.sh w/generate-configlist.sh
index 9d2ad6165d..75c39ade20 100755
--- c/generate-configlist.sh
+++ w/generate-configlist.sh
@@ -13,16 +13,16 @@ print_config_list () {
cat <<EOF
static const char *config_name_list[] = {
EOF
- sed -E '
-/^`?[a-zA-Z].*\..*`?::$/ {
+ sed -e '
+ /^`*[a-zA-Z].*\..*`*::$/ {
/deprecated/d;
s/::$//;
s/`//g;
s/^.*$/ "&",/;
p;};
-d' \
+ d' \
"$SOURCE_DIR"/Documentation/*config.adoc \
- "$SOURCE_DIR"/Documentation/config/*.adoc|
+ "$SOURCE_DIR"/Documentation/config/*.adoc |
sort
cat <<EOF
NULL,
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: Solaris sed
2025-06-12 4:03 ` Junio C Hamano
@ 2025-06-12 4:13 ` Brad Smith
2025-06-12 4:19 ` Collin Funk
2025-06-12 5:50 ` Eric Sunshine
1 sibling, 1 reply; 16+ messages in thread
From: Brad Smith @ 2025-06-12 4:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On 2025-06-12 12:03 a.m., Junio C Hamano wrote:
> Brad Smith <brad@comstyle.com> writes:
>
>> Building on Solaris I noticed the following two issues with Solaris sed.
>>
>> GEN version-def.h
>> sed: Missing newline at end of file standard input.
> Perhaps it is this input line it is complaining about. sed works on
> text files, and a file that ends in incomplete line was not quite
> text.
>
> -REPLACED=$(printf "%s" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
> +REPLACED=$(printf "%s\n" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
> -e "s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|" \
> -e "s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|" \
> -e "s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|" \
>
>> GEN config-list.h
>> sed: illegal option -- E
>> Usage: sed [-n] script [file...]
>> sed [-n] [-e script]...[-f script_file]...[file...]
> This is a bit trickier but should be doable. It does not like the
> -E option to use ERE (as opposed to BRE) for pattern matching used
> in generate-configlist.sh script.
>
> sed -E '
> /^`?[a-zA-Z].*\..*`?::$/ {
> /deprecated/d;
> s/::$//;
> s/`//g;
> s/^.*$/ "&",/;
> p;};
> d'
>
> I think the only problematic one is the first address, whose BRE
> equivalent I think is
>
> /^`\{0,1\}[a-zA-Z].*\..*`\{0,1\}::$/
>
> In practice, I suspect \{0,1\} is unnecessarily strict and using
> something looser like
>
> /^`*[a-zA-Z].*\..*`*::$/
>
> may be sufficient. Replace the address expression associated with
> the {editing command} and drop "-E", and use "-e" for readability,
> perhaps?
>
> Totally untested patch follows.
>
> GIT-VERSION-GEN | 2 +-
> generate-configlist.sh | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN
> index 208e91a17f..de989657fb 100755
> --- c/GIT-VERSION-GEN
> +++ w/GIT-VERSION-GEN
> @@ -82,7 +82,7 @@ read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trail
> $(echo "$GIT_VERSION" 0 0 0 0 | tr '.a-zA-Z-' ' ')
> EOF
>
> -REPLACED=$(printf "%s" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
> +REPLACED=$(printf "%s\n" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
> -e "s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|" \
> -e "s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|" \
> -e "s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|" \
> diff --git c/generate-configlist.sh w/generate-configlist.sh
> index 9d2ad6165d..75c39ade20 100755
> --- c/generate-configlist.sh
> +++ w/generate-configlist.sh
> @@ -13,16 +13,16 @@ print_config_list () {
> cat <<EOF
> static const char *config_name_list[] = {
> EOF
> - sed -E '
> -/^`?[a-zA-Z].*\..*`?::$/ {
> + sed -e '
> + /^`*[a-zA-Z].*\..*`*::$/ {
> /deprecated/d;
> s/::$//;
> s/`//g;
> s/^.*$/ "&",/;
> p;};
> -d' \
> + d' \
> "$SOURCE_DIR"/Documentation/*config.adoc \
> - "$SOURCE_DIR"/Documentation/config/*.adoc|
> + "$SOURCE_DIR"/Documentation/config/*.adoc |
> sort
> cat <<EOF
> NULL,
No errors or warnings after this is applied.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: Solaris sed
2025-06-12 4:13 ` Brad Smith
@ 2025-06-12 4:19 ` Collin Funk
2025-06-13 20:13 ` Jean-Noël AVILA
0 siblings, 1 reply; 16+ messages in thread
From: Collin Funk @ 2025-06-12 4:19 UTC (permalink / raw)
To: Brad Smith; +Cc: Junio C Hamano, git
Brad Smith <brad@comstyle.com> writes:
>> Totally untested patch follows.
>>
>> GIT-VERSION-GEN | 2 +-
>> generate-configlist.sh | 8 ++++----
>> 2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git c/GIT-VERSION-GEN w/GIT-VERSION-GEN
>> index 208e91a17f..de989657fb 100755
>> --- c/GIT-VERSION-GEN
>> +++ w/GIT-VERSION-GEN
>> @@ -82,7 +82,7 @@ read GIT_MAJOR_VERSION GIT_MINOR_VERSION GIT_MICRO_VERSION GIT_PATCH_LEVEL trail
>> $(echo "$GIT_VERSION" 0 0 0 0 | tr '.a-zA-Z-' ' ')
>> EOF
>> -REPLACED=$(printf "%s" "$INPUT" | sed -e
>> "s|@GIT_VERSION@|$GIT_VERSION|" \
>> +REPLACED=$(printf "%s\n" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
>> -e "s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|" \
>> -e "s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|" \
>> -e "s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|" \
>> diff --git c/generate-configlist.sh w/generate-configlist.sh
>> index 9d2ad6165d..75c39ade20 100755
>> --- c/generate-configlist.sh
>> +++ w/generate-configlist.sh
>> @@ -13,16 +13,16 @@ print_config_list () {
>> cat <<EOF
>> static const char *config_name_list[] = {
>> EOF
>> - sed -E '
>> -/^`?[a-zA-Z].*\..*`?::$/ {
>> + sed -e '
>> + /^`*[a-zA-Z].*\..*`*::$/ {
>> /deprecated/d;
>> s/::$//;
>> s/`//g;
>> s/^.*$/ "&",/;
>> p;};
>> -d' \
>> + d' \
>> "$SOURCE_DIR"/Documentation/*config.adoc \
>> - "$SOURCE_DIR"/Documentation/config/*.adoc|
>> + "$SOURCE_DIR"/Documentation/config/*.adoc |
>> sort
>> cat <<EOF
>> NULL,
>
>
> No errors or warnings after this is applied.
Likewise.
I checked on my Linux machine and both files are the same before and
after the patch. Before the patch on Solaris 10, the following is
generated:
/* Automatically generated by generate-configlist.sh */
static const char *config_name_list[] = {
NULL,
};
After the patch the output on Solaris is the same as on Linux.
So the patch is perfect.
Reviewed-by: Collin Funk <collin.funk1@gmail.com>
Collin
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: Solaris sed
2025-06-12 4:19 ` Collin Funk
@ 2025-06-13 20:13 ` Jean-Noël AVILA
2025-06-13 20:23 ` Eric Sunshine
2025-06-13 20:30 ` Collin Funk
0 siblings, 2 replies; 16+ messages in thread
From: Jean-Noël AVILA @ 2025-06-13 20:13 UTC (permalink / raw)
To: Brad Smith, Collin Funk; +Cc: Junio C Hamano, git
On Thursday, 12 June 2025 06:19:35 CEST Collin Funk wrote:
> Brad Smith <brad@comstyle.com> writes:
> > No errors or warnings after this is applied.
>
> Likewise.
>
> I checked on my Linux machine and both files are the same before and
> after the patch. Before the patch on Solaris 10, the following is
> generated:
>
> /* Automatically generated by generate-configlist.sh */
>
>
> static const char *config_name_list[] = {
> NULL,
> };
>
> After the patch the output on Solaris is the same as on Linux.
>
> So the patch is perfect.
>
> Reviewed-by: Collin Funk <collin.funk1@gmail.com>
>
> Collin
Hello,
Would it be possible to set up some kind of CI to check for compatibility with
such systems. This is the second time I introduced regressions without even
knowing it, and it would be really great to catch them before borking a
release process.
Thanks,
JN
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: Solaris sed
2025-06-13 20:13 ` Jean-Noël AVILA
@ 2025-06-13 20:23 ` Eric Sunshine
2025-06-13 20:30 ` Collin Funk
1 sibling, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2025-06-13 20:23 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: Brad Smith, Collin Funk, Junio C Hamano, git
On Fri, Jun 13, 2025 at 4:15 PM Jean-Noël AVILA <jn.avila@free.fr> wrote:
> Would it be possible to set up some kind of CI to check for compatibility with
> such systems. This is the second time I introduced regressions without even
> knowing it, and it would be really great to catch them before borking a
> release process.
Had this been in a test script, it would have been caught by
t/check-non-portable-shell.sh. We may want to apply the check to
build-related scripts, as well. For instance, it would have caught the
-E problem:
% ./t/check-non-portable-shell.pl generate-*.sh
generate-configlist.sh:16: error: sed option not portable (use
only -n, -e, -f): sed -E '
%
You can, of course, run check-non-portable-shell.pl manually after
editing a script, but perhaps this check could be enabled by a
(hopefully) minor tweak to the main Git Makefile?
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: Solaris sed
2025-06-13 20:13 ` Jean-Noël AVILA
2025-06-13 20:23 ` Eric Sunshine
@ 2025-06-13 20:30 ` Collin Funk
1 sibling, 0 replies; 16+ messages in thread
From: Collin Funk @ 2025-06-13 20:30 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: Brad Smith, Junio C Hamano, git
Jean-Noël AVILA <jn.avila@free.fr> writes:
> Would it be possible to set up some kind of CI to check for compatibility with
> such systems. This is the second time I introduced regressions without even
> knowing it, and it would be really great to catch them before borking a
> release process.
I'm sure that Solaris packagers are used to patching stuff like this. I
wouldn't feel guilty about it. It is difficult to remember all these
portability quirks.
With GitHub actions you can add Oracle Solaris and OmniOS (based on
illumos, which was based on OpenSolaris) using vmactions [1]. That might
help catch some stuff.
In this case, the build still works even with the broken sed commands.
Not sure if the tests would have caught it though.
Collin
[1] https://github.com/vmactions
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 4:03 ` Junio C Hamano
2025-06-12 4:13 ` Brad Smith
@ 2025-06-12 5:50 ` Eric Sunshine
2025-06-12 13:35 ` Paul Smith
1 sibling, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2025-06-12 5:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brad Smith, git
On Thu, Jun 12, 2025 at 12:05 AM Junio C Hamano <gitster@pobox.com> wrote:
> Brad Smith <brad@comstyle.com> writes:
> > Building on Solaris I noticed the following two issues with Solaris sed.
> > GEN version-def.h
> > sed: Missing newline at end of file standard input.
>
> Perhaps it is this input line it is complaining about. sed works on
> text files, and a file that ends in incomplete line was not quite
> text.
>
> -REPLACED=$(printf "%s" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
> +REPLACED=$(printf "%s\n" "$INPUT" | sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
> -e "s|@GIT_MAJOR_VERSION@|$GIT_MAJOR_VERSION|" \
> -e "s|@GIT_MINOR_VERSION@|$GIT_MINOR_VERSION|" \
> -e "s|@GIT_MICRO_VERSION@|$GIT_MICRO_VERSION|" \
It's curious that this is using:
printf "%s" "$foo"`
in the first place. Had it used the simpler:
echo "$foo"
this sort of problem (forgetting the "\n") would never have occurred.
In fact, it seems that f6a2efdc9b (GIT-VERSION-GEN: allow running
without input and output files, 2025-01-22), which introduced this
problem, also introduced a few similar cases in which the `printf
"%s\n"` idiom was employed when a simple `echo` would have sufficed.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: Solaris sed
2025-06-12 5:50 ` Eric Sunshine
@ 2025-06-12 13:35 ` Paul Smith
2025-06-12 16:40 ` Eric Sunshine
0 siblings, 1 reply; 16+ messages in thread
From: Paul Smith @ 2025-06-12 13:35 UTC (permalink / raw)
To: git
On Thu, 2025-06-12 at 01:50 -0400, Eric Sunshine wrote:
> Had it used the simpler:
>
> echo "$foo"
>
> this sort of problem (forgetting the "\n") would never have occurred.
Just be aware that echo is not well-standardized: many versions of echo
accept extra options or treat certain chars specially. So, printf
(which IS well-standardized) is always safer unless you are 100% sure
that the text on the echo command line is simple: cannot start with a
"-", doesn't contain special chars like backslash, etc.
For portability I (personally) always prefer printf unless I know
exactly what the text contains (like showing a static string).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
2025-06-12 13:35 ` Paul Smith
@ 2025-06-12 16:40 ` Eric Sunshine
0 siblings, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2025-06-12 16:40 UTC (permalink / raw)
To: paul; +Cc: git
On Thu, Jun 12, 2025 at 9:44 AM Paul Smith <paul@mad-scientist.net> wrote:
> On Thu, 2025-06-12 at 01:50 -0400, Eric Sunshine wrote:
> > Had it used the simpler:
> >
> > echo "$foo"
> >
> > this sort of problem (forgetting the "\n") would never have occurred.
>
> Just be aware that echo is not well-standardized: many versions of echo
> accept extra options or treat certain chars specially. So, printf
> (which IS well-standardized) is always safer unless you are 100% sure
> that the text on the echo command line is simple: cannot start with a
> "-", doesn't contain special chars like backslash, etc.
>
> For portability I (personally) always prefer printf unless I know
> exactly what the text contains (like showing a static string).
Yup, you're right. I always do the same when I can't trust the
argument to be `echo`-safe, but apparently I wasn't thinking of that
case when I wrote the email. Thanks for the dose of sanity.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Solaris sed
@ 2025-06-12 22:52 Ben Knoble
0 siblings, 0 replies; 16+ messages in thread
From: Ben Knoble @ 2025-06-12 22:52 UTC (permalink / raw)
To: paul; +Cc: git
> Le 12 juin 2025 à 09:42, Paul Smith <paul@mad-scientist.net> a écrit :
>
> On Thu, 2025-06-12 at 01:50 -0400, Eric Sunshine wrote:
>> Had it used the simpler:
>> echo "$foo"
>> this sort of problem (forgetting the "\n") would never have occurred.
>
> Just be aware that echo is not well-standardized: many versions of echo
> accept extra options or treat certain chars specially. So, printf
> (which IS well-standardized) is always safer unless you are 100% sure
> that the text on the echo command line is simple: cannot start with a
> "-", doesn't contain special chars like backslash, etc.
>
> For portability I (personally) always prefer printf unless I know
> exactly what the text contains (like showing a static string).
For exactly this reason, I attempted to write echo(1) to the standards I could find freely available; POSIX mentioned 4, IIRC.
https://github.com/benknoble/echocho
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-06-13 20:31 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-12 3:23 Solaris sed Brad Smith
2025-06-12 3:42 ` Collin Funk
2025-06-12 3:49 ` Brad Smith
2025-06-12 4:16 ` Eli Schwartz
2025-06-12 4:25 ` Collin Funk
2025-06-12 4:26 ` Brad Smith
2025-06-12 4:03 ` Junio C Hamano
2025-06-12 4:13 ` Brad Smith
2025-06-12 4:19 ` Collin Funk
2025-06-13 20:13 ` Jean-Noël AVILA
2025-06-13 20:23 ` Eric Sunshine
2025-06-13 20:30 ` Collin Funk
2025-06-12 5:50 ` Eric Sunshine
2025-06-12 13:35 ` Paul Smith
2025-06-12 16:40 ` Eric Sunshine
-- strict thread matches above, loose matches on Subject: below --
2025-06-12 22:52 Ben Knoble
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox