Git development
 help / color / mirror / Atom feed
* [PATCH] Makefile: use SHELL_PATH when running generate-cmdlist.sh
@ 2015-09-10 23:37 Alejandro R. Sedeño
  2015-09-11  0:35 ` Eric Sunshine
  0 siblings, 1 reply; 2+ messages in thread
From: Alejandro R. Sedeño @ 2015-09-10 23:37 UTC (permalink / raw)
  To: git; +Cc: Alejandro R. Sedeño

Some /bin/sh implementations can't deal with $() arithmetic and command
substitution. If we already have a better shell on hand, we should use it.

Fixes the build on SunOS, probably others.

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

diff --git a/Makefile b/Makefile
index ce0cfe2..6301cc8 100644
--- a/Makefile
+++ b/Makefile
@@ -1699,7 +1699,7 @@ $(BUILT_INS): git$X
 common-cmds.h: generate-cmdlist.sh command-list.txt
 
 common-cmds.h: $(wildcard Documentation/git-*.txt)
-	$(QUIET_GEN)./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Makefile: use SHELL_PATH when running generate-cmdlist.sh
  2015-09-10 23:37 [PATCH] Makefile: use SHELL_PATH when running generate-cmdlist.sh Alejandro R. Sedeño
@ 2015-09-11  0:35 ` Eric Sunshine
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Sunshine @ 2015-09-11  0:35 UTC (permalink / raw)
  To: Alejandro R. Sedeño; +Cc: Git List

On Thu, Sep 10, 2015 at 7:37 PM, Alejandro R. Sedeño <asedeno@mit.edu> wrote:
> Some /bin/sh implementations can't deal with $() arithmetic and command
> substitution. If we already have a better shell on hand, we should use it.
>
> Fixes the build on SunOS, probably others.

Makes sense. 527ec39^:generate-cmdlist.sh didn't use either of these
features, whereas 82aec45:generate-cmdlist.sh does, and older
(pre-POSIX) shells lacked these features. Thanks.

I'd probably re-word the commit message slightly to mention $(())
arithmetic expansion, not $(), and to state specifically $(...)
command substitution since saying only "command substitution" is
ambiguous considering that backtick `...` command substitution long
predates POSIX. Perhaps like this:

    Non-POSIX shells, such as /bin/sh on SunOS, do not support
    $((...)) arithmetic expansion or $(...) command substitution
    needed by generate-cmdlist.sh. Therefore, use the POSIX shell
    $(SHELL_PATH) when running generate-cmdlist.sh.

Other than that:

Acked-by: Eric Sunshine <sunshine@sunshineco.com>

> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> ---
> diff --git a/Makefile b/Makefile
> index ce0cfe2..6301cc8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1699,7 +1699,7 @@ $(BUILT_INS): git$X
>  common-cmds.h: generate-cmdlist.sh command-list.txt
>
>  common-cmds.h: $(wildcard Documentation/git-*.txt)
> -       $(QUIET_GEN)./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
> +       $(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
>
>  SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
>         $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
> --
> 2.5.2

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-09-11  0:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-10 23:37 [PATCH] Makefile: use SHELL_PATH when running generate-cmdlist.sh Alejandro R. Sedeño
2015-09-11  0:35 ` Eric Sunshine

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox