Hi Erik,
Is there any action that you want me to take on this?
Thanks,
Ali
On Mon, Mar 11, 2019 at 7:37 PM Ali Ince <ali.i...@gmail.com
<mailto:ali.i...@gmail.com>> wrote:
Hi Erik,
Thanks for sponsoring this improvement. I think it will improve
build environments for different versions of OpenJDK.
Yes, I have signed OCA.
Sorry for the typos, and it definitely looks ok to me.
Best regards,
Ali
> On 11 Mar 2019, at 17:26, Erik Joelsson
<erik.joels...@oracle.com <mailto:erik.joels...@oracle.com>> wrote:
>
> Hello Ali,
>
> This looks like an excellent improvement! I had no idea of this
ability in Visual Studio and am very pleased to learn of it.
>
> First of all, have you signed the OCA?
>
> I took your patch for a spin. I noticed that line 256 looked
weird and corrected it to refer to the variable
with_msvc_toolset_version. I also took the liberty of reformatting
and clarifying the comment. I uploaded the modified patch here:
http://cr.openjdk.java.net/~erikj/8220445/webrev.01/ Does that
look ok to you?
>
> /Erik
>
> On 2019-03-10 10:30, Ali Ince wrote:
>> Hi Everyone,
>>
>> I've been working on getting OpenJDK builds to support
side-by-side minor version MSVC toolsets which is now available in
VS2017.
>>
>> With VS2017, Microsoft switched to a more frequent update
policy where they have delivered 8 major updates as of today. As
part of this policy, they're no more providing download support
for all major updates and they've limited the number of major
updates available for public download [1]. However, they're
providing earlier minor versions of MSVC toolsets as components
that can be installed side-by-side with the latest available major
update of VS2017 [2].
>>
>> On the other hand, known MSVC compiler versions between OpenJDK
versions change slightly. For example, within jdk, jdk12u and
jdk11u the latest known version of VS2017 is 15.8 [3], [4], [5]
whereas it is 15.6 in jdk8u [6].
>>
>> The proposed patch below enables us to provide the intended
MSVC toolset version to use, with --with-msvc-toolset-version
parameter. The provided value is then passed as 'vcvars_ver'
parameter value to 'vcvarsall.bat' invocation. The supplied
parameter configures the build environment to use the specified
toolset rather than the default one (which is currently the
toolset provided with 15.9 - with MSC_VER value of 1916).
>>
>> It also adds support for the 'VCToolsRedistDir' environment
variable introduced with VS2017 that identifies the root directory
that includes the redistributable DLLs instead of searching
through all toolset directories.
>>
>> This patch will enable a single setup of VS2017, within
multiple toolset versions present, to support building OpenJDK
with corresponding toolset versions identified as known compilers.
>>
>> I've tested the patch with both WSL and Cygwin.
>>
>> Let me know if you have any feedback/comments.
>>
>> Thanks,
>>
>> Ali
>>
>> [1]
https://docs.microsoft.com/en-us/visualstudio/productinfo/installing-an-earlier-release-of-vs2017
>> [2]
https://devblogs.microsoft.com/cppblog/side-by-side-minor-version-msvc-toolsets-in-visual-studio-2017/
>> [3]
http://hg.openjdk.java.net/jdk/jdk/file/0324b3756aa2/src/hotspot/share/runtime/vm_version.cpp#l228
>> [4]
http://hg.openjdk.java.net/jdk-updates/jdk12u/file/a5881e9139ca/src/hotspot/share/runtime/vm_version.cpp#l228
>> [5]
http://hg.openjdk.java.net/jdk-updates/jdk11u/file/30892e0c9533/src/hotspot/share/runtime/vm_version.cpp#l230
>> [6]
http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/ce13ce932e31/src/share/vm/runtime/vm_version.cpp#l233
>>
>> —————————
>>
>> diff -r 0324b3756aa2 make/autoconf/toolchain_windows.m4
>> --- a/make/autoconf/toolchain_windows.m4 Fri Mar 08
17:45:40 2019 -0500
>> +++ b/make/autoconf/toolchain_windows.m4 Sun Mar 10
17:19:22 2019 +0000
>> @@ -87,6 +87,7 @@
>> VS_VS_PLATFORM_NAME_2017="v141"
>> VS_SDK_PLATFORM_NAME_2017=
>> VS_SUPPORTED_2017=true
>> +VS_TOOLSET_SUPPORTED_2017=true
>>
################################################################################
>> @@ -177,6 +178,13 @@
>> # build environment and assigns it to VS_ENV_CMD
>> AC_DEFUN([TOOLCHAIN_FIND_VISUAL_STUDIO_BAT_FILE],
>> [
>> +# VS2017 provides the option to install previous minor
versions of the MSVC toolsets.
>> +# It is not possible to download earlier minor versions of
VS2017 and in order to build
>> +# with a previous minor compiler toolset version, it is now
possible to compile with
>> +# earlier minor versions by passing
-vcvars_ver=<toolset_version> argument to vcvarsall.bat
>> + AC_ARG_WITH(msvc-toolset-version,
[AS_HELP_STRING([--with-msvc-toolset-version],
>> + [specific MSVC toolset version to use, passed as
vcvars_ver argument to pass to vcvarsall.bat (Windows only)])])
>> +
>> VS_VERSION="$1"
>> eval VS_COMNTOOLS_VAR="\${VS_ENVVAR_${VS_VERSION}}"
>> eval VS_COMNTOOLS="\$${VS_COMNTOOLS_VAR}"
>> @@ -184,6 +192,7 @@
>> eval VS_EDITIONS="\${VS_EDITIONS_${VS_VERSION}}"
>> eval SDK_INSTALL_DIR="\${VS_SDK_INSTALLDIR_${VS_VERSION}}"
>> eval VS_ENV_ARGS="\${VS_ENV_ARGS_${VS_VERSION}}"
>> + eval
VS_TOOLSET_SUPPORTED="\${VS_TOOLSET_SUPPORTED_${VS_VERSION}}"
>> # When using --with-tools-dir, assume it points to the
correct and default
>> # version of Visual Studio or that --with-toolchain-version
was also set.
>> @@ -240,6 +249,12 @@
>> TOOLCHAIN_CHECK_POSSIBLE_WIN_SDK_ROOT([${VS_VERSION}],
>> [C:/Program Files (x86)/$SDK_INSTALL_DIR], [well-known
name])
>> fi
>> +
>> + if test "x$VS_TOOLSET_SUPPORTED" != x; then
>> + if test "x$msvc-toolset-version" != x; then
>> + VS_ENV_ARGS="$VS_ENV_ARGS
-vcvars_ver=$with_msvc_toolset_version"
>> + fi
>> + fi
>> ])
>>
################################################################################
>> @@ -423,6 +438,8 @@
>> >> $EXTRACT_VC_ENV_BAT_FILE
>> $ECHO 'echo VCINSTALLDIR="%VCINSTALLDIR% " >>
set-vs-env.sh' \
>> >> $EXTRACT_VC_ENV_BAT_FILE
>> + $ECHO 'echo VCToolsRedistDir="%VCToolsRedistDir% " >>
set-vs-env.sh' \
>> + >> $EXTRACT_VC_ENV_BAT_FILE
>> $ECHO 'echo WindowsSdkDir="%WindowsSdkDir% " >>
set-vs-env.sh' \
>> >> $EXTRACT_VC_ENV_BAT_FILE
>> $ECHO 'echo WINDOWSSDKDIR="%WINDOWSSDKDIR% " >>
set-vs-env.sh' \
>> @@ -440,6 +457,8 @@
>> >> $EXTRACT_VC_ENV_BAT_FILE
>> $ECHO "$WINPATH_BASH -c 'echo
VCINSTALLDIR="'\"$VCINSTALLDIR \" >> set-vs-env.sh' \
>> >> $EXTRACT_VC_ENV_BAT_FILE
>> + $ECHO "$WINPATH_BASH -c 'echo
VCToolsRedistDir="'\"$VCToolsRedistDir \" >> set-vs-env.sh' \
>> + >> $EXTRACT_VC_ENV_BAT_FILE
>> $ECHO "$WINPATH_BASH -c 'echo
WindowsSdkDir="'\"$WindowsSdkDir \" >> set-vs-env.sh' \
>> >> $EXTRACT_VC_ENV_BAT_FILE
>> $ECHO "$WINPATH_BASH -c 'echo
WINDOWSSDKDIR="'\"$WINDOWSSDKDIR \" >> set-vs-env.sh' \
>> @@ -517,6 +536,7 @@
>> VS_INCLUDE=`$ECHO "$VS_INCLUDE" | $SED -e 's/\\\\*;* *$//'`
>> VS_LIB=`$ECHO "$VS_LIB" | $SED 's/\\\\*;* *$//'`
>> VCINSTALLDIR=`$ECHO "$VCINSTALLDIR" | $SED 's/\\\\* *$//'`
>> + VCToolsRedistDir=`$ECHO "$VCToolsRedistDir" | $SED
's/\\\\* *$//'`
>> WindowsSdkDir=`$ECHO "$WindowsSdkDir" | $SED 's/\\\\*
*$//'`
>> WINDOWSSDKDIR=`$ECHO "$WINDOWSSDKDIR" | $SED 's/\\\\*
*$//'`
>> if test -z "$WINDOWSSDKDIR"; then
>> @@ -638,11 +658,13 @@
>>
POSSIBLE_MSVC_DLL="$CYGWIN_VC_INSTALL_DIR/redist/x86/Microsoft.VC${VS_VERSION_INTERNAL}.CRT/$DLL_NAME"
>> fi
>> else
>> + CYGWIN_VC_TOOLS_REDIST_DIR="$VCToolsRedistDir"
>> + BASIC_FIXUP_PATH(CYGWIN_VC_TOOLS_REDIST_DIR)
>> # Probe: Using well-known location from VS 2017
>> if test "x$OPENJDK_TARGET_CPU_BITS" = x64; then
>> - POSSIBLE_MSVC_DLL="`ls
$CYGWIN_VC_INSTALL_DIR/Redist/MSVC/*/x64/Microsoft.VC${VS_VERSION_INTERNAL}.CRT/$DLL_NAME`"
>> + POSSIBLE_MSVC_DLL="`ls
$CYGWIN_VC_TOOLS_REDIST_DIR/x64/Microsoft.VC${VS_VERSION_INTERNAL}.CRT/$DLL_NAME`"
>> else
>> - POSSIBLE_MSVC_DLL="`ls
$CYGWIN_VC_INSTALL_DIR/Redist/MSVC/*/x86/Microsoft.VC${VS_VERSION_INTERNAL}.CRT/$DLL_NAME`"
>> + POSSIBLE_MSVC_DLL="`ls
$CYGWIN_VC_TOOLS_REDIST_DIR/x86/Microsoft.VC${VS_VERSION_INTERNAL}.CRT/$DLL_NAME`"
>> fi
>> fi
>> # In case any of the above finds more than one file,
loop over them.
>>
>>
