Fix overridable actions being tagged as duplicates.

9 files changed, 165 insertions, 10 deletions

diff --git a/src/WixToolset.Core.TestPackage/WixRunnerResult.cs b/src/WixToolset.Core.TestPackage/WixRunnerResult.cs
index 8fc7e14f..f4870ae3 100644
--- a/src/WixToolset.Core.TestPackage/WixRunnerResult.cs
+++ b/src/WixToolset.Core.TestPackage/WixRunnerResult.cs
@@ -1,9 +1,9 @@
-// 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.
+// 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 WixToolset.Core.TestPackage
 {
     using System;
-    using System.Linq;
+    using System.Collections.Generic;
     using WixToolset.Data;
     using Xunit;
 
@@ -15,8 +15,26 @@ namespace WixToolset.Core.TestPackage
 
         public WixRunnerResult AssertSuccess()
         {
-            Assert.True(0 == this.ExitCode, $"\r\n\r\nWixRunner failed with exit code: {this.ExitCode}\r\n   Output: {String.Join("\r\n           ", this.Messages.Select(m => m.ToString()).ToArray())}\r\n");
+            Assert.True(0 == this.ExitCode, $"\r\n\r\nWixRunner failed with exit code: {this.ExitCode}\r\n   Output: {String.Join("\r\n           ", FormatMessages(this.Messages))}\r\n");
             return this;
         }
+
+        private static IEnumerable<string> FormatMessages(IEnumerable<Message> messages)
+        {
+            foreach (var message in messages)
+            {
+                var filename = message.SourceLineNumbers?.FileName ?? "TEST";
+                var line = message.SourceLineNumbers?.LineNumber ?? -1;
+                var type = message.Level.ToString().ToLowerInvariant();
+                var output = message.Level >= MessageLevel.Warning ? Console.Out : Console.Error;
+
+                if (line > 0)
+                {
+                    filename = String.Concat(filename, "(", line, ")");
+                }
+
+                yield return String.Format("{0} : {1} {2}{3:0000}: {4}", filename, type, "TEST", message.Id, message.ToString());
+            }
+        }
     }
 }
diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs
index 0a356ba9..20df1fe8 100644
--- a/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs
+++ b/src/WixToolset.Core.WindowsInstaller/Bind/SequenceActionsCommand.cs
@@ -92,7 +92,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
                         }
                     }
 
