commit 41e70acb3c1f24a54742c088dbe7a3949f7ffdb0
parent 8282645bc1b75e91fcf6d547689d099eec410a47
Author: Flor Chacón <14323496+florelis@users.noreply.github.com>
Date: Tue, 6 Jun 2023 11:22:38 -0700
Make pinning a stable feature (#3315)
Co-authored-by: Ryan Fu <Fu.Ryan@microsoft.com>
Diffstat:
11 files changed, 16 insertions(+), 60 deletions(-)
diff --git a/doc/Settings.md b/doc/Settings.md
@@ -252,17 +252,6 @@ Experimental feature with the aim of managing dependencies, as of now it only sh
},
```
-### pinning
-
-This feature enables the ability to pin packages to prevent the Windows Package Manager from updating them.
-You can enable the feature as shown below.
-
-```json
- "experimentalFeatures": {
- "pinning": true
- },
-```
-
### configuration
This feature enables the configuration commands. These commands allow configuring the system into a desired state.
diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json
@@ -209,11 +209,6 @@
"type": "boolean",
"default": false
},
- "pinning": {
- "description": "Enable support for package pinning",
- "type": "boolean",
- "default": false
- },
"configuration": {
"description": "Enable support for configuration",
"type": "boolean",
diff --git a/src/AppInstallerCLICore/Commands/PinCommand.h b/src/AppInstallerCLICore/Commands/PinCommand.h
@@ -8,7 +8,7 @@ namespace AppInstaller::CLI
{
struct PinCommand final : public Command
{
- PinCommand(std::string_view parent) : Command("pin", {} /* aliases */, parent, Settings::ExperimentalFeature::Feature::Pinning) {}
+ PinCommand(std::string_view parent) : Command("pin", {} /* aliases */, parent) {}
std::vector<std::unique_ptr<Command>> GetCommands() const override;
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
@@ -770,7 +770,7 @@ namespace AppInstaller::CLI::Workflow
continue;
}
- if (m_onlyShowUpgrades && !updateAvailable && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning))
+ if (m_onlyShowUpgrades && !updateAvailable)
{
bool updateAvailableWithoutPins = match.Package->IsUpdateAvailable(PinBehavior::IgnorePins);
if (updateAvailableWithoutPins)
@@ -959,7 +959,7 @@ namespace AppInstaller::CLI::Workflow
std::shared_ptr<IPackage> package = context.Get<Execution::Data::Package>();
std::shared_ptr<IPackageVersion> requestedVersion;
- if (m_considerPins && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning))
+ if (m_considerPins)
{
bool isPinned = false;
diff --git a/src/AppInstallerCLIE2ETests/Pinning.cs b/src/AppInstallerCLIE2ETests/Pinning.cs
@@ -16,15 +16,6 @@ namespace AppInstallerCLIE2ETests
public class Pinning : BaseCommand
{
/// <summary>
- /// Setup done once before all the tests here.
- /// </summary>
- [OneTimeSetUp]
- public void OneTimeSetup()
- {
- WinGetSettingsHelper.ConfigureFeature("pinning", true);
- }
-
- /// <summary>
/// Set up for all tests.
/// </summary>
[SetUp]
diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp
@@ -589,7 +589,6 @@ TEST_CASE("CompositePackage_AvailableVersions_NoChannelFilteredOut", "[Composite
TEST_CASE("CompositeSource_MultipleAvailableSources_MatchAll", "[CompositeSource]")
{
TestCommon::TestUserSettings testSettings;
- testSettings.Set<Settings::Setting::EFPinning>(true);
std::string pfn = "sortof_apfn";
std::string firstName = "Name1";
@@ -1102,8 +1101,6 @@ TEST_CASE("CompositeSource_Pinning_AvailableVersionPinned", "[CompositeSource][P
TestHook::SetPinningIndex_Override pinningIndexOverride(indexFile.GetPath());
TestUserSettings userSettings;
- userSettings.Set<Settings::Setting::EFPinning>(true);
-
CompositeTestSetup setup;
auto installedPackage = TestPackage::Make(MakeDefaultManifest("1.0.1"sv), TestPackage::MetadataMap{});
@@ -1215,8 +1212,6 @@ TEST_CASE("CompositeSource_Pinning_OneSourcePinned", "[CompositeSource][PinFlow]
TestHook::SetPinningIndex_Override pinningIndexOverride(indexFile.GetPath());
TestUserSettings userSettings;
- userSettings.Set<Settings::Setting::EFPinning>(true);
-
CompositeTestSetup setup;
auto installedPackage = TestPackage::Make(MakeDefaultManifest("1.0"sv), TestPackage::MetadataMap{});
@@ -1273,8 +1268,6 @@ TEST_CASE("CompositeSource_Pinning_OneSourceGated", "[CompositeSource][PinFlow]"
TestHook::SetPinningIndex_Override pinningIndexOverride(indexFile.GetPath());
TestUserSettings userSettings;
- userSettings.Set<Settings::Setting::EFPinning>(true);
-
CompositeTestSetup setup;
auto installedPackage = TestPackage::Make(MakeDefaultManifest("1.0"sv), TestPackage::MetadataMap{});
@@ -1338,8 +1331,7 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo
TestHook::SetPinningIndex_Override pinningIndexOverride(indexFile.GetPath());
TestUserSettings userSettings;
- userSettings.Set<Settings::Setting::EFPinning>(true);
-
+
std::string packageId = "packageId";
std::string productCode1 = "product-code1";
std::string productCode2 = "product-code2";
diff --git a/src/AppInstallerCommonCore/ExperimentalFeature.cpp b/src/AppInstallerCommonCore/ExperimentalFeature.cpp
@@ -42,8 +42,6 @@ namespace AppInstaller::Settings
return userSettings.Get<Setting::EFDependencies>();
case ExperimentalFeature::Feature::DirectMSI:
return userSettings.Get<Setting::EFDirectMSI>();
- case ExperimentalFeature::Feature::Pinning:
- return userSettings.Get<Setting::EFPinning>();
case ExperimentalFeature::Feature::Configuration:
return userSettings.Get<Setting::EFConfiguration>();
case ExperimentalFeature::Feature::WindowsFeature:
@@ -79,8 +77,6 @@ namespace AppInstaller::Settings
return ExperimentalFeature{ "Show Dependencies Information", "dependencies", "https://aka.ms/winget-settings", Feature::Dependencies };
case Feature::DirectMSI:
return ExperimentalFeature{ "Direct MSI Installation", "directMSI", "https://aka.ms/winget-settings", Feature::DirectMSI };
- case Feature::Pinning:
- return ExperimentalFeature{ "Package Pinning", "pinning", "https://aka.ms/winget-settings", Feature::Pinning};
case Feature::Configuration:
return ExperimentalFeature{ "Configuration", "configuration", "https://aka.ms/winget-settings#configuration", Feature::Configuration };
case Feature::WindowsFeature:
diff --git a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h
@@ -24,9 +24,8 @@ namespace AppInstaller::Settings
Dependencies = 0x1,
// Before making DirectMSI non-experimental, it should be part of manifest validation.
DirectMSI = 0x2,
- Pinning = 0x4,
- Configuration = 0x8,
- WindowsFeature = 0x10,
+ Configuration = 0x4,
+ WindowsFeature = 0x8,
Max, // This MUST always be after all experimental features
// Features listed after Max will not be shown with the features command
diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h
@@ -71,7 +71,6 @@ namespace AppInstaller::Settings
EFExperimentalArg,
EFDependencies,
EFDirectMSI,
- EFPinning,
EFConfiguration,
EFWindowsFeature,
// Telemetry
@@ -143,7 +142,6 @@ namespace AppInstaller::Settings
SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalArg, bool, bool, false, ".experimentalFeatures.experimentalArg"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFDependencies, bool, bool, false, ".experimentalFeatures.dependencies"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv);
- SETTINGMAPPING_SPECIALIZATION(Setting::EFPinning, bool, bool, false, ".experimentalFeatures.pinning"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFConfiguration, bool, bool, false, ".experimentalFeatures.configuration"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFWindowsFeature, bool, bool, false, ".experimentalFeatures.windowsFeature"sv);
// Telemetry
diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp
@@ -260,7 +260,6 @@ namespace AppInstaller::Settings
WINGET_VALIDATE_PASS_THROUGH(EFExperimentalArg)
WINGET_VALIDATE_PASS_THROUGH(EFDependencies)
WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI)
- WINGET_VALIDATE_PASS_THROUGH(EFPinning)
WINGET_VALIDATE_PASS_THROUGH(EFConfiguration)
WINGET_VALIDATE_PASS_THROUGH(EFWindowsFeature)
WINGET_VALIDATE_PASS_THROUGH(AnonymizePathForDisplay)
diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp
@@ -466,7 +466,7 @@ namespace AppInstaller::Repository
std::vector<PackageVersionKey> GetAvailableVersionKeys(PinBehavior pinBehavior) const override
{
auto result = m_package->GetAvailableVersionKeys();
- if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value())
+ if (m_pin.has_value())
{
// Add pin information to all version keys
for (auto& pvk : result)
@@ -499,7 +499,7 @@ namespace AppInstaller::Repository
{
Pinning::PinType pinType = Pinning::PinType::Unknown;
- if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value())
+ if (m_pin.has_value())
{
pinType = GetPinnedStateForVersion(versionKey, m_pin.value(), PinBehavior::ConsiderPins);
}
@@ -770,6 +770,12 @@ namespace AppInstaller::Repository
{
for (auto& availablePackage : m_availablePackages)
{
+ // Safeguard in case a package with no available sneaks in as we intentionally do in tests
+ if (availablePackage.GetPackage()->GetAvailableVersionKeys().empty())
+ {
+ continue;
+ }
+
Pinning::PinKey pinKey = GetPinKeyForAvailable(availablePackage.GetPackage().get());
auto pin = pinningIndex.GetPin(pinKey);
@@ -813,9 +819,7 @@ namespace AppInstaller::Repository
std::optional<Pinning::Pin> GetInstalledPin() const
{
- return (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_installedPackage)
- ? m_installedPackage->GetPin()
- : std::nullopt;
+ return m_installedPackage ? m_installedPackage->GetPin() : std::nullopt;
}
std::optional<PinnablePackage> m_installedPackage;
@@ -1208,7 +1212,7 @@ namespace AppInstaller::Repository
// Adds all the pin information to the results from a search to a CompositeSource.
void AddPinInfoToCompositeSearchResult(CompositeResult& result)
{
- if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && !result.Matches.empty())
+ if (!result.Matches.empty())
{
// Look up any pins for the packages found
auto pinningIndex = PinningIndex::OpenOrCreateDefault();
@@ -1391,13 +1395,6 @@ namespace AppInstaller::Repository
// Search sources and add to result
for (const auto& source : m_availableSources)
{
- if (addedAvailablePackage && !ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning))
- {
- // Having multiple available packages is a new behavior introduced for package pinning,
- // so we gate it with the same feature in case it causes problems.
- break;
- }
-
// Do not attempt to correlate local packages against this source
if (!source.GetDetails().SupportInstalledSearchCorrelation)
{
