[m-rev.] diff: fix bug #339: --restricted-command-line broken with recent Java compilers

[Sebastian Godelet]

Hi Julien,

> -----Original Message-----
> From: Julien Fischer [mailto:jfischer at opturion.com]
> Sent: Wednesday, July 15, 2015 3:11 PM
> To: Julien Fischer
> Cc: Sebastian Godelet; reviews at lists.mercurylang.org
> Subject: RE: [m-rev.] diff: fix bug #339: --restricted-command-line broken with
> recent Java compilers
> 
> 
> On Wed, 15 Jul 2015, Julien Fischer wrote:
> 
> > On Wed, 15 Jul 2015, Sebastian Godelet wrote:
> >
> >> The compiler is compiled in a msys environment,
> >> But I tested it in a normal Windows command shell.
> >> In my Mercury.config file I have --env-type windows.
> >> Nevertheless, if I open an msys shell and use:
> >>
> >> $ mmc --use-grade-subdirs --env-type msys -s java -m fmt_bug
> >> cp: cannot stat `Mercury\\java\\i686-pc-mingw32\\Mercury\\bin\\fmt_bug':
> No
> >> such file or directory
> >> Made symlink/copy of Mercury\java\i686-pc-
> mingw32\Mercury\jars\fmt_bug.jar
> >> mercury_compile.exe: error copying
> >> `Mercury\java\i686-pc-mingw32\Mercury\bin\fmt_bug' to `fmt_bug', can't
> open
> >> input file: No such file or directory
> >>
> >> The same problem applies.
> >
> > Ok, I'll take a look at it.
> 
> I haven't been able to reproduce this one.

I think the problem was introduced in commit f903c238fb283f7e4047fbf06c62e8c9307cc5c8:
Fix problems with --use-grade-subdirs with C# and Java backends.
There Windows batch files where made subdir dependent:

@@ -1181,7 +1192,7 @@
 create_launcher_batch_file(Globals, MainModuleName, Pred, Succeeded, !IO) :-
     Extension = ".bat",
     module_name_to_file_name(Globals, MainModuleName, Extension,
-        do_not_create_dirs, FileName, !IO),
+        do_create_dirs, FileName, !IO),

But in the compile_target_code module in the post_link_make_symlink_or_copy predicate
module_name_to_file_name is called with an empty "" extension,
thus yielding the "bin" directory instead of the "bats" sub-directory (this is how the internals work for module_name_to_file_name afaik).
So when not compiling with --target-env windows this bug had no effect,
only the combination of --use(-grade)-subdirs and Windows causes this.

diff --git a/compiler/compile_target_code.m b/compiler/compile_target_code.m
index 82f4a91..eb2c280 100644
--- a/compiler/compile_target_code.m
+++ b/compiler/compile_target_code.m
@@ -2652,12 +2652,10 @@ post_link_make_symlink_or_copy(ErrorStream, LinkTargetType, ModuleName,
             )
         then
             ScriptExt = get_launcher_script_extension(Globals),
-            module_name_to_file_name(Globals, ModuleName, "",
-                do_not_create_dirs, OutputScriptName0, !IO),
-            OutputScriptName = OutputScriptName0 ++ ScriptExt,
-            module_name_to_file_name(NoSubdirGlobals, ModuleName, "",
-                do_not_create_dirs, UserDirScriptName0, !IO),
-            UserDirScriptName = UserDirScriptName0 ++ ScriptExt,
+            module_name_to_file_name(Globals, ModuleName, ScriptExt,
+                do_not_create_dirs, OutputScriptName, !IO),
+            module_name_to_file_name(NoSubdirGlobals, ModuleName, ScriptExt,
+                do_not_create_dirs, UserDirScriptName, !IO),

             same_timestamp(OutputScriptName, UserDirScriptName,
                 ScriptSameTimestamp, !IO),

