zsh-workers
 help / color / mirror / code / Atom feed
* command -p should enable builtins not in path
@ 2020-08-20 11:28 Vincent Lefevre
  2020-08-21 13:03 ` Peter Stephenson
  2020-08-21 15:47 ` Martijn Dekker
  0 siblings, 2 replies; 9+ messages in thread
From: Vincent Lefevre @ 2020-08-20 11:28 UTC (permalink / raw)
  To: zsh-workers

Though zsh isn't meant to conform to POSIX, it should follow its
requirements when they make sense.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html

says:

  The following options shall be supported:

  -p
    Perform the command search using a default value for PATH that is
    guaranteed to find all of the standard utilities.
                       ^^^

So, in particular, the standard utility "cd" must be found. Note that
the above sentence does not mean that the utility must be somewhere
in $PATH, just that the used PATH value allows the shell to find the
utility; in case of a builtin utility (like "cd"), this will just run
the utility without needing PATH.

This works with all usual shells, except zsh:

zira% sh -c "command -p cd /tmp; pwd"
/tmp
zira% bash -c "command -p cd /tmp; pwd"
/tmp
zira% ksh -c "command -p cd /tmp; pwd"
/tmp
zira% zsh -c "command -p cd /tmp; pwd"
zsh:1: command not found: cd
/home/vinc17
zira% echo $ZSH_VERSION
5.8

Setting the POSIX_BUILTINS option allows "cd" to be found, but with
the drawback that it will not disable builtins that are in $PATH.
So this option is a bad solution when using "zmodload zsh/files"
without -F, for instance.

Note: "command -p cd ..." is used by Intel's script to set up
environment variables for its compiler.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)


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

* Re: command -p should enable builtins not in path
  2020-08-20 11:28 command -p should enable builtins not in path Vincent Lefevre
@ 2020-08-21 13:03 ` Peter Stephenson
  2020-08-21 15:49   ` Martijn Dekker
  2020-08-21 15:47 ` Martijn Dekker
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2020-08-21 13:03 UTC (permalink / raw)
  To: zsh-workers

> On 20 August 2020 at 12:28 Vincent Lefevre <vincent@vinc17.net> wrote:
> 
> 
> Though zsh isn't meant to conform to POSIX, it should follow its
> requirements when they make sense.
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
> 
> says:
> 
>   The following options shall be supported:
> 
>   -p
>     Perform the command search using a default value for PATH that is
>     guaranteed to find all of the standard utilities.
>                        ^^^
> 
> So, in particular, the standard utility "cd" must be found. Note that
> the above sentence does not mean that the utility must be somewhere
> in $PATH, just that the used PATH value allows the shell to find the
> utility; in case of a builtin utility (like "cd"), this will just run
> the utility without needing PATH.
> 
> This works with all usual shells, except zsh:

I tend to agree there's not a lot of point in this incompatibility,
given where the "-p" flag originates --- it's a relatively late
addition to zsh.

Here's one possible fix.

pws

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 4b91db1fe..17ebba2ac 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -318,10 +318,15 @@ item(tt(command) [ tt(-pvV) ] var(simple command))(
 The simple command argument is taken as an external command instead of
 a function or builtin and is executed. If the tt(POSIX_BUILTINS) option
 is set, builtins will also be executed but certain special properties
-of them are suppressed. The tt(-p) flag causes a default path to be
-searched instead of that in tt($path). With the tt(-v) flag, tt(command)
-is similar to tt(whence) and with tt(-V), it is equivalent to tt(whence
--v).
+of them are suppressed.
+
+The tt(-p) flag causes a default path to be searched instead of that in
+tt($path).  The intention of the option is that all standard commands
+may be found, so builtins are also checked, but not other types of
+command such as functions.
+
+With the tt(-v) flag, tt(command) is similar to tt(whence) and with
+tt(-V), it is equivalent to tt(whence -v).
 
 See also ifzman(the section `Precommand Modifiers' in zmanref(zshmisc))\
 ifnzman(noderef(Precommand Modifiers)).
diff --git a/Src/exec.c b/Src/exec.c
index ecad923de..f18513703 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3165,8 +3165,19 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		}
 	    }
 	    hn = NULL;
