New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

[Juanma Barranquero]

I build Emacs on Windows by passing the following locallispath to configure:

  --enable-locallisppath='%emacs_dir%/../site-lisp:%emacs_dir%/share/emacs/@VER@/site-lisp:%emacs_dir%/share/emacs/site-lisp'

The %emacs_dir%/../site-lisp part is required to keep compatibility
with pre-MSYS builds, and @VER@ is to access the version-specific
"shared" site-lisp.

As of this change:

  2013-11-20  Dani Moncayo  <dmoncayo@gmail.com>

          * build-aux/msys-to-w32: New file.
          * Makefile.in (msys_to_w32, msys_lisppath_to_w32): Remove.
          (msys_w32prefix_subst): Rename from msys_prefix_subst.
          Operate on w32prefixpattern.
          (epaths-force-w32): Use build-aux/msys-to-w32.

it has stopped working. While building I get this error:

  ./build-aux/msys-to-w32: line 142: cd: %emacs_dir%/../: No such file
or directory
  msys-to-w32: invalid path: %emacs_dir%/share/emacs/@VER@/site-lisp

This is a serious regression IMO.

  J

Re: New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

[Dani Moncayo]

>   ./build-aux/msys-to-w32: line 142: cd: %emacs_dir%/../: No such file
> or directory
>   msys-to-w32: invalid path: %emacs_dir%/share/emacs/@VER@/site-lisp

The below patch fixes this problem.

(the change to 'Makefile.in' fixes a related bug reported yesterday)


