bug#17467: 24.3; locate-library returning spurious path

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Hello:

locate-library incorrectly generates a set of suffixes to extend the
base library name (".elc" ".elc.gz" ".el" ".el.gz" "" ".gz"), while it
should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is
nil. This leads to spurious paths found, like name.gz. I found
this issue because (locate-library "tramp") was returning
"/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc". The workaround
is (locate-file "tramp" load-path (get-load-suffixes))

Best,
Alex

[-- Attachment #2: Type: text/html, Size: 791 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Eli Zaretskii]

> From: Alex Kosorukoff <alex@3form.com>
> Date: Sun, 11 May 2014 09:06:10 -0700
> 
> locate-library incorrectly generates a set of suffixes to extend the
> base library name (".elc" ".elc.gz" ".el" ".el.gz" "" ".gz"), while it
> should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is
> nil. This leads to spurious paths found, like name.gz. I found
> this issue because (locate-library "tramp") was returning
> "/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc". The workaround
> is (locate-file "tramp" load-path (get-load-suffixes))

What if I say

  (locate-library "tramp.el")

Shouldn't it be able to find tramp.el.gz then?

bug#17467: 24.3; locate-library returning spurious path

[Glenn Morris]

Alex Kosorukoff wrote:

> I found this issue because (locate-library "tramp") was returning
> "/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc".

I assume there are some typos there...

Anyway, sounds like you have your ~/.emacs.d directory in load-path?
You are strongly encouraged not to do that.
Newer Emacs will in fact warn you about it.
Because you can get problems just like this one! :)

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