-	    if ((cflags & BINF_COMMAND) && unset(POSIXBUILTINS))
+	    if ((cflags & BINF_COMMAND) && unset(POSIXBUILTINS)) {
+		if (use_defpath && nonempty(preargs)) {
+		    /*
+		     * command -p can find builtins as these are
+		     * in a standard location.  So check.
+		     */
+		    if ((hn = builtintab->getnode(builtintab,
+						  (char *) peekfirst(preargs)))) {
+			checked = is_builtin = 1;
+		    }
+		}
 		break;
+	    }
 	    if (!nonempty(preargs))
 		execcmd_getargs(preargs, args, eparams->htok);
 	}
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 35a04e7d5..e62fb531b 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -200,6 +200,17 @@
 >cat is /*/cat
 >echo is a shell builtin
 
+  (mkdir testdir_for_command-p
+   command -p cd testdir_for_command-p
+   print ${PWD##*/})
+0:command -p finds builtins
+>testdir_for_command-p
+
+  func_not_searched() { This was not found; }
+  command -p func_not_searched
+127:command -p does not find functions
+?(eval):2: command not found: func_not_searched
+
   cd() { echo Not cd at all; }
   builtin cd . && unfunction cd
 0:`builtin' precommand modifier


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

* Re: command -p should enable builtins not in path
  2020-08-20 11:28 command -p should enable builtins not in path Vincent Lefevre
  2020-08-21 13:03 ` Peter Stephenson
@ 2020-08-21 15:47 ` Martijn Dekker
  1 sibling, 0 replies; 9+ messages in thread
From: Martijn Dekker @ 2020-08-21 15:47 UTC (permalink / raw)
  To: zsh-workers

Op 20-08-20 om 12:28 schreef Vincent Lefevre:
> Though zsh isn't meant to conform to POSIX, it should follow its
> requirements when they make sense.


Just to clarify, 'zsh --emulate sh' *is* meant to conform to POSIX, correct?

[...]
> Setting the POSIX_BUILTINS option allows "cd" to be found, but with
> the drawback that it will not disable builtins that are in $PATH.
> So this option is a bad solution when using "zmodload zsh/files"
> without -F, for instance.
> 
> Note: "command -p cd ..." is used by Intel's script to set up
> environment variables for its compiler.


If it wasn't written for native zsh, it probably has other 
incompatibilities as well. Wouldn't 'sticky emulation' allow you to 
seamlessly mix that code with native zsh code?

- M.

-- 
||	modernish -- harness the shell
||	https://github.com/modernish/modernish
||
||	KornShell lives!
||	https://github.com/ksh93/ksh


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

* Re: command -p should enable builtins not in path
  2020-08-21 13:03 ` Peter Stephenson
@ 2020-08-21 15:49   ` Martijn Dekker
  2020-08-21 16:08     ` Peter Stephenson
  2020-08-24 18:30     ` Vincent Lefevre
  0 siblings, 2 replies; 9+ messages in thread
From: Martijn Dekker @ 2020-08-21 15:49 UTC (permalink / raw)
  To: zsh-workers

Op 21-08-20 om 14:03 schreef Peter Stephenson:
> I tend to agree there's not a lot of point in this incompatibility,
> given where the "-p" flag originates --- it's a relatively late
> addition to zsh.
> 
> Here's one possible fix.

It seems inconsistent/unexpected if 'command -p cd' works, but 'command 
cd' doesn't.

- M.

-- 
||	modernish -- harness the shell
||	https://github.com/modernish/modernish
||
||	KornShell lives!
||	https://github.com/ksh93/ksh


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

* Re: command -p should enable builtins not in path
  2020-08-21 15:49   ` Martijn Dekker
@ 2020-08-21 16:08     ` Peter Stephenson
  2020-08-21 16:35       ` Martijn Dekker
  2020-08-24 18:30     ` Vincent Lefevre
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2020-08-21 16:08 UTC (permalink / raw)
  To: Martijn Dekker, zsh-workers


> On 21 August 2020 at 16:49 Martijn Dekker <martijn@inlv.org> wrote:
> 
> 
> Op 21-08-20 om 14:03 schreef Peter Stephenson:
> > I tend to agree there's not a lot of point in this incompatibility,
> > given where the "-p" flag originates --- it's a relatively late
> > addition to zsh.
> > 
> > Here's one possible fix.
> 
> It seems inconsistent/unexpected if 'command -p cd' works, but 'command 
> cd' doesn't.

However, "command" with no option is a very long-standing piece of zsh
syntax to pick an external command, so that's certainly not going to change.

pws


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

* Re: command -p should enable builtins not in path
  2020-08-21 16:08     ` Peter Stephenson
