Split install_location to file-per-architecture (#59404)
authorVitek Karas <vitek.karas@microsoft.com>
Wed, 22 Sep 2021 17:04:00 +0000 (10:04 -0700)
committerGitHub <noreply@github.com>
Wed, 22 Sep 2021 17:04:00 +0000 (10:04 -0700)
On non-Windows we used install_location file with multiple lines which contained locations per-architecture (and the default).

This changes the host to use separate file for each architecture:
* `install_location_<arch>` for the architecture specific
* `install_location` for the "legacy" file

This is a much easier format for installers to handle (and it also simplifies the apphost code a little bit).

Adapted tests to the new behavior. Removed some tests which don't make sense anymore. Some small test cleanup to deduplicate code.

Changed the test-only env. variable to point to the directory with configuration files instead of to the configuration file itself.

src/installer/tests/HostActivation.Tests/Constants.cs
src/installer/tests/HostActivation.Tests/InstallLocationCommandResultExtensions.cs
src/installer/tests/HostActivation.Tests/MultiArchInstallLocation.cs
src/installer/tests/HostActivation.Tests/NativeHosting/Nethost.cs
src/installer/tests/HostActivation.Tests/RegisteredInstallLocationOverride.cs
src/native/corehost/hostmisc/pal.unix.cpp

index f7e743a..25768a5 100644 (file)
@@ -53,7 +53,7 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             public const string DefaultInstallPath = "_DOTNET_TEST_DEFAULT_INSTALL_PATH";
             public const string RegistryPath = "_DOTNET_TEST_REGISTRY_PATH";
             public const string GloballyRegisteredPath = "_DOTNET_TEST_GLOBALLY_REGISTERED_PATH";
-            public const string InstallLocationFilePath = "_DOTNET_TEST_INSTALL_LOCATION_FILE_PATH";
+            public const string InstallLocationPath = "_DOTNET_TEST_INSTALL_LOCATION_PATH";
         }
 
         public static class RuntimeId
index c85b7d3..c4a8438 100644 (file)
@@ -3,6 +3,7 @@
 
 
 using System;
+using System.IO;
 using System.Runtime.InteropServices;
 using FluentAssertions;
 using Microsoft.DotNet.Cli.Build.Framework;
@@ -45,14 +46,14 @@ namespace HostActivation.Tests
             return assertion.HaveStdErrContaining($"Using global installation location [{installLocation}]");
         }
 
-        public static AndConstraint<CommandResultAssertions> HaveFoundDefaultInstallLocationInConfigFile(this CommandResultAssertions assertion, string installLocation)
+        public static AndConstraint<CommandResultAssertions> HaveLookedForDefaultInstallLocation(this CommandResultAssertions assertion, string installLocationPath)
         {
-            return assertion.HaveStdErrContaining($"Found install location path '{installLocation}'.");
+            return assertion.HaveStdErrContaining($"Looking for install_location file in '{Path.Combine(installLocationPath, "install_location")}'.");
         }
 
-        public static AndConstraint<CommandResultAssertions> HaveFoundArchSpecificInstallLocationInConfigFile(this CommandResultAssertions assertion, string installLocation, string arch)
+        public static AndConstraint<CommandResultAssertions> HaveLookedForArchitectureSpecificInstallLocation(this CommandResultAssertions assertion, string installLocationPath, string architecture)
         {
-            return assertion.HaveStdErrContaining($"Found architecture-specific install location path: '{installLocation}' ('{arch}').");
+            return assertion.HaveStdErrContaining($"Looking for architecture specific install_location file in '{Path.Combine(installLocationPath, "install_location_" + architecture.ToLowerInvariant())}'.");
         }
     }
 }
