unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
@ 2021-06-13  3:13 Matthew Bauer
  2021-06-13  6:37 ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Matthew Bauer @ 2021-06-13  3:13 UTC (permalink / raw)
  To: 48994; +Cc: Andrea Corallo

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-PATH_LOADSEARCH-to-get-absolute-path-of-native-c.patch --]
[-- Type: text/x-patch, Size: 1850 bytes --]

From 32022ee8d977196c72ad42d89d23142a6e59ff8e Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-13  3:13 bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS) Matthew Bauer
@ 2021-06-13  6:37 ` Eli Zaretskii
  2021-06-13 16:37   ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2021-06-13  6:37 UTC (permalink / raw)
  To: Matthew Bauer, Alan Third; +Cc: 48994, akrl

> From: Matthew Bauer <mjbauer95@gmail.com>
> Date: Sat, 12 Jun 2021 22:13:13 -0500
> Cc: Andrea Corallo <akrl@sdf.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.

Thanks.

Alan, any comments?

And could you explain to this macOS-illiterate why this mess happens
on macOS?

In any case, if this only happens on macOS, I see no reason to run the
additional code on other systems, it's just waste of cycles.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-13  6:37 ` Eli Zaretskii
@ 2021-06-13 16:37   ` Alan Third
  2021-06-13 17:21     ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-13 16:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 48994, Matthew Bauer, akrl

On Sun, Jun 13, 2021 at 09:37:15AM +0300, Eli Zaretskii wrote:
> > From: Matthew Bauer <mjbauer95@gmail.com>
> > Date: Sat, 12 Jun 2021 22:13:13 -0500
> > Cc: Andrea Corallo <akrl@sdf.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.
> 
> Thanks.
> 
> Alan, any comments?

I suggested previously we handle this like we do the other install
paths (see ns_etc_path, ns_exec_path and ns_load_path in nsterm.m),
but Andrea didn't like that solution.

> And could you explain to this macOS-illiterate why this mess happens
> on macOS?

It's because we support both a relocatable application format and a
normal unix style install. In the .app case the path to the installed
files has to be configured at run-time (although I think Andrea used a
directory relative to the executable), but in the unix case it's
hard-coded.

FWIW, we will have exactly the same problems with GNUstep builds,
because they use the same .app format.

-- 
Alan Third





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-13 16:37   ` Alan Third
@ 2021-06-13 17:21     ` Eli Zaretskii
  2021-06-13 19:30       ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2021-06-13 17:21 UTC (permalink / raw)
  To: Alan Third; +Cc: 48994, mjbauer95, akrl

> Date: Sun, 13 Jun 2021 17:37:55 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: Matthew Bauer <mjbauer95@gmail.com>, 48994@debbugs.gnu.org,
> 	akrl@sdf.org
> 
> I suggested previously we handle this like we do the other install
> paths (see ns_etc_path, ns_exec_path and ns_load_path in nsterm.m),
> but Andrea didn't like that solution.

Can you point me to that discussion, please?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-13 17:21     ` Eli Zaretskii
@ 2021-06-13 19:30       ` Alan Third
  2021-06-14 12:42         ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-13 19:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 48994, mjbauer95, akrl

On Sun, Jun 13, 2021 at 08:21:11PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 13 Jun 2021 17:37:55 +0100
> > From: Alan Third <alan@idiocy.org>
> > Cc: Matthew Bauer <mjbauer95@gmail.com>, 48994@debbugs.gnu.org,
> > 	akrl@sdf.org
> > 
> > I suggested previously we handle this like we do the other install
> > paths (see ns_etc_path, ns_exec_path and ns_load_path in nsterm.m),
> > but Andrea didn't like that solution.
> 
> Can you point me to that discussion, please?

There isn't much in the way of actual discussion.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47558#44
-- 
Alan Third





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-13 19:30       ` Alan Third
@ 2021-06-14 12:42         ` Eli Zaretskii
  2021-06-14 18:32           ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2021-06-14 12:42 UTC (permalink / raw)
  To: Alan Third; +Cc: 48994, mjbauer95, akrl

> Date: Sun, 13 Jun 2021 20:30:13 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: mjbauer95@gmail.com, 48994@debbugs.gnu.org, akrl@sdf.org
> 
> On Sun, Jun 13, 2021 at 08:21:11PM +0300, Eli Zaretskii wrote:
> > > Date: Sun, 13 Jun 2021 17:37:55 +0100
> > > From: Alan Third <alan@idiocy.org>
> > > Cc: Matthew Bauer <mjbauer95@gmail.com>, 48994@debbugs.gnu.org,
> > > 	akrl@sdf.org
> > > 
> > > I suggested previously we handle this like we do the other install
> > > paths (see ns_etc_path, ns_exec_path and ns_load_path in nsterm.m),
> > > but Andrea didn't like that solution.
> > 
> > Can you point me to that discussion, please?
> 
> There isn't much in the way of actual discussion.
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47558#44

Hmm... that problem was solved, though?  So how come it comes up once
again?

In the tree that you described in

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47558#23

can you where is each of the following:

  . the Emacs binary
  . the .pdmp file
  . the auxiliary executables (hexl, movemail)
  . the *.eln files

Also, I understand that the "normal" Unix-style install has the usual
tree hierarchy rooted at /usr or /usr/local, and there everything
"just works"?  If so, when (at configure time, at installation time,
at some other time?) is the decision made whether the install will be
one or the other?

The issue finding the *.eln files is tightly coupled with how Emacs
finds the .pdmp file, so we must consider these together to be sure
the solutions don't break.

Thanks.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-14 12:42         ` Eli Zaretskii
@ 2021-06-14 18:32           ` Alan Third
  2021-06-14 19:19             ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-14 18:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 48994, mjbauer95, akrl

On Mon, Jun 14, 2021 at 03:42:41PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 13 Jun 2021 20:30:13 +0100
> > From: Alan Third <alan@idiocy.org>
> > Cc: mjbauer95@gmail.com, 48994@debbugs.gnu.org, akrl@sdf.org
> > 
> > On Sun, Jun 13, 2021 at 08:21:11PM +0300, Eli Zaretskii wrote:
> > > > Date: Sun, 13 Jun 2021 17:37:55 +0100
> > > > From: Alan Third <alan@idiocy.org>
> > > > Cc: Matthew Bauer <mjbauer95@gmail.com>, 48994@debbugs.gnu.org,
> > > > 	akrl@sdf.org
> > > > 
> > > > I suggested previously we handle this like we do the other install
> > > > paths (see ns_etc_path, ns_exec_path and ns_load_path in nsterm.m),
> > > > but Andrea didn't like that solution.
> > > 
> > > Can you point me to that discussion, please?
> > 
> > There isn't much in the way of actual discussion.
> > 
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47558#44
> 
> Hmm... that problem was solved, though?  So how come it comes up once
> again?

It was solved for the .app use case, perhaps that broke the Unix-style
install case?

I have to admit that I don't really understand this and have a very
limited understanding of how the makefiles build the .app or not.

> In the tree that you described in
> 
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47558#23
> 
> can you where is each of the following:
> 
>   . the Emacs binary

Emacs.app/Contents/MacOS/Emacs

(This is different on GNUstep builds. I think it's
Emacs.app/Contents/Emacs.)

>   . the .pdmp file

The same location as the binary. That was my change and it was done
that way because I don't really understand any of this but that
happened to work.

>   . the auxiliary executables (hexl, movemail)

I believe they're in Emacs.app/Contents/MacOS/libexec.

(Again, this is different on GNUstep.)

>   . the *.eln files

Emacs.app/Contents/Resources/native-elisp, or something. I'm not at a
Mac to check right now, but definitely under Resources.

> Also, I understand that the "normal" Unix-style install has the usual
> tree hierarchy rooted at /usr or /usr/local, and there everything
> "just works"?  If so, when (at configure time, at installation time,
> at some other time?) is the decision made whether the install will be
> one or the other?

I believe there's a ./configure flag. But like I said before, for the
other paths there's a run-time check, so I'm not sure how it all ties
together.

-- 
Alan Third





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-14 18:32           ` Alan Third
@ 2021-06-14 19:19             ` Eli Zaretskii
  2021-06-16 18:25               ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2021-06-14 19:19 UTC (permalink / raw)
  To: Alan Third; +Cc: 48994, mjbauer95, akrl

> Date: Mon, 14 Jun 2021 19:32:36 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: 48994@debbugs.gnu.org, mjbauer95@gmail.com, akrl@sdf.org
> 
> > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47558#44
> > 
> > Hmm... that problem was solved, though?  So how come it comes up once
> > again?
> 
> It was solved for the .app use case, perhaps that broke the Unix-style
> install case?

Could very well be, see below.

> I have to admit that I don't really understand this and have a very
> limited understanding of how the makefiles build the .app or not.

My primary worry is not the Makefiles, it's what the installed Emacs
does upon startup to find its requisite files.

> >   . the Emacs binary
> 
> Emacs.app/Contents/MacOS/Emacs
> 
> (This is different on GNUstep builds. I think it's
> Emacs.app/Contents/Emacs.)
> 
> >   . the .pdmp file
> 
> The same location as the binary. That was my change and it was done
> that way because I don't really understand any of this but that
> happened to work.

If the pdumper file is near the binary, I think Emacs will decide it's
running uninstalled, and will look for the *.eln files in
../native-lisp relative to where the binary lives.  Which is not
necessarily what you want, AFAIU.

> >   . the auxiliary executables (hexl, movemail)
> 
> I believe they're in Emacs.app/Contents/MacOS/libexec.
> 
> (Again, this is different on GNUstep.)
> 
> >   . the *.eln files
> 
> Emacs.app/Contents/Resources/native-elisp, or something. I'm not at a
> Mac to check right now, but definitely under Resources.

OK, please tell after you find out.

> > Also, I understand that the "normal" Unix-style install has the usual
> > tree hierarchy rooted at /usr or /usr/local, and there everything
> > "just works"?

You didn't answer this one.  Are we using the Unix-style hierarchy in
this case, i.e.:

  . Emacs binary in /usr/bin
  . the .pdmp file in /usr/libexec/emacs/VERSION/ARCHITECTURE
  . the *eln files in /usr/lib/emacs/VERSION/native-lisp

?

> > If so, when (at configure time, at installation time,
> > at some other time?) is the decision made whether the install will be
> > one or the other?
> 
> I believe there's a ./configure flag. But like I said before, for the
> other paths there's a run-time check, so I'm not sure how it all ties
> together.

Which run-time check did you have in mind?





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-14 19:19             ` Eli Zaretskii
@ 2021-06-16 18:25               ` Alan Third
  2021-06-16 18:45                 ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-16 18:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 48994, mjbauer95, akrl

On Mon, Jun 14, 2021 at 10:19:50PM +0300, Eli Zaretskii wrote:
> > I have to admit that I don't really understand this and have a very
> > limited understanding of how the makefiles build the .app or not.
> 
> My primary worry is not the Makefiles, it's what the installed Emacs
> does upon startup to find its requisite files.

I've done a lot of digging about and it looks like a bit of a mess,
frankly.

configure hard-codes "lispdirrel" to Contents/Resources/lisp, which is
obviously wrong on a Unix style install, but is only "correct" on a
bundled app install because argv0 is set to the root of the app bundle
instead of the actual executable.

I don't know why argv0 is set that way, but it must be how NS apps
work because I haven't yet seen any code changing it yet.

The other place that's causing trouble is this wee bit in emacs.c:

  /* Assume the Emacs binary lives in a sibling directory as set up by
     the default installation configuration.  */
  const char *go_up = "../../../../bin/";

Which I assume works correctly in a Unix style install but doesn't
work in an app bundle install.

I'm feeling that perhaps we should expose the ns_self_contained
variable from configure/makefile to the C code so we can do different
things in a couple of places depending on what type of install we're
using. Trying to use the same code for both seems rather more
difficult than it needs to be.

> > >   . the Emacs binary
> > 
> > Emacs.app/Contents/MacOS/Emacs
> > 
> > (This is different on GNUstep builds. I think it's
> > Emacs.app/Contents/Emacs.)
> > 
> > >   . the .pdmp file
> > 
> > The same location as the binary. That was my change and it was done
> > that way because I don't really understand any of this but that
> > happened to work.
> 
> If the pdumper file is near the binary, I think Emacs will decide it's
> running uninstalled, and will look for the *.eln files in
> ../native-lisp relative to where the binary lives.  Which is not
> necessarily what you want, AFAIU.

That's easy enough to change. If I put it in the libexec directory it
works just as well.

> > >   . the auxiliary executables (hexl, movemail)
> > 
> > I believe they're in Emacs.app/Contents/MacOS/libexec.
> > 
> > (Again, this is different on GNUstep.)
> > 
> > >   . the *.eln files
> > 
> > Emacs.app/Contents/Resources/native-elisp, or something. I'm not at a
> > Mac to check right now, but definitely under Resources.
> 
> OK, please tell after you find out.

Yes, in Emacs.app/Contents/Resources/native-elisp, however that's easy
to change as well.

> > > Also, I understand that the "normal" Unix-style install has the usual
> > > tree hierarchy rooted at /usr or /usr/local, and there everything
> > > "just works"?
> 
> You didn't answer this one.  Are we using the Unix-style hierarchy in
> this case, i.e.:
> 
>   . Emacs binary in /usr/bin
>   . the .pdmp file in /usr/libexec/emacs/VERSION/ARCHITECTURE
>   . the *eln files in /usr/lib/emacs/VERSION/native-lisp
> 
> ?

Sorry, I must've missed it.

Yes, a normal Unix style install should be like that.

However the OP appears to be using a Unix style install with a
different install prefix and is getting app bundle install paths in
his errors, specifically Contents/Resources/lisp, which I mentioned at
the top of the email.

> > > If so, when (at configure time, at installation time,
> > > at some other time?) is the decision made whether the install will be
> > > one or the other?
> > 
> > I believe there's a ./configure flag. But like I said before, for the
> > other paths there's a run-time check, so I'm not sure how it all ties
> > together.
> 
> Which run-time check did you have in mind?

ns_exec_path, and friends. They return the app bundle paths if the
directories exist, and nil otherwise, then the code that calls them
modifies the search paths depending on the result.

I think I can probably fix this now, hopefully without breaking
anything... ;)

But any observations/recommendations will be gratefully received.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-16 18:25               ` Alan Third
@ 2021-06-16 18:45                 ` Eli Zaretskii
  2021-06-19 17:04                   ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2021-06-16 18:45 UTC (permalink / raw)
  To: Alan Third; +Cc: 48994, mjbauer95, akrl

> Date: Wed, 16 Jun 2021 19:25:14 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: 48994@debbugs.gnu.org, mjbauer95@gmail.com, akrl@sdf.org
> 
> > My primary worry is not the Makefiles, it's what the installed Emacs
> > does upon startup to find its requisite files.
> 
> I've done a lot of digging about and it looks like a bit of a mess,
> frankly.

It could be.  But let me first ask you: did you understand how Emacs
finds the directory with the *.eln files, and how that is related to
finding the .pdmp file?  These two parts are looked for together, and
the place where we find the .pdmp file tells us directly where to look
for the *.eln files.

If this is "messy" on macOS, in either of the two types of install,
then we should fix that first.

From what you describe, it sounds like not everything is well in that
department:

> configure hard-codes "lispdirrel" to Contents/Resources/lisp, which is
> obviously wrong on a Unix style install, but is only "correct" on a
> bundled app install because argv0 is set to the root of the app bundle
> instead of the actual executable.
> 
> I don't know why argv0 is set that way, but it must be how NS apps
> work because I haven't yet seen any code changing it yet.
> 
> The other place that's causing trouble is this wee bit in emacs.c:
> 
>   /* Assume the Emacs binary lives in a sibling directory as set up by
>      the default installation configuration.  */
>   const char *go_up = "../../../../bin/";
> 
> Which I assume works correctly in a Unix style install but doesn't
> work in an app bundle install.

So we need to make NS-specific changes that will make this work
correctly both in Unix-style and the app bundle style install.  Do you
understand how to fix this part?  I'll gladly help you if needed.

> I'm feeling that perhaps we should expose the ns_self_contained
> variable from configure/makefile to the C code so we can do different
> things in a couple of places depending on what type of install we're
> using. Trying to use the same code for both seems rather more
> difficult than it needs to be.

If the decision on the type of the install is at configure time, then
we can have compile-time conditions in the source, yes.

> > If the pdumper file is near the binary, I think Emacs will decide it's
> > running uninstalled, and will look for the *.eln files in
> > ../native-lisp relative to where the binary lives.  Which is not
> > necessarily what you want, AFAIU.
> 
> That's easy enough to change. If I put it in the libexec directory it
> works just as well.

Then let's do that.  In general, as much similarity between the two
cases as is feasible is better, because it will make the code less
messy, and will require us to remember fewer special cases.

> >   . Emacs binary in /usr/bin
> >   . the .pdmp file in /usr/libexec/emacs/VERSION/ARCHITECTURE
> >   . the *eln files in /usr/lib/emacs/VERSION/native-lisp
> > 
> > ?
> 
> Sorry, I must've missed it.
> 
> Yes, a normal Unix style install should be like that.

OK, then the Unix style install should already be working.  Then I
wonder how come this very install caused the OP trouble.  But we can
revisit it later, once the code whch looks for .pdmp and *.eln files
is finalized.

> However the OP appears to be using a Unix style install with a
> different install prefix and is getting app bundle install paths in
> his errors, specifically Contents/Resources/lisp, which I mentioned at
> the top of the email.

I wonder how this could happen, if the Unix style install is supposed
to "just work".

> > > I believe there's a ./configure flag. But like I said before, for the
> > > other paths there's a run-time check, so I'm not sure how it all ties
> > > together.
> > 
> > Which run-time check did you have in mind?
> 
> ns_exec_path, and friends. They return the app bundle paths if the
> directories exist, and nil otherwise, then the code that calls them
> modifies the search paths depending on the result.

It's okay to use that, although I'd expect the install location to be
known at compile time?

> I think I can probably fix this now, hopefully without breaking
> anything... ;)

Fine, please do, but let's try doing that one step at a time...

Thanks.





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-16 18:45                 ` Eli Zaretskii
@ 2021-06-19 17:04                   ` Alan Third
  2021-06-22  3:55                     ` Matthew Bauer
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-19 17:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 48994, mjbauer95, akrl

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

