Index V2 consumption #4462

JohnMcPMS · 2024-05-08T17:36:34Z

Change

This change adds the ability to actually use the V2 index database. This primarily required fixing the ignored search functionality from the previous PR and updating the ISource implementation to be aware of V2 and how to retrieve the package version manifest files. Additionally, a few places in the code were updated to avoid requesting all of the package versions when we could instead use only the latest version. This prevents the need to retrieve the package version file at all, which is the goal for all read-only operations (search, list, update [aka list but filtered to things with updates]). In reality, we do need to get the package version files when we need to do ARP version mapping, but that constitutes only 5% of the total packages in winget-pkgs currently.

While the V2 usage code is always enabled, an experimental feature (indexV2) is required to attempt to retrieve the V2 index from the CDN. Disabling this feature should naturally transition one back to V1 on the next update attempt.

File caching, based on the known SHA256 hash of the file, is added as a general utility. This is then used for V1 manifest files and also for V2 package version data and manifest files. While the files are generally small, it is not uncommon for them to be used a few times over a short period.

Thanks to @msftrubengu for pointing out that I forgot the include the exports in the .def file in the previous PR. 🥇

Validation

Added many new unit tests and updated many existing unit tests.
Updated E2E tests to generate a V2 index in addition to the V1, and added a test pass that runs all of the existing E2E tests using the V2 index.

Microsoft Reviewers: Open in CodeFlow

…d list paths

Trenly · 2024-05-08T17:43:02Z

doc/Settings.md

+### indexV2
+
+This feature enables the `winget` source to retrieve the V2 index, which is significantly smaller.
+Regardless of the state of this feature, if the index on the machine contains a V2 index, it will be used.


Just to clarify - If the V2 index is outdated, and a newer V1 index is available, will the V1 index be used?

[Editing for more complete answer]

If the feature is enabled, the V2 index will always be preferred if it exists.

If the feature is disabled, a newer V1 index will replace the V2 index when it becomes available.

Trenly · 2024-05-08T17:49:00Z

src/AppInstallerCLIE2ETests/Helpers/WinGetSettingsHelper.cs

+                        { "windowsFeature", false },
+                        { "resume", false },
+                        { "reboot", false },
+                    };


It seems like this table would need to be updated anytime an experimental feature is added or removed; Is there a way to make this dynamic? In fact, couldn't you just use an empty hashtable since the forcedExperimentalFeatures should add whatever experimental features you need to the hashtable?

I agree that an empty table is currently equivalent, but that also means that not updating it is equivalent. I'm not trying to change that right now.

But since I was looking at the file and then touched it, removed them.

Trenly · 2024-05-08T17:57:09Z

src/AppInstallerCLIE2ETests/Helpers/WinGetSettingsHelper.cs

+            ConfigureFeature(settingsJson, "directMSI", status);
+            ConfigureFeature(settingsJson, "windowsFeature", status);
+            ConfigureFeature(settingsJson, "resume", status);
+            ConfigureFeature(settingsJson, "reboot", status);


See above comment - seems like this could be a pain to maintain

To be clear, I'm not changing anything about that fact one way or the other here.

Trenly · 2024-05-08T18:05:00Z

src/AppInstallerCLITests/SQLiteIndex.cpp

+    if (!AreChannelsSupported(index))
+    {
+        return;
+    }


You repeat this a few times. Would a RETURNIF(condition , [return value]) macro be useful to reduce code at all?

I don't feel like that is worth the additional cognitive load of knowing what it does to save the lines.

Trenly · 2024-05-08T18:18:14Z

src/AppInstallerCommonCore/Public/winget/UserSettings.h

@@ -149,7 +150,7 @@ namespace AppInstaller::Settings
        SETTINGMAPPING_SPECIALIZATION(Setting::ProgressBarVisualStyle, std::string, VisualStyle, VisualStyle::Accent, ".visual.progressBar"sv);
        SETTINGMAPPING_SPECIALIZATION(Setting::AnonymizePathForDisplay, bool, bool, true, ".visual.anonymizeDisplayedPaths"sv);
        // Source
-        SETTINGMAPPING_SPECIALIZATION_POLICY(Setting::AutoUpdateTimeInMinutes, uint32_t, std::chrono::minutes, 5min, ".source.autoUpdateIntervalInMinutes"sv, ValuePolicy::SourceAutoUpdateIntervalInMinutes);
+        SETTINGMAPPING_SPECIALIZATION_POLICY(Setting::AutoUpdateTimeInMinutes, uint32_t, std::chrono::minutes, 15min, ".source.autoUpdateIntervalInMinutes"sv, ValuePolicy::SourceAutoUpdateIntervalInMinutes);


Not a comment about the code, just a reminder for @denelon that this change will require a docs update - https://learn.microsoft.com/en-us/windows/package-manager/winget/settings#autoupdateintervalinminutes

…iles while packaging

msftrubengu · 2024-05-09T00:29:16Z

src/AppInstallerCommonCore/PackageVersionDataManifest.cpp

@@ -70,6 +72,14 @@ namespace AppInstaller::Manifest
        return "versionData.mszyml"sv;
    }

