Trim patch baseline identifiers when they get too long

The PatchBaseline/@Id value is used as part of the name for the
patch transforms that get stored as substorages. Substorage name
length are very limited. Rather than error, we'll now trim the value
and let the user know via a warning.

Fixes 4434

9 files changed, 132 insertions, 44 deletions

diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs
index 76bcd532..66078d8d 100644
--- a/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs
+++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs
@@ -174,14 +174,26 @@ namespace WixToolset.Core.WindowsInstaller.Bind
                 productCodes.Add(productCode);
                 validTransform.Add(Tuple.Create(productCode, mainTransform.Transform));
 
-                // attach these transforms to the patch object
-                // TODO: is this an acceptable way to auto-generate transform stream names?
-                var transformName = mainTransform.Baseline + "." + validTransform.Count.ToString(CultureInfo.InvariantCulture);
-                subStorages.Add(new SubStorage(transformName, mainTransform.Transform));
-                subStorages.Add(new SubStorage("#" + transformName, pairedTransform));
+                // Attach the main and paired transforms to the patch object.
+                var baseTransformName = mainTransform.Baseline;
+                var countSuffix = "." + validTransform.Count.ToString(CultureInfo.InvariantCulture);
+
+                if (PatchConstants.PairedPatchTransformPrefix.Length + baseTransformName.Length + countSuffix.Length > PatchConstants.MaxPatchTransformName)
+                {
+                    var trimmedTransformName = baseTransformName.Substring(0, PatchConstants.MaxPatchTransformName - PatchConstants.PairedPatchTransformPrefix.Length - countSuffix.Length);
+
+                    this.Messaging.Write(WindowsInstallerBackendWarnings.LongPatchBaselineIdTrimmed(baselineSymbol.SourceLineNumbers, baseTransformName, trimmedTransformName));
 
+                    baseTransformName = trimmedTransformName;
+                }
+
+                var transformName = baseTransformName + countSuffix;
+                subStorages.Add(new SubStorage(transformName, mainTransform.Transform));
                 transformNames.Add(":" + transformName);
-                transformNames.Add(":#" + transformName);
+
+                var pairedTransformName = PatchConstants.PairedPatchTransformPrefix + transformName;
+                subStorages.Add(new SubStorage(pairedTransformName, pairedTransform));
+                transformNames.Add(":" + pairedTransformName);
             }
 
             if (validTransform.Count == 0)
diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs
index 2ac563ac..441038ec 100644
--- a/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs
+++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs
@@ -40,10 +40,10 @@ namespace WixToolset.Core.WindowsInstaller.Bind
             //var patchActualFileTable = this.Output.EnsureTable(this.TableDefinitions["File"]);
 
             // Index paired transforms by name without their "#" prefix.
-            var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith("#")).ToDictionary(s => s.Name, s => s.Data);
+            var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)).ToDictionary(s => s.Name, s => s.Data);
 
             // Enumerate through main transforms.
-            foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith("#")))
+            foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)))
             {
                 var mainTransform = substorage.Data;
                 var mainFileTable = mainTransform.Tables["File"];
@@ -54,7 +54,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
                 }
 
                 // Index File table of pairedTransform
-                var pairedTransform = pairedTransforms["#" + substorage.Name];
+                var pairedTransform = pairedTransforms[PatchConstants.PairedPatchTransformPrefix + substorage.Name];
                 var pairedFileRows = new RowDictionary<FileRow>(pairedTransform.Tables["File"]);
 
                 foreach (FileRow mainFileRow in mainFileTable.Rows.Where(f => f.Operation != RowOperation.Delete))
diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs
new file mode 100644
index 00000000..d37c7764
--- /dev/null
+++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs
@@ -0,0 +1,11 @@
+// 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.WindowsInstaller.Bind
+{
+    internal static class PatchConstants
+    {
+        public const int MaxPatchTransformName = 31;
+
+        public static readonly string PairedPatchTransformPrefix = "#";
+    }
+}
diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs
index 981fa0a4..ed865450 100644
--- a/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs
+++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs
@@ -52,16 +52,16 @@ namespace WixToolset.Core.WindowsInstaller.Bind
             var patchMediaRows = new RowDictionary<MediaRow>(this.Output.Tables["Media"]);
 
             // Index paired transforms by name without the "#" prefix.
