Remove MsuPackage/@KB and Permanent attributes

Windows no longer allows for the silent removal of MSUs which
renders the KB and Permanent attributes irrelevant. This removes
both.

Fixes 6749
Fixes 6801

14 files changed, 130 insertions, 39 deletions

diff --git a/src/api/wix/WixToolset.Data/Symbols/WixBundleMsuPackageSymbol.cs b/src/api/wix/WixToolset.Data/Symbols/WixBundleMsuPackageSymbol.cs
index e52a9b2d..04ebe748 100644
--- a/src/api/wix/WixToolset.Data/Symbols/WixBundleMsuPackageSymbol.cs
+++ b/src/api/wix/WixToolset.Data/Symbols/WixBundleMsuPackageSymbol.cs
@@ -10,8 +10,7 @@ namespace WixToolset.Data
             SymbolDefinitionType.WixBundleMsuPackage,
             new[]
             {
-                new IntermediateFieldDefinition(nameof(WixBundleMsuPackageSymbolFields.DetectCondition), IntermediateFieldType.String),
-                new IntermediateFieldDefinition(nameof(WixBundleMsuPackageSymbolFields.MsuKB), IntermediateFieldType.String),
+                new IntermediateFieldDefinition(nameof(WixBundleMsuPackageSymbolFields.DetectCondition), IntermediateFieldType.String)
             },
             typeof(WixBundleMsuPackageSymbol));
     }
@@ -22,7 +21,6 @@ namespace WixToolset.Data.Symbols
     public enum WixBundleMsuPackageSymbolFields
     {
         DetectCondition,
-        MsuKB,
     }
 
     public class WixBundleMsuPackageSymbol : IntermediateSymbol
@@ -42,11 +40,5 @@ namespace WixToolset.Data.Symbols
             get => (string)this.Fields[(int)WixBundleMsuPackageSymbolFields.DetectCondition];
             set => this.Set((int)WixBundleMsuPackageSymbolFields.DetectCondition, value);
         }
-
-        public string MsuKB
-        {
-            get => (string)this.Fields[(int)WixBundleMsuPackageSymbolFields.MsuKB];
-            set => this.Set((int)WixBundleMsuPackageSymbolFields.MsuKB, value);
-        }
     }
 }
\ No newline at end of file
diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index e286d702..57069902 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -1216,6 +1216,22 @@ namespace WixToolset.Converters
             this.ConvertSuppressSignatureValidation(element);
 
             this.UpdatePackageCacheAttribute(element);
+
+            var kbAttribute = element.Attribute("KB");
+
+            if (null != kbAttribute
+                && this.OnInformation(ConverterTestType.MsuPackageKBObsolete, element, "The MsuPackage element contains obsolete '{0}' attribute. Windows no longer supports silently removing MSUs so the attribute is unnecessary. The attribute will be removed.", kbAttribute.Name))
+            {
+                kbAttribute.Remove();
+            }
+
+            var permanentAttribute = element.Attribute("Permanent");
+
+            if (null != permanentAttribute
+                && this.OnInformation(ConverterTestType.MsuPackagePermanentObsolete, element, "The MsuPackage element contains obsolete '{0}' attribute. MSU packages are now always permanent because Windows no longer supports silently removing MSUs. The attribute will be removed.", permanentAttribute.Name))
+            {
+                permanentAttribute.Remove();
+            }
         }
 
         private void ConvertProductElement(XElement element)
@@ -2664,6 +2680,16 @@ namespace WixToolset.Converters
             /// Cache attribute value updated.
             /// </summary>
             BundlePackageCacheAttributeValueObsolete,
+
+            /// <summary>
+            /// The MsuPackage element contains obsolete '{0}' attribute. Windows no longer supports silently removing MSUs so the attribute is unnecessary. The attribute will be removed.
+            /// </summary>
+            MsuPackageKBObsolete,
+
+            /// <summary>
+            /// The MsuPackage element contains obsolete '{0}' attribute. MSU packages are now always permanent because Windows no longer supports silently removing MSUs. The attribute will be removed.
+            /// </summary>
+            MsuPackagePermanentObsolete,
         }
     }
 }
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
index 25a155d6..dda79aa1 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
@@ -499,7 +499,6 @@ namespace WixToolset.Core.Burn.Bundles
                     else if (package.SpecificPackageSymbol is WixBundleMsuPackageSymbol msuPackage) // MSU
                     {
                         writer.WriteAttributeString("DetectCondition", msuPackage.DetectCondition);
-                        writer.WriteAttributeString("KB", msuPackage.MsuKB);
                     }
 
                     var packageMsiProperties = msiPropertiesByPackage[package.PackageId];
