Skip to content

Commit

Permalink
[build] Use $(VSINSTALLDIR), not $(VSINSTALLROOT) (#1048)
Browse files Browse the repository at this point in the history
Context: 3824b97
Context: a3de91e
Context: 5c756b1
Context: https://learn.microsoft.com/en-us/cpp/build/reference/msbuild-visual-cpp-overview?view=msvc-170

There is an "interesting" dichotomy between local Windows builds and
CI builds of xamarin/Java.Interop regarding the `%VSINSTALLROOT%`
environment variable:

 1. `%VSINSTALLDIR%` is set automatically on CI for .NET Framework.

 2. `%VSINSTALLROOT%`is *not* set automatically for .NET Framework or
    .NET Core, which is why commit 5c756b1 added `VSInstallRoot` to
    the *global* variables block in `azure-pipelines.yaml`.

 3. `%VSINSTALLDIR%` *is* set, but `%VSINSTALLROOT%` is *not* set when
    using the [**Developer Command Prompt For…**][0] shortcut item.

The result of this is that `src/java-interop` is built on CI, thus
ensuring that it continues to build and works via unit tests (yay),
but it is *not* easy to build in a local checkout.  The build doesn't
fail; it just does not produce per-arch `java-interop.dll` files.
(Handy way to check for this: in a diagnostic build log, search for
`NativeToolchainSupported = True`.  If you don't see it, you don't
have Windows support.)

This makes things "weird" when trying to help people build the repo
on Windows.

Furthermore, CI *didn't* use `%VSINSTALLDIR%`, because it was not set
when building for .NET Core; `%VSINSTALLROOT%` was set "consistently"
for "everyone" (except for local builds).

Try to restore some degree of sanity: *probe* for and support *both*
`%VSINSTALLDIR%` *and* `%VSINSTALLROOT%`, with `%VSINSTALLROOT%`
taking priority, and `%VSINSTALLDIR%` as fallback.

Update `azure-pipelines.yaml` so that only .NET Core builds set the
`VSINSTALLDIR` environment variable.  .NET Framework builds will use
the implicitly set `%VsInstallRoot%` env var.

Finally, for .NET Core builds, `%VSINSTALLDIR%` ***MUST***
[end with a trailing `\`][1].  If it doesn't, the build will fail in
"mysterious" ways:

	  Task "Exec" (TaskId:943)
	    Task Parameter:Command=call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat"  x86_amd64 && cmake -G "NMake Makefiles" -S "D:/a/_work/1/s/src/java-interop/" -B "D:/a/_work/1/s/src/java-interop/obj//Release-net7.0/win-x64/" "-DJDK_INCLUDE_LIST=C:/hostedtoolcache/windows/Java_Temurin-Hotspot_jdk/8.0.345-1/x64/include;C:/hostedtoolcache/windows/Java_Temurin-Hotspot_jdk/8.0.345-1/x64/include/win32" "-DJNI_C_PATH=D:/a/_work/1/s/src/java-interop/obj//Release-net7.0/jni.c" && cmake --build "D:/a/_work/1/s/src/java-interop/obj//Release-net7.0/win-x64/" -v (TaskId:943)
	    call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat"  x86_amd64 && cmake -G "NMake Makefiles" -S "D:/a/_work/1/s/src/java-interop/" -B "D:/a/_work/1/s/src/java-interop/obj//Release-net7.0/win-x64/" "-DJDK_INCLUDE_LIST=C:/hostedtoolcache/windows/Java_Temurin-Hotspot_jdk/8.0.345-1/x64/include;C:/hostedtoolcache/windows/Java_Temurin-Hotspot_jdk/8.0.345-1/x64/include/win32" "-DJNI_C_PATH=D:/a/_work/1/s/src/java-interop/obj//Release-net7.0/jni.c" && cmake --build "D:/a/_work/1/s/src/java-interop/obj//Release-net7.0/win-x64/" -v (TaskId:943)
	    ********************************************************************** (TaskId:943)
	    ** Visual Studio 2022 Developer Command Prompt v17.3.5 (TaskId:943)
	    ** Copyright (c) 2022 Microsoft Corporation (TaskId:943)
	    ********************************************************************** (TaskId:943)
	    [ERROR:VsDevCmd.bat] *** VsDevCmd.bat encountered errors. Environment may be incomplete and/or incorrect. *** (TaskId:943)
	    [ERROR:VsDevCmd.bat] In an uninitialized command prompt, please 'set VSCMD_DEBUG=[value]' and then re-run (TaskId:943)
	    [ERROR:VsDevCmd.bat] vsdevcmd.bat [args] for additional details. (TaskId:943)
	    [ERROR:VsDevCmd.bat] Where [value] is: (TaskId:943)
	    [ERROR:VsDevCmd.bat]    1 : basic debug logging (TaskId:943)
	    [ERROR:VsDevCmd.bat]    2 : detailed debug logging (TaskId:943)
	    [ERROR:VsDevCmd.bat]    3 : trace level logging. Redirection of output to a file when using this level is recommended. (TaskId:943)
	    [ERROR:VsDevCmd.bat] Example: set VSCMD_DEBUG=3 (TaskId:943)
	    [ERROR:VsDevCmd.bat]          vsdevcmd.bat > vsdevcmd.trace.txt 2>&1 (TaskId:943)
	    CMake Error at CMakeLists.txt:3 (project): (TaskId:943)
	…
	##[warning]EXEC(0,0): Warning : CMAKE_CXX_COMPILER not set, after EnableLanguage
	##[warning]EXEC(0,0): Warning : CMAKE_C_COMPILER not set, after EnableLanguage

[0]: https://learn.microsoft.com/en-us/cpp/build/building-on-the-command-line?view=msvc-170#developer_command_file_locations
[1]: https://developercommunity.visualstudio.com/t/vcvars32bat-fails-after-upgrade-to-vs-2017-version/375841
  • Loading branch information
jonpryor authored Oct 10, 2022
1 parent 8e4c7d2 commit 16e1ecd
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
3 changes: 2 additions & 1 deletion build-tools/automation/azure-pipelines.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ variables:
1ESWindowsImage: AzurePipelinesWindows2022compliant
1ESMacPool: Azure Pipelines
1ESMacImage: internal-macos-11
VSInstallRoot: C:\Program Files\Microsoft Visual Studio\2022\Enterprise

jobs:
- job: windows_build
Expand Down Expand Up @@ -85,6 +84,8 @@ jobs:
name: $(1ESWindowsPool)
demands:
- ImageOverride -equals $(1ESWindowsImage)
variables:
VSINSTALLDIR: C:\Program Files\Microsoft Visual Studio\2022\Enterprise\
timeoutInMinutes: 20
workspace:
clean: all
Expand Down
6 changes: 4 additions & 2 deletions build-tools/scripts/NativeToolchain.targets
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>
<PropertyGroup Condition=" $([MSBuild]::IsOSPlatform ('windows')) And '$(VSINSTALLROOT)' != '' ">
<_VcvarsallPath>$(VSINSTALLROOT)\VC\Auxiliary\Build\vcvarsall.bat</_VcvarsallPath>
<PropertyGroup Condition=" $([MSBuild]::IsOSPlatform ('windows')) ">
<_VSInstallDir Condition=" '$(VsInstallRoot)' != '' ">$(VsInstallRoot)</_VSInstallDir>
<_VSInstallDir Condition=" '$(_VSInstallDir)' == '' And '$(VsInstallDir)' != '' ">$(VsInstallDir)</_VSInstallDir>
<_VcvarsallPath Condition=" '$(_VSInstallDir)' != '' ">$(_VSInstallDir)\VC\Auxiliary\Build\vcvarsall.bat</_VcvarsallPath>
</PropertyGroup>

<PropertyGroup Condition=" $([MSBuild]::IsOSPlatform ('windows')) And Exists ('$(_VcvarsallPath)') ">
Expand Down

0 comments on commit 16e1ecd

Please sign in to comment.