Subject: Re: bug#41242: Port feature/native-comp to Windows - Determine the emacs root dir...

From: Andrea Corallo <[email-masked]>
Date: Wed, 27 May 2020 21:02:45 +0000

Hi all,

I've attached the current Nico's patch in case someone wants to engage,
here some comments from me.

> * src/fileio.c: Introduce function emacs_root_dir(). Refactor
> `expand-file-name` to use it.
> * src/lisp.h: Separate emacs_root_dir() into dos_emacs_root_dir() and
> w32_emacs_root_dir().
> * src/msdos.c: Rename emacs_root_dir() to dos_emacs_root_dir().
> * src/w32.c: Rename emacs_root_dir() to w32_emacs_root_dir().

I think would be good to mention in the commit message/changelog the
reason of this change, why it wasn't working and why it is now.

> diff --git a/src/fileio.c b/src/fileio.c
> index 2f1d2f8243..e9be811841 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -781,6 +781,18 @@ user_homedir (char const *name)
>    return pw->pw_dir;
>  }
>  
> +static Lisp_Object
> +emacs_root_dir (void)
> +{
> +#ifdef DOS
> +      return build_string (dos_emacs_root_dir ());
> +#elif defined (WINDOWSNT)
> +      return build_string (w32_emacs_root_dir ());
> +#else
> +      return build_string ("/");
> +#endif
> +}

I believe the indentation of these returns should be two regular spaces.

>  DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
>         doc: /* Convert filename NAME to absolute, and canonicalize it.
>  Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
> @@ -851,21 +863,16 @@ the root directory.  */)
>      }
>  
>    /* As a last resort, we may have to use the root as
> -     default_directory below.  */
> -  Lisp_Object root;
> -#ifdef DOS_NT
> -      /* "/" is not considered a root directory on DOS_NT, so using it
> -	 as default_directory causes an infinite recursion in, e.g.,
> -	 the following:
> +     default_directory below.
>  
> -            (let (default-directory)
> -	      (expand-file-name "a"))
> +     "/" is not considered a root directory on DOS_NT, so using it
> +     as default_directory causes an infinite recursion in, e.g.,
> +     the following:
>  
> -	 To avoid this, we use the root of the current drive.  */
> -      root = build_string (emacs_root_dir ());
> -#else
> -      root = build_string ("/");
> -#endif
> +        (let (default-directory)
> +          (expand-file-name "a"))
> +
> +     To avoid this, we use the root of the current drive.  */

I suspect this commentary is not very ideal here given now the code has
been moved into emacs_root_dir, maybe the commentary should go there.

>    /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
>    if (NILP (default_directory))
> @@ -891,13 +898,13 @@ the root directory.  */)
>  	      Lisp_Object absdir
>  		= STRINGP (Vinvocation_directory)
>  		&& file_name_absolute_no_tilde_p (Vinvocation_directory)
> -		? Vinvocation_directory : root;
> +		? Vinvocation_directory : emacs_root_dir ();
>  	      default_directory = Fexpand_file_name (dir, absdir);
>  	    }
>  	}
>      }

Thanks

  Andrea

-- 
[email-masked]
>From 1ead6bad59d0c2739dce13d3720c0264f2c5c531 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <[email-masked]>
Date: Mon, 25 May 2020 17:55:23 -0300
Subject: [PATCH] Determine the emacs root dir only when necessary.

* src/fileio.c: Introduce function emacs_root_dir(). Refactor
`expand-file-name` to use it.
* src/lisp.h: Separate emacs_root_dir() into dos_emacs_root_dir() and
w32_emacs_root_dir().
* src/msdos.c: Rename emacs_root_dir() to dos_emacs_root_dir().
* src/w32.c: Rename emacs_root_dir() to w32_emacs_root_dir().
---
 src/fileio.c | 37 ++++++++++++++++++++++---------------
 src/lisp.h   | 11 +++++++----
 src/msdos.c  |  2 +-
 src/w32.c    |  2 +-
 4 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 2f1d2f8243..e9be811841 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -781,6 +781,18 @@ user_homedir (char const *name)
   return pw->pw_dir;
 }
 
