From 31e1a15531c1dcea538bdd081d631f530f0c18e1 Mon Sep 17 00:00:00 2001 From: Ryan Fu <69221034+ryfu-msft@users.noreply.github.com> Date: Thu, 28 Jul 2022 08:23:28 -0700 Subject: [PATCH] Remove scope filter from being applied to portables (#2383) --- doc/Settings.md | 31 +++++++++++ .../JSON/settings/settings.schema.0.2.json | 4 +- .../Workflows/ManifestComparator.cpp | 4 +- .../Workflows/PortableFlow.cpp | 30 ++++++++++- src/AppInstallerCLIE2ETests/BaseCommand.cs | 10 ++++ src/AppInstallerCLIE2ETests/Constants.cs | 11 ++++ src/AppInstallerCLIE2ETests/InstallCommand.cs | 40 +++++++++++++- src/AppInstallerCLIE2ETests/SetUpFixture.cs | 5 ++ src/AppInstallerCLIE2ETests/TestCommon.cs | 33 ++++++++---- src/AppInstallerCLIE2ETests/UpgradeCommand.cs | 28 ++++++++-- src/AppInstallerCLITests/UserSettings.cpp | 41 +++++++++++++-- src/AppInstallerCLITests/WorkFlow.cpp | 52 ++++++++++++++++--- .../Manifest/Manifest.cpp | 2 +- .../Manifest/ManifestCommon.cpp | 39 ++++++++++++-- .../Manifest/ManifestValidation.cpp | 6 +-- .../Manifest/ManifestYamlPopulator.cpp | 6 +-- .../Public/winget/ManifestCommon.h | 26 +++++++--- .../Public/winget/ManifestInstaller.h | 2 + .../Public/winget/UserSettings.h | 8 +-- src/AppInstallerCommonCore/Runtime.cpp | 16 +++--- src/AppInstallerCommonCore/UserSettings.cpp | 6 +-- 21 files changed, 337 insertions(+), 63 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index b732139c61..38c79e233a 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -49,6 +49,37 @@ Color of the progress bar that WinGet displays when not specified by arguments. The `installBehavior` settings affect the default behavior of installing and upgrading (where applicable) packages. +### Disable Install Notes +The `disableInstallNotes` behavior affects whether installation notes are shown after a successful install. Defaults to `false` if value is not set or is invalid. + +```json + "installBehavior": { + "disableInstallNotes": true + }, +``` + +### Portable Package User Root +The `portablePackageUserRoot` setting affects the default root directory where packages are installed to under `User` scope. This setting only applies to packages with the `portable` installer type. Defaults to `%LOCALAPPDATA%/Microsoft/WinGet/Packages/` if value is not set or is invalid. + +> Note: This setting value must be an absolute path. + +```json + "installBehavior": { + "portablePackageUserRoot": "C:/Users/FooBar/Packages" + }, +``` + +### Portable Package Machine Root +The `portablePackageMachineRoot` setting affects the default root directory where packages are installed to under `Machine` scope. This setting only applies to packages with the `portable` installer type. Defaults to `%PROGRAMFILES%/WinGet/Packages/` if value is not set or is invalid. + +> Note: This setting value must be an absolute path. + +```json + "installBehavior": { + "portablePackageMachineRoot": "C:/Program Files/Packages/Portable" + }, +``` + ### Preferences and Requirements Some of the settings are duplicated under `preferences` and `requirements`. `preferences` affect how the various available options are sorted when choosing the one to act on. For instance, the default scope of package installs is for the current user, but if that is not an option then a machine level installer will be chosen. `requirements` filter the options, potentially resulting in an empty list and a failure to install. In the previous example, a user scope requirement would result in no applicable installers and an error. diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index ac9e508b33..ccefd25810 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -107,12 +107,12 @@ "type": "boolean", "default": false }, - "PortablePackageUserRoot": { + "portablePackageUserRoot": { "description": "The default root directory where packages are installed to under User scope. Applies to the portable installer type.", "type": "string", "default": "%LOCALAPPDATA%/Microsoft/WinGet/Packages/" }, - "PortablePackageMachineRoot": { + "portablePackageMachineRoot": { "description": "The default root directory where packages are installed to under Machine scope. Applies to the portable installer type.", "type": "string", "default": "%PROGRAMFILES%/WinGet/Packages/" diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp index 39c056500d..11a8ccbdee 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -287,7 +287,7 @@ namespace AppInstaller::CLI::Workflow InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override { // We have to assume the unknown scope will match our required scope, or the entire catalog would stop working for upgrade. - if (installer.Scope == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement) + if (installer.Scope == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement || DoesInstallerIgnoreScopeFromManifest(installer)) { return InapplicabilityFlags::None; } @@ -342,7 +342,7 @@ namespace AppInstaller::CLI::Workflow InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override { - if (m_requirement == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement) + if (m_requirement == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement || DoesInstallerIgnoreScopeFromManifest(installer)) { return InapplicabilityFlags::None; } diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp index ed1f745856..b5044848a2 100644 --- a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp @@ -538,12 +538,38 @@ namespace AppInstaller::CLI::Workflow } } } + + void EnsureRunningAsAdminForMachineScopeInstall(Execution::Context& context) + { + // Admin is required for machine scope install or else creating a symlink in the %PROGRAMFILES% link location will fail. + Manifest::ScopeEnum scope = ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); + if (scope == Manifest::ScopeEnum::Machine) + { + context << Workflow::EnsureRunningAsAdmin; + } + } } void PortableInstallImpl(Execution::Context& context) { + Manifest::ScopeEnum scope = Manifest::ScopeEnum::Unknown; + bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); + if (isUpdate) + { + IPackageVersion::Metadata installationMetadata = context.Get()->GetMetadata(); + auto installerScopeItr = installationMetadata.find(Repository::PackageVersionMetadata::InstalledScope); + if (installerScopeItr != installationMetadata.end()) + { + scope = Manifest::ConvertToScopeEnum(installerScopeItr->second); + } + } + else + { + scope = ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)); + } + PortableARPEntry uninstallEntry = PortableARPEntry( - ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope)), + scope, context.Get()->Arch, GetPortableProductCode(context)); @@ -571,7 +597,6 @@ namespace AppInstaller::CLI::Workflow // Perform cleanup only if the install fails and is not an update. const auto& installReturnCode = context.Get(); - bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); if (installReturnCode != 0 && installReturnCode != APPINSTALLER_CLI_ERROR_PORTABLE_PACKAGE_ALREADY_EXISTS && !isUpdate) { @@ -616,6 +641,7 @@ namespace AppInstaller::CLI::Workflow { context << EnsureSymlinkCreationPrivilege << + EnsureRunningAsAdminForMachineScopeInstall << EnsureValidArgsForPortableInstall << EnsureVolumeSupportsReparsePoints; } diff --git a/src/AppInstallerCLIE2ETests/BaseCommand.cs b/src/AppInstallerCLIE2ETests/BaseCommand.cs index cc9a27f917..ca23e9727f 100644 --- a/src/AppInstallerCLIE2ETests/BaseCommand.cs +++ b/src/AppInstallerCLIE2ETests/BaseCommand.cs @@ -42,6 +42,16 @@ public void ConfigureFeature(string featureName, bool status) File.WriteAllText(Path.Combine(localAppDataPath, TestCommon.SettingsJsonFilePath), settingsJson.ToString()); } + public void ConfigureInstallBehavior(string settingName, string value) + { + string localAppDataPath = Environment.GetEnvironmentVariable(Constants.LocalAppData); + JObject settingsJson = JObject.Parse(File.ReadAllText(Path.Combine(localAppDataPath, TestCommon.SettingsJsonFilePath))); + JObject installBehavior = (JObject)settingsJson["installBehavior"]; + installBehavior[settingName] = value; + + File.WriteAllText(Path.Combine(localAppDataPath, TestCommon.SettingsJsonFilePath), settingsJson.ToString()); + } + public void InitializeAllFeatures(bool status) { ConfigureFeature("experimentalArg", status); diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs index daf66592ab..f7f991b5d6 100644 --- a/src/AppInstallerCLIE2ETests/Constants.cs +++ b/src/AppInstallerCLIE2ETests/Constants.cs @@ -58,6 +58,17 @@ public class Constants // Locations public const string LocalAppData = "LocalAppData"; + // Registry keys + public const string WinGetPackageIdentifier = "WinGetPackageIdentifier"; + public const string WinGetSourceIdentifier = "WinGetSourceIdentifier"; + public const string UninstallSubKey = @"Software\Microsoft\Windows\CurrentVersion\Uninstall"; + public const string PathSubKey_User = @"Environment"; + public const string PathSubKey_Machine = @"SYSTEM\CurrentControlSet\Control\Session Manager\Environment"; + + // User settings + public const string PortablePackageUserRoot = "portablePackageUserRoot"; + public const string PortablePackageMachineRoot = "portablePackageMachineRoot"; + public class ErrorCode { public const int S_OK = 0; diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs index f7e6580657..ab1ce157e1 100644 --- a/src/AppInstallerCLIE2ETests/InstallCommand.cs +++ b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -257,7 +257,7 @@ public void InstallPortableFailsWithCleanup() commandAlias = fileName = "AppInstallerTestExeInstaller.exe"; // Create a directory with the same name as the symlink in order to cause install to fail. - string symlinkDirectory = TestCommon.GetPortableSymlinkDirectory(); + string symlinkDirectory = TestCommon.GetPortableSymlinkDirectory(TestCommon.Scope.User); string conflictDirectory = Path.Combine(symlinkDirectory, commandAlias); Directory.CreateDirectory(conflictDirectory); @@ -283,7 +283,7 @@ public void ReinstallPortable() var result = TestCommon.RunAICLICommand("install", "AppInstallerTest.TestPortableExe"); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); - string symlinkDirectory = TestCommon.GetPortableSymlinkDirectory(); + string symlinkDirectory = TestCommon.GetPortableSymlinkDirectory(TestCommon.Scope.User); string symlinkPath = Path.Combine(symlinkDirectory, commandAlias); // Clean first install should not display file overwrite message. @@ -299,6 +299,42 @@ public void ReinstallPortable() TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true); } + [Test] + public void InstallPortable_UserScope() + { + string installDir = TestCommon.GetRandomTestDir(); + ConfigureInstallBehavior(Constants.PortablePackageUserRoot, installDir); + + string packageId, commandAlias, fileName, packageDirName, productCode; + packageId = "AppInstallerTest.TestPortableExe"; + packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; + commandAlias = fileName = "AppInstallerTestExeInstaller.exe"; + + var result = TestCommon.RunAICLICommand("install", "AppInstallerTest.TestPortableExe --scope user"); + ConfigureInstallBehavior(Constants.PortablePackageUserRoot, string.Empty); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + Assert.True(result.StdOut.Contains("Successfully installed")); + TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true); + } + + [Test] + public void InstallPortable_MachineScope() + { + string installDir = TestCommon.GetRandomTestDir(); + ConfigureInstallBehavior(Constants.PortablePackageMachineRoot, installDir); + + string packageId, commandAlias, fileName, packageDirName, productCode; + packageId = "AppInstallerTest.TestPortableExe"; + packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; + commandAlias = fileName = "AppInstallerTestExeInstaller.exe"; + + var result = TestCommon.RunAICLICommand("install", "AppInstallerTest.TestPortableExe --scope machine"); + ConfigureInstallBehavior(Constants.PortablePackageMachineRoot, string.Empty); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + Assert.True(result.StdOut.Contains("Successfully installed")); + TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true, TestCommon.Scope.Machine); + } + private bool VerifyTestExeInstalled(string installDir, string expectedContent = null) { if (!File.Exists(Path.Combine(installDir, Constants.TestExeInstalledFileName))) diff --git a/src/AppInstallerCLIE2ETests/SetUpFixture.cs b/src/AppInstallerCLIE2ETests/SetUpFixture.cs index 491af06af5..5653671b1d 100644 --- a/src/AppInstallerCLIE2ETests/SetUpFixture.cs +++ b/src/AppInstallerCLIE2ETests/SetUpFixture.cs @@ -215,6 +215,11 @@ public void InitializeWingetSettings() debugging = new { enableSelfInitiatedMinidump = true + }, + installBehavior = new + { + portablePackageUserRoot = "", + portablePackageMachineRoot = "", } }; diff --git a/src/AppInstallerCLIE2ETests/TestCommon.cs b/src/AppInstallerCLIE2ETests/TestCommon.cs index a90c8db85f..6e3d612a0c 100644 --- a/src/AppInstallerCLIE2ETests/TestCommon.cs +++ b/src/AppInstallerCLIE2ETests/TestCommon.cs @@ -8,7 +8,6 @@ namespace AppInstallerCLIE2ETests using System; using System.Diagnostics; using System.IO; - using System.Linq; using System.Threading; public class TestCommon @@ -44,6 +43,12 @@ public static string SettingsJsonFilePath { } } + public enum Scope + { + User, + Machine + } + public struct RunCommandResult { public int ExitCode; @@ -278,9 +283,16 @@ public static bool RemoveMsix(string name) return RunCommand("powershell", $"Get-AppxPackage \"{name}\" | Remove-AppxPackage"); } - public static string GetPortableSymlinkDirectory() + public static string GetPortableSymlinkDirectory(Scope scope) { - return Path.Combine(Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Links"); + if (scope == Scope.User) + { + return Path.Combine(Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Links"); + } + else + { + return Path.Combine(Environment.GetEnvironmentVariable("ProgramFiles"), "WinGet", "Links"); + } } public static string GetPortablePackagesDirectory() @@ -293,25 +305,28 @@ public static void VerifyPortablePackage( string commandAlias, string filename, string productCode, - bool shouldExist) + bool shouldExist, + Scope scope = Scope.User) { string exePath = Path.Combine(installDir, filename); bool exeExists = File.Exists(exePath); - string symlinkDirectory = GetPortableSymlinkDirectory(); + string symlinkDirectory = GetPortableSymlinkDirectory(scope); string symlinkPath = Path.Combine(symlinkDirectory, commandAlias); bool symlinkExists = File.Exists(symlinkPath); bool portableEntryExists; - string subKey = @$"Software\Microsoft\Windows\CurrentVersion\Uninstall"; - using (RegistryKey uninstallRegistryKey = Registry.CurrentUser.OpenSubKey(subKey, true)) + RegistryKey baseKey = (scope == Scope.User) ? Registry.CurrentUser : Registry.LocalMachine; + string uninstallSubKey = Constants.UninstallSubKey; + using (RegistryKey uninstallRegistryKey = baseKey.OpenSubKey(uninstallSubKey, true)) { RegistryKey portableEntry = uninstallRegistryKey.OpenSubKey(productCode, true); portableEntryExists = portableEntry != null; } bool isAddedToPath; - using (RegistryKey environmentRegistryKey = Registry.CurrentUser.OpenSubKey(@"Environment", true)) + string pathSubKey = (scope == Scope.User) ? Constants.PathSubKey_User : Constants.PathSubKey_Machine; + using (RegistryKey environmentRegistryKey = baseKey.OpenSubKey(pathSubKey, true)) { string pathName = "Path"; var currentPathValue = (string)environmentRegistryKey.GetValue(pathName); @@ -326,7 +341,7 @@ public static void VerifyPortablePackage( Assert.AreEqual(shouldExist, exeExists, $"Expected portable exe path: {exePath}"); Assert.AreEqual(shouldExist, symlinkExists, $"Expected portable symlink path: {symlinkPath}"); - Assert.AreEqual(shouldExist, portableEntryExists, $"Expected {productCode} subkey in path: {subKey}"); + Assert.AreEqual(shouldExist, portableEntryExists, $"Expected {productCode} subkey in path: {uninstallSubKey}"); Assert.AreEqual(shouldExist, isAddedToPath, $"Expected path variable: {symlinkDirectory}"); } diff --git a/src/AppInstallerCLIE2ETests/UpgradeCommand.cs b/src/AppInstallerCLIE2ETests/UpgradeCommand.cs index 08e1979a8d..b0222f0b88 100644 --- a/src/AppInstallerCLIE2ETests/UpgradeCommand.cs +++ b/src/AppInstallerCLIE2ETests/UpgradeCommand.cs @@ -22,7 +22,7 @@ public void OneTimeSetup() [Test] public void UpgradePortable() { - string installDir = Path.Combine(System.Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Packages"); + string installDir = TestCommon.GetPortablePackagesDirectory(); string packageId, commandAlias, fileName, packageDirName, productCode; packageId = "AppInstallerTest.TestPortableExe"; packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; @@ -64,7 +64,7 @@ public void UpgradePortableARPMismatch() [Test] public void UpgradePortableForcedOverride() { - string installDir = Path.Combine(System.Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Packages"); + string installDir = TestCommon.GetPortablePackagesDirectory(); string packageId, commandAlias, fileName, packageDirName, productCode; packageId = "AppInstallerTest.TestPortableExe"; packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; @@ -87,7 +87,7 @@ public void UpgradePortableForcedOverride() [Test] public void UpgradePortableUninstallPrevious() { - string installDir = Path.Combine(System.Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Packages"); + string installDir = TestCommon.GetPortablePackagesDirectory(); string packageId, commandAlias, fileName, packageDirName, productCode; packageId = "AppInstallerTest.TestPortableExe"; packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; @@ -103,6 +103,28 @@ public void UpgradePortableUninstallPrevious() TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true); } + [Test] + public void UpgradePortableMachineScope() + { + string installDir = TestCommon.GetRandomTestDir(); + ConfigureInstallBehavior(Constants.PortablePackageMachineRoot, installDir); + + string packageId, commandAlias, fileName, packageDirName, productCode; + packageId = "AppInstallerTest.TestPortableExe"; + packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; + commandAlias = fileName = "AppInstallerTestExeInstaller.exe"; + + var result = TestCommon.RunAICLICommand("install", $"{packageId} -v 1.0.0.0 --scope machine"); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + Assert.True(result.StdOut.Contains("Successfully installed")); + + var result2 = TestCommon.RunAICLICommand("upgrade", $"{packageId} -v 2.0.0.0"); + ConfigureInstallBehavior(Constants.PortablePackageMachineRoot, string.Empty); + Assert.AreEqual(Constants.ErrorCode.S_OK, result2.ExitCode); + Assert.True(result2.StdOut.Contains("Successfully installed")); + TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true, TestCommon.Scope.Machine); + } + private void ModifyPortableARPEntryValue(string productCode, string name, string value) { using (RegistryKey uninstallRegistryKey = Registry.CurrentUser.OpenSubKey(UninstallSubKey, true)) diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index 4edcb04ef5..ca706ec9a6 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -414,15 +414,50 @@ TEST_CASE("SettingsExperimentalCmd", "[settings]") } } -TEST_CASE("SettingsPortableAppRoot", "[settings]") +TEST_CASE("SettingsPortablePackageUserRoot", "[settings]") { SECTION("Relative path") { - std::string_view json = R"({ "installBehavior": { "portableAppUserRoot": %LOCALAPPDATA%/Portable/Root } })"; + DeleteUserSettingsFiles(); + std::string_view json = R"({ "installBehavior": { "portablePackageUserRoot": %LOCALAPPDATA%/Portable/Root } })"; SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; - REQUIRE(userSettingTest.Get().empty()); + REQUIRE(userSettingTest.Get().empty()); REQUIRE(userSettingTest.GetWarnings().size() == 1); } + SECTION("Valid path") + { + DeleteUserSettingsFiles(); + std::string_view json = R"({ "installBehavior": { "portablePackageUserRoot": "C:/Foo/Bar" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == "C:/Foo/Bar"); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } +} + +TEST_CASE("SettingsPortablePackageMachineRoot", "[settings]") +{ + SECTION("Relative path") + { + DeleteUserSettingsFiles(); + std::string_view json = R"({ "installBehavior": { "portablePackageMachineRoot": %LOCALAPPDATA%/Portable/Root } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get().empty()); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Valid path") + { + DeleteUserSettingsFiles(); + std::string_view json = R"({ "installBehavior": { "portablePackageMachineRoot": "C:/Foo/Bar" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == "C:/Foo/Bar"); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } } diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 857b6e9885..a71a5a5795 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -584,13 +584,6 @@ void OverrideForPortableUninstall(TestContext& context) } }); } -void OverrideForEnsureSupportForPortable(TestContext& context) -{ - context.Override({ EnsureSupportForPortableInstall, [](TestContext&) - { - } }); -} - void OverrideForDirectMsi(TestContext& context) { OverrideForCheckExistingInstaller(context); @@ -982,6 +975,50 @@ TEST_CASE("PortableInstallFlow", "[InstallFlow][workflow]") REQUIRE(std::filesystem::exists(portableInstallResultPath.GetPath())); } +TEST_CASE("PortableInstallFlow_UserScope", "[InstallFlow][workflow]") +{ + TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false); + TestCommon::TempFile portableInstallResultPath("TestPortableInstalled.txt"); + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForPortableInstallFlow(context); + context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Portable.yaml").GetPath().u8string()); + context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory); + context.Args.AddArg(Execution::Args::Type::InstallScope, "user"sv); + + InstallCommand install({}); + install.Execute(context); + INFO(installOutput.str()); + + REQUIRE(std::filesystem::exists(portableInstallResultPath.GetPath())); +} + +TEST_CASE("PortableInstallFlow_MachineScope", "[InstallFlow][workflow]") +{ + if (!AppInstaller::Runtime::IsRunningAsAdmin()) + { + WARN("Test requires admin privilege. Skipped."); + return; + } + + TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false); + TestCommon::TempFile portableInstallResultPath("TestPortableInstalled.txt"); + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForPortableInstallFlow(context); + context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Portable.yaml").GetPath().u8string()); + context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory); + context.Args.AddArg(Execution::Args::Type::InstallScope, "machine"sv); + + InstallCommand install({}); + install.Execute(context); + INFO(installOutput.str()); + REQUIRE(std::filesystem::exists(portableInstallResultPath.GetPath())); +} TEST_CASE("PortableInstallFlow_DevModeDisabled", "[InstallFlow][workflow]") { @@ -1575,7 +1612,6 @@ TEST_CASE("UpdateFlow_UpdatePortableWithManifest", "[UpdateFlow][workflow]") TestContext context{ updateOutput, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); - OverrideForEnsureSupportForPortable(context); OverrideForPortableInstallFlow(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("UpdateFlowTest_Portable.yaml").GetPath().u8string()); diff --git a/src/AppInstallerCommonCore/Manifest/Manifest.cpp b/src/AppInstallerCommonCore/Manifest/Manifest.cpp index 2fed4412eb..9551945edb 100644 --- a/src/AppInstallerCommonCore/Manifest/Manifest.cpp +++ b/src/AppInstallerCommonCore/Manifest/Manifest.cpp @@ -94,7 +94,7 @@ namespace AppInstaller::Manifest for (auto const& installer : Installers) { - if (DoesInstallerTypeSupportArpVersionRange(installer.InstallerType)) + if (DoesInstallerSupportArpVersionRange(installer)) { for (auto const& entry : installer.AppsAndFeaturesEntries) { diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index 90c85a1bd2..b73e25fe05 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -35,6 +35,17 @@ namespace AppInstaller::Manifest return CompatibilitySet::None; } } + + InstallerTypeEnum GetInstallerTypeFromInstaller(ManifestInstaller installer) + { + InstallerTypeEnum installerType = installer.InstallerType; + if (IsArchiveType(installerType)) + { + installerType = installer.NestedInstallerType; + } + + return installerType; + } } ManifestVer::ManifestVer(std::string_view version) @@ -417,13 +428,15 @@ namespace AppInstaller::Manifest return "Unknown"sv; } - bool DoesInstallerTypeUsePackageFamilyName(InstallerTypeEnum installerType) + bool DoesInstallerUsePackageFamilyName(ManifestInstaller installer) { + InstallerTypeEnum installerType = GetInstallerTypeFromInstaller(installer); return (installerType == InstallerTypeEnum::Msix || installerType == InstallerTypeEnum::MSStore); } - bool DoesInstallerTypeUseProductCode(InstallerTypeEnum installerType) + bool DoesInstallerUseProductCode(ManifestInstaller installer) { + InstallerTypeEnum installerType = GetInstallerTypeFromInstaller(installer); return ( installerType == InstallerTypeEnum::Exe || installerType == InstallerTypeEnum::Inno || @@ -435,8 +448,9 @@ namespace AppInstaller::Manifest ); } - bool DoesInstallerTypeWriteAppsAndFeaturesEntry(InstallerTypeEnum installerType) + bool DoesInstallerWriteAppsAndFeaturesEntry(ManifestInstaller installer) { + InstallerTypeEnum installerType = GetInstallerTypeFromInstaller(installer); return ( installerType == InstallerTypeEnum::Exe || installerType == InstallerTypeEnum::Inno || @@ -448,6 +462,12 @@ namespace AppInstaller::Manifest ); } + bool DoesInstallerSupportArpVersionRange(ManifestInstaller installer) + { + InstallerTypeEnum installerType = GetInstallerTypeFromInstaller(installer); + return DoesInstallerTypeSupportArpVersionRange(installerType); + } + bool DoesInstallerTypeSupportArpVersionRange(InstallerTypeEnum installerType) { return ( @@ -460,6 +480,19 @@ namespace AppInstaller::Manifest ); } + bool DoesInstallerIgnoreScopeFromManifest(ManifestInstaller installer) + { + InstallerTypeEnum installerType = GetInstallerTypeFromInstaller(installer); + return ( + installerType == InstallerTypeEnum::Portable + ); + } + + bool IsArchiveType(InstallerTypeEnum installerType) + { + return (installerType == InstallerTypeEnum::Zip); + } + bool IsInstallerTypeCompatible(InstallerTypeEnum type1, InstallerTypeEnum type2) { // Unknown type cannot be compatible with any other diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index dafc55187b..a34a3a70ca 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -102,17 +102,17 @@ namespace AppInstaller::Manifest // Validate system reference strings if they are set at the installer level // Allow PackageFamilyName to be declared with non msix installers to support nested installer scenarios after manifest version 1.1 - if (manifest.ManifestVersion <= ManifestVer{ s_ManifestVersionV1_1 } && !installer.PackageFamilyName.empty() && !DoesInstallerTypeUsePackageFamilyName(installer.InstallerType)) + if (manifest.ManifestVersion <= ManifestVer{ s_ManifestVersionV1_1 } && !installer.PackageFamilyName.empty() && !DoesInstallerUsePackageFamilyName(installer)) { resultErrors.emplace_back(ManifestError::InstallerTypeDoesNotSupportPackageFamilyName, "InstallerType", InstallerTypeToString(installer.InstallerType)); } - if (!installer.ProductCode.empty() && !DoesInstallerTypeUseProductCode(installer.InstallerType)) + if (!installer.ProductCode.empty() && !DoesInstallerUseProductCode(installer)) { resultErrors.emplace_back(ManifestError::InstallerTypeDoesNotSupportProductCode, "InstallerType", InstallerTypeToString(installer.InstallerType)); } - if (!installer.AppsAndFeaturesEntries.empty() && !DoesInstallerTypeWriteAppsAndFeaturesEntry(installer.InstallerType)) + if (!installer.AppsAndFeaturesEntries.empty() && !DoesInstallerWriteAppsAndFeaturesEntry(installer)) { resultErrors.emplace_back(ManifestError::InstallerTypeDoesNotWriteAppsAndFeaturesEntry, "InstallerType", InstallerTypeToString(installer.InstallerType)); } diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index 2cacce455c..a6fd87dd60 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp @@ -806,17 +806,17 @@ namespace AppInstaller::Manifest std::move(errors.begin(), errors.end(), std::inserter(resultErrors, resultErrors.end())); // Copy in system reference strings from the root if not set in the installer and appropriate - if (installer.PackageFamilyName.empty() && DoesInstallerTypeUsePackageFamilyName(installer.InstallerType)) + if (installer.PackageFamilyName.empty() && DoesInstallerUsePackageFamilyName(installer)) { installer.PackageFamilyName = manifest.DefaultInstallerInfo.PackageFamilyName; } - if (installer.ProductCode.empty() && DoesInstallerTypeUseProductCode(installer.InstallerType)) + if (installer.ProductCode.empty() && DoesInstallerUseProductCode(installer)) { installer.ProductCode = manifest.DefaultInstallerInfo.ProductCode; } - if (installer.AppsAndFeaturesEntries.empty() && DoesInstallerTypeWriteAppsAndFeaturesEntry(installer.InstallerType)) + if (installer.AppsAndFeaturesEntries.empty() && DoesInstallerWriteAppsAndFeaturesEntry(installer)) { installer.AppsAndFeaturesEntries = manifest.DefaultInstallerInfo.AppsAndFeaturesEntries; } diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index fa4a186d4e..76f057a0df 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -10,6 +10,9 @@ namespace AppInstaller::Manifest { + // Forward declaration + struct ManifestInstaller; + using string_t = Utility::NormalizedString; using namespace std::string_view_literals; @@ -265,17 +268,26 @@ bool HasExtension(std::string_view extension) const; std::string_view ScopeToString(ScopeEnum scope); - // Gets a value indicating whether the given installer type uses the PackageFamilyName system reference. - bool DoesInstallerTypeUsePackageFamilyName(InstallerTypeEnum installerType); + // Gets a value indicating whether the given installer uses the PackageFamilyName system reference. + bool DoesInstallerUsePackageFamilyName(ManifestInstaller installer); + + // Gets a value indicating whether the given installer uses the ProductCode system reference. + bool DoesInstallerUseProductCode(ManifestInstaller installer); - // Gets a value indicating whether the given installer type uses the ProductCode system reference. - bool DoesInstallerTypeUseProductCode(InstallerTypeEnum installerType); + // Gets a value indicating whether the given installer writes ARP entry. + bool DoesInstallerWriteAppsAndFeaturesEntry(ManifestInstaller installer); - // Gets a value indicating whether the given installer type writes ARP entry. - bool DoesInstallerTypeWriteAppsAndFeaturesEntry(InstallerTypeEnum installerType); + // Gets a value indicating whether the given installer supports ARP version range. + bool DoesInstallerSupportArpVersionRange(ManifestInstaller installer); // Gets a value indicating whether the given installer type supports ARP version range. - bool DoesInstallerTypeSupportArpVersionRange(InstallerTypeEnum installerType); + bool DoesInstallerTypeSupportArpVersionRange(InstallerTypeEnum installer); + + // Gets a value indicating whether the given installer ignores the Scope value from the manifest. + bool DoesInstallerIgnoreScopeFromManifest(ManifestInstaller installer); + + // Gets a value indicating whether the given installer type is an archive. + bool IsArchiveType(InstallerTypeEnum installerType); // Checks whether 2 installer types are compatible. E.g. inno and exe are update compatible bool IsInstallerTypeCompatible(InstallerTypeEnum type1, InstallerTypeEnum type2); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h index d02a3847c7..7214537c8b 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h @@ -42,6 +42,8 @@ namespace AppInstaller::Manifest // If present, has more precedence than root InstallerTypeEnum InstallerType = InstallerTypeEnum::Unknown; + InstallerTypeEnum NestedInstallerType = InstallerTypeEnum::Unknown; + ScopeEnum Scope = ScopeEnum::Unknown; std::vector InstallModes; diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index a16d0bb482..fd8d1d11d5 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -87,8 +87,8 @@ namespace AppInstaller::Settings LoggingLevelPreference, InstallIgnoreWarnings, DisableInstallNotes, - PortableAppUserRoot, - PortableAppMachineRoot, + PortablePackageUserRoot, + PortablePackageMachineRoot, UninstallPurgePortablePackage, Max }; @@ -139,8 +139,8 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::InstallLocaleRequirement, std::vector, std::vector, {}, ".installBehavior.requirements.locale"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallIgnoreWarnings, bool, bool, false, ".installBehavior.ignoreWarnings"sv); SETTINGMAPPING_SPECIALIZATION(Setting::DisableInstallNotes, bool, bool, false, ".installBehavior.disableInstallNotes"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::PortableAppUserRoot, std::string, std::filesystem::path, {}, ".installBehavior.portableAppUserRoot"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::PortableAppMachineRoot, std::string, std::filesystem::path, {}, ".installBehavior.portableAppMachineRoot"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::PortablePackageUserRoot, std::string, std::filesystem::path, {}, ".installBehavior.portablePackageUserRoot"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::PortablePackageMachineRoot, std::string, std::filesystem::path, {}, ".installBehavior.portablePackageMachineRoot"sv); SETTINGMAPPING_SPECIALIZATION(Setting::UninstallPurgePortablePackage, bool, bool, false, ".uninstallBehavior.purgePortablePackage"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EnableSelfInitiatedMinidump, bool, bool, false, ".debugging.enableSelfInitiatedMinidump"sv); diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp index 63b799bcd5..627c8d46b9 100644 --- a/src/AppInstallerCommonCore/Runtime.cpp +++ b/src/AppInstallerCommonCore/Runtime.cpp @@ -348,7 +348,7 @@ namespace AppInstaller::Runtime create = false; break; case PathName::PortablePackageUserRoot: - result = Settings::User().Get(); + result = Settings::User().Get(); if (result.empty()) { result = GetKnownFolderPath(FOLDERID_LocalAppData); @@ -359,7 +359,7 @@ namespace AppInstaller::Runtime create = true; break; case PathName::PortablePackageMachineRootX64: - result = Settings::User().Get(); + result = Settings::User().Get(); if (result.empty()) { result = GetKnownFolderPath(FOLDERID_ProgramFilesX64); @@ -369,7 +369,7 @@ namespace AppInstaller::Runtime create = true; break; case PathName::PortablePackageMachineRootX86: - result = Settings::User().Get(); + result = Settings::User().Get(); if (result.empty()) { result = GetKnownFolderPath(FOLDERID_ProgramFilesX86); @@ -386,7 +386,7 @@ namespace AppInstaller::Runtime create = true; break; case PathName::PortableLinksMachineLocation: - result = GetKnownFolderPath(FOLDERID_ProgramFilesX64); + result = GetKnownFolderPath(FOLDERID_ProgramFiles); result /= s_PortablePackageRoot; result /= s_LinksDirectory; create = true; @@ -437,7 +437,7 @@ namespace AppInstaller::Runtime create = false; break; case PathName::PortablePackageUserRoot: - result = Settings::User().Get(); + result = Settings::User().Get(); if (result.empty()) { result = GetKnownFolderPath(FOLDERID_LocalAppData); @@ -448,7 +448,7 @@ namespace AppInstaller::Runtime create = true; break; case PathName::PortablePackageMachineRootX64: - result = Settings::User().Get(); + result = Settings::User().Get(); if (result.empty()) { result = GetKnownFolderPath(FOLDERID_ProgramFilesX64); @@ -458,7 +458,7 @@ namespace AppInstaller::Runtime create = true; break; case PathName::PortablePackageMachineRootX86: - result = Settings::User().Get(); + result = Settings::User().Get(); if (result.empty()) { result = GetKnownFolderPath(FOLDERID_ProgramFilesX86); @@ -475,7 +475,7 @@ namespace AppInstaller::Runtime create = true; break; case PathName::PortableLinksMachineLocation: - result = GetKnownFolderPath(FOLDERID_ProgramFilesX64); + result = GetKnownFolderPath(FOLDERID_ProgramFiles); result /= s_PortablePackageRoot; result /= s_LinksDirectory; create = true; diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 4d211772b2..0d5659dfab 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -241,7 +241,7 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(DisableInstallNotes) WINGET_VALIDATE_PASS_THROUGH(UninstallPurgePortablePackage) - WINGET_VALIDATE_SIGNATURE(PortableAppUserRoot) + WINGET_VALIDATE_SIGNATURE(PortablePackageUserRoot) { std::filesystem::path root = ConvertToUTF16(value); if (!root.is_absolute()) @@ -252,9 +252,9 @@ namespace AppInstaller::Settings return root; } - WINGET_VALIDATE_SIGNATURE(PortableAppMachineRoot) + WINGET_VALIDATE_SIGNATURE(PortablePackageMachineRoot) { - return SettingMapping::Validate(value); + return SettingMapping::Validate(value); } WINGET_VALIDATE_SIGNATURE(InstallArchitecturePreference)