RFR: JDK-8042707: Source changes needed to build JDK 9 with Visual Studio 2013 (VS2013)

[Erik Joelsson]

Hello,

New webrev for root repo:
http://cr.openjdk.java.net/~erikj/8042707/webrev.root.02/

On 2015-01-09 15:45, Magnus Ihse Bursie wrote:
>
> It looks basically good to me. Some remarks on toolchain_windows.m4, 
> though.
>
> In TOOLCHAIN_CHECK_POSSIBLE_VISUAL_STUDIO_ROOT:
> # TODO: improve detection for other versions of VS
>
> This seems to be done now, so perhaps this can be removed :)
Done.
> ---
> Why is "vs_version" lowercase? It might be better to have local 
> variables in lower case (or not, I'm not sure we are consistent on 
> this?) but this breaks with the rest of the variables in the function 
> and looks strange, like it was intentionally signalling something. 
> (This goes for the vs_version in the other functions as well.)
Changed to upper case. Also introduced a common TOOLCHAIN_VERSION 
variable that is used in any non windows specific m4 file.
> ---
> # Work around the insanely named ProgramFiles(x86) env variable
> PROGRAMFILES_X86="`env | $SED -n 's/^ProgramFiles(x86)=//p'`"
>
> Yay! :-) I spent some time going crazy about that one before I gave 
> up. Never thought of that solution.
I think I found that on StackOverflow or similar site so can't claim credit.
> ---
> # FIXME will just assume default Visual Studio version
> if test "x$with_tools_dir" != x; then
> TOOLCHAIN_CHECK_POSSIBLE_VISUAL_STUDIO_ROOT([$with_tools_dir/../..],
>
> This seems broken. Have you tested it? You have to pass a version as 
> the first argument to TOOLCHAIN_CHECK_POSSIBLE_VISUAL_STUDIO_ROOT, 
> yes? I think we need to examine an explicitely given toolchain to have 
> it's version determined. Otherwise, the msvcr/msvcp handling code will 
> not function correctly. I suggest that we first check if 
> --with-toolchain-version is set and if so, respect it. Otherwise, 
> check the path given for the known names (VS_VS_INSTALLDIR_*), and if 
> none matches, abort and complain that version must be specified.
>
> Hm, actually, maybe the entire block of testing with_tools_dir should 
> be lifted up into TOOLCHAIN_FIND_VISUAL_STUDIO and handled 
> separately..? It doesn't really fit into 
> TOOLCHAIN_FIND_VISUAL_STUDIO_BAT_FILE anyhow.
>
I tinkered some with this, but ended up putting it back in 
TOOLCHAIN_FIND_VISUAL_STUDIO_BAT_FILE because it was simpler. It works 
now, but setting --with-tools-dir assumes you are pointing to the 
default version of Visual Studio. If you aren't, you must also set 
--with-toolchain-version for it to behave correctly. I suppose we could 
implement detection logic that would figure out which version it was, 
but I would rather not spend time on it when it's not the preferred way 
to configure this.

/Erik