threads / patch / 48774

patchMakefile: tweak sed invocation

Subject: [PATCH] Makefile: tweak sed invocation

## tl;dr

4 messages between Jun 25, 2018 and Jul 2, 2018. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Alejandro R. Sedeño· Jun 25, 2018, 19:13 UTC · lore

With GNU sed, the r command doesn't care if a space separates it and the filename it reads from.

With SunOS sed, the space is required.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to Makefile +1 −1

diff --git a/Makefile b/Makefile
index e4b503d..5bac181 100644
--- a/Makefile
+++ b/Makefile
@@ -2109,7 +2109,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1{' \
 	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-	    -e '	rGIT-PERL-HEADER' \
+	    -e '	r GIT-PERL-HEADER' \
 	    -e '	G' \
 	    -e '}' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \

-- 
2.1.4

Eric Sunshine· Jun 25, 2018, 20:15 UTC · re: Alejandro R. Sedeño · lore

Re: [PATCH] Makefile: tweak sed invocation

On Mon, Jun 25, 2018 at 3:18 PM Alejandro R. Sedeño <asedeno@mit.edu> wrote:

> With GNU sed, the r command doesn't care if a space separates it and
> the filename it reads from.
>
> With SunOS sed, the space is required.

MacOS and the various BSD's ship with BSD 'sed', not GNU 'sed', so it seemed prudent to check this change against them as well, which I did, and can report that it does not cause any regression on those platforms.

Therefore, the patch looks good. Thanks.

Show 12 quoted lines

> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> ---
> diff --git a/Makefile b/Makefile
> @@ -2109,7 +2109,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
>         $(QUIET_GEN)$(RM) $@ $@+ && \
>         sed -e '1{' \
>             -e '        s|#!.*perl|#!$(PERL_PATH_SQ)|' \
> -           -e '        rGIT-PERL-HEADER' \
> +           -e '        r GIT-PERL-HEADER' \
>             -e '        G' \
>             -e '}' \
>             -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \

Alejandro R. Sedeño· Jun 25, 2018, 20:27 UTC · re: Eric Sunshine · lore

Re: [PATCH] Makefile: tweak sed invocation

On 2018-06-25 16:15, Eric Sunshine wrote:

Show 12 quoted lines

> On Mon, Jun 25, 2018 at 3:18 PM Alejandro R. Sedeño <asedeno@mit.edu> wrote:
>> With GNU sed, the r command doesn't care if a space separates it and
>> the filename it reads from.
>>
>> With SunOS sed, the space is required.
> 
> MacOS and the various BSD's ship with BSD 'sed', not GNU 'sed', so it
> seemed prudent to check this change against them as well, which I did,
> and can report that it does not cause any regression on those
> platforms.
> 
> Therefore, the patch looks good. Thanks.

Thanks for checking on that, Eric. I tested MacOS locally before submitting as well. From a quick skim of the POSIX sed page, the space is expected, so this should be portable.

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

-Alejandro

Show 13 quoted lines

> 
>> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
>> ---
>> diff --git a/Makefile b/Makefile
>> @@ -2109,7 +2109,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
>>          $(QUIET_GEN)$(RM) $@ $@+ && \
>>          sed -e '1{' \
>>              -e '        s|#!.*perl|#!$(PERL_PATH_SQ)|' \
>> -           -e '        rGIT-PERL-HEADER' \
>> +           -e '        r GIT-PERL-HEADER' \
>>              -e '        G' \
>>              -e '}' \
>>              -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \

Alejandro R. Sedeño· Jul 2, 2018, 23:12 UTC · lore

Re: [PATCH] Makefile: tweak sed invocation

On 2018-06-26 14:35, Junio C Hamano wrote:

Show 5 quoted lines

> Having said that, I'm a bit surprised that our build infrastructure
> and shell scripts still work on tools on SunOS.  I used to have
> access to SunOS/Solaris boxes and tried to be careful not to break
> them unnecessarily, but these days I don't, so I expected to hear
> quite a huge bit-rotting.

I end up building new releases on SunOS all the time; when things break there is usually when you hear from me. I'm hoping this patch makes it into 2.18.1 so I don't have to apply it during my build process.

-Alejandro