MSI transaction cleanup.

9 files changed, 212 insertions, 15 deletions

diff --git a/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs b/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
index 9e27d5a3..44299fd5 100644
--- a/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
+++ b/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
@@ -44,9 +44,10 @@ namespace WixToolset.Core.Burn.Bundles
             // We handle uninstall (aka: backwards) rollback boundaries after
             // we get these install/repair (aka: forward) rollback boundaries
             // defined.
-            WixBundleRollbackBoundarySymbol previousRollbackBoundary = null;
+            WixBundleRollbackBoundarySymbol pendingRollbackBoundary = null;
             WixBundleRollbackBoundarySymbol lastRollbackBoundary = null;
             var boundaryHadX86Package = false;
+            var warnedMsiTransaction = false;
 
             foreach (var groupSymbol in this.GroupSymbols)
             {
@@ -54,24 +55,30 @@ namespace WixToolset.Core.Burn.Bundles
                 {
                     if (this.PackageFacades.TryGetValue(groupSymbol.ChildId, out var facade))
                     {
-                        if (null != previousRollbackBoundary)
+                        var insideMsiTransaction = lastRollbackBoundary != null && lastRollbackBoundary.Transaction.HasValue && lastRollbackBoundary.Transaction.Value;
+                        if (null != pendingRollbackBoundary)
                         {
-                            usedBoundaries.Add(previousRollbackBoundary);
-                            facade.PackageSymbol.RollbackBoundaryRef = previousRollbackBoundary.Id.Id;
-                            previousRollbackBoundary = null;
+                            if (insideMsiTransaction && !warnedMsiTransaction)
+                            {
+                                warnedMsiTransaction = true;
+                                this.Messaging.Write(WarningMessages.MsiTransactionLimitations(pendingRollbackBoundary.SourceLineNumbers));
+                            }
 
-                            boundaryHadX86Package = facade.PackageSymbol.Win64;
+                            usedBoundaries.Add(pendingRollbackBoundary);
+                            facade.PackageSymbol.RollbackBoundaryRef = pendingRollbackBoundary.Id.Id;
+                            pendingRollbackBoundary = null;
+
+                            boundaryHadX86Package = !facade.PackageSymbol.Win64;
                         }
 
                         // Error if MSI transaction has x86 package preceding x64 packages
-                        if ((lastRollbackBoundary != null)
-                            && lastRollbackBoundary.Transaction == true
+                        if (insideMsiTransaction
                             && boundaryHadX86Package
                             && facade.PackageSymbol.Win64)
                         {
-                            this.Messaging.Write(ErrorMessages.MsiTransactionX86BeforeX64(lastRollbackBoundary.SourceLineNumbers));
+                            this.Messaging.Write(ErrorMessages.MsiTransactionX86BeforeX64(facade.PackageSymbol.SourceLineNumbers));
                         }
-                        boundaryHadX86Package |= facade.PackageSymbol.Win64;
+                        boundaryHadX86Package |= !facade.PackageSymbol.Win64;
 
                         orderedFacades.Add(facade);
                     }
@@ -79,22 +86,21 @@ namespace WixToolset.Core.Burn.Bundles
                     {
                         // Discard the next rollback boundary if we have a previously defined boundary.
                         var nextRollbackBoundary = this.Boundaries[groupSymbol.ChildId];
-                        if (null != previousRollbackBoundary)
+                        if (null != pendingRollbackBoundary)
                         {
                             this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(nextRollbackBoundary.SourceLineNumbers, nextRollbackBoundary.Id.Id));
                         }
                         else
                         {
-                            previousRollbackBoundary = nextRollbackBoundary;
-                            lastRollbackBoundary = nextRollbackBoundary;
+                            lastRollbackBoundary = pendingRollbackBoundary = nextRollbackBoundary;
                         }
                     }
                 }
             }
 
-            if (null != previousRollbackBoundary)
+            if (null != pendingRollbackBoundary)
             {
-                this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(previousRollbackBoundary.SourceLineNumbers, previousRollbackBoundary.Id.Id));
+                this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(pendingRollbackBoundary.SourceLineNumbers, pendingRollbackBoundary.Id.Id));
             }
 
             // With the forward rollback boundaries assigned, we can now go