-            var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith("#")).ToDictionary(s => s.Name, s => s.Data);
+            var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)).ToDictionary(s => s.Name, s => s.Data);
 
             // Copy File bind data into substorages
-            foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith("#")))
+            foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)))
             {
                 var mainTransform = substorage.Data;
 
                 var mainMsiFileHashIndex = new RowDictionary<Row>(mainTransform.Tables["MsiFileHash"]);
 
-                var pairedTransform = pairedTransforms["#" + substorage.Name];
+                var pairedTransform = pairedTransforms[PatchConstants.PairedPatchTransformPrefix + substorage.Name];
 
                 // Copy Media.LastSequence.
                 var pairedMediaTable = pairedTransform.Tables["Media"];
diff --git a/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs b/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs
index d0986a4d..bc61c59c 100644
--- a/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs
+++ b/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs
@@ -6,10 +6,10 @@ namespace WixToolset.Core.WindowsInstaller
 
     internal static class WindowsInstallerBackendWarnings
     {
-        //public static Message ReplaceThisWithTheFirstWarning(SourceLineNumber sourceLineNumbers)
-        //{
-        //    return Message(sourceLineNumbers, Ids.ReplaceThisWithTheFirstWarning, "format string", arg1, arg2);
-        //}
+        internal static Message LongPatchBaselineIdTrimmed(SourceLineNumber sourceLineNumbers, string baseTransformName, string trimmedTransformName)
+        {
+            return Message(sourceLineNumbers, Ids.LongPatchBaselineIdTrimmed, "The PatchBaseline/@Id='{0}' is too long. It is recommended to use short identifiers like 'RTM' and 'SP1'. The identifier has been trimmed to '{1}' so the patch can be created.", baseTransformName, trimmedTransformName);
+        }
 
         private static Message Message(SourceLineNumber sourceLineNumber, Ids id, string format, params object[] args)
         {
@@ -18,7 +18,7 @@ namespace WixToolset.Core.WindowsInstaller
 
         public enum Ids
         {
-            // ReplaceThisWithTheFirstWarning = 7100,
+            LongPatchBaselineIdTrimmed = 7100,
         } // last available is 7499. 7500 is WindowsInstallerBackendErrors.
     }
 }
diff --git a/src/wix/WixToolset.Core/Compiler.cs b/src/wix/WixToolset.Core/Compiler.cs
index 7361f501..d04ae491 100644
--- a/src/wix/WixToolset.Core/Compiler.cs
+++ b/src/wix/WixToolset.Core/Compiler.cs
@@ -7745,10 +7745,6 @@ namespace WixToolset.Core
                 this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, node.Name.LocalName, "Id"));
                 id = Identifier.Invalid;
             }
