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

[Alan Third <alan@idiocy.org>]

[-- 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