+    std::filesystem::path PackageVersionDataManifest::GetRelativeDirectoryPath(std::string_view packageIdentifier, std::string_view manifestHash)
+    {
+        std::filesystem::path result = "packages";


static constexpr std::string_view?

If it was to be used elsewhere, but this function is meant to prevent that need.

msftrubengu · 2024-05-09T00:30:46Z

src/AppInstallerCommonCore/ExperimentalFeature.cpp

@@ -52,6 +52,8 @@ namespace AppInstaller::Settings
                return userSettings.Get<Setting::EFConfigureSelfElevation>();
            case ExperimentalFeature::Feature::StoreDownload:
                return userSettings.Get<Setting::EFStoreDownload>();
+            case ExperimentalFeature::Feature::IndexV2:


triggers OCD

I'm going to assume that my lazy "Accept Both Changes" click in Code that put my case first is the issue here. 🦪

msftrubengu · 2024-05-09T00:38:02Z

src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp

@@ -31,375 +31,894 @@ namespace AppInstaller::Repository::Microsoft
            std::weak_ptr<SQLiteIndexSource> m_source;
        };

-        // The IPackageVersion impl for SQLiteIndexSource.
-        struct PackageVersion : public SourceReference, public IPackageVersion
+        namespace V1


I'm sure this is easier, but it might be worthy to have them in separate files for maintainability.

…o content type mapping so that they will be served

yao-msft · 2024-05-10T02:57:04Z

doc/Settings.md

+Regardless of the state of this feature, if the index on the machine contains a V2 index, it will be used.
+If there is a bug with the V2 index stopping the `winget` CLI from working, disable the feature in your settings file and run this command:
+```
+> winget uninstall -s msstore Microsoft.Winget.Source_8wekyb3d8bbwe


nit: "-s msstore", is this listed in store source?

No, that was an intentional "if the winget source is broken, lets avoid using it" flag. Commands targeting the local data don't really need a source, but we don't have a way to use a null one.

yao-msft · 2024-05-10T03:40:39Z

src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h

+
+    namespace details
+    {
+        // The base for the package objects.


nit: seems like wrong description

No, it isn't wrong, but it probably made more sense before I refactored it away from the things that inherit it.

JohnMcPMS added 13 commits May 1, 2024 20:27

EF, index retrieval, file caching, part of source update done

de58c99

Source update done

a6e1566

Product code complete?

db8d933

Undo CRLF changes

7160b50

Merge from master

b3e5387

Working on file cache tests

e2e370a

unit test progress

94cea95

Minor changes to help with path length

67d7c62

Fix/mitigate long path issues

25eba2f

Remove some unnecessary uses of getting all version keys in search an…

5d8c384

…d list paths

all unit tests pass

259f113

work toward e2e test pass for v2

fa7533c

Add an E2E test pass that always uses index v2

d13e68c

JohnMcPMS requested a review from a team as a code owner May 8, 2024 17:36

Trenly reviewed May 8, 2024

View reviewed changes

JohnMcPMS added 2 commits May 8, 2024 12:00

PR feedback, fix arguments with default EF string (none), overwrite f…

07daa84

…iles while packaging

replace backslashes as Kestrel may not like them

f8466ab

msftrubengu previously approved these changes May 9, 2024

View reviewed changes

Move index source package implementations to own files, add .mszyml t…

8c66bf2

…o content type mapping so that they will be served

JohnMcPMS dismissed msftrubengu’s stale review via 8c66bf2 May 9, 2024 18:00

This comment has been minimized.

Sign in to view

JohnMcPMS added 2 commits May 9, 2024 11:03

spelling

7c8ea4d

Fix tests

0e0ca0b

yao-msft previously approved these changes May 10, 2024

View reviewed changes

JohnMcPMS added 2 commits May 10, 2024 09:05

Merge from master

3e74a07

Update comment per PR feedback

bcde1ab

JohnMcPMS dismissed yao-msft’s stale review via bcde1ab May 10, 2024 17:14

msftrubengu approved these changes May 10, 2024

View reviewed changes

yao-msft approved these changes May 10, 2024

View reviewed changes

JohnMcPMS merged commit 5fe31fc into microsoft:master May 10, 2024
8 checks passed

JohnMcPMS deleted the index-v2-use branch May 10, 2024 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index V2 consumption #4462

Index V2 consumption #4462

JohnMcPMS commented May 8, 2024 •

edited by microsoft-github-policy-service bot

Trenly May 8, 2024

JohnMcPMS May 8, 2024 •

edited

Trenly May 8, 2024

JohnMcPMS May 8, 2024

JohnMcPMS May 8, 2024

Trenly May 8, 2024

JohnMcPMS May 8, 2024

Trenly May 8, 2024

JohnMcPMS May 8, 2024

Trenly May 8, 2024

msftrubengu May 9, 2024

JohnMcPMS May 9, 2024

msftrubengu May 9, 2024

JohnMcPMS May 9, 2024

msftrubengu May 9, 2024

This comment has been minimized.

yao-msft May 10, 2024

JohnMcPMS May 10, 2024

yao-msft May 10, 2024

JohnMcPMS May 10, 2024

Index V2 consumption #4462

Index V2 consumption #4462

Conversation

JohnMcPMS commented May 8, 2024 • edited by microsoft-github-policy-service bot

Change

Validation

Microsoft Reviewers: Open in CodeFlow

Choose a reason for hiding this comment

JohnMcPMS May 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JohnMcPMS commented May 8, 2024 •

edited by microsoft-github-policy-service bot

JohnMcPMS May 8, 2024 •

edited