diff --git a/src/wix/WixToolset.Core/Compiler_Bundle.cs b/src/wix/WixToolset.Core/Compiler_Bundle.cs
index be976a22..f3fc0271 100644
--- a/src/wix/WixToolset.Core/Compiler_Bundle.cs
+++ b/src/wix/WixToolset.Core/Compiler_Bundle.cs
@@ -2019,7 +2019,6 @@ namespace WixToolset.Core
             string detectCondition = null;
             string protocol = null;
             long? installSize = null;
-            string msuKB = null;
             var enableFeatureSelection = YesNoType.NotSet;
             var forcePerMachine = YesNoType.NotSet;
             CompilerPackagePayload childCompilerPackagePayload = null;
@@ -2114,6 +2113,7 @@ namespace WixToolset.Core
                             break;
                         case "Permanent":
                             permanent = this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib);
+                            allowed = (packageType != WixBundlePackageType.Msu);
                             break;
                         case "Visible":
                             visible = this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib);
@@ -2153,10 +2153,6 @@ namespace WixToolset.Core
                         case "InstallSize":
                             installSize = this.Core.GetAttributeLongValue(sourceLineNumbers, attrib, 0, Int64.MaxValue);
                             break;
-                        case "KB":
-                            msuKB = this.Core.GetAttributeValue(sourceLineNumbers, attrib);
-                            allowed = (packageType == WixBundlePackageType.Msu);
-                            break;
                         case "Compressed":
                             compilerPayload.ParseCompressed(attrib);
                             hasPayloadInfo = true;
@@ -2464,21 +2460,6 @@ namespace WixToolset.Core
             }
             else if (packageType == WixBundlePackageType.Msu)
             {
-                if (permanent == YesNoType.No)
-                {
-                    if (String.IsNullOrEmpty(msuKB))
-                    {
-                        this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, node.Name.LocalName, "KB", "Permanent", "no"));
-                    }
-                }
-                else if (permanent == YesNoType.NotSet)
-                {
-                    if (String.IsNullOrEmpty(msuKB))
-                    {
-                        this.Core.Write(ErrorMessages.ExpectedAttributeWithoutOtherAttribute(sourceLineNumbers, node.Name.LocalName, "KB", "Permanent"));
-                    }
-                }
-
                 // Detect condition is recommended for Msu packages.
                 if (String.IsNullOrEmpty(detectCondition))
                 {
@@ -2583,8 +2564,7 @@ namespace WixToolset.Core
                     case WixBundlePackageType.Msu:
                         this.Core.AddSymbol(new WixBundleMsuPackageSymbol(sourceLineNumbers, id)
                         {
-                            DetectCondition = detectCondition,
-                            MsuKB = msuKB
+                            DetectCondition = detectCondition
                         });
                         break;
                 }