On Wed, Jun 16, 2021 at 09:45:50PM +0300, Eli Zaretskii wrote:
> > Date: Wed, 16 Jun 2021 19:25:14 +0100
> > From: Alan Third <alan@idiocy.org>
> > Cc: 48994@debbugs.gnu.org, mjbauer95@gmail.com, akrl@sdf.org
> > 
> > > My primary worry is not the Makefiles, it's what the installed Emacs
> > > does upon startup to find its requisite files.
> > 
> > I've done a lot of digging about and it looks like a bit of a mess,
> > frankly.
> 
> It could be.  But let me first ask you: did you understand how Emacs
> finds the directory with the *.eln files, and how that is related to
> finding the .pdmp file?  These two parts are looked for together, and
> the place where we find the .pdmp file tells us directly where to look
> for the *.eln files.
> 
> If this is "messy" on macOS, in either of the two types of install,
> then we should fix that first.

I think I did understand that. As far as I can tell that bit actually
works fine, as long as the pdmp file can be found the eln files can be
found.

> So we need to make NS-specific changes that will make this work
> correctly both in Unix-style and the app bundle style install.  Do you
> understand how to fix this part?  I'll gladly help you if needed.

I've attached a patch which, I hope, should solve these problems. I've
tried to align the code more with how other terms work, so I've
removed all the functions that calculate the various paths Emacs uses,
and replaced them with a single function that, given a relative path
inside the application bundle, can work out what the full path should
be, but will pass any absolute paths unchanged.

It's similar to w32_relocate, but not exactly the same, and I'm using
it in more places due to the app bundle layout being quite different.

I've also followed the w32 lead in changing how epaths.h is generated.
Basically I post-process it so that any paths that are internal to the
app bundle are converted to relative paths, so the above mentioned
function can deal with them.

> > >   . Emacs binary in /usr/bin
> > >   . the .pdmp file in /usr/libexec/emacs/VERSION/ARCHITECTURE
> > >   . the *eln files in /usr/lib/emacs/VERSION/native-lisp
> > > 
> > > ?
> > 
> > Sorry, I must've missed it.
> > 
> > Yes, a normal Unix style install should be like that.
> 
> OK, then the Unix style install should already be working.  Then I
> wonder how come this very install caused the OP trouble.  But we can
> revisit it later, once the code whch looks for .pdmp and *.eln files
> is finalized.
>
> > However the OP appears to be using a Unix style install with a
> > different install prefix and is getting app bundle install paths in
> > his errors, specifically Contents/Resources/lisp, which I mentioned at
> > the top of the email.
> 
> I wonder how this could happen, if the Unix style install is supposed
> to "just work".

I think it's because of some hard-coding of things that was added to
work around problems in the app bundle code. They haven't been
suitably separated from the Unix style install code, so they end up
interfering with each other.

I think that now a Unix style install shouldn't be affected at all by
the app bundle code, but I'd like some extra testing by people who run
it that way before I'm fully convinced I've been successful in that.

> > > > I believe there's a ./configure flag. But like I said before, for the
> > > > other paths there's a run-time check, so I'm not sure how it all ties
> > > > together.
> > > 
> > > Which run-time check did you have in mind?
> > 
> > ns_exec_path, and friends. They return the app bundle paths if the
> > directories exist, and nil otherwise, then the code that calls them
> > modifies the search paths depending on the result.
> 
> It's okay to use that, although I'd expect the install location to be
> known at compile time?

The install location is known, but in the app bundle case the app can
be copied to anywhere and then the full paths will be different, so
they can't just be hard coded at install time.

> > I think I can probably fix this now, hopefully without breaking
> > anything... ;)
> 
> Fine, please do, but let's try doing that one step at a time...

I tried, but it ended up quite a large patch as fixing one thing would
invariably break another, and I decided I had to sort out the epaths.h
situation before I could do much else, and most of the rest of the
patch is dealing with the fallout from that.

Anyway, it's attached. It's working here for the app bundle case and I
believe it should work well for the Unix style install, but I've not
tested that in any depth.

Matthew, could you please try the attached patch and see if it solves
your problems (or makes them worse!)?
-- 
Alan Third

[-- Attachment #2: 0001-Fix-NS-native-compilation-builds.patch --]
[-- Type: text/x-diff, Size: 17973 bytes --]

From ec5a25237cbee92d110e2226eb4f70d781d535c8 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Wed, 16 Jun 2021 21:28:10 +0100
Subject: [PATCH] Fix NS native compilation builds

* Makefile.in (ns_appdir): New variable.
(.PHONY): Include new rule.
(epaths-force-ns-self-contained): Remove the app bundle directory from
all paths.
* configure.ac (NS_SELF_CONTAINED): Set the default site-lisp
directory instead of hard-coding it in the ObjC code, and use the new
epaths generating make rule.
* src/callproc.c (init_callproc_1):
(init_callproc): Remove all the NS specific code as the special cases
are now handled by decode_env_path.
* src/emacs.c (load_pdump):
(decode_env_path): Use ns_relocate to find the correct directory after
relocation.
* src/lread.c (load_path_default): Remove all the NS specific code as
the special cases are now handled by decode_env_path.
* src/nsterm.h: Update function definitions.
* src/nsterm.m (ns_etc_directory):
(ns_exec_path):
(ns_load_path): Remove functions that are no longer needed.
(ns_relocate): New function to calculate paths within the NS app
bundle.
---
 Makefile.in          |  17 +++++-
 configure.ac         |  12 +++--
 nextstep/Makefile.in |  10 ++--
 src/callproc.c       |  36 ++-----------
 src/emacs.c          |  16 +++++-
 src/lread.c          |   7 +--
 src/nsterm.h         |   4 +-
 src/nsterm.m         | 125 ++++++++-----------------------------------
 8 files changed, 71 insertions(+), 156 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 3facfa59a9..68fe7d781a 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -104,8 +104,10 @@ HAVE_NATIVE_COMP =
 
 # Location to install Emacs.app under GNUstep / macOS.
 # Later values may use these.
+ns_appdir=@ns_appdir@
 ns_appbindir=@ns_appbindir@
 ns_appresdir=@ns_appresdir@
+ns_applibdir=@ns_applibdir@
 # Either yes or no depending on whether this is a relocatable Emacs.app.
 ns_self_contained=@ns_self_contained@
 
@@ -328,12 +330,12 @@ BIN_DESTDIR=
 ELN_DESTDIR = $(DESTDIR)${libdir}/emacs/${version}/
 else
 BIN_DESTDIR='${ns_appbindir}/'
-ELN_DESTDIR = ${ns_appresdir}/
+ELN_DESTDIR = ${ns_applibdir}/emacs/${version}/
 endif
 
 all: ${SUBDIR} info
 
-.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 etc-emacsver
+.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 epaths-force-ns-self-contained etc-emacsver
 
 # If configure were to just generate emacsver.tex from emacsver.tex.in
 # in the normal way, the timestamp of emacsver.tex would always be
@@ -402,6 +404,17 @@ epaths-force-w32:
 	  -e "/^.*#/s|@SRC@|$${w32srcdir}|g") &&		\
 	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
 
+# A NextStep style app bundle is relocatable, so instead of
+# hard-coding paths try to generate them at run-time.
+#
+# The paths are mostly the same, and the bundle paths are different
+# between macOS and GNUstep, so just replace any references to the app
+# bundle root itself with the relative path.
+epaths-force-ns-self-contained: epaths-force
+	@(sed < ${srcdir}/src/epaths.h > epaths.h.$$$$	\
+	  -e 's;${ns_appdir}/;;') &&			\
+	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
+
 lib-src src: $(NTDIR) lib
 
 src: lib-src
diff --git a/configure.ac b/configure.ac
index c828f8d782..05afdbc8b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1891,10 +1891,10 @@ AC_DEFUN
   # so avoid NS_IMPL_COCOA if macuvs.h is absent.
   # Even a headless Emacs can build macuvs.h, so this should let you bootstrap.
   if test "${opsys}" = darwin && test -f "$srcdir/src/macuvs.h"; then
-     lispdirrel=Contents/Resources/lisp
      NS_IMPL_COCOA=yes
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}/Contents/MacOS
+     ns_applibdir=${ns_appdir}/Contents/MacOS/lib
      ns_appresdir=${ns_appdir}/Contents/Resources
      ns_appsrc=Cocoa/Emacs.base
      ns_fontfile=macfont.o
@@ -1952,6 +1952,7 @@ AC_DEFUN
   if test $NS_IMPL_GNUSTEP = yes; then
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}
+     ns_applibdir=${ns_appdir}/lib
      ns_appresdir=${ns_appdir}/Resources
      ns_appsrc=GNUstep/Emacs.base
      ns_fontfile=nsfont.o
@@ -2008,6 +2009,7 @@ AC_DEFUN
   window_system=nextstep
   # set up packaging dirs
   if test "${EN_NS_SELF_CONTAINED}" = yes; then
+     AC_DEFINE(NS_SELF_CONTAINED, 1, [Build an NS bundled app])
      ns_self_contained=yes
      prefix=${ns_appresdir}
      exec_prefix=${ns_appbindir}
@@ -2021,7 +2023,7 @@ AC_DEFUN
      infodir="\${ns_appresdir}/info"
      mandir="\${ns_appresdir}/man"
      lispdir="\${ns_appresdir}/lisp"
-     test "$locallisppathset" = no && locallisppath=""
+     test "$locallisppathset" = no && locallisppath="\${ns_appresdir}/site-lisp"
      INSTALL_ARCH_INDEP_EXTRA=
   fi
 
@@ -5409,6 +5411,7 @@ AC_DEFUN
 AC_SUBST(X_TOOLKIT_TYPE)
 AC_SUBST(ns_appdir)
 AC_SUBST(ns_appbindir)
+AC_SUBST(ns_applibdir)
 AC_SUBST(ns_appresdir)
 AC_SUBST(ns_appsrc)
 AC_SUBST(GNU_OBJC_CFLAGS)
