Remove scope filter from being applied to portables

doc/Settings.md

@@ -58,25 +58,25 @@ The `disableInstallNotes` behavior affects whether installation notes are shown
     },
 ```
 
-### 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.
+### Portable App User Root
+The `portableAppUserRoot` 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"
+        "portableAppUserRoot": "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.
+### Portable App Machine Root
+The `portableAppMachineRoot` 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"
+        "portableAppMachineRoot": "C:/Program Files/Packages/Portable"
     },
 ```
 

schemas/JSON/settings/settings.schema.0.2.json

@@ -107,13 +107,13 @@
           "type": "boolean",
           "default":  false
         },
-        "PortablePackageUserRoot": {
-          "description": "The default root directory where packages are installed to under User scope. Applies to the portable installer type.",
+        "portableAppUserRoot": {
+          "description": "The default root directory where apps are installed to under User scope. Applies to the portable installer type.",
           "type": "string",
           "default": "%LOCALAPPDATA%/Microsoft/WinGet/Packages/"
         },
-        "PortablePackageMachineRoot": {
-          "description": "The default root directory where packages are installed to under Machine scope. Applies to the portable installer type.",
+        "portableAppMachineRoot": {
+          "description": "The default root directory where apps are installed to under Machine scope. Applies to the portable installer type.",
           "type": "string",
           "default": "%PROGRAMFILES%/WinGet/Packages/"
         }

src/AppInstallerCLICore/Workflows/ManifestComparator.cpp

@@ -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 || DoesInstallerTypeIgnoreScopeFromManifest(installer.InstallerType))
                 {
                     return InapplicabilityFlags::None;
                 }

src/AppInstallerCLICore/Workflows/PortableFlow.cpp

@@ -54,16 +54,16 @@ namespace AppInstaller::CLI::Workflow
             {
                 if (arch == Utility::Architecture::X86)
                 {
-                    return Runtime::GetPathTo(Runtime::PathName::PortablePackageMachineRootX86);
+                    return Runtime::GetPathTo(Runtime::PathName::PortableAppMachineRootX86);
                 }
                 else
                 {
-                    return Runtime::GetPathTo(Runtime::PathName::PortablePackageMachineRootX64);
+                    return Runtime::GetPathTo(Runtime::PathName::PortableAppMachineRootX64);
                 }
             }
             else
             {
-                return Runtime::GetPathTo(Runtime::PathName::PortablePackageUserRoot);
+                return Runtime::GetPathTo(Runtime::PathName::PortableAppUserRoot);
             }
         }
 
@@ -556,6 +556,16 @@ 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)
@@ -634,6 +644,7 @@ namespace AppInstaller::CLI::Workflow
         {
             context <<
                 EnsureSymlinkCreationPrivilege <<
+                EnsureRunningAsAdminForMachineScopeInstall <<
                 EnsureValidArgsForPortableInstall <<
                 EnsureVolumeSupportsReparsePoints;
         }

src/AppInstallerCLIE2ETests/BaseCommand.cs

@@ -37,6 +37,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);

src/AppInstallerCLIE2ETests/InstallCommand.cs

@@ -259,7 +259,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);
 
@@ -285,7 +285,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.
@@ -301,6 +301,40 @@ public void ReinstallPortable()
             TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true);
         }
 
+        [Test]
+        public void InstallPortable_UserScope()
+        {
+            string installDir = TestCommon.GetRandomTestDir();
+            ConfigureInstallBehavior("portableAppUserRoot", 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");
+            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("portableAppMachineRoot", 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");
+            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);
+        }
+
         [Test]
         public void InstallZipWithExe()
         {

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
@@ -46,6 +45,12 @@ public static string SettingsJsonFilePath {
             }
         }
 
+        public enum Scope
+        {
+            User,
+            Machine
+        }
+
         public struct RunCommandResult
         {
             public int ExitCode;
@@ -280,9 +285,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()
@@ -295,12 +307,13 @@ 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);
 

src/AppInstallerCLITests/UserSettings.cpp

@@ -414,15 +414,50 @@ TEST_CASE("SettingsExperimentalCmd", "[settings]")
     }
 }
 
