bug#24471: 25.1.50; Error on empty PATH component

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

In GNU Emacs 25.1 (x86_64 for Windows)

I've updated from Emacs 24.5 to Emacs 25.1 this morning at work.
Starting the new Emacs produced an error during initialization trying to
process (directory-file-name nil).  The reason it turned out was that
our IT department in their infinite wisdom had ended the system PATH
with a ';'.  Emacs extended that path with ';/usr/bin/' (or something
like that, I'm making this report from home), this created an empty path
element that ended up at directory-file-name.  Emacs should sanitize the
PATH if it can't process nil in downstream functions.



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

bug#24471: 25.1.50; Error on empty PATH component

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Mon, 19 Sep 2016 21:06:38 +0200
> 
> I've updated from Emacs 24.5 to Emacs 25.1 this morning at work.
> Starting the new Emacs produced an error during initialization trying to
> process (directory-file-name nil).  The reason it turned out was that
> our IT department in their infinite wisdom had ended the system PATH
> with a ';'.  Emacs extended that path with ';/usr/bin/' (or something
> like that, I'm making this report from home), this created an empty path
> element that ended up at directory-file-name.  Emacs should sanitize the
> PATH if it can't process nil in downstream functions.

Please show the Lisp backtrace from the error, so that the path from
PATH to directory-file-name could be seen.

Thanks.

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

Eli Zaretskii writes:
> Please show the Lisp backtrace from the error, so that the path from
> PATH to directory-file-name could be seen.

Sorry I can't, I have no Windows machine w/ Emacs 25.1 at home.  It
should be reproducible when you add a superfluous ";" at the end of your
PATH.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

DIY Stuff:
http://Synth.Stromeko.net/DIY.html

bug#24471: 25.1.50; Error on empty PATH component

[Noam Postavsky]

tag 24471 unreproducible moreinfo
quit

On Mon, Sep 19, 2016 at 5:41 PM, Achim Gratz <Stromeko@nexgo.de> wrote:
> It
> should be reproducible when you add a superfluous ";" at the end of your
> PATH.

It's not.

(Extra ";" in PATH result in "." in exec-path, which may be wrong but
does not trigger an error.)

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

Noam Postavsky writes:
> (Extra ";" in PATH result in "." in exec-path, which may be wrong but
> does not trigger an error.)

That's another bug, I think, due to folding POSIX and Windows
functionality.  Empty PATH elements mean "." in POSIX, but to the best
of my knowledge, they don't mean anything special on Windows, so they
should be removed or ignored.

Here's the (redacted) backtrace leading to the error I encountered.

--8<---------------cut here---------------start------------->8---
Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  directory-file-name(nil)
  mapcar(directory-file-name ("c:/WINDOWS/SYSTEM32/" "c:/WINDOWS/" "c:/WINDOWS/SYSTEM32/WBEM/" nil "c:/Local_Install/share/emacs-25.1/libexec/emacs/25.1/x86_64-w64-mingw32/"))
  eval((mapcar (quote directory-file-name) (append (parse-colon-path (getenv "PATH")) (list exec-directory))))
  custom-push-theme(theme-value exec-path user set (quote ("/usr/bin")))
  custom-theme-set-variables(user ((exec-path (quote ("/usr/bin")))))
  apply(custom-theme-set-variables user ((exec-path (quote ("/usr/bin")))))
  custom-set-variables((exec-path (quote ("/usr/bin"))))
  eval-buffer(#<buffer  *load*> nil "//server/Gratz/GNU/.emacs" nil t)  ; Reading at buffer position 3839
  load-with-code-conversion("//server/Gratz/GNU/.emacs" "//server/Gratz/GNU/.emacs" t t)
  load("~/.emacs" t t)
  [compiled]
  command-line()
  normal-top-level()

In GNU Emacs 25.1.1 (x86_64-w64-mingw32)
 of 2016-09-17 built on LAPHROAIG
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --without-dbus --without-compress-install CFLAGS=-static'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: DEU
  locale-coding-system: cp1252

Major mode: Debugger

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Loading time...done
Entering debugger...
--8<---------------cut here---------------end--------------->8---



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#24471: 25.1.50; Error on empty PATH component

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Thu, 22 Sep 2016 20:44:00 +0200
> 
> Here's the (redacted) backtrace leading to the error I encountered.
> 
> --8<---------------cut here---------------start------------->8---
> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>   directory-file-name(nil)
>   mapcar(directory-file-name ("c:/WINDOWS/SYSTEM32/" "c:/WINDOWS/" "c:/WINDOWS/SYSTEM32/WBEM/" nil "c:/Local_Install/share/emacs-25.1/libexec/emacs/25.1/x86_64-w64-mingw32/"))
>   eval((mapcar (quote directory-file-name) (append (parse-colon-path (getenv "PATH")) (list exec-directory))))
>   custom-push-theme(theme-value exec-path user set (quote ("/usr/bin")))
>   custom-theme-set-variables(user ((exec-path (quote ("/usr/bin")))))
>   apply(custom-theme-set-variables user ((exec-path (quote ("/usr/bin")))))
>   custom-set-variables((exec-path (quote ("/usr/bin"))))
>   eval-buffer(#<buffer  *load*> nil "//server/Gratz/GNU/.emacs" nil t)  ; Reading at buffer position 3839

Thanks.  Does the patch below fix the problem?

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 001d638..bd2b280 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -174,8 +174,10 @@ minibuffer-prompt-properties--setter
                         nil
                         :standard
                         (mapcar 'directory-file-name
-                                (append (parse-colon-path (getenv "PATH"))
-                                        (list exec-directory))))
+                                (delq nil
+                                      (append
+                                       (parse-colon-path (getenv "PATH"))
+                                       (list exec-directory)))))
 	     (exec-suffixes execute (repeat string))
 	     ;; charset.c
 	     (charset-map-path installation

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

Eli Zaretskii writes:
> Thanks.  Does the patch below fix the problem?

Thanks.  I have no doubt it will, but I can't test it for a few days.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

bug#24471: 25.1.50; Error on empty PATH component

[Noam Postavsky]

found 24471 25.1
tag 24471 = patch
quit

On Thu, Sep 22, 2016 at 3:30 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Achim Gratz <Stromeko@nexgo.de>
>> Date: Thu, 22 Sep 2016 20:44:00 +0200
>>
>> Here's the (redacted) backtrace leading to the error I encountered.
>>
>> --8<---------------cut here---------------start------------->8---
>> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>>   directory-file-name(nil)
>>   mapcar(directory-file-name ("c:/WINDOWS/SYSTEM32/" "c:/WINDOWS/" "c:/WINDOWS/SYSTEM32/WBEM/" nil "c:/Local_Install/share/emacs-25.1/libexec/emacs/25.1/x86_64-w64-mingw32/"))
>>   eval((mapcar (quote directory-file-name) (append (parse-colon-path (getenv "PATH")) (list exec-directory))))
>>   custom-push-theme(theme-value exec-path user set (quote ("/usr/bin")))
>>   custom-theme-set-variables(user ((exec-path (quote ("/usr/bin")))))
>>   apply(custom-theme-set-variables user ((exec-path (quote ("/usr/bin")))))
>>   custom-set-variables((exec-path (quote ("/usr/bin"))))
>>   eval-buffer(#<buffer  *load*> nil "//server/Gratz/GNU/.emacs" nil t)  ; Reading at buffer position 3839
>
> Thanks.  Does the patch below fix the problem?

Ah, the custom :standard value is different from the initial value,
which uses decode_env_path. Is it possible to use the same code for
both? It seems that decode_env_path handles some extra things: file
name encoding, checking for file name handlers.

bug#24471: 25.1.50; Error on empty PATH component

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Thu, 22 Sep 2016 22:10:43 +0200
> 
> Eli Zaretskii writes:
> > Thanks.  Does the patch below fix the problem?
> 
> Thanks.  I have no doubt it will, but I can't test it for a few days.

Thanks, I'll wait.

Another issue is whether we want to filter out nil or leave it alone
(i.e. change the function used by mapcar so it skips nil elements)?
Any thoughts on this one?

bug#24471: 25.1.50; Error on empty PATH component

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 22 Sep 2016 18:28:46 -0400
> Cc: Achim Gratz <Stromeko@nexgo.de>, 24471@debbugs.gnu.org
> 
> Ah, the custom :standard value is different from the initial value,
> which uses decode_env_path. Is it possible to use the same code for
> both? It seems that decode_env_path handles some extra things: file
> name encoding, checking for file name handlers.

File name encoding should not be the problem, because the Lisp getenv
takes care of that already (see make_string).  As for file-name
handlers, do we expect PATH to include such file names?
(decode_env_path is more general, and used not just for PATH.)

bug#24471: 25.1.50; Error on empty PATH component

[Glenn Morris]

Eli Zaretskii wrote:

> Another issue is whether we want to filter out nil or leave it alone
> (i.e. change the function used by mapcar so it skips nil elements)?
> Any thoughts on this one?

Please just replace nil elements with ".".

The point of that code in cus-start is so that exec-path does not show
up in customize-rogue output under normal circumstances. A test case is

PATH=:$PATH emacs -Q -f customize-rogue

bug#24471: 25.1.50; Error on empty PATH component

[Glenn Morris]

Version: 25.2

Glenn Morris wrote:

> Please just replace nil elements with ".".
>
> The point of that code in cus-start is so that exec-path does not show
> up in customize-rogue output under normal circumstances. A test case is
>
> PATH=:$PATH emacs -Q -f customize-rogue

Done.

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

Glenn Morris writes:
> Eli Zaretskii wrote:
>
>> Another issue is whether we want to filter out nil or leave it alone
>> (i.e. change the function used by mapcar so it skips nil elements)?
>> Any thoughts on this one?
>
> Please just replace nil elements with ".".

The code in parse-colon-path mentions that a nil element is supposed to
be treated as "default-directory".  This is wrong at least for PATH and
POSIX, so maybe there should be some more consideration on what exactly
the purpose of nil elements should be.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

Eli Zaretskii writes:
> Thanks.  Does the patch below fix the problem?
>
> diff --git a/lisp/cus-start.el b/lisp/cus-start.el
> index 001d638..bd2b280 100644
> --- a/lisp/cus-start.el
> +++ b/lisp/cus-start.el
> @@ -174,8 +174,10 @@ minibuffer-prompt-properties--setter
>                          nil
>                          :standard
>                          (mapcar 'directory-file-name
> -                                (append (parse-colon-path (getenv "PATH"))
> -                                        (list exec-directory))))
> +                                (delq nil
> +                                      (append
> +                                       (parse-colon-path (getenv "PATH"))
> +                                       (list exec-directory)))))
>  	     (exec-suffixes execute (repeat string))
>  	     ;; charset.c
>  	     (charset-map-path installation

I've tried to patch this in-place, first by recompiling the affected
custom files, then by removing all the .elc files.  I failed, somehow
this eval never changes to include the delq nil introduced with the
patch and I can't seem to find the code path that it takes to get to the
non-changed version.  I can't recompile Emacs on Windows.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

bug#24471: 25.1.50; Error on empty PATH component

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Mon, 17 Oct 2016 18:25:17 +0200
> 
> Eli Zaretskii writes:
> > Thanks.  Does the patch below fix the problem?
> >
> > diff --git a/lisp/cus-start.el b/lisp/cus-start.el
> > index 001d638..bd2b280 100644
> > --- a/lisp/cus-start.el
> > +++ b/lisp/cus-start.el
> > @@ -174,8 +174,10 @@ minibuffer-prompt-properties--setter
> >                          nil
> >                          :standard
> >                          (mapcar 'directory-file-name
> > -                                (append (parse-colon-path (getenv "PATH"))
> > -                                        (list exec-directory))))
> > +                                (delq nil
> > +                                      (append
> > +                                       (parse-colon-path (getenv "PATH"))
> > +                                       (list exec-directory)))))
> >  	     (exec-suffixes execute (repeat string))
> >  	     ;; charset.c
> >  	     (charset-map-path installation
> 
> I've tried to patch this in-place, first by recompiling the affected
> custom files, then by removing all the .elc files.  I failed, somehow
> this eval never changes to include the delq nil introduced with the
> patch and I can't seem to find the code path that it takes to get to the
> non-changed version.  I can't recompile Emacs on Windows.

You must re-dump Emacs for this to take effect.

Anyway, the change that was committed is different, so the above is no
longer pertinent.

(I don't think I understand what you meant by your comment about "nil
meaning default-directory being wrong for PATH and POSIX".)

bug#24471: 25.1.50; Error on empty PATH component

[Achim Gratz]

Eli Zaretskii writes:
> You must re-dump Emacs for this to take effect.

Ah, OK.  I had hoped that re-loading the changed files would do it, but
no dice.

> Anyway, the change that was committed is different, so the above is no
> longer pertinent.
>
> (I don't think I understand what you meant by your comment about "nil
> meaning default-directory being wrong for PATH and POSIX".)

POSIX specifically prescribes that an empty PATH element equals "." and
declares that a legacy feature that strictly conforming applications
shall not use, but in other environment variables an empty path element
is also allowed and replaced by different defaults.  For NLSPATH that
default is %N and for MANPATH it usually means some system-defined
(POSIX doesn't mention that possibility).

Whether default-directory equates "." seems to depend on when it gets
evaluated, since it's normally set to some absolute path.  So a textual
replacement with "." seems more correct than some hand-waving about nil
representing current-directory in the case of PATH.  OTOH, if other
environment variables are intended to get processed by parse-colon-path,
then that replacement would need to be different for each such variable.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

bug#24471: 25.1.50; Error on empty PATH component

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Mon, 17 Oct 2016 20:16:17 +0200
> 
> POSIX specifically prescribes that an empty PATH element equals "." and
> declares that a legacy feature that strictly conforming applications
> shall not use, but in other environment variables an empty path element
> is also allowed and replaced by different defaults.  For NLSPATH that
> default is %N and for MANPATH it usually means some system-defined
> (POSIX doesn't mention that possibility).
> 
> Whether default-directory equates "." seems to depend on when it gets
> evaluated, since it's normally set to some absolute path.  So a textual
> replacement with "." seems more correct than some hand-waving about nil
> representing current-directory in the case of PATH.

I think you are wrong, because you don't realize what is Emacs's
interpretation of "." in exec-path.  The interpretation is exactly
default-directory, AFAIR.  And that is TRT, because Emacs interprets
"." and default-directory as being local to each buffer.  IOW,
conceptually, when you switch to another buffer, you effectively chdir
into its default-directory.

Bottom line, "legacy feature" aside, I think converting an empty PATH
element to "." in exec-path conforms to POSIX, and therefore there's
no issue here left after Glenn pushed his changes.