Bug #8571
closed
Makefile.master should not trust $PATH
100%
Description
There are a small number of places in Makefile.master where we do not specify full paths. With true(1) this is probably not a big deal, but with echo(1) and install(1) this may matter. We should not be so trusting of $PATH. Especially in the case of install(1), where we're the practically the only user of the SysV version.
Related issues
Updated by Rich Lowe about 5 years ago
- Related to Bug #4163: Illumos build should not break if /usr/bin/install is installed added
Updated by Rich Lowe about 5 years ago
The downside of this, is that in cases where make is not able to use its builtin shell 'mksh' and has to use a real shell, the builtin versions of echo and true will not be used if we have specified the full paths.
In the current source tree this is a vast minority of instances, though partly that's because people don't use $(ECHO) and $(TRUE) as they should. Even then in the case of echo, it is a tiny instance, and it seems to me that consistent behaviour of echo is more important.
I struggle to believe someone could implement an incompatible 'true', but also struggle to believe it'll make sufficient difference to warrant commentary and the lack of uniformity
Updated by Rich Lowe about 5 years ago
Casual experimentation shows that I'm wrong in the comment above. It does indeed make a noticeable difference to not have the built-in echo, and we should only fix INS.
This is less bad, correctness-wise, since it's the echo builtin to the shell used by make, which will either be /bin/sh, or explicit in the Makefile, so at least it's still going to be as expected (unless that shell lacks a builtin echo, in which case we've time-travelled).
Updated by Electric Monk about 5 years ago
- Status changed from In Progress to Closed
- % Done changed from 0 to 100
git commit 6af78b59cf0bb5ad194f2ab51b2302d8c45d2543
commit 6af78b59cf0bb5ad194f2ab51b2302d8c45d2543
Author: Richard Lowe <richlowe@richlowe.net>
Date: 2018-01-27T21:41:20.000Z
8571 Makefile.master should not trust $PATH
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Peter Tribble <peter.tribble@gmail.com>
Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
Approved by: Dan McDonald <danmcd@joyent.com>
Updated by Electric Monk about 5 years ago
git commit 6c9c1c0d3911e7534e0028fb6e2a09aa4609730d
commit 6c9c1c0d3911e7534e0028fb6e2a09aa4609730d
Author: Richard Lowe <richlowe@richlowe.net>
Date: 2018-01-28T15:27:38.000Z
backout: 8571 Makefile.master should not trust $PATH (insufficient testing, broke bootstrap)
Updated by Electric Monk about 5 years ago
git commit c08555783b53af86d5998de8a9ea7c58d83d75c9
commit c08555783b53af86d5998de8a9ea7c58d83d75c9
Author: Richard Lowe <richlowe@richlowe.net>
Date: 2018-02-12T10:56:22.000Z
8571 Makefile.master should not trust $PATH
Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Approved by: Hans Rosenfeld <hans.rosenfeld@joyent.com>
Updated by Rich Lowe almost 4 years ago
- Related to Bug #11246: krb5 makefiles should use INS or INS.file added