[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: Friday, July 17, 2015 12:13 PM
> To: Sebastian Godelet
> Cc: Julien Fischer; reviews at lists.mercurylang.org
> Subject: RE: [m-rev.] diff: fix bug #339: --restricted-command-line broken with
> recent Java compilers
> 
> 
> 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).

Ok I'll wait for your test results of my patch then I'll compile again.

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

Sorry I meant Msys2 with the mingw64 toolchain ( I didn't know that mingw64 works with the
Original Msys distribution as well, thanks for that hint).

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

Oh I see, that was the version I tried. I was hoping for Msys2 to work there (I like the pacman package manager),
Also I thought it might have a speed advantage.
Will the eventually upcoming Boehm GC update help with compiling on that platform?

> 
> Cheers,
> Julien.

Thanks for looking into this.
I recently checked my other patch I submitted with the use_win32 and stream redirection problem w.r.t. Windows XPsp3,
And as it seems to work ok, I'll re-submit that patch later.
Had you had time to look into my remark (in the bug report) on the Java library permission problem on Fedora/RedHat?

Cheers, Sebastian.