diff --git a/src/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs b/src/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs
new file mode 100644
index 00000000..5a29eb9e
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/MsiTransactionFixture.cs
@@ -0,0 +1,129 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
+
+namespace WixToolsetTest.CoreIntegration
+{
+    using System.IO;
+    using WixBuildTools.TestSupport;
+    using WixToolset.Core.TestPackage;
+    using Xunit;
+
+    public class MsiTransactionFixture
+    {
+        [Fact]
+        public void CantBuildX64AfterX86Bundle()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var binFolder = Path.Combine(baseFolder, "bin");
+                var exePath = Path.Combine(binFolder, "test.exe");
+
+                BuildMsiPackages(folder, intermediateFolder, binFolder);
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "MsiTransaction", "X64AfterX86Bundle.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-bindpath", binFolder,
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", exePath,
+                });
+
+                Assert.Equal(390, result.ExitCode);
+            }
+        }
+
+        [Fact]
+        public void CanBuildX86AfterX64Bundle()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var binFolder = Path.Combine(baseFolder, "bin");
+                var exePath = Path.Combine(binFolder, "test.exe");
+
+                BuildMsiPackages(folder, intermediateFolder, binFolder);
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "MsiTransaction", "X86AfterX64Bundle.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-bindpath", binFolder,
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", exePath,
+                });
+
+                result.AssertSuccess();
+
+                Assert.True(File.Exists(exePath));
+            }
+        }
+
+        private static void BuildMsiPackages(string folder, string intermediateFolder, string binFolder)
+        {
+            var result = WixRunner.Execute(new[]
+            {
+                "build",
+                Path.Combine(folder, "MsiTransaction", "FirstX86.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
+                "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                "-intermediateFolder", intermediateFolder,
+                "-o", Path.Combine(binFolder, "FirstX86.msi"),
+            });
+
+            result.AssertSuccess();
+
+            result = WixRunner.Execute(new[]
+            {
+                "build",
+                Path.Combine(folder, "MsiTransaction", "SecondX86.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
+                "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                "-intermediateFolder", intermediateFolder,
+                "-o", Path.Combine(binFolder, "SecondX86.msi"),
+            });
+
+            result.AssertSuccess();
+
+            result = WixRunner.Execute(new[]
+            {
+                "build",
+                Path.Combine(folder, "MsiTransaction", "FirstX64.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
+                "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                "-intermediateFolder", intermediateFolder,
+                "-arch", "x64",
+                "-o", Path.Combine(binFolder, "FirstX64.msi"),
+            });
+
+            result.AssertSuccess();
+
+            result = WixRunner.Execute(new[]
+            {
+                "build",
+                Path.Combine(folder, "MsiTransaction", "SecondX64.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "MinimalComponentGroup.wxs"),
+                Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
+                "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                "-intermediateFolder", intermediateFolder,
+                "-arch", "x64",
+                "-o", Path.Combine(binFolder, "SecondX64.msi"),
+            });
+
+            result.AssertSuccess();
+        }
+    }
+}
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/FirstX64.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/FirstX64.wxs
new file mode 100644
index 00000000..e72b6402
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/FirstX64.wxs
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <ComponentGroup Id="ProductComponents">
+            <ComponentGroupRef Id="MinimalComponentGroup" />
+        </ComponentGroup>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/FirstX86.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/FirstX86.wxs
new file mode 100644
index 00000000..e72b6402
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/FirstX86.wxs
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <ComponentGroup Id="ProductComponents">
+            <ComponentGroupRef Id="MinimalComponentGroup" />
+        </ComponentGroup>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/SecondX64.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/SecondX64.wxs
new file mode 100644
index 00000000..e72b6402
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/SecondX64.wxs
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <ComponentGroup Id="ProductComponents">
+            <ComponentGroupRef Id="MinimalComponentGroup" />
+        </ComponentGroup>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/SecondX86.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/SecondX86.wxs
new file mode 100644
index 00000000..e72b6402
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/SecondX86.wxs
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <ComponentGroup Id="ProductComponents">
+            <ComponentGroupRef Id="MinimalComponentGroup" />
+        </ComponentGroup>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/X64AfterX86Bundle.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/X64AfterX86Bundle.wxs
new file mode 100644
index 00000000..8f4fc8bd
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/X64AfterX86Bundle.wxs
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="BundlePackages">
+            <MsiPackage SourceFile="FirstX64.msi" />
+            <RollbackBoundary Transaction="yes" />
+            <MsiPackage SourceFile="FirstX86.msi" />
+            <MsiPackage SourceFile="SecondX86.msi" />
+            <MsiPackage SourceFile="SecondX64.msi" />
+        </PackageGroup>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/X86AfterX64Bundle.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/X86AfterX64Bundle.wxs
new file mode 100644
index 00000000..221f06c5
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/MsiTransaction/X86AfterX64Bundle.wxs
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="BundlePackages">
+            <MsiPackage SourceFile="FirstX86.msi" />
+            <RollbackBoundary Transaction="yes" />
+            <MsiPackage SourceFile="FirstX64.msi" />
+            <MsiPackage SourceFile="SecondX64.msi" />
+            <MsiPackage SourceFile="SecondX86.msi" />
+        </PackageGroup>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj b/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
index fdb56987..8e5a005c 100644
--- a/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
+++ b/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
@@ -64,6 +64,12 @@
     <Content Include="TestData\Font\TrueType.wxs" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\Icon\SampleIcon.wxs" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\LockPermissions\EmptyPermissions.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\MsiTransaction\FirstX64.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\MsiTransaction\FirstX86.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\MsiTransaction\SecondX64.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\MsiTransaction\SecondX86.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\MsiTransaction\X64AfterX86Bundle.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\MsiTransaction\X86AfterX64Bundle.wxs" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\PatchFamilyFilter\.data\Av1.0.0.txt" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\PatchFamilyFilter\.data\Av1.0.1.txt" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\PatchFamilyFilter\.data\Bv1.0.0.txt" CopyToOutputDirectory="PreserveNewest" />