* build: sed portability fixes
@ 2025-06-12 17:04 Junio C Hamano
2025-06-13 10:59 ` Jeff King
0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2025-06-12 17:04 UTC (permalink / raw)
To: git; +Cc: Brad Smith, Collin Funk
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: build: sed portability fixes
2025-06-12 17:04 build: sed portability fixes Junio C Hamano
@ 2025-06-13 10:59 ` Jeff King
2025-06-13 16:07 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Jeff King @ 2025-06-13 10:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Brad Smith, Collin Funk
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.
> 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
> 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. ;)
> -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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: build: sed portability fixes
2025-06-13 10:59 ` Jeff King
@ 2025-06-13 16:07 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2025-06-13 16:07 UTC (permalink / raw)
To: Jeff King; +Cc: git, Brad Smith, Collin Funk
Jeff King <peff@peff.net> writes:
> 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.
>> 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.
>> 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/
> And if somebody
> did write
>
> ```foo.bar```::
>
> it is probably OK to parse that anyway. ;)
probably ;-).
>> -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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-13 16:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-12 17:04 build: sed portability fixes Junio C Hamano
2025-06-13 10:59 ` Jeff King
2025-06-13 16:07 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox