From 89c6a26189fea9a7a23204ffd48da7a75d36eb00 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 28 May 2021 13:48:14 -0700 Subject: [PATCH 01/23] Add support for multiple architectures inside install_locations --- src/native/corehost/fxr_resolver.cpp | 2 +- src/native/corehost/hostmisc/pal.h | 1 + src/native/corehost/hostmisc/pal.unix.cpp | 65 ++++++++++++++++---- src/native/corehost/hostmisc/pal.windows.cpp | 7 +++ src/native/corehost/hostmisc/utils.cpp | 9 +++ 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/native/corehost/fxr_resolver.cpp b/src/native/corehost/fxr_resolver.cpp index d457047ddc8f9f..e4e9c5fc84a096 100644 --- a/src/native/corehost/fxr_resolver.cpp +++ b/src/native/corehost/fxr_resolver.cpp @@ -65,7 +65,7 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o return true; } - // For framework-dependent apps, use DOTNET_ROOT + // For framework-dependent apps, use DOTNET_ROOT_ pal::string_t default_install_location; pal::string_t dotnet_root_env_var_name = get_dotnet_root_env_var_name(); if (get_file_path_from_env(dotnet_root_env_var_name.c_str(), out_dotnet_root)) diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 27daf76c72723e..83bc87af2ede25 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -252,6 +252,7 @@ namespace pal string_t get_timestamp(); bool getcwd(string_t* recv); + string_t to_lower(const string_t& in); string_t to_lower(const char_t* in); diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 2db2dd8488f562..33d599db1ab42c 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -57,6 +57,13 @@ pal::string_t pal::to_lower(const pal::char_t* in) return ret; } +pal::string_t pal::to_upper(const pal::string_t& in) +{ + pal::string_t ret = in; + std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); + return ret; +} + pal::string_t pal::get_timestamp() { std::time_t t = std::time(nullptr); @@ -424,31 +431,63 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) FILE* install_location_file = pal::file_open(install_location_file_path, "r"); if (install_location_file == nullptr) { - trace::verbose(_X("The install_location file failed to open.")); + trace::error(_X("The install_location file failed to open.")); return false; } - bool result = false; - - char buf[PATH_MAX]; - char* install_location = fgets(buf, sizeof(buf), install_location_file); - if (install_location != nullptr) + char buffer[PATH_MAX]; + pal::string_t install_location, arch_install_location; + bool is_first_line = true; + while (fgets(buffer, sizeof(buffer), install_location_file)) { - size_t len = pal::strlen(install_location); - + size_t len = strlen(buffer); // fgets includes the newline character in the string - so remove it. - if (len > 0 && len < PATH_MAX && install_location[len - 1] == '\n') + if (len > 0 && len < PATH_MAX && buffer[len - 1] == '\n') { - install_location[len - 1] = '\0'; + buffer[len - 1] = '\0'; } - trace::verbose(_X("Using install location '%s'."), install_location); + char* pDelimiter = strchr(buffer, '='); + if (pDelimiter == nullptr) + { + if (is_first_line) + { + install_location = pal::string_t(buffer); + trace::verbose(_X("Found install location path '%s'.", install_location.c_str())) + } + else + { + trace::error(_X("Only the first line in '%s' may not have an architecture prefix.", install_location_file_path.c_str())); + } + + is_first_line = false; + continue; + } + + auto delimiter_index = (int)(strlen(buffer) - strlen(pDelimiter)); + pal::string_t arch_prefix = pal::string_t(buffer).substr(0, delimiter_index); + pal::string_t path_to_location = pal::string_t(buffer).substr(delimiter_index + 1); + + trace::verbose(_X("Found architecture-specific install logcation path: '%s' ('%s').", path_to_location.c_str(), arch_prefix.c_str())); + if (arch_prefix == get_arch()) + { + arch_install_location = path_to_location; + trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'.", arch_prefix.c_str(), arch_install_location.c_str())); + } + } + + if (architecture_prefixed_install_location != nullptr) + { + *recv = architecture_prefixed_install_location; + } + else if (install_location != nullptr) + { *recv = install_location; - result = true; } else { - trace::verbose(_X("The install_location file first line could not be read.")); + trace::error(_X("Did not find any install location in '%s'", install_location_file_path.c_str())); + return false; } fclose(install_location_file); diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 8a9e35a3236f0e..563f9a011243fd 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -47,6 +47,13 @@ pal::string_t pal::to_lower(const pal::char_t* in) return ret; } +pal::string_t pal::to_upper(const pal::string_t& in) +{ + pal::string_t ret = in; + std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); + return ret; +} + pal::string_t pal::get_timestamp() { std::time_t t = std::time(nullptr); diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index 055b4f6a31dd81..fdb9e343badfe6 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -350,11 +350,20 @@ bool try_stou(const pal::string_t& str, unsigned* num) pal::string_t get_dotnet_root_env_var_name() { + pal::string_t dotnet_root_arch = _X("DOTNET_ROOT_"), path; + dotnet_root_arch.append(pal::to_upper(get_arch())); + if (pal::getenv(dotnet_root_arch.c_str(), &path)) + return dotnet_root_arch; + +#if defined(WIN32) if (pal::is_running_in_wow64()) { return pal::string_t(_X("DOTNET_ROOT(x86)")); } +#endif + // If no architecture-specific environment variable was set + // fallback to the default DOTNET_ROOT. return pal::string_t(_X("DOTNET_ROOT")); } From fd767f12dc8510edb89daf0ed8d571428da1d6c3 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Tue, 1 Jun 2021 10:42:29 -0700 Subject: [PATCH 02/23] Fix traces --- src/native/corehost/hostmisc/pal.unix.cpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 33d599db1ab42c..a5545272399391 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -436,7 +436,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } char buffer[PATH_MAX]; - pal::string_t install_location, arch_install_location; + pal::string_t install_location, architecture_install_location; bool is_first_line = true; while (fgets(buffer, sizeof(buffer), install_location_file)) { @@ -453,11 +453,11 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) if (is_first_line) { install_location = pal::string_t(buffer); - trace::verbose(_X("Found install location path '%s'.", install_location.c_str())) + trace::verbose(_X("Found install location path '%s'."), install_location.c_str()); } else { - trace::error(_X("Only the first line in '%s' may not have an architecture prefix.", install_location_file_path.c_str())); + trace::error(_X("Only the first line in '%s' may not have an architecture prefix."), install_location_file_path.c_str()); } is_first_line = false; @@ -468,30 +468,31 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) pal::string_t arch_prefix = pal::string_t(buffer).substr(0, delimiter_index); pal::string_t path_to_location = pal::string_t(buffer).substr(delimiter_index + 1); - trace::verbose(_X("Found architecture-specific install logcation path: '%s' ('%s').", path_to_location.c_str(), arch_prefix.c_str())); + trace::verbose(_X("Found architecture-specific install logcation path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); if (arch_prefix == get_arch()) { - arch_install_location = path_to_location; - trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'.", arch_prefix.c_str(), arch_install_location.c_str())); + architecture_install_location = path_to_location; + trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'."), arch_prefix.c_str(), architecture_install_location.c_str()); } } - if (architecture_prefixed_install_location != nullptr) + if (architecture_install_location != "") { - *recv = architecture_prefixed_install_location; + *recv = architecture_install_location; } - else if (install_location != nullptr) + else if (install_location != "") { *recv = install_location; } else { - trace::error(_X("Did not find any install location in '%s'", install_location_file_path.c_str())); + trace::error(_X("Did not find any install location in '%s'"), install_location_file_path.c_str()); return false; } + trace::verbose(_X("Using install location '%s'."), (*recv).c_str()); fclose(install_location_file); - return result; + return true; } bool pal::get_default_installation_dir(pal::string_t* recv) From 1de63dc0281bf41d498c7998a9bee7344970a6a3 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Tue, 1 Jun 2021 15:24:49 -0700 Subject: [PATCH 03/23] Add install_location tests --- .../MultilevelSDKLookup.cs | 2 +- .../NativeHosting/Nethost.cs | 93 ++++++++++++++++++- .../PortableAppActivation.cs | 2 +- .../RegisteredInstallLocationOverride.cs | 7 +- 4 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs b/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs index 97b3387604b623..39d8ca1a79c2e2 100644 --- a/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs +++ b/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs @@ -501,7 +501,7 @@ public void SdkMultilevelLookup_RegistryAccess() using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(DotNet.GreatestVersionHostFxrFilePath)) { - registeredInstallLocationOverride.SetInstallLocation(_regDir, RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation(new string[] { _regDir }, RepoDirectories.BuildArchitecture); // Add SDK versions AddAvailableSdkVersions(_regSdkBaseDir, "9999.0.4"); diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index 0f18161b026827..ed98ea915482eb 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -116,7 +116,7 @@ public void GetHostFxrPath_GlobalInstallation(bool explicitLoad, bool useAssembl { if (useRegisteredLocation) { - registeredInstallLocationOverride.SetInstallLocation(installLocation, sharedState.RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation(new string[] { installLocation }, sharedState.RepoDirectories.BuildArchitecture); } result = Command.Create(sharedState.NativeHostPath, $"{GetHostFxrPath} {explicitLoad} {(useAssemblyPath ? sharedState.TestAssemblyPath : string.Empty)}") @@ -194,7 +194,7 @@ public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - File.WriteAllText(registeredInstallLocationOverride.PathValueOverride, string.Format(value, installLocation)); + registeredInstallLocationOverride.SetInstallLocation(new string[] { string.Format(value, installLocation) }); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -223,6 +223,95 @@ public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) } } + [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] + public void GetHostFxrPath_GlobalInstallation_HasMoreThanOneDefaultInstallationPath() + { + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); + + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) + { + registeredInstallLocationOverride.SetInstallLocation(new string[] { installLocation, installLocation }); + + CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .EnvironmentVariable( // Redirect the default install location to an invalid location so that it doesn't cause the test to pass + Constants.TestOnlyEnvironmentVariables.DefaultInstallPath, + sharedState.InvalidInstallRoot) + .DotNetRoot(null) + .Execute(); + + result.Should().Pass() + .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") + .And.HaveStdOutContaining($"Found install location path '{installLocation}'.") + .And.HaveStdErrContaining($"Only the first line in '{registeredInstallLocationOverride.PathValueOverride}' may not have an architecture prefix.") + .And.HaveStdErrContaining($"Using install location '{installLocation}'.") + .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); + } + } + + [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] + public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() + { + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); + + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) + { + registeredInstallLocationOverride.SetInstallLocation(new string[] { + $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", + $"someOtherArch={installLocation}/invalid/" + }); + + CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .EnvironmentVariable( // Redirect the default install location to an invalid location so that it doesn't cause the test to pass + Constants.TestOnlyEnvironmentVariables.DefaultInstallPath, + sharedState.InvalidInstallRoot) + .DotNetRoot(null) + .Execute(); + + result.Should().Pass() + .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") + .And.HaveStdOutContaining($"Found architecture-specific install logcation path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") + .And.HaveStdErrContaining($"Using install location '{installLocation}'.") + .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); + } + } + + [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] + public void GetHostFxrPath_GlobalInstallation_ArchitectureSpecificPathIsPickedOverDefaultPath() + { + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); + + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) + { + registeredInstallLocationOverride.SetInstallLocation(new string[] { + $"{installLocation}/a/b/c", + $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", + }); + + CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .EnvironmentVariable( // Redirect the default install location to an invalid location so that it doesn't cause the test to pass + Constants.TestOnlyEnvironmentVariables.DefaultInstallPath, + sharedState.InvalidInstallRoot) + .DotNetRoot(null) + .Execute(); + + result.Should().Pass() + .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") + .And.HaveStdOutContaining($"Found install location path '{installLocation}/a/b/c'.") + .And.HaveStdOutContaining($"Found architecture-specific install logcation path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") + .And.HaveStdErrContaining($"Using install location '{installLocation}'.") + .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); + } + } + [Fact] public void GetHostFxrPath_InvalidParameters() { diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index 14060b5e944bfa..a93e95e8e83d14 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -300,7 +300,7 @@ public void AppHost_FrameworkDependent_GlobalLocation_Succeeds(bool useRegistere string architecture = fixture.CurrentRid.Split('-')[1]; if (useRegisteredLocation) { - registeredInstallLocationOverride.SetInstallLocation(builtDotnet, architecture); + registeredInstallLocationOverride.SetInstallLocation(new string[] { builtDotnet }, architecture); } // Verify running with the default working directory diff --git a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs index 7a30c5d6fd08dc..88c68d24d07090 100644 --- a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs +++ b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs @@ -61,18 +61,19 @@ public RegisteredInstallLocationOverride(string productBinaryPath) } } - public void SetInstallLocation(string installLocation, string architecture) + public void SetInstallLocation(string[] installLocation, string architecture = "") { if (OperatingSystem.IsWindows()) { + Debug.Assert(installLocation.Length == 1 && !string.IsNullOrEmpty(architecture)); using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{architecture}")) { - dotnetLocationKey.SetValue("InstallLocation", installLocation); + dotnetLocationKey.SetValue("InstallLocation", installLocation[0]); } } else { - File.WriteAllText(PathValueOverride, installLocation); + File.WriteAllText(PathValueOverride, string.Join (Environment.NewLine, installLocation)); } } From 48be3190383b6d5c7726abc81e1498402944fd15 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Wed, 2 Jun 2021 09:11:06 -0700 Subject: [PATCH 04/23] Message printed to stderr --- .../HostActivation.Tests/NativeHosting/Nethost.cs | 11 ++++++----- src/native/corehost/hostmisc/pal.unix.cpp | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index ed98ea915482eb..eed7c226eafc49 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -244,7 +244,7 @@ public void GetHostFxrPath_GlobalInstallation_HasMoreThanOneDefaultInstallationP result.Should().Pass() .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") - .And.HaveStdOutContaining($"Found install location path '{installLocation}'.") + .And.HaveStdErrContaining($"Found install location path '{installLocation}'.") .And.HaveStdErrContaining($"Only the first line in '{registeredInstallLocationOverride.PathValueOverride}' may not have an architecture prefix.") .And.HaveStdErrContaining($"Using install location '{installLocation}'.") .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); @@ -261,7 +261,7 @@ public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() { registeredInstallLocationOverride.SetInstallLocation(new string[] { $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", - $"someOtherArch={installLocation}/invalid/" + $"someOtherArch={installLocation}/invalid" }); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) @@ -275,7 +275,8 @@ public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() result.Should().Pass() .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") - .And.HaveStdOutContaining($"Found architecture-specific install logcation path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") + .And.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") + .And.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}/invalid' ('someOtherArch').") .And.HaveStdErrContaining($"Using install location '{installLocation}'.") .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); } @@ -305,8 +306,8 @@ public void GetHostFxrPath_GlobalInstallation_ArchitectureSpecificPathIsPickedOv result.Should().Pass() .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") - .And.HaveStdOutContaining($"Found install location path '{installLocation}/a/b/c'.") - .And.HaveStdOutContaining($"Found architecture-specific install logcation path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") + .And.HaveStdErrContaining($"Found install location path '{installLocation}/a/b/c'.") + .And.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") .And.HaveStdErrContaining($"Using install location '{installLocation}'.") .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index a5545272399391..20f31a66ef35bd 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -448,7 +448,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } char* pDelimiter = strchr(buffer, '='); - if (pDelimiter == nullptr) + if (pDelimiter == nullptr && len > 0) { if (is_first_line) { @@ -468,7 +468,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) pal::string_t arch_prefix = pal::string_t(buffer).substr(0, delimiter_index); pal::string_t path_to_location = pal::string_t(buffer).substr(delimiter_index + 1); - trace::verbose(_X("Found architecture-specific install logcation path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); + trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); if (arch_prefix == get_arch()) { architecture_install_location = path_to_location; From f1a2b7defae3809f4797ed7452dfed207b6f076b Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Wed, 2 Jun 2021 13:06:51 -0700 Subject: [PATCH 05/23] Set arch specific DOTNET_ROOT env var --- .../HostActivation.Tests/CommandExtensions.cs | 9 +++-- .../NativeHosting/ComhostSideBySide.cs | 4 +-- .../NativeHosting/Nethost.cs | 33 ++++++++++++------- .../PortableAppActivation.cs | 10 +++--- .../RegisteredInstallLocationOverride.cs | 2 +- .../StandaloneAppActivation.cs | 3 +- 6 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/CommandExtensions.cs b/src/installer/tests/HostActivation.Tests/CommandExtensions.cs index 054e6c39d14368..b1fadfbb314db3 100644 --- a/src/installer/tests/HostActivation.Tests/CommandExtensions.cs +++ b/src/installer/tests/HostActivation.Tests/CommandExtensions.cs @@ -35,11 +35,16 @@ public static Command EnableTracingAndCaptureOutputs(this Command command) .CaptureStdErr(); } - public static Command DotNetRoot(this Command command, string dotNetRoot) + public static Command DotNetRoot(this Command command, string dotNetRoot, string architecture = null) { - return command + command = command .EnvironmentVariable("DOTNET_ROOT", dotNetRoot) .EnvironmentVariable("DOTNET_ROOT(x86)", dotNetRoot); + if (!string.IsNullOrEmpty(architecture)) + command = command + .EnvironmentVariable($"DOTNET_ROOT_{architecture.ToUpper()}", dotNetRoot); + + return command; } public static Command MultilevelLookup(this Command command, bool enable) diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs index 623f93a8fdce5d..55ddfab6ba7049 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs @@ -33,7 +33,7 @@ public void ActivateClass() CommandResult result = Command.Create(sharedState.ComSxsPath, args) .EnableTracingAndCaptureOutputs() - .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath) + .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath, sharedState.RepoDirectories.BuildArchitecture) .MultilevelLookup(false) .Execute(); @@ -52,7 +52,7 @@ public void LocateEmbeddedTlb() CommandResult result = Command.Create(sharedState.ComSxsPath, args) .EnableTracingAndCaptureOutputs() - .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath) + .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath, sharedState.RepoDirectories.BuildArchitecture) .MultilevelLookup(false) .Execute(); diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index eed7c226eafc49..65504b9922db2e 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -35,7 +35,7 @@ public void GetHostFxrPath_DotNetRootEnvironment(bool explicitLoad, bool useAsse string dotNetRoot = isValid ? Path.Combine(sharedState.ValidInstallRoot, "dotnet") : sharedState.InvalidInstallRoot; CommandResult result = Command.Create(sharedState.NativeHostPath, $"{GetHostFxrPath} {explicitLoad} {(useAssemblyPath ? sharedState.TestAssemblyPath : string.Empty)}") .EnableTracingAndCaptureOutputs() - .DotNetRoot(dotNetRoot) + .DotNetRoot(dotNetRoot, sharedState.RepoDirectories.BuildArchitecture) .Execute(); result.Should().HaveStdErrContaining("Using environment variable"); @@ -179,7 +179,6 @@ public void GetHostFxrPath_HostFxrAlreadyLoaded() } [Theory] - [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] [InlineData("{0}", true)] [InlineData("{0}\n", true)] [InlineData("{0}\nSome other text", true)] @@ -190,11 +189,15 @@ public void GetHostFxrPath_HostFxrAlreadyLoaded() [InlineData("{0} ", false)] public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) { + // This test targets the install_location config file which is only used on Linux and macOS. + if (OperatingSystem.IsWindows()) + return; + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { string.Format(value, installLocation) }); + registeredInstallLocationOverride.SetInstallLocation(new string[] { string.Format(value, installLocation) }, sharedState.RepoDirectories.BuildArchitecture); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -224,14 +227,16 @@ public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void GetHostFxrPath_GlobalInstallation_HasMoreThanOneDefaultInstallationPath() { - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); + // This test targets the install_location config file which is only used on Linux and macOS. + if (OperatingSystem.IsWindows()) + return; + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { installLocation, installLocation }); + registeredInstallLocationOverride.SetInstallLocation(new string[] { installLocation, installLocation }, sharedState.RepoDirectories.BuildArchitecture); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -252,17 +257,19 @@ public void GetHostFxrPath_GlobalInstallation_HasMoreThanOneDefaultInstallationP } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() { - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); + // This test targets the install_location config file which is only used on Linux and macOS. + if (OperatingSystem.IsWindows()) + return; + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { registeredInstallLocationOverride.SetInstallLocation(new string[] { $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", $"someOtherArch={installLocation}/invalid" - }); + }, sharedState.RepoDirectories.BuildArchitecture); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -283,17 +290,19 @@ public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void GetHostFxrPath_GlobalInstallation_ArchitectureSpecificPathIsPickedOverDefaultPath() { - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); + // This test targets the install_location config file which is only used on Linux and macOS. + if (OperatingSystem.IsWindows()) + return; + string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { registeredInstallLocationOverride.SetInstallLocation(new string[] { $"{installLocation}/a/b/c", $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", - }); + }, sharedState.RepoDirectories.BuildArchitecture); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index a93e95e8e83d14..e1dfbfced589ce 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -257,7 +257,7 @@ public void AppHost_FrameworkDependent_Succeeds() Command.Create(appExe) .CaptureStdErr() .CaptureStdOut() - .DotNetRoot(builtDotnet) + .DotNetRoot(builtDotnet, sharedTestState.RepoDirectories.BuildArchitecture) .MultilevelLookup(false) .Execute() .Should().Pass() @@ -268,7 +268,7 @@ public void AppHost_FrameworkDependent_Succeeds() // Verify running from within the working directory Command.Create(appExe) .WorkingDirectory(fixture.TestProject.OutputDirectory) - .DotNetRoot(builtDotnet) + .DotNetRoot(builtDotnet, sharedTestState.RepoDirectories.BuildArchitecture) .MultilevelLookup(false) .CaptureStdErr() .CaptureStdOut() @@ -357,7 +357,7 @@ public void MissingRuntimeConfig_Fails(bool useAppHost) if (useAppHost) { command = Command.Create(sharedTestState.MockApp.AppExe) - .DotNetRoot(sharedTestState.BuiltDotNet.BinPath); + .DotNetRoot(sharedTestState.BuiltDotNet.BinPath, sharedTestState.RepoDirectories.BuildArchitecture); } else { @@ -386,7 +386,7 @@ public void MissingFrameworkInRuntimeConfig_Fails(bool useAppHost) if (useAppHost) { command = Command.Create(app.AppExe) - .DotNetRoot(sharedTestState.BuiltDotNet.BinPath); + .DotNetRoot(sharedTestState.BuiltDotNet.BinPath, sharedTestState.RepoDirectories.BuildArchitecture); } else { @@ -540,7 +540,7 @@ public void AppHost_GUI_NoCustomErrorWriter_FrameworkMissing_ErrorReportedInDial Command command = Command.Create(appExe) .EnableTracingAndCaptureOutputs() - .DotNetRoot(dotnet.BinPath) + .DotNetRoot(dotnet.BinPath, sharedTestState.RepoDirectories.BuildArchitecture) .MultilevelLookup(false) .Start(); diff --git a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs index 88c68d24d07090..284a19b71bdab8 100644 --- a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs +++ b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs @@ -65,7 +65,7 @@ public void SetInstallLocation(string[] installLocation, string architecture = " { if (OperatingSystem.IsWindows()) { - Debug.Assert(installLocation.Length == 1 && !string.IsNullOrEmpty(architecture)); + Debug.Assert(installLocation.Length >= 1 && !string.IsNullOrEmpty(architecture)); using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{architecture}")) { dotnetLocationKey.SetValue("InstallLocation", installLocation[0]); diff --git a/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs b/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs index b7ec6eb9688916..352abb04f0dc1b 100644 --- a/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs @@ -204,8 +204,7 @@ public void Running_Publish_Output_Standalone_EXE_With_DOTNET_ROOT_Fails() // self-contained layout since a flat layout of the shared framework is not supported. Command.Create(appExe) .EnvironmentVariable("COREHOST_TRACE", "1") - .EnvironmentVariable("DOTNET_ROOT", newOutDir) - .EnvironmentVariable("DOTNET_ROOT(x86)", newOutDir) + .DotNetRoot(newOutDir) .CaptureStdErr() .CaptureStdOut() .Execute(fExpectedToFail: true) From 7fe0238cec96e2d47215077442b9114634dcd74c Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Wed, 2 Jun 2021 20:42:18 -0700 Subject: [PATCH 06/23] Inline pal::to_lower/upper --- src/native/corehost/hostmisc/pal.h | 12 ++++++++++-- src/native/corehost/hostmisc/pal.unix.cpp | 14 -------------- src/native/corehost/hostmisc/pal.windows.cpp | 14 -------------- 3 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 83bc87af2ede25..73397b2f93a545 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -252,9 +252,17 @@ namespace pal string_t get_timestamp(); bool getcwd(string_t* recv); - string_t to_lower(const string_t& in); - string_t to_lower(const char_t* in); + inline string_t to_lower(const char_t* in) { + pal::string_t ret = in; + std::transform(ret.begin(), ret.end(), ret.begin(), ::tolower); + return ret; + } + inline string_t to_upper(const char_t* in) { + pal::string_t ret = in; + std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); + return ret; + } inline void file_flush(FILE *f) { std::fflush(f); } inline void err_flush() { std::fflush(stderr); } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 20f31a66ef35bd..102b2309201340 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -50,20 +50,6 @@ #error "Don't know how to obtain max path on this platform" #endif -pal::string_t pal::to_lower(const pal::char_t* in) -{ - pal::string_t ret = in; - std::transform(ret.begin(), ret.end(), ret.begin(), ::tolower); - return ret; -} - -pal::string_t pal::to_upper(const pal::string_t& in) -{ - pal::string_t ret = in; - std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); - return ret; -} - pal::string_t pal::get_timestamp() { std::time_t t = std::time(nullptr); diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 563f9a011243fd..24ae7c1a899d9e 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -40,20 +40,6 @@ bool GetModuleHandleFromAddress(void *addr, HMODULE *hModule) return (res != FALSE); } -pal::string_t pal::to_lower(const pal::char_t* in) -{ - pal::string_t ret = in; - std::transform(ret.begin(), ret.end(), ret.begin(), ::towlower); - return ret; -} - -pal::string_t pal::to_upper(const pal::string_t& in) -{ - pal::string_t ret = in; - std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); - return ret; -} - pal::string_t pal::get_timestamp() { std::time_t t = std::time(nullptr); From e40c6180d8479d114ab5d088b3e5deeb42702f56 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 4 Jun 2021 15:12:15 -0700 Subject: [PATCH 07/23] Update tests Move to_lower, to_upper to utils Add MultiArchInstallLocation tests --- .../MultiArchInstallLocation.cs | 126 ++++++++++++++++++ .../MultilevelSDKLookup.cs | 2 +- .../NativeHosting/Nethost.cs | 44 +++--- .../PortableAppActivation.cs | 2 +- .../RegisteredInstallLocationOverride.cs | 16 ++- .../tests/TestUtils/TestProjectFixture.cs | 4 +- src/native/corehost/deps_format.cpp | 2 +- src/native/corehost/fxr/command_line.cpp | 2 +- src/native/corehost/hostmisc/pal.h | 11 -- src/native/corehost/hostmisc/utils.cpp | 14 +- src/native/corehost/hostmisc/utils.h | 3 + .../corehost/test/nativehost/nativehost.cpp | 4 +- 12 files changed, 178 insertions(+), 52 deletions(-) create mode 100644 src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs new file mode 100644 index 00000000000000..5b66d23cbe80e7 --- /dev/null +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -0,0 +1,126 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Text; +using System.Threading.Tasks; +using Microsoft.DotNet.Cli.Build.Framework; +using Microsoft.DotNet.CoreSetup.Test; +using Microsoft.DotNet.CoreSetup.Test.HostActivation; +using Xunit; + +namespace HostActivation.Tests +{ + public class MultiArchInstallLocation : IClassFixture + { + private string InstallLocationFile => Path.Combine(sharedTestState.InstallLocation, "install_location"); + + private SharedTestState sharedTestState; + + public MultiArchInstallLocation(SharedTestState fixture) + { + sharedTestState = fixture; + } + + [Fact] + public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() + { + var fixture = sharedTestState.PortableAppFixture + .Copy(); + + var appExe = fixture.TestProject.AppExe; + var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); + Command.Create(appExe) + .EnvironmentVariable("COREHOST_TRACE", "1") + .EnvironmentVariable($"DOTNET_ROOT_{arch}", fixture.SdkDotnet.BinPath) + .CaptureStdErr() + .Execute() + .Should().HaveStdErrContaining($"DOTNET_ROOT_{arch}"); + } + + [Fact] + public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() + { + var fixture = sharedTestState.PortableAppFixture + .Copy(); + + var appExe = fixture.TestProject.AppExe; + Command.Create(appExe) + .EnableTracingAndCaptureOutputs() + .Execute() + .Should().HaveStdErrContaining($"DOTNET_ROOT"); + } + + [Fact] + public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() + { + var fixture = sharedTestState.PortableAppFixture + .Copy(); + + var appExe = fixture.TestProject.AppExe; + var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); + Command.Create(appExe) + .EnableTracingAndCaptureOutputs() + .EnvironmentVariable("DOTNET_ROOT", "non_existent_path") + .EnvironmentVariable($"DOTNET_ROOT_{arch}", fixture.SdkDotnet.BinPath) + .Execute() + .Should().HaveStdErrContaining($"DOTNET_ROOT_{arch}") + .And.NotHaveStdErrContaining("[DOTNET_ROOT] directory"); + } + + [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 + .Copy(); + + var dotnet = fixture.BuiltDotnet; + var appDll = fixture.TestProject.AppDll; + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(InstallLocationFile)) + { + registeredInstallLocationOverride.SetInstallLocation(("x/y/z", fixture.RepoDirProvider.BuildArchitecture)); + dotnet.Exec(appDll) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .EnvironmentVariable(Constants.TestOnlyEnvironmentVariables.DefaultInstallPath, InstallLocationFile) + .DotNetRoot(null) + .Execute() + .Should().Pass() + .And.HaveStdErrContaining(""); + } + } + + public class SharedTestState : IDisposable + { + public string BaseDirectory { get; } + public TestProjectFixture PortableAppFixture { get; } + public RepoDirectoriesProvider RepoDirectories { get; } + public string InstallLocation { get; } + + public SharedTestState() + { + RepoDirectories = new RepoDirectoriesProvider(); + var fixture = new TestProjectFixture("PortableApp", RepoDirectories); + fixture + .EnsureRestored() + .BuildProject(); + + PortableAppFixture = fixture; + BaseDirectory = Path.GetDirectoryName(PortableAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); + var install_location_dir = Path.Combine(fixture.TestProject.Location, "install_location"); + Directory.CreateDirectory(install_location_dir); + InstallLocation = install_location_dir; + } + + public void Dispose() + { + PortableAppFixture.Dispose(); + } + } + } +} diff --git a/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs b/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs index 39d8ca1a79c2e2..76d11337c03ab5 100644 --- a/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs +++ b/src/installer/tests/HostActivation.Tests/MultilevelSDKLookup.cs @@ -501,7 +501,7 @@ public void SdkMultilevelLookup_RegistryAccess() using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(DotNet.GreatestVersionHostFxrFilePath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { _regDir }, RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { (RepoDirectories.BuildArchitecture, _regDir) }); // Add SDK versions AddAvailableSdkVersions(_regSdkBaseDir, "9999.0.4"); diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index 65504b9922db2e..ea5b58f811817a 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -116,7 +116,7 @@ public void GetHostFxrPath_GlobalInstallation(bool explicitLoad, bool useAssembl { if (useRegisteredLocation) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { installLocation }, sharedState.RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation((sharedState.RepoDirectories.BuildArchitecture, installLocation)); } result = Command.Create(sharedState.NativeHostPath, $"{GetHostFxrPath} {explicitLoad} {(useAssemblyPath ? sharedState.TestAssemblyPath : string.Empty)}") @@ -179,6 +179,7 @@ public void GetHostFxrPath_HostFxrAlreadyLoaded() } [Theory] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] [InlineData("{0}", true)] [InlineData("{0}\n", true)] [InlineData("{0}\nSome other text", true)] @@ -189,15 +190,11 @@ public void GetHostFxrPath_HostFxrAlreadyLoaded() [InlineData("{0} ", false)] public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) { - // This test targets the install_location config file which is only used on Linux and macOS. - if (OperatingSystem.IsWindows()) - return; - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { string.Format(value, installLocation) }, sharedState.RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation((sharedState.RepoDirectories.BuildArchitecture, string.Format(value, installLocation))); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -227,16 +224,15 @@ public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) } [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void GetHostFxrPath_GlobalInstallation_HasMoreThanOneDefaultInstallationPath() { - // This test targets the install_location config file which is only used on Linux and macOS. - if (OperatingSystem.IsWindows()) - return; - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { installLocation, installLocation }, sharedState.RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { + (string.Empty, installLocation), (string.Empty, installLocation) + }); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -257,19 +253,16 @@ public void GetHostFxrPath_GlobalInstallation_HasMoreThanOneDefaultInstallationP } [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() { - // This test targets the install_location config file which is only used on Linux and macOS. - if (OperatingSystem.IsWindows()) - return; - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { - $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", - $"someOtherArch={installLocation}/invalid" - }, sharedState.RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { + (sharedState.RepoDirectories.BuildArchitecture, installLocation), + ("someOtherArch", $"{installLocation}/invalid") + }); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() @@ -290,19 +283,16 @@ public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() } [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void GetHostFxrPath_GlobalInstallation_ArchitectureSpecificPathIsPickedOverDefaultPath() { - // This test targets the install_location config file which is only used on Linux and macOS. - if (OperatingSystem.IsWindows()) - return; - string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { - $"{installLocation}/a/b/c", - $"{sharedState.RepoDirectories.BuildArchitecture.ToLower()}={installLocation}", - }, sharedState.RepoDirectories.BuildArchitecture); + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { + (string.Empty, $"{installLocation}/a/b/c"), + (sharedState.RepoDirectories.BuildArchitecture, installLocation) + }); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index e1dfbfced589ce..22fdb7acad8ae0 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -300,7 +300,7 @@ public void AppHost_FrameworkDependent_GlobalLocation_Succeeds(bool useRegistere string architecture = fixture.CurrentRid.Split('-')[1]; if (useRegisteredLocation) { - registeredInstallLocationOverride.SetInstallLocation(new string[] { builtDotnet }, architecture); + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { (architecture, builtDotnet) }); } // Verify running with the default working directory diff --git a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs index 284a19b71bdab8..d0ec4a9e010538 100644 --- a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs +++ b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs @@ -4,8 +4,10 @@ using Microsoft.DotNet.Cli.Build.Framework; using Microsoft.Win32; using System; +using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Linq; using System.Runtime.InteropServices; namespace Microsoft.DotNet.CoreSetup.Test.HostActivation @@ -61,19 +63,23 @@ public RegisteredInstallLocationOverride(string productBinaryPath) } } - public void SetInstallLocation(string[] installLocation, string architecture = "") + public void SetInstallLocation(params (string Architecture, string Path)[] locations) { + var locationsList = locations.ToList(); + Debug.Assert(locationsList.Count >= 1); if (OperatingSystem.IsWindows()) { - Debug.Assert(installLocation.Length >= 1 && !string.IsNullOrEmpty(architecture)); - using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{architecture}")) + // Only the first location is used on Windows + using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{locationsList[0].Architecture}")) { - dotnetLocationKey.SetValue("InstallLocation", installLocation[0]); + dotnetLocationKey.SetValue("InstallLocation", locationsList[0].Path); } } else { - File.WriteAllText(PathValueOverride, string.Join (Environment.NewLine, installLocation)); + File.WriteAllText(PathValueOverride, string.Join (Environment.NewLine, + locationsList.Select(l => string.Format("{0}{1}".ToLower(), + (!string.IsNullOrWhiteSpace(l.Architecture) ? l.Architecture + "=" : ""), l.Path)))); } } diff --git a/src/installer/tests/TestUtils/TestProjectFixture.cs b/src/installer/tests/TestUtils/TestProjectFixture.cs index 2ccce4dded1558..7b9da29d1a908e 100644 --- a/src/installer/tests/TestUtils/TestProjectFixture.cs +++ b/src/installer/tests/TestUtils/TestProjectFixture.cs @@ -358,7 +358,7 @@ public TestProjectFixture RestoreProject(string[] fallbackSources, string extraM public TestProjectFixture EnsureRestored(params string[] fallbackSources) { - if ( ! TestProject.IsRestored()) + if (!TestProject.IsRestored()) { RestoreProject(fallbackSources); } @@ -368,7 +368,7 @@ public TestProjectFixture EnsureRestored(params string[] fallbackSources) public TestProjectFixture EnsureRestoredForRid(string rid, params string[] fallbackSources) { - if ( ! TestProject.IsRestored()) + if (!TestProject.IsRestored()) { string extraMSBuildProperties = $"/p:TestTargetRid={rid}"; RestoreProject(fallbackSources, extraMSBuildProperties); diff --git a/src/native/corehost/deps_format.cpp b/src/native/corehost/deps_format.cpp index f0beb6ed7e54ed..8ab0f22caff2cb 100644 --- a/src/native/corehost/deps_format.cpp +++ b/src/native/corehost/deps_format.cpp @@ -84,7 +84,7 @@ void deps_json_t::reconcile_libraries_with_targets( size_t pos = lib_name.find(_X("/")); entry.library_name = lib_name.substr(0, pos); entry.library_version = lib_name.substr(pos + 1); - entry.library_type = pal::to_lower(library.value[_X("type")].GetString()); + entry.library_type = to_lower(library.value[_X("type")].GetString()); entry.library_hash = hash; entry.library_path = library_path; entry.library_hash_path = library_hash_path; diff --git a/src/native/corehost/fxr/command_line.cpp b/src/native/corehost/fxr/command_line.cpp index 082f81d802c128..deb80b38b7f522 100644 --- a/src/native/corehost/fxr/command_line.cpp +++ b/src/native/corehost/fxr/command_line.cpp @@ -86,7 +86,7 @@ namespace while (arg_i < argc) { const pal::char_t* arg = argv[arg_i]; - pal::string_t arg_lower = pal::to_lower(arg); + pal::string_t arg_lower = to_lower(arg); const auto &iter = std::find_if(known_opts.cbegin(), known_opts.cend(), [&](const known_options &opt) { return arg_lower == get_host_option(opt).option; }); if (iter == known_opts.cend()) diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 73397b2f93a545..95b4fcf84caa4e 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -252,17 +252,6 @@ namespace pal string_t get_timestamp(); bool getcwd(string_t* recv); - inline string_t to_lower(const char_t* in) { - pal::string_t ret = in; - std::transform(ret.begin(), ret.end(), ret.begin(), ::tolower); - return ret; - } - - inline string_t to_upper(const char_t* in) { - pal::string_t ret = in; - std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); - return ret; - } inline void file_flush(FILE *f) { std::fflush(f); } inline void err_flush() { std::fflush(stderr); } diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index fdb9e343badfe6..49093b97d72cca 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -351,7 +351,7 @@ bool try_stou(const pal::string_t& str, unsigned* num) pal::string_t get_dotnet_root_env_var_name() { pal::string_t dotnet_root_arch = _X("DOTNET_ROOT_"), path; - dotnet_root_arch.append(pal::to_upper(get_arch())); + dotnet_root_arch.append(to_upper(get_arch())); if (pal::getenv(dotnet_root_arch.c_str(), &path)) return dotnet_root_arch; @@ -450,6 +450,18 @@ pal::string_t get_download_url(const pal::char_t *framework_name, const pal::cha return url; } +pal::string_t to_lower(const pal::char_t* in) { + pal::string_t ret = in; + std::transform(ret.begin(), ret.end(), ret.begin(), ::tolower); + return ret; +} + +pal::string_t to_upper(const pal::char_t* in) { + pal::string_t ret = in; + std::transform(ret.begin(), ret.end(), ret.begin(), ::toupper); + return ret; +} + #define TEST_ONLY_MARKER "d38cc827-e34f-4453-9df4-1e796e9f1d07" // Retrieves environment variable which is only used for testing. diff --git a/src/native/corehost/hostmisc/utils.h b/src/native/corehost/hostmisc/utils.h index d0dc381b69cca1..622dfc36bcd839 100644 --- a/src/native/corehost/hostmisc/utils.h +++ b/src/native/corehost/hostmisc/utils.h @@ -54,6 +54,9 @@ pal::string_t get_dotnet_root_from_fxr_path(const pal::string_t &fxr_path); // If no framework is specified, a download URL for the runtime is returned pal::string_t get_download_url(const pal::char_t *framework_name = nullptr, const pal::char_t *framework_version = nullptr); +pal::string_t to_lower(const pal::char_t* in); +pal::string_t to_upper(const pal::char_t* in); + // Retrieves environment variable which is only used for testing. // This will return the value of the variable only if the product binary is stamped // with test-only marker. diff --git a/src/native/corehost/test/nativehost/nativehost.cpp b/src/native/corehost/test/nativehost/nativehost.cpp index b323f95a82949a..80280b7bdbbeb9 100644 --- a/src/native/corehost/test/nativehost/nativehost.cpp +++ b/src/native/corehost/test/nativehost/nativehost.cpp @@ -40,7 +40,7 @@ int main(const int argc, const pal::char_t *argv[]) // args: ... [] [] [] [] bool explicit_load = false; if (argc >= 3) - explicit_load = pal::strcmp(pal::to_lower(argv[2]).c_str(), _X("true")) == 0; + explicit_load = pal::strcmp(to_lower(argv[2]).c_str(), _X("true")) == 0; const pal::char_t *assembly_path = nullptr; if (argc >= 4 && pal::strcmp(argv[3], _X("nullptr")) != 0) @@ -117,7 +117,7 @@ int main(const int argc, const pal::char_t *argv[]) if (static_cast(res) == StatusCode::Success) { std::cout << "get_hostfxr_path succeeded" << std::endl; - std::cout << "hostfxr_path: " << tostr(pal::to_lower(fxr_path.c_str())).data() << std::endl; + std::cout << "hostfxr_path: " << tostr(to_lower(fxr_path.c_str())).data() << std::endl; return EXIT_SUCCESS; } else From 98814eab7a6f4bc555a33b187ed45ed87c5fd074 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 4 Jun 2021 22:40:05 -0700 Subject: [PATCH 08/23] Update tests --- .../MultiArchInstallLocation.cs | 96 +++++++++++++------ 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 5b66d23cbe80e7..a1139be63e744c 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -17,8 +17,6 @@ namespace HostActivation.Tests { public class MultiArchInstallLocation : IClassFixture { - private string InstallLocationFile => Path.Combine(sharedTestState.InstallLocation, "install_location"); - private SharedTestState sharedTestState; public MultiArchInstallLocation(SharedTestState fixture) @@ -29,38 +27,46 @@ public MultiArchInstallLocation(SharedTestState fixture) [Fact] public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() { - var fixture = sharedTestState.PortableAppFixture + var fixture = sharedTestState.StandaloneAppFixture .Copy(); + // Removing hostfxr from the app's folder will force us to get DOTNET_ROOT env. + File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); Command.Create(appExe) - .EnvironmentVariable("COREHOST_TRACE", "1") + .EnableTracingAndCaptureOutputs() .EnvironmentVariable($"DOTNET_ROOT_{arch}", fixture.SdkDotnet.BinPath) - .CaptureStdErr() + .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) .Execute() - .Should().HaveStdErrContaining($"DOTNET_ROOT_{arch}"); + .Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT_{arch}"); } [Fact] public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() { - var fixture = sharedTestState.PortableAppFixture + var fixture = sharedTestState.StandaloneAppFixture .Copy(); + // Removing hostfxr from the app's folder will force us to get DOTNET_ROOT env. + File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; + var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); Command.Create(appExe) .EnableTracingAndCaptureOutputs() + .DotNetRoot(fixture.BuiltDotnet.BinPath) .Execute() - .Should().HaveStdErrContaining($"DOTNET_ROOT"); + .Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT"); } [Fact] public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() { - var fixture = sharedTestState.PortableAppFixture + var fixture = sharedTestState.StandaloneAppFixture .Copy(); + // Removing hostfxr from the app's folder will force us to get DOTNET_ROOT env. + File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); Command.Create(appExe) @@ -69,57 +75,93 @@ public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() .EnvironmentVariable($"DOTNET_ROOT_{arch}", fixture.SdkDotnet.BinPath) .Execute() .Should().HaveStdErrContaining($"DOTNET_ROOT_{arch}") - .And.NotHaveStdErrContaining("[DOTNET_ROOT] directory"); + .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); } [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 + var fixture = sharedTestState.StandaloneAppFixture .Copy(); - var dotnet = fixture.BuiltDotnet; - var appDll = fixture.TestProject.AppDll; - using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(InstallLocationFile)) + File.Delete(fixture.TestProject.HostFxrDll); + var appExe = fixture.TestProject.AppExe; + + var arch1 = fixture.RepoDirProvider.BuildArchitecture; + var path1 = "a/b/c"; + var arch2 = "someArch"; + var path2 = "x/y/z"; + + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) + { + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { + (string.Empty, path1), + (arch1, path1), + (arch2, path2) + }); + Command.Create(appExe) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .DotNetRoot(null) + .Execute() + .Should().HaveStdErrContaining($"Found install location path '{path1}'.") + .And.HaveStdErrContaining($"Found architecture-specific install location path: '{path1}' ('{arch1}').") + .And.HaveStdErrContaining($"Found architecture-specific install location path matching the current OS architecture ('{arch1}'): '{path1}'.") + .And.HaveStdErrContaining($"Found architecture-specific install location path: '{path2}' ('{arch2}').") + .And.HaveStdErrContaining($"Using global installation location [{path1}] as runtime location."); + } + } + + [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] + public void InstallLocationFile_OnlyFirstLineMayNotSpecifyArchitecture() + { + var fixture = sharedTestState.StandaloneAppFixture + .Copy(); + + File.Delete(fixture.TestProject.HostFxrDll); + var appExe = fixture.TestProject.AppExe; + + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) { - registeredInstallLocationOverride.SetInstallLocation(("x/y/z", fixture.RepoDirProvider.BuildArchitecture)); - dotnet.Exec(appDll) + registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { + (string.Empty, "a/b/c"), + (string.Empty, "x/y/z"), + }); + Command.Create(appExe) .EnableTracingAndCaptureOutputs() .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) - .EnvironmentVariable(Constants.TestOnlyEnvironmentVariables.DefaultInstallPath, InstallLocationFile) .DotNetRoot(null) .Execute() - .Should().Pass() - .And.HaveStdErrContaining(""); + .Should().HaveStdErrContaining($"Found install location path 'a/b/c'.") + .And.HaveStdErrContaining($"Only the first line in '{registeredInstallLocationOverride.PathValueOverride}' may not have an architecture prefix.") + .And.HaveStdErrContaining($"Using install location 'a/b/c'."); } } public class SharedTestState : IDisposable { public string BaseDirectory { get; } - public TestProjectFixture PortableAppFixture { get; } + public TestProjectFixture StandaloneAppFixture { get; } public RepoDirectoriesProvider RepoDirectories { get; } public string InstallLocation { get; } public SharedTestState() { RepoDirectories = new RepoDirectoriesProvider(); - var fixture = new TestProjectFixture("PortableApp", RepoDirectories); + var fixture = new TestProjectFixture("StandaloneApp", RepoDirectories); fixture .EnsureRestored() .BuildProject(); - PortableAppFixture = fixture; - BaseDirectory = Path.GetDirectoryName(PortableAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); - var install_location_dir = Path.Combine(fixture.TestProject.Location, "install_location"); - Directory.CreateDirectory(install_location_dir); - InstallLocation = install_location_dir; + StandaloneAppFixture = fixture; + BaseDirectory = Path.GetDirectoryName(StandaloneAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); } public void Dispose() { - PortableAppFixture.Dispose(); + // StandaloneAppFixture.Dispose(); } } } From 3a40dfd62566488f975ef7a79935f08fbc19483c Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 4 Jun 2021 22:42:56 -0700 Subject: [PATCH 09/23] Sort usings --- .../tests/HostActivation.Tests/MultiArchInstallLocation.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index a1139be63e744c..c6bef19b534e80 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -2,12 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.IO; -using System.Linq; -using System.Runtime.InteropServices; -using System.Text; -using System.Threading.Tasks; using Microsoft.DotNet.Cli.Build.Framework; using Microsoft.DotNet.CoreSetup.Test; using Microsoft.DotNet.CoreSetup.Test.HostActivation; @@ -15,6 +10,7 @@ namespace HostActivation.Tests { + public class MultiArchInstallLocation : IClassFixture { private SharedTestState sharedTestState; From e1d55c99c5c0d6f26d0f34af934a47102fe4ff10 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 7 Jun 2021 13:03:47 -0700 Subject: [PATCH 10/23] PR Feedback --- .../HostActivation.Tests/CommandExtensions.cs | 11 +++- .../MultiArchInstallLocation.cs | 60 +++++++++++++------ .../NativeHosting/ComhostSideBySide.cs | 4 +- .../NativeHosting/Nethost.cs | 2 +- .../RegisteredInstallLocationOverride.cs | 8 +-- src/native/corehost/fxr_resolver.cpp | 4 +- src/native/corehost/hostmisc/pal.unix.cpp | 4 +- src/native/corehost/hostmisc/utils.cpp | 16 ++--- src/native/corehost/hostmisc/utils.h | 2 +- 9 files changed, 70 insertions(+), 41 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/CommandExtensions.cs b/src/installer/tests/HostActivation.Tests/CommandExtensions.cs index b1fadfbb314db3..3bfa97e4e4d7b9 100644 --- a/src/installer/tests/HostActivation.Tests/CommandExtensions.cs +++ b/src/installer/tests/HostActivation.Tests/CommandExtensions.cs @@ -37,13 +37,18 @@ public static Command EnableTracingAndCaptureOutputs(this Command command) public static Command DotNetRoot(this Command command, string dotNetRoot, string architecture = null) { - command = command - .EnvironmentVariable("DOTNET_ROOT", dotNetRoot) - .EnvironmentVariable("DOTNET_ROOT(x86)", dotNetRoot); if (!string.IsNullOrEmpty(architecture)) + { command = command .EnvironmentVariable($"DOTNET_ROOT_{architecture.ToUpper()}", dotNetRoot); + return command; + } + + command = command + .EnvironmentVariable("DOTNET_ROOT", dotNetRoot) + .EnvironmentVariable("DOTNET_ROOT(x86)", dotNetRoot); + return command; } diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index c6bef19b534e80..dda8b064c98677 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Runtime.InteropServices; using Microsoft.DotNet.Cli.Build.Framework; using Microsoft.DotNet.CoreSetup.Test; using Microsoft.DotNet.CoreSetup.Test.HostActivation; @@ -26,16 +27,14 @@ public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() var fixture = sharedTestState.StandaloneAppFixture .Copy(); - // Removing hostfxr from the app's folder will force us to get DOTNET_ROOT env. - File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); Command.Create(appExe) .EnableTracingAndCaptureOutputs() - .EnvironmentVariable($"DOTNET_ROOT_{arch}", fixture.SdkDotnet.BinPath) .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) .Execute() - .Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT_{arch}"); + .Should().Pass() + .And.HaveStdErrContaining($"Using environment variable DOTNET_ROOT_{arch}"); } [Fact] @@ -44,15 +43,14 @@ public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() var fixture = sharedTestState.StandaloneAppFixture .Copy(); - // Removing hostfxr from the app's folder will force us to get DOTNET_ROOT env. - File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); Command.Create(appExe) .EnableTracingAndCaptureOutputs() .DotNetRoot(fixture.BuiltDotnet.BinPath) .Execute() - .Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT"); + .Should().Pass() + .And.HaveStdErrContaining($"Using environment variable DOTNET_ROOT="); } [Fact] @@ -61,19 +59,49 @@ public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() var fixture = sharedTestState.StandaloneAppFixture .Copy(); - // Removing hostfxr from the app's folder will force us to get DOTNET_ROOT env. - File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); Command.Create(appExe) .EnableTracingAndCaptureOutputs() - .EnvironmentVariable("DOTNET_ROOT", "non_existent_path") - .EnvironmentVariable($"DOTNET_ROOT_{arch}", fixture.SdkDotnet.BinPath) + .DotNetRoot("non_existent_path") + .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) .Execute() - .Should().HaveStdErrContaining($"DOTNET_ROOT_{arch}") + .Should().Pass() + .And.HaveStdErrContaining($"DOTNET_ROOT_{arch}") .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); } + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(false)] + [InlineData(true)] + public void GlobalInstallation_WindowsX86(bool setArchSpecificDotnetRoot) + { + var fixture = sharedTestState.StandaloneAppFixture + .Copy(); + + if (RuntimeInformation.OSArchitecture != Architecture.X86) + return; + + var appExe = fixture.TestProject.AppExe; + var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); + var dotnet = fixture.BuiltDotnet.BinPath; + var command = Command.Create(appExe) + .EnableTracingAndCaptureOutputs() + .DotNetRoot(dotnet); + + if (setArchSpecificDotnetRoot) + command.DotNetRoot(dotnet, arch); + + var result = command.Execute(); + result.Should().Pass(); + + if (setArchSpecificDotnetRoot) + result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT_X86=[{dotnet}] as runtime location."); + else + result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT(x86)=[{dotnet}] as runtime location."); + } + [Fact] [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() @@ -81,9 +109,7 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() var fixture = sharedTestState.StandaloneAppFixture .Copy(); - File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; - var arch1 = fixture.RepoDirProvider.BuildArchitecture; var path1 = "a/b/c"; var arch2 = "someArch"; @@ -116,9 +142,7 @@ public void InstallLocationFile_OnlyFirstLineMayNotSpecifyArchitecture() var fixture = sharedTestState.StandaloneAppFixture .Copy(); - File.Delete(fixture.TestProject.HostFxrDll); var appExe = fixture.TestProject.AppExe; - using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) { registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { @@ -148,8 +172,8 @@ public SharedTestState() RepoDirectories = new RepoDirectoriesProvider(); var fixture = new TestProjectFixture("StandaloneApp", RepoDirectories); fixture - .EnsureRestored() - .BuildProject(); + .EnsureRestoredForRid(fixture.CurrentRid) + .PublishProject(runtime: fixture.CurrentRid, selfContained: false); StandaloneAppFixture = fixture; BaseDirectory = Path.GetDirectoryName(StandaloneAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs index 55ddfab6ba7049..623f93a8fdce5d 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs @@ -33,7 +33,7 @@ public void ActivateClass() CommandResult result = Command.Create(sharedState.ComSxsPath, args) .EnableTracingAndCaptureOutputs() - .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath, sharedState.RepoDirectories.BuildArchitecture) + .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath) .MultilevelLookup(false) .Execute(); @@ -52,7 +52,7 @@ public void LocateEmbeddedTlb() CommandResult result = Command.Create(sharedState.ComSxsPath, args) .EnableTracingAndCaptureOutputs() - .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath, sharedState.RepoDirectories.BuildArchitecture) + .DotNetRoot(sharedState.ComLibraryFixture.BuiltDotnet.BinPath) .MultilevelLookup(false) .Execute(); diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index ea5b58f811817a..f72fa7210c0ed7 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -35,7 +35,7 @@ public void GetHostFxrPath_DotNetRootEnvironment(bool explicitLoad, bool useAsse string dotNetRoot = isValid ? Path.Combine(sharedState.ValidInstallRoot, "dotnet") : sharedState.InvalidInstallRoot; CommandResult result = Command.Create(sharedState.NativeHostPath, $"{GetHostFxrPath} {explicitLoad} {(useAssemblyPath ? sharedState.TestAssemblyPath : string.Empty)}") .EnableTracingAndCaptureOutputs() - .DotNetRoot(dotNetRoot, sharedState.RepoDirectories.BuildArchitecture) + .DotNetRoot(dotNetRoot) .Execute(); result.Should().HaveStdErrContaining("Using environment variable"); diff --git a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs index d0ec4a9e010538..f1b13b8f4aef59 100644 --- a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs +++ b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs @@ -69,11 +69,9 @@ public void SetInstallLocation(params (string Architecture, string Path)[] locat Debug.Assert(locationsList.Count >= 1); if (OperatingSystem.IsWindows()) { - // Only the first location is used on Windows - using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{locationsList[0].Architecture}")) - { - dotnetLocationKey.SetValue("InstallLocation", locationsList[0].Path); - } + foreach (var location in locationsList) + using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{location.Architecture}")) + dotnetLocationKey.SetValue("InstallLocation", location.Path); } else { diff --git a/src/native/corehost/fxr_resolver.cpp b/src/native/corehost/fxr_resolver.cpp index e4e9c5fc84a096..fbd045f0084257 100644 --- a/src/native/corehost/fxr_resolver.cpp +++ b/src/native/corehost/fxr_resolver.cpp @@ -67,8 +67,8 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o // For framework-dependent apps, use DOTNET_ROOT_ pal::string_t default_install_location; - pal::string_t dotnet_root_env_var_name = get_dotnet_root_env_var_name(); - if (get_file_path_from_env(dotnet_root_env_var_name.c_str(), out_dotnet_root)) + pal::string_t dotnet_root_env_var_name; + if (get_dotnet_root_from_env(&dotnet_root_env_var_name, out_dotnet_root)) { trace::info(_X("Using environment variable %s=[%s] as runtime location."), dotnet_root_env_var_name.c_str(), out_dotnet_root->c_str()); } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 102b2309201340..17978536af614c 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -417,7 +417,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) FILE* install_location_file = pal::file_open(install_location_file_path, "r"); if (install_location_file == nullptr) { - trace::error(_X("The install_location file failed to open.")); + trace::error(_X("The install_location file '%s' failed to open with error %d.", install_location_file.c_str(), errno)); return false; } @@ -472,7 +472,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } else { - trace::error(_X("Did not find any install location in '%s'"), install_location_file_path.c_str()); + trace::error(_X("Did not find any install location in '%s'."), install_location_file_path.c_str()); return false; } diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index 49093b97d72cca..c00632233f56ba 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -348,23 +348,25 @@ bool try_stou(const pal::string_t& str, unsigned* num) return true; } -pal::string_t get_dotnet_root_env_var_name() +bool get_dotnet_root_from_env(pal::string_t* dotnet_root_env_var_name, pal::string_t* recv) { - pal::string_t dotnet_root_arch = _X("DOTNET_ROOT_"), path; - dotnet_root_arch.append(to_upper(get_arch())); - if (pal::getenv(dotnet_root_arch.c_str(), &path)) - return dotnet_root_arch; + *dotnet_root_env_var_name = _X("DOTNET_ROOT_"); + dotnet_root_env_var_name->append(to_upper(get_arch())); + if (get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv)) + return true; #if defined(WIN32) if (pal::is_running_in_wow64()) { - return pal::string_t(_X("DOTNET_ROOT(x86)")); + *dotnet_root_env_var_name = _X("DOTNET_ROOT(x86)"); + return get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv); } #endif // If no architecture-specific environment variable was set // fallback to the default DOTNET_ROOT. - return pal::string_t(_X("DOTNET_ROOT")); + *dotnet_root_env_var_name = _X("DOTNET_ROOT"); + return get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv); } /** diff --git a/src/native/corehost/hostmisc/utils.h b/src/native/corehost/hostmisc/utils.h index 622dfc36bcd839..513459ee75aa7d 100644 --- a/src/native/corehost/hostmisc/utils.h +++ b/src/native/corehost/hostmisc/utils.h @@ -43,7 +43,7 @@ void get_framework_and_sdk_locations(const pal::string_t& dotnet_dir, std::vecto bool get_file_path_from_env(const pal::char_t* env_key, pal::string_t* recv); size_t index_of_non_numeric(const pal::string_t& str, size_t i); bool try_stou(const pal::string_t& str, unsigned* num); -pal::string_t get_dotnet_root_env_var_name(); +bool get_dotnet_root_from_env(pal::string_t* used_dotnet_root_env_var_name, pal::string_t* recv); pal::string_t get_deps_from_app_binary(const pal::string_t& app_base, const pal::string_t& app); pal::string_t get_runtime_config_path(const pal::string_t& path, const pal::string_t& name); pal::string_t get_runtime_config_dev_path(const pal::string_t& path, const pal::string_t& name); From 9602d928f23ff37c8693c412f21e981cdc6c12dd Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 7 Jun 2021 13:13:01 -0700 Subject: [PATCH 11/23] Fix trace --- .../tests/HostActivation.Tests/MultiArchInstallLocation.cs | 2 +- src/native/corehost/hostmisc/pal.unix.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index dda8b064c98677..e670a9926d6809 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -181,7 +181,7 @@ public SharedTestState() public void Dispose() { - // StandaloneAppFixture.Dispose(); + StandaloneAppFixture.Dispose(); } } } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 17978536af614c..a76d99b2389011 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -417,7 +417,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) FILE* install_location_file = pal::file_open(install_location_file_path, "r"); if (install_location_file == nullptr) { - trace::error(_X("The install_location file '%s' failed to open with error %d.", install_location_file.c_str(), errno)); + trace::error(_X("The install_location file '%s' failed to open with error %d."), install_location_file_path.c_str(), errno); return false; } From c1b7af2fcf5f2d5aae4cdfc0317afa0d836c1821 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 7 Jun 2021 13:44:02 -0700 Subject: [PATCH 12/23] Fail if DOTNET_ROOT_ARCH is defined but path doesn't exist Modify trace error message --- src/native/corehost/hostmisc/pal.unix.cpp | 2 +- src/native/corehost/hostmisc/utils.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index a76d99b2389011..7b09fdbfec3804 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -417,7 +417,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) FILE* install_location_file = pal::file_open(install_location_file_path, "r"); if (install_location_file == nullptr) { - trace::error(_X("The install_location file '%s' failed to open with error %d."), install_location_file_path.c_str(), errno); + trace::error(_X("The install_location file ['%s'] failed to open: %s."), install_location_file_path.c_str(), pal::strerror(errno)); return false; } diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index c00632233f56ba..cbc0aef651bc8c 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -352,8 +352,8 @@ bool get_dotnet_root_from_env(pal::string_t* dotnet_root_env_var_name, pal::stri { *dotnet_root_env_var_name = _X("DOTNET_ROOT_"); dotnet_root_env_var_name->append(to_upper(get_arch())); - if (get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv)) - return true; + if (pal::getenv(dotnet_root_env_var_name->c_str(), recv)) + return pal::file_exists(*recv); #if defined(WIN32) if (pal::is_running_in_wow64()) From 104357c27ba41bd4b9eacf1569c86e1ea64a362d Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 7 Jun 2021 17:26:36 -0700 Subject: [PATCH 13/23] PR feedback --- .../MultiArchInstallLocation.cs | 31 ++++++++++--------- .../NativeHosting/Nethost.cs | 1 - src/native/corehost/hostmisc/pal.unix.cpp | 27 +++++++--------- src/native/corehost/hostmisc/utils.cpp | 16 ++++++++++ src/native/corehost/hostmisc/utils.h | 1 + 5 files changed, 44 insertions(+), 32 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index e670a9926d6809..ea37cf318816ab 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -24,7 +24,7 @@ public MultiArchInstallLocation(SharedTestState fixture) [Fact] public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() { - var fixture = sharedTestState.StandaloneAppFixture + var fixture = sharedTestState.PortableAppFixture .Copy(); var appExe = fixture.TestProject.AppExe; @@ -40,7 +40,7 @@ public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() [Fact] public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() { - var fixture = sharedTestState.StandaloneAppFixture + var fixture = sharedTestState.PortableAppFixture .Copy(); var appExe = fixture.TestProject.AppExe; @@ -56,18 +56,19 @@ public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() [Fact] public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() { - var fixture = sharedTestState.StandaloneAppFixture + var fixture = sharedTestState.PortableAppFixture .Copy(); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); + var dotnet = fixture.BuiltDotnet.BinPath; Command.Create(appExe) .EnableTracingAndCaptureOutputs() .DotNetRoot("non_existent_path") - .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) + .DotNetRoot(dotnet, arch) .Execute() .Should().Pass() - .And.HaveStdErrContaining($"DOTNET_ROOT_{arch}") + .And.HaveStdErrContaining($"Using environment variable DOTNET_ROOT_{arch}=[{dotnet}] as runtime location.") .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); } @@ -77,7 +78,7 @@ public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() [InlineData(true)] public void GlobalInstallation_WindowsX86(bool setArchSpecificDotnetRoot) { - var fixture = sharedTestState.StandaloneAppFixture + var fixture = sharedTestState.PortableAppFixture .Copy(); if (RuntimeInformation.OSArchitecture != Architecture.X86) @@ -106,7 +107,7 @@ public void GlobalInstallation_WindowsX86(bool setArchSpecificDotnetRoot) [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.StandaloneAppFixture + var fixture = sharedTestState.PortableAppFixture .Copy(); var appExe = fixture.TestProject.AppExe; @@ -139,7 +140,7 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void InstallLocationFile_OnlyFirstLineMayNotSpecifyArchitecture() { - var fixture = sharedTestState.StandaloneAppFixture + var fixture = sharedTestState.PortableAppFixture .Copy(); var appExe = fixture.TestProject.AppExe; @@ -163,25 +164,25 @@ public void InstallLocationFile_OnlyFirstLineMayNotSpecifyArchitecture() public class SharedTestState : IDisposable { public string BaseDirectory { get; } - public TestProjectFixture StandaloneAppFixture { get; } + public TestProjectFixture PortableAppFixture { get; } public RepoDirectoriesProvider RepoDirectories { get; } public string InstallLocation { get; } public SharedTestState() { RepoDirectories = new RepoDirectoriesProvider(); - var fixture = new TestProjectFixture("StandaloneApp", RepoDirectories); + var fixture = new TestProjectFixture("PortableApp", RepoDirectories); fixture - .EnsureRestoredForRid(fixture.CurrentRid) - .PublishProject(runtime: fixture.CurrentRid, selfContained: false); + .EnsureRestored() + .PublishProject(); - StandaloneAppFixture = fixture; - BaseDirectory = Path.GetDirectoryName(StandaloneAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); + PortableAppFixture = fixture; + BaseDirectory = Path.GetDirectoryName(PortableAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); } public void Dispose() { - StandaloneAppFixture.Dispose(); + PortableAppFixture.Dispose(); } } } diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index f72fa7210c0ed7..ef2a50d2ea1720 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -276,7 +276,6 @@ public void GetHostFxrPath_GlobalInstallation_HasNoDefaultInstallationPath() result.Should().Pass() .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.") .And.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}' ('{sharedState.RepoDirectories.BuildArchitecture.ToLower()}').") - .And.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}/invalid' ('someOtherArch').") .And.HaveStdErrContaining($"Using install location '{installLocation}'.") .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower()); } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 7b09fdbfec3804..15386b1e9e8456 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -421,24 +421,17 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) return false; } - char buffer[PATH_MAX]; - pal::string_t install_location, architecture_install_location; + pal::string_t install_location, architecture_install_location, file_line; bool is_first_line = true; - while (fgets(buffer, sizeof(buffer), install_location_file)) - { - size_t len = strlen(buffer); - // fgets includes the newline character in the string - so remove it. - if (len > 0 && len < PATH_MAX && buffer[len - 1] == '\n') - { - buffer[len - 1] = '\0'; - } - char* pDelimiter = strchr(buffer, '='); - if (pDelimiter == nullptr && len > 0) + while (get_line_from_file(install_location_file, file_line)) + { + size_t arch_sep = file_line.find(_X('=')); + if (arch_sep == pal::string_t::npos) { if (is_first_line) { - install_location = pal::string_t(buffer); + install_location = file_line; trace::verbose(_X("Found install location path '%s'."), install_location.c_str()); } else @@ -450,16 +443,18 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) continue; } - auto delimiter_index = (int)(strlen(buffer) - strlen(pDelimiter)); - pal::string_t arch_prefix = pal::string_t(buffer).substr(0, delimiter_index); - pal::string_t path_to_location = pal::string_t(buffer).substr(delimiter_index + 1); + pal::string_t arch_prefix = file_line.substr(0, arch_sep); + pal::string_t path_to_location = file_line.substr(arch_sep + 1); trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); if (arch_prefix == get_arch()) { architecture_install_location = path_to_location; trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'."), arch_prefix.c_str(), architecture_install_location.c_str()); + break; } + + is_first_line = false; } if (architecture_install_location != "") diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index cbc0aef651bc8c..dd17e76c4dadb5 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -98,6 +98,22 @@ pal::string_t strip_file_ext(const pal::string_t& path) return path.substr(0, dot_pos); } +bool get_line_from_file(FILE* pFile, pal::string_t& line) +{ + std::vector buffer; + char last_char; + while ((last_char = fgetc(pFile))) + { + if (last_char == '\n' || last_char == EOF) + break; + + buffer.push_back((pal::char_t)last_char); + } + + line = pal::string_t(buffer.begin(), buffer.end()); + return buffer.size() != 0; +} + pal::string_t get_filename_without_ext(const pal::string_t& path) { if (path.empty()) diff --git a/src/native/corehost/hostmisc/utils.h b/src/native/corehost/hostmisc/utils.h index 513459ee75aa7d..c4f46d45b24fb1 100644 --- a/src/native/corehost/hostmisc/utils.h +++ b/src/native/corehost/hostmisc/utils.h @@ -41,6 +41,7 @@ bool get_global_shared_store_dirs(std::vector* dirs, const pal::s bool multilevel_lookup_enabled(); void get_framework_and_sdk_locations(const pal::string_t& dotnet_dir, std::vector* locations); bool get_file_path_from_env(const pal::char_t* env_key, pal::string_t* recv); +bool get_line_from_file(FILE* pFile, pal::string_t& line); size_t index_of_non_numeric(const pal::string_t& str, size_t i); bool try_stou(const pal::string_t& str, unsigned* num); bool get_dotnet_root_from_env(pal::string_t* used_dotnet_root_env_var_name, pal::string_t* recv); From 3280fffd1cd6c36020cfc382e2feffa874275b8a Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Tue, 8 Jun 2021 12:43:38 -0700 Subject: [PATCH 14/23] Fallback to DOTNET_ROOT on win32 Warn instead of error Change from vector to fgets --- .../MultiArchInstallLocation.cs | 13 +++---- src/native/corehost/hostmisc/pal.unix.cpp | 36 ++++++++----------- src/native/corehost/hostmisc/utils.cpp | 27 ++++++++------ 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index ea37cf318816ab..443e44aca81988 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -111,10 +111,10 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() .Copy(); var appExe = fixture.TestProject.AppExe; - var arch1 = fixture.RepoDirProvider.BuildArchitecture; - var path1 = "a/b/c"; - var arch2 = "someArch"; - var path2 = "x/y/z"; + var arch1 = "someArch"; + var path1 = "x/y/z"; + var arch2 = fixture.RepoDirProvider.BuildArchitecture; + var path2 = "a/b/c"; using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) { @@ -123,6 +123,7 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() (arch1, path1), (arch2, path2) }); + Command.Create(appExe) .EnableTracingAndCaptureOutputs() .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) @@ -130,9 +131,9 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() .Execute() .Should().HaveStdErrContaining($"Found install location path '{path1}'.") .And.HaveStdErrContaining($"Found architecture-specific install location path: '{path1}' ('{arch1}').") - .And.HaveStdErrContaining($"Found architecture-specific install location path matching the current OS architecture ('{arch1}'): '{path1}'.") .And.HaveStdErrContaining($"Found architecture-specific install location path: '{path2}' ('{arch2}').") - .And.HaveStdErrContaining($"Using global installation location [{path1}] as runtime location."); + .And.HaveStdErrContaining($"Found architecture-specific install location path matching the current OS architecture ('{arch2}'): '{path2}'.") + .And.HaveStdErrContaining($"Using global installation location [{path2}] as runtime location."); } } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 15386b1e9e8456..3045638ebff025 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -421,57 +421,51 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) return false; } - pal::string_t install_location, architecture_install_location, file_line; - bool is_first_line = true; + pal::string_t install_location; + bool is_first_line = true, install_location_found = false; - while (get_line_from_file(install_location_file, file_line)) + while (get_line_from_file(install_location_file, install_location)) { - size_t arch_sep = file_line.find(_X('=')); + size_t arch_sep = install_location.find(_X('=')); if (arch_sep == pal::string_t::npos) { if (is_first_line) { - install_location = file_line; + *recv = install_location; + install_location_found = true; trace::verbose(_X("Found install location path '%s'."), install_location.c_str()); } else { - trace::error(_X("Only the first line in '%s' may not have an architecture prefix."), install_location_file_path.c_str()); + trace::warning(_X("Only the first line in '%s' may not have an architecture prefix."), install_location_file_path.c_str()); } is_first_line = false; continue; } - pal::string_t arch_prefix = file_line.substr(0, arch_sep); - pal::string_t path_to_location = file_line.substr(arch_sep + 1); + pal::string_t arch_prefix = install_location.substr(0, arch_sep); + pal::string_t path_to_location = install_location.substr(arch_sep + 1); trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); if (arch_prefix == get_arch()) { - architecture_install_location = path_to_location; - trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'."), arch_prefix.c_str(), architecture_install_location.c_str()); + *recv = path_to_location; + install_location_found = true; + trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'."), arch_prefix.c_str(), path_to_location.c_str()); break; } is_first_line = false; } - if (architecture_install_location != "") + if (!install_location_found) { - *recv = architecture_install_location; - } - else if (install_location != "") - { - *recv = install_location; - } - else - { - trace::error(_X("Did not find any install location in '%s'."), install_location_file_path.c_str()); + trace::warning(_X("Did not find any install location in '%s'."), install_location_file_path.c_str()); return false; } - trace::verbose(_X("Using install location '%s'."), (*recv).c_str()); + trace::verbose(_X("Using install location '%s'."), recv->c_str()); fclose(install_location_file); return true; } diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index dd17e76c4dadb5..20d321c03f6540 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -100,18 +100,22 @@ pal::string_t strip_file_ext(const pal::string_t& path) bool get_line_from_file(FILE* pFile, pal::string_t& line) { - std::vector buffer; - char last_char; - while ((last_char = fgetc(pFile))) + line = pal::string_t(); + char buffer[256]; + while (fgets(buffer, sizeof(buffer), pFile)) { - if (last_char == '\n' || last_char == EOF) - break; + line += (pal::char_t*)buffer; + size_t len = line.length(); - buffer.push_back((pal::char_t)last_char); + // fgets includes the newline character in the string - so remove it. + if (len > 0 && line[len - 1] == '\n') + { + line[len - 1] = '\0'; + break; + } } - line = pal::string_t(buffer.begin(), buffer.end()); - return buffer.size() != 0; + return !line.empty(); } pal::string_t get_filename_without_ext(const pal::string_t& path) @@ -368,14 +372,15 @@ bool get_dotnet_root_from_env(pal::string_t* dotnet_root_env_var_name, pal::stri { *dotnet_root_env_var_name = _X("DOTNET_ROOT_"); dotnet_root_env_var_name->append(to_upper(get_arch())); - if (pal::getenv(dotnet_root_env_var_name->c_str(), recv)) - return pal::file_exists(*recv); + if (get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv)) + return true; #if defined(WIN32) if (pal::is_running_in_wow64()) { *dotnet_root_env_var_name = _X("DOTNET_ROOT(x86)"); - return get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv); + if (get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv)) + return true; } #endif From 82363907ebacaf8f203008e6ecfac7b7ef84fa3b Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 10 Jun 2021 09:33:47 -0700 Subject: [PATCH 15/23] Add tests with really long install location path Move get_line_from_file to unix pal --- .../HostActivation.Tests/CommandExtensions.cs | 11 +- .../MultiArchInstallLocation.cs | 29 ++++- src/native/corehost/fxr_resolver.cpp | 6 +- src/native/corehost/hostmisc/pal.h | 54 +++++----- src/native/corehost/hostmisc/pal.unix.cpp | 102 +++++++++++------- src/native/corehost/hostmisc/utils.cpp | 32 ++---- src/native/corehost/hostmisc/utils.h | 7 +- 7 files changed, 131 insertions(+), 110 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/CommandExtensions.cs b/src/installer/tests/HostActivation.Tests/CommandExtensions.cs index 3bfa97e4e4d7b9..d8460c1368bdb5 100644 --- a/src/installer/tests/HostActivation.Tests/CommandExtensions.cs +++ b/src/installer/tests/HostActivation.Tests/CommandExtensions.cs @@ -38,18 +38,11 @@ public static Command EnableTracingAndCaptureOutputs(this Command command) public static Command DotNetRoot(this Command command, string dotNetRoot, string architecture = null) { if (!string.IsNullOrEmpty(architecture)) - { - command = command - .EnvironmentVariable($"DOTNET_ROOT_{architecture.ToUpper()}", dotNetRoot); - - return command; - } + return command.EnvironmentVariable($"DOTNET_ROOT_{architecture.ToUpper()}", dotNetRoot); - command = command + return command .EnvironmentVariable("DOTNET_ROOT", dotNetRoot) .EnvironmentVariable("DOTNET_ROOT(x86)", dotNetRoot); - - return command; } public static Command MultilevelLookup(this Command command, bool enable) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 443e44aca81988..0b435984114f33 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -162,6 +162,32 @@ public void InstallLocationFile_OnlyFirstLineMayNotSpecifyArchitecture() } } + [Fact] + [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] + public void InstallLocationFile_ReallyLongInstallPathIsParsedCorrectly() + { + var fixture = sharedTestState.PortableAppFixture + .Copy(); + + var appExe = fixture.TestProject.AppExe; + using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) + { + var reallyLongPath = + "reallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreally" + + "reallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreally" + + "reallyreallyreallyreallyreallyreallyreallyreallyreallyreallylongpath"; + registeredInstallLocationOverride.SetInstallLocation((string.Empty, reallyLongPath)); + + Command.Create(appExe) + .EnableTracingAndCaptureOutputs() + .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) + .DotNetRoot(null) + .Execute() + .Should().HaveStdErrContaining($"Found install location path '{reallyLongPath}'.") + .And.HaveStdErrContaining($"Using install location '{reallyLongPath}'."); + } + } + public class SharedTestState : IDisposable { public string BaseDirectory { get; } @@ -175,7 +201,8 @@ public SharedTestState() var fixture = new TestProjectFixture("PortableApp", RepoDirectories); fixture .EnsureRestored() - .PublishProject(); + // App Host generation is turned off by default on macOS + .PublishProject(extraArgs: "/p:UseAppHost=true"); PortableAppFixture = fixture; BaseDirectory = Path.GetDirectoryName(PortableAppFixture.SdkDotnet.GreatestVersionHostFxrFilePath); diff --git a/src/native/corehost/fxr_resolver.cpp b/src/native/corehost/fxr_resolver.cpp index fbd045f0084257..4ad416175b6362 100644 --- a/src/native/corehost/fxr_resolver.cpp +++ b/src/native/corehost/fxr_resolver.cpp @@ -77,7 +77,7 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o if (pal::get_dotnet_self_registered_dir(&default_install_location) || pal::get_default_installation_dir(&default_install_location)) { trace::info(_X("Using global installation location [%s] as runtime location."), default_install_location.c_str()); - out_dotnet_root->assign(default_install_location); + out_dotnet_root->assign(default_install_location.c_str()); } else { @@ -134,7 +134,7 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o #endif // !FEATURE_APPHOST && !FEATURE_LIBHOST } -bool fxr_resolver::try_get_path_from_dotnet_root(const pal::string_t &dotnet_root, pal::string_t *out_fxr_path) +bool fxr_resolver::try_get_path_from_dotnet_root(const pal::string_t& dotnet_root, pal::string_t* out_fxr_path) { pal::string_t fxr_dir = dotnet_root; append_path(&fxr_dir, _X("host")); @@ -148,7 +148,7 @@ bool fxr_resolver::try_get_path_from_dotnet_root(const pal::string_t &dotnet_roo return get_latest_fxr(std::move(fxr_dir), out_fxr_path); } -bool fxr_resolver::try_get_existing_fxr(pal::dll_t *out_fxr, pal::string_t *out_fxr_path) +bool fxr_resolver::try_get_existing_fxr(pal::dll_t* out_fxr, pal::string_t* out_fxr_path) { if (!pal::get_loaded_library(LIBFXR_NAME, "hostfxr_main", out_fxr, out_fxr_path)) return false; diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 95b4fcf84caa4e..5da1d6a98c4932 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -104,13 +104,13 @@ namespace pal { #if defined(_WIN32) - #ifdef EXPORT_SHARED_API - #define SHARED_API extern "C" __declspec(dllexport) - #else - #define SHARED_API extern "C" - #endif +#ifdef EXPORT_SHARED_API +#define SHARED_API extern "C" __declspec(dllexport) +#else +#define SHARED_API extern "C" +#endif - #define STDMETHODCALLTYPE __stdcall +#define STDMETHODCALLTYPE __stdcall typedef wchar_t char_t; typedef std::wstring string_t; @@ -151,13 +151,13 @@ namespace pal inline int strcasecmp(const char_t* str1, const char_t* str2) { return ::_wcsicmp(str1, str2); } inline int strncmp(const char_t* str1, const char_t* str2, size_t len) { return ::wcsncmp(str1, str2, len); } inline int strncasecmp(const char_t* str1, const char_t* str2, size_t len) { return ::_wcsnicmp(str1, str2, len); } - inline int pathcmp(const pal::string_t &path1, const pal::string_t &path2) { return strcasecmp(path1.c_str(), path2.c_str()); } + inline int pathcmp(const pal::string_t& path1, const pal::string_t& path2) { return strcasecmp(path1.c_str(), path2.c_str()); } inline string_t to_string(int value) { return std::to_wstring(value); } inline size_t strlen(const char_t* str) { return ::wcslen(str); } #pragma warning(suppress : 4996) // error C4996: '_wfopen': This function or variable may be unsafe. - inline FILE * file_open(const string_t& path, const char_t* mode) { return ::_wfopen(path.c_str(), mode); } + inline FILE* file_open(const string_t& path, const char_t* mode) { return ::_wfopen(path.c_str(), mode); } inline void file_vprintf(FILE* f, const char_t* format, va_list vl) { ::vfwprintf(f, format, vl); ::fputwc(_X('\n'), f); } inline void err_fputs(const char_t* message) { ::fputws(message, stderr); ::fputwc(_X('\n'), stderr); } @@ -170,32 +170,32 @@ namespace pal // Suppressing warning since the 'safe' version requires an input buffer that is unnecessary for // uses of this function. #pragma warning(suppress : 4996) // error C4996: '_wcserror': This function or variable may be unsafe. - inline const char_t* strerror(int errnum){ return ::_wcserror(errnum); } + inline const char_t* strerror(int errnum) { return ::_wcserror(errnum); } bool pal_utf8string(const string_t& str, std::vector* out); bool pal_clrstring(const string_t& str, std::vector* out); bool clr_palstring(const char* cstr, string_t* out); inline bool mkdir(const char_t* dir, int mode) { return CreateDirectoryW(dir, NULL) != 0; } - inline bool rmdir (const char_t* path) { return RemoveDirectoryW(path) != 0; } + inline bool rmdir(const char_t* path) { return RemoveDirectoryW(path) != 0; } inline int rename(const char_t* old_name, const char_t* new_name) { return ::_wrename(old_name, new_name); } inline int remove(const char_t* path) { return ::_wremove(path); } inline bool munmap(void* addr, size_t length) { return UnmapViewOfFile(addr) != 0; } inline int get_pid() { return GetCurrentProcessId(); } inline void sleep(uint32_t milliseconds) { Sleep(milliseconds); } #else - #ifdef EXPORT_SHARED_API - #define SHARED_API extern "C" __attribute__((__visibility__("default"))) - #else - #define SHARED_API extern "C" - #endif - - #define __cdecl /* nothing */ - #define __stdcall /* nothing */ - #if !defined(TARGET_FREEBSD) - #define __fastcall /* nothing */ - #endif - #define STDMETHODCALLTYPE __stdcall +#ifdef EXPORT_SHARED_API +#define SHARED_API extern "C" __attribute__((__visibility__("default"))) +#else +#define SHARED_API extern "C" +#endif + +#define __cdecl /* nothing */ +#define __stdcall /* nothing */ +#if !defined(TARGET_FREEBSD) +#define __fastcall /* nothing */ +#endif +#define STDMETHODCALLTYPE __stdcall typedef char char_t; typedef std::string string_t; @@ -219,7 +219,7 @@ namespace pal inline string_t to_string(int value) { return std::to_string(value); } inline size_t strlen(const char_t* str) { return ::strlen(str); } - inline FILE * file_open(const string_t& path, const char_t* mode) { return fopen(path.c_str(), mode); } + inline FILE* file_open(const string_t& path, const char_t* mode) { return fopen(path.c_str(), mode); } inline void file_vprintf(FILE* f, const char_t* format, va_list vl) { ::vfprintf(f, format, vl); ::fputc('\n', f); } inline void err_fputs(const char_t* message) { ::fputs(message, stderr); ::fputc(_X('\n'), stderr); } inline void out_vprintf(const char_t* format, va_list vl) { ::vfprintf(stdout, format, vl); ::fputc('\n', stdout); } @@ -238,6 +238,8 @@ namespace pal inline int get_pid() { return getpid(); } inline void sleep(uint32_t milliseconds) { usleep(milliseconds * 1000); } + bool get_line_from_file(FILE* pFile, pal::string_t& line); + #endif inline int snwprintf(char_t* buffer, size_t count, const char_t* format, ...) @@ -253,7 +255,7 @@ namespace pal bool getcwd(string_t* recv); - inline void file_flush(FILE *f) { std::fflush(f); } + inline void file_flush(FILE* f) { std::fflush(f); } inline void err_flush() { std::fflush(stderr); } inline void out_flush() { std::fflush(stdout); } @@ -281,7 +283,7 @@ namespace pal bool get_own_module_path(string_t* recv); bool get_method_module_path(string_t* recv, void* method); bool get_module_path(dll_t mod, string_t* recv); - bool get_current_module(dll_t *mod); + bool get_current_module(dll_t* mod); bool getenv(const char_t* name, string_t* recv); bool get_default_servicing_directory(string_t* recv); @@ -305,7 +307,7 @@ namespace pal int xtoi(const char_t* input); - bool get_loaded_library(const char_t *library_name, const char *symbol_name, /*out*/ dll_t *dll, /*out*/ string_t *path); + bool get_loaded_library(const char_t* library_name, const char* symbol_name, /*out*/ dll_t* dll, /*out*/ string_t* path); bool load_library(const string_t* path, dll_t* dll); proc_t get_symbol(dll_t library, const char* name); void unload_library(dll_t library); diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 3045638ebff025..1adcb5dd75eafb 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -68,7 +68,7 @@ bool pal::touch_file(const pal::string_t& path) trace::warning(_X("open(%s) failed in %s"), path.c_str(), _STRINGIFY(__FUNCTION__)); return false; } - (void) close(fd); + (void)close(fd); return true; } @@ -139,12 +139,12 @@ bool pal::getcwd(pal::string_t* recv) namespace { - bool get_loaded_library_from_proc_maps(const pal::char_t *library_name, pal::dll_t *dll, pal::string_t *path) + bool get_loaded_library_from_proc_maps(const pal::char_t* library_name, pal::dll_t* dll, pal::string_t* path) { - char *line = nullptr; + char* line = nullptr; size_t lineLen = 0; ssize_t read; - FILE *file = pal::file_open(_X("/proc/self/maps"), _X("r")); + FILE* file = pal::file_open(_X("/proc/self/maps"), _X("r")); if (file == nullptr) return false; @@ -185,10 +185,10 @@ namespace } bool pal::get_loaded_library( - const char_t *library_name, - const char *symbol_name, - /*out*/ dll_t *dll, - /*out*/ pal::string_t *path) + const char_t* library_name, + const char* symbol_name, + /*out*/ dll_t* dll, + /*out*/ pal::string_t* path) { pal::string_t library_name_local; #if defined(TARGET_OSX) @@ -333,7 +333,7 @@ bool pal::get_default_servicing_directory(string_t* recv) bool is_read_write_able_directory(pal::string_t& dir) { return pal::realpath(&dir) && - (access(dir.c_str(), R_OK | W_OK | X_OK) == 0); + (access(dir.c_str(), R_OK | W_OK | X_OK) == 0); } bool get_extraction_base_parent_directory(pal::string_t& directory) @@ -424,7 +424,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) pal::string_t install_location; bool is_first_line = true, install_location_found = false; - while (get_line_from_file(install_location_file, install_location)) + while (pal::get_line_from_file(install_location_file, install_location)) { size_t arch_sep = install_location.find(_X('=')); if (arch_sep == pal::string_t::npos) @@ -459,6 +459,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) is_first_line = false; } + fclose(install_location_file); if (!install_location_found) { trace::warning(_X("Did not find any install location in '%s'."), install_location_file_path.c_str()); @@ -466,7 +467,6 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } trace::verbose(_X("Using install location '%s'."), recv->c_str()); - fclose(install_location_file); return true; } @@ -482,17 +482,37 @@ bool pal::get_default_installation_dir(pal::string_t* recv) // *************************** #if defined(TARGET_OSX) - recv->assign(_X("/usr/local/share/dotnet")); + recv->assign(_X("/usr/local/share/dotnet")); #else - recv->assign(_X("/usr/share/dotnet")); + recv->assign(_X("/usr/share/dotnet")); #endif - return true; + return true; +} + +bool pal::get_line_from_file(FILE* pFile, pal::string_t& line) +{ + line = pal::string_t(); + char buffer[256]; + while (fgets(buffer, sizeof(buffer), pFile)) + { + line += (pal::char_t*)buffer; + size_t len = line.length(); + + // fgets includes the newline character in the string - so remove it. + if (len > 0 && line[len - 1] == '\n') + { + line.pop_back(); + break; + } + } + + return !line.empty(); } pal::string_t trim_quotes(pal::string_t stringToCleanup) { - pal::char_t quote_array[2] = {'\"', '\''}; - for (size_t index = 0; index < sizeof(quote_array)/sizeof(quote_array[0]); index++) + pal::char_t quote_array[2] = { '\"', '\'' }; + for (size_t index = 0; index < sizeof(quote_array) / sizeof(quote_array[0]); index++) { size_t pos = stringToCleanup.find(quote_array[index]); while (pos != std::string::npos) @@ -568,11 +588,11 @@ pal::string_t pal::get_current_os_rid_platform() if (ret == 0) { - char *pos = strchr(str, '.'); + char* pos = strchr(str, '.'); if (pos) { ridOS.append(_X("freebsd.")) - .append(str, pos - str); + .append(str, pos - str); } } @@ -604,7 +624,7 @@ pal::string_t pal::get_current_os_rid_platform() if (strncmp(utsname_obj.version, "omnios", strlen("omnios")) == 0) { ridOS.append(_X("omnios.")) - .append(utsname_obj.version, strlen("omnios-r"), 2); // e.g. omnios.15 + .append(utsname_obj.version, strlen("omnios-r"), 2); // e.g. omnios.15 } else if (strncmp(utsname_obj.version, "illumos-", strlen("illumos-")) == 0) { @@ -613,7 +633,7 @@ pal::string_t pal::get_current_os_rid_platform() else if (strncmp(utsname_obj.version, "joyent_", strlen("joyent_")) == 0) { ridOS.append(_X("smartos.")) - .append(utsname_obj.version, strlen("joyent_"), 4); // e.g. smartos.2020 + .append(utsname_obj.version, strlen("joyent_"), 4); // e.g. smartos.2020 } return ridOS; @@ -636,11 +656,11 @@ pal::string_t pal::get_current_os_rid_platform() return ridOS; } - char *pos = strchr(utsname_obj.version, '.'); + char* pos = strchr(utsname_obj.version, '.'); if (pos) { ridOS.append(_X("solaris.")) - .append(utsname_obj.version, pos - utsname_obj.version); // e.g. solaris.11 + .append(utsname_obj.version, pos - utsname_obj.version); // e.g. solaris.11 } return ridOS; @@ -786,7 +806,7 @@ bool pal::get_own_executable_path(pal::string_t* recv) bool pal::get_own_module_path(string_t* recv) { Dl_info info; - if (dladdr((void *)&pal::get_own_module_path, &info) == 0) + if (dladdr((void*)&pal::get_own_module_path, &info) == 0) return false; recv->assign(info.dli_fname); @@ -808,7 +828,7 @@ bool pal::get_module_path(dll_t module, string_t* recv) return false; } -bool pal::get_current_module(dll_t *mod) +bool pal::get_current_module(dll_t* mod) { return false; } @@ -891,31 +911,31 @@ static void readdir(const pal::string_t& path, const pal::string_t& pattern, boo } break; - // Handle symlinks and file systems that do not support d_type + // Handle symlinks and file systems that do not support d_type case DT_LNK: case DT_UNKNOWN: - { - struct stat sb; + { + struct stat sb; - if (fstatat(dirfd(dir), entry->d_name, &sb, 0) == -1) - { - continue; - } + if (fstatat(dirfd(dir), entry->d_name, &sb, 0) == -1) + { + continue; + } - if (onlydirectories) - { - if (!S_ISDIR(sb.st_mode)) - { - continue; - } - break; - } - else if (!S_ISREG(sb.st_mode) && !S_ISDIR(sb.st_mode)) + if (onlydirectories) + { + if (!S_ISDIR(sb.st_mode)) { continue; } + break; } - break; + else if (!S_ISREG(sb.st_mode) && !S_ISDIR(sb.st_mode)) + { + continue; + } + } + break; default: continue; diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index 20d321c03f6540..3fa385507e5303 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -98,26 +98,6 @@ pal::string_t strip_file_ext(const pal::string_t& path) return path.substr(0, dot_pos); } -bool get_line_from_file(FILE* pFile, pal::string_t& line) -{ - line = pal::string_t(); - char buffer[256]; - while (fgets(buffer, sizeof(buffer), pFile)) - { - line += (pal::char_t*)buffer; - size_t len = line.length(); - - // fgets includes the newline character in the string - so remove it. - if (len > 0 && line[len - 1] == '\n') - { - line[len - 1] = '\0'; - break; - } - } - - return !line.empty(); -} - pal::string_t get_filename_without_ext(const pal::string_t& path) { if (path.empty()) @@ -181,7 +161,7 @@ void remove_trailing_dir_seperator(pal::string_t* dir) void replace_char(pal::string_t* path, pal::char_t match, pal::char_t repl) { - size_t pos = 0; + size_t pos = 0; while ((pos = path->find(match, pos)) != pal::string_t::npos) { (*path)[pos] = repl; @@ -190,7 +170,7 @@ void replace_char(pal::string_t* path, pal::char_t match, pal::char_t repl) pal::string_t get_replaced_char(const pal::string_t& path, pal::char_t match, pal::char_t repl) { - size_t pos = path.find(match); + size_t pos = path.find(match); if (pos == pal::string_t::npos) { return path; @@ -261,7 +241,7 @@ bool get_env_shared_store_dirs(std::vector* dirs, const pal::stri return true; } -bool get_global_shared_store_dirs(std::vector* dirs, const pal::string_t& arch, const pal::string_t& tfm) +bool get_global_shared_store_dirs(std::vector* dirs, const pal::string_t& arch, const pal::string_t& tfm) { std::vector global_dirs; if (!pal::get_global_dotnet_dirs(&global_dirs)) @@ -386,7 +366,7 @@ bool get_dotnet_root_from_env(pal::string_t* dotnet_root_env_var_name, pal::stri // If no architecture-specific environment variable was set // fallback to the default DOTNET_ROOT. - *dotnet_root_env_var_name = _X("DOTNET_ROOT"); + * dotnet_root_env_var_name = _X("DOTNET_ROOT"); return get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv); } @@ -434,7 +414,7 @@ void get_runtime_config_paths(const pal::string_t& path, const pal::string_t& na trace::verbose(_X("Runtime config is cfg=%s dev=%s"), cfg->c_str(), dev_cfg->c_str()); } -pal::string_t get_dotnet_root_from_fxr_path(const pal::string_t &fxr_path) +pal::string_t get_dotnet_root_from_fxr_path(const pal::string_t& fxr_path) { // If coreclr exists next to hostfxr, assume everything is local (e.g. self-contained) pal::string_t fxr_dir = get_directory(fxr_path); @@ -446,7 +426,7 @@ pal::string_t get_dotnet_root_from_fxr_path(const pal::string_t &fxr_path) return get_directory(get_directory(fxr_root)); } -pal::string_t get_download_url(const pal::char_t *framework_name, const pal::char_t *framework_version) +pal::string_t get_download_url(const pal::char_t* framework_name, const pal::char_t* framework_version) { pal::string_t url = DOTNET_CORE_APPLAUNCH_URL _X("?"); if (framework_name != nullptr && pal::strlen(framework_name) > 0) diff --git a/src/native/corehost/hostmisc/utils.h b/src/native/corehost/hostmisc/utils.h index c4f46d45b24fb1..b4e31b599fac1a 100644 --- a/src/native/corehost/hostmisc/utils.h +++ b/src/native/corehost/hostmisc/utils.h @@ -41,7 +41,6 @@ bool get_global_shared_store_dirs(std::vector* dirs, const pal::s bool multilevel_lookup_enabled(); void get_framework_and_sdk_locations(const pal::string_t& dotnet_dir, std::vector* locations); bool get_file_path_from_env(const pal::char_t* env_key, pal::string_t* recv); -bool get_line_from_file(FILE* pFile, pal::string_t& line); size_t index_of_non_numeric(const pal::string_t& str, size_t i); bool try_stou(const pal::string_t& str, unsigned* num); bool get_dotnet_root_from_env(pal::string_t* used_dotnet_root_env_var_name, pal::string_t* recv); @@ -49,11 +48,11 @@ pal::string_t get_deps_from_app_binary(const pal::string_t& app_base, const pal: pal::string_t get_runtime_config_path(const pal::string_t& path, const pal::string_t& name); pal::string_t get_runtime_config_dev_path(const pal::string_t& path, const pal::string_t& name); void get_runtime_config_paths(const pal::string_t& path, const pal::string_t& name, pal::string_t* cfg, pal::string_t* dev_cfg); -pal::string_t get_dotnet_root_from_fxr_path(const pal::string_t &fxr_path); +pal::string_t get_dotnet_root_from_fxr_path(const pal::string_t& fxr_path); // Get a download URL for a specific framework and version // If no framework is specified, a download URL for the runtime is returned -pal::string_t get_download_url(const pal::char_t *framework_name = nullptr, const pal::char_t *framework_version = nullptr); +pal::string_t get_download_url(const pal::char_t* framework_name = nullptr, const pal::char_t* framework_version = nullptr); pal::string_t to_lower(const pal::char_t* in); pal::string_t to_upper(const pal::char_t* in); @@ -67,7 +66,7 @@ bool test_only_getenv(const pal::char_t* name, pal::string_t* recv); class propagate_error_writer_t { public: - typedef trace::error_writer_fn(__cdecl *set_error_writer_fn)(trace::error_writer_fn error_writer); + typedef trace::error_writer_fn(__cdecl* set_error_writer_fn)(trace::error_writer_fn error_writer); private: set_error_writer_fn m_set_error_writer; From 190bc43a9f5c8100f2cdcec1c8ef7883a109988c Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 10 Jun 2021 13:25:26 -0700 Subject: [PATCH 16/23] Fix tests --- .../HostActivation.Tests/MultiArchInstallLocation.cs | 12 ++++++++---- .../HostActivation.Tests/PortableAppActivation.cs | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 0b435984114f33..fd77ddbbb15af2 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -45,12 +45,16 @@ public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); - Command.Create(appExe) + var result = Command.Create(appExe) .EnableTracingAndCaptureOutputs() .DotNetRoot(fixture.BuiltDotnet.BinPath) - .Execute() - .Should().Pass() - .And.HaveStdErrContaining($"Using environment variable DOTNET_ROOT="); + .Execute(); + + result.Should().Pass(); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && RuntimeInformation.OSArchitecture == Architecture.X86) + result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT(x86)="); + else + result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT="); } [Fact] diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index 22fdb7acad8ae0..a2a0d963e1f094 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -297,7 +297,7 @@ public void AppHost_FrameworkDependent_GlobalLocation_Succeeds(bool useRegistere using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe)) { - string architecture = fixture.CurrentRid.Split('-')[1]; + string architecture = fixture.RepoDirProvider.BuildArchitecture; if (useRegisteredLocation) { registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { (architecture, builtDotnet) }); From bbc05c8d6740b5860b2765042cb265cb468f42c6 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 14 Jun 2021 10:44:57 -0700 Subject: [PATCH 17/23] Make consistent use of str assign --- .../HostActivation.Tests/MultiArchInstallLocation.cs | 2 +- src/native/corehost/fxr_resolver.cpp | 2 +- src/native/corehost/hostmisc/pal.unix.cpp | 8 ++++---- src/native/corehost/hostmisc/pal.windows.cpp | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index fd77ddbbb15af2..a6706670811e46 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -51,7 +51,7 @@ public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() .Execute(); result.Should().Pass(); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && RuntimeInformation.OSArchitecture == Architecture.X86) + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && arch == "X86") result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT(x86)="); else result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT="); diff --git a/src/native/corehost/fxr_resolver.cpp b/src/native/corehost/fxr_resolver.cpp index 4ad416175b6362..6e1356457d49ae 100644 --- a/src/native/corehost/fxr_resolver.cpp +++ b/src/native/corehost/fxr_resolver.cpp @@ -77,7 +77,7 @@ bool fxr_resolver::try_get_path(const pal::string_t& root_path, pal::string_t* o if (pal::get_dotnet_self_registered_dir(&default_install_location) || pal::get_default_installation_dir(&default_install_location)) { trace::info(_X("Using global installation location [%s] as runtime location."), default_install_location.c_str()); - out_dotnet_root->assign(default_install_location.c_str()); + out_dotnet_root->assign(default_install_location); } else { diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 1adcb5dd75eafb..c19ec520c61a6f 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -381,13 +381,13 @@ bool pal::get_global_dotnet_dirs(std::vector* recv) bool pal::get_dotnet_self_registered_config_location(pal::string_t* recv) { - *recv = _X("/etc/dotnet/install_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 = environment_install_location_override; + recv->assign(environment_install_location_override); } return true; @@ -431,7 +431,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) { if (is_first_line) { - *recv = install_location; + recv->assign(install_location); install_location_found = true; trace::verbose(_X("Found install location path '%s'."), install_location.c_str()); } @@ -450,7 +450,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); if (arch_prefix == get_arch()) { - *recv = path_to_location; + recv->assign(path_to_location); install_location_found = true; trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'."), arch_prefix.c_str(), path_to_location.c_str()); break; diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 24ae7c1a899d9e..c992f92da8294e 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -26,7 +26,7 @@ bool GetModuleFileNameWrapper(HMODULE hModule, pal::string_t* recv) return false; path.resize(dwModuleFileName); - *recv = path; + recv->assign(path); return true; } @@ -332,7 +332,7 @@ bool pal::get_dotnet_self_registered_config_location(pal::string_t* recv) const pal::char_t* value; get_dotnet_install_location_registry_path(&key_hive, &sub_key, &value); - *recv = (key_hive == HKEY_CURRENT_USER ? _X("HKCU\\") : _X("HKLM\\")) + sub_key + _X("\\") + value; + recv->assign((key_hive == HKEY_CURRENT_USER ? _X("HKCU\\") : _X("HKLM\\")) + sub_key + _X("\\") + value); return true; #endif } From e5b18e564920aeb7731cd6682f675552d8e98d4c Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 17 Jun 2021 12:48:32 -0700 Subject: [PATCH 18/23] PR feedback --- .../InstallLocationCommandResultExtensions.cs | 49 +++++++++++++ .../MultiArchInstallLocation.cs | 72 +++++-------------- .../NativeHosting/Nethost.cs | 31 +++++--- .../RegisteredInstallLocationOverride.cs | 13 ++-- src/native/corehost/hostmisc/pal.h | 3 - src/native/corehost/hostmisc/pal.unix.cpp | 36 ++++++---- src/native/corehost/hostmisc/utils.cpp | 2 +- 7 files changed, 118 insertions(+), 88 deletions(-) create mode 100644 src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs diff --git a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs new file mode 100644 index 00000000000000..b31d93160f4b4a --- /dev/null +++ b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + + +using System; +using System.Runtime.InteropServices; +using FluentAssertions; +using Microsoft.DotNet.Cli.Build.Framework; +using Microsoft.DotNet.CoreSetup.Test; + +namespace HostActivation.Tests +{ + internal static class InstallLocationCommandResultExtensions + { + private static bool IsRunningInWoW64() => OperatingSystem.IsWindows() && !Environment.Is64BitProcess && Environment.Is64BitOperatingSystem; + + public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string installLocation) + { + return assertion.HaveUsedDotNetRootInstallLocation(null, installLocation); + } + + public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string arch, string installLocation) + { + string expectedEnvironmentVariable = !string.IsNullOrEmpty(arch) ? $"DOTNET_ROOT_{arch.ToUpper()}" : IsRunningInWoW64() ? "DOTNET_ROOT(x86)" : "DOTNET_ROOT"; + + return assertion.HaveStdErrContaining($"Using environment variable {expectedEnvironmentVariable}=[{installLocation}] as runtime location."); + } + + public static AndConstraint HaveUsedConfigFileInstallLocation(this CommandResultAssertions assertion, string installLocation) + { + return assertion.HaveStdErrContaining($"Using install location '{installLocation}'."); + } + + public static AndConstraint HaveUsedGlobalInstallLocation(this CommandResultAssertions assertion, string installLocation) + { + return assertion.HaveStdErrContaining($"Using global installation location [{installLocation}]"); + } + + public static AndConstraint HaveFoundDefaultInstallLocationInConfigFile(this CommandResultAssertions assertion, string installLocation) + { + return assertion.HaveStdErrContaining($"Found install location path '{installLocation}'."); + } + + public static AndConstraint HaveFoundArchSpecificInstallLocationInConfigFile(this CommandResultAssertions assertion, string arch, string installLocation) + { + return assertion.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}' ('{arch}')."); + } + } +} diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index a6706670811e46..7a8956953d2a3c 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -11,7 +11,6 @@ namespace HostActivation.Tests { - public class MultiArchInstallLocation : IClassFixture { private SharedTestState sharedTestState; @@ -22,7 +21,7 @@ public MultiArchInstallLocation(SharedTestState fixture) } [Fact] - public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() + public void EnvironmentVariable_CurrentArchitectureIsUsedIfEnvVarSet() { var fixture = sharedTestState.PortableAppFixture .Copy(); @@ -34,31 +33,27 @@ public void GlobalInstallation_CurrentArchitectureIsUsedIfEnvVarSet() .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) .Execute() .Should().Pass() - .And.HaveStdErrContaining($"Using environment variable DOTNET_ROOT_{arch}"); + .And.HaveUsedDotNetRootInstallLocation(arch, fixture.BuiltDotnet.BinPath); } [Fact] - public void GlobalInstallation_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUed() + public void EnvironmentVariable_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUsed() { var fixture = sharedTestState.PortableAppFixture .Copy(); var appExe = fixture.TestProject.AppExe; var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); - var result = Command.Create(appExe) + Command.Create(appExe) .EnableTracingAndCaptureOutputs() .DotNetRoot(fixture.BuiltDotnet.BinPath) - .Execute(); - - result.Should().Pass(); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && arch == "X86") - result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT(x86)="); - else - result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT="); + .Execute() + .Should().Pass() + .And.HaveUsedDotNetRootInstallLocation(fixture.BuiltDotnet.BinPath); } [Fact] - public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() + public void EnvironmentVariable_ArchSpecificDotnetRootIsUsedOverDotnetRoot() { var fixture = sharedTestState.PortableAppFixture .Copy(); @@ -72,41 +67,10 @@ public void GlobalInstallation_ArchSpecificDotnetRootIsUsedOverDotnetRoot() .DotNetRoot(dotnet, arch) .Execute() .Should().Pass() - .And.HaveStdErrContaining($"Using environment variable DOTNET_ROOT_{arch}=[{dotnet}] as runtime location.") + .And.HaveUsedDotNetRootInstallLocation(arch, dotnet) .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); } - [Theory] - [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(false)] - [InlineData(true)] - public void GlobalInstallation_WindowsX86(bool setArchSpecificDotnetRoot) - { - var fixture = sharedTestState.PortableAppFixture - .Copy(); - - if (RuntimeInformation.OSArchitecture != Architecture.X86) - return; - - var appExe = fixture.TestProject.AppExe; - var arch = fixture.RepoDirProvider.BuildArchitecture.ToUpper(); - var dotnet = fixture.BuiltDotnet.BinPath; - var command = Command.Create(appExe) - .EnableTracingAndCaptureOutputs() - .DotNetRoot(dotnet); - - if (setArchSpecificDotnetRoot) - command.DotNetRoot(dotnet, arch); - - var result = command.Execute(); - result.Should().Pass(); - - if (setArchSpecificDotnetRoot) - result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT_X86=[{dotnet}] as runtime location."); - else - result.Should().HaveStdErrContaining($"Using environment variable DOTNET_ROOT(x86)=[{dotnet}] as runtime location."); - } - [Fact] [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() @@ -133,11 +97,11 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) .DotNetRoot(null) .Execute() - .Should().HaveStdErrContaining($"Found install location path '{path1}'.") - .And.HaveStdErrContaining($"Found architecture-specific install location path: '{path1}' ('{arch1}').") - .And.HaveStdErrContaining($"Found architecture-specific install location path: '{path2}' ('{arch2}').") - .And.HaveStdErrContaining($"Found architecture-specific install location path matching the current OS architecture ('{arch2}'): '{path2}'.") - .And.HaveStdErrContaining($"Using global installation location [{path2}] as runtime location."); + .Should().HaveFoundDefaultInstallLocationInConfigFile(path1) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch1, path1) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch2, path1) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch2, path2) + .And.HaveUsedGlobalInstallLocation(path2); } } @@ -160,9 +124,9 @@ public void InstallLocationFile_OnlyFirstLineMayNotSpecifyArchitecture() .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) .DotNetRoot(null) .Execute() - .Should().HaveStdErrContaining($"Found install location path 'a/b/c'.") + .Should().HaveFoundDefaultInstallLocationInConfigFile("a/b/c") .And.HaveStdErrContaining($"Only the first line in '{registeredInstallLocationOverride.PathValueOverride}' may not have an architecture prefix.") - .And.HaveStdErrContaining($"Using install location 'a/b/c'."); + .And.HaveUsedConfigFileInstallLocation("a/b/c"); } } @@ -187,8 +151,8 @@ public void InstallLocationFile_ReallyLongInstallPathIsParsedCorrectly() .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride) .DotNetRoot(null) .Execute() - .Should().HaveStdErrContaining($"Found install location path '{reallyLongPath}'.") - .And.HaveStdErrContaining($"Using install location '{reallyLongPath}'."); + .Should().HaveFoundDefaultInstallLocationInConfigFile(reallyLongPath) + .And.HaveUsedConfigFileInstallLocation(reallyLongPath); } } diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs index ef2a50d2ea1720..0af2ce34774127 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs @@ -180,21 +180,32 @@ public void GetHostFxrPath_HostFxrAlreadyLoaded() [Theory] [SkipOnPlatform(TestPlatforms.Windows, "This test targets the install_location config file which is only used on Linux and macOS.")] - [InlineData("{0}", true)] - [InlineData("{0}\n", true)] - [InlineData("{0}\nSome other text", true)] - [InlineData("", false)] - [InlineData("\n{0}", false)] - [InlineData(" {0}", false)] - [InlineData("{0} \n", false)] - [InlineData("{0} ", false)] - public void GetHostFxrPath_InstallLocationFile(string value, bool shouldPass) + [InlineData("{0}", false, true)] + [InlineData("{0}\n", false, true)] + [InlineData("{0}\nSome other text", false, true)] + [InlineData("", false, false)] + [InlineData("\n{0}", false, false)] + [InlineData(" {0}", false, false)] + [InlineData("{0} \n", false, false)] + [InlineData("{0} ", false, false)] + [InlineData("{0}", true, true)] + [InlineData("{0}\n", true, true)] + [InlineData("{0}\nSome other text", true, true)] + [InlineData("", true, false)] + [InlineData("\n{0}", true, false)] + [InlineData(" {0}", true, false)] + [InlineData("{0} \n", true, false)] + [InlineData("{0} ", true, false)] + public void GetHostFxrPath_InstallLocationFile(string value, bool shouldUseArchSpecificInstallLocation, bool shouldPass) { string installLocation = Path.Combine(sharedState.ValidInstallRoot, "dotnet"); using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(sharedState.NethostPath)) { - registeredInstallLocationOverride.SetInstallLocation((sharedState.RepoDirectories.BuildArchitecture, string.Format(value, installLocation))); + if (shouldUseArchSpecificInstallLocation) + registeredInstallLocationOverride.SetInstallLocation((sharedState.RepoDirectories.BuildArchitecture, string.Format(value, installLocation))); + else + registeredInstallLocationOverride.SetInstallLocation((string.Empty, string.Format(value, installLocation))); CommandResult result = Command.Create(sharedState.NativeHostPath, GetHostFxrPath) .EnableTracingAndCaptureOutputs() diff --git a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs index f1b13b8f4aef59..d37cfafe649537 100644 --- a/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs +++ b/src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs @@ -65,18 +65,21 @@ public RegisteredInstallLocationOverride(string productBinaryPath) public void SetInstallLocation(params (string Architecture, string Path)[] locations) { - var locationsList = locations.ToList(); - Debug.Assert(locationsList.Count >= 1); + Debug.Assert(locations.Length >= 1); if (OperatingSystem.IsWindows()) { - foreach (var location in locationsList) + foreach (var location in locations) + { using (RegistryKey dotnetLocationKey = key.CreateSubKey($@"Setup\InstalledVersions\{location.Architecture}")) + { dotnetLocationKey.SetValue("InstallLocation", location.Path); + } + } } else { - File.WriteAllText(PathValueOverride, string.Join (Environment.NewLine, - locationsList.Select(l => string.Format("{0}{1}".ToLower(), + File.WriteAllText(PathValueOverride, string.Join(Environment.NewLine, + locations.Select(l => string.Format("{0}{1}", (!string.IsNullOrWhiteSpace(l.Architecture) ? l.Architecture + "=" : ""), l.Path)))); } } diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 5da1d6a98c4932..b6622cd5dff52b 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -237,9 +237,6 @@ namespace pal inline bool munmap(void* addr, size_t length) { return ::munmap(addr, length) == 0; } inline int get_pid() { return getpid(); } inline void sleep(uint32_t milliseconds) { usleep(milliseconds * 1000); } - - bool get_line_from_file(FILE* pFile, pal::string_t& line); - #endif inline int snwprintf(char_t* buffer, size_t count, const char_t* format, ...) diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index c19ec520c61a6f..048ddd3252f508 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -422,10 +422,12 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } pal::string_t install_location; + int current_line = 0; bool is_first_line = true, install_location_found = false; while (pal::get_line_from_file(install_location_file, install_location)) { + current_line++; size_t arch_sep = install_location.find(_X('=')); if (arch_sep == pal::string_t::npos) { @@ -437,6 +439,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) } else { + trace::warning(_X("Found unprefixed install location path '%s' on line %d."), install_location.c_str(), current_line); trace::warning(_X("Only the first line in '%s' may not have an architecture prefix."), install_location_file_path.c_str()); } @@ -448,11 +451,11 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) pal::string_t path_to_location = install_location.substr(arch_sep + 1); trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); - if (arch_prefix == get_arch()) + if (strcasecmp(arch_prefix, get_arch()) == 0) { recv->assign(path_to_location); install_location_found = true; - trace::verbose(_X("Found architecture-specific install location path matching the current OS architecture ('%s'): '%s'."), arch_prefix.c_str(), path_to_location.c_str()); + trace::verbose(_X("Found architecture-specific install location path matching the current host architecture ('%s'): '%s'."), arch_prefix.c_str(), path_to_location.c_str()); break; } @@ -489,24 +492,27 @@ bool pal::get_default_installation_dir(pal::string_t* recv) return true; } -bool pal::get_line_from_file(FILE* pFile, pal::string_t& line) +namespace { - line = pal::string_t(); - char buffer[256]; - while (fgets(buffer, sizeof(buffer), pFile)) + bool pal::get_line_from_file(FILE* pFile, pal::string_t& line) { - line += (pal::char_t*)buffer; - size_t len = line.length(); - - // fgets includes the newline character in the string - so remove it. - if (len > 0 && line[len - 1] == '\n') + line = pal::string_t(); + char buffer[256]; + while (fgets(buffer, sizeof(buffer), pFile)) { - line.pop_back(); - break; + line += (pal::char_t*)buffer; + size_t len = line.length(); + + // fgets includes the newline character in the string - so remove it. + if (len > 0 && line[len - 1] == '\n') + { + line.pop_back(); + break; + } } - } - return !line.empty(); + return !line.empty(); + } } pal::string_t trim_quotes(pal::string_t stringToCleanup) diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index 3fa385507e5303..abf1aeee89e0a1 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -366,7 +366,7 @@ bool get_dotnet_root_from_env(pal::string_t* dotnet_root_env_var_name, pal::stri // If no architecture-specific environment variable was set // fallback to the default DOTNET_ROOT. - * dotnet_root_env_var_name = _X("DOTNET_ROOT"); + *dotnet_root_env_var_name = _X("DOTNET_ROOT"); return get_file_path_from_env(dotnet_root_env_var_name->c_str(), recv); } From 53edd071e6ac6ddf2fecee8527ae6889f157d08e Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 17 Jun 2021 13:39:57 -0700 Subject: [PATCH 19/23] Fix unix pal --- src/native/corehost/hostmisc/pal.unix.cpp | 50 +++++++++++------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 048ddd3252f508..411298a08f4cef 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -393,6 +393,29 @@ bool pal::get_dotnet_self_registered_config_location(pal::string_t* recv) return true; } +namespace +{ + bool get_line_from_file(FILE* pFile, pal::string_t& line) + { + line = pal::string_t(); + char buffer[256]; + while (fgets(buffer, sizeof(buffer), pFile)) + { + line += (pal::char_t*)buffer; + size_t len = line.length(); + + // fgets includes the newline character in the string - so remove it. + if (len > 0 && line[len - 1] == '\n') + { + line.pop_back(); + break; + } + } + + return !line.empty(); + } +} + bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) { recv->clear(); @@ -425,7 +448,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) int current_line = 0; bool is_first_line = true, install_location_found = false; - while (pal::get_line_from_file(install_location_file, install_location)) + while (get_line_from_file(install_location_file, install_location)) { current_line++; size_t arch_sep = install_location.find(_X('=')); @@ -451,7 +474,7 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv) pal::string_t path_to_location = install_location.substr(arch_sep + 1); trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str()); - if (strcasecmp(arch_prefix, get_arch()) == 0) + if (pal::strcasecmp(arch_prefix.c_str(), get_arch()) == 0) { recv->assign(path_to_location); install_location_found = true; @@ -492,29 +515,6 @@ bool pal::get_default_installation_dir(pal::string_t* recv) return true; } -namespace -{ - bool pal::get_line_from_file(FILE* pFile, pal::string_t& line) - { - line = pal::string_t(); - char buffer[256]; - while (fgets(buffer, sizeof(buffer), pFile)) - { - line += (pal::char_t*)buffer; - size_t len = line.length(); - - // fgets includes the newline character in the string - so remove it. - if (len > 0 && line[len - 1] == '\n') - { - line.pop_back(); - break; - } - } - - return !line.empty(); - } -} - pal::string_t trim_quotes(pal::string_t stringToCleanup) { pal::char_t quote_array[2] = { '\"', '\'' }; From c9ad20720d20c348a384a4494ff91c69af4751c1 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 17 Jun 2021 14:45:00 -0700 Subject: [PATCH 20/23] Test --- .../InstallLocationCommandResultExtensions.cs | 2 +- .../tests/HostActivation.Tests/MultiArchInstallLocation.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs index b31d93160f4b4a..b1eb79a3e24c3e 100644 --- a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs +++ b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs @@ -12,7 +12,7 @@ namespace HostActivation.Tests { internal static class InstallLocationCommandResultExtensions { - private static bool IsRunningInWoW64() => OperatingSystem.IsWindows() && !Environment.Is64BitProcess && Environment.Is64BitOperatingSystem; + private static bool IsRunningInWoW64() => OperatingSystem.IsWindows() && !Environment.Is64BitProcess; public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string installLocation) { diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 7a8956953d2a3c..7156b0637420cc 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -89,7 +89,7 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { (string.Empty, path1), (arch1, path1), - (arch2, path2) + (arch2, path2)EnvironmentVariable_IfNoArchSpecificEnvVarIsFoundDotnetR }); Command.Create(appExe) @@ -99,7 +99,6 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() .Execute() .Should().HaveFoundDefaultInstallLocationInConfigFile(path1) .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch1, path1) - .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch2, path1) .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch2, path2) .And.HaveUsedGlobalInstallLocation(path2); } From 6ae64aa63cda4964b2961180a7f08fcf06df665d Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 17 Jun 2021 14:47:23 -0700 Subject: [PATCH 21/23] Remove nonsense string --- .../tests/HostActivation.Tests/MultiArchInstallLocation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 7156b0637420cc..790dde3c0beda5 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -89,7 +89,7 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] { (string.Empty, path1), (arch1, path1), - (arch2, path2)EnvironmentVariable_IfNoArchSpecificEnvVarIsFoundDotnetR + (arch2, path2) }); Command.Create(appExe) From a04b84667d799ef8410cbe660fa350f20d951970 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 18 Jun 2021 11:47:24 -0700 Subject: [PATCH 22/23] Put optional arguments last Check the rid of the launched process --- .../InstallLocationCommandResultExtensions.cs | 14 +++++++++----- .../MultiArchInstallLocation.cs | 8 ++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs index b1eb79a3e24c3e..71d2616007c669 100644 --- a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs +++ b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs @@ -12,16 +12,20 @@ namespace HostActivation.Tests { internal static class InstallLocationCommandResultExtensions { - private static bool IsRunningInWoW64() => OperatingSystem.IsWindows() && !Environment.Is64BitProcess; + private static bool IsRunningInWoW64(string rid) => OperatingSystem.IsWindows() && Environment.Is64BitOperatingSystem && rid.Equals("win-x86"); public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string installLocation) { - return assertion.HaveUsedDotNetRootInstallLocation(null, installLocation); + return assertion.HaveUsedDotNetRootInstallLocation(installLocation, null, null); } - public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string arch, string installLocation) + public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, + string installLocation, + string arch, + string rid) { - string expectedEnvironmentVariable = !string.IsNullOrEmpty(arch) ? $"DOTNET_ROOT_{arch.ToUpper()}" : IsRunningInWoW64() ? "DOTNET_ROOT(x86)" : "DOTNET_ROOT"; + string expectedEnvironmentVariable = !string.IsNullOrEmpty(arch) ? $"DOTNET_ROOT_{arch.ToUpper()}" : + IsRunningInWoW64(rid) ? "DOTNET_ROOT(x86)" : "DOTNET_ROOT"; return assertion.HaveStdErrContaining($"Using environment variable {expectedEnvironmentVariable}=[{installLocation}] as runtime location."); } @@ -41,7 +45,7 @@ public static AndConstraint HaveFoundDefaultInstallLoca return assertion.HaveStdErrContaining($"Found install location path '{installLocation}'."); } - public static AndConstraint HaveFoundArchSpecificInstallLocationInConfigFile(this CommandResultAssertions assertion, string arch, string installLocation) + public static AndConstraint HaveFoundArchSpecificInstallLocationInConfigFile(this CommandResultAssertions assertion, string installLocation, string arch) { return assertion.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}' ('{arch}')."); } diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index 790dde3c0beda5..d12efb7a5cfbda 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -33,7 +33,7 @@ public void EnvironmentVariable_CurrentArchitectureIsUsedIfEnvVarSet() .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) .Execute() .Should().Pass() - .And.HaveUsedDotNetRootInstallLocation(arch, fixture.BuiltDotnet.BinPath); + .And.HaveUsedDotNetRootInstallLocation(fixture.BuiltDotnet.BinPath, arch, fixture.CurrentRid); } [Fact] @@ -67,7 +67,7 @@ public void EnvironmentVariable_ArchSpecificDotnetRootIsUsedOverDotnetRoot() .DotNetRoot(dotnet, arch) .Execute() .Should().Pass() - .And.HaveUsedDotNetRootInstallLocation(arch, dotnet) + .And.HaveUsedDotNetRootInstallLocation(dotnet, arch, fixture.CurrentRid) .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); } @@ -98,8 +98,8 @@ public void InstallLocationFile_ArchSpecificLocationIsPickedFirst() .DotNetRoot(null) .Execute() .Should().HaveFoundDefaultInstallLocationInConfigFile(path1) - .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch1, path1) - .And.HaveFoundArchSpecificInstallLocationInConfigFile(arch2, path2) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(path1, arch1) + .And.HaveFoundArchSpecificInstallLocationInConfigFile(path2, arch2) .And.HaveUsedGlobalInstallLocation(path2); } } From 2402b617117bc9060ffbb70dd3bd6ebe9b9cb73d Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 18 Jun 2021 13:21:10 -0700 Subject: [PATCH 23/23] Pass RID --- .../InstallLocationCommandResultExtensions.cs | 13 +++++++++---- .../MultiArchInstallLocation.cs | 6 +++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs index 71d2616007c669..c85b7d3e56cc5d 100644 --- a/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs +++ b/src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs @@ -7,6 +7,7 @@ using FluentAssertions; using Microsoft.DotNet.Cli.Build.Framework; using Microsoft.DotNet.CoreSetup.Test; +using Xunit; namespace HostActivation.Tests { @@ -14,16 +15,20 @@ internal static class InstallLocationCommandResultExtensions { private static bool IsRunningInWoW64(string rid) => OperatingSystem.IsWindows() && Environment.Is64BitOperatingSystem && rid.Equals("win-x86"); - public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string installLocation) + public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string installLocation, string rid) { - return assertion.HaveUsedDotNetRootInstallLocation(installLocation, null, null); + return assertion.HaveUsedDotNetRootInstallLocation(installLocation, rid, null); } public static AndConstraint HaveUsedDotNetRootInstallLocation(this CommandResultAssertions assertion, string installLocation, - string arch, - string rid) + string rid, + string arch) { + // If no arch is passed and we are on Windows, we need the used RID for determining whether or not we are running on WoW64. + if (string.IsNullOrEmpty(arch)) + Assert.NotNull(rid); + string expectedEnvironmentVariable = !string.IsNullOrEmpty(arch) ? $"DOTNET_ROOT_{arch.ToUpper()}" : IsRunningInWoW64(rid) ? "DOTNET_ROOT(x86)" : "DOTNET_ROOT"; diff --git a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs index d12efb7a5cfbda..c1b00cafd4e6ae 100644 --- a/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs +++ b/src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs @@ -33,7 +33,7 @@ public void EnvironmentVariable_CurrentArchitectureIsUsedIfEnvVarSet() .DotNetRoot(fixture.BuiltDotnet.BinPath, arch) .Execute() .Should().Pass() - .And.HaveUsedDotNetRootInstallLocation(fixture.BuiltDotnet.BinPath, arch, fixture.CurrentRid); + .And.HaveUsedDotNetRootInstallLocation(fixture.BuiltDotnet.BinPath, fixture.CurrentRid, arch); } [Fact] @@ -49,7 +49,7 @@ public void EnvironmentVariable_IfNoArchSpecificEnvVarIsFoundDotnetRootIsUsed() .DotNetRoot(fixture.BuiltDotnet.BinPath) .Execute() .Should().Pass() - .And.HaveUsedDotNetRootInstallLocation(fixture.BuiltDotnet.BinPath); + .And.HaveUsedDotNetRootInstallLocation(fixture.BuiltDotnet.BinPath, fixture.CurrentRid); } [Fact] @@ -67,7 +67,7 @@ public void EnvironmentVariable_ArchSpecificDotnetRootIsUsedOverDotnetRoot() .DotNetRoot(dotnet, arch) .Execute() .Should().Pass() - .And.HaveUsedDotNetRootInstallLocation(dotnet, arch, fixture.CurrentRid) + .And.HaveUsedDotNetRootInstallLocation(dotnet, fixture.CurrentRid, arch) .And.NotHaveStdErrContaining("Using environment variable DOTNET_ROOT="); }