diff --git a/src/wix/test/WixToolsetTest.Converters/MsuPackageFixture.cs b/src/wix/test/WixToolsetTest.Converters/MsuPackageFixture.cs
new file mode 100644
index 00000000..5ec109b2
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.Converters/MsuPackageFixture.cs
@@ -0,0 +1,60 @@
+// 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.Converters
+{
+    using System;
+    using System.Linq;
+    using System.Xml.Linq;
+    using WixBuildTools.TestSupport;
+    using WixToolset.Converters;
+    using WixToolsetTest.Converters.Mocks;
+    using Xunit;
+
+    public class MsuPackageFixture : BaseConverterFixture
+    {
+        [Fact]
+        public void CanRemoveMsuPackageDeprecatedAttributes()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>",
+                "  <Fragment>",
+                "    <PackageGroup Id='msu'>",
+                "      <MsuPackage Id='PermanentMsuPackage' KB='1234' Permanent='yes' DetectCondition='none'>",
+                "        <MsuPackagePayload DownloadUrl='example.com' SourceFile='ignored.msu' />",
+                "      </MsuPackage>",
+                "    </PackageGroup>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
+                "  <Fragment>",
+                "    <PackageGroup Id=\"msu\">",
+                "      <MsuPackage Id=\"PermanentMsuPackage\" DetectCondition=\"none\">",
+                "        <MsuPackagePayload DownloadUrl=\"example.com\" SourceFile=\"ignored.msu\" />",
+                "      </MsuPackage>",
+                "    </PackageGroup>",
+                "  </Fragment>",
+                "</Wix>"
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+
+            var errors = converter.ConvertDocument(document);
+
+            var actualLines = UnformattedDocumentLines(document);
+            WixAssert.CompareLineByLine(new[]
+            {
+                "[Converted] The MsuPackage element contains obsolete 'KB' attribute. Windows no longer supports silently removing MSUs so the attribute is unnecessary. The attribute will be removed. (MsuPackageKBObsolete)",
+                "[Converted] The MsuPackage element contains obsolete 'Permanent' attribute. MSU packages are now always permanent because Windows no longer supports silently removing MSUs. The attribute will be removed. (MsuPackagePermanentObsolete)",
+            }, messaging.Messages.Select(m => m.ToString()).ToArray());
+            WixAssert.CompareLineByLine(expected, actualLines);
+
+            Assert.Equal(2, errors);
+        }
+    }
+}
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/MsuPackageFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/MsuPackageFixture.cs
index 37499ea9..0ebaf868 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/MsuPackageFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/MsuPackageFixture.cs
@@ -88,5 +88,29 @@ namespace WixToolsetTest.CoreIntegration
                 Assert.Equal("The MsuPackage/@CacheId attribute was not found; it is required when attribute CertificatePublicKey is specified.", message.ToString());
             }
         }
+
+        [Fact]
+        public void ErrorWhenSpecifyingPermanent()
+        {
+            var folder = TestData.Get(@"TestData", "MsuPackage");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+
+                var result = WixRunner.Execute(false, new[]
+                {
+                    "build",
+                    Path.Combine(folder, "PermanentMsuPackage.wxs"),
+                    "-o", Path.Combine(baseFolder, "test.wixlib")
+                });
+
+                WixAssert.CompareLineByLine(new[]
+                {
+                    "The MsuPackage element contains an unexpected attribute 'Permanent'.",
+                }, result.Messages.Select(m => m.ToString()).ToArray());
+                Assert.Equal(4, result.ExitCode);
+            }
+        }
     }
 }
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/RemotePayloadFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/RemotePayloadFixture.cs
index fc105880..c4f5c5c7 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/RemotePayloadFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/RemotePayloadFixture.cs
@@ -289,7 +289,7 @@ namespace WixToolsetTest.CoreIntegration
                 }, elements);
 
                 // Append required attributes to build.
-                elements[0] = elements[0].Replace(">", " KB='KB2937592' DetectCondition='test'>");
+                elements[0] = elements[0].Replace(">", " DetectCondition='test'>");
 
                 var remotePayloadSourceText = "<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>" +
                     "  <Fragment>" +
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/Bundle.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/Bundle.wxs
index 48222546..dbca3393 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/Bundle.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/Bundle.wxs
@@ -5,7 +5,7 @@
         </BootstrapperApplication>
         
         <Chain>
-            <MsuPackage DetectCondition="DetectedTheMsu" KB="xyz" SourceFile="test.msu" />
+            <MsuPackage DetectCondition="DetectedTheMsu" SourceFile="test.msu" />
         </Chain>
     </Bundle>
 </Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerification.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerification.wxs
index dcae2cf8..e10ac40d 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerification.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerification.wxs
@@ -5,7 +5,7 @@
         </BootstrapperApplication>
         
         <Chain>