@@ -6009,10 +6012,13 @@ m4_define
 AC_CONFIG_COMMANDS([src/epaths.h], [
 if test "${opsys}" = "mingw32"; then
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-w32
+elif test "$EN_NS_SELF_CONTAINED" = "yes"; then
+  ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-ns-self-contained
 else
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force
 fi || AC_MSG_ERROR(['src/epaths.h' could not be made.])
-], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"])
+], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"
+    EN_NS_SELF_CONTAINED="$EN_NS_SELF_CONTAINED"])
 
 dnl NB we have to cheat and use the ac_... version because abs_top_srcdir
 dnl is not yet set, sigh.  Or we could use ../$srcdir/src/.gdbinit,
diff --git a/nextstep/Makefile.in b/nextstep/Makefile.in
index 3168fee76c..c0beb454d1 100644
--- a/nextstep/Makefile.in
+++ b/nextstep/Makefile.in
@@ -38,13 +38,14 @@ ns_appdir =
 ns_appbindir = @ns_appbindir@
 ## GNUstep/Emacs.base or Cocoa/Emacs.base.
 ns_appsrc = @ns_appsrc@
+ns_applibexecdir = @libexecdir@
 ## GNUstep: GNUstep/Emacs.base/Resources/Info-gnustep.plist
 ## macOS: Cocoa/Emacs.base/Contents/Info.plist
 ns_check_file = @ns_appdir@/@ns_check_file@
 
 .PHONY: all
 
-all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_appbindir}/Emacs.pdmp
+all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_applibexecdir}/Emacs.pdmp
 
 ${ns_check_file} ${ns_appdir}: ${srcdir}/${ns_appsrc} ${ns_appsrc}
 	rm -rf ${ns_appdir}
@@ -63,8 +64,8 @@ ${ns_appbindir}/Emacs:
 	${MKDIR_P} ${ns_appbindir}
 	cp -f ../src/emacs${EXEEXT} $@
 
-${ns_appbindir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
-	${MKDIR_P} ${ns_appbindir}
+${ns_applibexecdir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
+	${MKDIR_P} ${ns_applibexecdir}
 	cp -f ../src/emacs${EXEEXT}.pdmp $@
 
 .PHONY: FORCE
@@ -85,9 +86,8 @@ links:
 	ln -s $(top_srcdir_abs)/info ${ns_appdir}/Contents/Resources
 	${MKDIR_P} ${ns_appbindir}
 	ln -s $(abs_top_builddir)/src/emacs${EXEEXT} ${ns_appbindir}/Emacs
-	ln -s $(abs_top_builddir)/src/emacs${EXEEXT}.pdmp ${ns_appbindir}/Emacs.pdmp
 	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/bin
-	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/libexec
+	ln -s $(abs_top_builddir)/lib-src ${ns_applibexecdir}
 	${MKDIR_P} ${ns_appdir}/Contents/Resources/etc
 	for f in $(shell cd $(top_srcdir_abs)/etc; ls); do ln -s $(top_srcdir_abs)/etc/$$f ${ns_appdir}/Contents/Resources/etc; done
 	ln -s $(abs_top_builddir)/etc/DOC ${ns_appdir}/Contents/Resources/etc
diff --git a/src/callproc.c b/src/callproc.c
index e44e243680..aabc39313b 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1661,32 +1661,15 @@ make_environment_block (Lisp_Object current_dir)
 void
 init_callproc_1 (void)
 {
-#ifdef HAVE_NS
-  const char *etc_dir = ns_etc_directory ();
-  const char *path_exec = ns_exec_path ();
-#endif
-
-  Vdata_directory = decode_env_path ("EMACSDATA",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DATA, 0);
+  Vdata_directory = decode_env_path ("EMACSDATA", PATH_DATA, 0);
   Vdata_directory = Ffile_name_as_directory (Fcar (Vdata_directory));
 
-  Vdoc_directory = decode_env_path ("EMACSDOC",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DOC, 0);
+  Vdoc_directory = decode_env_path ("EMACSDOC", PATH_DOC, 0);
   Vdoc_directory = Ffile_name_as_directory (Fcar (Vdoc_directory));
 
   /* Check the EMACSPATH environment variable, defaulting to the
      PATH_EXEC path from epaths.h.  */
-  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-                                path_exec ? path_exec :
-#endif
-                                PATH_EXEC, 0);
+  Vexec_path = decode_env_path ("EMACSPATH", PATH_EXEC, 0);
   Vexec_directory = Ffile_name_as_directory (Fcar (Vexec_path));
   /* FIXME?  For ns, path_exec should go at the front?  */
   Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
@@ -1701,10 +1684,6 @@ init_callproc (void)
 
   char *sh;
   Lisp_Object tempdir;
-#ifdef HAVE_NS
-  if (data_dir == 0)
-    data_dir = ns_etc_directory () != 0;
-#endif
 
   if (!NILP (Vinstallation_directory))
     {
@@ -1716,15 +1695,8 @@ init_callproc (void)
 	  /* MSDOS uses wrapped binaries, so don't do this.  */
       if (NILP (Fmember (tem, Vexec_path)))
 	{
-#ifdef HAVE_NS
-	  const char *path_exec = ns_exec_path ();
-#endif
 	  /* Running uninstalled, so default to tem rather than PATH_EXEC.  */
-	  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-					path_exec ? path_exec :
-#endif
-					SSDATA (tem), 0);
+	  Vexec_path = decode_env_path ("EMACSPATH", SSDATA (tem), 0);
 	  Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
 	}
 
diff --git a/src/emacs.c b/src/emacs.c
index 60a57a693c..b7982ece64 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -835,7 +835,13 @@ load_pdump (int argc, char **argv)
     NULL
 #endif
     ;
-  const char *argv0_base = "emacs";
+  const char *argv0_base =
+#ifdef NS_SELF_CONTAINED
+    "Emacs"
+#else
+    "emacs"
+#endif
+    ;
 
   /* TODO: maybe more thoroughly scrub process environment in order to
      make this use case (loading a dump file in an unexeced emacs)
@@ -912,6 +918,8 @@ load_pdump (int argc, char **argv)
   /* On MS-Windows, PATH_EXEC normally starts with a literal
      "%emacs_dir%", so it will never work without some tweaking.  */
   path_exec = w32_relocate (path_exec);
+#elif defined (HAVE_NS)
+  path_exec = ns_relocate (path_exec);
 #endif
 
   /* Look for "emacs.pdmp" in PATH_EXEC.  We hardcode "emacs" in
@@ -929,6 +937,7 @@ load_pdump (int argc, char **argv)
     }
   sprintf (dump_file, "%s%c%s%s",
            path_exec, DIRECTORY_SEP, argv0_base, suffix);
+#if !defined (NS_SELF_CONTAINED)
   /* Assume the Emacs binary lives in a sibling directory as set up by
      the default installation configuration.  */
   const char *go_up = "../../../../bin/";
@@ -943,6 +952,7 @@ load_pdump (int argc, char **argv)
   sprintf (emacs_executable, "%s%c%s%s%s",
 	   path_exec, DIRECTORY_SEP, go_up, argv0_base,
 	   strip_suffix ? strip_suffix : "");
+#endif
   result = pdumper_load (dump_file, emacs_executable);
 
   if (result == PDUMPER_LOAD_FILE_NOT_FOUND)
@@ -2960,7 +2970,11 @@ decode_env_path (const char *evarname, const char *defalt, bool empty)
     path = 0;
   if (!path)
     {
+#ifdef NS_SELF_CONTAINED
+      path = ns_relocate (defalt);
+#else
       path = defalt;
+#endif
 #ifdef WINDOWSNT
       defaulted = 1;
 #endif
diff --git a/src/lread.c b/src/lread.c
index 0b33fd0f25..4617ffd626 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -4769,14 +4769,9 @@ load_path_default (void)
     return decode_env_path (0, PATH_DUMPLOADSEARCH, 0);
 
   Lisp_Object lpath = Qnil;
-  const char *normal = PATH_LOADSEARCH;
   const char *loadpath = NULL;
 
-#ifdef HAVE_NS
-  loadpath = ns_load_path ();
-#endif
-
-  lpath = decode_env_path (0, loadpath ? loadpath : normal, 0);
+  lpath = decode_env_path (0, PATH_LOADSEARCH, 0);
 
   if (!NILP (Vinstallation_directory))
     {
diff --git a/src/nsterm.h b/src/nsterm.h
index e7ea907569..139c37a29e 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1186,9 +1186,7 @@ #define NSAPP_DATA2_RUNASSCRIPT 10
 #define NSAPP_DATA2_RUNFILEDIALOG 11
 extern void ns_run_file_dialog (void);
 
-extern const char *ns_etc_directory (void);
-extern const char *ns_exec_path (void);
-extern const char *ns_load_path (void);
+extern const char *ns_relocate (const char *epath);
 extern void syms_of_nsterm (void);
 extern void syms_of_nsfns (void);
 extern void syms_of_nsmenu (void);
diff --git a/src/nsterm.m b/src/nsterm.m
index 838c14d5ab..7b0b169e10 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -499,118 +499,35 @@ - (NSColor *)colorUsingDefaultColorSpace
 
 
 const char *
-ns_etc_directory (void)
-/* If running as a self-contained app bundle, return as a string the
-   filename of the etc directory, if present; else nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath;
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
+ns_relocate (const char *epath)
+/* If we're running in a self-contained app bundle some hard-coded
+   paths are relative to the root of the bundle, so work out the full
+   path.
 
-  resourcePath = [resourceDir stringByAppendingPathComponent: @"etc"];
-  if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-    {
-      if (isDir) return [resourcePath UTF8String];
-    }
-  return NULL;
-}
-
-
-const char *
-ns_exec_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the libexec and bin directories, ie libexec:bin.
-   Otherwise, return nil.
-   Normally, Emacs does not add its own bin/ directory to the PATH.
-   However, a self-contained NS build has a different layout, with
-   bin/ and libexec/ subdirectories in the directory that contains
-   Emacs.app itself.
-   We put libexec first, because init_callproc_1 uses the first
-   element to initialize exec-directory.  An alternative would be
-   for init_callproc to check for invocation-directory/libexec.
-*/
+   FIXME: I think this should be able to handle cases where multiple
+   directories are separated by colons.  */
 {
+#ifdef NS_SELF_CONTAINED
   NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *binDir = [bundle bundlePath];
-  NSString *resourcePath, *resourcePaths;
-  NSRange range;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
+  NSString *root = [bundle bundlePath];
+  NSString *original = [NSString stringWithUTF8String:epath];
+  NSString *fixedPath = [NSString pathWithComponents:@[root, original]];
   NSFileManager *fileManager = [NSFileManager defaultManager];
-  NSArray *paths;
-  NSEnumerator *pathEnum;
-  BOOL isDir;
 
-  range = [resourceDir rangeOfString: @"Contents"];
-  if (range.location != NSNotFound)
-    {
-      binDir = [binDir stringByAppendingPathComponent: @"Contents"];
-#ifdef NS_IMPL_COCOA
-      binDir = [binDir stringByAppendingPathComponent: @"MacOS"];
-#endif
-    }
+  if (![original isAbsolutePath]
+      && [fileManager fileExistsAtPath:fixedPath isDirectory:nil])
+    return [fixedPath UTF8String];
 
-  paths = [binDir stringsByAppendingPaths:
-                [NSArray arrayWithObjects: @"libexec", @"bin", nil]];
-  pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
+  /* If we reach here either the path is absolute and therefore we
+     don't need to complete it, or we're unable to relocate the
+     file/directory.  If it's the latter it may be because the user is
+     trying to use a bundled app as though it's a Unix style install
+     and we have no way to guess what was intended, so return the
+     original string unaltered.  */
 
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
-
-  return NULL;
-}
-
-
-const char *
-ns_load_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the site-lisp and lisp directories.
-   Ie, site-lisp:lisp.  Otherwise, return nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath, *resourcePaths;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
-  NSArray *paths = [resourceDir stringsByAppendingPaths:
-                              [NSArray arrayWithObjects:
-                                         @"site-lisp", @"lisp", nil]];
-  NSEnumerator *pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
-
-  /* Hack to skip site-lisp.  */
-  if (no_site_lisp) resourcePath = [pathEnum nextObject];
-
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
+#endif
 
-  return NULL;
+  return epath;
 }
 
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-19 17:04                   ` Alan Third
@ 2021-06-22  3:55                     ` Matthew Bauer
  2021-06-22  8:32                       ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Matthew Bauer @ 2021-06-22  3:55 UTC (permalink / raw)
  To: Alan Third, Eli Zaretskii, 48994, Matthew Bauer, Andrea Corallo


[-- Attachment #1.1: Type: text/plain, Size: 5212 bytes --]

> Matthew, could you please try the attached patch and see if it solves
> your problems (or makes them worse!)?

That fixes the problem! I had to make a small change for it to work though
(needed to add prefix & exec_prefix to nextstep/Makefile.in).

On Sat, Jun 19, 2021 at 12:04 PM Alan Third <alan@idiocy.org> wrote:

> On Wed, Jun 16, 2021 at 09:45:50PM +0300, Eli Zaretskii wrote:
> > > Date: Wed, 16 Jun 2021 19:25:14 +0100
> > > From: Alan Third <alan@idiocy.org>
> > > Cc: 48994@debbugs.gnu.org, mjbauer95@gmail.com, akrl@sdf.org
> > >
> > > > My primary worry is not the Makefiles, it's what the installed Emacs
> > > > does upon startup to find its requisite files.
> > >
> > > I've done a lot of digging about and it looks like a bit of a mess,
> > > frankly.
> >
> > It could be.  But let me first ask you: did you understand how Emacs
> > finds the directory with the *.eln files, and how that is related to
> > finding the .pdmp file?  These two parts are looked for together, and
> > the place where we find the .pdmp file tells us directly where to look
> > for the *.eln files.
> >
> > If this is "messy" on macOS, in either of the two types of install,
> > then we should fix that first.
>
> I think I did understand that. As far as I can tell that bit actually
> works fine, as long as the pdmp file can be found the eln files can be
> found.
>
> > So we need to make NS-specific changes that will make this work
> > correctly both in Unix-style and the app bundle style install.  Do you
> > understand how to fix this part?  I'll gladly help you if needed.
>
> I've attached a patch which, I hope, should solve these problems. I've
> tried to align the code more with how other terms work, so I've
> removed all the functions that calculate the various paths Emacs uses,
> and replaced them with a single function that, given a relative path
> inside the application bundle, can work out what the full path should
> be, but will pass any absolute paths unchanged.
>
> It's similar to w32_relocate, but not exactly the same, and I'm using
> it in more places due to the app bundle layout being quite different.
>
> I've also followed the w32 lead in changing how epaths.h is generated.
> Basically I post-process it so that any paths that are internal to the
> app bundle are converted to relative paths, so the above mentioned
> function can deal with them.
>
> > > >   . Emacs binary in /usr/bin
> > > >   . the .pdmp file in /usr/libexec/emacs/VERSION/ARCHITECTURE
> > > >   . the *eln files in /usr/lib/emacs/VERSION/native-lisp
> > > >
> > > > ?
> > >
> > > Sorry, I must've missed it.
> > >
> > > Yes, a normal Unix style install should be like that.
> >
> > OK, then the Unix style install should already be working.  Then I
> > wonder how come this very install caused the OP trouble.  But we can
> > revisit it later, once the code whch looks for .pdmp and *.eln files
> > is finalized.
> >
> > > However the OP appears to be using a Unix style install with a
> > > different install prefix and is getting app bundle install paths in
> > > his errors, specifically Contents/Resources/lisp, which I mentioned at
> > > the top of the email.
> >
> > I wonder how this could happen, if the Unix style install is supposed
> > to "just work".
>
> I think it's because of some hard-coding of things that was added to
> work around problems in the app bundle code. They haven't been
> suitably separated from the Unix style install code, so they end up
> interfering with each other.
>
> I think that now a Unix style install shouldn't be affected at all by
> the app bundle code, but I'd like some extra testing by people who run
> it that way before I'm fully convinced I've been successful in that.
>
> > > > > I believe there's a ./configure flag. But like I said before, for
> the
> > > > > other paths there's a run-time check, so I'm not sure how it all
> ties
> > > > > together.
> > > >
> > > > Which run-time check did you have in mind?
> > >
> > > ns_exec_path, and friends. They return the app bundle paths if the
> > > directories exist, and nil otherwise, then the code that calls them
> > > modifies the search paths depending on the result.
> >
> > It's okay to use that, although I'd expect the install location to be
> > known at compile time?
>
> The install location is known, but in the app bundle case the app can
> be copied to anywhere and then the full paths will be different, so
> they can't just be hard coded at install time.
>
> > > I think I can probably fix this now, hopefully without breaking
> > > anything... ;)
> >
> > Fine, please do, but let's try doing that one step at a time...
>
> I tried, but it ended up quite a large patch as fixing one thing would
> invariably break another, and I decided I had to sort out the epaths.h
> situation before I could do much else, and most of the rest of the
> patch is dealing with the fallout from that.
>
> Anyway, it's attached. It's working here for the app bundle case and I
> believe it should work well for the Unix style install, but I've not
> tested that in any depth.
>
> Matthew, could you please try the attached patch and see if it solves
> your problems (or makes them worse!)?
> --
> Alan Third
>

[-- Attachment #1.2: Type: text/html, Size: 6697 bytes --]

[-- Attachment #2: 0001-Fix-NS-native-compilation-builds.patch --]
[-- Type: application/octet-stream, Size: 18432 bytes --]

From c062743661639b05ada7618b3968183298f33d06 Mon Sep 17 00:00:00 2001
From: Matthew Bauer <mjbauer95@gmail.com>
Date: Mon, 21 Jun 2021 21:18:05 -0500
Subject: [PATCH] Fix NS native compilation builds

* Makefile.in (ns_appdir): New variable.
(.PHONY): Include new rule.
(epaths-force-ns-self-contained): Remove the app bundle directory from
all paths.
* configure.ac (NS_SELF_CONTAINED): Set the default site-lisp
directory instead of hard-coding it in the ObjC code, and use the new
epaths generating make rule.
* src/callproc.c (init_callproc_1):
(init_callproc): Remove all the NS specific code as the special cases
are now handled by decode_env_path.
* src/emacs.c (load_pdump):
(decode_env_path): Use ns_relocate to find the correct directory after
relocation.
* src/lread.c (load_path_default): Remove all the NS specific code as
the special cases are now handled by decode_env_path.
* src/nsterm.h: Update function definitions.
* src/nsterm.m (ns_etc_directory):
(ns_exec_path):
(ns_load_path): Remove functions that are no longer needed.
(ns_relocate): New function to calculate paths within the NS app
bundle.
---
 Makefile.in          |  17 +++++-
 configure.ac         |  12 +++--
 nextstep/Makefile.in |  13 +++--
 src/callproc.c       |  36 ++-----------
 src/emacs.c          |  16 +++++-
 src/lread.c          |   7 +--
 src/nsterm.h         |   4 +-
 src/nsterm.m         | 125 ++++++++-----------------------------------
 8 files changed, 74 insertions(+), 156 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 3facfa59a9..68fe7d781a 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -104,8 +104,10 @@ HAVE_NATIVE_COMP = @HAVE_NATIVE_COMP@
 
 # Location to install Emacs.app under GNUstep / macOS.
 # Later values may use these.
+ns_appdir=@ns_appdir@
 ns_appbindir=@ns_appbindir@
 ns_appresdir=@ns_appresdir@
+ns_applibdir=@ns_applibdir@
 # Either yes or no depending on whether this is a relocatable Emacs.app.
 ns_self_contained=@ns_self_contained@
 
@@ -328,12 +330,12 @@ BIN_DESTDIR='$(DESTDIR)${bindir}/'
 ELN_DESTDIR = $(DESTDIR)${libdir}/emacs/${version}/
 else
 BIN_DESTDIR='${ns_appbindir}/'
-ELN_DESTDIR = ${ns_appresdir}/
+ELN_DESTDIR = ${ns_applibdir}/emacs/${version}/
 endif
 
 all: ${SUBDIR} info
 
-.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 etc-emacsver
+.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 epaths-force-ns-self-contained etc-emacsver
 
 # If configure were to just generate emacsver.tex from emacsver.tex.in
 # in the normal way, the timestamp of emacsver.tex would always be
@@ -402,6 +404,17 @@ epaths-force-w32:
 	  -e "/^.*#/s|@SRC@|$${w32srcdir}|g") &&		\
 	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
 
+# A NextStep style app bundle is relocatable, so instead of
+# hard-coding paths try to generate them at run-time.
+#
+# The paths are mostly the same, and the bundle paths are different
+# between macOS and GNUstep, so just replace any references to the app
+# bundle root itself with the relative path.
+epaths-force-ns-self-contained: epaths-force
+	@(sed < ${srcdir}/src/epaths.h > epaths.h.$$$$	\
+	  -e 's;${ns_appdir}/;;') &&			\
+	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
+
 lib-src src: $(NTDIR) lib
 
 src: lib-src
diff --git a/configure.ac b/configure.ac
index c828f8d782..05afdbc8b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1891,10 +1891,10 @@ if test "${with_ns}" != no; then
   # so avoid NS_IMPL_COCOA if macuvs.h is absent.
   # Even a headless Emacs can build macuvs.h, so this should let you bootstrap.
   if test "${opsys}" = darwin && test -f "$srcdir/src/macuvs.h"; then
-     lispdirrel=Contents/Resources/lisp
      NS_IMPL_COCOA=yes
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}/Contents/MacOS
+     ns_applibdir=${ns_appdir}/Contents/MacOS/lib
      ns_appresdir=${ns_appdir}/Contents/Resources
      ns_appsrc=Cocoa/Emacs.base
      ns_fontfile=macfont.o
@@ -1952,6 +1952,7 @@ fail;
   if test $NS_IMPL_GNUSTEP = yes; then
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}
+     ns_applibdir=${ns_appdir}/lib
      ns_appresdir=${ns_appdir}/Resources
      ns_appsrc=GNUstep/Emacs.base
      ns_fontfile=nsfont.o
@@ -2008,6 +2009,7 @@ if test "${HAVE_NS}" = yes; then
   window_system=nextstep
   # set up packaging dirs
   if test "${EN_NS_SELF_CONTAINED}" = yes; then
+     AC_DEFINE(NS_SELF_CONTAINED, 1, [Build an NS bundled app])
      ns_self_contained=yes
      prefix=${ns_appresdir}
      exec_prefix=${ns_appbindir}
@@ -2021,7 +2023,7 @@ if test "${HAVE_NS}" = yes; then
      infodir="\${ns_appresdir}/info"
      mandir="\${ns_appresdir}/man"
      lispdir="\${ns_appresdir}/lisp"
-     test "$locallisppathset" = no && locallisppath=""
+     test "$locallisppathset" = no && locallisppath="\${ns_appresdir}/site-lisp"
      INSTALL_ARCH_INDEP_EXTRA=
   fi
 
@@ -5409,6 +5411,7 @@ AC_SUBST(CFLAGS)
 AC_SUBST(X_TOOLKIT_TYPE)
 AC_SUBST(ns_appdir)
 AC_SUBST(ns_appbindir)
+AC_SUBST(ns_applibdir)
 AC_SUBST(ns_appresdir)
 AC_SUBST(ns_appsrc)
 AC_SUBST(GNU_OBJC_CFLAGS)
@@ -6009,10 +6012,13 @@ dnl the use of force in the 'epaths-force' rule in Makefile.in.
 AC_CONFIG_COMMANDS([src/epaths.h], [
 if test "${opsys}" = "mingw32"; then
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-w32
+elif test "$EN_NS_SELF_CONTAINED" = "yes"; then
+  ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-ns-self-contained
 else
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force
 fi || AC_MSG_ERROR(['src/epaths.h' could not be made.])
-], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"])
+], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"
+    EN_NS_SELF_CONTAINED="$EN_NS_SELF_CONTAINED"])
 
 dnl NB we have to cheat and use the ac_... version because abs_top_srcdir
 dnl is not yet set, sigh.  Or we could use ../$srcdir/src/.gdbinit,