-                    if (overridableActionRows.TryGetValue(actionRow.Id.Id, out var collidingActionRow))
+                    if (requiredActionRows.TryGetValue(actionRow.Id.Id, out var collidingActionRow))
                     {
                         this.Messaging.Write(ErrorMessages.ActionCollision(actionRow.SourceLineNumbers, actionRow.SequenceTable.ToString(), actionRow.Action));
                         if (null != collidingActionRow.SourceLineNumbers)
diff --git a/src/WixToolset.Core/Link/ResolveReferencesCommand.cs b/src/WixToolset.Core/Link/ResolveReferencesCommand.cs
index 07eabbd6..ed11095f 100644
--- a/src/WixToolset.Core/Link/ResolveReferencesCommand.cs
+++ b/src/WixToolset.Core/Link/ResolveReferencesCommand.cs
@@ -66,7 +66,7 @@ namespace WixToolset.Core.Link
             {
                 // If we're building a Merge Module, ignore all references to the Media table
                 // because Merge Modules don't have Media tables.
-                if (this.BuildingMergeModule && wixSimpleReferenceRow.Table== "Media")
+                if (this.BuildingMergeModule && wixSimpleReferenceRow.Table == "Media")
                 {
                     continue;
                 }
@@ -77,7 +77,7 @@ namespace WixToolset.Core.Link
                 }
                 else // see if the symbol (and any of its duplicates) are appropriately accessible.
                 {
-                    IList<Symbol> accessible = DetermineAccessibleSymbols(section, symbol);
+                    IList<Symbol> accessible = this.DetermineAccessibleSymbols(section, symbol);
                     if (!accessible.Any())
                     {
                         this.Messaging.Write(ErrorMessages.UnresolvedReference(wixSimpleReferenceRow.SourceLineNumbers, wixSimpleReferenceRow.SymbolicName, symbol.Access));
@@ -89,7 +89,7 @@ namespace WixToolset.Core.Link
 
                         if (null != accessibleSymbol.Section)
                         {
-                            RecursivelyResolveReferences(accessibleSymbol.Section);
+                            this.RecursivelyResolveReferences(accessibleSymbol.Section);
                         }
                     }
                     else // display errors for the duplicate symbols.
@@ -125,14 +125,22 @@ namespace WixToolset.Core.Link
         {
             List<Symbol> symbols = new List<Symbol>();
 
-            if (AccessibleSymbol(referencingSection, symbol))
+            if (this.AccessibleSymbol(referencingSection, symbol))
             {
                 symbols.Add(symbol);
             }
 
             foreach (Symbol dupe in symbol.PossiblyConflictingSymbols)
             {
-                if (AccessibleSymbol(referencingSection, dupe))
+                // don't count overridable WixActionTuples
+                WixActionTuple symbolAction = symbol.Row as WixActionTuple;
+                WixActionTuple dupeAction = dupe.Row as WixActionTuple;
+                if (symbolAction?.Overridable != dupeAction?.Overridable)
+                {
+                    continue;
+                }
+
+                if (this.AccessibleSymbol(referencingSection, dupe))
                 {
                     symbols.Add(dupe);
                 }
@@ -140,7 +148,7 @@ namespace WixToolset.Core.Link
 
             foreach (Symbol dupe in symbol.RedundantSymbols)
             {
-                if (AccessibleSymbol(referencingSection, dupe))
+                if (this.AccessibleSymbol(referencingSection, dupe))
                 {
                     symbols.Add(dupe);
                 }
diff --git a/src/test/WixToolsetTest.CoreIntegration/LinkerFixture.cs b/src/test/WixToolsetTest.CoreIntegration/LinkerFixture.cs
new file mode 100644
index 00000000..fcbff1aa
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/LinkerFixture.cs
@@ -0,0 +1,54 @@
+
+// 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 System.Linq;
+    using WixBuildTools.TestSupport;
+    using WixToolset.Core.TestPackage;
+    using WixToolset.Data;
+    using WixToolset.Data.Tuples;
+    using WixToolset.Data.WindowsInstaller;
+    using Xunit;
+
+    public class LinkerFixture
+    {
+        [Fact]
+        public void CanBuildWithOverridableActions()
+        {
+            var folder = TestData.Get(@"TestData\OverridableActions");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "Package.wxs"),
+                    Path.Combine(folder, "PackageComponents.wxs"),
+                    "-loc", Path.Combine(folder, "Package.en-us.wxl"),
+                    "-bindpath", Path.Combine(folder, "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", Path.Combine(baseFolder, @"bin\test.msi")
+                });
+
+                result.AssertSuccess();
+
+                Assert.True(File.Exists(Path.Combine(baseFolder, @"bin\test.msi")));
+                Assert.True(File.Exists(Path.Combine(baseFolder, @"bin\test.wixpdb")));
+                Assert.True(File.Exists(Path.Combine(baseFolder, @"bin\MsiPackage\test.txt")));
+
+                var intermediate = Intermediate.Load(Path.Combine(intermediateFolder, @"test.wir"));
+                var section = intermediate.Sections.Single();
+
+                var actions = section.Tuples.OfType<WixActionTuple>().Where(wat => wat.Action.StartsWith("Set")).ToList();
+                Assert.Equal(2, actions.Count);
+                //Assert.Equal(Path.Combine(folder, @"data\test.txt"), wixFile[WixFileTupleFields.Source].AsPath().Path);
+                //Assert.Equal(@"test.txt", wixFile[WixFileTupleFields.Source].PreviousValue.AsPath().Path);
+            }
+        }
+    }
+}
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/Package.en-us.wxl b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/Package.en-us.wxl
new file mode 100644
index 00000000..38c12ac1
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/Package.en-us.wxl
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<!--
+This file contains the declaration of all the localizable strings.
+-->
+<WixLocalization xmlns="http://wixtoolset.org/schemas/v4/wxl" Culture="en-US">
+
+  <String Id="DowngradeError">A newer version of [ProductName] is already installed.</String>
+  <String Id="FeatureTitle">MsiPackage</String>
+
+</WixLocalization>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/Package.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/Package.wxs
new file mode 100644
index 00000000..d8743747
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/Package.wxs
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Product Id="*" Name="MsiPackage" Language="1033" Version="1.0.0.0" Manufacturer="Example Corporation" UpgradeCode="047730a5-30fe-4a62-a520-da9381b8226a">
+        <Package InstallerVersion="200" Compressed="no" InstallScope="perMachine" />
+
+        <MajorUpgrade DowngradeErrorMessage="!(loc.DowngradeError)" />
+        <MediaTemplate />
+
+        <Feature Id="ProductFeature" Title="!(loc.FeatureTitle)">
+            <ComponentGroupRef Id="ProductComponents" />
+            <ComponentGroupRef Id="Foo1" />
+            <ComponentGroupRef Id="Foo2" />
+        </Feature>
+
+        <!--<CustomActionRef Id="SetFoo" />-->
+
+    </Product>
+
+    <Fragment Id="SetFoo">
+        <CustomAction Id="SetFoo" Property="FOO" Value="BOB" />
+        <CustomAction Id="SetBar" Property="BAR" Value="BOB" />
+    </Fragment>
+
+    <Fragment Id="Foo1">
+        <ComponentGroup Id="Foo1" />
+
+        <InstallExecuteSequence>
+            <Custom Action="SetFoo" Before="SetBar" />
+            <Custom Action="SetBar" Overridable="yes" Before="AppSearch" />
+        </InstallExecuteSequence>
+    </Fragment>
+
+    <Fragment Id="Foo2">
+        <ComponentGroup Id="Foo2" />
+        
+        <InstallExecuteSequence>
+            <Custom Action="SetBar" Before="AppSearch" />
+        </InstallExecuteSequence>
+    </Fragment>
+
+    <Fragment Id="Directories">
+        <Directory Id="TARGETDIR" Name="SourceDir">
+            <Directory Id="ProgramFilesFolder">
+                <Directory Id="INSTALLFOLDER" Name="MsiPackage" />
+            </Directory>
+        </Directory>
+    </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/PackageComponents.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/PackageComponents.wxs
new file mode 100644
index 00000000..e26c4509
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/PackageComponents.wxs
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+  <Fragment>
+    <ComponentGroup Id="ProductComponents" Directory="INSTALLFOLDER">
+       <Component>
+         <File Source="test.txt" />
+       </Component>
+    </ComponentGroup>
+  </Fragment>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/data/test.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/data/test.txt
new file mode 100644
index 00000000..cd0db0e1
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/OverridableActions/data/test.txt
@@ -0,0 +1 @@
+This is test.txt.
\ No newline at end of file
diff --git a/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj b/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
index 7365f140..2b202350 100644
--- a/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
+++ b/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
@@ -77,6 +77,10 @@
     <Content Include="TestData\Variables\Package.en-us.wxl" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\Variables\Package.wxs" CopyToOutputDirectory="PreserveNewest" />
     <Content Include="TestData\Variables\PackageComponents.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\OverridableActions\data\test.txt" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\OverridableActions\Package.en-us.wxl" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\OverridableActions\Package.wxs" CopyToOutputDirectory="PreserveNewest" />
+    <Content Include="TestData\OverridableActions\PackageComponents.wxs" CopyToOutputDirectory="PreserveNewest" />
   </ItemGroup>
 
   <ItemGroup>