Discard the correct RollbackBoundary.

5 files changed, 135 insertions, 11 deletions

diff --git a/src/wix/WixToolset.Core.Burn/Bind/GenerateManifestDataFromIRCommand.cs b/src/wix/WixToolset.Core.Burn/Bind/GenerateManifestDataFromIRCommand.cs
index 9d214211..396e0eff 100644
--- a/src/wix/WixToolset.Core.Burn/Bind/GenerateManifestDataFromIRCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bind/GenerateManifestDataFromIRCommand.cs
@@ -94,6 +94,7 @@ namespace WixToolset.Core.Burn.Bind
                     case SymbolDefinitionType.WixBundleVariable:
                     case SymbolDefinitionType.WixBuildInfo:
                     case SymbolDefinitionType.WixChain:
+                    case SymbolDefinitionType.WixChainItem:
                     case SymbolDefinitionType.WixComponentSearch:
                     case SymbolDefinitionType.WixDependencyProvider:
                     case SymbolDefinitionType.WixFileSearch:
@@ -114,10 +115,7 @@ namespace WixToolset.Core.Burn.Bind
                     case SymbolDefinitionType.WixVariable:
                         break;
 
-                    // Symbols to investigate:
-                    case SymbolDefinitionType.WixChainItem:
-                        break;
-
+                    // All other symbols need to be processed.
                     case SymbolDefinitionType.WixBundleCustomData:
                         unknownSymbol = !this.IndexBundleCustomDataSymbol((WixBundleCustomDataSymbol)symbol, customDataById);
                         break;
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
index ccf6b1c2..df798583 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/OrderPackagesAndRollbackBoundariesCommand.cs
@@ -94,15 +94,14 @@ namespace WixToolset.Core.Burn.Bundles
                     {
                         // Discard the next rollback boundary if we have a previously defined boundary.
                         var nextRollbackBoundary = boundariesById[groupSymbol.ChildId];
-                        if (null != pendingRollbackBoundary)
+                        if (null != pendingRollbackBoundary && pendingRollbackBoundary.Id.Id != BurnConstants.BundleDefaultBoundaryId)
                         {
-                            if (pendingRollbackBoundary.Id.Id != BurnConstants.BundleDefaultBoundaryId)
-                            {
-                                this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(nextRollbackBoundary.SourceLineNumbers, nextRollbackBoundary.Id.Id));
-                            }
+                            this.Messaging.Write(WarningMessages.DiscardedRollbackBoundary(nextRollbackBoundary.SourceLineNumbers, nextRollbackBoundary.Id.Id));
+                        }
+                        else
+                        {
+                            lastRollbackBoundary = pendingRollbackBoundary = nextRollbackBoundary;
                         }
