Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record Product Codes in pinning table #3167

Merged
merged 28 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9f7e200
Stop removing stale pins
florelis Apr 13, 2023
117102c
Store product codes on pin table
florelis Apr 18, 2023
8ff23c2
Add TODOs to tests
florelis Apr 18, 2023
a52fdc6
Remove test TODOs
florelis Apr 18, 2023
553f429
Update src/AppInstallerCommonCore/Public/winget/Pin.h
florelis Apr 19, 2023
3bccb45
Add ToString methods
florelis Apr 19, 2023
c25ce81
Increase pinning index schema version
florelis Apr 19, 2023
4ebfef2
Improve SQL schema version logging
florelis Apr 19, 2023
cff554d
Merge branch 'master' into pinningIssues
florelis Apr 19, 2023
0b84462
Fix schema version
florelis Apr 19, 2023
7d0f3da
Add composite source test
florelis Apr 21, 2023
81a7679
unneeded whitespace
florelis Apr 21, 2023
41bc9aa
Merge branch 'master' into pinningIssues
florelis Apr 26, 2023
4c65086
Latest schema log
florelis Apr 26, 2023
e214cb4
Revert schema and pin key changes
florelis Apr 27, 2023
329e9a8
Handle pins for installed on composite
florelis May 2, 2023
f608e85
Pin on installed
florelis May 3, 2023
3963a4a
Fix list of available versions
florelis May 3, 2023
a93a7d5
Fix pinned state for available versions
florelis May 3, 2023
2d56521
Use package ID directly without installed check
florelis May 4, 2023
f1b56fd
Add missing spaces
florelis May 9, 2023
049f48a
Update comment
florelis May 9, 2023
2fc037c
Use string_view
florelis May 9, 2023
a306756
nullopt
florelis May 9, 2023
d493bb3
Simplify comparison
florelis May 9, 2023
fe502e9
Update comment
florelis May 9, 2023
a7656f9
Include pins on non-installed
florelis May 9, 2023
bd39644
Update comments
florelis May 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/AppInstallerCLICore/Commands/PinCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ namespace AppInstaller::CLI
Workflow::GetAllPins <<
Workflow::OpenSource() <<
Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) <<
Workflow::CrossReferencePinsWithSource <<
Workflow::ReportPins;
}

Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(PinResettingAll);
WINGET_DEFINE_RESOURCE_STRINGID(PinResetUseForceArg);
WINGET_DEFINE_RESOURCE_STRINGID(PinType);
WINGET_DEFINE_RESOURCE_STRINGID(PinVersion);
WINGET_DEFINE_RESOURCE_STRINGID(PoliciesPolicy);
WINGET_DEFINE_RESOURCE_STRINGID(PortableHashMismatchOverridden);
WINGET_DEFINE_RESOURCE_STRINGID(PortableHashMismatchOverrideRequired);
Expand Down
206 changes: 129 additions & 77 deletions src/AppInstallerCLICore/Workflows/PinFlow.cpp

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/Workflows/PinFlow.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ namespace AppInstaller::CLI::Workflow
void RemovePin(Execution::Context& context);

// Report the pins in a table.
// This includes searching for the corresponding installed packages
// to be able to show more info, like the package name.
// Required Args: None
// Inputs: Pins
// Outputs: None
Expand Down
4 changes: 4 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -1843,4 +1843,8 @@ Please specify one of them using the --source option to proceed.</value>
<data name="InstallWaitingOnAnother" xml:space="preserve">
<value>Waiting for another install/uninstall to complete...</value>
</data>
<data name="PinVersion" xml:space="preserve">
<value>Pinned version</value>
<comment>Table header for the version to which a package is pinned; meaning it should not update from that version.</comment>
</data>
</root>
6 changes: 3 additions & 3 deletions src/AppInstallerCLITests/CompositeSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1116,7 +1116,7 @@ TEST_CASE("CompositeSource_PinnedAvailable", "[CompositeSource][PinFlow]")
// The result when ignoring pins is always the same
expectedResult.ResultsForPinBehavior[PinBehavior::IgnorePins] = { /* IsUpdateAvailable */ true, /* LatestAvailableVersion */ "1.1.0" };

PinKey pinKey("Id", setup.Available->Details.Identifier);
PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "");
auto pinningIndex = PinningIndex::OpenOrCreateDefault();
REQUIRE(pinningIndex);

Expand Down Expand Up @@ -1232,7 +1232,7 @@ TEST_CASE("CompositeSource_OneSourcePinned", "[CompositeSource][PinFlow]")
};

{
PinKey pinKey("Id", setup.Available->Details.Identifier);
PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "");
auto pinningIndex = PinningIndex::OpenOrCreateDefault();
REQUIRE(pinningIndex);
pinningIndex->AddPin(Pin::CreatePinningPin(PinKey{ pinKey }));
Expand Down Expand Up @@ -1295,7 +1295,7 @@ TEST_CASE("CompositeSource_OneSourceGated", "[CompositeSource][PinFlow]")
};

