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