Re: [PATCH] Makefile: tweak sed invocation

2018-07-02 Thread Alejandro R . Sedeño

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

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


Re: [PATCH] Makefile: tweak sed invocation

2018-06-25 Thread Alejandro R . Sedeño

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

On Mon, Jun 25, 2018 at 3:18 PM Alejandro R. Sedeño  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




Signed-off-by: Alejandro R. Sedeño 
---
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' \




Re: [PATCH] Makefile: tweak sed invocation

2018-06-25 Thread Eric Sunshine
On Mon, Jun 25, 2018 at 3:18 PM Alejandro R. Sedeño  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.

> Signed-off-by: Alejandro R. Sedeño 
> ---
> 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' \


[PATCH] Makefile: tweak sed invocation

2018-06-25 Thread Alejandro R . Sedeño
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 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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