{
PinKey pinKey("Id", setup.Available->Details.Identifier);
PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "");
auto pinningIndex = PinningIndex::OpenOrCreateDefault();
REQUIRE(pinningIndex);
pinningIndex->AddPin(Pin::CreateGatingPin(PinKey{ pinKey }, GatedVersion{ "1.*"sv }));
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCLITests/PinFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]")
auto pins = index.GetAllPins();
REQUIRE(pins.size() == 1);
REQUIRE(pins[0].GetType() == PinType::Blocking);
REQUIRE(pins[0].GetPackageId() == "AppInstallerCliTest.TestExeInstaller");
REQUIRE(pins[0].GetSourceId() == "*TestSource");
REQUIRE(pins[0].GetGatedVersion().ToString() == "");
REQUIRE(pins[0].GetKey().PackageId == "AppInstallerCliTest.TestExeInstaller");
REQUIRE(pins[0].GetKey().SourceId == "*TestSource");

std::ostringstream pinListOutput;
TestContext listContext{ pinListOutput, std::cin };
Expand Down
17 changes: 8 additions & 9 deletions src/AppInstallerCLITests/PinningIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]")
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
INFO("Using temporary file named: " << tempFile.GetPath());

Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId" });
Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId", ExtraIdStringType::None, "" });

{
PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 });
Expand All @@ -77,8 +77,7 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]")
REQUIRE(pinFromIndex.value() == pin);

REQUIRE(pinFromIndex->GetType() == pin.GetType());
REQUIRE(pinFromIndex->GetPackageId() == pin.GetPackageId());
REQUIRE(pinFromIndex->GetSourceId() == pin.GetSourceId());
REQUIRE(pinFromIndex->GetKey() == pin.GetKey());
}

{
Expand All @@ -99,8 +98,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]")
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
INFO("Using temporary file named: " << tempFile.GetPath());

Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId" }, { "1.0.*"sv });
Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId" });
Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId", ExtraIdStringType::None, ""}, { "1.0.*"sv });
Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId", ExtraIdStringType::None, ""});

{
PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 });
Expand Down Expand Up @@ -133,8 +132,8 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]")
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
INFO("Using temporary file named: " << tempFile.GetPath());

Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1" });
Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2" });
Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1", ExtraIdStringType::None, "" });
Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2", ExtraIdStringType::None, "" });

// Add two pins to the index, then check that they show up when queried
PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 });
Expand All @@ -144,7 +143,7 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]")
REQUIRE(index.GetAllPins().size() == 2);
REQUIRE(index.GetPin(pin1.GetKey()).has_value());
REQUIRE(index.GetPin(pin2.GetKey()).has_value());
REQUIRE(!index.GetPin({ "pkg", "src" }).has_value());
REQUIRE(!index.GetPin({ "pkg", "src", ExtraIdStringType::None, "" }).has_value());

// Reset the index, then check that there are no pins
index.ResetAllPins();
Expand All @@ -158,7 +157,7 @@ TEST_CASE("PinningIndex_AddDuplicatePin", "[pinningIndex]")
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
INFO("Using temporary file named: " << tempFile.GetPath());

Pin pin = Pin::CreateGatingPin({ "pkg", "src" }, { "1.*"sv });
Pin pin = Pin::CreateGatingPin({ "pkg", "src", ExtraIdStringType::None, "" }, { "1.*"sv });

PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 });
index.AddPin(pin);
Expand Down
61 changes: 53 additions & 8 deletions src/AppInstallerCommonCore/Pin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,70 @@ namespace AppInstaller::Pinning
if (Utility::CaseInsensitiveEquals(in, "Blocking"sv))
{
return PinType::Blocking;
}
}
else if (Utility::CaseInsensitiveEquals(in, "Pinning"sv))
{
{
return PinType::Pinning;
}
}
else if (Utility::CaseInsensitiveEquals(in, "Gating"sv))
{
{
return PinType::Gating;
}
}
else if (Utility::CaseInsensitiveEquals(in, "PinnedByManifest"sv))
{
{
return PinType::PinnedByManifest;
}
}
else
{
{
return PinType::Unknown;
}
}

PinType StrictestPinType(PinType first, PinType second)
{
return std::max(first, second);
}

std::string_view ToString(ExtraIdStringType type)
{
switch (type)
{
case ExtraIdStringType::ProductCode:
return "ProductCode"sv;
case ExtraIdStringType::PackageFamilyName:
return "PackageFamilyName";
case ExtraIdStringType::None:
default:
return "None";
}
}

std::vector<PinKey> GetPinKeysForAvailablePackage(
std::string_view packageId,
std::string sourceId,
const std::vector<Utility::LocIndString>& installedProductCodes,
const std::vector<Utility::LocIndString>& installedPackageFamilyNames)
{
std::vector<Pinning::PinKey> pinKeys;

for (const auto& productCode : installedProductCodes)
{
pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::ProductCode, productCode);
}

for (const auto& packageFamilyName : installedPackageFamilyNames)
{
pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::PackageFamilyName, packageFamilyName);
}

if (pinKeys.empty())
{
pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::None, "");
}

return pinKeys;
}