I think locate-library has an extra parameter nosuffix, so (locate-library
"tramp.el" 'nosuffix) will find "tramp.el." I guess for backward
compatibility we can set nosuffix to t whenever the name has a valid suffix
already.


On Sun, May 11, 2014 at 10:03 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Alex Kosorukoff <alex@3form.com>
> > Date: Sun, 11 May 2014 09:06:10 -0700
> >
> > locate-library incorrectly generates a set of suffixes to extend the
> > base library name (".elc" ".elc.gz" ".el" ".el.gz" "" ".gz"), while it
> > should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is
> > nil. This leads to spurious paths found, like name.gz. I found
> > this issue because (locate-library "tramp") was returning
> > "/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc". The workaround
> > is (locate-file "tramp" load-path (get-load-suffixes))
>
> What if I say
>
>   (locate-library "tramp.el")
>
> Shouldn't it be able to find tramp.el.gz then?
>

[-- Attachment #2: Type: text/html, Size: 1571 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

You are right, I had my ~/.emacs.d in the load-path. I removed it as my
first step to resolve this issue, but then I thought that even if it were
in the load-path, returning the path to a file without a valid library
extension is an unexpected behavior for locate-library.


On Sun, May 11, 2014 at 10:37 AM, Glenn Morris <rgm@gnu.org> wrote:

> Alex Kosorukoff wrote:
>
> > I found this issue because (locate-library "tramp") was returning
> > "/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc".
>
> I assume there are some typos there...
>
> Anyway, sounds like you have your ~/.emacs.d directory in load-path?
> You are strongly encouraged not to do that.
> Newer Emacs will in fact warn you about it.
> Because you can get problems just like this one! :)
>

[-- Attachment #2: Type: text/html, Size: 1152 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Eli Zaretskii]

> From: Alex Kosorukoff <alex@3form.com>
> Date: Sun, 11 May 2014 10:38:39 -0700
> Cc: 17467@debbugs.gnu.org
> 
> I think locate-library has an extra parameter nosuffix, so (locate-library
> "tramp.el" 'nosuffix) will find "tramp.el." I guess for backward
> compatibility we can set nosuffix to t whenever the name has a valid suffix
> already.

But what about adding ".gz" to it?

In any case, it is very convenient to not have to worry whether
there's already a suffix there.  You cannot always know that when user
input is involved.

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

I am not sure what use case did you mean exactly.

(locate-library "tramp.el.gz" 'nosuffix)  will return the path to
tramp.el.gz as expected

did you mean the following

(locate-library "tramp.el") returning the path to tramp.el.gz?


On Sun, May 11, 2014 at 10:46 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Alex Kosorukoff <alex@3form.com>
> > Date: Sun, 11 May 2014 10:38:39 -0700
> > Cc: 17467@debbugs.gnu.org
> >
> > I think locate-library has an extra parameter nosuffix, so
> (locate-library
> > "tramp.el" 'nosuffix) will find "tramp.el." I guess for backward
> > compatibility we can set nosuffix to t whenever the name has a valid
> suffix
> > already.
>
> But what about adding ".gz" to it?
>
> In any case, it is very convenient to not have to worry whether
> there's already a suffix there.  You cannot always know that when user
> input is involved.
>

[-- Attachment #2: Type: text/html, Size: 1517 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Eli Zaretskii]

> From: Alex Kosorukoff <alex@3form.com>
> Date: Sun, 11 May 2014 10:53:34 -0700
> Cc: 17467@debbugs.gnu.org
> 
> did you mean the following
> 
> (locate-library "tramp.el") returning the path to tramp.el.gz?

Yes.  But also (locate-library "tramp.el") being able to find
tramp.el.

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 3576 bytes --]

Yes, this makes sense. Here is a patch that the issues you mentioned.

(locate-library "tramp.el.gz")
(locate-library "tramp.el")
(locate-library "tramp")

all are working as expected

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: alex@3form.com-20140511184813-t6p0r8ac4em9kuyf
# target_branch: :parent
# testament_sha1: 66b596e6da58c1cb71dedd7fa9ba2fcf20e2964c
# timestamp: 2014-05-11 11:48:22 -0700
# base_revision_id: monnier@iro.umontreal.ca-20140511034953-\
#   1mzcrftziwhrw9hl
#
# Begin patch
=== modified file 'lisp/subr.el'
--- lisp/subr.el        2014-04-09 01:48:07 +0000
+++ lisp/subr.el        2014-05-11 18:48:13 +0000
@@ -1857,10 +1857,13 @@
                                        load-path (get-load-suffixes)))
                     nil nil
                     t))
-  (let ((file (locate-file library
-                          (or path load-path)
-                          (append (unless nosuffix (get-load-suffixes))
-                                  load-file-rep-suffixes))))
+  (let ((file
+         (locate-file library
+                      (or path load-path)
+                      (unless (or nosuffix (string-suffix-p ".el.gz"
library))
+                        (if (string-suffix-p ".el" library)
+                            load-file-rep-suffixes
+                          (get-load-suffixes))))))
     (if interactive-call
        (if file
            (message "Library is file %s" (abbreviate-file-name file))

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWYwVAfIAAv3/gDIwFUBQY//3
cwgAAL////BQBZY8tZs1llc7udAaowkqTTQZAM0gBoxAaaAAAJKajTEap+ijaR6I0AA0AAACRTQk
2iTTyJiTTR6j0jJoNAyYgHMAAAAAAAAAABUogmgJgmJiGmpkm0j1MIaAyTLbLDZxNRGLHnrLAyW9
r/aHw7fgG/B1bYJK8emKqTIKmSSJCycvA8sP7hSOWceg7jgMYw8fyi0yw11PqgkQRnWDExNp4bj8
RqO+pIz8yQ2Fww34eP+zJEGZqnl4bjuROkkiD4Ps7JHW+xjfE2vX85EqNzn/x57Z2nVtP9DoxN6P
Dx6uqxRre9Oq+7Y/xotaU8c1dRpQ6+5KWepxa8+ita+XW4lFLHOLnZNgjZOOdrqoLX0kJBMRZUNh
SPeX3UE4RGEyH9RCEE88XW3iMhYSKhgywGRWVrOSpZC37Qy9RhZCYmLzLgVCpqg1zy5r55azmFoF
9mLE1MZFiWq2u1nfQ0/XIm9OebVuTe48Jgal/BGZspfft7pfRpNz1qmChGCYGXkuXN2wpuNGMkp5
sDLMNpfcDpmwpCrjabGLxRHsWsmWnMvxIVR8k2VMoi7Vd0d4gYjGK1bd4IxiQpzAzHBfUTypURs6
AyNExSsr5mphmGZbkw0tYmDK1XsZbIcc7PlcbcJpzYI0nKtLUaGj2Ymdxb+C4CkZk+YRlaGQ6Zlr
lvpPcUMal5eLIhgDg2rIFXMLm1X0maHF1M9uiOYk5GA8CNBQkbKUl3OoQxewiAoDhV31WkywYsmX
tqFRr5EahVsWkK641lETqtGlOo9OqcEmaQoc1DQnY/y9E913ZerM80YJVYzrGVS9eqLlUovUtMUl
+JRpbHzODoye/fSlaLFHk+zW9UOJ7mVhSiGx+UoNJiaSaD5cIDPgh65gbJdO02V4bYqLt4mjQ2yV
N7gptiiqlf6eHa7WDQjzzosjlN8bKBHAQ/RERDRV9VYo5xQD6PwtQjOBAS0nWYG3G0pR/YEUSfc1
kwhj7CTKQ2GiuILRIo3AqVyGQuxwaY7dCqiMrIimZDyQt7KlhiNdhXwhaIkZVi0docU1mcuhkZ9x
KB7TiW1Kw5FC5Lr9wYbjidMEz8fnmTez3Vezk8RfdjclHhu/lYcE2HvSkl3dwczZ3vYsU2uZTmyC
pGCWvoxbFZxnDbiWK3R0F0VCmwaytPBnFWzFgJUHGY1GknHUEbBFk9XGUWvWGR5+nNydadRj5T5z
SnowbUU3afKKana/CUVL2vob4WG74SaMjF3Na1sH1PgNL0cJofdSTEu0o80cZsdsmpxk7pO8pDIM
io8ZPZ31VyN/3Y0OPNR8lnc8X3uvYVaHQNaGlKqxkd7SclywzFyp1l9wyLlklikzNRgWl7YmZFhi
qlE45zGmiSvR+fEjEPBrF82OsnFmkj2sT8NrVpYmr2TM27xjE6wFxhtqWFuEUb7RYB7BMIShLUFE
ZcHNnN0x+WDJCbytnYuXWukzWKFC1sUKSJLFaW3eWAZVe1p6CZcKqW44HeMqWXK1L8WfaquSxY+R
2LJVoYHitMnNlMJMaKKVWVTsc50S9NdIfFFr4mDau6p8kyptF8zTp3pnTC1eoUQpFEvkXI/dHRPP
Vt6zGnryeP4u5IpwoSEYKgPk



On Sun, May 11, 2014 at 11:10 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Alex Kosorukoff <alex@3form.com>
> > Date: Sun, 11 May 2014 10:53:34 -0700
> > Cc: 17467@debbugs.gnu.org
> >
> > did you mean the following
> >
> > (locate-library "tramp.el") returning the path to tramp.el.gz?
>
> Yes.  But also (locate-library "tramp.el") being able to find
> tramp.el.
>

[-- Attachment #2: Type: text/html, Size: 5028 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> it should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is nil.

Why?
Did you find some documentation indicating that this is how it should work?
Or is it the behavior you'd prefer, and if so, can you explain why you'd
prefer that behavior?  Which concrete cases do you specifically care about?

The current behavior has been in use for *many* years and I expect that
a fair bit of code relies on it, so we'd need a really good reason to
change it.  Maybe we can accommodate your specific concrete cases in
some other way.


        Stefan

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

The issue is that locate-library returns spurious paths like ".*/tramp" or
".*xxx/tramp.gz" instead of returning a valid path to the library or nil if
no matching path is found. This is both unexpected and incorrect given this
function name and spec. It can cause user inconvenience or pose a
security/privacy issue because a random file named "tramp" or "tramp.gz"
placed in some directory of the load-path can be loaded instead of the
standard library without user knowledge. This is why I would prefer to fix
it.

On Sun, May 11, 2014 at 12:50 PM, Stefan Monnier
<monnier@iro.umontreal.ca>wrote:

> > it should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is nil.
>
> Why?
> Did you find some documentation indicating that this is how it should work?
> Or is it the behavior you'd prefer, and if so, can you explain why you'd
> prefer that behavior?  Which concrete cases do you specifically care about?
>

This was the first and simplest way to address the issue above. Eli
Zaretskii made a valid point that it is not consistent with the way this
function worked before and not the most convenient for the user. I agree
with this, so I posted a patch that handles the cases he described, except
that it addresses the issue above.

>
> The current behavior has been in use for *many* years and I expect that
> a fair bit of code relies on it, so we'd need a really good reason to
> change it.  Maybe we can accommodate your specific concrete cases in
> some other way.
>

I understand that the bug was there for many years and many
people implemented workarounds (I did). I don't think this is a valid
reason to keep it though. We just need to be careful to make sure we don't
introduce a regression while fixing it. Unit tests can help.


>
>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 2830 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 5589 bytes --]

Stefan is right that the bug was there for a long time. I would like my
patch be compatible with older versions of emacs that don't have
string-suffix-p, so here is the revised version

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: alex@3form.com-20140511205538-1asyz50g7d3y4lgw
# target_branch: :parent
# testament_sha1: d9ca6f57d0d8486d42ed77a739877a208a34f22e
# timestamp: 2014-05-11 13:55:53 -0700
# base_revision_id: monnier@iro.umontreal.ca-20140511034953-\
#   1mzcrftziwhrw9hl
#
# Begin patch
=== modified file 'lisp/subr.el'
--- lisp/subr.el        2014-04-09 01:48:07 +0000
+++ lisp/subr.el        2014-05-11 20:55:38 +0000
@@ -1857,10 +1857,14 @@
                                        load-path (get-load-suffixes)))
                     nil nil
                     t))
-  (let ((file (locate-file library
-                          (or path load-path)
-                          (append (unless nosuffix (get-load-suffixes))
-                                  load-file-rep-suffixes))))
+  (let ((file
+         (locate-file library
+                      (or path load-path)
+                      (unless (or nosuffix
+                                  (string-match "\\.elc?\\.gz\\'" library))
+                        (if (string-match "\\.elc?\\'" library)
+                            load-file-rep-suffixes
+                          (get-load-suffixes))))))
     (if interactive-call
        (if file
            (message "Library is file %s" (abbreviate-file-name file))

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWfbc99AABHZ/gDIwFUFQ4//3
8wgABL////BgBs96932joSoL3t0q2rG2TpgbVEE2kE9J6jNTyaZNNTagHqAAaaeoOYBNMAmQwABM
EwAAAShU02oAA0AAANAAAAG0qNQNT1MTCaepgmgwBDAI0000EUgRMgalP2mqaZpPVP0U9T0h6jah
kGJo0EVE0aaRgITaJMJoaRieoNAAZMyxwwqXQuSoc5zx7XqLgxjChKpd+GuM3swscixOW2FuYpwb
BJ76XVaN7ooEkNIl8j4H03G7ZaZmUngNpcGMaPziEkCQaXe13WIEZAjrzUVAplMmPv1v9TyDhZk/
TOCTEDcwOWPX5fOPrmzaublzrn8a9huAiuc8PAHYZNSwwjGhSFdSjC5SpYYbMwwpQrOXBEKJ3iZF
xMl+oRaRFCFZtuLh4YuClOwDzJ2cWwwkZCevW+rQM0X3eaEPAlD5B1eGOUYxZmw4cVlwgMkDZ3GZ
9AtqjnMIiLHfetqjn0CpARpIshrEt/05xJEWdXxLOe+NJoQjuMKkKlnUWzmFb+kN1g6CdRNKmoiP
XNismTs0wuRr35bDn4u+xGCEADgWqFbY5VAay3aHG6u7TB4iYIfEL7zJwFZoUJUM3c3+8duzI+Gw
TRDux3mtEOw6Q5pQdWZPmJiOJlrCNKcdm2BaHY/HpUhRgK0QoJtiadSVUybnyQxg9d/ZQTHS+Btw
7CqlMt1Lxhg65OAldu/FyOViFxXEMItpVgQNeBKhvHO8sS5hrOBR0qqutnrT+EWImQ5V23kQTlqx
Fd9ZIWwiyGqDeRU13oRurcJn2BtgIL6CS1uqhvgeodpiWMd+vZDjQTDE0HY5Uw6xPQahmK4du8xy
e2W5sXMjaOdRESRqGFxkVnUTqMqHoxIuLpmP/IdEJHApoJ1apwziupdVOEDpGWxWb7DEuLlM22QQ
HWza0l7xPf4ZYl72QOl+XZuqKiVvBkUiubNTzGuZihbGiGqmcryB2wyacpyqEoJRh29sQJmtZiZt
pFszcI94kc6nnwLmkkNifK9vqQuobBWhKhQLtXdG0hlrVXIqIOQyLL7DDcxheex8xPZKdjEMEytU
kzdrE3WxZsyEoyQhsxQbhLXMO90f2G6WyRYpm24mRjLZxKxYWUQXErpKlSbSTY97XjMdYvvZlTZD
ORCO615GiQTND6RrmmfrsoU7lnoZ3CefiGW4rk+sSgcjiR1pBIiSQSIl8np6nm2uInuxVqAqHwHE
JEX3WHA5m230El7Dobc9lW9l4cvcho8QPqh+IniDxPK2HCHlOtFMzOlPwPMOD0Dwz1a9DjU9BJY5
6Fu/mfa9fCHJF+x+5uqFMjn7XfhnHuG9j3JLFes6IXnEraIXdKYTeJGspva7eg4iYPXVDDsTQIC8
33xlGuvV5sEqFJROfJ8nfWm5hFm3CeftDC7At8jwuQx6fiWIcubjY+8XkVBUCgvxQFK2tK6JPofR
GtSFpRcJMQsVOp9gac33JNjR5jHMwBkAF6FH0LjRk7eT2aVLnZXFKQzqcSa6aDGntmRuUjRktEnl
aYpEbgXl2ntPa+jrU8dyLMNHsYaBmPkjiePIfAzO4h74eQO7ufN0Q7m15CQ20vAho8n6oQYhY6He
PMGoez7BMLgn0dGTqD8w94OT7Xm4P0YATa8hPUJ0DV4iZvRTYT1BBG4G5iL3ofJUtGxmnAcINyKK
RiV4/OVmEEKc04jYhEogyEkyCxDg9zkPa2ZhrCrIe9tUHFqzEqYJR0C0ZBY8EN6tQTihBDreFEMA
I+L8OoATB73QCwNXyE6u8EPlUh9Xg55E3LzE1vD1MMIRaY3OPGzghoJUiPyQxToJATEmpuCquN72
uweLq6XhgqGgSnuatYGAljGDRArjU6IiSlO1ptukSJDJ56MbIvaNyFhNA5vvYbODvIItsG+ODIqJ
Nm/oO5mMna3j7GiYdriN4mpGAiTOSG57XmhYhpBT4CSfgFrwa/WJ6IbkOAPkk2F749yGCFJM2CQR
YDBCaBWJ8xMR8RLx3eg6R4+TvDDZwf/F3JFOFCQ9tz30AA==





On Sun, May 11, 2014 at 1:45 PM, Alex Kosorukoff <alex@3form.com> wrote:

> The issue is that locate-library returns spurious paths like ".*/tramp" or
> ".*xxx/tramp.gz" instead of returning a valid path to the library or nil if
> no matching path is found. This is both unexpected and incorrect given this
> function name and spec. It can cause user inconvenience or pose a
> security/privacy issue because a random file named "tramp" or "tramp.gz"
> placed in some directory of the load-path can be loaded instead of the
> standard library without user knowledge. This is why I would prefer to fix
> it.
>
> On Sun, May 11, 2014 at 12:50 PM, Stefan Monnier <monnier@iro.umontreal.ca
> > wrote:
>
>> > it should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is
>> nil.
>>
>> Why?
>> Did you find some documentation indicating that this is how it should
>> work?
>> Or is it the behavior you'd prefer, and if so, can you explain why you'd
>> prefer that behavior?  Which concrete cases do you specifically care
>> about?
>>
>
> This was the first and simplest way to address the issue above. Eli
> Zaretskii made a valid point that it is not consistent with the way this
> function worked before and not the most convenient for the user. I agree
> with this, so I posted a patch that handles the cases he described, except
> that it addresses the issue above.
>
>>
>> The current behavior has been in use for *many* years and I expect that
>> a fair bit of code relies on it, so we'd need a really good reason to
>> change it.  Maybe we can accommodate your specific concrete cases in
>> some other way.
>>
>
> I understand that the bug was there for many years and many
> people implemented workarounds (I did). I don't think this is a valid
> reason to keep it though. We just need to be careful to make sure we don't
> introduce a regression while fixing it. Unit tests can help.
>
>
>>
>>
>>         Stefan
>>
>
>

[-- Attachment #2: Type: text/html, Size: 7878 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Glenn Morris]

Alex Kosorukoff wrote:

> It can cause user inconvenience or pose a security/privacy issue
> because a random file named "tramp" or "tramp.gz" placed in some
> directory of the load-path can be loaded instead of the standard
> library without user knowledge.

This argument does not fly, because if someone can write a "tramp" file
to a directory in your load-path, they can just as easily write
"tramp.el". Random files should not be being written to your load-path,
and you should not be adding inappropriate directories to that path.
Your immediate problem was having ~/.emacs.d in load-path.

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> The issue is that locate-library returns spurious paths like ".*/tramp" or
> ".*xxx/tramp.gz"

I don't see why these are necessarily spurious.  Please give very
concrete examples, so as to make it crystal clear why they're spurious.

> This is both unexpected and incorrect given this function name and
> spec.

Unexpected to you, obviously, but I'm not convinced it's unexpected in
general (after all, I don't remember other bug-reports in this area) and
definitely not incorrect.  See the docstring of `load':

   Execute a file of Lisp code named FILE.
   First try FILE with `.elc' appended, then try with `.el',
   then try FILE unmodified (the exact suffixes in the exact order are
   determined by `load-suffixes').  Environment variable references in
   [...]

Of course, there's an ambiguity about how the search is performed,
w.r.t. to whether it does:

   (dolist (s suffixes) (dolist (d load-path) ...)))
or
   (dolist (d load-path) (dolist (s suffixes) ...)))

We do the second, so that a compiled file in a later directory does not
override a non-compiled file in an earlier directory.


        Stefan

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

I think you are overlooking something. If I notice a random tramp.el in
some unusual place, I will investigate it right away because I know .el
files can be executed by emacs. I wouldn't do it for a random data file
without extension or a compressed .gz archive unless they have executable
permission for some unknown reason. Data files are created by many
applications and it is concerning me as long as no program I frequently use
will execute them randomly. You can say that data files should never be in
the load-path of emacs and I will agree with you. However, I can see
scenarios when this can happen unintentionally. It would be careless not to
try to add a simple safeguard to prevent this kind of execution.

I did fix the proximal cause already, worked around this function and
patched my emacs, so this bug doesn't affect me in any way now. Now I am
trying hard to fix the root cause. This is why I reported this bug, shared
my patches and addressed all valid concerns that were expressed here, even
those that aren't that important for me personally. The most difficult part
seems to be in persuading developers that this is an issue to be fixed. If
I fail at this, I simply will be less confident in using emacs.



On Sun, May 11, 2014 at 2:19 PM, Glenn Morris <rgm@gnu.org> wrote:

> Alex Kosorukoff wrote:
>
> > It can cause user inconvenience or pose a security/privacy issue
> > because a random file named "tramp" or "tramp.gz" placed in some
> > directory of the load-path can be loaded instead of the standard
> > library without user knowledge.
>
> This argument does not fly, because if someone can write a "tramp" file
> to a directory in your load-path, they can just as easily write
> "tramp.el". Random files should not be being written to your load-path,
> and you should not be adding inappropriate directories to that path.
> Your immediate problem was having ~/.emacs.d in load-path.
>

[-- Attachment #2: Type: text/html, Size: 2383 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> Yes, this makes sense. Here is a patch that the issues you mentioned.

I'm still not sure which situations you want to exclude, so it's hard to
judge whether your patch does do it...

> +         (locate-file library
> +                      (or path load-path)
> +                      (unless (or nosuffix (string-suffix-p ".el.gz" library))

...but special casing ".el.gz" is definitely not a good idea.  Why would
it need special treatment?
It's extremely rare for `library' to end in ".el.gz" here.


        Stefan

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 2290 bytes --]

On Sun, May 11, 2014 at 2:56 PM, Stefan Monnier <monnier@iro.umontreal.ca>wrote:

> > The issue is that locate-library returns spurious paths like ".*/tramp"
> or
> > ".*xxx/tramp.gz"
>
> I don't see why these are necessarily spurious.  Please give very
> concrete examples, so as to make it crystal clear why they're spurious.
>

I think these file names are more appropriate for data files, not
executable ones. It is undesirable that a name "tramp.gz" will shadow a
valid library file "tramp.elc" that won't be found as a result. When you
say those names aren't spurious, do you have a particular  example of an
emacs elisp library in mind which file name ends with a suffix other than
.el .elc .el.gz .elc.gz? I think the main difference is that I assume that
this list is exhaustive and you imply that it is not. You can prove me
wrong by a single example.

> This is both unexpected and incorrect given this function name and
> > spec.
>
> Unexpected to you, obviously, but I'm not convinced it's unexpected in
> general (after all, I don't remember other bug-reports in this area) and
> definitely not incorrect.  See the docstring of `load':
>
>    Execute a file of Lisp code named FILE.
>    First try FILE with `.elc' appended, then try with `.el',
>    then try FILE unmodified (the exact suffixes in the exact order are
>    determined by `load-suffixes').  Environment variable references in
>    [...]
>

By incorrect, I only meant that the function fails to do what its name
suggests when it returns something other than a library. My understanding
is that load is used to load any files, not just libraries.


> Of course, there's an ambiguity about how the search is performed,
> w.r.t. to whether it does:
>
>    (dolist (s suffixes) (dolist (d load-path) ...)))
> or
>    (dolist (d load-path) (dolist (s suffixes) ...)))
>
> We do the second, so that a compiled file in a later directory does not
> override a non-compiled file in an earlier directory.
>

Yes, I noticed that require won't attempt to load files like "trump" or
"trump.gz" even if they are in the load-path, unlike load that will try to
load any file regardless of its suffix. My understanding was that require
is used to load libraries, while load is used to load general files.


>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 3412 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Glenn Morris]

Alex Kosorukoff wrote:

> I think these file names are more appropriate for data files, not
> executable ones. It is undesirable that a name "tramp.gz" will shadow a
> valid library file "tramp.elc" that won't be found as a result. When you
> say those names aren't spurious, do you have a particular  example of an
> emacs elisp library in mind which file name ends with a suffix other than
> .el .elc .el.gz .elc.gz? I think the main difference is that I assume that
> this list is exhaustive and you imply that it is not. You can prove me
> wrong by a single example.

I've somewhat lost track of exactly what you want an example of, but:

       When Gnus starts, it will read the `gnus-site-init-file'
    (`.../site-lisp/gnus-init' by default) and `gnus-init-file' (`~/.gnus'
    by default) files.  These are normal Emacs Lisp files and can be used
    to avoid cluttering your `~/.emacs' and `site-init' files with Gnus
    stuff.  Gnus will also check for files with the same names as these,
    but with `.elc' and `.el' suffixes.  In other words, if you have set
    `gnus-init-file' to `~/.gnus', it will look for `~/.gnus.elc',
    `~/.gnus.el', and finally `~/.gnus' (in this order).

and it uses locate-library to do that.

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

On Sun, May 11, 2014 at 3:55 PM, Stefan Monnier <monnier@iro.umontreal.ca>wrote:

> > Yes, this makes sense. Here is a patch that the issues you mentioned.
>
> I'm still not sure which situations you want to exclude, so it's hard to
> judge whether your patch does do it...


I exclude anything that is not ending with .el, .elc, .el.gz, or .elc.gz,
for example, my patch won't return any files that have no extension and
will not return files that have only .gz that is not preceded by el or elc.
Otherwise, the latest patch works the same way as the original
locate-library

> +         (locate-file library
> > +                      (or path load-path)
> > +                      (unless (or nosuffix (string-suffix-p ".el.gz"
> library))
>
> ...but special casing ".el.gz" is definitely not a good idea.  Why would
> it need special treatment?
> It's extremely rare for `library' to end in ".el.gz" here.
>

This is because if a user specified .el.gz already, we shouldn't try to
extend it by appending extra suffixes, e.g. looking for .el.gz.el,
el.gz.elc or .el.gz.gz, none of those suffixes can't possibly result in
valid library names. If a user specified only .el or .elc, then we still
can have two options for suffixes ("", ".gz"). If none of the known
suffixes were specified, we have four options produced by
 (get-load-suffixes). BTW, this was not the last patch, it won't apply to
older emacs versions and also not handling .elc.gz (which should not be
extended just like .el.gz). My last patch is using string-match.


>
>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 2625 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]

Thank you for the example. You are right, gnus-start.el is using
locate-library to check existence of its init files and uses load to search
for them again right after. Given how that code is written, we probably
should keep locate-library as is since at least some people people are
relying on its ability to locate arbitrary files that are not libraries.


On Sun, May 11, 2014 at 5:32 PM, Glenn Morris <rgm@gnu.org> wrote:

> Alex Kosorukoff wrote:
>
> > I think these file names are more appropriate for data files, not
> > executable ones. It is undesirable that a name "tramp.gz" will shadow a
> > valid library file "tramp.elc" that won't be found as a result. When you
> > say those names aren't spurious, do you have a particular  example of an
> > emacs elisp library in mind which file name ends with a suffix other than
> > .el .elc .el.gz .elc.gz? I think the main difference is that I assume
> that
> > this list is exhaustive and you imply that it is not. You can prove me
> > wrong by a single example.
>
> I've somewhat lost track of exactly what you want an example of, but:
>
>        When Gnus starts, it will read the `gnus-site-init-file'
>     (`.../site-lisp/gnus-init' by default) and `gnus-init-file' (`~/.gnus'
>     by default) files.  These are normal Emacs Lisp files and can be used
>     to avoid cluttering your `~/.emacs' and `site-init' files with Gnus
>     stuff.  Gnus will also check for files with the same names as these,
>     but with `.elc' and `.el' suffixes.  In other words, if you have set
>     `gnus-init-file' to `~/.gnus', it will look for `~/.gnus.elc',
>     `~/.gnus.el', and finally `~/.gnus' (in this order).
>
> and it uses locate-library to do that.
>

[-- Attachment #2: Type: text/html, Size: 2227 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 2127 bytes --]

I am wondering whether we can deprecate the usage of this function in ways
other than locating libraries? In the case of gnus the call to
locate-library can be simply removed assuming the second parameter of load
is set to t. Gnus will start faster without this redundant load-path
traversal.


On Sun, May 11, 2014 at 6:35 PM, Alex Kosorukoff <alex@3form.com> wrote:

> Thank you for the example. You are right, gnus-start.el is using
> locate-library to check existence of its init files and uses load to search
> for them again right after. Given how that code is written, we probably
> should keep locate-library as is since at least some people people are
> relying on its ability to locate arbitrary files that are not libraries.
>
>
> On Sun, May 11, 2014 at 5:32 PM, Glenn Morris <rgm@gnu.org> wrote:
>
>> Alex Kosorukoff wrote:
>>
>> > I think these file names are more appropriate for data files, not
>> > executable ones. It is undesirable that a name "tramp.gz" will shadow a
>> > valid library file "tramp.elc" that won't be found as a result. When you
>> > say those names aren't spurious, do you have a particular  example of an
>> > emacs elisp library in mind which file name ends with a suffix other
>> than
>> > .el .elc .el.gz .elc.gz? I think the main difference is that I assume
>> that
>> > this list is exhaustive and you imply that it is not. You can prove me
>> > wrong by a single example.
>>
>> I've somewhat lost track of exactly what you want an example of, but:
>>
>>        When Gnus starts, it will read the `gnus-site-init-file'
>>     (`.../site-lisp/gnus-init' by default) and `gnus-init-file' (`~/.gnus'
>>     by default) files.  These are normal Emacs Lisp files and can be used
>>     to avoid cluttering your `~/.emacs' and `site-init' files with Gnus
>>     stuff.  Gnus will also check for files with the same names as these,
>>     but with `.elc' and `.el' suffixes.  In other words, if you have set
>>     `gnus-init-file' to `~/.gnus', it will look for `~/.gnus.elc',
>>     `~/.gnus.el', and finally `~/.gnus' (in this order).
>>
>> and it uses locate-library to do that.
>>
>
>

[-- Attachment #2: Type: text/html, Size: 2939 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> I think these file names are more appropriate for data files, not
> executable ones.  It is undesirable that a name "tramp.gz" will shadow a
> valid library file "tramp.elc" that won't be found as a result.

I think I'm beginning to see what you mean.  So far we have simply
considered "if it hurts, don't do it".  And it worked well enough.

> When you say those names aren't spurious, do you have a particular
> example of an emacs elisp library in mind which file name ends with
> a suffix other than .el .elc .el.gz .elc.gz?

There are a few (~/.emacs being the most obvious), but admittedly,
I think they all share the property of not being searched for in
load-path.  So we could probably strengthen the search along the lines
you suggest without (hopefully) breaking existing code with a hack along
the lines of the one below.


        Stefan


=== modified file 'lisp/subr.el'
--- lisp/subr.el	2014-04-15 17:03:15 +0000
+++ lisp/subr.el	2014-05-12 02:15:04 +0000
@@ -1878,10 +1878,15 @@
                                        load-path (get-load-suffixes)))
 		     nil nil
 		     t))
-  (let ((file (locate-file library
-			   (or path load-path)
-			   (append (unless nosuffix (get-load-suffixes))
-				   load-file-rep-suffixes))))
+  (let* ((suffixes
+          (nconc (unless nosuffix (get-load-suffixes))
+                 (when (or (file-name-absolute-p library)
+                           ;; (load "foo.el") should find /bar/foo.el.gz,
+                           ;; but (load "foo") should not find /bar/foo.gz.
+                           (string-match "\\.el\\(\\.[[:alnum:]]+\\)?"
+                                         library))
+                   load-file-rep-suffixes)))
+         (file (locate-file library (or path load-path) suffixes)))
     (if interactive-call
 	(if file
 	    (message "Library is file %s" (abbreviate-file-name file))

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 6157 bytes --]

I like the idea of optimizing out the second string-match, though that
variant matched tramp.el.old which is not a valid library name. Here is a
modifed version using the same idea except it skips files like tramp.el.old.

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: alex@3form.com-20140512042218-zjdg3v68bbja2rj2
# target_branch: :parent
# testament_sha1: 330ab5b4527e49ea46d8d16a6d47e5822247ce77
# timestamp: 2014-05-11 21:23:50 -0700
# base_revision_id: monnier@iro.umontreal.ca-20140511034953-\
#   1mzcrftziwhrw9hl
#
# Begin patch
=== modified file 'lisp/subr.el'
--- lisp/subr.el        2014-04-09 01:48:07 +0000
+++ lisp/subr.el        2014-05-12 04:22:18 +0000
@@ -1857,10 +1857,14 @@
                                        load-path (get-load-suffixes)))
                     nil nil
                     t))
-  (let ((file (locate-file library
-                          (or path load-path)
-                          (append (unless nosuffix (get-load-suffixes))
-                                  load-file-rep-suffixes))))
+  (let ((file
+         (locate-file
+          library
+          (or path load-path)
+          (unless nosuffix
+            (if (string-match "\\.elc?\\(\\.gz\\)?\\'" library)
+                (if (= 2 (length (match-data))) load-file-rep-suffixes)
+              (get-load-suffixes))))))
     (if interactive-call
        (if file
            (message "Library is file %s" (abbreviate-file-name file))

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWdDpudEABgd/gDN0FUFQ4//3
8wgABL////BgCI+FAA0AAAAAAAAMkk2p6mmgBoDQaADTQAABoGlQZPSMmynpqDJo0AaZADTIyABz
CYBMAJhMJpgAAEyaaBjmEwCYATCYTTAAAJk00DHMJgEwAmEwmmAAATJpoGCpRAJoATEDRDQCYJT1
NNqY1NkylAzQWMD1e2laABxxxTQHTYJXxEfUiG4Q6b22N1Uag1frP3pbf2aB7XuzlcK84k5F8VNn
K7C8uNBohJUOdK//P1n8bZy3bNmOF6+X2+h/c3FChQf7/zDTaaSkcqN+uu/7XlheXcb+7RLjabTu
P27z/NnCO/mwLDn/FKKlFBiUHHTs3fk/3T+uRYAsB4BdK8urI8LfFgkLYyS2WyCO5UQmtUUEMWlG
zCpPr9qvxRVZXznh5+Va2K/lUqoc7pu4dvX+RkjytniPmaD/hD4GJ5o2+573zPmWFJ/EuPsT5sF7
mXta2bUb099NVld5uEfL5kp0ltH3PA47Nta1rWta1+Px8PlTbiJYcq/XF1wJvdameWVgilnVjdWk
xvQv+zpoWXSQuRejL+HbVarMLMpf26a1ywJSizNrtJZC9haxswi4/Wmd6OCO9G+wsVHDcVhaWztM
teiJdp29o3eo03xKEoRxYpFxebbktJwu36bqr+zRhdy4bDkjST9kyybzSIsJara5G044Zf3G7Zwf
rwTPYjoTrna5aCap2U6SZDiYMOiNqNrDdxpZZz4Z8+2YtFV/bmf8WdlHKytPxSy4msoI0prJRJhc
m7v5JxU8GCzQ0GGWom/Cps09tsEupZqmGlKuiquk7JkArLJwBSQA4Yjggswlu0AkkXDE0r6pv4WT
NbwXqs+xr7MNSxGqmxhJqL64VoyLuN91Nd+ei223D9hFiPYm5GzDDKUpLK8M9QjffUmGmmBLkzrf
Teowa7yU3XWYyOXibKFKFGNqK3NEpa30HEZNbNgs3adVJnkuRozdqNRu0YeNX6IsGAizX10tm062
79MxxRqpvW5o32KotcDbqb11VlyODLRe1f2K2NTJotYfyT0JWXreaOGSzTfZOK9WyxoZ25Uo3b54
W4drBjaorX+kyXcGtqDsNFpQacOWlru+SPl62/bMmubsquHfsus02LF3ZfoqXNFYbjFJ5Iu0sW3D
JUlLM0USi9Rwkb7tSjxpuNunLnlfYjORmX18fCsi/GccJvbjQzWGPA219SK604WzHpdymktuJ2rt
3DabbSZ6GmLmdIXZs4t2zTbZjOtd/DJdjctlDeLWWu9sxuUWYV1vdPij3W5YFZNsm7EKlk1lLDMp
eXlSWlSULylowiX4FDcdD/senkaPEo+PhSlKUpWhYUPE/Y4HL7UfQR5nka00UpIpQp/FbDafVH7v
SlNcrw3nAyg/HvsHSm47lLO24PV39/2S71+7Q8dczcOrDyVvyR9/nJluXcDxiWnqdqnhFFVKxRVS
sUVUr+57e47jE2I+WwiyRYj0jXQ2JcVPjg5O5h320596L/cv73bXVdYlGhRlOa7u8ZPzJ0Ocj6E+
qPeHBzer5Y6eV99k8rrNS6RvYbqccMPyn0OU9ibOeezo1XPzRXPDn0Ydvmeo7+yhuEcD2zC6TLc7
vaadm+vVGor0ncou2w8MdBNbmuNBM+/LplXbXKdGC2i7fLtvknrGtGo77yZ+MnclEya8eKvGmv64
XfYorZcurZXPLLuPQtuk0lKmBnEw0Ph7i9su2Mu967yUy8ftNzAnTuNvHG7Tcfc09T1pL8d2V55P
Syw9ffws/YoM5vRu5j4Eokx8fA8jt8z8CwpkeYp5zQFSRiS0/NgdCs9NnnPHt9S73188uMxY4PB4
+BLbu7TjqW4W+NmyaRr1EsehjuHwkn1+H0KT5H4NMk920RZHQ8JQtjinJHwg0zsu8Uek5TrFI9lP
YGnrPfOBOpic0U7uGqRTWcz+iUKl5vOwdILBz+MTXojDuN5acA/Q+AbT2nSaz+SkjAu0p7E3IwTq
jvjicomB3yO6J1KQaQ0lZDzifR1qV6ZlP5MhHuR/54+RSPwXeB6z+bpfNCptPYQ4iN6KlYvl9TQe
ZvHUwLDSXFR7ZfcGkuLIlhRM8zgZC4wOwl5FhhUlCddRkTZEr7D8vAkYB1N4vjge2J4GmCfWwn9H
E3bWBufVGk49ChQlF6wYGfK80E5ouUo9CZp7EULIlkk2FxGbE8DUOUy9mMfNM4idCy3aXl1p7JqW
FIUFpxKF1SsajXSlFKUpkaAoXz20lKSlJTEtMCXxOkeo/sULzRN0oUv1Ixp2FZciwsPuK7CiKmsx
HmeVwz6mkZRNAUilSypNp1nqJeThSSfRFp9DE7C72IvT7kzJ2BtjGUjVPX5E1k0WmBRKCKQoTCSL
kfojNHrRqRn90b6uz0mmNfbO9Nh/+LuSKcKEhodNzog=



On Sun, May 11, 2014 at 7:18 PM, Stefan Monnier <monnier@iro.umontreal.ca>wrote:

> > I think these file names are more appropriate for data files, not
> > executable ones.  It is undesirable that a name "tramp.gz" will shadow a
> > valid library file "tramp.elc" that won't be found as a result.
>
> I think I'm beginning to see what you mean.  So far we have simply
> considered "if it hurts, don't do it".  And it worked well enough.
>
> > When you say those names aren't spurious, do you have a particular
> > example of an emacs elisp library in mind which file name ends with
> > a suffix other than .el .elc .el.gz .elc.gz?
>
> There are a few (~/.emacs being the most obvious), but admittedly,
> I think they all share the property of not being searched for in
> load-path.  So we could probably strengthen the search along the lines
> you suggest without (hopefully) breaking existing code with a hack along
> the lines of the one below.
>
>
>         Stefan
>
>
> === modified file 'lisp/subr.el'
> --- lisp/subr.el        2014-04-15 17:03:15 +0000
> +++ lisp/subr.el        2014-05-12 02:15:04 +0000
> @@ -1878,10 +1878,15 @@
>                                         load-path (get-load-suffixes)))
>                      nil nil
>                      t))
> -  (let ((file (locate-file library
> -                          (or path load-path)
> -                          (append (unless nosuffix (get-load-suffixes))
> -                                  load-file-rep-suffixes))))
> +  (let* ((suffixes
> +          (nconc (unless nosuffix (get-load-suffixes))
> +                 (when (or (file-name-absolute-p library)
> +                           ;; (load "foo.el") should find /bar/foo.el.gz,
> +                           ;; but (load "foo") should not find
> /bar/foo.gz.
> +                           (string-match "\\.el\\(\\.[[:alnum:]]+\\)?"
> +                                         library))
> +                   load-file-rep-suffixes)))
> +         (file (locate-file library (or path load-path) suffixes)))
>      (if interactive-call
>         (if file
>             (message "Library is file %s" (abbreviate-file-name file))
>
>

[-- Attachment #2: Type: text/html, Size: 7997 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> variant matched tramp.el.old which is not a valid library name.

Who cares?  The point is that if the user asks to load foo.el.old, we
should consider load-file-rep-suffixes, whereas for "foo" we shouldn't.
I'm not particularly worried about finding files with name "foo.el.old.el".

> +          (unless nosuffix
> +            (if (string-match "\\.elc?\\(\\.gz\\)?\\'" library)

I don't want to hardcode "gz" here.  We have load-file-rep-suffixes for that.

> +                (if (= 2 (length (match-data))) load-file-rep-suffixes)
> +              (get-load-suffixes))))))

If you only use (get-load-suffixes) that will fail when we (load "~/.gnus").
My check for absolute-file-name-p was not an optimization.


        Stefan

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

On Sun, May 11, 2014 at 11:39 PM, Stefan Monnier
<monnier@iro.umontreal.ca>wrote:

> > variant matched tramp.el.old which is not a valid library name.
>
> Who cares?  The point is that if the user asks to load foo.el.old, we
> should consider load-file-rep-suffixes, whereas for "foo" we shouldn't.
> I'm not particularly worried about finding files with name "foo.el.old.el".


Do you mean we need this shortcut due to some performance considerations? I
am not sure why else we would want a partial solution when we can have a
complete one. In my opinion we should only consider load-file-rep-suffixes
if the name matches \\.elc?\\' (the difference is the end of string
marker), so foo.el and foo.el.old.el are both ok to extend with .gz, but
foo.el.old and foo shouldn't be extended. Am I missing something?

> +          (unless nosuffix
> > +            (if (string-match "\\.elc?\\(\\.gz\\)?\\'" library)
>
> I don't want to hardcode "gz" here.  We have load-file-rep-suffixes for
> that.
>

Yes, you are right, .gz shouldn't be hardcoded. Though I am not sure if
allowing anything there is ok. Maybe we can just use (sring-match (concat
"\\.elc?\\(" (regexp_opt load-file-rep-suffixes) "\\)\\'") library)?


>
> > +                (if (= 2 (length (match-data))) load-file-rep-suffixes)
> > +              (get-load-suffixes))))))
>
> If you only use (get-load-suffixes) that will fail when we (load
> "~/.gnus").
> My check for absolute-file-name-p was not an optimization.
>

Got it.

        Stefan
>

[-- Attachment #2: Type: text/html, Size: 2851 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> locate-library incorrectly generates a set of suffixes to extend the
> base library name (".elc" ".elc.gz" ".el" ".el.gz" "" ".gz"), while it
> should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is
> nil.

FWIW, this simply reflects what `load' does, so changing it in
`locate-library' would mean that it doesn't do what `load' does, which
I would count as a bug.

The main issue I see is that `load' includes a `must-suffix' argument
which provides the behavior you're looking for (and which is used by
`require') whereas locate-library doesn't provide it.

> This leads to spurious paths found, like name.gz. I found
> this issue because (locate-library "tramp") was returning
> "/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc". The workaround
> is (locate-file "tramp" load-path (get-load-suffixes))

IIUC the problem you had was with `load' rather than with
`locate-library', so I think what this boils down to is that the `load'
that looks for `trump' should be changed to provide `must-suffix'.
WDYT?


        Stefan

bug#17467: 24.3; locate-library returning spurious path

[Alex Kosorukoff]

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Thu, May 15, 2014 at 12:39 PM, Stefan Monnier
<monnier@iro.umontreal.ca>wrote:

> > locate-library incorrectly generates a set of suffixes to extend the
> > base library name (".elc" ".elc.gz" ".el" ".el.gz" "" ".gz"), while it
> > should be just (".elc" ".elc.gz" ".el" ".el.gz") when nosuffix is
> > nil.
>
> FWIW, this simply reflects what `load' does, so changing it in
> `locate-library' would mean that it doesn't do what `load' does, which
> I would count as a bug.
>

I agree with you that locate library should do what load does.


> The main issue I see is that `load' includes a `must-suffix' argument
> which provides the behavior you're looking for (and which is used by
> `require') whereas locate-library doesn't provide it.
>
> > This leads to spurious paths found, like name.gz. I found
> > this issue because (locate-library "tramp") was returning
> > "/home/alex/.emacs.d/trump" not "../lisp/net/trum.elc". The workaround
> > is (locate-file "tramp" load-path (get-load-suffixes))
>
> IIUC the problem you had was with `load' rather than with
> `locate-library', so I think what this boils down to is that the `load'
> that looks for `trump' should be changed to provide `must-suffix'.
> WDYT?
>

In fact, I was looking for a function that would locate library and return
the path to it, so I could compile the explicit path into .elc file that
will avoid searching load-path and save time when it is run. locate-library
seems like a perfect tool for this purpose, but turned out to be not as
useful as it sounds because it is not currently capable of correctly
reproducing search behavior of load or require. As a result, I switched to
locate-file. This currently seems to be the only reliable way to find
correct paths to libraries. I think we could make locate-library more
useful by extending it in one of two possible ways. It can either accept
symbol argument as well as string and behave exactly like require does in
this case (currently it will just fail with error if given a symbol). An
alternative way is to add an optional must-suffix argument to make it
consistent with load. Both ways will keep it backward compatible and will
allow it to emulate the logic of both load and require.


>
>
>         Stefan
>

[-- Attachment #2: Type: text/html, Size: 3296 bytes --]

bug#17467: 24.3; locate-library returning spurious path

[Lars Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> There are a few (~/.emacs being the most obvious), but admittedly,
> I think they all share the property of not being searched for in
> load-path.  So we could probably strengthen the search along the lines
> you suggest without (hopefully) breaking existing code with a hack along
> the lines of the one below.

I've respun the patch for Emacs 28.  It sounds reasonable to me, but the
use case isn't really compelling.  It breaks the
so-long-tests-commentary test, which basically does:

(finder-commentary "so-long")

So I'm not sure whether it makes sense to proceed with this change...

diff --git a/lisp/subr.el b/lisp/subr.el
index a58a873a33..6eb2f61eb0 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2302,10 +2302,15 @@ locate-library
 string.  When run interactively, the argument INTERACTIVE-CALL is t,
 and the file name is displayed in the echo area."
   (interactive (list (read-library-name) nil nil t))
-  (let ((file (locate-file library
-			   (or path load-path)
-			   (append (unless nosuffix (get-load-suffixes))
-				   load-file-rep-suffixes))))
+  (let* ((suffixes
+          (nconc (unless nosuffix (get-load-suffixes))
+                 (when (or (file-name-absolute-p library)
+                           ;; (load "foo.el") should find /bar/foo.el.gz,
+                           ;; but (load "foo") should not find /bar/foo.gz.
+                           (string-match "\\.el\\(\\.[[:alnum:]]+\\)?"
+                                         library))
+                   load-file-rep-suffixes)))
+         (file (locate-file library (or path load-path) suffixes)))
     (if interactive-call
 	(if file
 	    (message "Library is file %s" (abbreviate-file-name file))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#17467: 24.3; locate-library returning spurious path

[Stefan Monnier]

> I've respun the patch for Emacs 28.  It sounds reasonable to me, but the
> use case isn't really compelling.  It breaks the
> so-long-tests-commentary test, which basically does:
>
> (finder-commentary "so-long")

Why does it fail?


        Stefan

bug#17467: 24.3; locate-library returning spurious path

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I've respun the patch for Emacs 28.  It sounds reasonable to me, but the
>> use case isn't really compelling.  It breaks the
>> so-long-tests-commentary test, which basically does:
>>
>> (finder-commentary "so-long")
>
> Why does it fail?

I didn't really investigate -- I thought if we weren't going to do this
after all, then there wasn't much point spending time on it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#17467: 24.3; locate-library returning spurious path

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So I'm not sure whether it makes sense to proceed with this change...

This was six weeks ago, and it didn't seem like there was much
enthusiasm for this change, so I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no