-TEST_CASE("SettingsPortableAppRoot", "[settings]")
+TEST_CASE("SettingsPortableAppUserRoot", "[settings]")
 {
     SECTION("Relative path")
     {
+        DeleteUserSettingsFiles();
         std::string_view json = R"({ "installBehavior": { "portableAppUserRoot": %LOCALAPPDATA%/Portable/Root } })";
         SetSetting(Stream::PrimaryUserSettings, json);
         UserSettingsTest userSettingTest;
 
         REQUIRE(userSettingTest.Get<Setting::PortableAppUserRoot>().empty());
         REQUIRE(userSettingTest.GetWarnings().size() == 1);
     }
+    SECTION("Valid path")
+    {
+        DeleteUserSettingsFiles();
+        std::string_view json = R"({ "installBehavior": { "portableAppUserRoot": "C:/Foo/Bar" } })";
+        SetSetting(Stream::PrimaryUserSettings, json);
+        UserSettingsTest userSettingTest;
+
+        REQUIRE(userSettingTest.Get<Setting::PortableAppMachineRoot>() == "C:/Foo/Bar");
+        REQUIRE(userSettingTest.GetWarnings().size() == 0);
+    }
+}
+
+TEST_CASE("SettingsPortableAppMachineRoot", "[settings]")
+{
+    SECTION("Relative path")
+    {
+        DeleteUserSettingsFiles();
+        std::string_view json = R"({ "installBehavior": { "portableAppMachineRoot": %LOCALAPPDATA%/Portable/Root } })";
+        SetSetting(Stream::PrimaryUserSettings, json);
+        UserSettingsTest userSettingTest;
+
+        REQUIRE(userSettingTest.Get<Setting::PortableAppMachineRoot>().empty());
+        REQUIRE(userSettingTest.GetWarnings().size() == 1);
+    }
+    SECTION("Valid path")
+    {
+        DeleteUserSettingsFiles();
+        std::string_view json = R"({ "installBehavior": { "portableAppMachineRoot": "C:/Foo/Bar" } })";
+        SetSetting(Stream::PrimaryUserSettings, json);
+        UserSettingsTest userSettingTest;
+
+        REQUIRE(userSettingTest.Get<Setting::PortableAppMachineRoot>() == "C:/Foo/Bar");
+        REQUIRE(userSettingTest.GetWarnings().size() == 0);
+    }
 }

src/AppInstallerCLITests/WorkFlow.cpp

@@ -615,13 +615,6 @@ void OverrideForPortableUninstall(TestContext& context)
     } });
 }
 
-void OverrideForEnsureSupportForPortable(TestContext& context)
-{
-    context.Override({ EnsureSupportForPortableInstall, [](TestContext&)
-    {
-    } });
-}
-
 void OverrideForArchiveInstall(TestContext& context)
 {
     context.Override({ ExtractFilesFromArchive, [](TestContext&)
@@ -1217,6 +1210,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]")
 {
@@ -1863,7 +1900,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());
 

src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp

@@ -481,6 +481,14 @@ namespace AppInstaller::Manifest
             );
     }
 
+    bool DoesInstallerTypeIgnoreScopeFromManifest(InstallerTypeEnum installerType)
+    {
+        return (
+            installerType == InstallerTypeEnum::Msix ||
+            installerType == InstallerTypeEnum::Portable
+            );
+    }
+
     bool IsArchiveType(InstallerTypeEnum installerType)
     {
         return (installerType == InstallerTypeEnum::Zip);

src/AppInstallerCommonCore/Public/AppInstallerRuntime.h

@@ -52,15 +52,15 @@ namespace AppInstaller::Runtime
         SecureSettingsForWrite,
         // The value of %USERPROFILE%.
         UserProfile,
-        // The location where portable packages are installed to with user scope.
-        PortablePackageUserRoot,
-        // The location where portable packages are installed to with machine scope (x64).
-        PortablePackageMachineRootX64,
-        // The location where portable packages are installed to with machine scope (x86).
-        PortablePackageMachineRootX86,
-        // The location where symlinks to portable packages are stored under user scope.
+        // The location where portable apps are installed to with user scope.
+        PortableAppUserRoot,
+        // The location where portable apps are installed to with machine scope (x64).
+        PortableAppMachineRootX64,
+        // The location where portable apps are installed to with machine scope (x86).
+        PortableAppMachineRootX86,
+        // The location where symlinks to portable apps are stored under user scope.
         PortableLinksUserLocation,
-        // The location where symlinks to portable packages are stored under machine scope.
+        // The location where symlinks to portable apps are stored under machine scope.
         PortableLinksMachineLocation,
     };