Pin Pin::CreateBlockingPin(PinKey&& pinKey)
{
return { PinType::Blocking, std::move(pinKey) };
Expand Down
79 changes: 66 additions & 13 deletions src/AppInstallerCommonCore/Public/winget/Pin.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,63 +6,116 @@

namespace AppInstaller::Pinning
{
enum class PinType
// The pin types are ordered by how "strict" they are.
// Meaning, the one that is more restrictive goes later.
// This is used to decide which pin to report if there are multiple pins.
enum class PinType : int64_t
{
// Unknown pin type or not pinned
Unknown,
// Pinned by the manifest using the RequiresExplicitUpgrade field.
// Behaves the same as Pinning pins
PinnedByManifest,
// The package is blocked from 'upgrade --all' and 'upgrade <package>'.
// User has to unblock to allow update.
Blocking,
// The package is excluded from 'upgrade --all', unless '--include-pinned' is added.
// 'upgrade <package>' is not blocked.
Pinning,
// The package is pinned to a specific version range.
Gating,
// The package is blocked from 'upgrade --all' and 'upgrade <package>'.
// User has to unblock to allow update.
Blocking,
};

std::string_view ToString(PinType type);
PinType ConvertToPinTypeEnum(std::string_view in);

// The set of values needed to uniquely identify a Pin
// Determines which of two pin types is more strict.
PinType StrictestPinType(PinType first, PinType second);

// The type of an extra string used for identifying an installed app for pinning
enum class ExtraIdStringType
{
None,
ProductCode,
PackageFamilyName,
};

std::string_view ToString(ExtraIdStringType type);

// The set of values needed to uniquely identify a Pin.
// We use both a PackageId and a ProductCode or PackageFamilyName
// to be able to deal with packages that match with multiple installed apps.
// This helps us deal better with apps that we don't match properly elsewhere.
struct PinKey
{
PinKey() {}
PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId)
: PackageId(packageId), SourceId(sourceId) {}
PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId, ExtraIdStringType extraIdType, std::string_view extraId = {})
florelis marked this conversation as resolved.
Show resolved Hide resolved
: PackageId(packageId), SourceId(sourceId), ExtraIdType(extraIdType), ExtraId(extraId) {}

bool operator==(const PinKey& other) const
{
return PackageId == other.PackageId && SourceId == other.SourceId;
return PackageId == other.PackageId
&& SourceId == other.SourceId
&& ExtraIdType == other.ExtraIdType
&& ExtraId == other.ExtraId;
}

bool operator!=(const PinKey& other) const
{
return !(*this == other);
}

bool operator<(const PinKey& other) const
{
return PackageId < other.PackageId || (PackageId == other.PackageId && SourceId < other.SourceId);
if (PackageId != other.PackageId)
{
return PackageId < other.PackageId;
}
else if (SourceId != other.SourceId)
{
return SourceId < other.SourceId;
}
else if (ExtraIdType != other.ExtraIdType)
{
return ExtraIdType < other.ExtraIdType;
}
else
{
return ExtraId < other.ExtraId;
}
florelis marked this conversation as resolved.
Show resolved Hide resolved
}

Manifest::Manifest::string_t PackageId;
std::string SourceId;
const Manifest::Manifest::string_t PackageId;
const std::string SourceId;
const ExtraIdStringType ExtraIdType = ExtraIdStringType::None;
const std::string ExtraId;
};

// Gets a list of the possible pin keys that we may use for an available package
// given the ProductCodes/PackageFamilyNames we know for the installed version.
// If there is no ProductCode/PackageFamilyName, returns a single element with no extra id.
std::vector<PinKey> GetPinKeysForAvailablePackage(
std::string_view packageId,
std::string sourceId,
florelis marked this conversation as resolved.
Show resolved Hide resolved
const std::vector<Utility::LocIndString>& installedProductCodes,
const std::vector<Utility::LocIndString>& installedPackageFamilyNames);

struct Pin
{
static Pin CreateBlockingPin(PinKey&& pinKey);
static Pin CreatePinningPin(PinKey&& pinKey);
static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion&& gatedVersion);

static Pin CreateBlockingPin(const PinKey& pinKey) { return CreateBlockingPin(PinKey{ pinKey }); }
static Pin CreatePinningPin(const PinKey& pinKey) { return CreatePinningPin(PinKey{ pinKey }); }
static Pin CreateGatingPin(const PinKey& pinKey, const Utility::GatedVersion& gatedVersion) { return CreateGatingPin(PinKey{ pinKey }, Utility::GatedVersion{ gatedVersion }); }

PinType GetType() const { return m_type; }
const PinKey& GetKey() const { return m_key; }
const Manifest::Manifest::string_t& GetPackageId() const { return m_key.PackageId; }
const std::string& GetSourceId() const { return m_key.SourceId; }
const Utility::GatedVersion& GetGatedVersion() const { return m_gatedVersion; }

bool operator==(const Pin& other) const;
bool operator<(const Pin& other) const { return m_key < other.m_key; }

private:
Pin(PinType type, PinKey&& pinKey, Utility::GatedVersion&& gatedVersion = {})
Expand Down
Loading