Emacs 27.1 - Issue with pdumper

Emacs 27.1 - Issue with pdumper

[José Manuel García-Patos]

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


Hello,

I'm using GNU Emacs 27.1 on Slackware Linux. This last piece of
information is important because the way Emacs is packaged here is
through a double compilation which results in two different binaries:
One with support for X and one without. The latter can be used for
things like editing git commit messages or composing email messages
with Mutt, like I'm doing right now.

Anyway, the issue I (and all other Slackware users, I suppose) have
with pdumper is that there is no way to create two different .pdmp
files (because emacs.pdmp is hardcoded). That means the second binary
can never run and instead fails with the following error:

desired fingerprint: b1af5fedf3d750a1ba93f43999aa141cf316edd0bb344916fb5529aea2cf68e2
found fingerprint: 8093d49d8a9b7b588b1f66b4bafad1f9f5e0dc766973ba4d83e38d25cfa96e4e
emacs: could not load dump file "/usr/libexec/emacs/27.1/x86_64-slackware-linux/emacs.pdmp": not built for this Emacs executable

The most obvious workaround is to rename the .pdmp file created during
the second compilation, right after it and before the actual packaging,
but that doesn't work because Emacs' code checks for ‘emacs.pdmp’
before ‘emacs-no-x11.pdmp’ (or whatever name we want to give it),
hence the error above.

You might argue that there's the option --dump-file=FILE, but having
to use that would defeat the purpose of installing something and
having it just work.

So, I'd like to suggest a modification to the function load_pdump() in
the file emacs.c consisting of changing its order of evaluation,
leaving the search for emacs.pdmp (the default case) for last and
trying to find <name-of-executable>.pdmp before that. I've tried this
change myself and it works, which is the reason why I'm able to
compose this email. I have attached a diff file in case you want to
use it as orientation or as is.

I think leaving the (hardcoded) default case for last makes sense, and
it would help us a lot, as we would only have to change one line in
the packaging script.

Thank you.

-- 
José Manuel García-Patos
Madrid

