Add Installer Url Matching for Update Command and Improve Error Messa…

…ges (#108) * Add url string matching for update command * remove comment for testutils * fix unit test and update notice txt * fix logic for update multiple url * fix PR comments * make archmismatch field a private set property * fix test failures * improve mismatch warning strings * Create detectedArch class and fix bugs * fix test failures * fix msix bug * make record urlArch nullable * add unit tests * fix msixbundle path * clean up unit tests * update notice txt
microsoft · Jul 16, 2021 · 636b37e · 636b37e
1 parent 250ce53
commit 636b37e
Show file tree

Hide file tree

Showing 13 changed files with 425 additions and 56 deletions.
diff --git a/NOTICE.txt b/NOTICE.txt
@@ -17,6 +17,37 @@ required to debug changes to any libraries licensed under the GNU Lesser General
 
 ---------------------------------------------------------
 
+Newtonsoft.Json.Schema 3.0.14 - AGPL-3.0-only OR OTHER
+
+
+(c) 2008 VeriSign, Inc.
+Copyright Newtonsoft 2014
+Copyright (c) 2014 Newtonsoft
+Copyright (c) Newtonsoft 2014
+
+# Json.NET Schema
+
+Copyright (c) 2014 Newtonsoft
+
+This program is free software: you can redistribute it and/or modify it 
+under the terms of the GNU Affero General Public License as published by the 
+Free Software Foundation, either version 3 of the License, see 
+http://www.gnu.org/licenses/agpl-3.0.html.
+
+This program is distributed in the hope that it will be useful, but WITHOUT 
+ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS 
+FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details.
+
+
+## Commercial License
+
+In addition to this license, Json.NET Schema is offered under a commercial license.
+
+
+---------------------------------------------------------
+
+---------------------------------------------------------
+
 Castle.Core 4.4.0 - Apache-2.0
 
 
@@ -151,10 +182,9 @@ limitations under the License.
 
 ---------------------------------------------------------
 
-RestSharp 106.11.7 - Apache-2.0
+RestSharp 106.11.8-alpha.0.13 - Apache-2.0
 
 
-(c) 2008 VeriSign, Inc.
 
 Apache License
 

diff --git a/src/WingetCreateCLI/Commands/BaseCommand.cs b/src/WingetCreateCLI/Commands/BaseCommand.cs
@@ -6,6 +6,7 @@ namespace Microsoft.WingetCreateCLI.Commands
     using System;
     using System.Collections.Generic;
     using System.IO;
+    using System.Linq;
     using System.Net.Http;
     using System.Threading.Tasks;
     using CommandLine;
@@ -256,6 +257,27 @@ protected static bool ValidateManifestsInTempDir(Manifests manifests)
             return result;
         }
 
+        /// <summary>
+        /// Displays the appropriate warning messages for installers with detected architecture mismatches.
+        /// </summary>
+        /// <param name="detectedArchs">List of DetectedArch objects that represent each installers detected architectures.</param>
+        protected static void DisplayMismatchedArchitectures(List<PackageParser.DetectedArch> detectedArchs)
+        {
+            var mismatchedArchInstallers = detectedArchs.Where(i => i.UrlArch.HasValue && i.UrlArch != i.BinaryArch);
+
+            if (mismatchedArchInstallers.Any())
+            {
+                Logger.WarnLocalized(nameof(Resources.DetectedArchMismatch_Message));
+                Console.WriteLine();
+                foreach (var installer in mismatchedArchInstallers)
+                {
+                    Logger.WarnLocalized(nameof(Resources.InstallerBinaryMismatch_Message), installer.UrlArch, installer.BinaryArch);
+                    Logger.Warn($"{installer.Url}");
+                    Console.WriteLine();
+                }
+            }
+        }
+
         /// <summary>
         /// Launches the OAuth flow to allow the user to login to their GitHub account and grant permission to the app.
         /// </summary>

diff --git a/src/WingetCreateCLI/Commands/NewCommand.cs b/src/WingetCreateCLI/Commands/NewCommand.cs
@@ -39,7 +39,7 @@ public class NewCommand : BaseCommand
         /// <summary>
         /// Installer types for which we can trust that the detected architecture is correct, so don't need to prompt the user to confirm.
         /// </summary>
