Overridable WixVariables should be treated as virtual

Fixes 8528

3 files changed, 51 insertions, 6 deletions

diff --git a/src/wix/WixToolset.Core/Compiler_Package.cs b/src/wix/WixToolset.Core/Compiler_Package.cs
index 853c6ed2..479a1ab3 100644
--- a/src/wix/WixToolset.Core/Compiler_Package.cs
+++ b/src/wix/WixToolset.Core/Compiler_Package.cs
@@ -5033,6 +5033,11 @@ namespace WixToolset.Core
 
             if (!this.Core.EncounteredError)
             {
+                if (overridable)
+                {
+                    id = new Identifier(AccessModifier.Virtual, id.Id);
+                }
+
                 this.Core.AddSymbol(new WixVariableSymbol(sourceLineNumbers, id)
                 {
                     Value = value,
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/WixVariable/PackageWithOverriddenBindVariable.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/WixVariable/PackageWithOverriddenBindVariable.wxs
new file mode 100644
index 00000000..fdc2a461
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/WixVariable/PackageWithOverriddenBindVariable.wxs
@@ -0,0 +1,13 @@
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+  <Package Version="1.0.0" Name="MsiPackage" Manufacturer="Example Corporation" UpgradeCode="047730a5-30fe-4a62-a520-da9381b8226a">
+    <PropertyRef Id="Test" />
+
+    <WixVariable Id="override TestWixVariable" Value="0"/>
+  </Package>
+
+  <Fragment>
+    <Property Id="Test" Value="!(wix.TestWixVariable)" />
+
+    <WixVariable Id="TestWixVariable" Value="1" Overridable="true"/>
+  </Fragment>
+</Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/WixVariableFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/WixVariableFixture.cs
index 50199b74..b6aea217 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/WixVariableFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/WixVariableFixture.cs
@@ -35,7 +35,7 @@ namespace WixToolsetTest.CoreIntegration
 
                 result.AssertSuccess();
 
-                var productVersion = GetProductVersionFromMsi(msiPath);
+                var productVersion = GetPropertyFromMsi(msiPath, "ProductVersion");
                 Assert.Equal("4.3.2.1", productVersion);
             }
         }
@@ -62,7 +62,7 @@ namespace WixToolsetTest.CoreIntegration
 
                 result.AssertSuccess();
 
-                var productVersion = GetProductVersionFromMsi(msiPath);
+                var productVersion = GetPropertyFromMsi(msiPath, "ProductVersion");
                 Assert.Equal("1.1.1.1", productVersion);
 
                 var directoryTable = Query.QueryDatabase(msiPath, new[] { "Directory" }).OrderBy(s => s).ToArray();
@@ -98,7 +98,7 @@ namespace WixToolsetTest.CoreIntegration
 
                 result.AssertSuccess();
 
-                var productVersion = GetProductVersionFromMsi(msiPath);
+                var productVersion = GetPropertyFromMsi(msiPath, "ProductVersion");
                 Assert.Equal("9.8.7.6", productVersion);
 
                 var directoryTable = Query.QueryDatabase(msiPath, new[] { "Directory" }).OrderBy(s => s).ToArray();
@@ -112,6 +112,33 @@ namespace WixToolsetTest.CoreIntegration
         }
 
         [Fact]
+        public void CanBuildMsiWithOverridableBindVariable()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var msiPath = Path.Combine(baseFolder, "bin", "test1.msi");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "WixVariable", "PackageWithOverriddenBindVariable.wxs"),
+                    "-bindpath", Path.Combine(folder, "SingleFile", "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", msiPath
+                });
+
+                result.AssertSuccess();
+
+                var testValue = GetPropertyFromMsi(msiPath, "Test");
+                Assert.Equal("0", testValue);
+            }
+        }
+
+        [Fact]
         public void CanBuildBundleWithBindVariable()
         {
             var folder = TestData.Get(@"TestData");
@@ -150,7 +177,7 @@ namespace WixToolsetTest.CoreIntegration
 
                 result3.AssertSuccess();
 
-                var productVersion = GetProductVersionFromMsi(msiPath);
+                var productVersion = GetPropertyFromMsi(msiPath, "ProductVersion");
                 WixAssert.StringEqual("255.255.65535", productVersion);
 
                 var extractResult = BundleExtractor.ExtractAllContainers(null, bundlePath, Path.Combine(baseFolder, "ba"), Path.Combine(baseFolder, "attached"), Path.Combine(baseFolder, "extract"));
@@ -163,10 +190,10 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
 
-        private static string GetProductVersionFromMsi(string msiPath)
+        private static string GetPropertyFromMsi(string msiPath, string propertyId)
         {
             var propertyTable = Query.QueryDatabase(msiPath, new[] { "Property" }).Select(r => r.Split('\t')).ToDictionary(r => r[0].Substring("Property:".Length), r => r[1]);
-            Assert.True(propertyTable.TryGetValue("ProductVersion", out var productVersion));
+            Assert.True(propertyTable.TryGetValue(propertyId, out var productVersion), $"Failed to find requested property: '{propertyId}'");
 
             return productVersion;
         }