Remove absolute pathnames to binaries and rely on PATH #29
Conversation
do
Outdated
| @@ -24,7 +24,7 @@ usage() { | |||
| } | |||
|
|
|||
| mydir=$(dirname "$0") | |||
| cd "$(/bin/pwd)" && cd "$mydir" || die "can't find self in dir: $mydir" | |||
| cd "$(pwd)" && cd "$mydir" || die "can't find self in dir: $mydir" | |||
There was a problem hiding this comment.
We can't do this, as it will end up using the shell's internal pwd implementation. That has subtle differences which we are intentionally trying to bypass.
There was a problem hiding this comment.
Then we should call pwd as /usr/bin/env pwd or capture the path to pwd in a variable if the performance penalty is too high.
There was a problem hiding this comment.
Well, just to clarify, do you really truly have a system where the 'pwd' command is not in /bin? Because that seems like a hopelessly broken system to me. Why on earth would a system with no /bin/pwd have a /usr/bin/env?
There was a problem hiding this comment.
NixOS! NixOS has no /bin/pwd, but it maintains env and sh in typical locations for compatibility with scripts.
There was a problem hiding this comment.
Ok. We can use env pwd then if we must, although it seems a bit ridiculous. (There is no need for /usr/bin/env here; the explicit path is just to force sh to not use its internal version. If it has an internal env, that's ok.)
| @@ -10,13 +10,13 @@ fi | |||
|
|
|||
| # builds 1xx*/all to test for basic/dangerous functionality. | |||
| # We don't want to run more advanced tests if the basics don't work. | |||
| /bin/ls 1[0-9][0-9]*/all.do | | |||
| ls 1[0-9][0-9]*/all.do | | |||
There was a problem hiding this comment.
...whereas yes, I think it's fine to remove the /bin when calling ls.
|
Yes, I do. It is called NixOS. The reasoning behind this is to have a system in which all runtime dependencies are statically resolved for all binaries. Depending on context this might mean different things. It involves, though, that programs don't assume to find other programs in fixed locations with two exceptions (to fix the bootstrapping problem): /bin/sh, and /usr/bin/env.
This allows you to guarantee that a particular build of a program only ever calls a particular build of, e.g., pwd, regardless of other versions of pwd existing on the system.
|
|
On the topic of portability, |
|
redo's sh selector (t/shelltest.od) will reject any sh that doesn't have
'local'. I still haven't found one that doesn't, so that's not a big
problem, even if it isn't technically in POSIX.
ᐧ
…On Tue, Oct 27, 2020 at 3:19 PM periish ***@***.***> wrote:
On the topic of portability, sh isn’t guaranteed to have the local
builtin.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#29 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAFA4G6PWCC27NVPPWI7JTSM4MMRANCNFSM4IIOR5ZA>
.
--
Avery Pennarun // CEO @ Tailscale
|
|
How should we proceed? Shall I keep the path to pwd in a variable? |
Fixes issues with distributions that use non-standard binary paths (e.g., in isolated build environments). Scripts rely on PATH for other programs, anyway. Use /usr/bin/env to find the pwd binary avoiding the shell built-in.
spacefrogg
force-pushed
the
path-fixes
branch
from
5977e99
to
c232b51
Compare
|
Why not $PWD? It’s part of POSIX.
… On 11 Nov 2020, at 12:30, spacefrogg ***@***.***> wrote:
@spacefrogg commented on this pull request.
In do:
> @@ -24,7 +24,7 @@ usage() {
}
mydir=$(dirname "$0")
-cd "$(/bin/pwd)" && cd "$mydir" || die "can't find self in dir: $mydir"
+cd "$(pwd)" && cd "$mydir" || die "can't find self in dir: $mydir"
Done.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
This particular chunk of code is trying to defeat the unhelpful
physical-logical distinction in $PWD. If you don't know the difference
between `pwd -P` and `pwd -L`, go read about those first :)
But there was some other obscure reason I couldn't simply use the
sh-internal pwd -P, which I've now forgotten. I do remember it was
extra double plus arcane.
…On Wed, Nov 11, 2020 at 7:38 PM periish ***@***.***> wrote:
Why not $PWD? It’s part of POSIX.
> On 11 Nov 2020, at 12:30, spacefrogg ***@***.***> wrote:
>
>
> @spacefrogg commented on this pull request.
>
> In do:
>
> > @@ -24,7 +24,7 @@ usage() {
> }
>
> mydir=$(dirname "$0")
> -cd "$(/bin/pwd)" && cd "$mydir" || die "can't find self in dir: $mydir"
> +cd "$(pwd)" && cd "$mydir" || die "can't find self in dir: $mydir"
> Done.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or unsubscribe.
--
Avery Pennarun // CEO @ Tailscale
|
|
I incorporated all changes. I am a bit puzzled, now. @apenwarr, do you accept this PR or does it need further work? |
|
@apenwarr As this PR hasn't seen any attention for the last year now, I want to reach out to you again concerning the time you are willing to invest into this topic. Apart from this particular PR, I have spent some effort brushing up redoconf. The current implementation has several bugs, some deficiencies and lacked modularity/extensibility in certain respects (as it was very focussed on compiling C code). I kept its function completely compatible with the libssl/hello world example from the documentation. I also added builders and feature detectors for building TeX documents. Before publishing my extended version of redoconf, I wanted to hear your opinion on the way forward. I don't want to distract you from anything, which is why I would be willing to publish and push forward redoconf separately from redo. |
Fixes issues with distributions that use non-standard binary paths (e.g., in
isolated build environments). Scripts rely on PATH for other programs, anyway.