Re: build: sed portability fixes
- From
Jeff King <peff@peff.net>
- Date
- Jun 13, 2025, 10:59 UTC
- Message-ID
- <20250613105932.GA1995623@coredump.intra.peff.net>
- In-Reply-To
- <xmqqplf8evr9.fsf@gitster.g>
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