-
-                        lastRollbackBoundary = pendingRollbackBoundary = nextRollbackBoundary;
                     }
                 }
             }
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
index 9e19abb0..c69ff894 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/RollbackBoundaryFixture.cs
@@ -2,7 +2,10 @@
 
 namespace WixToolsetTest.CoreIntegration
 {
+    using System.Collections.Generic;
     using System.IO;
+    using System.Linq;
+    using System.Xml;
     using WixBuildTools.TestSupport;
     using WixToolset.Core.TestPackage;
     using Xunit;
@@ -37,5 +40,107 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
 
+        [Fact]
+        public void CannotHaveRollbackBoundaryAndChainPackageWithSameId()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var exePath = Path.Combine(baseFolder, @"bin\test.exe");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "RollbackBoundary", "SharedIdWithPackage.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "MinimalPackageGroup.wxs"),
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", exePath,
+                });
+
+                Assert.Equal(92, result.ExitCode);
+
+                WixAssert.CompareLineByLine(new[]
+                {
+                    "Duplicate symbol 'WixChainItem:collision' found. This typically means that an Id is duplicated. Access modifiers (internal, protected, private) cannot prevent these conflicts. Ensure all your identifiers of a given type (File, Component, Feature) are unique.",
+                    "Location of symbol related to previous error.",
+                }, result.Messages.Select(m => m.ToString()).ToArray());
+
+                Assert.False(File.Exists(exePath));
+            }
+        }
+
+        [Fact]
+        public void DiscardsConsecutiveRollbackBoundaries()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var exePath = Path.Combine(baseFolder, @"bin\test.exe");
+                var baFolderPath = Path.Combine(baseFolder, "ba");
+                var extractFolderPath = Path.Combine(baseFolder, "extract");
+
+                var result = WixRunner.Execute(false, new[]
+                {
+                    "build",
+                    Path.Combine(folder, "RollbackBoundary", "ConsecutiveRollbackBoundaries.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "MinimalPackageGroup.wxs"),
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", exePath,
+                });
+
+                result.AssertSuccess();
+
+                WixAssert.CompareLineByLine(new[]
+                {
+                    "The RollbackBoundary 'Second' was discarded because it was not followed by a package. Without a package the rollback boundary doesn't do anything. Verify that the RollbackBoundary element is not followed by another RollbackBoundary and that the element is not at the end of the chain.",
+                }, result.Messages.Select(m => m.ToString()).ToArray());
+
+                Assert.True(File.Exists(exePath));
+
+                var extractResult = BundleExtractor.ExtractBAContainer(null, exePath, baFolderPath, extractFolderPath);
+                extractResult.AssertSuccess();
+
+                var rollbackBoundaries = extractResult.SelectManifestNodes("/burn:BurnManifest/burn:RollbackBoundary")
+                                                      .Cast<XmlElement>()
+                                                      .Select(e => e.GetTestXml())
+                                                      .ToArray();
+                WixAssert.CompareLineByLine(new string[]
+                {
+                    "<RollbackBoundary Id='First' Vital='yes' Transaction='no' />",
+                }, rollbackBoundaries);
+
+                var ignoreAttributesByElementName = new Dictionary<string, List<string>>
+                {
+                    { "MsiPackage", new List<string> { "Size" } },
+                };
+                var chainPackages = extractResult.SelectManifestNodes("/burn:BurnManifest/burn:Chain/*")
+                                                      .Cast<XmlElement>()
+                                                      .Select(e => e.GetTestXml(ignoreAttributesByElementName))
+                                                      .ToArray();
+                WixAssert.CompareLineByLine(new string[]
+                {
+                    "<MsiPackage Id='test.msi' Cache='keep' CacheId='{040011E1-F84C-4927-AD62-50A5EC19CA32}v1.0.0.0' InstallSize='34' Size='*' PerMachine='yes' Permanent='no' Vital='yes' RollbackBoundaryForward='First' RollbackBoundaryBackward='First' LogPathVariable='WixBundleLog_test.msi' RollbackLogPathVariable='WixBundleRollbackLog_test.msi' ProductCode='{040011E1-F84C-4927-AD62-50A5EC19CA32}' Language='1033' Version='1.0.0.0' UpgradeCode='{047730A5-30FE-4A62-A520-DA9381B8226A}'>" +
+                    "<MsiProperty Id='ARPSYSTEMCOMPONENT' Value='1' />" +
+                    "<MsiProperty Id='MSIFASTINSTALL' Value='7' />" +
+                    "<Provides Key='{040011E1-F84C-4927-AD62-50A5EC19CA32}_v1.0.0.0' Version='1.0.0.0' DisplayName='MsiPackage' />" +
+                    "<RelatedPackage Id='{047730A5-30FE-4A62-A520-DA9381B8226A}' MaxVersion='1.0.0.0' MaxInclusive='no' OnlyDetect='no' LangInclusive='yes'><Language Id='1033' /></RelatedPackage>" +
+                    "<RelatedPackage Id='{047730A5-30FE-4A62-A520-DA9381B8226A}' MinVersion='1.0.0.0' MinInclusive='no' OnlyDetect='yes' LangInclusive='yes'><Language Id='1033' /></RelatedPackage>" +
+                    "<PayloadRef Id='test.msi' />" +
+                    "<PayloadRef Id='fhuZsOcBDTuIX8rF96kswqI6SnuI' />" +
+                    "<PayloadRef Id='faf_OZ741BG7SJ6ZkcIvivZ2Yzo8' />" +
+                    "</MsiPackage>",
+                }, chainPackages);
+            }
+        }
     }
 }
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RollbackBoundary/ConsecutiveRollbackBoundaries.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RollbackBoundary/ConsecutiveRollbackBoundaries.wxs
new file mode 100644
index 00000000..e12b5656
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RollbackBoundary/ConsecutiveRollbackBoundaries.wxs
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="PreBundlePackages">
+            <RollbackBoundary Id="First" />
+        </PackageGroup>
+        <PackageGroup Id="BundlePackages">
+            <PackageGroupRef Id="PreBundlePackages" />
+            <RollbackBoundary Id="Second" />
+            <MsiPackage SourceFile="test.msi" />
+        </PackageGroup>
+    </Fragment>
+</Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RollbackBoundary/SharedIdWithPackage.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RollbackBoundary/SharedIdWithPackage.wxs
new file mode 100644
index 00000000..8a7a21a7
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RollbackBoundary/SharedIdWithPackage.wxs
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="BundlePackages">
+            <RollbackBoundary Id="collision" />
+            <MsiPackage Id="collision" SourceFile="test.msi" />
+        </PackageGroup>
+    </Fragment>
+</Wix>