diff --git a/nextstep/Makefile.in b/nextstep/Makefile.in
index 3168fee76c..8a5c656597 100644
--- a/nextstep/Makefile.in
+++ b/nextstep/Makefile.in
@@ -32,19 +32,23 @@ top_srcdir_abs = $(shell cd @top_srcdir@; pwd -P)
 
 MKDIR_P = @MKDIR_P@
 
+prefix=@prefix@
+exec_prefix=@exec_prefix@
+
 ## Emacs.app.
 ns_appdir = @ns_appdir@
 ## GNUstep: ns_appdir; macOS: ns_appdir/Contents/MacOS
 ns_appbindir = @ns_appbindir@
 ## GNUstep/Emacs.base or Cocoa/Emacs.base.
 ns_appsrc = @ns_appsrc@
+ns_applibexecdir = @libexecdir@
 ## GNUstep: GNUstep/Emacs.base/Resources/Info-gnustep.plist
 ## macOS: Cocoa/Emacs.base/Contents/Info.plist
 ns_check_file = @ns_appdir@/@ns_check_file@
 
 .PHONY: all
 
-all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_appbindir}/Emacs.pdmp
+all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_applibexecdir}/Emacs.pdmp
 
 ${ns_check_file} ${ns_appdir}: ${srcdir}/${ns_appsrc} ${ns_appsrc}
 	rm -rf ${ns_appdir}
@@ -63,8 +67,8 @@ ${ns_appbindir}/Emacs: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}
 	${MKDIR_P} ${ns_appbindir}
 	cp -f ../src/emacs${EXEEXT} $@
 