diff --git a/Makefile.in b/Makefile.in
index 3e3a4f5..065168b 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -338,7 +338,7 @@ msys_sed_sh_escape=sed -e 's/[];$$*.^[]/\\\\&/g'
 epaths-force-w32: FRC
  @(w32srcdir=`${srcdir}/build-aux/msys-to-w32 "${srcdir}"`; \
   w32prefix=`${srcdir}/build-aux/msys-to-w32 "${prefix}" N`; \
-  w32prefixpattern=`echo "${w32prefix}" | ${msys_sed_sh_escape}` ; \
+  w32prefixpattern=`echo "$${w32prefix}" | ${msys_sed_sh_escape}` ; \
   w32locallisppath=`${srcdir}/build-aux/msys-to-w32
"${locallisppath}" N ":" "\\;" | ${msys_w32prefix_subst}` ; \
   sed < ${srcdir}/nt/epaths.nt > epaths.h.$$$$ \
   -e 's;\(#.*PATH_SITELOADSEARCH\).*$$;\1 "'"$${w32locallisppath}"'";' \
diff --git a/build-aux/msys-to-w32 b/build-aux/msys-to-w32
index e45ec3c..7a8fa86 100755
--- a/build-aux/msys-to-w32
+++ b/build-aux/msys-to-w32
@@ -36,7 +36,8 @@ transformations:
 3. Replace two consecutive slashes with single ones.
 4. Translate to Windows-native format those paths that are not in such
    format already. The translated paths will not end with a slash,
-   except for root directories (e.g. 'c:/' or 'c:/foo').
+   except for root directories (e.g. 'c:/' or 'c:/foo').  Paths
+   starting with '%emacs_dir%' will not be translated.
 5. Escape with backslashes every occurrence of SEPARATOR2 within the paths.
 6. Concatenate the translated paths with SEPARATOR2.

@@ -100,7 +101,11 @@ do
     p="${p//\\//}"
     p="${p//\/\///}"

-    if test -d "$p"
+    if test "${p:0:11}" = "%emacs_dir%"
+    then
+ # Paths starting with "%emacs_dir%" will not be translated
+        w32p=$p
+    elif test -d "$p"
     then
  # The path exists, so just translate it
  w32p=`cd "$p" && pwd -W`
@@ -162,4 +167,4 @@ do
 done

 # Write the translated pathlist to the standard output
-printf "${w32pathlist}"
+printf "%s" "${w32pathlist}"




-- 
Dani Moncayo

Re: New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

[Juanma Barranquero]

On Sat, Nov 30, 2013 at 8:12 AM, Dani Moncayo <dmoncayo@gmail.com> wrote:

> The below patch fixes this problem.

Thanks, it works and I've committed it, though I still think there's a
deeper issue. Why should msys-to-w32 worry about directories in
the load path outside the standard ones? We already discussed this a
few months ago, and came to the conclusion that non-standard
directories are the user's responsibility. The fact that I pass
--enable-locallisppath=C:/mydir to configure does NOT mean that
C:/mydir should exist at build time. Why should msys-to-w32 complain
about it?

> (the change to 'Makefile.in' fixes a related bug reported yesterday)

This was already fixed in the trunk.

Re: New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

[Dani Moncayo]

>> The below patch fixes this problem.
>
> Thanks, it works and I've committed it,

Thank you.

> though I still think there's a
> deeper issue. Why should msys-to-w32 worry about directories in
> the load path outside the standard ones?

It doesn't.  It just translates paths.  See below.

> We already discussed this a
> few months ago, and came to the conclusion that non-standard
> directories are the user's responsibility. The fact that I pass
> --enable-locallisppath=C:/mydir to configure does NOT mean that
> C:/mydir should exist at build time.

Agreed.  The directories you specify with --enable-locallisppath don't
have to exist at build time.  That is why, in the "epaths-force-w32"
target, the translation of "${locallisppath}" is done by passing "N"
to the argument MUSTEXIST of the msys-to-w32 script.

> Why should msys-to-w32 complain
> about it?

It complains because it receives a pathlist like this:

  "C:/mydir:<other_path>:<yet_another_path>"

and since the argument SEPARATOR is set to ":", it splits the input pathlist as:

 C
 /mydir
 <other_path>
 <yet_another_path>

And when it tries to translate the first path ("C"), it fails, obviously.

Conclusion: always use MSYS-style paths ("/c/mydir") instead of
windows native paths ("c:/mydir") when specifying paths with
--enable-locallisppath, because the colon ":" will be used as
separator in the path list.

-- 
Dani Moncayo

Re: New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

[Juanma Barranquero]

On Sat, Nov 30, 2013 at 5:37 PM, Dani Moncayo <dmoncayo@gmail.com> wrote:

> Agreed.  The directories you specify with --enable-locallisppath don't
> have to exist at build time.  That is why, in the "epaths-force-w32"
> target, the translation of "${locallisppath}" is done by passing "N"
> to the argument MUSTEXIST of the msys-to-w32 script.

Then why did it complain about %emacs_dir%/share/emacs/@VER@/site-lisp
? It does not contain any colon.

> Conclusion: always use MSYS-style paths ("/c/mydir") instead of
> windows native paths ("c:/mydir") when specifying paths with
> --enable-locallisppath, because the colon ":" will be used as
> separator in the path list.

I used c:/mydir as an example. What I mean is,
--enable-locallisppath=/c/mydir should not complain. I haven't checked
that it does, but if not, I'm puzzled that it complained about the
%emacs_dir% path.

   J

Re: New msys-to-w32 breaks using %emacs_dir%, @VER@ in locallisppath

[Dani Moncayo]

>> Agreed.  The directories you specify with --enable-locallisppath don't
>> have to exist at build time.  That is why, in the "epaths-force-w32"
>> target, the translation of "${locallisppath}" is done by passing "N"
>> to the argument MUSTEXIST of the msys-to-w32 script.
>
> Then why did it complain about %emacs_dir%/share/emacs/@VER@/site-lisp
> ? It does not contain any colon.

Because the script didn't know how to translate such a path.  (now it
does: the translations of that kind of paths are literally the
original ones).

But aside from that exception that was introduced yesterday, all
translations performed by the script are ultimately based on "cd"ing
to an _existing_ directory (the one received as parameter, if it
exists, or else the deepest existing directory among its parents) and
then using "pwd -W" to get its translation (if some trailing
directories were discarded, they are concatenated to the translation
to from the final translation).

I designed the script that way, because I think it's the only reliable
way of doing the job. Imagine for example that "c:/foo" is mounted in
MSYS as "/c/bar".  Then the translation of "/c/bar/emacs" should be
"c:/foo/emacs" (not "c:/bar/emacs").  IOW, I wanted to completely
avoid the pattern-matching approach, and base all translations on the
"pwd -W" feature of the MSYS bash shell.

>> Conclusion: always use MSYS-style paths ("/c/mydir") instead of
>> windows native paths ("c:/mydir") when specifying paths with
>> --enable-locallisppath, because the colon ":" will be used as
>> separator in the path list.
>
> I used c:/mydir as an example. What I mean is,
> --enable-locallisppath=/c/mydir should not complain. I haven't checked
> that it does, but if not, I'm puzzled that it complained about the
> %emacs_dir% path.

"--enable-locallisppath=/c/mydir" should work fine.  See above.


-- 
Dani Moncayo