index 97de522..3829a30 100644 (file)
@@ -174,9 +174,8 @@ namespace HostActivation.Tests
 
                 if (!OperatingSystem.IsWindows())
                 {
-                    result.Should().HaveFoundDefaultInstallLocationInConfigFile(path1)
-                        .And.HaveFoundArchSpecificInstallLocationInConfigFile(path1, arch1)
-                        .And.HaveFoundArchSpecificInstallLocationInConfigFile(path2, arch2);
+                    result.Should()
+                        .HaveLookedForArchitectureSpecificInstallLocation(registeredInstallLocationOverride.PathValueOverride, arch2);
                 }
 
                 result.Should().HaveUsedGlobalInstallLocation(path2);
@@ -185,31 +184,6 @@ namespace HostActivation.Tests
 
         [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.PortableAppFixture
-                .Copy();
-
-            var appExe = fixture.TestProject.AppExe;
-            using (var registeredInstallLocationOverride = new RegisteredInstallLocationOverride(appExe))
-            {
-                registeredInstallLocationOverride.SetInstallLocation(new (string, string)[] {
-                    (string.Empty, "a/b/c"),
-                    (string.Empty, "x/y/z"),
-                });
-                Command.Create(appExe)
-                    .EnableTracingAndCaptureOutputs()
-                    .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride)
-                    .DotNetRoot(null)
-                    .Execute()
-                    .Should().HaveFoundDefaultInstallLocationInConfigFile("a/b/c")
-                    .And.HaveStdErrContaining($"Only the first line in '{registeredInstallLocationOverride.PathValueOverride}' may not have an architecture prefix.")
-                    .And.HaveUsedConfigFileInstallLocation("a/b/c");
-            }
-        }
-
-        [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
@@ -229,7 +203,7 @@ namespace HostActivation.Tests
                     .ApplyRegisteredInstallLocationOverride(registeredInstallLocationOverride)
                     .DotNetRoot(null)
                     .Execute()
-                    .Should().HaveFoundDefaultInstallLocationInConfigFile(reallyLongPath)
+                    .Should().HaveLookedForDefaultInstallLocation(registeredInstallLocationOverride.PathValueOverride)
                     .And.HaveUsedConfigFileInstallLocation(reallyLongPath);
             }
         }
@@ -247,16 +221,15 @@ namespace HostActivation.Tests
             {
                 Directory.CreateDirectory(testArtifactsPath);
 
-                string directory = Path.Combine(testArtifactsPath, "installLocationOverride");
-                Directory.CreateDirectory(directory);
-                string nonExistentLocationFile = Path.Combine(directory, "install_location");
+                string installLocationDirectory = Path.Combine(testArtifactsPath, "installLocationOverride");
+                Directory.CreateDirectory(installLocationDirectory);
                 string defaultInstallLocation = Path.Combine(testArtifactsPath, "defaultInstallLocation");
 
                 Command.Create(appExe)
                     .CaptureStdErr()
                     .EnvironmentVariable(
-                        Constants.TestOnlyEnvironmentVariables.InstallLocationFilePath,
-                        nonExistentLocationFile)
+                        Constants.TestOnlyEnvironmentVariables.InstallLocationPath,
+                        installLocationDirectory)
                     .EnvironmentVariable(
                         Constants.TestOnlyEnvironmentVariables.DefaultInstallPath,
                         defaultInstallLocation)
index 0af2ce3..9a90ad8 100644 (file)
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using HostActivation.Tests;
 using Microsoft.DotNet.Cli.Build.Framework;
 using System;
 using System.IO;
@@ -216,12 +217,21 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
                     .DotNetRoot(null)
                     .Execute();
 
-                result.Should().HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.");
+                if (shouldUseArchSpecificInstallLocation)
+                {
+                    result.Should().HaveLookedForArchitectureSpecificInstallLocation(
+                        registeredInstallLocationOverride.PathValueOverride,
+                        sharedState.RepoDirectories.BuildArchitecture);
+                }
+                else
+                {
+                    result.Should().HaveLookedForDefaultInstallLocation(registeredInstallLocationOverride.PathValueOverride);
+                }
 
                 if (shouldPass)
                 {
                     result.Should().Pass()
-                        .And.HaveStdErrContaining($"Using install location '{installLocation}'.")
+                        .And.HaveUsedConfigFileInstallLocation(installLocation)
                         .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower());
                 }
                 else