> 
> >> Again, without the --use-grade-subdirs the correct file gets copid.
> >> I should mention that in the former case (Windows cmd), the subdir
> >> Mercury\java\i686-pc-mingw32\Mercury\bats
> >> Does contain the bat file, just it seems that the `cp' command is not
> >> looking in that folder.
> >>
> >> On a side note, if I use --env-type msys, the java class path still uses
> >> the c:/ Windows path convention in the
> >> Shell script and thus cannot be executed (though I think that is a
> >> different problem).
> >
> > I don't think the MSYS use case was really considered when we added
> > support for launcher scripts.  (The reason is probably that it's
> > somewhat awkward as it requires Unix conventions in some spots and
> > Windows ones in others -- also, at the time I don't think the Mercury
> > compiler had sufficient information about the environment to actually
> > handle it properly anyway.)  I'll take a look at this one as well
> 
> The following patch should fix the latter problem, although I don't have
> a Windows machine at hand to test it at the moment.

I haven't yet tried your inline patch for this, it takes quite a while to compile the compiler on my machine (using Windows).
On the same machine using Fedora, it builds much faster (x2-x3), my bet is all this shell script forking is much slower in msys 32-bit.
I have not been able to build with any version of msys 64 bit on Windows, always failing while configuring Boehm GC not knowing about the target architecture.

> 
> Cheers,
> Julien.
> 
> diff --git a/compiler/module_cmds.m b/compiler/module_cmds.m
> index 72fff72..bc68591 100644
> --- a/compiler/module_cmds.m
> +++ b/compiler/module_cmds.m
> @@ -768,12 +768,16 @@ create_java_shell_script(Globals, MainModuleName,
> Succeeded, !IO) :-
>       (
>           ( TargetEnvType = env_type_posix
>           ; TargetEnvType = env_type_cygwin
> -        ; TargetEnvType = env_type_msys
>           ),
>           create_launcher_shell_script(Globals, MainModuleName,
>               write_java_shell_script(Globals, MainModuleName, JarFileName),
>               Succeeded, !IO)
>       ;
> +        TargetEnvType = env_type_msys,
> +        create_launcher_shell_script(Globals, MainModuleName,
> +            write_java_msys_shell_script(Globals, MainModuleName,
> JarFileName),
> +            Succeeded, !IO)
> +    ;
>           % XXX should create a .ps1 file on PowerShell.
>           ( TargetEnvType = env_type_win_cmd
>           ; TargetEnvType = env_type_powershell
> @@ -812,6 +816,44 @@ write_java_shell_script(Globals, MainModuleName,
> JarFileName, Stream, !IO) :-
>           "exec \"$JAVA\" jmercury.", ClassName, " \"$@\"\n"
>       ], !IO).
> 
> +    % For the MSYS version of the Java launcher script, there are a few
> +    % differences:
> +    %
> +    % 1. The value of the CLASSPATH environment variable we construct for
> +    % the Java interpreter must contain Windows style paths.
> +    %
> +    % 2. We use forward slashes as directory separators rather than back
> +    % slashes since the latter require escaping inside the shell script.
> +    %
> +    % 3. The path of the Java interpreter must be a Unix style path as it
> +    % will be invoked directly from the MSYS shell.
> +    %
> +:- pred write_java_msys_shell_script(globals::in, module_name::in,
> +    file_name::in, io.text_output_stream::in, io::di, io::uo) is det.
> +
> +write_java_msys_shell_script(Globals, MainModuleName, JarFileName,
> Stream, !IO) :-
> +    get_mercury_std_libs_for_java(Globals, MercuryStdLibs),
> +    globals.lookup_accumulating_option(Globals, java_classpath,
> +        UserClasspath),
> +    % We prepend the .class files' directory and the current CLASSPATH.
> +    Java_Incl_Dirs = ["\"$DIR/" ++ JarFileName ++ "\""] ++
> +        MercuryStdLibs ++ ["$CLASSPATH" | UserClasspath],
> +    ClassPath0 = string.join_list(";", Java_Incl_Dirs),
> +    ClassPath = string.replace_all(ClassPath0, "\\", "/"),
> +
> +    globals.lookup_string_option(Globals, java_interpreter, Java),
> +    mangle_sym_name_for_java(MainModuleName, module_qual, ".",
> ClassName),
> +
> +    io.write_strings(Stream, [
> +        "#!/bin/sh\n",
> +        "DIR=${0%/*}\n",
> +        "DIR=$( cd \"${DIR}\" && pwd -W )\n",
> +        "CLASSPATH=", ClassPath, "\n",
> +        "export CLASSPATH\n",
> +        "JAVA=${JAVA:-", Java, "}\n",
> +        "exec \"$JAVA\" jmercury.", ClassName, " \"$@\"\n"
> +    ], !IO).
> +
>   :- pred write_java_batch_file(globals::in, module_name::in, file_name::in,
>       io.text_output_stream::in, io::di, io::uo) is det.
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-java-batch-creation.diff
Type: application/octet-stream
Size: 1142 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20150716/295eda57/attachment.obj>