bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

emacs -Q
M-x byte-compile-file <RET> ~/.emacs.d/init.el <RET>
C-x C-c
emacs
;;; init.el is now native-compiled
C-h v user-init-file <RET>
;;; => "d:/mypath/Home/.emacs.d/init.el"
C-x C-c
emacs
C-h v user-init-file <RET>
;;; =>
"d:/mypath/Home/.emacs.d/native/29.0.50-a4a10996/init-7890dda9-d677a456.eln"

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Thu, 17 Nov 2022 10:28:44 +0100
> 
> emacs -Q
> M-x byte-compile-file <RET> ~/.emacs.d/init.el <RET>
> C-x C-c
> emacs
> ;;; init.el is now native-compiled
> C-h v user-init-file <RET>
> ;;; => "d:/mypath/Home/.emacs.d/init.el"
> C-x C-c
> emacs
> C-h v user-init-file <RET>
> ;;; => "d:/mypath/Home/.emacs.d/native/29.0.50-a4a10996/init-7890dda9-d677a456.eln"

After the last step, what does the below produce:

  M-: (gethash "init-7890dda9-d677a456.eln" comp-eln-to-el-h) RET

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

M-: (gethash "init-7890dda9-d677a456.eln" comp-eln-to-el-h) RET

=> "d:/mypath/Home/.emacs.d/init.el"

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Thu, 17 Nov 2022 14:30:27 +0100
> Cc: 59334@debbugs.gnu.org
> 
> M-: (gethash "init-7890dda9-d677a456.eln" comp-eln-to-el-h) RET
> 
> => "d:/mypath/Home/.emacs.d/init.el"

OK, thanks.  So using this, can you propose a patch for
startup--load-user-init-file, where it figures out the name of
user-init-file?  It happens in this fragment:

            ;; If we loaded a compiled file, set `user-init-file' to
            ;; the source version if that exists.
            (when (equal (file-name-extension user-init-file)
                         "elc")
              (let* ((source (file-name-sans-extension user-init-file))
                     (alt (concat source ".el")))
                (setq source (cond ((file-exists-p alt) alt)
                                   ((file-exists-p source) source)
                                   (t nil)))
                (when source
                  (when (file-newer-than-file-p source user-init-file)
                    (message "Warning: %s is newer than %s"
                             source user-init-file)
                    (sit-for 1))
                  (setq user-init-file source))))

Note that the gethash trick I suggested needs a _basename_ of the .eln
file, without any leading directories.

(It is best if you propose a patch because it should be easy for you
to test it.)

Andrea, do you agree with this solution?

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juanma Barranquero <lekktu@gmail.com>
>> Date: Thu, 17 Nov 2022 14:30:27 +0100
>> Cc: 59334@debbugs.gnu.org
>> 
>> M-: (gethash "init-7890dda9-d677a456.eln" comp-eln-to-el-h) RET
>> 
>> => "d:/mypath/Home/.emacs.d/init.el"
>
> OK, thanks.  So using this, can you propose a patch for
> startup--load-user-init-file, where it figures out the name of
> user-init-file?  It happens in this fragment:
>
>             ;; If we loaded a compiled file, set `user-init-file' to
>             ;; the source version if that exists.
>             (when (equal (file-name-extension user-init-file)
>                          "elc")
>               (let* ((source (file-name-sans-extension user-init-file))
>                      (alt (concat source ".el")))
>                 (setq source (cond ((file-exists-p alt) alt)
>                                    ((file-exists-p source) source)
>                                    (t nil)))
>                 (when source
>                   (when (file-newer-than-file-p source user-init-file)
>                     (message "Warning: %s is newer than %s"
>                              source user-init-file)
>                     (sit-for 1))
>                   (setq user-init-file source))))
>
> Note that the gethash trick I suggested needs a _basename_ of the .eln
> file, without any leading directories.
>
> (It is best if you propose a patch because it should be easy for you
> to test it.)
>
> Andrea, do you agree with this solution?

Hi Eli,