-${ns_appbindir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
-	${MKDIR_P} ${ns_appbindir}
+${ns_applibexecdir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
+	${MKDIR_P} ${ns_applibexecdir}
 	cp -f ../src/emacs${EXEEXT}.pdmp $@
 
 .PHONY: FORCE
@@ -85,9 +89,8 @@ links: ../src/emacs${EXEEXT}
 	ln -s $(top_srcdir_abs)/info ${ns_appdir}/Contents/Resources
 	${MKDIR_P} ${ns_appbindir}
 	ln -s $(abs_top_builddir)/src/emacs${EXEEXT} ${ns_appbindir}/Emacs
-	ln -s $(abs_top_builddir)/src/emacs${EXEEXT}.pdmp ${ns_appbindir}/Emacs.pdmp
 	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/bin
-	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/libexec
+	ln -s $(abs_top_builddir)/lib-src ${ns_applibexecdir}
 	${MKDIR_P} ${ns_appdir}/Contents/Resources/etc
 	for f in $(shell cd $(top_srcdir_abs)/etc; ls); do ln -s $(top_srcdir_abs)/etc/$$f ${ns_appdir}/Contents/Resources/etc; done
 	ln -s $(abs_top_builddir)/etc/DOC ${ns_appdir}/Contents/Resources/etc
diff --git a/src/callproc.c b/src/callproc.c
index e44e243680..aabc39313b 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1661,32 +1661,15 @@ make_environment_block (Lisp_Object current_dir)
 void
 init_callproc_1 (void)
 {
-#ifdef HAVE_NS
-  const char *etc_dir = ns_etc_directory ();
-  const char *path_exec = ns_exec_path ();
-#endif
-
-  Vdata_directory = decode_env_path ("EMACSDATA",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DATA, 0);
+  Vdata_directory = decode_env_path ("EMACSDATA", PATH_DATA, 0);
   Vdata_directory = Ffile_name_as_directory (Fcar (Vdata_directory));
 
-  Vdoc_directory = decode_env_path ("EMACSDOC",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DOC, 0);
+  Vdoc_directory = decode_env_path ("EMACSDOC", PATH_DOC, 0);
   Vdoc_directory = Ffile_name_as_directory (Fcar (Vdoc_directory));
 
   /* Check the EMACSPATH environment variable, defaulting to the
      PATH_EXEC path from epaths.h.  */
-  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-                                path_exec ? path_exec :
-#endif
-                                PATH_EXEC, 0);
+  Vexec_path = decode_env_path ("EMACSPATH", PATH_EXEC, 0);
   Vexec_directory = Ffile_name_as_directory (Fcar (Vexec_path));
   /* FIXME?  For ns, path_exec should go at the front?  */
   Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
@@ -1701,10 +1684,6 @@ init_callproc (void)
 
   char *sh;
   Lisp_Object tempdir;
-#ifdef HAVE_NS
-  if (data_dir == 0)
-    data_dir = ns_etc_directory () != 0;
-#endif
 
   if (!NILP (Vinstallation_directory))
     {
@@ -1716,15 +1695,8 @@ init_callproc (void)
 	  /* MSDOS uses wrapped binaries, so don't do this.  */
       if (NILP (Fmember (tem, Vexec_path)))
 	{
-#ifdef HAVE_NS
-	  const char *path_exec = ns_exec_path ();
-#endif
 	  /* Running uninstalled, so default to tem rather than PATH_EXEC.  */
-	  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-					path_exec ? path_exec :
-#endif
-					SSDATA (tem), 0);
+	  Vexec_path = decode_env_path ("EMACSPATH", SSDATA (tem), 0);
 	  Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
 	}
 
diff --git a/src/emacs.c b/src/emacs.c
index 60a57a693c..b7982ece64 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -835,7 +835,13 @@ load_pdump (int argc, char **argv)
     NULL
 #endif
     ;
-  const char *argv0_base = "emacs";
+  const char *argv0_base =
+#ifdef NS_SELF_CONTAINED
+    "Emacs"
+#else
+    "emacs"
+#endif
+    ;
 
   /* TODO: maybe more thoroughly scrub process environment in order to
      make this use case (loading a dump file in an unexeced emacs)
@@ -912,6 +918,8 @@ load_pdump (int argc, char **argv)
   /* On MS-Windows, PATH_EXEC normally starts with a literal
      "%emacs_dir%", so it will never work without some tweaking.  */
   path_exec = w32_relocate (path_exec);
+#elif defined (HAVE_NS)
+  path_exec = ns_relocate (path_exec);
 #endif
 
   /* Look for "emacs.pdmp" in PATH_EXEC.  We hardcode "emacs" in
@@ -929,6 +937,7 @@ load_pdump (int argc, char **argv)
     }
   sprintf (dump_file, "%s%c%s%s",
            path_exec, DIRECTORY_SEP, argv0_base, suffix);
+#if !defined (NS_SELF_CONTAINED)
   /* Assume the Emacs binary lives in a sibling directory as set up by
      the default installation configuration.  */
   const char *go_up = "../../../../bin/";
@@ -943,6 +952,7 @@ load_pdump (int argc, char **argv)
   sprintf (emacs_executable, "%s%c%s%s%s",
 	   path_exec, DIRECTORY_SEP, go_up, argv0_base,
 	   strip_suffix ? strip_suffix : "");
+#endif
   result = pdumper_load (dump_file, emacs_executable);
 
   if (result == PDUMPER_LOAD_FILE_NOT_FOUND)
@@ -2960,7 +2970,11 @@ decode_env_path (const char *evarname, const char *defalt, bool empty)
     path = 0;
   if (!path)
     {
+#ifdef NS_SELF_CONTAINED
+      path = ns_relocate (defalt);
+#else
       path = defalt;
+#endif
 #ifdef WINDOWSNT
       defaulted = 1;
 #endif
diff --git a/src/lread.c b/src/lread.c
index 0b33fd0f25..4617ffd626 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -4769,14 +4769,9 @@ load_path_default (void)
     return decode_env_path (0, PATH_DUMPLOADSEARCH, 0);
 
   Lisp_Object lpath = Qnil;
-  const char *normal = PATH_LOADSEARCH;
   const char *loadpath = NULL;
 
-#ifdef HAVE_NS
-  loadpath = ns_load_path ();
-#endif
-
-  lpath = decode_env_path (0, loadpath ? loadpath : normal, 0);
+  lpath = decode_env_path (0, PATH_LOADSEARCH, 0);
 
   if (!NILP (Vinstallation_directory))
     {
diff --git a/src/nsterm.h b/src/nsterm.h
index f64354b8a7..b29e76cc63 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1190,9 +1190,7 @@ extern void ns_run_ascript (void);
 #define NSAPP_DATA2_RUNFILEDIALOG 11
 extern void ns_run_file_dialog (void);
 
-extern const char *ns_etc_directory (void);
-extern const char *ns_exec_path (void);
-extern const char *ns_load_path (void);
+extern const char *ns_relocate (const char *epath);
 extern void syms_of_nsterm (void);
 extern void syms_of_nsfns (void);
 extern void syms_of_nsmenu (void);
diff --git a/src/nsterm.m b/src/nsterm.m
index e81a4cbc0d..e1a219a13b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -499,118 +499,35 @@ - (NSColor *)colorUsingDefaultColorSpace
 
 
 const char *
-ns_etc_directory (void)
-/* If running as a self-contained app bundle, return as a string the
-   filename of the etc directory, if present; else nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath;
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
+ns_relocate (const char *epath)
+/* If we're running in a self-contained app bundle some hard-coded
+   paths are relative to the root of the bundle, so work out the full
+   path.
 
-  resourcePath = [resourceDir stringByAppendingPathComponent: @"etc"];
-  if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-    {
-      if (isDir) return [resourcePath UTF8String];
-    }
-  return NULL;
-}
-
-
-const char *
-ns_exec_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the libexec and bin directories, ie libexec:bin.
-   Otherwise, return nil.
-   Normally, Emacs does not add its own bin/ directory to the PATH.
-   However, a self-contained NS build has a different layout, with
-   bin/ and libexec/ subdirectories in the directory that contains
-   Emacs.app itself.
-   We put libexec first, because init_callproc_1 uses the first
-   element to initialize exec-directory.  An alternative would be
-   for init_callproc to check for invocation-directory/libexec.
-*/
+   FIXME: I think this should be able to handle cases where multiple
+   directories are separated by colons.  */
 {
+#ifdef NS_SELF_CONTAINED
   NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *binDir = [bundle bundlePath];
-  NSString *resourcePath, *resourcePaths;
-  NSRange range;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
+  NSString *root = [bundle bundlePath];
+  NSString *original = [NSString stringWithUTF8String:epath];
+  NSString *fixedPath = [NSString pathWithComponents:@[root, original]];
   NSFileManager *fileManager = [NSFileManager defaultManager];
-  NSArray *paths;
-  NSEnumerator *pathEnum;
-  BOOL isDir;
 
-  range = [resourceDir rangeOfString: @"Contents"];
-  if (range.location != NSNotFound)
-    {
-      binDir = [binDir stringByAppendingPathComponent: @"Contents"];
-#ifdef NS_IMPL_COCOA
-      binDir = [binDir stringByAppendingPathComponent: @"MacOS"];
-#endif
-    }
+  if (![original isAbsolutePath]
+      && [fileManager fileExistsAtPath:fixedPath isDirectory:nil])
+    return [fixedPath UTF8String];
 
-  paths = [binDir stringsByAppendingPaths:
-                [NSArray arrayWithObjects: @"libexec", @"bin", nil]];
-  pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
+  /* If we reach here either the path is absolute and therefore we
+     don't need to complete it, or we're unable to relocate the
+     file/directory.  If it's the latter it may be because the user is
+     trying to use a bundled app as though it's a Unix style install
+     and we have no way to guess what was intended, so return the
+     original string unaltered.  */
 
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
-
-  return NULL;
-}
-
-
-const char *
-ns_load_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the site-lisp and lisp directories.
-   Ie, site-lisp:lisp.  Otherwise, return nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath, *resourcePaths;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
-  NSArray *paths = [resourceDir stringsByAppendingPaths:
-                              [NSArray arrayWithObjects:
-                                         @"site-lisp", @"lisp", nil]];
-  NSEnumerator *pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
-
-  /* Hack to skip site-lisp.  */
-  if (no_site_lisp) resourcePath = [pathEnum nextObject];
-
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
+#endif
 
-  return NULL;
+  return epath;
 }
 
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-22  3:55                     ` Matthew Bauer
@ 2021-06-22  8:32                       ` Alan Third
  2021-06-22 16:51                         ` Matthew Bauer
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-22  8:32 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 48994, Andrea Corallo

On Mon, Jun 21, 2021 at 10:55:40PM -0500, Matthew Bauer wrote:
> > Matthew, could you please try the attached patch and see if it solves
> > your problems (or makes them worse!)?
> 
> That fixes the problem! I had to make a small change for it to work though
> (needed to add prefix & exec_prefix to nextstep/Makefile.in).

!?

As far as I'm aware nextstep/Makefile shouldn't need to know about the
prefix because it's not relevant to that type of install...

What configure flags are you using? (They should be listed at the top
of configure.log.)
-- 
Alan Third





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-22  8:32                       ` Alan Third
@ 2021-06-22 16:51                         ` Matthew Bauer
  2021-06-22 16:59                           ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Matthew Bauer @ 2021-06-22 16:51 UTC (permalink / raw)
  To: Alan Third, Matthew Bauer, Eli Zaretskii, 48994, Andrea Corallo

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

> !?

> As far as I'm aware nextstep/Makefile shouldn't need to know about the
> prefix because it's not relevant to that type of install...

> What configure flags are you using? (They should be listed at the top
> of configure.log.)

There's probably a better to way to fix it - my familiarity with autoconf
is not great.

But in nextstep/Makefile we get this line:

ns_applibexecdir = ${exec_prefix}/libexec
>

and exec_prefix undefined without defining prefix & exec_prefix.

On Tue, Jun 22, 2021 at 3:32 AM Alan Third <alan@idiocy.org> wrote:

> On Mon, Jun 21, 2021 at 10:55:40PM -0500, Matthew Bauer wrote:
> > > Matthew, could you please try the attached patch and see if it solves
> > > your problems (or makes them worse!)?
> >
> > That fixes the problem! I had to make a small change for it to work
> though
> > (needed to add prefix & exec_prefix to nextstep/Makefile.in).
>
> !?
>
> As far as I'm aware nextstep/Makefile shouldn't need to know about the
> prefix because it's not relevant to that type of install...
>
> What configure flags are you using? (They should be listed at the top
> of configure.log.)
> --
> Alan Third
>

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-22 16:51                         ` Matthew Bauer
@ 2021-06-22 16:59                           ` Alan Third
  2021-06-22 17:24                             ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-22 16:59 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 48994, Andrea Corallo

On Tue, Jun 22, 2021 at 11:51:57AM -0500, Matthew Bauer wrote:
> > !?
> 
> > As far as I'm aware nextstep/Makefile shouldn't need to know about the
> > prefix because it's not relevant to that type of install...
> 
> > What configure flags are you using? (They should be listed at the top
> > of configure.log.)
> 
> There's probably a better to way to fix it - my familiarity with autoconf
> is not great.
> 
> But in nextstep/Makefile we get this line:
> 
> ns_applibexecdir = ${exec_prefix}/libexec
> >
> 
> and exec_prefix undefined without defining prefix & exec_prefix.

But you are building with the flag --disable-ns-self-contained?

If so there's no need for nextstep/Makefile to do anything, so it
shouldn't matter. I suppose we probably want to avoid generating that
makefile, or not call it, or something... I'm not sure.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-22 16:59                           ` Alan Third
@ 2021-06-22 17:24                             ` Alan Third
  2021-06-22 23:01                               ` Matthew Bauer
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-22 17:24 UTC (permalink / raw)
  To: Matthew Bauer, Eli Zaretskii, 48994, Andrea Corallo

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

On Tue, Jun 22, 2021 at 05:59:59PM +0100, Alan Third wrote:
> If so there's no need for nextstep/Makefile to do anything, so it
> shouldn't matter. I suppose we probably want to avoid generating that
> makefile, or not call it, or something... I'm not sure.

Turns out there's another variable that should only be set when
building the app.

Please try the attached patch.
-- 
Alan Third

[-- Attachment #2: v2-0001-Fix-NS-native-compilation-builds.patch --]
[-- Type: text/plain, Size: 18833 bytes --]

From 7e4f2fe5470e6a6468c57f9167c23726ec636208 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Wed, 16 Jun 2021 21:28:10 +0100
Subject: [PATCH v2] Fix NS native compilation builds

* Makefile.in (ns_appdir): New variable.
(.PHONY): Include new rule.
(epaths-force-ns-self-contained): Remove the app bundle directory from
all paths.
* configure.ac (NS_SELF_CONTAINED): Set the default site-lisp
directory instead of hard-coding it in the ObjC code, and use the new
epaths generating make rule.
* src/callproc.c (init_callproc_1):
(init_callproc): Remove all the NS specific code as the special cases
are now handled by decode_env_path.
* src/emacs.c (load_pdump):
(decode_env_path): Use ns_relocate to find the correct directory after
relocation.
* src/lread.c (load_path_default): Remove all the NS specific code as
the special cases are now handled by decode_env_path.
* src/nsterm.h: Update function definitions.
* src/nsterm.m (ns_etc_directory):
(ns_exec_path):
(ns_load_path): Remove functions that are no longer needed.
(ns_relocate): New function to calculate paths within the NS app
bundle.
---
 Makefile.in          |  17 +++++-
 configure.ac         |  14 +++--
 nextstep/Makefile.in |  10 ++--
 src/Makefile.in      |   2 +-
 src/callproc.c       |  36 ++-----------
 src/emacs.c          |  16 +++++-
 src/lread.c          |   7 +--
 src/nsterm.h         |   4 +-
 src/nsterm.m         | 125 ++++++++-----------------------------------
 9 files changed, 73 insertions(+), 158 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 3facfa59a9..68fe7d781a 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -104,8 +104,10 @@ HAVE_NATIVE_COMP =
 
 # Location to install Emacs.app under GNUstep / macOS.
 # Later values may use these.
+ns_appdir=@ns_appdir@
 ns_appbindir=@ns_appbindir@
 ns_appresdir=@ns_appresdir@
+ns_applibdir=@ns_applibdir@
 # Either yes or no depending on whether this is a relocatable Emacs.app.
 ns_self_contained=@ns_self_contained@
 
@@ -328,12 +330,12 @@ BIN_DESTDIR=
 ELN_DESTDIR = $(DESTDIR)${libdir}/emacs/${version}/
 else
 BIN_DESTDIR='${ns_appbindir}/'
-ELN_DESTDIR = ${ns_appresdir}/
+ELN_DESTDIR = ${ns_applibdir}/emacs/${version}/
 endif
 
 all: ${SUBDIR} info
 
-.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 etc-emacsver
+.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 epaths-force-ns-self-contained etc-emacsver
 
 # If configure were to just generate emacsver.tex from emacsver.tex.in
 # in the normal way, the timestamp of emacsver.tex would always be
@@ -402,6 +404,17 @@ epaths-force-w32:
 	  -e "/^.*#/s|@SRC@|$${w32srcdir}|g") &&		\
 	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
 
+# A NextStep style app bundle is relocatable, so instead of
+# hard-coding paths try to generate them at run-time.
+#
+# The paths are mostly the same, and the bundle paths are different
+# between macOS and GNUstep, so just replace any references to the app
+# bundle root itself with the relative path.
+epaths-force-ns-self-contained: epaths-force
+	@(sed < ${srcdir}/src/epaths.h > epaths.h.$$$$	\
+	  -e 's;${ns_appdir}/;;') &&			\
+	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
+
 lib-src src: $(NTDIR) lib
 
 src: lib-src
diff --git a/configure.ac b/configure.ac
index c828f8d782..bf03d10093 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1891,10 +1891,10 @@ AC_DEFUN
   # so avoid NS_IMPL_COCOA if macuvs.h is absent.
   # Even a headless Emacs can build macuvs.h, so this should let you bootstrap.
   if test "${opsys}" = darwin && test -f "$srcdir/src/macuvs.h"; then
-     lispdirrel=Contents/Resources/lisp
      NS_IMPL_COCOA=yes
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}/Contents/MacOS
+     ns_applibdir=${ns_appdir}/Contents/MacOS/lib
      ns_appresdir=${ns_appdir}/Contents/Resources
      ns_appsrc=Cocoa/Emacs.base
      ns_fontfile=macfont.o
@@ -1952,6 +1952,7 @@ AC_DEFUN
   if test $NS_IMPL_GNUSTEP = yes; then
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}
+     ns_applibdir=${ns_appdir}/lib
      ns_appresdir=${ns_appdir}/Resources
      ns_appsrc=GNUstep/Emacs.base
      ns_fontfile=nsfont.o
@@ -2008,6 +2009,7 @@ AC_DEFUN
   window_system=nextstep
   # set up packaging dirs
   if test "${EN_NS_SELF_CONTAINED}" = yes; then
+     AC_DEFINE(NS_SELF_CONTAINED, 1, [Build an NS bundled app])
      ns_self_contained=yes
      prefix=${ns_appresdir}
      exec_prefix=${ns_appbindir}
@@ -2021,8 +2023,9 @@ AC_DEFUN
      infodir="\${ns_appresdir}/info"
      mandir="\${ns_appresdir}/man"
      lispdir="\${ns_appresdir}/lisp"
-     test "$locallisppathset" = no && locallisppath=""
+     test "$locallisppathset" = no && locallisppath="\${ns_appresdir}/site-lisp"
      INSTALL_ARCH_INDEP_EXTRA=
+     OTHER_FILES=ns-app
   fi
 
   NS_OBJC_OBJ="nsterm.o nsfns.o nsmenu.o nsselect.o nsimage.o $ns_fontfile"
@@ -4071,7 +4074,6 @@ AC_DEFUN
       GNU_OBJC_CFLAGS="$GNU_OBJC_CFLAGS -fgnu-runtime -Wno-import -fconstant-string-class=NSConstantString -DGNUSTEP_BASE_LIBRARY=1 -DGNU_GUI_LIBRARY=1 -DGNU_RUNTIME=1 -DGSWARN -DGSDIAGNOSE"
     fi
   fi
-  OTHER_FILES=ns-app
 fi
 
 ### Use session management (-lSM -lICE) if available
@@ -5409,6 +5411,7 @@ AC_DEFUN
 AC_SUBST(X_TOOLKIT_TYPE)
 AC_SUBST(ns_appdir)
 AC_SUBST(ns_appbindir)
+AC_SUBST(ns_applibdir)
 AC_SUBST(ns_appresdir)
 AC_SUBST(ns_appsrc)
 AC_SUBST(GNU_OBJC_CFLAGS)
@@ -6009,10 +6012,13 @@ m4_define
 AC_CONFIG_COMMANDS([src/epaths.h], [
 if test "${opsys}" = "mingw32"; then
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-w32
+elif test "$EN_NS_SELF_CONTAINED" = "yes"; then
+  ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-ns-self-contained
 else
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force
 fi || AC_MSG_ERROR(['src/epaths.h' could not be made.])
-], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"])
+], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"
+    EN_NS_SELF_CONTAINED="$EN_NS_SELF_CONTAINED"])
 
 dnl NB we have to cheat and use the ac_... version because abs_top_srcdir
 dnl is not yet set, sigh.  Or we could use ../$srcdir/src/.gdbinit,
