From 08b249b2037a62bf7ebc1e3a7b371f724538c298 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 31 Aug 2021 09:03:17 -0700 Subject: [PATCH] [release/6.0] Enable win registry install location for all architectures (#57851) * Multi-arch support for win registry install locations * Change get_dotnet_self_registered_config_location signature * Link advapi32.lib on win-arm * PR Feedback * Indent * Update get_dotnet_self_registered_config_location() Co-authored-by: Mateo Torres Ruiz --- .../MultiArchInstallLocation.cs | 43 ++++++++++++++++--- .../apphost/standalone/CMakeLists.txt | 2 +- src/native/corehost/comhost/CMakeLists.txt | 2 +- src/native/corehost/common.cmake | 2 +- src/native/corehost/fxr_resolver.cpp | 9 +--- src/native/corehost/hostmisc/pal.h | 2 +- src/native/corehost/hostmisc/pal.unix.cpp | 16 ++----- src/native/corehost/hostmisc/pal.windows.cpp | 14 +----- src/native/corehost/ijwhost/CMakeLists.txt | 2 +- .../corehost/test/nativehost/CMakeLists.txt | 2 +- 10 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 120b899cff451..97de5222c6913 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -71,6 +71,31 @@ public void EnvironmentVariable_ArchSpecificDotnetRootIsUsedOverDotnetRoot() .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); } + [Fact] + public void EnvironmentVariable_DotNetRootIsUsedOverInstallLocationIfSet() + { + var fixture = sharedTestState.PortableAppFixture + .Copy(); + + var appExe = fixture.TestProject.AppExe; + var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); + var dotnet = fixture.BuiltDotnet.BinPath; + + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) + { + registeredInstallLocationOverride.SetInstallLocation((arch, "some/install/location")); + + Command.Create(appExe) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .DotNetRoot(dotnet, arch) + .Execute() + .Should().Pass() + .And.HaveUsedDotNetRootInstallLocation(dotnet, fixture.CurrentRid, arch) + .And.NotHaveStdErrContaining("Using global install location"); + } + } + [Fact] public void EnvironmentVariable_DotnetRootPathDoesNotExist() { @@ -122,7 +147,6 @@ public void EnvironmentVariable_DotnetRootPathExistsButHasNoHost() } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() { var fixture = sharedTestState.PortableAppFixture @@ -142,15 +166,20 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() (arch2, path2) }); - Command.Create(appExe) + CommandResult result = Command.Create(appExe) .EnableTracingAndCaptureOutputs() .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) .DotNetRoot(null) - .Execute() - .Should().HaveFoundDefaultInstallLocationInConfigFile(path1) - .And.HaveFoundArchSpecificInstallLocationInConfigFile(path1, arch1) - .And.HaveFoundArchSpecificInstallLocationInConfigFile(path2, arch2) - .And.HaveUsedGlobalInstallLocation(path2); + .Execute(); + + if (!OperatingSystem.IsWindows()) + { + result.Should().HaveFoundDefaultInstallLocationInConfigFile(path1) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(path1, arch1) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(path2, arch2); + } + + result.Should().HaveUsedGlobalInstallLocation(path2); } } diff --git a/src/native/corehost/apphost/standalone/CMakeLists.txt b/src/native/corehost/apphost/standalone/CMakeLists.txt index b17e63beaf4f1..d7a52a8534f99 100644 --- a/src/native/corehost/apphost/standalone/CMakeLists.txt +++ b/src/native/corehost/apphost/standalone/CMakeLists.txt @@ -49,5 +49,5 @@ endif() # Specify non-default Windows libs to be used for Arm/Arm64 builds if (CLR_CMAKE_TARGET_WIN32 AND (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64)) - target_link_libraries(apphost Advapi32.lib shell32.lib) + target_link_libraries(apphost shell32.lib) endif() diff --git a/src/native/corehost/comhost/CMakeLists.txt b/src/native/corehost/comhost/CMakeLists.txt index 7517306cdc1d6..36251ad79598b 100644 --- a/src/native/corehost/comhost/CMakeLists.txt +++ b/src/native/corehost/comhost/CMakeLists.txt @@ -35,7 +35,7 @@ if (CLR_CMAKE_TARGET_WIN32) # Specify non-default Windows libs to be used for Arm/Arm64 builds if (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64) - list(APPEND WINLIBS Advapi32.lib Ole32.lib OleAut32.lib) + list(APPEND WINLIBS Ole32.lib OleAut32.lib) endif() target_link_libraries(comhost ${WINLIBS}) diff --git a/src/native/corehost/common.cmake b/src/native/corehost/common.cmake index eb5faa6b5602d..717c3b5d89cf2 100644 --- a/src/native/corehost/common.cmake +++ b/src/native/corehost/common.cmake @@ -48,7 +48,7 @@ function(set_common_libs TargetType) # Specify the import library to link against for Arm32 build since the default set is minimal if (CLR_CMAKE_TARGET_ARCH_ARM) if (CLR_CMAKE_TARGET_WIN32) - target_link_libraries(${DOTNET_PROJECT_NAME} shell32.lib) + target_link_libraries(${DOTNET_PROJECT_NAME} shell32.lib advapi32.lib) else() target_link_libraries(${DOTNET_PROJECT_NAME} atomic.a) endif() diff --git a/src/native/corehost/fxr_resolver.cpp b/src/native/corehost/fxr_resolver.cpp index 6e1356457d49a..b59c59f10fd80 100644 --- a/src/native/corehost/fxr_resolver.cpp +++ b/src/native/corehost/fxr_resolver.cpp @@ -100,13 +100,8 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o pal::get_default_installation_dir(&default_install_location); } - pal::string_t self_registered_config_location; - pal::string_t self_registered_message; - if (pal::get_dotnet_self_registered_config_location(&self_registered_config_location)) - { - self_registered_message = - pal::string_t(_X(" or register the runtime location in [") + self_registered_config_location + _X("]")); - } + pal::string_t self_registered_config_location = pal::get_dotnet_self_registered_config_location(); + pal::string_t self_registered_message = _X(" or register the runtime location in [") + self_registered_config_location + _X("]"); trace::error(_X("A fatal error occurred. The required library %s could not be found.\n" "If this is a self-contained application, that library should exist in [%s].\n" diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index b6622cd5dff52..0932e0db5b60e 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -287,7 +287,7 @@ namespace pal // Returns the globally registered install location (if any) bool get_dotnet_self_registered_dir(string_t* recv); // Returns name of the global registry location (for error messages) - bool get_dotnet_self_registered_config_location(string_t* recv); + string_t get_dotnet_self_registered_config_location(); // Returns the default install location for a given platform bool get_default_installation_dir(string_t* recv); diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 4a3e86f2f567a..ef644bdf0d7a3 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -379,18 +379,16 @@ bool pal::get_global_dotnet_dirs(std::vector* recv) return false; } -bool pal::get_dotnet_self_registered_config_location(pal::string_t* recv) +pal::string_t pal::get_dotnet_self_registered_config_location() { - recv->assign(_X("/etc/dotnet/install_location")); - // ***Used only for testing*** pal::string_t environment_install_location_override; if (test_only_getenv(_X("_DOTNET_TEST_INSTALL_LOCATION_FILE_PATH"), &environment_install_location_override)) { - recv->assign(environment_install_location_override); + return environment_install_location_override; } - return true; + return _X("/etc/dotnet/install_location"); } namespace @@ -429,13 +427,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } // *************************** - pal::string_t install_location_file_path; - if (!get_dotnet_self_registered_config_location(&install_location_file_path)) - { - return false; - } - // *************************** - + pal::string_t install_location_file_path = get_dotnet_self_registered_config_location(); trace::verbose(_X("Looking for install_location file in '%s'."), install_location_file_path.c_str()); FILE* install_location_file = pal::file_open(install_location_file_path, "r"); if (install_location_file == nullptr) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 87a5508badbf5..2a964f06868e7 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -322,27 +322,18 @@ namespace } } -bool pal::get_dotnet_self_registered_config_location(pal::string_t* recv) +pal::string_t pal::get_dotnet_self_registered_config_location() { -#if !defined(TARGET_AMD64) && !defined(TARGET_X86) - return false; -#else HKEY key_hive; pal::string_t sub_key; const pal::char_t* value; get_dotnet_install_location_registry_path(&key_hive, &sub_key, &value); - recv->assign((key_hive == HKEY_CURRENT_USER ? _X("HKCU\\") : _X("HKLM\\")) + sub_key + _X("\\") + value); - return true; -#endif + return (key_hive == HKEY_CURRENT_USER ? _X("HKCU\\") : _X("HKLM\\")) + sub_key + _X("\\") + value; } bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) { -#if !defined(TARGET_AMD64) && !defined(TARGET_X86) - // Self-registered SDK installation directory is only supported for x64 and x86 architectures. - return false; -#else recv->clear(); // ***Used only for testing*** @@ -392,7 +383,6 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) recv->assign(buffer.data()); ::RegCloseKey(hkey); return true; -#endif } bool pal::get_global_dotnet_dirs(std::vector* dirs) diff --git a/src/native/corehost/ijwhost/CMakeLists.txt b/src/native/corehost/ijwhost/CMakeLists.txt index d790b01709f12..036ae1d386e4c 100644 --- a/src/native/corehost/ijwhost/CMakeLists.txt +++ b/src/native/corehost/ijwhost/CMakeLists.txt @@ -49,7 +49,7 @@ include(../lib.cmake) # Specify non-default Windows libs to be used for Arm/Arm64 builds if (CLR_CMAKE_TARGET_WIN32 AND (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64)) - target_link_libraries(ijwhost Advapi32.lib Ole32.lib) + target_link_libraries(ijwhost Ole32.lib) endif() install_with_stripped_symbols(ijwhost TARGETS corehost) diff --git a/src/native/corehost/test/nativehost/CMakeLists.txt b/src/native/corehost/test/nativehost/CMakeLists.txt index 44282bc19e432..c21e95a1c71d9 100644 --- a/src/native/corehost/test/nativehost/CMakeLists.txt +++ b/src/native/corehost/test/nativehost/CMakeLists.txt @@ -56,5 +56,5 @@ endif() # Specify non-default Windows libs to be used for Arm/Arm64 builds if (CLR_CMAKE_TARGET_WIN32 AND (CLR_CMAKE_TARGET_ARCH_ARM OR CLR_CMAKE_TARGET_ARCH_ARM64)) - target_link_libraries(${DOTNET_PROJECT_NAME} Advapi32.lib Ole32.lib OleAut32.lib) + target_link_libraries(${DOTNET_PROJECT_NAME} Ole32.lib OleAut32.lib) endif()