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

[Julien Fischer]

Hi Sebastian,

On Thu, 16 Jul 2015, Sebastian Godelet wrote:

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

Given that --use-grade-subdirs didn't work properly for the Java backend
before that commit, I'd say that's pretty certain.

> 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),

It should be passing ScriptExt to the calls to module_name_to_file_name
instead of "" (and not manually appending the extension).  That at
least, fixes the problem on Windows.  (I suspect the above is simply a
cut-and-paste error).  I'm presently testing this fix.

>>>> 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).

Don't bother testing that version, it's incorrect.  I've since committed a
corrected version (commit a868cae).

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

Both the Cygwin and MSYS shells are quite slow -- not really a Mercury
problem as such.  (If you need to speed Windows builds up you can use
mingw cross compiler on Linux.)

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

I'm not quite sure what you mean by "msys 64 bit" since that could refer
to a number of things.  The way I usually build 64-bit versions of
Mercury on Windows -- which is what is described in README.MinGW -- is
to use the standard 32-bit MSYS as the build environment, but use the
64 bit MinGW64 toolchain.  The only gotcha with this is that you
*must* pass:

    --host=x86_64-w64-mingw32

to the configure script.

If you using the MSYS2 distribution (i.e the *-pc-msys architecture)
then you're out of luck as the current version of Boehm we are using
doesn't support that architecture.

Cheers,
Julien.