Resolve issues 4366 and 4420 for better patch performance

History.md

@@ -1,3 +1,7 @@
+* HeathS: WIXBUG:4420 - Suppress patch sequence data to optimize patch performance
+
+* HeathS: WIXBUG:4366 - Correctly enum all products for non-specific patches
+
 * BobArnson: WIXBUG:4443 - Ensure that MsiPackages in a Bundle have ProductVersions that fit in a QWORD, how Burn represents a four-field version number with each field a WORD.
 
 * BobArnson: WIXBUG:3838 - Since the compiler coerces null values to empty strings, check for those in ColumnDefinition.

src/burn/engine/mspengine.cpp

@@ -22,6 +22,7 @@
 struct POSSIBLE_TARGETPRODUCT
 {
     WCHAR wzProductCode[39];
+    LPWSTR pszLocalPackage;
     MSIINSTALLCONTEXT context;
 };
 
@@ -131,8 +132,8 @@ extern "C" HRESULT MspEngineDetectInitialize(
     AssertSz(pPackages->cPatchInfo, "MspEngineDetectInitialize() should only be called if there are MSP packages.");
 
     HRESULT hr = S_OK;
-    POSSIBLE_TARGETPRODUCT* rgPossibleTargetProductCodes = NULL;
-    DWORD cPossibleTargetProductCodes = 0;
+    POSSIBLE_TARGETPRODUCT* rgPossibleTargetProducts = NULL;
+    DWORD cPossibleTargetProducts = 0;
 
 #ifdef DEBUG
     // All patch info should be initialized to zero.
@@ -146,18 +147,28 @@ extern "C" HRESULT MspEngineDetectInitialize(
 
     // Figure out which product codes to target on the machine. In the worst case all products on the machine
     // will be returned.
-    hr = GetPossibleTargetProductCodes(pPackages, &rgPossibleTargetProductCodes, &cPossibleTargetProductCodes);
+    hr = GetPossibleTargetProductCodes(pPackages, &rgPossibleTargetProducts, &cPossibleTargetProducts);
     ExitOnFailure(hr, "Failed to get possible target product codes.");
 
     // Loop through possible target products, testing the collective patch applicability against each product in
     // the appropriate context. Store the result with the appropriate patch package.
-    for (UINT iSearch = 0; iSearch < cPossibleTargetProductCodes; ++iSearch)
+    for (UINT iSearch = 0; iSearch < cPossibleTargetProducts; ++iSearch)
     {
-        POSSIBLE_TARGETPRODUCT* pPossibleTargetProduct = rgPossibleTargetProductCodes + iSearch;
+        const POSSIBLE_TARGETPRODUCT* pPossibleTargetProduct = rgPossibleTargetProducts + iSearch;
 
         LogId(REPORT_STANDARD, MSG_DETECT_CALCULATE_PATCH_APPLICABILITY, pPossibleTargetProduct->wzProductCode, LoggingMsiInstallContext(pPossibleTargetProduct->context));
 
-        hr = WiuDeterminePatchSequence(pPossibleTargetProduct->wzProductCode, NULL, pPossibleTargetProduct->context, pPackages->rgPatchInfo, pPackages->cPatchInfo);
+        if (pPossibleTargetProduct->pszLocalPackage)
+        {
+            // Ignores current machine state to determine just patch applicability.
+            // Superseded and obsolesced patches will be planned separately.
+            hr = WiuDetermineApplicablePatches(pPossibleTargetProduct->pszLocalPackage, pPackages->rgPatchInfo, pPackages->cPatchInfo);
+        }
+        else
+        {
+            hr = WiuDeterminePatchSequence(pPossibleTargetProduct->wzProductCode, NULL, pPossibleTargetProduct->context, pPackages->rgPatchInfo, pPackages->cPatchInfo);
+        }
+
         if (SUCCEEDED(hr))
         {
             for (DWORD iPatchInfo = 0; iPatchInfo < pPackages->cPatchInfo; ++iPatchInfo)
@@ -183,7 +194,14 @@ extern "C" HRESULT MspEngineDetectInitialize(
     }
 
 LExit:
-    ReleaseMem(rgPossibleTargetProductCodes);
+    if (rgPossibleTargetProducts)
+    {
+        for (DWORD i = 0; i < cPossibleTargetProducts; ++i)
+        {
+            ReleaseStr(rgPossibleTargetProducts[i].pszLocalPackage);
+        }
+        MemFree(rgPossibleTargetProducts);
+    }
 
     return hr;
 }
@@ -620,6 +638,7 @@ static HRESULT GetPossibleTargetProductCodes(
 {
     HRESULT hr = S_OK;
     STRINGDICT_HANDLE sdUniquePossibleTargetProductCodes = NULL;
+    BOOL fCheckAll = FALSE;
     WCHAR wzPossibleTargetProductCode[MAX_GUID_CHARS + 1];
 
     // Use a dictionary to ensure we capture unique product codes. Otherwise, we could end up
@@ -642,10 +661,8 @@ static HRESULT GetPossibleTargetProductCodes(
                 hr = AddPossibleTargetProduct(sdUniquePossibleTargetProductCodes, pTargetCode->sczTargetCode, MSIINSTALLCONTEXT_NONE, prgPossibleTargetProducts, pcPossibleTargetProducts);
                 ExitOnFailure(hr, "Failed to add product code to possible target product codes.");
             }
-            else // must be an upgrade code.
+            else if (BURN_PATCH_TARGETCODE_TYPE_UPGRADE == pTargetCode->type)
             {
-                Assert(BURN_PATCH_TARGETCODE_TYPE_UPGRADE == pTargetCode->type);
-
                 // Enumerate all unique related products to the target upgrade code.
                 for (DWORD iProduct = 0; SUCCEEDED(hr); ++iProduct)
                 {
@@ -670,9 +687,22 @@ static HRESULT GetPossibleTargetProductCodes(
                 }
                 ExitOnFailure1(hr, "Failed to enumerate all products to patch related to upgrade code: %ls", pTargetCode->sczTargetCode);
             }
+            else
+            {
+                // The element does not target a specific product.
+                fCheckAll = TRUE;
+
+                break;
+            }
         }
     }
-    else // one more more patches didn't target specific products so we'll search everything on the machine.
+    else
+    {
+        fCheckAll = TRUE;
+    }
+
+    // One or more of the patches do not target a specific product so search everything on the machine.
+    if (fCheckAll)
     {
         for (DWORD iProduct = 0; SUCCEEDED(hr); ++iProduct)
         {
@@ -686,7 +716,7 @@ static HRESULT GetPossibleTargetProductCodes(
             }
             else if (E_BADCONFIGURATION == hr)
             {
-                // Skip product's with bad configuration and continue.
+                // Skip products with bad configuration and continue.
                 LogId(REPORT_STANDARD, MSG_DETECT_BAD_PRODUCT_CONFIGURATION, wzPossibleTargetProductCode);
 
                 hr = S_OK;
@@ -715,6 +745,7 @@ static HRESULT AddPossibleTargetProduct(
     )
 {
     HRESULT hr = S_OK;
+    LPWSTR pszLocalPackage = NULL;
 
     // Only add this possible target code if we haven't queried for it already.
     if (E_NOTFOUND == DictKeyExists(sdUniquePossibleTargetProductCodes, wzPossibleTargetProductCode))
@@ -736,15 +767,32 @@ static HRESULT AddPossibleTargetProduct(
         hr = MemEnsureArraySize(reinterpret_cast<LPVOID*>(prgPossibleTargetProducts), *pcPossibleTargetProducts + 1, sizeof(POSSIBLE_TARGETPRODUCT), 3);
         ExitOnFailure(hr, "Failed to grow array of possible target products.");
 
-        hr = ::StringCchCopyW((*prgPossibleTargetProducts)[*pcPossibleTargetProducts].wzProductCode, countof(prgPossibleTargetProducts[*pcPossibleTargetProducts]->wzProductCode), wzPossibleTargetProductCode);
+        POSSIBLE_TARGETPRODUCT *const pPossibleTargetProduct = *prgPossibleTargetProducts + *pcPossibleTargetProducts;
+
+        hr = ::StringCchCopyW(pPossibleTargetProduct->wzProductCode, countof(pPossibleTargetProduct->wzProductCode), wzPossibleTargetProductCode);
         ExitOnFailure(hr, "Failed to copy possible target product code.");
 
+        // Attempt to get the local package path so we can more quickly determine patch applicability later.
+        hr = WiuGetProductInfoEx(wzPossibleTargetProductCode, NULL, context, INSTALLPROPERTY_LOCALPACKAGE, &pszLocalPackage);
+        if (SUCCEEDED(hr))
+        {
+            pPossibleTargetProduct->pszLocalPackage = pszLocalPackage;
+            pszLocalPackage = NULL;
+        }
+        else
+        {
+            // Will instead call MsiDeterminePatchSequence later.
+            hr = S_OK;
+        }
+
         (*prgPossibleTargetProducts)[*pcPossibleTargetProducts].context = context;
 
         ++(*pcPossibleTargetProducts);
     }
 
 LExit:
+    ReleaseStr(pszLocalPackage);
+
     return hr;
 }
 

src/chm/documents/msbuild/task_reference/light.html.md

@@ -410,6 +410,14 @@ The following table describes the parameters that are specific to the <b>Light</
     Specifies that the linker should suppress processing the data in the MsiAssembly table. This is equivalent to the -sma switch in light.exe.</td>
   </tr>
 
+  <tr>
+    <td><b>SuppressPatchSequenceData</b></td>
+
+    <td>Optional <b>boolean</b> parameter.<br />
+    <br />
+    Specifies that the linker should suppress patch sequence data in patch XML to decrease bundle size and increase patch applicability performance (patch packages themselves are not modified).</td>
+  </tr>
+
   <tr>
     <td><b>SuppressPdbOutput</b></td>
 

src/libs/dutil/inc/wiutil.h

@@ -171,7 +171,12 @@ typedef UINT (WINAPI *PFN_MSIDETERMINEPATCHSEQUENCEW)(
     __in DWORD cPatchInfo,
     __in PMSIPATCHSEQUENCEINFOW pPatchInfo
     );
-typedef UINT (WINAPI *PFN_MSIINSTALLPRODUCTW)(
+typedef UINT(WINAPI *PFN_MSIDETERMINEAPPLICABLEPATCHESW)(
+    __in_z LPCWSTR wzProductPackagePath,
+    __in DWORD cPatchInfo,
+    __in PMSIPATCHSEQUENCEINFOW pPatchInfo
+    );
+typedef UINT(WINAPI *PFN_MSIINSTALLPRODUCTW)(
     __in LPCWSTR szPackagePath,
     __in_opt LPCWSTR szCommandLine
     );
@@ -300,6 +305,11 @@ HRESULT DAPI WiuDeterminePatchSequence(
     __in PMSIPATCHSEQUENCEINFOW pPatchInfo,
     __in DWORD cPatchInfo
     );
+HRESULT DAPI WiuDetermineApplicablePatches(
+    __in_z LPCWSTR wzProductPackagePath,
+    __in PMSIPATCHSEQUENCEINFOW pPatchInfo,
+    __in DWORD cPatchInfo
+    );
 HRESULT DAPI WiuEnumProducts(
     __in DWORD iProductIndex,
     __out_ecount(MAX_GUID_CHARS + 1) LPWSTR wzProductCode

src/libs/dutil/wiutil.cpp

@@ -38,6 +38,7 @@ static PFN_MSIENUMRELATEDPRODUCTSW vpfnMsiEnumRelatedProductsW = ::MsiEnumRelate
 
 // MSI 3.0+
 static PFN_MSIDETERMINEPATCHSEQUENCEW vpfnMsiDeterminePatchSequenceW = NULL;
+static PFN_MSIDETERMINEAPPLICABLEPATCHESW vpfnMsiDetermineApplicablePatchesW = NULL;
 static PFN_MSIENUMPRODUCTSEXW vpfnMsiEnumProductsExW = NULL;
 static PFN_MSIGETPATCHINFOEXW vpfnMsiGetPatchInfoExW = NULL;
 static PFN_MSIGETPRODUCTINFOEXW vpfnMsiGetProductInfoExW = NULL;
@@ -46,6 +47,7 @@ static PFN_MSISOURCELISTADDSOURCEEXW vpfnMsiSourceListAddSourceExW = NULL;
 
 static HMODULE vhMsiDll = NULL;
 static PFN_MSIDETERMINEPATCHSEQUENCEW vpfnMsiDeterminePatchSequenceWFromLibrary = NULL;
+static PFN_MSIDETERMINEAPPLICABLEPATCHESW vpfnMsiDetermineApplicablePatchesWFromLibrary = NULL;
 static PFN_MSIENUMPRODUCTSEXW vpfnMsiEnumProductsExWFromLibrary = NULL;
 static PFN_MSIGETPATCHINFOEXW vpfnMsiGetPatchInfoExWFromLibrary = NULL;
 static PFN_MSIGETPRODUCTINFOEXW vpfnMsiGetProductInfoExWFromLibrary = NULL;
@@ -127,7 +129,7 @@ void UninitializeMessageData(
 
 
 /********************************************************************
- WiuInitialize - initializes wioutil
+ WiuInitialize - initializes wiutil
 
 *********************************************************************/
 extern "C" HRESULT DAPI WiuInitialize(
@@ -148,6 +150,12 @@ extern "C" HRESULT DAPI WiuInitialize(
         vpfnMsiDeterminePatchSequenceW = vpfnMsiDeterminePatchSequenceWFromLibrary;
     }
 
+    vpfnMsiDetermineApplicablePatchesWFromLibrary = reinterpret_cast<PFN_MSIDETERMINEAPPLICABLEPATCHESW>(::GetProcAddress(vhMsiDll, "MsiDetermineApplicablePatchesW"));
+    if (NULL == vpfnMsiDetermineApplicablePatchesW)
+    {
+        vpfnMsiDetermineApplicablePatchesW = vpfnMsiDetermineApplicablePatchesWFromLibrary;
+    }
+
     vpfnMsiEnumProductsExWFromLibrary = reinterpret_cast<PFN_MSIENUMPRODUCTSEXW>(::GetProcAddress(vhMsiDll, "MsiEnumProductsExW"));
     if (NULL == vpfnMsiEnumProductsExW)
     {
@@ -202,6 +210,7 @@ extern "C" void DAPI WiuUninitialize(
         vpfnMsiGetProductInfoExWFromLibrary = NULL;
         vpfnMsiGetPatchInfoExWFromLibrary = NULL;
         vpfnMsiEnumProductsExWFromLibrary = NULL;
+        vpfnMsiDetermineApplicablePatchesWFromLibrary = NULL;
         vpfnMsiDeterminePatchSequenceWFromLibrary = NULL;
         vpfnMsiSourceListAddSourceExWFromLibrary = NULL;
     }
@@ -530,6 +539,28 @@ extern "C" HRESULT DAPI WiuDeterminePatchSequence(
 }
 
 
+extern "C" HRESULT DAPI WiuDetermineApplicablePatches(
+    __in_z LPCWSTR wzProductPackagePath,
+    __in PMSIPATCHSEQUENCEINFOW pPatchInfo,
+    __in DWORD cPatchInfo
+    )
+{
+    HRESULT hr = S_OK;
+    DWORD er = ERROR_SUCCESS;
+
+    if (!vpfnMsiDetermineApplicablePatchesW)
+    {
+        ExitFunction1(hr = E_NOTIMPL);
+    }
+
+    er = vpfnMsiDetermineApplicablePatchesW(wzProductPackagePath, cPatchInfo, pPatchInfo);
+    ExitOnWin32Error(er, hr, "Failed to determine applicable patches for product package.");
+
+LExit:
+    return hr;
+}
+
+
 extern "C" HRESULT DAPI WiuEnumProducts(
     __in DWORD iProductIndex,
     __out_ecount(MAX_GUID_CHARS + 1) LPWSTR wzProductCode

src/tools/WixTasks/Light.cs

@@ -72,6 +72,7 @@ public sealed class Light : WixToolTask
         private bool suppressLayout;
         private bool suppressLocalization;
         private bool suppressMsiAssemblyTableProcessing;
+        private bool suppressPatchSequenceData;
         private bool suppressPdbOutput;
         private bool suppressSchemaValidation;
         private bool suppressValidation;
@@ -242,6 +243,12 @@ public ITaskItem OutputFile
             set { this.outputFile = value; }
         }
 
+        public bool SuppressPatchSequenceData
+        {
+            get { return this.suppressPatchSequenceData; }
+            set { this.suppressPatchSequenceData = value; }
+        }
+
         [Output]
         public ITaskItem PdbOutputFile
         {
@@ -488,6 +495,7 @@ protected override void BuildCommandLine(WixCommandLineBuilder commandLineBuilde
             commandLineBuilder.AppendIfTrue("-sl", this.SuppressLayout);
             commandLineBuilder.AppendIfTrue("-sloc", this.SuppressLocalization);
             commandLineBuilder.AppendIfTrue("-spdb", this.SuppressPdbOutput);
+            commandLineBuilder.AppendIfTrue("-spsd", this.SuppressPatchSequenceData);
             commandLineBuilder.AppendIfTrue("-ss", this.SuppressSchemaValidation);
             commandLineBuilder.AppendIfTrue("-sts", this.SuppressTagSectionIdAttributeOnTuples);
             commandLineBuilder.AppendIfTrue("-sui", this.SuppressDefaultUISequenceActions);

src/tools/WixTasks/wix200x.targets

@@ -2520,6 +2520,7 @@
       NoLogo="$(LinkerNoLogo)"
       OutputAsXml="$(OutputAsXml)"
       OutputFile="$(TargetDir)%(CultureGroup.OutputFolder)$(TargetName)$(TargetExt)"
+      SuppressPatchSequenceData="$(SuppressPatchSequenceData)"
       PdbOutputFile="$(PdbOutputFile)"
       Pedantic="$(LinkerPedantic)"
       ReferencePaths="$(ReferencePaths)"

src/tools/WixTasks/wix2010.targets

@@ -2514,6 +2514,7 @@
       SuppressLayout="$(SuppressLayout)"
       SuppressLocalization="$(SuppressLocalization)"
       SuppressMsiAssemblyTableProcessing="$(SuppressMsiAssemblyTableProcessing)"
+      SuppressPatchSequenceData="$(SuppressPatchSequenceData)"
       SuppressPdbOutput="$(SuppressPdbOutput)"
       SuppressSchemaValidation="$(LinkerSuppressSchemaValidation)"
       SuppressValidation="$(SuppressValidation)"

src/tools/wix/Binder.cs

@@ -93,6 +93,8 @@ public sealed class Binder : WixBinder, IDisposable
         // as outlined in RFC 4122, this is our namespace for generating name-based (version 3) UUIDs
         private static readonly Guid WixComponentGuidNamespace = new Guid("{3064E5C6-FB63-4FE9-AC49-E446A792EFA5}");
 
+        internal static readonly string PARAM_SPSD_NAME = "SuppressPatchSequenceData";
+
         // The following constants must stay in sync with src\burn\engine\core.h
         private const string BURN_BUNDLE_NAME = "WixBundleName";
         private const string BURN_BUNDLE_ORIGINAL_SOURCE = "WixBundleOriginalSource";
@@ -114,6 +116,7 @@ public sealed class Binder : WixBinder, IDisposable
         private bool suppressFileHashAndInfo;
         private StringCollection suppressICEs;
         private bool suppressLayout;
+        private bool suppressPatchSequenceData;
         private bool suppressWixPdb;
         private bool suppressValidation;
 
@@ -477,6 +480,10 @@ public override StringCollection ParseCommandLine(string[] args, ConsoleMessageH
                     {
                         this.suppressWixPdb = true;
                     }
+                    else if (parameter.Equals("spsd", StringComparison.Ordinal))
+                    {
+                        this.suppressPatchSequenceData = true;
+                    }
                     else if (parameter.Equals("sval", StringComparison.Ordinal))
                     {
                         this.suppressValidation = true;
@@ -580,6 +587,13 @@ public override bool Bind(Output output, string file)
             this.core = new BinderCore(this.MessageHandler);
             this.FileManager.MessageHandler = this.core;
 
+            // Add properties we need to pass to other classes.
+            if (this.suppressPatchSequenceData)
+            {
+                // Avoid unnecessary boxing if false.
+                this.core.SetProperty(Binder.PARAM_SPSD_NAME, true);
+            }
+
             foreach (BinderExtension extension in this.extensions)
             {
                 extension.Core = this.core;