CmdPal: Fix winget in release builds (#41183)

Seems as though that doing `.ToArray` on the `IIterable<>` things we get
back from the winget API were not trim-safe. I'm not entirely sure why.

But we're totally okay just manually iterating over these things. That
works like a charm.

Closes #41172

As a drive-by, I wrapped the line from #41025 in a try-catch. If that
doesn't fix it, hopefully it at least gives us more logging.

src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/CommandItemViewModel.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Diagnostics.CodeAnalysis;
+using ManagedCommon;
 using Microsoft.CmdPal.Core.ViewModels.Messages;
 using Microsoft.CmdPal.Core.ViewModels.Models;
 using Microsoft.CommandPalette.Extensions;
@@ -181,15 +182,15 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
         if (more is not null)
         {
             MoreCommands = more
-                .Select(item =>
+                .Select<IContextItem, IContextItemViewModel>(item =>
                 {
                     if (item is ICommandContextItem contextItem)
                     {
-                        return new CommandContextItemViewModel(contextItem, PageContext) as IContextItemViewModel;
+                        return new CommandContextItemViewModel(contextItem, PageContext);
                     }
                     else
                     {
-                        return new SeparatorViewModel() as IContextItemViewModel;
+                        return new SeparatorViewModel();
                     }
                 })
                 .ToList();
@@ -237,8 +238,9 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
             FastInitializeProperties();
             return true;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
+            Logger.LogError("error fast initializing CommandItemViewModel", ex);
             Command = new(null, PageContext);
             _itemTitle = "Error";
             Subtitle = "Item failed to load";
@@ -257,9 +259,10 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
             SlowInitializeProperties();
             return true;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
             Initialized |= InitializedState.Error;
+            Logger.LogError("error slow initializing CommandItemViewModel", ex);
         }
 
         return false;
@@ -272,8 +275,9 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
             InitializeProperties();
             return true;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
+            Logger.LogError("error initializing CommandItemViewModel", ex);
             Command = new(null, PageContext);
             _itemTitle = "Error";
             Subtitle = "Item failed to load";
@@ -342,15 +346,15 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
                 if (more is not null)
                 {
                     var newContextMenu = more
-                        .Select(item =>
+                        .Select<IContextItem, IContextItemViewModel>(item =>
                         {
                             if (item is ICommandContextItem contextItem)
                             {
-                                return new CommandContextItemViewModel(contextItem, PageContext) as IContextItemViewModel;
+                                return new CommandContextItemViewModel(contextItem, PageContext);
                             }
                             else
                             {
-                                return new SeparatorViewModel() as IContextItemViewModel;
+                                return new SeparatorViewModel();
                             }
                         })
                         .ToList();

src/modules/cmdpal/Microsoft.CmdPal.UI/Microsoft.CmdPal.UI.csproj

@@ -23,6 +23,12 @@
     <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
     <AppendRuntimeIdentifierToOutputPath>false</AppendRuntimeIdentifierToOutputPath>
   </PropertyGroup>
+  
+  <!-- For debugging purposes, uncomment this block to enable AOT builds -->
+  <!-- <PropertyGroup>
+    <EnableCmdPalAOT>true</EnableCmdPalAOT>
+    <CIBuild>true</CIBuild>
+  </PropertyGroup> -->
 
   <PropertyGroup Condition="'$(EnableCmdPalAOT)' == 'true'">
     <SelfContained>true</SelfContained>

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WinGet/Pages/InstallPackageListItem.cs

@@ -32,7 +32,16 @@ public partial class InstallPackageListItem : ListItem
     {
         _package = package;
 
-        var version = _package.DefaultInstallVersion ?? _package.InstalledVersion;
+        PackageVersionInfo? version = null;
+        try
+        {
+            version = _package.DefaultInstallVersion ?? _package.InstalledVersion;
+        }
+        catch (Exception e)
+        {
+            Logger.LogError("Could not get package version", e);
+        }
+
         var versionTagText = "Unknown";
         if (version is not null)
         {
@@ -57,20 +66,27 @@ public partial class InstallPackageListItem : ListItem
         }
         catch (COMException ex)
         {
-            Logger.LogWarning($"{ex.ErrorCode}");
+            Logger.LogWarning($"GetCatalogPackageMetadata error {ex.ErrorCode}");
         }
 
         if (metadata is not null)
         {
-            if (metadata.Tags.Where(t => t.Equals(WinGetExtensionPage.ExtensionsTag, StringComparison.OrdinalIgnoreCase)).Any())
+            for (var i = 0; i < metadata.Tags.Count; i++)
             {
-                if (_installCommand is not null)
+                if (metadata.Tags[i].Equals(WinGetExtensionPage.ExtensionsTag, StringComparison.OrdinalIgnoreCase))
                 {
-                    _installCommand.SkipDependencies = true;
+                    if (_installCommand is not null)
+                    {
+                        _installCommand.SkipDependencies = true;
+                    }
+
+                    break;
                 }
             }
 
-            var description = string.IsNullOrEmpty(metadata.Description) ? metadata.ShortDescription : metadata.Description;
+            var description = string.IsNullOrEmpty(metadata.Description) ?
+                metadata.ShortDescription :
+                metadata.Description;
             var detailsBody = $"""
 
 {description}
@@ -116,9 +132,11 @@ public partial class InstallPackageListItem : ListItem
             // These can be l o n g
             { Properties.Resources.winget_release_notes, (metadata.ReleaseNotes, string.Empty) },
         };
-        var docs = metadata.Documentations.ToArray();
+        var docs = metadata.Documentations;
-        foreach (var item in docs)
+        var count = docs.Count;
+        for (var i = 0; i < count; i++)
         {
+            var item = docs[i];
             simpleData.Add(item.DocumentLabel, (string.Empty, item.DocumentUrl));
         }
 
@@ -141,7 +159,7 @@ public partial class InstallPackageListItem : ListItem
             }
         }
 
-        if (metadata.Tags.Any())
+        if (metadata.Tags.Count > 0)
         {
             DetailsElement pair = new()
             {
@@ -169,6 +187,7 @@ public partial class InstallPackageListItem : ListItem
         {
             // Handle other exceptions
             ExtensionHost.LogMessage($"[WinGet] UpdatedInstalledStatus throw exception: {ex.Message}");
+            Logger.LogError($"[WinGet] UpdatedInstalledStatus throw exception", ex);
             return;
         }
 
@@ -233,10 +252,10 @@ public partial class InstallPackageListItem : ListItem
             Stopwatch s = new();
             Logger.LogDebug($"Starting RefreshPackageCatalogAsync");
             s.Start();
-            var refs = WinGetStatics.AvailableCatalogs.ToArray();
+            var refs = WinGetStatics.AvailableCatalogs;
-
+            for (var i = 0; i < refs.Count; i++)
-            foreach (var catalog in refs)
             {
+                var catalog = refs[i];
                 var operation = catalog.RefreshPackageCatalogAsync();
                 operation.Wait();
             }

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WinGet/Pages/WinGetExtensionPage.cs

@@ -32,7 +32,7 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
     private CancellationTokenSource? _cancellationTokenSource;
     private Task<IEnumerable<CatalogPackage>>? _currentSearchTask;
 
-    private IEnumerable<CatalogPackage>? _results;
+    private List<CatalogPackage>? _results;
 
     public static string ExtensionsTag => "windows-commandpalette-extension";
 
@@ -48,7 +48,6 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
 
     public override IListItem[] GetItems()
     {
-        IListItem[] items = [];
         lock (_resultsLock)
         {
             // emptySearchForTag ===
@@ -61,12 +60,28 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
             {
                 IsLoading = true;
                 DoUpdateSearchText(string.Empty);
-                return items;
+                return [];
             }
 
-            if (_results is not null && _results.Any())
+            if (_results is not null && _results.Count != 0)
             {
-                ListItem[] results = _results.Select(PackageToListItem).ToArray();
+                var count = _results.Count;
+                var results = new ListItem[count];
+                var next = 0;
+                for (var i = 0; i < count; i++)
+                {
+                    try
+                    {
+                        var li = PackageToListItem(_results[i]);
+                        results[next] = li;
+                        next++;
+                    }
+                    catch (Exception ex)
+                    {
+                        Logger.LogError("error converting result to listitem", ex);
+                    }
+                }
+
                 IsLoading = false;
                 return results;
             }
@@ -82,7 +97,7 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
 
         IsLoading = false;
 
-        return items;
+        return [];
     }
 
     private static ListItem PackageToListItem(CatalogPackage p) => new InstallPackageListItem(p);
@@ -108,7 +123,7 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
 
         _cancellationTokenSource = new CancellationTokenSource();
 
-        CancellationToken cancellationToken = _cancellationTokenSource.Token;
+        var cancellationToken = _cancellationTokenSource.Token;
 
         IsLoading = true;
 
@@ -139,7 +154,7 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
     {
         try
         {
-            IEnumerable<CatalogPackage> results = await searchTask;
+            var results = await searchTask;
 
             // Ensure this is still the latest task
             if (_currentSearchTask == searchTask)
@@ -156,7 +171,7 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
         catch (Exception ex)
         {
             // Handle other exceptions
-            Logger.LogError(ex.Message);
+            Logger.LogError("Unexpected error while processing results", ex);
         }
     }
 
@@ -165,10 +180,10 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
         Logger.LogDebug($"Completed search for '{query}'");
         lock (_resultsLock)
         {
-            this._results = results;
+            this._results = results.ToList();
         }
 
-        RaiseItemsChanged(this._results.Count());
+        RaiseItemsChanged();
     }
 
     private async Task<IEnumerable<CatalogPackage>> DoSearchAsync(string query, CancellationToken ct)
@@ -190,12 +205,12 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
         HashSet<CatalogPackage> results = new(new PackageIdCompare());
 
         // Default selector: this is the way to do a `winget search <query>`
-        PackageMatchFilter selector = WinGetStatics.WinGetFactory.CreatePackageMatchFilter();
+        var selector = WinGetStatics.WinGetFactory.CreatePackageMatchFilter();
         selector.Field = Microsoft.Management.Deployment.PackageMatchField.CatalogDefault;
         selector.Value = query;
         selector.Option = PackageFieldMatchOption.ContainsCaseInsensitive;
 
-        FindPackagesOptions opts = WinGetStatics.WinGetFactory.CreateFindPackagesOptions();
+        var opts = WinGetStatics.WinGetFactory.CreateFindPackagesOptions();
         opts.Selectors.Add(selector);
 
         // testing
@@ -204,7 +219,7 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
         // Selectors is "OR", Filters is "AND"
         if (HasTag)
         {
-            PackageMatchFilter tagFilter = WinGetStatics.WinGetFactory.CreatePackageMatchFilter();
+            var tagFilter = WinGetStatics.WinGetFactory.CreatePackageMatchFilter();
             tagFilter.Field = Microsoft.Management.Deployment.PackageMatchField.Tag;
             tagFilter.Value = _tag;
             tagFilter.Option = PackageFieldMatchOption.ContainsCaseInsensitive;
@@ -215,11 +230,11 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
         // Clean up here, then...
         ct.ThrowIfCancellationRequested();
 
-        Lazy<Task<PackageCatalog>> catalogTask = HasTag ? WinGetStatics.CompositeWingetCatalog : WinGetStatics.CompositeAllCatalog;
+        var catalogTask = HasTag ? WinGetStatics.CompositeWingetCatalog : WinGetStatics.CompositeAllCatalog;
 
         // Both these catalogs should have been instantiated by the
         // WinGetStatics static ctor when we were created.
-        PackageCatalog catalog = await catalogTask.Value;
+        var catalog = await catalogTask.Value;
 
         if (catalog is null)
         {
@@ -235,8 +250,8 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
 
             // BODGY, re: microsoft/winget-cli#5151
             // FindPackagesAsync isn't actually async.
-            Task<FindPackagesResult> internalSearchTask = Task.Run(() => catalog.FindPackages(opts), ct);
+            var internalSearchTask = Task.Run(() => catalog.FindPackages(opts), ct);
-            FindPackagesResult searchResults = await internalSearchTask;
+            var searchResults = await internalSearchTask;
 
             // TODO more error handling like this:
             if (searchResults.Status != FindPackagesResultStatus.Ok)
@@ -247,13 +262,17 @@ internal sealed partial class WinGetExtensionPage : DynamicListPage, IDisposable
             }
 
             Logger.LogDebug($"    got results for ({query})", memberName: nameof(DoSearchAsync));
-            foreach (Management.Deployment.MatchResult? match in searchResults.Matches.ToArray())
+
+            // FYI Using .ToArray or any other kind of enumerable loop
+            // on arrays returned by the winget API are NOT trim safe
+            var count = searchResults.Matches.Count;
+            for (var i = 0; i < count; i++)
             {
+                var match = searchResults.Matches[i];
+
                 ct.ThrowIfCancellationRequested();
 
-                // Print the packages
+                var package = match.CatalogPackage;
-                CatalogPackage package = match.CatalogPackage;
-
                 results.Add(package);
             }