public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Brad Smith <brad@comstyle.com>,
	Collin Funk <collin.funk1@gmail.com>
Subject: Re: build: sed portability fixes
Date: Fri, 13 Jun 2025 06:59:32 -0400	[thread overview]
Message-ID: <20250613105932.GA1995623@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqplf8evr9.fsf@gitster.g>

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

  reply	other threads:[~2025-06-13 10:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-12 17:04 build: sed portability fixes Junio C Hamano
2025-06-13 10:59 ` Jeff King [this message]
2025-06-13 16:07   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250613105932.GA1995623@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=brad@comstyle.com \
    --cc=collin.funk1@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox