threads / discuss / 63635

build: sed portability fixes

Subject: build: sed portability fixes

## tl;dr

3 messages between Jun 12, 2025 and Jun 13, 2025.

replies: 2people: 2as markdown or json

Junio C Hamano· Jun 12, 2025, 17:04 UTC · lore

Recently generating the version-def.h file and the config-list.h file have been updated, which broke versions of "sed" that do not want to be fed a file that ends with an incomplete line, and/or that do not understand the more recent "-E" option to use extended regular expression.

Fix them in response to a build-failure reported on Solaris boxes.

cf. https://lore.kernel.org/git/09f954b8-d9c3-418f-ad4b-9cb9b063f4ae@comstyle.com/

Reported-by: Brad Smith <brad@comstyle.com>
Reviewed-by: Collin Funk <collin.funk1@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This time with a proposed log message.  I may fast-track it down
   to 'master' before the release.  I personally am undecided, and I
   do know that I hate the style of this particular sed script and
   am tempted to fix it before committing, but I'll refrain from
   doing so before the release.

 GIT-VERSION-GEN        | 2 +-
 generate-configlist.sh | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 1047b8d11d..ad3aa59045 100755
--- a/GIT-VERSION-GEN
+++ b/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 a/generate-configlist.sh b/generate-configlist.sh
index 9d2ad6165d..75c39ade20 100755
--- a/generate-configlist.sh
+++ b/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,

-- 
2.50.0-rc2-215-g56f75d5edf

Jeff King· Jun 13, 2025, 10:59 UTC · re: Junio C Hamano · lore

Re: build: sed portability fixes

On Thu, Jun 12, 2025 at 10:04:58AM -0700, Junio C Hamano wrote:

Show 5 quoted lines

>  * This time with a proposed log message.  I may fast-track it down
>    to 'master' before the release.  I personally am undecided, and I
>    do know that I hate the style of this particular sed script and
>    am tempted to fix it before committing, but I'll refrain from
>    doing so before the release.

The newline-less input is in v2.49.0 already, but the use of "sed -E" is new in the 2.50 cycle. So it probably is worth addressing before the release. In which case I tried to give the patch a very careful read to avoid any brown paper bags.

Show 13 quoted lines

> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index 1047b8d11d..ad3aa59045 100755
> --- a/GIT-VERSION-GEN
> +++ b/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|" \

OK, makes sense since we now stick the content into the INPUT variable. That sometimes comes from a file, but we get it via process substitution with $(cat), so the shell will strip off the trailing newline there. So we can unconditionally add it back here. Goo.d

Show 12 quoted lines

> diff --git a/generate-configlist.sh b/generate-configlist.sh
> index 9d2ad6165d..75c39ade20 100755
> --- a/generate-configlist.sh
> +++ b/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].*\..*`*::$/ {

OK, this is just replacing the use of "?" with "*". I think it is OK to be loose here, as we are parsing our own config docs. And if somebody did write

  ```foo.bar```::

it is probably OK to parse that anyway. ;)

Show 5 quoted lines

> -d' \
> +	d' \
>  	    "$SOURCE_DIR"/Documentation/*config.adoc \
> -	    "$SOURCE_DIR"/Documentation/config/*.adoc|
> +	    "$SOURCE_DIR"/Documentation/config/*.adoc |

And then this (plus the indentation above) is just non-semantic whitespace tidying.

So the whole thing looks good to me.

-Peff

Junio C Hamano· Jun 13, 2025, 16:07 UTC · re: Jeff King · lore

Re: build: sed portability fixes

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> On Thu, Jun 12, 2025 at 10:04:58AM -0700, Junio C Hamano wrote:
>
>>  * This time with a proposed log message.  I may fast-track it down
>>    to 'master' before the release.  I personally am undecided, and I
>>    do know that I hate the style of this particular sed script and
>>    am tempted to fix it before committing, but I'll refrain from
>>    doing so before the release.
>
> The newline-less input is in v2.49.0 already, but the use of "sed -E" is
> new in the 2.50 cycle. So it probably is worth addressing before the
> release. In which case I tried to give the patch a very careful read to
> avoid any brown paper bags.

Yeah, I could split them into two patches, but I do not think it is worth keeping one half broken for the sake of being bug-to-bug compatible with a previous version in this case.

Show 18 quoted lines

>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
>> index 1047b8d11d..ad3aa59045 100755
>> --- a/GIT-VERSION-GEN
>> +++ b/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|" \
>
> OK, makes sense since we now stick the content into the INPUT variable.
> That sometimes comes from a file, but we get it via process substitution
> with $(cat), so the shell will strip off the trailing newline there. So
> we can unconditionally add it back here. Goo.d

Yup, the commit that introduced this sprinkled the same 'printf "%s\n" used as a better/safer version of echo, feeding a pipe' pattern and this I think is merely a typo.

Show 15 quoted lines

>> diff --git a/generate-configlist.sh b/generate-configlist.sh
>> index 9d2ad6165d..75c39ade20 100755
>> --- a/generate-configlist.sh
>> +++ b/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].*\..*`*::$/ {
>
> OK, this is just replacing the use of "?" with "*". I think it is OK to
> be loose here, as we are parsing our own config docs.

Exactly. Similar reasoning appears in the precursor of this patch, i.e. https://lore.kernel.org/git/xmqqo6utfvxu.fsf@gitster.g/

Show 6 quoted lines

> And if somebody
> did write
>
>   ```foo.bar```::
>
> it is probably OK to parse that anyway. ;)

probably ;-).

Show 10 quoted lines

>> -d' \
>> +	d' \
>>  	    "$SOURCE_DIR"/Documentation/*config.adoc \
>> -	    "$SOURCE_DIR"/Documentation/config/*.adoc|
>> +	    "$SOURCE_DIR"/Documentation/config/*.adoc |
>
> And then this (plus the indentation above) is just non-semantic
> whitespace tidying.
>
> So the whole thing looks good to me.

Thanks.

Will fast-track.