@ 2020-08-21 16:35       ` Martijn Dekker
  0 siblings, 0 replies; 9+ messages in thread
From: Martijn Dekker @ 2020-08-21 16:35 UTC (permalink / raw)
  To: Peter Stephenson, zsh-workers

Op 21-08-20 om 17:08 schreef Peter Stephenson:
> However, "command" with no option is a very long-standing piece of zsh
> syntax to pick an external command, so that's certainly not going to change.

Of course. So what I'm trying to suggest is that this change to -p is 
maybe not a very good idea either. There is a long-standing expectation 
that 'command' runs an external command in native zsh mode, so why 
should that be different just because you're searching the default path?

Code that expects POSIX behaviour, such as that Intel build script, 
shouldn't be run in native zsh mode anyway. If it needs to cooperate 
with native zsh code, it can be loaded with sticky emulation.

- M.

-- 
||	modernish -- harness the shell
||	https://github.com/modernish/modernish
||
||	KornShell lives!
||	https://github.com/ksh93/ksh


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

* Re: command -p should enable builtins not in path
  2020-08-21 15:49   ` Martijn Dekker
  2020-08-21 16:08     ` Peter Stephenson
@ 2020-08-24 18:30     ` Vincent Lefevre
  2020-08-24 19:59       ` Martijn Dekker
  1 sibling, 1 reply; 9+ messages in thread
From: Vincent Lefevre @ 2020-08-24 18:30 UTC (permalink / raw)
  To: zsh-workers

On 2020-08-21 16:49:06 +0100, Martijn Dekker wrote:
> Op 21-08-20 om 14:03 schreef Peter Stephenson:
> > I tend to agree there's not a lot of point in this incompatibility,
> > given where the "-p" flag originates --- it's a relatively late
> > addition to zsh.
> > 
> > Here's one possible fix.
> 
> It seems inconsistent/unexpected if 'command -p cd' works, but 'command cd'
> doesn't.

However, "command cd" is not guaranteed to work in a POSIX shell,
e.g. if $PATH has changed.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)


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

* Re: command -p should enable builtins not in path
  2020-08-24 18:30     ` Vincent Lefevre
@ 2020-08-24 19:59       ` Martijn Dekker
  2020-10-02 13:17         ` Vincent Lefevre
  0 siblings, 1 reply; 9+ messages in thread
From: Martijn Dekker @ 2020-08-24 19:59 UTC (permalink / raw)
  To: zsh-workers

Op 24-08-20 om 19:30 schreef Vincent Lefevre:
> However, "command cd" is not guaranteed to work in a POSIX shell,
> e.g. if $PATH has changed.

That's actually not true, 'cd' is intrinsically a built-in utility (it 
couldn't possibly function otherwise, as it must affect the current 
environment) and it is also explicitly exempt from a $PATH search by XCU 
2.9.1.4(d)[*] (not that most shells follow that absurd $PATH search rule 
for builtins anyway, but that's a different rant). Point being, POSIXly, 
'command cd' is safe.

- Martijn

[*] 
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01

-- 
||	modernish -- harness the shell
||	https://github.com/modernish/modernish
||
||	KornShell lives!
||	https://github.com/ksh93/ksh


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

* Re: command -p should enable builtins not in path
  2020-08-24 19:59       ` Martijn Dekker
@ 2020-10-02 13:17         ` Vincent Lefevre
  0 siblings, 0 replies; 9+ messages in thread
From: Vincent Lefevre @ 2020-10-02 13:17 UTC (permalink / raw)
  To: zsh-workers

On 2020-08-24 20:59:52 +0100, Martijn Dekker wrote:
> Op 24-08-20 om 19:30 schreef Vincent Lefevre:
> > However, "command cd" is not guaranteed to work in a POSIX shell,
> > e.g. if $PATH has changed.
> 
> That's actually not true, 'cd' is intrinsically a built-in utility (it
> couldn't possibly function otherwise, as it must affect the current
> environment)

Well, AFAIK, some OS's provide a way for processes to affect the
environment of their parent. So an implementation could theoretically
choose to provide cd as an external utility.

> and it is also explicitly exempt from a $PATH search by XCU
> 2.9.1.4(d)[*] (not that most shells follow that absurd $PATH search
> rule for builtins anyway, but that's a different rant). Point being,
> POSIXly, 'command cd' is safe.

OK, I agree.

So, shouldn't this be changed to match the POSIX behavior for the
utility list from "Command Search and Execution"?

  https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01

I'm wondering whether any zsh user relies on an external program
for such utilities. Or, if there is any issue with such a change,
add another option, similar to POSIX_BUILTINS, but restricted to
this list.

FYI, I've just found that the "command -p" was added in Intel's scripts
because of a bug report from a user who defined a function named cd:

  https://posts663.rssing.com/chan-20519098/all_p8.html

("command" would have been sufficient as you said, but there would
be the same problem in zsh).

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)


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

end of thread, other threads:[~2020-10-02 13:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20 11:28 command -p should enable builtins not in path Vincent Lefevre
2020-08-21 13:03 ` Peter Stephenson
2020-08-21 15:49   ` Martijn Dekker
2020-08-21 16:08     ` Peter Stephenson
2020-08-21 16:35       ` Martijn Dekker
2020-08-24 18:30     ` Vincent Lefevre
2020-08-24 19:59       ` Martijn Dekker
2020-10-02 13:17         ` Vincent Lefevre
2020-08-21 15:47 ` Martijn Dekker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).