diff --git a/nextstep/Makefile.in b/nextstep/Makefile.in
index 3168fee76c..c0beb454d1 100644
--- a/nextstep/Makefile.in
+++ b/nextstep/Makefile.in
@@ -38,13 +38,14 @@ ns_appdir =
 ns_appbindir = @ns_appbindir@
 ## GNUstep/Emacs.base or Cocoa/Emacs.base.
 ns_appsrc = @ns_appsrc@
+ns_applibexecdir = @libexecdir@
 ## GNUstep: GNUstep/Emacs.base/Resources/Info-gnustep.plist
 ## macOS: Cocoa/Emacs.base/Contents/Info.plist
 ns_check_file = @ns_appdir@/@ns_check_file@
 
 .PHONY: all
 
-all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_appbindir}/Emacs.pdmp
+all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_applibexecdir}/Emacs.pdmp
 
 ${ns_check_file} ${ns_appdir}: ${srcdir}/${ns_appsrc} ${ns_appsrc}
 	rm -rf ${ns_appdir}
@@ -63,8 +64,8 @@ ${ns_appbindir}/Emacs:
 	${MKDIR_P} ${ns_appbindir}
 	cp -f ../src/emacs${EXEEXT} $@
 
-${ns_appbindir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
-	${MKDIR_P} ${ns_appbindir}
+${ns_applibexecdir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
+	${MKDIR_P} ${ns_applibexecdir}
 	cp -f ../src/emacs${EXEEXT}.pdmp $@
 
 .PHONY: FORCE
@@ -85,9 +86,8 @@ links:
 	ln -s $(top_srcdir_abs)/info ${ns_appdir}/Contents/Resources
 	${MKDIR_P} ${ns_appbindir}
 	ln -s $(abs_top_builddir)/src/emacs${EXEEXT} ${ns_appbindir}/Emacs
-	ln -s $(abs_top_builddir)/src/emacs${EXEEXT}.pdmp ${ns_appbindir}/Emacs.pdmp
 	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/bin
-	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/libexec
+	ln -s $(abs_top_builddir)/lib-src ${ns_applibexecdir}
 	${MKDIR_P} ${ns_appdir}/Contents/Resources/etc
 	for f in $(shell cd $(top_srcdir_abs)/etc; ls); do ln -s $(top_srcdir_abs)/etc/$$f ${ns_appdir}/Contents/Resources/etc; done
 	ln -s $(abs_top_builddir)/etc/DOC ${ns_appdir}/Contents/Resources/etc
diff --git a/src/Makefile.in b/src/Makefile.in
index 79cddb35b5..22c7aeed5c 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -55,7 +55,7 @@ lwlibdir =
 # Configuration files for .o files to depend on.
 config_h = config.h $(srcdir)/conf_post.h
 
-## ns-app if HAVE_NS, else empty.
+## ns-app if NS self contained app, else empty.
 OTHER_FILES = @OTHER_FILES@
 
 ## Flags to pass for profiling builds
diff --git a/src/callproc.c b/src/callproc.c
index e44e243680..aabc39313b 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1661,32 +1661,15 @@ make_environment_block (Lisp_Object current_dir)
 void
 init_callproc_1 (void)
 {
-#ifdef HAVE_NS
-  const char *etc_dir = ns_etc_directory ();
-  const char *path_exec = ns_exec_path ();
-#endif
-
-  Vdata_directory = decode_env_path ("EMACSDATA",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DATA, 0);
+  Vdata_directory = decode_env_path ("EMACSDATA", PATH_DATA, 0);
   Vdata_directory = Ffile_name_as_directory (Fcar (Vdata_directory));
 
-  Vdoc_directory = decode_env_path ("EMACSDOC",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DOC, 0);
+  Vdoc_directory = decode_env_path ("EMACSDOC", PATH_DOC, 0);
   Vdoc_directory = Ffile_name_as_directory (Fcar (Vdoc_directory));
 
   /* Check the EMACSPATH environment variable, defaulting to the
      PATH_EXEC path from epaths.h.  */
-  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-                                path_exec ? path_exec :
-#endif
-                                PATH_EXEC, 0);
+  Vexec_path = decode_env_path ("EMACSPATH", PATH_EXEC, 0);
   Vexec_directory = Ffile_name_as_directory (Fcar (Vexec_path));
   /* FIXME?  For ns, path_exec should go at the front?  */
   Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
@@ -1701,10 +1684,6 @@ init_callproc (void)
 
   char *sh;
   Lisp_Object tempdir;
-#ifdef HAVE_NS
-  if (data_dir == 0)
-    data_dir = ns_etc_directory () != 0;
-#endif
 
   if (!NILP (Vinstallation_directory))
     {
@@ -1716,15 +1695,8 @@ init_callproc (void)
 	  /* MSDOS uses wrapped binaries, so don't do this.  */
       if (NILP (Fmember (tem, Vexec_path)))
 	{
-#ifdef HAVE_NS
-	  const char *path_exec = ns_exec_path ();
-#endif
 	  /* Running uninstalled, so default to tem rather than PATH_EXEC.  */
-	  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-					path_exec ? path_exec :
-#endif
-					SSDATA (tem), 0);
+	  Vexec_path = decode_env_path ("EMACSPATH", SSDATA (tem), 0);
 	  Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
 	}
 
diff --git a/src/emacs.c b/src/emacs.c
index 60a57a693c..b7982ece64 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -835,7 +835,13 @@ load_pdump (int argc, char **argv)
     NULL
 #endif
     ;
-  const char *argv0_base = "emacs";
+  const char *argv0_base =
+#ifdef NS_SELF_CONTAINED
+    "Emacs"
+#else
+    "emacs"
+#endif
+    ;
 
   /* TODO: maybe more thoroughly scrub process environment in order to
      make this use case (loading a dump file in an unexeced emacs)
@@ -912,6 +918,8 @@ load_pdump (int argc, char **argv)
   /* On MS-Windows, PATH_EXEC normally starts with a literal
      "%emacs_dir%", so it will never work without some tweaking.  */
   path_exec = w32_relocate (path_exec);
+#elif defined (HAVE_NS)
+  path_exec = ns_relocate (path_exec);
 #endif
 
   /* Look for "emacs.pdmp" in PATH_EXEC.  We hardcode "emacs" in
@@ -929,6 +937,7 @@ load_pdump (int argc, char **argv)
     }
   sprintf (dump_file, "%s%c%s%s",
            path_exec, DIRECTORY_SEP, argv0_base, suffix);
+#if !defined (NS_SELF_CONTAINED)
   /* Assume the Emacs binary lives in a sibling directory as set up by
      the default installation configuration.  */
   const char *go_up = "../../../../bin/";
@@ -943,6 +952,7 @@ load_pdump (int argc, char **argv)
   sprintf (emacs_executable, "%s%c%s%s%s",
 	   path_exec, DIRECTORY_SEP, go_up, argv0_base,
 	   strip_suffix ? strip_suffix : "");
+#endif
   result = pdumper_load (dump_file, emacs_executable);
 
   if (result == PDUMPER_LOAD_FILE_NOT_FOUND)
@@ -2960,7 +2970,11 @@ decode_env_path (const char *evarname, const char *defalt, bool empty)
     path = 0;
   if (!path)
     {
+#ifdef NS_SELF_CONTAINED
+      path = ns_relocate (defalt);
+#else
       path = defalt;
+#endif
 #ifdef WINDOWSNT
       defaulted = 1;
 #endif
diff --git a/src/lread.c b/src/lread.c
index 0b33fd0f25..4617ffd626 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -4769,14 +4769,9 @@ load_path_default (void)
     return decode_env_path (0, PATH_DUMPLOADSEARCH, 0);
 
   Lisp_Object lpath = Qnil;
-  const char *normal = PATH_LOADSEARCH;
   const char *loadpath = NULL;
 
-#ifdef HAVE_NS
-  loadpath = ns_load_path ();
-#endif
-
-  lpath = decode_env_path (0, loadpath ? loadpath : normal, 0);
+  lpath = decode_env_path (0, PATH_LOADSEARCH, 0);
 
   if (!NILP (Vinstallation_directory))
     {
diff --git a/src/nsterm.h b/src/nsterm.h
index e7ea907569..139c37a29e 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1186,9 +1186,7 @@ #define NSAPP_DATA2_RUNASSCRIPT 10
 #define NSAPP_DATA2_RUNFILEDIALOG 11
 extern void ns_run_file_dialog (void);
 
-extern const char *ns_etc_directory (void);
-extern const char *ns_exec_path (void);
-extern const char *ns_load_path (void);
+extern const char *ns_relocate (const char *epath);
 extern void syms_of_nsterm (void);
 extern void syms_of_nsfns (void);
 extern void syms_of_nsmenu (void);
diff --git a/src/nsterm.m b/src/nsterm.m
index 838c14d5ab..7b0b169e10 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -499,118 +499,35 @@ - (NSColor *)colorUsingDefaultColorSpace
 
 
 const char *
-ns_etc_directory (void)
-/* If running as a self-contained app bundle, return as a string the
-   filename of the etc directory, if present; else nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath;
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
+ns_relocate (const char *epath)
+/* If we're running in a self-contained app bundle some hard-coded
+   paths are relative to the root of the bundle, so work out the full
+   path.
 
-  resourcePath = [resourceDir stringByAppendingPathComponent: @"etc"];
-  if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-    {
-      if (isDir) return [resourcePath UTF8String];
-    }
-  return NULL;
-}
-
-
-const char *
-ns_exec_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the libexec and bin directories, ie libexec:bin.
-   Otherwise, return nil.
-   Normally, Emacs does not add its own bin/ directory to the PATH.
-   However, a self-contained NS build has a different layout, with
-   bin/ and libexec/ subdirectories in the directory that contains
-   Emacs.app itself.
-   We put libexec first, because init_callproc_1 uses the first
-   element to initialize exec-directory.  An alternative would be
-   for init_callproc to check for invocation-directory/libexec.
-*/
+   FIXME: I think this should be able to handle cases where multiple
+   directories are separated by colons.  */
 {
+#ifdef NS_SELF_CONTAINED
   NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *binDir = [bundle bundlePath];
-  NSString *resourcePath, *resourcePaths;
-  NSRange range;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
+  NSString *root = [bundle bundlePath];
+  NSString *original = [NSString stringWithUTF8String:epath];
+  NSString *fixedPath = [NSString pathWithComponents:@[root, original]];
   NSFileManager *fileManager = [NSFileManager defaultManager];
-  NSArray *paths;
-  NSEnumerator *pathEnum;
-  BOOL isDir;
 
-  range = [resourceDir rangeOfString: @"Contents"];
-  if (range.location != NSNotFound)
-    {
-      binDir = [binDir stringByAppendingPathComponent: @"Contents"];
-#ifdef NS_IMPL_COCOA
-      binDir = [binDir stringByAppendingPathComponent: @"MacOS"];
-#endif
-    }
+  if (![original isAbsolutePath]
+      && [fileManager fileExistsAtPath:fixedPath isDirectory:nil])
+    return [fixedPath UTF8String];
 
-  paths = [binDir stringsByAppendingPaths:
-                [NSArray arrayWithObjects: @"libexec", @"bin", nil]];
-  pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
+  /* If we reach here either the path is absolute and therefore we
+     don't need to complete it, or we're unable to relocate the
+     file/directory.  If it's the latter it may be because the user is
+     trying to use a bundled app as though it's a Unix style install
+     and we have no way to guess what was intended, so return the
+     original string unaltered.  */
 
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
-
-  return NULL;
-}
-
-
-const char *
-ns_load_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the site-lisp and lisp directories.
-   Ie, site-lisp:lisp.  Otherwise, return nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath, *resourcePaths;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
-  NSArray *paths = [resourceDir stringsByAppendingPaths:
-                              [NSArray arrayWithObjects:
-                                         @"site-lisp", @"lisp", nil]];
-  NSEnumerator *pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
-
-  /* Hack to skip site-lisp.  */
-  if (no_site_lisp) resourcePath = [pathEnum nextObject];
-
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
+#endif
 
-  return NULL;
+  return epath;
 }
 
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-22 17:24                             ` Alan Third
@ 2021-06-22 23:01                               ` Matthew Bauer
  2021-06-23 15:59                                 ` Alan Third
  0 siblings, 1 reply; 21+ messages in thread
From: Matthew Bauer @ 2021-06-22 23:01 UTC (permalink / raw)
  To: Alan Third, Matthew Bauer, Eli Zaretskii, 48994, Andrea Corallo

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

> 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

On Tue, Jun 22, 2021 at 12:24 PM Alan Third <alan@idiocy.org> wrote:

> On Tue, Jun 22, 2021 at 05:59:59PM +0100, Alan Third wrote:
> > If so there's no need for nextstep/Makefile to do anything, so it
> > shouldn't matter. I suppose we probably want to avoid generating that
> > makefile, or not call it, or something... I'm not sure.
>
> Turns out there's another variable that should only be set when
> building the app.
>
> Please try the attached patch.
> --
> Alan Third
>

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-22 23:01                               ` Matthew Bauer
@ 2021-06-23 15:59                                 ` Alan Third
  2021-06-23 20:46                                   ` Matthew Bauer
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Third @ 2021-06-23 15:59 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 48994, Andrea Corallo

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

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

[-- Attachment #2: v3-0001-Fix-NS-native-compilation-builds.patch --]
[-- Type: text/x-diff, Size: 19113 bytes --]

From 230988308c157ff07ae9b7d2ce51ba1913c81fe2 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Wed, 16 Jun 2021 21:28:10 +0100
Subject: [PATCH v3] Fix NS native compilation builds

* Makefile.in (ns_applibexecdir):
(ns_applibdir):
(ns_appdir): New variables.
(.PHONY): Include new rule.
(epaths-force-ns-self-contained): Remove the app bundle directory from
all paths.
* configure.ac (NS_SELF_CONTAINED): Set the default site-lisp
directory instead of hard-coding it in the ObjC code, and use the new
epaths generating make rule.
* src/callproc.c (init_callproc_1):
(init_callproc): Remove all the NS specific code as the special cases
are now handled by decode_env_path.
* src/emacs.c (load_pdump):
(decode_env_path): Use ns_relocate to find the correct directory after
relocation.
* src/lread.c (load_path_default): Remove all the NS specific code as
the special cases are now handled by decode_env_path.
* src/nsterm.h: Update function definitions.
* src/nsterm.m (ns_etc_directory):
(ns_exec_path):
(ns_load_path): Remove functions that are no longer needed.
(ns_relocate): New function to calculate paths within the NS app
bundle.
* nextstep/Makefile.in (ns_applibexecdir): New variable, and update
anything relying on the libexec location.
---
 Makefile.in          |  18 ++++++-
 configure.ac         |  19 +++++--
 nextstep/Makefile.in |  10 ++--
 src/Makefile.in      |   2 +-
 src/callproc.c       |  36 ++-----------
 src/emacs.c          |  16 +++++-
 src/lread.c          |   7 +--
 src/nsterm.h         |   4 +-
 src/nsterm.m         | 125 ++++++++-----------------------------------
 9 files changed, 78 insertions(+), 159 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index b750288023..420cb544a4 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -106,8 +106,11 @@ USE_STARTUP_NOTIFICATION =
 
 # Location to install Emacs.app under GNUstep / macOS.
 # Later values may use these.
+ns_appdir=@ns_appdir@
 ns_appbindir=@ns_appbindir@
+ns_applibexecdir=@ns_applibexecdir@
 ns_appresdir=@ns_appresdir@
+ns_applibdir=@ns_applibdir@
 # Either yes or no depending on whether this is a relocatable Emacs.app.
 ns_self_contained=@ns_self_contained@
 
@@ -330,12 +333,12 @@ BIN_DESTDIR=
 ELN_DESTDIR = $(DESTDIR)${libdir}/emacs/${version}/
 else
 BIN_DESTDIR='${ns_appbindir}/'
-ELN_DESTDIR = ${ns_appresdir}/
+ELN_DESTDIR = ${ns_applibdir}/emacs/${version}/
 endif
 
 all: ${SUBDIR} info
 
-.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 etc-emacsver
+.PHONY: all ${SUBDIR} blessmail epaths-force epaths-force-w32 epaths-force-ns-self-contained etc-emacsver
 
 # If configure were to just generate emacsver.tex from emacsver.tex.in
 # in the normal way, the timestamp of emacsver.tex would always be
@@ -404,6 +407,17 @@ epaths-force-w32:
 	  -e "/^.*#/s|@SRC@|$${w32srcdir}|g") &&		\
 	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
 
+# A NextStep style app bundle is relocatable, so instead of
+# hard-coding paths try to generate them at run-time.
+#
+# The paths are mostly the same, and the bundle paths are different
+# between macOS and GNUstep, so just replace any references to the app
+# bundle root itself with the relative path.
+epaths-force-ns-self-contained: epaths-force
+	@(sed < ${srcdir}/src/epaths.h > epaths.h.$$$$	\
+	  -e 's;${ns_appdir}/;;') &&			\
+	${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
+
 lib-src src: $(NTDIR) lib
 
 src: lib-src
diff --git a/configure.ac b/configure.ac
index 830f33844b..92527056b9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1891,10 +1891,11 @@ AC_DEFUN
   # so avoid NS_IMPL_COCOA if macuvs.h is absent.
   # Even a headless Emacs can build macuvs.h, so this should let you bootstrap.
   if test "${opsys}" = darwin && test -f "$srcdir/src/macuvs.h"; then
-     lispdirrel=Contents/Resources/lisp
      NS_IMPL_COCOA=yes
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}/Contents/MacOS
+     ns_applibexecdir=${ns_appdir}/Contents/MacOS/libexec
+     ns_applibdir=${ns_appdir}/Contents/MacOS/lib
      ns_appresdir=${ns_appdir}/Contents/Resources
      ns_appsrc=Cocoa/Emacs.base
      ns_fontfile=macfont.o
@@ -1952,6 +1953,8 @@ AC_DEFUN
   if test $NS_IMPL_GNUSTEP = yes; then
      ns_appdir=`pwd`/nextstep/Emacs.app
      ns_appbindir=${ns_appdir}
+     ns_applibexecdir=${ns_appdir}/libexec
+     ns_applibdir=${ns_appdir}/lib
      ns_appresdir=${ns_appdir}/Resources
      ns_appsrc=GNUstep/Emacs.base
      ns_fontfile=nsfont.o
@@ -2008,12 +2011,13 @@ AC_DEFUN
   window_system=nextstep
   # set up packaging dirs
   if test "${EN_NS_SELF_CONTAINED}" = yes; then
+     AC_DEFINE(NS_SELF_CONTAINED, 1, [Build an NS bundled app])
      ns_self_contained=yes
      prefix=${ns_appresdir}
      exec_prefix=${ns_appbindir}
      dnl This one isn't really used, only archlibdir is.
-     libexecdir="\${ns_appbindir}/libexec"
-     archlibdir="\${ns_appbindir}/libexec"
+     libexecdir="\${ns_applibexecdir}"
+     archlibdir="\${ns_applibexecdir}"
      etcdocdir="\${ns_appresdir}/etc"
      etcdir="\${ns_appresdir}/etc"
      dnl FIXME maybe set datarootdir instead.
@@ -2021,7 +2025,7 @@ AC_DEFUN
      infodir="\${ns_appresdir}/info"
      mandir="\${ns_appresdir}/man"
      lispdir="\${ns_appresdir}/lisp"
-     test "$locallisppathset" = no && locallisppath=""
+     test "$locallisppathset" = no && locallisppath="\${ns_appresdir}/site-lisp"
      INSTALL_ARCH_INDEP_EXTRA=
   fi
 
@@ -5414,6 +5418,8 @@ AC_DEFUN
 AC_SUBST(X_TOOLKIT_TYPE)
 AC_SUBST(ns_appdir)
 AC_SUBST(ns_appbindir)
+AC_SUBST(ns_applibexecdir)
+AC_SUBST(ns_applibdir)
 AC_SUBST(ns_appresdir)
 AC_SUBST(ns_appsrc)
 AC_SUBST(GNU_OBJC_CFLAGS)
@@ -6014,10 +6020,13 @@ m4_define
 AC_CONFIG_COMMANDS([src/epaths.h], [
 if test "${opsys}" = "mingw32"; then
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-w32
+elif test "$EN_NS_SELF_CONTAINED" = "yes"; then
+  ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force-ns-self-contained
 else
   ${MAKE-make} MAKEFILE_NAME=do-not-make-Makefile epaths-force
 fi || AC_MSG_ERROR(['src/epaths.h' could not be made.])
-], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"])
+], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"
+    EN_NS_SELF_CONTAINED="$EN_NS_SELF_CONTAINED"])
 
 dnl NB we have to cheat and use the ac_... version because abs_top_srcdir
 dnl is not yet set, sigh.  Or we could use ../$srcdir/src/.gdbinit,
diff --git a/nextstep/Makefile.in b/nextstep/Makefile.in
index 3168fee76c..f556663510 100644
--- a/nextstep/Makefile.in
+++ b/nextstep/Makefile.in
@@ -36,6 +36,7 @@ MKDIR_P =
 ns_appdir = @ns_appdir@
 ## GNUstep: ns_appdir; macOS: ns_appdir/Contents/MacOS
 ns_appbindir = @ns_appbindir@
+ns_applibexecdir = @ns_applibexecdir@
 ## GNUstep/Emacs.base or Cocoa/Emacs.base.
 ns_appsrc = @ns_appsrc@
 ## GNUstep: GNUstep/Emacs.base/Resources/Info-gnustep.plist
@@ -44,7 +45,7 @@ ns_check_file =
 
 .PHONY: all
 
-all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_appbindir}/Emacs.pdmp
+all: ${ns_appdir} ${ns_appbindir}/Emacs ${ns_applibexecdir}/Emacs.pdmp
 
 ${ns_check_file} ${ns_appdir}: ${srcdir}/${ns_appsrc} ${ns_appsrc}
 	rm -rf ${ns_appdir}