@@ -236,35 +246,6 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
 
         [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, string)[] {
-                    (string.Empty, installLocation), (string.Empty, 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.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());
-            }
-        }
-
-        [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");
@@ -285,9 +266,10 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
                     .Execute();
 
                 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($"Using install location '{installLocation}'.")
+                    .And.HaveLookedForArchitectureSpecificInstallLocation(
+                        registeredInstallLocationOverride.PathValueOverride,
+                        sharedState.RepoDirectories.BuildArchitecture)
+                    .And.HaveUsedConfigFileInstallLocation(installLocation)
                     .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower());
             }
         }
@@ -314,10 +296,10 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
                     .Execute();
 
                 result.Should().Pass()
-                    .And.HaveStdErrContaining($"Looking for install_location file in '{registeredInstallLocationOverride.PathValueOverride}'.")
-                    .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.HaveLookedForArchitectureSpecificInstallLocation(
+                        registeredInstallLocationOverride.PathValueOverride,
+                        sharedState.RepoDirectories.BuildArchitecture)
+                    .And.HaveUsedConfigFileInstallLocation(installLocation)
                     .And.HaveStdOutContaining($"hostfxr_path: {sharedState.HostFxrPath}".ToLower());
             }
         }
index d37cfaf..fcb2947 100644 (file)
@@ -56,10 +56,11 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
                 // On Linux/macOS the install location is registered in a file which is normally
                 // located in /etc/dotnet/install_location
                 // So we need to redirect it to a different place here.
-                string directory = Path.Combine(TestArtifact.TestArtifactsPath, "installLocationOverride");
+                string directory = Path.Combine(TestArtifact.TestArtifactsPath, "installLocationOverride" + Process.GetCurrentProcess().Id.ToString());
+                if (Directory.Exists(directory))
+                    Directory.Delete(directory, true);
                 Directory.CreateDirectory(directory);
-                PathValueOverride = Path.Combine(directory, "install_location." + Process.GetCurrentProcess().Id.ToString());
-                File.WriteAllText(PathValueOverride, "");
+                PathValueOverride = directory;
             }
         }
 
@@ -78,9 +79,12 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             }
             else
             {
-                File.WriteAllText(PathValueOverride, string.Join(Environment.NewLine,
-                    locations.Select(l => string.Format("{0}{1}",
-                        (!string.IsNullOrWhiteSpace(l.Architecture) ? l.Architecture + "=" : ""), l.Path))));
+                foreach (var location in locations)
+                {
+                    string installLocationFileName = "install_location" +
+                        (!string.IsNullOrWhiteSpace(location.Architecture) ? ("_" + location.Architecture) : string.Empty);
+                    File.WriteAllText(Path.Combine(PathValueOverride, installLocationFileName), location.Path);
+                }
             }
         }
 
@@ -127,7 +131,7 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             else
             {
                 return command.EnvironmentVariable(
-                    Constants.TestOnlyEnvironmentVariables.InstallLocationFilePath,
+                    Constants.TestOnlyEnvironmentVariables.InstallLocationPath,
                     registeredInstallLocationOverride.PathValueOverride);
             }
         }
index a09e6ea..54d98f3 100644 (file)
@@ -418,12 +418,12 @@ pal::string_t pal::get_dotnet_self_registered_config_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))
+    if (test_only_getenv(_X("_DOTNET_TEST_INSTALL_LOCATION_PATH"), &environment_install_location_override))
     {
         return environment_install_location_override;
     }
 
-    return _X("/etc/dotnet/install_location");
+    return _X("/etc/dotnet");
 }
 
 namespace
@@ -449,6 +449,42 @@ namespace
     }
 }
 
