Add default rollback boundary in the Burn binder instead of the compiler

Fixes wixtoolset/issues#6312

4 files changed, 35 insertions, 31 deletions

diff --git a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
index 12a530ae..e58e2464 100644
--- a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
+++ b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
@@ -357,10 +357,7 @@ namespace WixToolset.Core.Burn
             IEnumerable<PackageFacade> orderedFacades;
             IEnumerable<WixBundleRollbackBoundarySymbol> boundaries;
             {
-                var groupSymbols = section.Symbols.OfType<WixGroupSymbol>();
-                var boundarySymbolsById = section.Symbols.OfType<WixBundleRollbackBoundarySymbol>().ToDictionary(b => b.Id.Id);
-
-                var command = new OrderPackagesAndRollbackBoundariesCommand(this.Messaging, groupSymbols, boundarySymbolsById, facades);
+                var command = new OrderPackagesAndRollbackBoundariesCommand(this.Messaging, section, facades);
                 command.Execute();
 
                 orderedFacades = command.OrderedPackageFacades;
diff --git a/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs b/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
index 44299fd5..f16ac707 100644
--- a/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
+++ b/src/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
@@ -4,27 +4,27 @@ namespace WixToolset.Core.Burn.Bundles
 {
     using System;
     using System.Collections.Generic;
+    using System.Linq;
     using WixToolset.Data;
     using WixToolset.Data.Symbols;
     using WixToolset.Extensibility.Services;
 
     internal class OrderPackagesAndRollbackBoundariesCommand
     {
-        public OrderPackagesAndRollbackBoundariesCommand(IMessaging messaging, IEnumerable<WixGroupSymbol> groupSymbols, Dictionary<string, WixBundleRollbackBoundarySymbol> boundarySymbols, IDictionary<string, PackageFacade> packageFacades)
+        private const string DefaultBoundaryId = "WixDefaultBoundary";
+
+        public OrderPackagesAndRollbackBoundariesCommand(IMessaging messaging, IntermediateSection section, IDictionary<string, PackageFacade> packageFacades)
         {
             this.Messaging = messaging;
-            this.GroupSymbols = groupSymbols;
-            this.Boundaries = boundarySymbols;
+            this.Section = section;
             this.PackageFacades = packageFacades;
         }
 
         private IMessaging Messaging { get; }
 
-        public IEnumerable<WixGroupSymbol> GroupSymbols { get; }
-
-        public Dictionary<string, WixBundleRollbackBoundarySymbol> Boundaries { get; }
+        private IntermediateSection Section { get; }
 
-        public IDictionary<string, PackageFacade> PackageFacades { get; }
+        private IDictionary<string, PackageFacade> PackageFacades { get; }
 
         public IEnumerable<PackageFacade> OrderedPackageFacades { get; private set; }
 
@@ -32,6 +32,9 @@ namespace WixToolset.Core.Burn.Bundles
 
         public void Execute()
         {
+            var groupSymbols = this.Section.Symbols.OfType<WixGroupSymbol>().ToList();
+            var boundariesById = this.Section.Symbols.OfType<WixBundleRollbackBoundarySymbol>().ToDictionary(b => b.Id.Id);
+
             var orderedFacades = new List<PackageFacade>();
             var usedBoundaries = new List<WixBundleRollbackBoundarySymbol>();
 
@@ -44,20 +47,27 @@ namespace WixToolset.Core.Burn.Bundles
             // We handle uninstall (aka: backwards) rollback boundaries after
             // we get these install/repair (aka: forward) rollback boundaries
             // defined.
-            WixBundleRollbackBoundarySymbol pendingRollbackBoundary = null;
-            WixBundleRollbackBoundarySymbol lastRollbackBoundary = null;
+            var pendingRollbackBoundary = new WixBundleRollbackBoundarySymbol(null, new Identifier(AccessModifier.Section, DefaultBoundaryId)) { Vital = true };
+            var lastRollbackBoundary = pendingRollbackBoundary;
             var boundaryHadX86Package = false;
             var warnedMsiTransaction = false;
 
-            foreach (var groupSymbol in this.GroupSymbols)
+            foreach (var groupSymbol in groupSymbols)
             {
                 if (ComplexReferenceChildType.Package == groupSymbol.ChildType && ComplexReferenceParentType.PackageGroup == groupSymbol.ParentType && "WixChain" == groupSymbol.ParentId)
                 {
                     if (this.PackageFacades.TryGetValue(groupSymbol.ChildId, out var facade))
                     {
-                        var insideMsiTransaction = lastRollbackBoundary != null && lastRollbackBoundary.Transaction.HasValue && lastRollbackBoundary.Transaction.Value;
+                        var insideMsiTransaction = lastRollbackBoundary?.Transaction ?? false;
+
                         if (null != pendingRollbackBoundary)
                         {
+                            // If we used the default boundary, ensure the symbol is added to the section.
+                            if (pendingRollbackBoundary.Id.Id == DefaultBoundaryId)
+                            {
+                                this.Section.Symbols.Add(pendingRollbackBoundary);
+                            }
+
                             if (insideMsiTransaction && !warnedMsiTransaction)
                             {
                                 warnedMsiTransaction = true;
@@ -72,12 +82,11 @@ namespace WixToolset.Core.Burn.Bundles
                         }
 
                         // Error if MSI transaction has x86 package preceding x64 packages
-                        if (insideMsiTransaction
-                            && boundaryHadX86Package
-                            && facade.PackageSymbol.Win64)
+                        if (insideMsiTransaction && boundaryHadX86Package && facade.PackageSymbol.Win64)
                         {
                             this.Messaging.Write(ErrorMessages.MsiTransactionX86BeforeX64(facade.PackageSymbol.SourceLineNumbers));
                         }
+
                         boundaryHadX86Package |= !facade.PackageSymbol.Win64;
 
                         orderedFacades.Add(facade);
@@ -85,15 +94,16 @@ namespace WixToolset.Core.Burn.Bundles
                     else // must be a rollback boundary.
                     {
                         // Discard the next rollback boundary if we have a previously defined boundary.
-                        var nextRollbackBoundary = this.Boundaries[groupSymbol.ChildId];
+                        var nextRollbackBoundary = boundariesById[groupSymbol.ChildId];
                         if (null != pendingRollbackBoundary)
                         {
-                            this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(nextRollbackBoundary.SourceLineNumbers, nextRollbackBoundary.Id.Id));
-                        }
-                        else
-                        {
-                            lastRollbackBoundary = pendingRollbackBoundary = nextRollbackBoundary;
+                            if (pendingRollbackBoundary.Id.Id != DefaultBoundaryId)
+                            {
+                                this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(nextRollbackBoundary.SourceLineNumbers, nextRollbackBoundary.Id.Id));
+                            }
                         }
+
+                        lastRollbackBoundary = pendingRollbackBoundary = nextRollbackBoundary;
                     }
                 }
             }
@@ -135,7 +145,7 @@ namespace WixToolset.Core.Burn.Bundles
             string previousRollbackBoundaryId = null;
             PackageFacade previousFacade = null;
 
-            foreach (PackageFacade package in orderedFacades)
+            foreach (var package in orderedFacades)
             {
                 if (null != package.PackageSymbol.RollbackBoundaryRef)
                 {
diff --git a/src/WixToolset.Core/Compiler_Bundle.cs b/src/WixToolset.Core/Compiler_Bundle.cs
index 89ca94ba..cc4550a8 100644
--- a/src/WixToolset.Core/Compiler_Bundle.cs
+++ b/src/WixToolset.Core/Compiler_Bundle.cs
@@ -1726,11 +1726,8 @@ namespace WixToolset.Core
                 }
             }
 
-            // Ensure there is always a rollback boundary at the beginning of the chain.
-            this.CreateRollbackBoundary(sourceLineNumbers, new Identifier(AccessModifier.Global, "WixDefaultBoundary"), YesNoType.Yes, YesNoType.No, ComplexReferenceParentType.PackageGroup, "WixChain", ComplexReferenceChildType.Unknown, null);
-
-            var previousId = "WixDefaultBoundary";
-            var previousType = ComplexReferenceChildType.Package;
+            string previousId = null;
+            var previousType = ComplexReferenceChildType.Unknown;
 
             foreach (var child in node.Elements())
             {
diff --git a/src/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs b/src/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
index b713920c..9e19abb0 100644
--- a/src/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
@@ -9,7 +9,7 @@ namespace WixToolsetTest.CoreIntegration
 
     public class RollbackBoundaryFixture
     {
-        [Fact(Skip = "Test demonstrates failure")]
+        [Fact]
         public void CanStartChainWithRollbackBoundary()
         {
             var folder = TestData.Get(@"TestData");