@@ -63,8 +64,8 @@ ${ns_appbindir}/Emacs:
 	${MKDIR_P} ${ns_appbindir}
 	cp -f ../src/emacs${EXEEXT} $@
 
-${ns_appbindir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
-	${MKDIR_P} ${ns_appbindir}
+${ns_applibexecdir}/Emacs.pdmp: ${ns_appdir} ${ns_check_file} ../src/emacs${EXEEXT}.pdmp
+	${MKDIR_P} ${ns_applibexecdir}
 	cp -f ../src/emacs${EXEEXT}.pdmp $@
 
 .PHONY: FORCE
@@ -85,9 +86,8 @@ links:
 	ln -s $(top_srcdir_abs)/info ${ns_appdir}/Contents/Resources
 	${MKDIR_P} ${ns_appbindir}
 	ln -s $(abs_top_builddir)/src/emacs${EXEEXT} ${ns_appbindir}/Emacs
-	ln -s $(abs_top_builddir)/src/emacs${EXEEXT}.pdmp ${ns_appbindir}/Emacs.pdmp
 	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/bin
-	ln -s $(abs_top_builddir)/lib-src ${ns_appbindir}/libexec
+	ln -s $(abs_top_builddir)/lib-src ${ns_applibexecdir}
 	${MKDIR_P} ${ns_appdir}/Contents/Resources/etc
 	for f in $(shell cd $(top_srcdir_abs)/etc; ls); do ln -s $(top_srcdir_abs)/etc/$$f ${ns_appdir}/Contents/Resources/etc; done
 	ln -s $(abs_top_builddir)/etc/DOC ${ns_appdir}/Contents/Resources/etc
diff --git a/src/Makefile.in b/src/Makefile.in
index 79cddb35b5..22c7aeed5c 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -55,7 +55,7 @@ lwlibdir =
 # Configuration files for .o files to depend on.
 config_h = config.h $(srcdir)/conf_post.h
 
-## ns-app if HAVE_NS, else empty.
+## ns-app if NS self contained app, else empty.
 OTHER_FILES = @OTHER_FILES@
 
 ## Flags to pass for profiling builds
diff --git a/src/callproc.c b/src/callproc.c
index e44e243680..aabc39313b 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1661,32 +1661,15 @@ make_environment_block (Lisp_Object current_dir)
 void
 init_callproc_1 (void)
 {
-#ifdef HAVE_NS
-  const char *etc_dir = ns_etc_directory ();
-  const char *path_exec = ns_exec_path ();
-#endif
-
-  Vdata_directory = decode_env_path ("EMACSDATA",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DATA, 0);
+  Vdata_directory = decode_env_path ("EMACSDATA", PATH_DATA, 0);
   Vdata_directory = Ffile_name_as_directory (Fcar (Vdata_directory));
 
-  Vdoc_directory = decode_env_path ("EMACSDOC",
-#ifdef HAVE_NS
-                                             etc_dir ? etc_dir :
-#endif
-                                             PATH_DOC, 0);
+  Vdoc_directory = decode_env_path ("EMACSDOC", PATH_DOC, 0);
   Vdoc_directory = Ffile_name_as_directory (Fcar (Vdoc_directory));
 
   /* Check the EMACSPATH environment variable, defaulting to the
      PATH_EXEC path from epaths.h.  */
-  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-                                path_exec ? path_exec :
-#endif
-                                PATH_EXEC, 0);
+  Vexec_path = decode_env_path ("EMACSPATH", PATH_EXEC, 0);
   Vexec_directory = Ffile_name_as_directory (Fcar (Vexec_path));
   /* FIXME?  For ns, path_exec should go at the front?  */
   Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
@@ -1701,10 +1684,6 @@ init_callproc (void)
 
   char *sh;
   Lisp_Object tempdir;
-#ifdef HAVE_NS
-  if (data_dir == 0)
-    data_dir = ns_etc_directory () != 0;
-#endif
 
   if (!NILP (Vinstallation_directory))
     {
@@ -1716,15 +1695,8 @@ init_callproc (void)
 	  /* MSDOS uses wrapped binaries, so don't do this.  */
       if (NILP (Fmember (tem, Vexec_path)))
 	{
-#ifdef HAVE_NS
-	  const char *path_exec = ns_exec_path ();
-#endif
 	  /* Running uninstalled, so default to tem rather than PATH_EXEC.  */
-	  Vexec_path = decode_env_path ("EMACSPATH",
-#ifdef HAVE_NS
-					path_exec ? path_exec :
-#endif
-					SSDATA (tem), 0);
+	  Vexec_path = decode_env_path ("EMACSPATH", SSDATA (tem), 0);
 	  Vexec_path = nconc2 (decode_env_path ("PATH", "", 0), Vexec_path);
 	}
 
diff --git a/src/emacs.c b/src/emacs.c
index 60a57a693c..b7982ece64 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -835,7 +835,13 @@ load_pdump (int argc, char **argv)
     NULL
 #endif
     ;
-  const char *argv0_base = "emacs";
+  const char *argv0_base =
+#ifdef NS_SELF_CONTAINED
+    "Emacs"
+#else
+    "emacs"
+#endif
+    ;
 
   /* TODO: maybe more thoroughly scrub process environment in order to
      make this use case (loading a dump file in an unexeced emacs)
@@ -912,6 +918,8 @@ load_pdump (int argc, char **argv)
   /* On MS-Windows, PATH_EXEC normally starts with a literal
      "%emacs_dir%", so it will never work without some tweaking.  */
   path_exec = w32_relocate (path_exec);
+#elif defined (HAVE_NS)
+  path_exec = ns_relocate (path_exec);
 #endif
 
   /* Look for "emacs.pdmp" in PATH_EXEC.  We hardcode "emacs" in
@@ -929,6 +937,7 @@ load_pdump (int argc, char **argv)
     }
   sprintf (dump_file, "%s%c%s%s",
            path_exec, DIRECTORY_SEP, argv0_base, suffix);
+#if !defined (NS_SELF_CONTAINED)
   /* Assume the Emacs binary lives in a sibling directory as set up by
      the default installation configuration.  */
   const char *go_up = "../../../../bin/";
@@ -943,6 +952,7 @@ load_pdump (int argc, char **argv)
   sprintf (emacs_executable, "%s%c%s%s%s",
 	   path_exec, DIRECTORY_SEP, go_up, argv0_base,
 	   strip_suffix ? strip_suffix : "");
+#endif
   result = pdumper_load (dump_file, emacs_executable);
 
   if (result == PDUMPER_LOAD_FILE_NOT_FOUND)
@@ -2960,7 +2970,11 @@ decode_env_path (const char *evarname, const char *defalt, bool empty)
     path = 0;
   if (!path)
     {
+#ifdef NS_SELF_CONTAINED
+      path = ns_relocate (defalt);
+#else
       path = defalt;
+#endif
 #ifdef WINDOWSNT
       defaulted = 1;
 #endif
diff --git a/src/lread.c b/src/lread.c
index 0b33fd0f25..4617ffd626 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -4769,14 +4769,9 @@ load_path_default (void)
     return decode_env_path (0, PATH_DUMPLOADSEARCH, 0);
 
   Lisp_Object lpath = Qnil;
-  const char *normal = PATH_LOADSEARCH;
   const char *loadpath = NULL;
 
-#ifdef HAVE_NS
-  loadpath = ns_load_path ();
-#endif
-
-  lpath = decode_env_path (0, loadpath ? loadpath : normal, 0);
+  lpath = decode_env_path (0, PATH_LOADSEARCH, 0);
 
   if (!NILP (Vinstallation_directory))
     {
diff --git a/src/nsterm.h b/src/nsterm.h
index f64354b8a7..b29e76cc63 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1190,9 +1190,7 @@ #define NSAPP_DATA2_RUNASSCRIPT 10
 #define NSAPP_DATA2_RUNFILEDIALOG 11
 extern void ns_run_file_dialog (void);
 
-extern const char *ns_etc_directory (void);
-extern const char *ns_exec_path (void);
-extern const char *ns_load_path (void);
+extern const char *ns_relocate (const char *epath);
 extern void syms_of_nsterm (void);
 extern void syms_of_nsfns (void);
 extern void syms_of_nsmenu (void);
diff --git a/src/nsterm.m b/src/nsterm.m
index e81a4cbc0d..8497138039 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -499,118 +499,35 @@ - (NSColor *)colorUsingDefaultColorSpace
 
 
 const char *
-ns_etc_directory (void)
-/* If running as a self-contained app bundle, return as a string the
-   filename of the etc directory, if present; else nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath;
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
+ns_relocate (const char *epath)
+/* If we're running in a self-contained app bundle some hard-coded
+   paths are relative to the root of the bundle, so work out the full
+   path.
 
-  resourcePath = [resourceDir stringByAppendingPathComponent: @"etc"];
-  if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-    {
-      if (isDir) return [resourcePath UTF8String];
-    }
-  return NULL;
-}
-
-
-const char *
-ns_exec_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the libexec and bin directories, ie libexec:bin.
-   Otherwise, return nil.
-   Normally, Emacs does not add its own bin/ directory to the PATH.
-   However, a self-contained NS build has a different layout, with
-   bin/ and libexec/ subdirectories in the directory that contains
-   Emacs.app itself.
-   We put libexec first, because init_callproc_1 uses the first
-   element to initialize exec-directory.  An alternative would be
-   for init_callproc to check for invocation-directory/libexec.
-*/
+   FIXME: I think this should be able to handle cases where multiple
+   directories are separated by colons.  */
 {
+#ifdef NS_SELF_CONTAINED
   NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *binDir = [bundle bundlePath];
-  NSString *resourcePath, *resourcePaths;
-  NSRange range;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
+  NSString *root = [bundle bundlePath];
+  NSString *original = [NSString stringWithUTF8String:epath];
+  NSString *fixedPath = [NSString pathWithComponents:@[root, original]];
   NSFileManager *fileManager = [NSFileManager defaultManager];
-  NSArray *paths;
-  NSEnumerator *pathEnum;
-  BOOL isDir;
 
-  range = [resourceDir rangeOfString: @"Contents"];
-  if (range.location != NSNotFound)
-    {
-      binDir = [binDir stringByAppendingPathComponent: @"Contents"];
-#ifdef NS_IMPL_COCOA
-      binDir = [binDir stringByAppendingPathComponent: @"MacOS"];
-#endif
-    }
+  if (![original isAbsolutePath]
+      && [fileManager fileExistsAtPath:fixedPath isDirectory:NULL])
+    return [fixedPath UTF8String];
 
-  paths = [binDir stringsByAppendingPaths:
-                [NSArray arrayWithObjects: @"libexec", @"bin", nil]];
-  pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
+  /* If we reach here either the path is absolute and therefore we
+     don't need to complete it, or we're unable to relocate the
+     file/directory.  If it's the latter it may be because the user is
+     trying to use a bundled app as though it's a Unix style install
+     and we have no way to guess what was intended, so return the
+     original string unaltered.  */
 
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
-
-  return NULL;
-}
-
-
-const char *
-ns_load_path (void)
-/* If running as a self-contained app bundle, return as a path string
-   the filenames of the site-lisp and lisp directories.
-   Ie, site-lisp:lisp.  Otherwise, return nil.  */
-{
-  NSBundle *bundle = [NSBundle mainBundle];
-  NSString *resourceDir = [bundle resourcePath];
-  NSString *resourcePath, *resourcePaths;
-  NSString *pathSeparator = [NSString stringWithFormat: @"%c", SEPCHAR];
-  NSFileManager *fileManager = [NSFileManager defaultManager];
-  BOOL isDir;
-  NSArray *paths = [resourceDir stringsByAppendingPaths:
-                              [NSArray arrayWithObjects:
-                                         @"site-lisp", @"lisp", nil]];
-  NSEnumerator *pathEnum = [paths objectEnumerator];
-  resourcePaths = @"";
-
-  /* Hack to skip site-lisp.  */
-  if (no_site_lisp) resourcePath = [pathEnum nextObject];
-
-  while ((resourcePath = [pathEnum nextObject]))
-    {
-      if ([fileManager fileExistsAtPath: resourcePath isDirectory: &isDir])
-        if (isDir)
-          {
-            if ([resourcePaths length] > 0)
-              resourcePaths
-                = [resourcePaths stringByAppendingString: pathSeparator];
-            resourcePaths
-              = [resourcePaths stringByAppendingString: resourcePath];
-          }
-    }
-  if ([resourcePaths length] > 0) return [resourcePaths UTF8String];
+#endif
 
-  return NULL;
+  return epath;
 }
 
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-23 15:59                                 ` Alan Third
@ 2021-06-23 20:46                                   ` Matthew Bauer
  2021-06-23 20:48                                     ` Alan Third
  2021-06-26  9:38                                     ` Alan Third
  0 siblings, 2 replies; 21+ messages in thread
From: Matthew Bauer @ 2021-06-23 20:46 UTC (permalink / raw)
  To: Alan Third, Matthew Bauer; +Cc: 48994

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

That works for me! No apparent issues.

On Wed, Jun 23, 2021 at 10:59 AM Alan Third <alan@idiocy.org> 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
>

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-23 20:46                                   ` Matthew Bauer
@ 2021-06-23 20:48                                     ` Alan Third
  2021-06-26  9:38                                     ` Alan Third
  1 sibling, 0 replies; 21+ messages in thread
From: Alan Third @ 2021-06-23 20:48 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 48994

Thanks!

I'll leave it a few days before pushing it just in case anyone else
spots anything wrong with it.

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 <alan@idiocy.org> 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





^ permalink raw reply	[flat|nested] 21+ messages in thread

* bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS)
  2021-06-23 20:46                                   ` Matthew Bauer
  2021-06-23 20:48                                     ` Alan Third
@ 2021-06-26  9:38                                     ` Alan Third
  1 sibling, 0 replies; 21+ messages in thread
From: Alan Third @ 2021-06-26  9:38 UTC (permalink / raw)
  To: Matthew Bauer; +Cc: 48994-done

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 <alan@idiocy.org> 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





^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-06-26  9:38 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-13  3:13 bug#48994: 28.0.50; [PATCH] 28.0.50; Native compilation unnecessarily recompiles .eln (macOS) Matthew Bauer
2021-06-13  6:37 ` Eli Zaretskii
2021-06-13 16:37   ` Alan Third
2021-06-13 17:21     ` Eli Zaretskii
2021-06-13 19:30       ` Alan Third
2021-06-14 12:42         ` Eli Zaretskii
2021-06-14 18:32           ` Alan Third
2021-06-14 19:19             ` Eli Zaretskii
2021-06-16 18:25               ` Alan Third
2021-06-16 18:45                 ` Eli Zaretskii
2021-06-19 17:04                   ` Alan Third
2021-06-22  3:55                     ` Matthew Bauer
2021-06-22  8:32                       ` Alan Third
2021-06-22 16:51                         ` Matthew Bauer
2021-06-22 16:59                           ` Alan Third
2021-06-22 17:24                             ` Alan Third
2021-06-22 23:01                               ` Matthew Bauer
2021-06-23 15:59                                 ` Alan Third
2021-06-23 20:46                                   ` Matthew Bauer
2021-06-23 20:48                                     ` Alan Third
2021-06-26  9:38                                     ` Alan Third

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).