-            <MsuPackage DetectCondition="DetectedTheMsu" KB="xyz" CacheId="8cf75b99-13c0-4184-82ce-dbde45dcd55a">
+            <MsuPackage DetectCondition="DetectedTheMsu" CacheId="8cf75b99-13c0-4184-82ce-dbde45dcd55a">
               <MsuPackagePayload Name='Windows8.1-KB2937592-x86.msu' DownloadUrl='http://wixtoolset.org' CertificatePublicKey="abc" CertificateThumbprint="123" Size='309544' />
             </MsuPackage>
         </Chain>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerificationWithoutCacheId.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerificationWithoutCacheId.wxs
index f9282e37..759bcf9b 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerificationWithoutCacheId.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/BundleUsingCertificateVerificationWithoutCacheId.wxs
@@ -5,7 +5,7 @@
         </BootstrapperApplication>
         
         <Chain>
-            <MsuPackage DetectCondition="DetectedTheMsu" KB="xyz" >
+            <MsuPackage DetectCondition="DetectedTheMsu">
               <MsuPackagePayload Name='Windows8.1-KB2937592-x86.msu' DownloadUrl='http://wixtoolset.org' CertificatePublicKey="abc" CertificateThumbprint="123" Size='309544' />
             </MsuPackage>
         </Chain>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/PermanentMsuPackage.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/PermanentMsuPackage.wxs
new file mode 100644
index 00000000..d1104e89
--- /dev/null
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/MsuPackage/PermanentMsuPackage.wxs
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="BundlePackages">
+            <MsuPackage Id="PermanentMsuPackage" Permanent="yes" DetectCondition="none">
+                <MsuPackagePayload DownloadUrl="example.com" SourceFile="ignored.msu" />
+            </MsuPackage>
+        </PackageGroup>
+    </Fragment>
+</Wix>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/MissingSourceFileAndHash.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/MissingSourceFileAndHash.wxs
index 5e1b99ff..89aa3454 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/MissingSourceFileAndHash.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/MissingSourceFileAndHash.wxs
@@ -2,7 +2,7 @@
 <Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
     <Fragment>
         <PackageGroup Id="BundlePackages">
-            <MsuPackage Id="MissingSourceFileAndHash" Permanent="yes" DetectCondition="none">
+            <MsuPackage Id="MissingSourceFileAndHash" DetectCondition="none">
                 <MsuPackagePayload DownloadUrl="example.com" />
             </MsuPackage>
         </PackageGroup>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/SpecifiedHashAndMissingDownloadUrl.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/SpecifiedHashAndMissingDownloadUrl.wxs
index 8e62f660..49384c16 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/SpecifiedHashAndMissingDownloadUrl.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PackagePayload/SpecifiedHashAndMissingDownloadUrl.wxs
@@ -2,7 +2,7 @@
 <Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
     <Fragment>
         <PackageGroup Id="BundlePackages">
-            <MsuPackage Id="SpecifiedHashAndMissingDownloadUrl" Permanent="yes" DetectCondition="none">
+            <MsuPackage Id="SpecifiedHashAndMissingDownloadUrl" DetectCondition="none">
                 <MsuPackagePayload Name="example.msu" Hash="abcd" Size="1" Version="1.0.0.0" ProductName="KB1234567" Description="fake msu" />
             </MsuPackage>
         </PackageGroup>
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RemotePayload/DiversePayloadsBundle.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RemotePayload/DiversePayloadsBundle.wxs
index f65b5db6..38457862 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RemotePayload/DiversePayloadsBundle.wxs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/RemotePayload/DiversePayloadsBundle.wxs
@@ -6,7 +6,7 @@
         <Chain>
             <PackageGroupRef Id="ExternalContainerPackages" />
             <ExePackage SourceFile="signed_bundle_engine.exe" DetectCondition="none" Permanent="yes" />
-            <MsuPackage SourceFile="Windows8.1-KB2937592-x86.msu" DetectCondition="none" Permanent="yes" Compressed="no" DownloadUrl="http://example.com/test.msu" />
+            <MsuPackage SourceFile="Windows8.1-KB2937592-x86.msu" DetectCondition="none" Compressed="no" DownloadUrl="http://example.com/test.msu" />
             <MsiPackage SourceFile="test.msi" Compressed="no" />
         </Chain>
         <Container Name="External.cab">