RFR: 7181175 Enable hotspot builds on Windows with MinGW/MSYS

[Volker Simonis]

Hi,

please see my comments inline..

On Tue, Jul 3, 2012 at 7:07 AM, David Holmes <david.holmes at oracle.com> wrote:
> Hi Tim,
>
>
> On 3/07/2012 1:52 PM, Tim Bell wrote:
>>
>> All-
>>
>> This is the hotspot-only portion of the changes to allow building on
>> Windows with any of MKS, Cygwin, or MinGW/MSYS as the 'Unix-like' shell
>> environment:
>>
>> http://cr.openjdk.java.net/~tbell/7181175/webrev.00/
>>
>> A Windows-only JPRT build/test run was successful: job
>> 2012-07-02-230003.tbell.hotspot
>
>
> Did you try both -release 7 and -release 8? There are additional build
> platforms for 8 (Windows versions) though both use VS2010.
>
>
>> Bug fix #7152336 [1] will be used to update the other repos after these
>> changes are in hotspot.
>>
>> I'd like a quick review from some HS runtime members, and then I will
>
>
> I'm concerned by a few changes in sa.make.
>
> First all of the changes from \ to /. I can't help but think this was left
> at \ for a reason. Do we know this works on all build platforms? Further we
> still use \ in other makefiles so surely they all need to be fixed - eg see
> adlc.make (the existing inconsistency here is surprising and disturbing)

I think the '\' are a leftover from the times when Windows could not
handle forward slashes in path names (Win95/98 ??). As far as I can
tell, that's no a problem any more. For the initial submission I've
tested the change with MinGW/MSYS, Cygwin and MKS on WindowsXP and
Windows Server 2003 and Windows 7 and didn't find any problems.

As I explained in the initial webrev
(http://cr.openjdk.java.net/~simonis/MinGW_MSYS_hotspot.v1):

"..The patch mainly replaces forward slash style options (e.g. /NOLOG)
for calls to Windows programs with their corresponding dash form (e.g
-NOLOG) and backslash path separators with forward path separators (or
prevents the replacement of forward path separators with backslash
path separators if this was not necessary).

Note that this change is minimal in the sense that I only did "just
enough" changes to be able to build on Windows with either one of the
three Unix emulation environments (MKS, Cygwin, MinGW/MSYS).

Particularly I haven't done the above changes in the nmake make files
although this would have been possible. I'm very much in favor of
completely removing the special nmake make files and do the complete
build with GNU make. Besides the fact that this unifies the makefiles
and removes yet another dependency from the build, this will also
allow to take advantage of parallel HotSpot makes as this is currently
done under Linux and which is much faster on recent multi-core
hardware than the nmake batch build mode. As noted, these improvements
are left for a future change.."

But I agree, that at least on "file-level" the changes should be
consistent. For sa.make (which is a nmake file but for some reason I
had to touch it so I did all the mentioned changes right away), this
would mean that for the following lines:

  98 !if "$(ENABLE_FULL_DEBUG_SYMBOLS)" == "1"
  99 SA_CFLAGS = $(SA_CFLAGS) /ZI
 100 !endif
 101 !endif
 102 !if "$(MT)" != ""
 103 SA_LD_FLAGS = /manifest $(SA_LD_FLAGS)
 104 !endif

the "/"-Options should be replaced by "-"-Options. (Lines 98 trough
101 have not been there when I did the initial change and I somehow
must have missed the "old-style" option in line 103).

>
> Second this seems wrong:
>
>  "$(COMPILE_RMIC)" -classpath ...
>
> If COMPILE_RMIC is re-defined as "rmic <some-arg>" then the above will fail
> to execute. Plus didn't you already handle the spaces-in-paths problem in
> rules.make when you defined COMPILE_RMIC?
>
> Third the changing of the compiler options from /opt to -opt. In
> compile.make we use /opt for everything. So either your change is
> unnecessary, or there seems to be a lot of other changes needed. ???
>

See comments above..

>
>> need to ask for a volunteer to push the change in (unless I can do that
>> myself by submitting to JPRT... glad to do it if it is OK).
>
>
> Sorry but you don't have Committer status, nor Author status for the HSX
> project, so will need a sponsor for this contribution.
>
> Cheers,
> David
> -----
>
>
>> Thanks in advance-
>>
>> Tim Bell
>>
>>
>> [1] http://cr.openjdk.java.net/~tbell/7152336/webrev.02/
>>
>