-            else if (27 < id.Id.Length)
-            {
-                this.Core.Write(ErrorMessages.IdentifierTooLongError(sourceLineNumbers, node.Name.LocalName, "Id", id.Id, 27));
-            }
 
             if (!String.IsNullOrEmpty(baselineFile) || !String.IsNullOrEmpty(updateFile))
             {
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs
index 53c9ae4d..a26472fa 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs
@@ -25,7 +25,7 @@ namespace WixToolsetTest.CoreIntegration
         [Fact]
         public void CanBuildSimplePatch()
         {
-            var folder = TestData.Get(@"TestData\PatchSingle");
+            var folder = TestData.Get(@"TestData", "PatchSingle");
 
             using (var fs = new DisposableFileSystem())
             {
@@ -57,7 +57,7 @@ namespace WixToolsetTest.CoreIntegration
         [Fact]
         public void CanBuildSimplePatchWithNoFileChanges()
         {
-            var folder = TestData.Get(@"TestData\PatchNoFileChanges");
+            var folder = TestData.Get(@"TestData", "PatchNoFileChanges");
 
             using (var fs = new DisposableFileSystem())
             {
@@ -86,6 +86,31 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
 
+        [Fact]
+        public void CanBuildSimplePatchWithBaselineIdTooLong()
+        {
+            var folder = TestData.Get(@"TestData", "PatchBaselineIdTooLong");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var tempFolderBaseline = Path.Combine(baseFolder, "baseline");
+                var tempFolderUpdate = Path.Combine(baseFolder, "update");
+                var tempFolderPatch = Path.Combine(baseFolder, "patch");
+
+                var baselinePdb = BuildMsi("Baseline.msi", folder, tempFolderBaseline, "1.0.0", "1.0.0", "1.0.0");
+                var update1Pdb = BuildMsi("Update.msi", folder, tempFolderUpdate, "1.0.1", "1.0.1", "1.0.1");
+                var patchPdb = BuildMsp("Patch1.msp", folder, tempFolderPatch, "1.0.1", bindpaths: new[] { Path.GetDirectoryName(baselinePdb), Path.GetDirectoryName(update1Pdb) }, hasNoFiles: true, warningsAsErrors: false);
+                var patchPath = Path.ChangeExtension(patchPdb, ".msp");
+
+                var doc = GetExtractPatchXml(patchPath);
+                WixAssert.StringEqual("{11111111-2222-3333-4444-555555555555}", doc.Root.Element(PatchNamespace + "TargetProductCode").Value);
+
+                var names = Query.GetSubStorageNames(patchPath);
+                WixAssert.CompareLineByLine(new[] { "#ThisBaseLineIdIsTooLongAndGe.1", "ThisBaseLineIdIsTooLongAndGe.1" }, names);
+            }
+        }
+
         [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6387")]
         public void CanBuildPatchFromProductWithFilesFromWixlib()
         {
@@ -110,7 +135,7 @@ namespace WixToolsetTest.CoreIntegration
         [Fact]
         public void CanBuildBundleWithNonSpecificPatches()
         {
-            var folder = TestData.Get(@"TestData\PatchNonSpecific");
+            var folder = TestData.Get(@"TestData", "PatchNonSpecific");
 
             using (var fs = new DisposableFileSystem())
             {
@@ -125,23 +150,23 @@ namespace WixToolsetTest.CoreIntegration
                 var bundleBPdb = BuildBundle("BundleB.exe", Path.Combine(folder, "BundleB"), tempFolder);
                 var bundleCPdb = BuildBundle("BundleC.exe", Path.Combine(folder, "BundleC"), tempFolder);
 
-                VerifyPatchTargetCodes(bundleAPdb, new[]
+                VerifyPatchTargetCodesInBurnManifest(bundleAPdb, new[]
                 {
                     "<PatchTargetCode TargetCode='{26309973-0A5E-4979-B142-98A6E064EDC0}' Product='yes' />",
                 });
-                VerifyPatchTargetCodes(bundleBPdb, new[]
+                VerifyPatchTargetCodesInBurnManifest(bundleBPdb, new[]
                 {
                     "<PatchTargetCode TargetCode='{26309973-0A5E-4979-B142-98A6E064EDC0}' Product='yes' />",
                     "<PatchTargetCode TargetCode='{32B0396A-CE36-4570-B16E-F88FA42DC409}' Product='no' />",
                 });
-                VerifyPatchTargetCodes(bundleCPdb, new string[0]);
+                VerifyPatchTargetCodesInBurnManifest(bundleCPdb, new string[0]);
             }
         }
 
         [Fact]
         public void CanBuildBundleWithSlipstreamPatch()
         {
-            var folder = TestData.Get(@"TestData\PatchSingle");
+            var folder = TestData.Get(@"TestData", "PatchSingle");
 
             using (var fs = new DisposableFileSystem())
             {
@@ -167,20 +192,6 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
 
-        private static void VerifyPatchTargetCodes(string pdbPath, string[] expected)
-        {
-            using (var wixOutput = WixOutput.Read(pdbPath))
-            {
-                var manifestData = wixOutput.GetData(BurnConstants.BurnManifestWixOutputStreamName);
-                var doc = new XmlDocument();
-                doc.LoadXml(manifestData);
-                var nsmgr = BundleExtractor.GetBurnNamespaceManager(doc, "w");
-                var patchTargetCodes = doc.SelectNodes("/w:BurnManifest/w:PatchTargetCode", nsmgr).GetTestXmlLines();
-
-                WixAssert.CompareLineByLine(expected, patchTargetCodes);
-            }
-        }
-
         private static string BuildMsi(string outputName, string sourceFolder, string baseFolder, string defineV, string defineA, string defineB)
         {
             var extensionPath = Path.GetFullPath(new Uri(typeof(ExampleExtensionFactory).Assembly.CodeBase).LocalPath);
@@ -204,7 +215,7 @@ namespace WixToolsetTest.CoreIntegration
             return Path.ChangeExtension(outputPath, ".wixpdb");
         }
 
-        private static string BuildMsp(string outputName, string sourceFolder, string baseFolder, string defineV, IEnumerable<string> bindpaths = null, bool hasNoFiles = false)
+        private static string BuildMsp(string outputName, string sourceFolder, string baseFolder, string defineV, IEnumerable<string> bindpaths = null, bool hasNoFiles = false, bool warningsAsErrors = true)
         {
             var outputPath = Path.Combine(baseFolder, Path.Combine("bin", outputName));
 
@@ -225,7 +236,7 @@ namespace WixToolsetTest.CoreIntegration
                 args.Add(additionaBindPath);
             }
 
-            var result = WixRunner.Execute(args.ToArray());
+            var result = WixRunner.Execute(warningsAsErrors, args.ToArray());
 
             result.AssertSuccess();
 
@@ -266,6 +277,20 @@ namespace WixToolsetTest.CoreIntegration
             return XDocument.Parse(buffer.ToString());
         }
 
+        private static void VerifyPatchTargetCodesInBurnManifest(string pdbPath, string[] expected)
+        {
+            using (var wixOutput = WixOutput.Read(pdbPath))
+            {
+                var manifestData = wixOutput.GetData(BurnConstants.BurnManifestWixOutputStreamName);
+                var doc = new XmlDocument();
+                doc.LoadXml(manifestData);
+                var nsmgr = BundleExtractor.GetBurnNamespaceManager(doc, "w");
+                var patchTargetCodes = doc.SelectNodes("/w:BurnManifest/w:PatchTargetCode", nsmgr).GetTestXmlLines();
+
+                WixAssert.CompareLineByLine(expected, patchTargetCodes);
+            }
+        }
+
         [DllImport("msi.dll", EntryPoint = "MsiExtractPatchXMLDataW", CharSet = CharSet.Unicode, ExactSpelling = true)]
         private static extern int MsiExtractPatchXMLData(string szPatchPath, int dwReserved, StringBuilder szXMLData, ref int pcchXMLData);
 
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs
new file mode 100644
index 00000000..1c8a34c5
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs
@@ -0,0 +1,28 @@
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+  <Package Name="~Test Package" Version="$(var.V)" Manufacturer="Example Corporation" Language="1033" UpgradeCode="{6B8097B9-A5D0-4BDE-B21E-AF6622DDCA01}" Scope="perMachine" ProductCode="{11111111-2222-3333-4444-555555555555}">
+    <MajorUpgrade DowngradeErrorMessage="Newer version already installed." />
+    <MediaTemplate EmbedCab="yes" />
+
+    <CustomAction Id="CAFromExtension" DllEntry="DoesntExist" BinaryRef="BinFromWir" />
+
+    <StandardDirectory Id="ProgramFilesFolder">
+      <Directory Id="INSTALLFOLDER" Name="~Test App" />
+    </StandardDirectory>
+
+    <Feature Id="Main">
+      <ComponentGroupRef Id="Components" />
+    </Feature>
+  </Package>
+
+  <Fragment>
+    <ComponentGroup Id="Components" Directory="INSTALLFOLDER">
+      <Component>
+        <File Source="$(sys.SOURCEFILEPATH)" />
+      </Component>
+
+      <Component>
+        <RegistryValue Root="HKLM" Key="SOFTWARE\!(bind.property.ProductName)\Patch" Name="Version" Value="$(var.V)" />
+      </Component>
+    </ComponentGroup>
+  </Fragment>
+</Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs
new file mode 100644
index 00000000..fdad0b87
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs
@@ -0,0 +1,16 @@
+<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>
+    <Patch
+    AllowRemoval="yes"
+    DisplayName="~Test Patch v$(var.V)"
+    Description="~Test Small Update Patch v$(var.V)"
+    MoreInfoURL="http://www.example.com/"
+    Manufacturer="Example Corporation"
+    Classification="Update">
+
+    <Media Id="1" Cabinet="foo.cab">
+      <PatchBaseline Id="ThisBaseLineIdIsTooLongAndGetsTrimmed" BaselineFile="Baseline.wixpdb" UpdateFile="Update.wixpdb" />
+    </Media>
+
+    <PatchFamily Id='SequenceFamily' Version='$(var.V)' />
+  </Patch>
+</Wix>