Record Product Codes in pinning table

[florelis]

This PR changes the pinning table to record an installed pinned app's Product Code or Package Family Name. This allows us to distinguish between multiple installed apps that we want to pin and have the same package ID, in particular the Windows SDK.

This includes a breaking change to the schema of the pinning DB. The pinning index now uses v1.1, but it is not backwards compatible with v1.0. If, when opening an existing index, we see that it has version 1.0, we delete it and create a new one with the new version. This will drop all existing pins; requiring them to be re-added.I could try to recover the pins, but I think it is better to re-add them to get the new fields I'm adding in this PR.

I'm adding the ProductCode or PackageFamilyName as an extra identifier in the PinKey. If an installed package has multiple, we create multiple pins.

Closes #2974
Closes #3142, as the new DB schema won't even allow null for the version field
Edit: Seems to also fix #2977

Edit: Updated note about deleting old DB

Microsoft Reviewers: Open in CodeFlow

[Trenly]

This includes a breaking change to the schema of the pinning DB. Opening an existing pinning DB will fail, and one has to manually delete the DB file located at %localappdata%\Packages<dev or release package name>\LocalState\pinning.db. Since pinning has only been in preview; I consider this acceptable instead of creating a new DB schema version.

I think that it's acceptable to not create a new DB schema version. Like you said, it's still an experimental feature. That said, it is annoying to have to go in and manually delete a file, and I'm sure there will be a few issues opened about it and closed after we link back to this PR. @denelon may have a rough estimate of how many users are using the pin command?

To make it a bit more seamless, perhaps it may be beneficial to add a check that the pinning table matches at least one of the valid schemas. If it doesn't match any known schema, delete it and create a new one. Or perhaps make a method to restore the table to match the schema using the SQLite AlterTable functions to add any columns that are missing, reset the data type on existing columns, and drop any columns not present in the schema.

The "delete" or "repair" functionality could potentially be useful in the future as the installed.db is leveraged more and as new functionality is added.

[florelis]

This includes a breaking change to the schema of the pinning DB. Opening an existing pinning DB will fail, and one has to manually delete the DB file located at %localappdata%\Packages\LocalState\pinning.db. Since pinning has only been in preview; I consider this acceptable instead of creating a new DB schema version.

I think that it's acceptable to not create a new DB schema version. Like you said, it's still an experimental feature. That said, it is annoying to have to go in and manually delete a file, and I'm sure there will be a few issues opened about it and closed after we link back to this PR. @denelon Demitrius Nelon FTE may have a rough estimate of how many users are using the pin command?

To make it a bit more seamless, perhaps it may be beneficial to add a check that the pinning table matches at least one of the valid schemas. If it doesn't match any known schema, delete it and create a new one. Or perhaps make a method to restore the table to match the schema using the SQLite AlterTable functions to add any columns that are missing, reset the data type on existing columns, and drop any columns not present in the schema.

The "delete" or "repair" functionality could potentially be useful in the future as the installed.db is leveraged more and as new functionality is added.

I changed it to actually use a new schema version. The pinning index now uses v1.1, but it is not backwards compatible with v1.0. (Now that I'm typing it, I realize that it doesn't respect sem-ver...). If, when opening an existing index, we see that it has version 1.0, we delete it and create a new one with the new version.

This would no longer require manual deletion of the index, but we're still dropping all existing pins. I could try to recover the pins, but I think it is better to re-add them to get the new fields I'm adding in this PR.

[Trenly]

I changed it to actually use a new schema version. The pinning index now uses v1.1, but it is not backwards compatible with v1.0. (Now that I'm typing it, I realize that it doesn't respect sem-ver...). If, when opening an existing index, we see that it has version 1.0, we delete it and create a new one with the new version.

This would no longer require manual deletion of the index, but we're still dropping all existing pins. I could try to recover the pins, but I think it is better to re-add them to get the new fields I'm adding in this PR.

I think dropping them all is fine since its experimental anyways. Thanks for at least removing the manual step!!

[yao-msft]

Maybe I missed something, is logic here basically same as logic in line 420 above?

