Subject: bug#48994: closed (Re: bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS))

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 26 Jun 2021 09:39:01 +0000

Your bug report

#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
48994: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=48994
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
From: Alan Third <[email-masked]>
To: Matthew Bauer <[email-masked]>
Cc: [email-masked]
Date: Sat, 26 Jun 2021 10:38:19 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

I've pushed it to master.

Thanks!

On Wed, Jun 23, 2021 at 03:46:12PM -0500, Matthew Bauer wrote:
> That works for me! No apparent issues.
> 
> On Wed, Jun 23, 2021 at 10:59 AM Alan Third <[email-masked]> wrote:
> 
> > On Tue, Jun 22, 2021 at 06:01:25PM -0500, Matthew Bauer wrote:
> > > > Turns out there's another variable that should only be set when
> > > > building the app.
> > >
> > > > Please try the attached patch.
> > >
> > > That works for me !
> > >
> > > So previously the nextstep/Emacs.app was getting generated even
> > > with --disable-ns-self-contained. I think that's fine to not build in
> > > this case - in fact it duplicates the Emacs executable - but just a note
> > > that it kind of changes things for packages. I have a fix for Nixpkgs
> > > (which previously installed nexstep/Emacs.app), but I think
> > > homebrew-emacs-plus would also be effected:
> > >
> > > https://github.com/d12frosted/homebrew-emacs-plus
> >
> > This sounds like madness to me, but I see it's a supported
> > configuration mentioned in our install files.
> >
> > So, attempt three attached!
> >

-- 
Alan Third

Subject: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
From: Matthew Bauer <[email-masked]>
To: [email-masked]
Cc: Andrea Corallo <[email-masked]>
Date: Sat, 12 Jun 2021 22:13:13 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

This is similar to bug#48497, but appears to still happen even after
commit 3f207753a0.

The basic problem is that the installed lisp path does not match either
of the search expressions in comp-el-to-eln-rel-filename, meaning that
native lisp needs to be recompiled needlessly.

The problem seems to come from PATH_REL_LOADSEARCH being set for me (on
macOS) to Contents/Resources/lisp, but the lisp files actually being
installed to
/nix/store/...-emacs-gcc-20210612.0/share/emacs/28.0.50/lisp (path
generated by Nix). As a result, comp-el-to-eln-rel-filename used the
filename comp-034d3699-516ce4bf.eln for comp.el.gz where 516ce4bf is the
md5sum of the contents of comp.el and 034d3699 is the md5sum of the full
path of comp.el, not of //emacs-lisp/comp.el (7672a6ed), which is what
Emacs installs.

To fix this, I propose using PATH_LOADSEARCH in addition to
PATH_REL_LOADSEARCH so that we can catch both types of macOS installs
(.app and unix). I’ve attached a patch which implements this, adding
relative and absolute loadsearch resolution.

- Matthew

>From 32022ee8d977196c72ad42d89d23142a6e59ff8e Mon Sep 17 00:00:00 2001
From: Matthew Bauer <[email-masked]>
Date: Sat, 12 Jun 2021 00:37:28 -0500
Subject: [PATCH] Use PATH_LOADSEARCH to get absolute path of native comp

We need to so that
/usr/local/share/emacs/28.0.50/lisp/emacs-lisp/comp.el

becomes:

//emacs-lisp/comp.el

which is what we install eln files as.
---
 src/comp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/comp.c b/src/comp.c
index 056d0860d8..8ccd99d583 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -4049,12 +4049,14 @@ DEFUN ("comp-el-to-eln-rel-filename", Fcomp_el_to_eln_rel_filename,
 
      As installing .eln files compiled during the build changes their
      absolute path we need an hashing mechanism that is not sensitive
-     to that.  For this we replace if match PATH_DUMPLOADSEARCH or
-     *PATH_REL_LOADSEARCH with '//' before computing the hash.  */
+     to that.  For this we replace if match PATH_DUMPLOADSEARCH,
+     PATH_REL_LOADSEARCH, or PATH_LOADSEARCH with '//' before
+     computing the hash.  */
 
   if (NILP (loadsearch_re_list))
     {
-      Lisp_Object sys_re =
+      Lisp_Object sys_abs_re = Fregexp_quote (build_string (PATH_LOADSEARCH "/"));
+      Lisp_Object sys_rel_re =
 	concat2 (build_string ("\\`[[:ascii:]]+"),
 		 Fregexp_quote (build_string ("/" PATH_REL_LOADSEARCH "/")));
       Lisp_Object dump_load_search =
@@ -4062,7 +4064,7 @@ DEFUN ("comp-el-to-eln-rel-filename", Fcomp_el_to_eln_rel_filename,
 #ifdef WINDOWSNT
       dump_load_search = Fw32_long_file_name (dump_load_search);
 #endif
-      loadsearch_re_list = list2 (sys_re, Fregexp_quote (dump_load_search));
+      loadsearch_re_list = list3 (sys_abs_re, sys_rel_re, Fregexp_quote (dump_load_search));
     }
 
   Lisp_Object lds_re_tail = loadsearch_re_list;
-- 
2.29.2