[1/4] Use $(SHELL_PATH) instead of sh in Makefile.
diff mbox series

Message ID 1f63b4fc7c2ae1970d7226bbf0b66901528fb9d8.1596675905.git.gitgitgadget@gmail.com
State Superseded
Headers show
Series
  • Fit the building tools to Plan 9 environment
Related show

Commit Message

Sean Barag via GitGitGadget Aug. 6, 2020, 1:05 a.m. UTC
From: lufia <lufia@lufia.org>

In the not POSIX environment, like Plan 9, sh might not be looked up
in the directories named by the $PATH.

Signed-off-by: lufia <lufia@lufia.org>
---
 config.mak.uname | 12 ++++++------
 git-gui/Makefile |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)

Comments

brian m. carlson Aug. 6, 2020, 2:13 a.m. UTC | #1
On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> From: lufia <lufia@lufia.org>
> 
> In the not POSIX environment, like Plan 9, sh might not be looked up
> in the directories named by the $PATH.

I think Git's editor handling assumes that sh is somewhere in the PATH,
so it might be fine for us to just ask the user to adjust PATH
appropriately before running make.  I don't have a strong preference; if
this works on a standard Unix machine, which it looks like it should
(although I haven't tested), I'm fine with it.
Eric Sunshine Aug. 6, 2020, 4:10 a.m. UTC | #2
On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> > In the not POSIX environment, like Plan 9, sh might not be looked up
> > in the directories named by the $PATH.
>
> I think Git's editor handling assumes that sh is somewhere in the PATH,
> so it might be fine for us to just ask the user to adjust PATH
> appropriately before running make.  I don't have a strong preference; if
> this works on a standard Unix machine, which it looks like it should
> (although I haven't tested), I'm fine with it.

This does, however, have a bit of a chicken-and-egg feel to it. The
results of the "uname_FOO" assignments in config.mak.uname are
consulted later in the file to _assign_ a value to SHELL_PATH on a
number of platforms. So, making the "uname_FOO" assignments themselves
depend upon SHELL_PATH is rather circular and confusing.
Kyohei Kadota Aug. 6, 2020, 2:39 p.m. UTC | #3
> On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> > > In the not POSIX environment, like Plan 9, sh might not be looked up
> > > in the directories named by the $PATH.
> >
> > I think Git's editor handling assumes that sh is somewhere in the PATH,
> > so it might be fine for us to just ask the user to adjust PATH
> > appropriately before running make.  I don't have a strong preference; if
> > this works on a standard Unix machine, which it looks like it should
> > (although I haven't tested), I'm fine with it.
>
> This does, however, have a bit of a chicken-and-egg feel to it. The
> results of the "uname_FOO" assignments in config.mak.uname are
> consulted later in the file to _assign_ a value to SHELL_PATH on a
> number of platforms. So, making the "uname_FOO" assignments themselves
> depend upon SHELL_PATH is rather circular and confusing.

The problem is, Plan 9 APE's sh is placed on /bin/ape/sh by default.
By running /bin/ape/psh
APE commands existed under /bin/ape such as sh, uname, etc, will be
mounted on /bin.
In running of ape/psh, APE commands will prioritize than native commands.
So I've wanted to run /bin/ape/psh before executing some shell
commands by gmake.

----
kadota
Junio C Hamano Aug. 6, 2020, 5:30 p.m. UTC | #4
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>> On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
>> > In the not POSIX environment, like Plan 9, sh might not be looked up
>> > in the directories named by the $PATH.
>>
>> I think Git's editor handling assumes that sh is somewhere in the PATH,
>> so it might be fine for us to just ask the user to adjust PATH
>> appropriately before running make.  I don't have a strong preference; if
>> this works on a standard Unix machine, which it looks like it should
>> (although I haven't tested), I'm fine with it.
>
> This does, however, have a bit of a chicken-and-egg feel to it. The
> results of the "uname_FOO" assignments in config.mak.uname are
> consulted later in the file to _assign_ a value to SHELL_PATH on a
> number of platforms. So, making the "uname_FOO" assignments themselves
> depend upon SHELL_PATH is rather circular and confusing.

Is that just being circular and confusing, or does it produce an
incorrect result depending on the circumstances?  We would end up
special casing SHELL_PATH (e.g. exclude it from the variables that
are set based on uname), which I would want to avoid, as I would
suspect that there will be even more variables that need similar
treatment. 

This does have a bad smell.  Even on a not-so-posix Windows, we do
not use such a hack.
Kyohei Kadota Aug. 10, 2020, 9:04 a.m. UTC | #5
After a moment's thought I decided to run ape/psh before building git.
If so then, sh will be found from the directories named in PATH
because it is mounted onto /bin.

2020年8月7日(金) 2:30 Junio C Hamano <gitster@pobox.com>:
>
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > On Wed, Aug 5, 2020 at 10:14 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> >> On 2020-08-06 at 01:05:01, lufia via GitGitGadget wrote:
> >> > In the not POSIX environment, like Plan 9, sh might not be looked up
> >> > in the directories named by the $PATH.
> >>
> >> I think Git's editor handling assumes that sh is somewhere in the PATH,
> >> so it might be fine for us to just ask the user to adjust PATH
> >> appropriately before running make.  I don't have a strong preference; if
> >> this works on a standard Unix machine, which it looks like it should
> >> (although I haven't tested), I'm fine with it.
> >
> > This does, however, have a bit of a chicken-and-egg feel to it. The
> > results of the "uname_FOO" assignments in config.mak.uname are
> > consulted later in the file to _assign_ a value to SHELL_PATH on a
> > number of platforms. So, making the "uname_FOO" assignments themselves
> > depend upon SHELL_PATH is rather circular and confusing.
>
> Is that just being circular and confusing, or does it produce an
> incorrect result depending on the circumstances?  We would end up
> special casing SHELL_PATH (e.g. exclude it from the variables that
> are set based on uname), which I would want to avoid, as I would
> suspect that there will be even more variables that need similar
> treatment.
>
> This does have a bad smell.  Even on a not-so-posix Windows, we do
> not use such a hack.
>

Patch
diff mbox series

diff --git a/config.mak.uname b/config.mak.uname
index c7eba69e54..f3eb2b91a2 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -4,12 +4,12 @@ 
 # Microsoft's Safe Exception Handling in libraries (such as zlib).
 # Typically required for VS2013+/32-bit compilation on Vista+ versions.
 
-uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not')
-uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo not')
-uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not')
-uname_R := $(shell sh -c 'uname -r 2>/dev/null || echo not')
-uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')
-uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
+uname_S := $(shell $(SHELL_PATH) -c 'uname -s 2>/dev/null || echo not')
+uname_M := $(shell $(SHELL_PATH) -c 'uname -m 2>/dev/null || echo not')
+uname_O := $(shell $(SHELL_PATH) -c 'uname -o 2>/dev/null || echo not')
+uname_R := $(shell $(SHELL_PATH) -c 'uname -r 2>/dev/null || echo not')
+uname_P := $(shell $(SHELL_PATH) -c 'uname -p 2>/dev/null || echo not')
+uname_V := $(shell $(SHELL_PATH) -c 'uname -v 2>/dev/null || echo not')
 
 ifdef MSVC
 	# avoid the MingW and Cygwin configuration sections
diff --git a/git-gui/Makefile b/git-gui/Makefile
index f10caedaa7..c47603c397 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -11,9 +11,9 @@  GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
 
-uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not')
-uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not')
-uname_R := $(shell sh -c 'uname -r 2>/dev/null || echo not')
+uname_S := $(shell $(SHELL_PATH) -c 'uname -s 2>/dev/null || echo not')
+uname_O := $(shell $(SHELL_PATH) -c 'uname -o 2>/dev/null || echo not')
+uname_R := $(shell $(SHELL_PATH) -c 'uname -r 2>/dev/null || echo not')
 
 SCRIPT_SH = git-gui.sh
 GITGUI_MAIN := git-gui