+static Lisp_Object
+emacs_root_dir (void)
+{
+#ifdef DOS
+      return build_string (dos_emacs_root_dir ());
+#elif defined (WINDOWSNT)
+      return build_string (w32_emacs_root_dir ());
+#else
+      return build_string ("/");
+#endif
+}
+
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
        doc: /* Convert filename NAME to absolute, and canonicalize it.
 Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
@@ -851,21 +863,16 @@ the root directory.  */)
     }
 
   /* As a last resort, we may have to use the root as
-     default_directory below.  */
-  Lisp_Object root;
-#ifdef DOS_NT
-      /* "/" is not considered a root directory on DOS_NT, so using it
-	 as default_directory causes an infinite recursion in, e.g.,
-	 the following:
+     default_directory below.
 
-            (let (default-directory)
-	      (expand-file-name "a"))
+     "/" is not considered a root directory on DOS_NT, so using it
+     as default_directory causes an infinite recursion in, e.g.,
+     the following:
 
-	 To avoid this, we use the root of the current drive.  */
-      root = build_string (emacs_root_dir ());
-#else
-      root = build_string ("/");
-#endif
+        (let (default-directory)
+          (expand-file-name "a"))
+
+     To avoid this, we use the root of the current drive.  */
 
   /* Use the buffer's default-directory if DEFAULT_DIRECTORY is omitted.  */
   if (NILP (default_directory))
@@ -891,13 +898,13 @@ the root directory.  */)
 	      Lisp_Object absdir
 		= STRINGP (Vinvocation_directory)
 		&& file_name_absolute_no_tilde_p (Vinvocation_directory)
-		? Vinvocation_directory : root;
+		? Vinvocation_directory : emacs_root_dir ();
 	      default_directory = Fexpand_file_name (dir, absdir);
 	    }
 	}
     }
   if (! STRINGP (default_directory))
-    default_directory = root;
+    default_directory = emacs_root_dir ();
 
   handler = Ffind_file_name_handler (default_directory, Qexpand_file_name);
   if (!NILP (handler))
diff --git a/src/lisp.h b/src/lisp.h
index 52242791aa..3dd8b9ad14 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4749,10 +4749,13 @@ extern bool profiler_memory_running;
 extern void malloc_probe (size_t);
 extern void syms_of_profiler (void);
 
-#ifdef DOS_NT
-/* Defined in msdos.c, w32.c.  */
-extern char *emacs_root_dir (void);
-#endif /* DOS_NT */
+#ifdef MSDOS
+/* Defined in msdos.c.  */
+extern char *dos_emacs_root_dir (void);
+#elif defined (WINDOWSNT)
+/* Defined in w32.c.  */
+extern char *w32_emacs_root_dir (void);
+#endif /* MSDOS */
 
 #ifdef HAVE_NATIVE_COMP
 INLINE bool
diff --git a/src/msdos.c b/src/msdos.c
index b5f06c99c3..0827cc96cd 100644
--- a/src/msdos.c
+++ b/src/msdos.c
@@ -3350,7 +3350,7 @@ getdefdir (int drive, char *dst)
 }
 
 char *
-emacs_root_dir (void)
+dos_emacs_root_dir (void)
 {
   static char root_dir[4];
 
diff --git a/src/w32.c b/src/w32.c
index 38bbc49656..a3f5844574 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -3147,7 +3147,7 @@ init_environment (char ** argv)
 /* Called from expand-file-name when default-directory is not a string.  */
 
 char *
-emacs_root_dir (void)
+w32_emacs_root_dir (void)
 {
   static char root_dir[MAX_UTF8_PATH];
   const char *p;
-- 
2.20.1