JDK-8187366 : Remove hardcoded MSVC Version in win.gradle
  • Type: Bug
  • Component: javafx
  • Sub-Component: build
  • Affected Version: 8u151,9.0.1,10
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2017-09-08
  • Updated: 2017-12-11
  • Resolved: 2017-09-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10 JDK 8
10Fixed 8u172Fixed
Related Reports
Duplicate :  
Relates :  
Description
Currently we hardcoded "14.10.25017" to extract the msvcBInDir.  When i installed the MSVC 2017 pro update, the version changed to "14.11.25503".
When i clean build and tried to compile with new version (14.11.25503), im getting below error.

Execution failed for task ':graphics:linkWinIio'.
> A problem occurred starting process 'command 'C:/Program Files (x86)/Microsoft Visual Studio/2017/Professional/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/link.exe''

To proceed compilation, I manually edited below code in win.gradle from 25017 to 25503.  I feel it is better to avoid hardcoded values in win.gradle.


def msvcBinDir = ""
if (winVsVer == 150) {
    msvcBinDir = (IS_64
              ? "$WINDOWS_VS_VSINSTALLDIR/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64"
             : "$WINDOWS_VS_VSINSTALLDIR/VC/Tools/MSVC/14.10.25017/bin/HostX86/x86")
   } else if (winVsVer <= 120) {
              msvcBinDir = (IS_64
                        ? "$WINDOWS_VS_VSINSTALLDIR/VC/BIN/amd64"
                       : "$WINDOWS_VS_VSINSTALLDIR/VC/BIN")

Comments
Note that if the 10-dev changeset imports cleanly then there is no need for a separate review.
16-10-2017

Approved to backport to 8u-dev for 8u172.
16-10-2017

8-backport request: This fix is recommended to be back ported to 8, considering developers unable to build once they upgrade to VS 2017.
16-10-2017

Changeset: c77ae1a020b3 Author: mbilla Date: 2017-09-29 08:55 +0530 URL: http://hg.openjdk.java.net/openjfx/10-dev/rt/rev/c77ae1a020b3
29-09-2017

+1
29-09-2017

We might consider for an 8u backport, since otherwise developers with recent VS 2017 installations are unable to build without patching locally.
29-09-2017

For #2, that is what we tested with and what we use in production (there were some problems with the 14.10.25017 redist libraries, so we ship the 14.10.25008 binaries).
28-09-2017

webrev: http://cr.openjdk.java.net/~mbilla/8187366/webrev.02/ 1) Incorporated 1st comment. 2) Not sure about why the versions for MSVC_VER and MSVC_REDIST_VER are different as these versions are reflected as part of VS 2017 installation.
28-09-2017

lgtm with nits. 1. I guess the below statement can be rewritten using groovy elvis operator(?:) for better readablity. +def msvcVer = System.getenv("MSVC_VER") +if (msvcVer == null || msvcVer == "") { + msvcVer = "14.10.25017" +} def msvcVer = System.getenv("MSVC_VER") ?: "14.10.25017" 2. Why are we using 2 different versions for MSVC_VER and MSVC_REDIST_VER
28-09-2017

webrev: http://cr.openjdk.java.net/~mbilla/8187366/webrev.01/ webrev.01 includes changes for fxpackager issues as well.. Developers need to set 3 system variables to override default values: export MSVC_VER="14.11.25503" (default: 14.10.25017) export MSVC_REDIST_VER="14.11.25325" (default: 14.10.25008) export WINDOWS_CRT_VER="141" (default: 150)
28-09-2017

webrev: http://cr.openjdk.java.net/~mbilla/8187366/webrev.00/ Developers need to set the system variable "MSVC_VER" to the corresponding VS 2017 version like export MSVC_VER="14.11.25503" If "MSVC_VER" value is not set, then default value (which is 14.10.25017) will be used.
27-09-2017

This might be good cleanup to do some day, but it would need to be done carefully, since for production we want to control the version used.
08-09-2017