RFR: 8289174: JavaFX build fails on Windows when VS150COMNTOOLS is not set [v2]

[Phil Race]

On Mon, 12 Aug 2024 15:37:10 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> 1. Added logic to look in the standard locations for Visual Studio 2022, with a fallback to 2019 and then 2017 if 2022 is not found. It will look in the following locations:
>> 
>> C:("Program Files"|"Program Files (x86)")\Microsoft Visual Studio(2022|2019|2017)\
>>      (Enterprise|Professional|Community)\VC\Auxiliary\Build
>> 
>> 2. Use `VSCOMNTOOLS` as the primary way for a developer to specify a custom location for Visual Studio, with `VS150COMNTOOLS` as a legacy fallback (a warning is printed if `VS150COMNTOOLS` is set). If a developer installs Visual Studio in the default location, there will be no need to set this any more.
>> 3. Removed support for older compilers (VS2010 and VS2013 in particular), since they will no longer work anyway. We were still using the 32-bit version of the script in many cases, which is unnecessary and no longer makes sense, so as part of this cleanup, I now consistently use `vcvars64.bat` to set up the environment.
>> 4. Modified the GitHub actions build to no longer specify `VS150COMNTOOLS`, since it will now find it automatically.
>> 5. Removed the following unused variables:
>> 
>> DEVENVCMD
>> DXSDK_DIR
>> VS_VER
>> 
>> 6. Changed most of the defaults in `win.gradle` to the empty string. The defaults were a misguided attempt to set values to something in case the Visual Studio installation cannot be found. By not having defaults, it will be more obvious if something goes wrong (and will fail fast).
>> 7. If the Visual Studio Installation cannot be found, it will print a sensible error message and retry the next time the build is run (making it less likely that a manual `rm -rf build` is needed).
>> 8. Fixed a bug where the Microsoft redist files were not being located and copied into the build dir (build/sdk/bin).
>> 
>> 
>> I left some debug print statements in, and will remove them in a follow-on commit.
>> 
>> I have tested this with a local installation of Visual Studio 2022 Community and, via GitHub Actions, an installation of Visual Studio 2022 Enterprise. On my local system, I built with and without Media and WebKit.
>
> Kevin Rushforth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert "Add debug prints"
>   
>   This reverts commit 3ea8ee58867d21e5c0aeb6a22170cdc28dd7a486.

buildSrc/genVSproperties.bat line 27:

> 25: 
> 26: REM Windows bat file that runs vcvars64.bat for Visual Studio
> 27: REM and echos out a property file with the values of the environment

nit : I think it is usually spelt echoes.

buildSrc/genVSproperties.bat line 55:

> 53:             set edition=%%b
> 54:             for %%c in ("Program Files", "Program Files (x86)") do (
> 55:                 set ProgramFiles=%%~c

What does the ~ do here ?

buildSrc/win.gradle line 93:

> 91:         defineProperty("WINDOWS_VS_LIB", properties, "$WINDOWS_VS_VCINSTALLDIR/LIB;" + "$WINDOWS_SDK_DIR/lib;")
> 92:         defineProperty("WINDOWS_VS_LIBPATH", properties, "$WINDOWS_VS_VCINSTALLDIR/LIB;")
> 93:         def parfaitPath = IS_COMPILE_PARFAIT ? System.getenv().get("PARFAIT_PATH") + ";" : "";

Did you mean to remove parfaitPath ?

buildSrc/win.gradle line 113:

> 111:     IS_DEBUG_NATIVE ? ["/MDd", "/Od", "/Zi", "/DDEBUG"] : ["/O2", "/MD", "/DNDEBUG"]
> 112: 
> 113: // Serialize access to PDB file for debug builds if on VS2013 or later

The "if on VS2013 or later" part of the comment is obsolete.

buildSrc/win.gradle line 141:

> 139: 
> 140: // Remove C++ static linking if not on VS2010
> 141: ccFlags -= ["/D_STATIC_CPPLIB", "/D_DISABLE_DEPRECATE_STATIC_CPPLIB"]

The "if not on VS2010" part of the comment is obsolete.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1534#discussion_r1714334014
PR Review Comment: https://git.openjdk.org/jfx/pull/1534#discussion_r1714339338
PR Review Comment: https://git.openjdk.org/jfx/pull/1534#discussion_r1714344403
PR Review Comment: https://git.openjdk.org/jfx/pull/1534#discussion_r1714342671
PR Review Comment: https://git.openjdk.org/jfx/pull/1534#discussion_r1714343779