[-- Attachment #2: emacs-git.diff --]
[-- Type: text/plain, Size: 3400 bytes --]

diff --git a/src/emacs.c b/src/emacs.c
index 059e1c6d8f..ad92fd6fca 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -864,15 +864,51 @@ load_pdump (int argc, char **argv)
   path_exec = w32_relocate (path_exec);
 #endif
 
-  /* Look for "emacs.pdmp" in PATH_EXEC.  We hardcode "emacs" in
-     "emacs.pdmp" so that the Emacs binary still works if the user
-     copies and renames it.  */
-  const char *argv0_base = "emacs";
+  /* Look for basename(argv0)+".pdmp" in PATH_EXEC.
+     This way, they can rename both the executable and its pdump
+     file in PATH_EXEC, and have several Emacs configurations in
+     the same versioned libexec subdirectory.  */
+  char *p, *last_sep = NULL;
+  for (p = argv[0]; *p; p++)
+     {
+        if (IS_DIRECTORY_SEP (*p))
+           last_sep = p;
+     }
+  const char *argv0_base = last_sep ? last_sep + 1 : argv[0];
   ptrdiff_t needed = (strlen (path_exec)
                       + 1
                       + strlen (argv0_base)
                       + strlen (suffix)
                       + 1);
+  if (bufsize < needed)
+     {
+        xfree (dump_file);
+        dump_file = xmalloc (needed);
+     }
+#ifdef DOS_NT
+  ptrdiff_t argv0_len = strlen (argv0_base);
+  if (argv0_len >= 4
+      && c_strcasecmp (argv0_base + argv0_len - 4, ".exe") == 0)
+     sprintf (dump_file, "%s%c%.*s%s", path_exec, DIRECTORY_SEP,
+              (int)(argv0_len - 4), argv0_base, suffix);
+  else
+#endif
+     sprintf (dump_file, "%s%c%s%s",
+              path_exec, DIRECTORY_SEP, argv0_base, suffix);
+  result = pdumper_load (dump_file);
+
+  if (result == PDUMPER_LOAD_SUCCESS)
+     goto out;
+
+  /* Finally, look for "emacs.pdmp" in PATH_EXEC.  We hardcode "emacs" in
+     "emacs.pdmp" so that the Emacs binary still works if the user
+     copies and renames it.  */
+  argv0_base = "emacs";
+  needed = (strlen (path_exec)
+            + 1
+            + strlen (argv0_base)
+            + strlen (suffix)
+            + 1);
   if (bufsize < needed)
     {
       xfree (dump_file);
@@ -882,42 +918,6 @@ load_pdump (int argc, char **argv)
            path_exec, DIRECTORY_SEP, argv0_base, suffix);
   result = pdumper_load (dump_file);
 
-  if (result == PDUMPER_LOAD_FILE_NOT_FOUND)
-    {
-      /* Finally, look for basename(argv0)+".pdmp" in PATH_EXEC.
-	 This way, they can rename both the executable and its pdump
-	 file in PATH_EXEC, and have several Emacs configurations in
-	 the same versioned libexec subdirectory.  */
-      char *p, *last_sep = NULL;
-      for (p = argv[0]; *p; p++)
-	{
-	  if (IS_DIRECTORY_SEP (*p))
-	    last_sep = p;
-	}
-      argv0_base = last_sep ? last_sep + 1 : argv[0];
-      ptrdiff_t needed = (strlen (path_exec)
-			  + 1
-			  + strlen (argv0_base)
-			  + strlen (suffix)
-			  + 1);
-      if (bufsize < needed)
-	{
-	  xfree (dump_file);
-	  dump_file = xmalloc (needed);
-	}
-#ifdef DOS_NT
-      ptrdiff_t argv0_len = strlen (argv0_base);
-      if (argv0_len >= 4
-	  && c_strcasecmp (argv0_base + argv0_len - 4, ".exe") == 0)
-	sprintf (dump_file, "%s%c%.*s%s", path_exec, DIRECTORY_SEP,
-		 (int)(argv0_len - 4), argv0_base, suffix);
-      else
-#endif
-      sprintf (dump_file, "%s%c%s%s",
-	       path_exec, DIRECTORY_SEP, argv0_base, suffix);
-      result = pdumper_load (dump_file);
-    }
-
   if (result != PDUMPER_LOAD_SUCCESS)
     {
       if (result != PDUMPER_LOAD_FILE_NOT_FOUND)

Re: Emacs 27.1 - Issue with pdumper

[Eli Zaretskii]

> Date: Thu, 27 Aug 2020 23:32:33 +0200
> From: José Manuel García-Patos <jm.inform@gesaku.es>
> 
> The most obvious workaround is to rename the .pdmp file created during
> the second compilation, right after it and before the actual packaging,
> but that doesn't work because Emacs' code checks for ‘emacs.pdmp’
> before ‘emacs-no-x11.pdmp’ (or whatever name we want to give it),
> hence the error above.

That's not my reading of the code.  How do you see that Emacs looks
for emacs.pdmp before looking for BASENAME_OF_EMACS.pdmp?  My reading
of the code is that we do the exact opposite: emacs.pdmp is looked for
as the last resort.

> I think leaving the (hardcoded) default case for last makes sense, and
> it would help us a lot, as we would only have to change one line in
> the packaging script.

AFAICT, we already do that, so I'm puzzled why it didn't work for you,
or what am I missing.

Re: Emacs 27.1 - Issue with pdumper

[José Manuel García-Patos]

Eli Zaretskii escribió
> That's not my reading of the code.  How do you see that Emacs looks
> for emacs.pdmp before looking for BASENAME_OF_EMACS.pdmp?  My reading
> of the code is that we do the exact opposite: emacs.pdmp is looked for
> as the last resort.

The way I see it, the current order is:

(1) --dump-file=FILE command-line option
(2) <basename>.pdmp (in the same directory as the executable)
(3) emacs.pdmp (in PATH_EXEC)
(4) <basename>.pdmp in PATH_EXEC

Personally, I don't see the point of (2), but if it's there, I'm sure
there is a reason. My proposal is to swap (3) and (4).

(3') <basename>.pdmp in PATH_EXEC
(4') emacs.pdmp

That's my actual set-up: emacs-no-x11 goes through (3') and regular
emacs goes through (4'). With the current order, emacs.pdmp would be
found in (3), (4) would never be executed and I would get the error I
mentioned when I tried to run emacs-no-x11. Regular emacs would not be
affected either way, because it is the default.

You might say: “You can use (2), too.” Yes, but leaving dump files in
/usr/bin is not as clean a solution as leaving them where they
belong. Even the comment in the code hints at that:

      /* Finally, look for basename(argv0)+".pdmp" in PATH_EXEC.
      This way, they can rename both the executable and its pdump
      file in PATH_EXEC, and have several Emacs configurations in
      the same versioned libexec subdirectory.  */

All the best.

-- 
José Manuel García-Patos
Madrid