Request for Review: Add hotspot support for building with mingw/msys on build-infra (the new build)

[Magnus Ihse Bursie]

On 2012-10-26 03:24, Kelly O'Hair wrote:
> It has been my experience that using cygpath -w created problems with shell escape logic,
> and that is why cygpath -m with a make level subst change is done, it was less problematic.
It think it would work, but I understand why you are cautious. I can 
revert that part.
> I don't understand why you would even want to change the cygwin logic at all, if it isn't broken why mess with it?
It turned out that it was indeed broken, but I had forgot what I fixed. 
:-) The current code works if CC is defined like
CC=/path/to/compiler
but it will not work if CC is defined like
CC=/path/to/compiler -extra -compiler -options
That is what the $(firstword) stuff is about, just converting the 
compiler path.

This would be just a complaint in general that it broke expectations on 
how to setup CC, if not this turned out to be a problem for the latest 
changes in build-infra.

In older build-infra, we send in
CC=/path/to/fixpath /path/to/compiler
and by a lucky co-incidence, this will actually work, altough not as 
expected. (As far as I can understand, the hotspot makefile cygpath 
rewrite will change both paths.)

However, when I added support for msys in build-infra, I changed the 
syntax for fixpath (formerly uncygdrive). It now requires an switch 
telling it to work in cygwin or msys mode, so we send in:
CC=/path/to/fixpath -c /path/to/compiler
which will make the hotspot rewrite code break. (My guess is that 
cygpath converts /path/to/fixpath but ignores everything from -c and 
forward).

Since fixpath actually isn't technically needed when building hotspot, 
we can work around this differently in Hotspot. Maybe that is the 
pragmatic way. But I still believe it's theoretically wrong with not 
handling a CC= which contains more than a path to an executable. :-)


> So my suggestion would be to add the msys logic, and leave the old cygwin logic alone.
>
> I could have sworn that Tim Bell already did this change, and yes, if I look at:
>
>      http://hg.openjdk.java.net/jdk8/build/hotspot/file/dccd40de8db1/make/windows/makefiles/defs.make
>
> These changes are there already, or something equivalent.
>
> Have you talked with Tim Bell about this change?

Not specifically about this change, but we communicated a lot while I 
was developing msys support for build-infra.

I'm not sure what you want to tell me by the link. There was a change 
(7181175: Enable builds on Windows with MinGW/MSYS),
http://hg.openjdk.java.net/jdk8/build/hotspot/rev/0d8e265ba727
which added the USING_MINGW variable that I need to check. However I 
can't see anything in this file that would fix the CXX etc variables for 
msys, the way my patch is doing it.

But then again, we can probably do without this patch, if we make things 
differently in build-infra. We're already doing some weird special stuff 
for Hotspot builds that I guess adding another exception won't be that 
different. I'll look into it.

/Magnus