+bool get_install_location_from_file(const pal::string_t& file_path, bool& file_found, pal::string_t& install_location)
+{
+    file_found = true;
+    bool install_location_found = false;
+    FILE* install_location_file = pal::file_open(file_path, "r");
+    if (install_location_file != nullptr)
+    {
+        if (!get_line_from_file(install_location_file, install_location))
+        {
+            trace::warning(_X("Did not find any install location in '%s'."), file_path.c_str());
+        }
+        else
+        {
+            install_location_found = true;
+        }
+
+        fclose(install_location_file);
+        if (install_location_found)
+            return true;
+    }
+    else
+    {
+        if (errno == ENOENT)
+        {
+            trace::verbose(_X("The install_location file ['%s'] does not exist - skipping."), file_path.c_str());
+            file_found = false;
+        }
+        else
+        {
+            trace::error(_X("The install_location file ['%s'] failed to open: %s."), file_path.c_str(), pal::strerror(errno));
+        }
+    }
+
+    return false;
+}
+
 bool pal::get_dotnet_self_registered_dir(pal::string_t* recv)
 {
     recv->clear();
@@ -462,71 +498,31 @@ bool pal::get_dotnet_self_registered_dir(pal::string_t* recv)
     }
     //  ***************************
 
-    pal::string_t install_location_file_path = get_dotnet_self_registered_config_location();
-    trace::verbose(_X("Looking for install_location file in '%s'."), install_location_file_path.c_str());
-    FILE* install_location_file = pal::file_open(install_location_file_path, "r");
-    if (install_location_file == nullptr)
-    {
-        if (errno == ENOENT)
-        {
-            trace::verbose(_X("The install_location file ['%s'] does not exist - skipping."), install_location_file_path.c_str());
-        }
-        else
-        {
-            trace::error(_X("The install_location file ['%s'] failed to open: %s."), install_location_file_path.c_str(), pal::strerror(errno));
-        }
-
-        return false;
-    }
+    pal::string_t install_location_path = get_dotnet_self_registered_config_location();
+    pal::string_t arch_specific_install_location_file_path = install_location_path;
+    append_path(&arch_specific_install_location_file_path, (_X("install_location_") + to_lower(get_arch())).c_str());
+    trace::verbose(_X("Looking for architecture specific install_location file in '%s'."), arch_specific_install_location_file_path.c_str());
 
     pal::string_t install_location;
-    int current_line = 0;
-    bool is_first_line = true, install_location_found = false;
-
-    while (get_line_from_file(install_location_file, install_location))
+    bool file_found = false;
+    if (!get_install_location_from_file(arch_specific_install_location_file_path, file_found, install_location))
     {
-        current_line++;
-        size_t arch_sep = install_location.find(_X('='));
-        if (arch_sep == pal::string_t::npos)
+        if (file_found)
         {
-            if (is_first_line)
-            {
-                recv->assign(install_location);
-                install_location_found = true;
-                trace::verbose(_X("Found install location path '%s'."), install_location.c_str());
-            }
-            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());
-            }
-
-            is_first_line = false;
-            continue;
+            return false;
         }
 
-        pal::string_t arch_prefix = install_location.substr(0, arch_sep);
-        pal::string_t path_to_location = install_location.substr(arch_sep + 1);
+        pal::string_t legacy_install_location_file_path = install_location_path;
+        append_path(&legacy_install_location_file_path, _X("install_location"));
+        trace::verbose(_X("Looking for install_location file in '%s'."), legacy_install_location_file_path.c_str());
 
-        trace::verbose(_X("Found architecture-specific install location path: '%s' ('%s')."), path_to_location.c_str(), arch_prefix.c_str());
-        if (pal::strcasecmp(arch_prefix.c_str(), get_arch()) == 0)
+        if (!get_install_location_from_file(legacy_install_location_file_path, file_found, install_location))
         {
-            recv->assign(path_to_location);
-            install_location_found = true;
-            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;
+            return false;
         }
-
-        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());
-        return false;
     }
 
+    recv->assign(install_location);
     trace::verbose(_X("Using install location '%s'."), recv->c_str());
     return true;
 }