[yao-msft]

The initial thought for pinning a specific installed version is to add a pin from installed source to the pinning table so the previous pinning table will not break.

PackageId SourceId
PFN or Product code *installed

But if this approach works, it's ok too since this is still in experimental (and seems easier in implementation?)

[florelis]

After talking offline with @yao-msft I realize that this change is breaking the basic use case of "I update this app outside of winget, so don't touch it" because once there is an update, the product code will change and the pin will no longer apply.

The case I was thinking for this PR was the one we see with Windows SDK, where multiple installed packages match with the same available package, so we need to pin the specific installed version, which is done with the product code.

Taking into account both cases makes it weird. For upgrades outside of winget, we have to pin just by package ID & source to be able to track across versions. But for packages where matching doesn't quite work or have multiple installed packages, we have to pin by product code. I'm thinking the solution will have to be guessing the intent at the time of adding a pin and either add it by ID or by product code.

I like @yao-msft 's idea for the DB so we don't have to change the schema, so I'll change it to that. That way we don't have to deal with deleting old DBs. It also makes sense because for specific installed packages the source doesn't matter; there could be multiple sources for it but the moment one of them updates the package, the pin stops applying to all of them.

[florelis]

I've updated the PR to follow the schema suggested by @yao-msft for representing the pins in the DB. Given how different it all is I could probably have made a new PR but I want to keep the discussion...

I'm introducing the concept of pinning an available package or pinning an installed package

• Pinning an available package is done through its package ID and source. When we pin an available package, we can preserve the pin if there are updates outside of winget. The drawback is that we can't add a pin when there are multiple installed apps matching the available package (like Windows SDK). This is because when looking for the package to pin we can't find a single match. This would probably be fixed by implementing [Pinning] - Usage of --id should bypass installed package check #3141
• Pinning an installed package is done through its product code or package family name. Doing this allows us to distinguish between multiple installed apps for the same available package. The drawback is that if the app updates outside of winget, we most likely would lose the pin as the product code would change. There's not much we can do about that because we wouldn't be able to tell if the update was on the pinned or on the other side-by-side install

By default pinning works on available. To pin a specific installed app I'm adding a --installed flag. Would be great if we can just guess the intent and avoid the arg, but I could not find an easy way to do it

Pins for an installed package are represented in the DB by using the ProductCode or PFN as package ID and *PredefinedInstalledSource for the source ID (this could be any string, doesn't actually have to match the real installed source ID)

Most of the work is done in the CompositeSource. When creating a composite package, we query the pinning index for pins on the installed and available packages that make it up. Then when querying the composite package (e.g. for IsUpdateAvailable) we just reason about what the pin state would be for each version.

[Trenly]

Would be great if we can just guess the intent and avoid the arg,

Perhaps if pin by available fails (due to multiple packages, as one scenario) then pinning by installed could be a fallback?

[yao-msft]

So previously, our goal for pinning available package is: if we find one available package to pin, all other available packages from other sources are pinned, except if user provides -s <source>

Do you think we should do this for --id pins as well?

And our previous statement is we only pin packages that we can find an installed one with. This is allowing pinning packages that may not be installed yet. I guess it's ok, but just wanted to bring up, so that we think through all scenarios and it's ok.

[florelis]

So previously, our goal for pinning available package is: if we find one available package to pin, all other available packages from other sources are pinned, except if user provides -s

Do you think we should do this for --id pins as well?

I hadn't thought about that. With the code as it is, it will add pins to all sources matching the query (--id or otherwise). So if you do pin add Microsoft.PowerToys and multiple sources have Microsoft.PowerToys we'd be adding pins to all of them, which seems okay to me. But if the ID in one of the sources is just "PowerToys" we wouldn't pin it. That also seems okay to me because we can't guess what queries we'd have to make to match the package from another source as it can be totally different. And this applies whether we are using --id or some other filter

[yao-msft]

We could construct a query for systemReferenceStrings to find available packages from other sources. We can do it later if such asks emerges. But the behavior is different from what I specc'ed before.

[yao-msft]