Re: command_not_found_handle not run when PATH is empty

2023-03-09 Thread Andreas Schwab
On Mär 08 2023, Grisha Levit wrote:

> I think it might make sense to change code that looks at the value of
> PATH to explicitly treat an empty value as `.' so that all such
> behavior is consistent.

But an unset PATH is *not* the same as PATH=.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."



Re: command_not_found_handle not run when PATH is empty

2023-03-09 Thread Robert Elz
Date:Thu, 09 Mar 2023 09:26:18 +0100
From:Andreas Schwab 
Message-ID:  

  | But an unset PATH is *not* the same as PATH=.

That might be true, but POSIX says:

If PATH is unset or is set to null, or if a path prefix in PATH
contains a  character ('%'), the path search is
implementation-defined.

Ignore the "or if" part, but what the rest of that says is that bash
(or any other shell) gets to define where to search (which could be
nowhere, or '.', or some standard path setup) when PATH is unset, or
(as in the case from the OP) when PATH=''

So, if bash wants to define PATH= as being the same as PATH=. that's
just fine - and more than that, is probably the right thing to do as
PATH=:/bin is the same as PATH=.:/bin and PATH=/bin: is the same as
PATH=/bin:. and PATH=/bin::/usr/bin is ...  - an empty component of PATH
is treated as '.' (always has been in all Bourne shells), PATH= is really
just a path with only one component, which is empty.

The case for an unset PATH is not so clear, and either doing no search
at all, or searching a system defined standard command path, is probably
a better choice in those cases, rather than treating it as '.' (two ways
to say '.' is arguably overkill already, three is too much).   But that
case isn't the OP's case, so is irrelevant in the current discussion.

kre

ps: whatever bash decides to do, it should be consistent for all uses of
PATH of course, not different for searching then for completion, or for
"not found" handlers.





Re: command_not_found_handle not run when PATH is empty

2023-03-09 Thread Moshe Looks
Thank you this is very illuminating and makes it clear that my naive
one-line fix would be inappropriate. Given the current state of
affairs there is really no good reason for PATH to ever be unset or
set to empty vs. explicitly set to '.', right? So might be worth
spelling out in the docs that this is inadvisable :).

On Wed, Mar 8, 2023 at 7:31 PM Grisha Levit  wrote:
>
> On Wed, Mar 8, 2023 at 4:28 PM Moshe Looks  wrote:
> > This is very old code and I have no idea what the original intention
> > was in returning 'name' here or if doing so is still performing useful
> > work, but the tests pass at least.
>
> A null $PATH component is treated as the current directory, and bash
> treats an empty $PATH as having only a single null component, so we
> can't just fail to return a command when $PATH is empty.
>
> The current code's behavior of returning the path as provided when
> $PATH is empty accomplishes the goal of "finding" the path in $PATH
> but ignores any flags that would normally limit the results to files
> that exist/are executable/etc.
>
> This leads to weirdness not just with command_not_found_handle but
> with command name completion and `type -a' and `command output as
> well.
>
> $ cd /bin
> $ PATH= compgen -c -X '!bash'
> $ PATH= type -a bash
> -bash: type: bash: not found
>
> I think it might make sense to change code that looks at the value of
> PATH to explicitly treat an empty value as `.' so that all such
> behavior is consistent.
>
> diff --git a/bashline.c b/bashline.c
> index 3238b13a..e0d6107c 100644
> --- a/bashline.c
> +++ b/bashline.c
> @@ -2063,6 +2063,8 @@ command_word_completion_function (const char
> *hint_text, int state)
>   dequoted_hint = bash_dequote_filename (hint, 0);
>
>path = get_string_value ("PATH");
> +  if (path == 0 || *path == '\0')
> + path = ".";
>path_index = dot_in_path = 0;
>
>/* Initialize the variables for each type of command word. */
> diff --git a/findcmd.c b/findcmd.c
> index 186871ac..b413c870 100644
> --- a/findcmd.c
> +++ b/findcmd.c
> @@ -256,13 +256,13 @@ _find_user_command_internal (const char *name, int 
> flags)
>
>/* Search for the value of PATH in both the temporary environments and
>   in the regular list of variables. */
> -  if (var = find_variable_tempenv ("PATH")) /* XXX could be array? */
> -path_list = value_cell (var);
> +  if (var = find_variable_tempenv ("PATH"))
> +path_list = get_variable_value (var);
>else
> -path_list = (char *)NULL;
> +path_list = 0;
>
>if (path_list == 0 || *path_list == '\0')
> -return (savestring (name));
> +path_list = ".";
>
>cmd = find_user_command_in_path (name, path_list, flags, (int *)0);
>
> @@ -363,11 +363,14 @@ search_for_command (const char *pathname, int flags)
>  {
>if (flags & CMDSRCH_STDPATH)
>   path_list = conf_standard_path ();
> -  else if (temp_path || path)
> - path_list = value_cell (path);
> +  else if (path)
> + path_list = get_variable_value (path);
>else
>   path_list = 0;
>
> +  if (path_list == 0 || *path_list == '\0')
> + path_list = ".";
> +
>command = find_user_command_in_path (pathname, path_list,
> FS_EXEC_PREFERRED|FS_NODIRS, &st);
>
>if (command && hashing_enabled && temp_path == 0 && (flags &
> CMDSRCH_HASH))
> @@ -440,7 +443,9 @@ user_command_matches (const char *name, int flags,
> int state)
> if (stat (".", &dotinfo) < 0)
>   dotinfo.st_dev = dotinfo.st_ino = 0; /* so same_file won't match */
> path_list = get_string_value ("PATH");
> -path_index = 0;
> +   if (path_list == 0 || *path_list == '\0')
> + path_list = ".";
> +   path_index = 0;
>   }
>
>while (path_list && path_list[path_index])