On Mon, 12 Aug 2024 15:37:10 GMT, Kevin Rushforth <k...@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