-        private static readonly InstallerType[] ReliableArchitectureInstallerTypes = new[] { InstallerType.Msi, InstallerType.Msix, InstallerType.Appx };
+        private static readonly InstallerType[] ReliableArchitectureInstallerTypes = new[] { InstallerType.Msix, InstallerType.Appx };
 
         /// <summary>
         /// Gets the usage examples for the New command.
@@ -107,11 +107,17 @@ public override async Task<bool> Execute()
                     return false;
                 }
 
-                if (!PackageParser.ParsePackages(packageFiles, this.InstallerUrls, manifests))
+                if (!PackageParser.ParsePackages(
+                    packageFiles,
+                    this.InstallerUrls,
+                    manifests,
+                    out List<PackageParser.DetectedArch> detectedArchs))
                 {
                     Logger.ErrorLocalized(nameof(Resources.PackageParsing_Error));
                     return false;
                 }
+
+                DisplayMismatchedArchitectures(detectedArchs);
 
                 Console.WriteLine(Resources.NewCommand_Header);
                 Console.WriteLine();
@@ -266,11 +272,11 @@ private static void PromptInstallerProperties<T>(T manifest, PropertyInfo proper
 
                     PromptInstallerSwitchesForExe(manifest);
                 }
-
+
                 foreach (var requiredProperty in requiredInstallerProperties)
                 {
                     var currentValue = requiredProperty.GetValue(installer);
-
+
                     // Only prompt if the value isn't already set, or if it's the Architecture property and we don't trust the parser to have gotten it correct for this InstallerType.
                     if (currentValue == null ||
                         (requiredProperty.Name == nameof(Installer.Architecture) && !ReliableArchitectureInstallerTypes.Contains(installer.InstallerType.Value)))

diff --git a/src/WingetCreateCLI/Commands/UpdateCommand.cs b/src/WingetCreateCLI/Commands/UpdateCommand.cs
@@ -3,6 +3,7 @@
 
 namespace Microsoft.WingetCreateCLI.Commands
 {
+    using System;
     using System.Collections.Generic;
     using System.IO;
     using System.Linq;
@@ -194,21 +195,44 @@ public async Task<Manifests> DeserializeExistingManifestsAndUpdate(List<string>
                 return null;
             }
 
-            if (!PackageParser.UpdateInstallerNodes(installerManifest, this.InstallerUrls, packageFiles, out bool installerMismatch, out WingetCreateCore.Models.Installer.Installer installersMissingMatch))
+            if (!PackageParser.UpdateInstallerNodesAsync(
+                installerManifest,
+                this.InstallerUrls,
+                packageFiles,
+                out bool installerMismatch,
+                out List<WingetCreateCore.Models.Installer.Installer> unmatchedInstallers,
+                out List<WingetCreateCore.Models.Installer.Installer> multipleMatchedInstallers,
+                out List<PackageParser.DetectedArch> detectedArchOfInstallers))
             {
                 if (installerMismatch)
                 {
-                    Logger.ErrorLocalized(nameof(Resources.MultipleInstallerUpdateDiscrepancy_Error));
-                    if (installersMissingMatch != null)
+                    Logger.ErrorLocalized(nameof(Resources.NewInstallerUrlMustMatchExisting_Message));
+
+                    if (unmatchedInstallers.Any())
                     {
-                        Logger.ErrorLocalized(nameof(Resources.MissingPackageError_Message), installersMissingMatch.InstallerType, installersMissingMatch.Architecture);
+                        Logger.ErrorLocalized(nameof(Resources.InstallerMatchFailedError_Message));
+                        Console.WriteLine();
+                        unmatchedInstallers.ForEach(i => Logger.ErrorLocalized(nameof(Resources.InstallerDetectedFromUrl_Message), i.Architecture, i.InstallerType, i.InstallerUrl));
+                    }
+
+                    if (multipleMatchedInstallers.Any())
+                    {
+                        Logger.ErrorLocalized(nameof(Resources.MultipleMatchingInstallerNodes_Error));
+                        Console.WriteLine();
+                        multipleMatchedInstallers.ForEach(i =>
+                        {
+                            Logger.ErrorLocalized(nameof(Resources.InstallerDetectedFromUrl_Message), i.Architecture, i.InstallerType, i.InstallerUrl);
+                            Console.WriteLine();
+                        });
                     }
                 }
                 else
                 {
                     Logger.ErrorLocalized(nameof(Resources.PackageParsing_Error));
                 }
 
+                DisplayMismatchedArchitectures(detectedArchOfInstallers);
+
                 return null;
             }
 
@@ -244,7 +268,7 @@ public async Task<bool> ExecuteManifestUpdate(List<string> latestManifestContent
             {
                 if (this.SubmitToGitHub)
                 {
-                    if (!this.VerifyUpdatedInstallerHash(originalManifests, updatedManifests.InstallerManifest))
+                    if (!VerifyUpdatedInstallerHash(originalManifests, updatedManifests.InstallerManifest))
                     {
                         Logger.ErrorLocalized(nameof(Resources.NoChangeDetectedInUpdatedManifest_Message));
                         Logger.ErrorLocalized(nameof(Resources.CompareUpdatedManifestWithExisting_Message));
@@ -307,7 +331,7 @@ private static void UpdatePropertyForLocaleManifests(string propertyName, string
         /// <param name="oldManifest">The original manifest object model.</param>
         /// <param name="newManifest">The updated installer manifest object model.</param>
         /// <returns>A boolean value indicating whether the updated manifest has new changes compared to the original manifest.</returns>
-        private bool VerifyUpdatedInstallerHash(Manifests oldManifest, InstallerManifest newManifest)
+        private static bool VerifyUpdatedInstallerHash(Manifests oldManifest, InstallerManifest newManifest)
         {
             IEnumerable<string> oldHashes = oldManifest.InstallerManifest == null
                 ? oldManifest.SingletonManifest.Installers.Select(i => i.InstallerSha256).Distinct()

diff --git a/src/WingetCreateCLI/Properties/Resources.Designer.cs b/src/WingetCreateCLI/Properties/Resources.Designer.cs
diff --git a/src/WingetCreateCLI/Properties/Resources.resx b/src/WingetCreateCLI/Properties/Resources.resx
@@ -496,7 +496,7 @@
     <value>The installer minimum operating system version</value>
   </data>
   <data name="MultipleInstallerUpdateDiscrepancy_Error" xml:space="preserve">
-    <value>Updating a manifest is only supported with the same number of installer URLs, and the same installer types and architectures</value>
+    <value>Updating a manifest is only supported with the same number of installer URLs.</value>
   </data>
   <data name="Upgrade_KeywordDescription" xml:space="preserve">
     <value>The upgrade switches</value>
@@ -596,10 +596,8 @@
   <data name="NoChangeDetectedInUpdatedManifest_Message" xml:space="preserve">
     <value>Submitting a manifest without any updated changes is not allowed. </value>
   </data>
-  <data name="MissingPackageError_Message" xml:space="preserve">
-    <value>New package found for {0} {1} installer, but not found in existing manifest</value>
-    <comment>{0} - will be replaced with installer type
-{1} - will be replaced with installer architecture</comment>
+  <data name="InstallerMatchFailedError_Message" xml:space="preserve">
+    <value>The following installers failed to match an existing installer node:</value>
   </data>
   <data name="UnboundArguments_Message" xml:space="preserve">
     <value>The following unbound argument(s) were detected: "{0}"</value>
@@ -611,4 +609,24 @@
   <data name="ModifyOptionalFields_Message" xml:space="preserve">
     <value>Would you like to modify the optional fields?</value>
   </data>
+  <data name="InstallerDetectedFromUrl_Message" xml:space="preserve">
+    <value>{0} {1} installer detected from the url: {2}</value>
+    <comment>{0} - will be replaced with installer architecture
+{1} - will be replaced with the installer type 
+{2} - will be replaced with the installer url</comment>
+  </data>
+  <data name="MultipleMatchingInstallerNodes_Error" xml:space="preserve">
+    <value>Multiple existing installer nodes matched the following installer(s):</value>
+  </data>
+  <data name="NewInstallerUrlMustMatchExisting_Message" xml:space="preserve">
+    <value>Each new installer URL must have a match to an existing installer node based on installer type and architecture.</value>
+  </data>
+  <data name="DetectedArchMismatch_Message" xml:space="preserve">
+    <value>The architecture detected from the binary might be different than what is specified in the installer URL for the following installer(s):</value>
+  </data>
+  <data name="InstallerBinaryMismatch_Message" xml:space="preserve">
+    <value>Using architecture detected from URL ({0}), overriding architecture detected from binary ({1})</value>
+    <comment>{0} - Architecture detected from the installer URL
+{1} - Architecture determined from the installer binary</comment>
+  </data>
 </root>