yes, I'd have suggested the same.

  Andrea

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Juanma Barranquero <lekktu@gmail.com>, 59334@debbugs.gnu.org
> Date: Thu, 17 Nov 2022 20:04:42 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Andrea, do you agree with this solution?
> 
> Hi Eli,
> 
> yes, I'd have suggested the same.

Great, thanks.

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

This fix works. It assumes (as the original code does) that the source will
either be an .el file, or extensionless (though when an .eln was loaded,
the source will always be .el), so the following code will check for
staleness of the .elc as usual.

In other words, with this patch, it will still warn if the source file is
newer than the .elc (as it does now Emacs), regardless of whether the init
file was loaded from an .elc or .eln. (The age of the .eln is irrelevant
here, that's for the native compiler to check and act upon.)


diff --git i/lisp/startup.el w/lisp/startup.el
index 70267fc857..62984426e5 100644
--- i/lisp/startup.el
+++ w/lisp/startup.el
@@ -1064,7 +1064,11 @@ startup--load-user-init-file
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
-            (when (equal (file-name-extension user-init-file)
-                         "elc")
-              (let* ((source (file-name-sans-extension user-init-file))
+            (when (member (file-name-extension user-init-file)
+                          '("elc" "eln"))
+              (let* ((source (file-name-sans-extension
+                              (or (gethash
+                                   (file-name-nondirectory user-init-file)
+                                   comp-eln-to-el-h)
+                                  user-init-file)))
                      (alt (concat source ".el")))
                 (setq source (cond ((file-exists-p alt) alt)


Testing this patch allowed me to discover another bug:

If you use ~/.emacs instead of ~.emacs.d/init.el, and happen to compile
.emacs, Emacs loads .emacs.elc (as it has always done), but the native
compiler does *not* know how to compile it (because it does not know how to
locate its source), so it gives a warning:

022-11-18 02:54:01+0100 Warning (comp): Cannot look up eln file as no
source file was found for d:/Home/.emacs.elc

and then it assigns the user-init-file to warnings.el!

ELISP> user-init-file
"d:/Home/.emacs.d/native/29.0.50-a4a10996/warnings-28e75f4d-02da775e.eln"

which, if my patch is installed, gets converted to its source file:

ELISP> user-init-file
"d:/Devel/emacs/repo/trunk/lisp/emacs-lisp/warnings.el"

So this is a double bug for anyone compiling their .emacs: they will always
get a warning, and an erroneous user-init-file.

Do you want me to file this as a separate bug?

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

On Fri, Nov 18, 2022 at 3:25 AM Juanma Barranquero <lekktu@gmail.com> wrote:

> In other words, with this patch, it will still warn if the source file is
newer than the .elc
> (as it does now Emacs), regardless of whether the init file was loaded
from an .elc or
> .eln. (The age of the .eln is irrelevant here, that's for the native
compiler to check and
> act upon.)

Sorry, this part is not true. It's comparing the source with
user-init-file. Let me fix that and get back.

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

Hmm. Before I start changing it...

If the init file was loaded from .eln, do we want it to warn the user
- when init.el is newer than init.elc, or
- when init.el is newer than [whatever]/init-xxxxxxxx-yyyyyyy.eln (as it's
doing now)?

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Nov 2022 03:25:34 +0100
> Cc: Andrea Corallo <akrl@sdf.org>, 59334@debbugs.gnu.org
> 
> If you use ~/.emacs instead of ~.emacs.d/init.el, and happen to compile .emacs, Emacs loads .emacs.elc
> (as it has always done), but the native compiler does *not* know how to compile it (because it does not know
> how to locate its source), so it gives a warning:
> 
> 022-11-18 02:54:01+0100 Warning (comp): Cannot look up eln file as no source file was found for
> d:/Home/.emacs.elc
> 
> and then it assigns the user-init-file to warnings.el!
> 
> ELISP> user-init-file
> "d:/Home/.emacs.d/native/29.0.50-a4a10996/warnings-28e75f4d-02da775e.eln"
> 
> which, if my patch is installed, gets converted to its source file:
> 
> ELISP> user-init-file
> "d:/Devel/emacs/repo/trunk/lisp/emacs-lisp/warnings.el"
> 
> So this is a double bug for anyone compiling their .emacs: they will always get a warning, and an erroneous
> user-init-file.
> 
> Do you want me to file this as a separate bug?

Yes, please.

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Nov 2022 03:25:34 +0100
> Cc: Andrea Corallo <akrl@sdf.org>, 59334@debbugs.gnu.org
> 
> diff --git i/lisp/startup.el w/lisp/startup.el
> index 70267fc857..62984426e5 100644
> --- i/lisp/startup.el
> +++ w/lisp/startup.el
> @@ -1064,7 +1064,11 @@ startup--load-user-init-file
>              ;; If we loaded a compiled file, set `user-init-file' to
>              ;; the source version if that exists.
> -            (when (equal (file-name-extension user-init-file)
> -                         "elc")
> -              (let* ((source (file-name-sans-extension user-init-file))
> +            (when (member (file-name-extension user-init-file)
> +                          '("elc" "eln"))
> +              (let* ((source (file-name-sans-extension
> +                              (or (gethash
> +                                   (file-name-nondirectory user-init-file)
> +                                   comp-eln-to-el-h)
> +                                  user-init-file)))
>                       (alt (concat source ".el")))
>                  (setq source (cond ((file-exists-p alt) alt)

I think the call to gethash should only be done if the file has the
.eln extension, otherwise you might have false positives.

The logic should be something like

  . if the extension is .eln then
    - try to get the corresponding .el file by gethash
    - if that fails, leave user-init-file to be the original .eln
      file name, perhaps with a warning
    - otherwise, set user-init-file to the .el file from gethash
  . else <the original logic which yields .el or .elc>

The file-exists-and-is-newer test is not relevant to the .eln case,
because Emacs will not load a .eln file that is outdated wrt its
source, and will not use a .eln file whose source is not available.
Andrea, please correct me if I'm wrong.

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

On Fri, Nov 18, 2022 at 8:28 AM Eli Zaretskii <eliz@gnu.org> wrote:

> I think the call to gethash should only be done if the file has the
> .eln extension, otherwise you might have false positives.

Do you mean the hash comp-eln-to-el-h could have keys that match something
that doesn't end in .eln? Or that someone could've an init file with
extension .eln and matching one of the keys? Both seem extremely unlikely,
but ok.

> The file-exists-and-is-newer test is not relevant to the .eln case,
> because Emacs will not load a .eln file that is outdated wrt its
> source, and will not use a .eln file whose source is not available.

Ok. Easier, then.

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Nov 2022 04:03:32 +0100
> Cc: Andrea Corallo <akrl@sdf.org>, 59334@debbugs.gnu.org
> 
> If the init file was loaded from .eln, do we want it to warn the user
> - when init.el is newer than init.elc, or
> - when init.el is newer than [whatever]/init-xxxxxxxx-yyyyyyy.eln (as it's doing now)?

The second one cannot happen, AFAIU: Emacs will not load an outdated
.eln file.  That's why Emacs insists on having access to the .el
source file when it is about to load a .eln file.

As for the first one, I'm not sure if a warning is necessary.  For
starters, it will be a very rare one, since when we compile to produce
an updated .eln file, we also produce an updated .elc file.  And
emitting a warning in that case might confuse users, making them think
Emacs loaded the .elc file (otherwise why should Emacs bother about
its being outdated?).

So, on balance, I think the answer is NO to both.

Andrea, WDYT?

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Nov 2022 08:45:03 +0100
> Cc: akrl@sdf.org, 59334@debbugs.gnu.org
> 
> > I think the call to gethash should only be done if the file has the
> > .eln extension, otherwise you might have false positives.
> 
> Do you mean the hash comp-eln-to-el-h could have keys that match something that doesn't end in .eln? Or
> that someone could've an init file with extension .eln and matching one of the keys? Both seem extremely
> unlikely, but ok.

I thought about a possibility that the session loaded a .eln file, but
then the user or some Lisp explicitly loaded the .el file by hand.
I'm not sure in this case the hash table is updated.  Andrea, am I
right?

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

On Fri, Nov 18, 2022 at 9:46 AM Eli Zaretskii <eliz@gnu.org> wrote:

> I thought about a possibility that the session loaded a .eln file, but
> then the user or some Lisp explicitly loaded the .el file by hand.
> I'm not sure in this case the hash table is updated.

That's a whole another problem, isn't it?

On one hand, it would not affect user-init-file, as it's not the usual
startup procedure. And, on the other hand, my patch sets user-init-file to
the source .el, so after reloading that file it would still have the right
value, wouldn't it?

Anyway, here's the reworked patch.

diff --git i/lisp/startup.el w/lisp/startup.el
index 70267fc857..d7d8743336 100644
--- i/lisp/startup.el
+++ w/lisp/startup.el
@@ -1064,17 +1064,25 @@ startup--load-user-init-file
             ;; If we loaded a compiled file, set `user-init-file' to
             ;; the source version if that exists.
-            (when (equal (file-name-extension user-init-file)
-                         "elc")
-              (let* ((source (file-name-sans-extension user-init-file))
-                     (alt (concat source ".el")))
-                (setq source (cond ((file-exists-p alt) alt)
-                                   ((file-exists-p source) source)
-                                   (t nil)))
-                (when source
-                  (when (file-newer-than-file-p source user-init-file)
-                    (message "Warning: %s is newer than %s"
-                             source user-init-file)
-                    (sit-for 1))
-                  (setq user-init-file source))))
+            (if (equal (file-name-extension user-init-file) "elc")
+                (let* ((source (file-name-sans-extension user-init-file))
+                       (alt (concat source ".el")))
+                  (setq source (cond ((file-exists-p alt) alt)
+                                     ((file-exists-p source) source)
+                                     (t nil)))
+                  (when source
+                    (when (file-newer-than-file-p source user-init-file)
+                      (message "Warning: %s is newer than %s"
+                               source user-init-file)
+                      (sit-for 1))
+                    (setq user-init-file source)))
+              ;; Else, perhaps the user init file was compiled
+              (when (equal (file-name-extension user-init-file) "eln")
+                (if-let (source (gethash (file-name-nondirectory
user-init-file)
+                                         comp-eln-to-el-h))
+                    ;; source exists or the .eln file would not load
+                    (setq user-init-file source)
+                  (message "Warning: unknown source file for init file %S"
+                           user-init-file)
+                  (sit-for 1))))

             (when (and load-defaults


The original code is untouched, other than changing `when' to `if'; the
else part deals with the .eln.

I've checked that gethash returns a value, but not for the file's existence
because in that case comp already complains:

2022-11-18 10:01:15+0100 Warning (comp): Cannot look up eln file as no
source file was found for d:/Home/.emacs.d/init.elc

The warning above is only for the unlikely case that user-init-file points
to an .eln but the gethash lookup returns nil.

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

On Fri, Nov 18, 2022 at 10:05 AM Juanma Barranquero <lekktu@gmail.com>
wrote:

> The warning above is only for the unlikely case that user-init-file
points to an .eln
> but the gethash lookup returns nil.

Meaning the warning I've added in my code, not the warning I pasted about
the missing .el, of course.

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juanma Barranquero <lekktu@gmail.com>
>> Date: Fri, 18 Nov 2022 08:45:03 +0100
>> Cc: akrl@sdf.org, 59334@debbugs.gnu.org
>> 
>> > I think the call to gethash should only be done if the file has the
>> > .eln extension, otherwise you might have false positives.
>> 
>> Do you mean the hash comp-eln-to-el-h could have keys that match something that doesn't end in .eln? Or
>> that someone could've an init file with extension .eln and matching one of the keys? Both seem extremely
>> unlikely, but ok.
>
> I thought about a possibility that the session loaded a .eln file, but
> then the user or some Lisp explicitly loaded the .el file by hand.
> I'm not sure in this case the hash table is updated.

In that case (manual load af an .el file) the hash table is just not
updated (and still all the keys in it should be only .eln files).

  Andrea

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Nov 2022 10:05:50 +0100
> Cc: akrl@sdf.org, 59334@debbugs.gnu.org
> 
> > I thought about a possibility that the session loaded a .eln file, but
> > then the user or some Lisp explicitly loaded the .el file by hand.
> > I'm not sure in this case the hash table is updated. 
> 
> That's a whole another problem, isn't it?

Not necessarily.

> On one hand, it would not affect user-init-file, as it's not the
> usual startup procedure.

It could be part of startup if the forced loading of "init.el" is in
the code inside user's init file itself.  Crazy, I know, but not
impossible.

> And, on the other hand,
> my patch sets user-init-file to the source .el, so after reloading that file it would still have the right value,
> wouldn't it?

If that is the same file, yes.  But what if there's an init.el in
another place?

In any case, we don't need to keep arguing about this, since your
pat6ch indeed uses gethash only if the init file has the .eln
extension.

> The original code is untouched, other than changing `when' to `if'; the else part deals with the .eln.

I think we should compare the extensions case-insensitively, but other
than that, this LGTM.

Andrea, any comments?

> I've checked that gethash returns a value, but not for the file's existence because in that case comp already
> complains:
> 
> 2022-11-18 10:01:15+0100 Warning (comp): Cannot look up eln file as no source file was found for
> d:/Home/.emacs.d/init.elc
> 
> The warning above is only for the unlikely case that user-init-file points to an .eln but the gethash lookup
> returns nil.

SGTM, thanks.

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

On Fri, Nov 18, 2022 at 1:33 PM Eli Zaretskii <eliz@gnu.org> wrote:

> It could be part of startup if the forced loading of "init.el" is in
> the code inside user's init file itself.  Crazy, I know, but not
> impossible.

I suppose I'm not devious enough to think of that possibility ;-)

> I think we should compare the extensions case-insensitively, but other
> than that, this LGTM.

Hmm. I can use `string-equal-ignore-case', if you want, but the original
code was already checking for "el" case-sensitively.

If so, do you want it just when checking for .eln, or for .el above also?

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Nov 2022 13:40:44 +0100
> Cc: akrl@sdf.org, 59334@debbugs.gnu.org
> 
> Hmm. I can use `string-equal-ignore-case', if you want, but the original code was already checking for "el"
> case-sensitively.

Actually, forget about this.  It could mean complications with unibyte
file names at startup, so let's leave this alone.

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Juanma Barranquero]

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

close 59334 29.0.50
quit

Fixed by commit

c116d9f74c  of  2022-11-18  Avoid `user-init-file' being set to an eln file
(bug#59334)

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

bug#59334: 29.0.50; loading native-compiled init file sets user-init-file to .eln

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juanma Barranquero <lekktu@gmail.com>
>> Date: Fri, 18 Nov 2022 10:05:50 +0100
>> Cc: akrl@sdf.org, 59334@debbugs.gnu.org
>> 
>> > I thought about a possibility that the session loaded a .eln file, but
>> > then the user or some Lisp explicitly loaded the .el file by hand.
>> > I'm not sure in this case the hash table is updated. 
>> 
>> That's a whole another problem, isn't it?
>
> Not necessarily.
>
>> On one hand, it would not affect user-init-file, as it's not the
>> usual startup procedure.
>
> It could be part of startup if the forced loading of "init.el" is in
> the code inside user's init file itself.  Crazy, I know, but not
> impossible.
>
>> And, on the other hand,
>> my patch sets user-init-file to the source .el, so after reloading that file it would still have the right value,
>> wouldn't it?
>
> If that is the same file, yes.  But what if there's an init.el in
> another place?
>
> In any case, we don't need to keep arguing about this, since your
> pat6ch indeed uses gethash only if the init file has the .eln
> extension.
>
>> The original code is untouched, other than changing `when' to `if'; the else part deals with the .eln.
>
> I think we should compare the extensions case-insensitively, but other
> than that, this LGTM.
>
> Andrea, any comments?

Agree and I don't have any further comment.

Thanks

  Andrea