Skip to content

Commit

Permalink
Enable win registry install location for all architectures (#54698)
Browse files Browse the repository at this point in the history
* 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()
  • Loading branch information
mateoatr authored Aug 20, 2021
1 parent bfc8732 commit 4209a95
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/apphost/standalone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
2 changes: 1 addition & 1 deletion src/native/corehost/comhost/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/common.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
9 changes: 2 additions & 7 deletions src/native/corehost/fxr_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/hostmisc/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 4 additions & 12 deletions src/native/corehost/hostmisc/pal.unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,18 +379,16 @@ bool pal::get_global_dotnet_dirs(std::vector<pal::string_t>* 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
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 2 additions & 12 deletions src/native/corehost/hostmisc/pal.windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand Down Expand Up @@ -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<pal::string_t>* dirs)
Expand Down
2 changes: 1 addition & 1 deletion src/native/corehost/ijwhost/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion src/native/corehost/test/nativehost/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit 4209a95

Please sign in to comment.