From b4210b23ea28e0acc5a8b7ca5357d7d3926071b4 Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Sun, 25 Apr 2021 15:36:31 -0500 Subject: [PATCH] Allow DownloadUrl on embedded payloads. #5253 --- .../Bind/BindBundleCommand.cs | 2 +- .../Bind/ResolveDownloadUrlsCommand.cs | 17 +++--- .../Bundles/BurnCommon.cs | 3 +- .../Bundles/CreateBurnManifestCommand.cs | 61 ++++++++++--------- .../Bundles/CreateContainerCommand.cs | 7 --- .../Compile/CompilerPayload.cs | 11 +--- .../ContainerFixture.cs | 50 ++++++++++++++- .../HarvestIntoAttachedContainer.wxs | 17 ++++++ 8 files changed, 110 insertions(+), 58 deletions(-) create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs diff --git a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs index deab4d7..4a4f06f 100644 --- a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs +++ b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs @@ -300,7 +300,7 @@ namespace WixToolset.Core.Burn if (PackagingType.Embedded == payload.Packaging && String.IsNullOrEmpty(payload.EmbeddedId)) { - payload.EmbeddedId = String.Format(CultureInfo.InvariantCulture, BurnCommon.BurnAttachedContainerEmbeddedIdFormat, payloadIndex); + payload.EmbeddedId = String.Format(CultureInfo.InvariantCulture, BurnCommon.BurnAuthoredContainerEmbeddedIdFormat, payloadIndex); ++payloadIndex; } } diff --git a/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs b/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs index e41c105..c678b11 100644 --- a/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs +++ b/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs @@ -5,6 +5,7 @@ namespace WixToolset.Core.Burn.Bind using System; using System.Collections.Generic; using WixToolset.Data; + using WixToolset.Data.Burn; using WixToolset.Data.Symbols; using WixToolset.Extensibility; using WixToolset.Extensibility.Services; @@ -60,7 +61,14 @@ namespace WixToolset.Core.Burn.Bind { foreach (var payload in this.PayloadsById.Values) { - if (payload.Packaging == PackagingType.External) + if (payload.Packaging == PackagingType.Embedded && payload.ContainerRef == BurnConstants.BurnUXContainerName) + { + if (!String.IsNullOrEmpty(payload.DownloadUrl)) + { + this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForBAPayloads(payload.SourceLineNumbers, payload.Id.Id)); + } + } + else { var packageId = payload.ParentPackagePayloadRef; var parentUrl = payload.ParentPackagePayloadRef == null ? null : this.PayloadsById[payload.ParentPackagePayloadRef].DownloadUrl; @@ -70,13 +78,6 @@ namespace WixToolset.Core.Burn.Bind payload.DownloadUrl = resolvedUrl; } } - else if (payload.Packaging == PackagingType.Embedded) - { - if (!String.IsNullOrEmpty(payload.DownloadUrl)) - { - this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForEmbeddedPayloads(payload.SourceLineNumbers, payload.Id.Id)); - } - } } } diff --git a/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs b/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs index ab3b789..1eb3563 100644 --- a/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs +++ b/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs @@ -19,8 +19,7 @@ namespace WixToolset.Core.Burn.Bundles { public const string BurnNamespace = "http://wixtoolset.org/schemas/v4/2008/Burn"; public const string BurnUXContainerEmbeddedIdFormat = "u{0}"; - public const string BurnUXContainerPayloadIdFormat = "p{0}"; - public const string BurnAttachedContainerEmbeddedIdFormat = "a{0}"; + public const string BurnAuthoredContainerEmbeddedIdFormat = "a{0}"; public const string BADataFileName = "BootstrapperApplicationData.xml"; public const string BADataNamespace = "http://wixtoolset.org/schemas/v4/BootstrapperApplicationData"; diff --git a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs index db09b54..5655d23 100644 --- a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs +++ b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs @@ -166,9 +166,7 @@ namespace WixToolset.Core.Burn.Bundles // write the UX allPayloads... foreach (var payload in this.UXContainerPayloads) { - writer.WriteStartElement("Payload"); - this.WriteBurnManifestPayloadAttributes(writer, payload, true, this.Payloads); - writer.WriteEndElement(); + this.WriteBurnManifestUXPayload(writer, payload); } writer.WriteEndElement(); // @@ -183,20 +181,9 @@ namespace WixToolset.Core.Burn.Bundles } } - foreach (var payload in this.Payloads.Values) + foreach (var payload in this.Payloads.Values.Where(p => p.ContainerRef != BurnConstants.BurnUXContainerName)) { - if (PackagingType.Embedded == payload.Packaging && BurnConstants.BurnUXContainerName != payload.ContainerRef) - { - writer.WriteStartElement("Payload"); - this.WriteBurnManifestPayloadAttributes(writer, payload, true, this.Payloads); - writer.WriteEndElement(); - } - else if (PackagingType.External == payload.Packaging) - { - writer.WriteStartElement("Payload"); - this.WriteBurnManifestPayloadAttributes(writer, payload, false, this.Payloads); - writer.WriteEndElement(); - } + this.WriteBurnManifestPayload(writer, payload); } foreach (var rollbackBoundary in this.RollbackBoundaries) @@ -654,9 +641,9 @@ namespace WixToolset.Core.Burn.Bundles } } - private void WriteBurnManifestPayloadAttributes(XmlTextWriter writer, WixBundlePayloadSymbol payload, bool embeddedOnly, Dictionary allPayloads) + private void WriteBurnManifestPayload(XmlTextWriter writer, WixBundlePayloadSymbol payload) { - Debug.Assert(!embeddedOnly || PackagingType.Embedded == payload.Packaging); + writer.WriteStartElement("Payload"); writer.WriteAttributeString("Id", payload.Id.Id); writer.WriteAttributeString("FilePath", payload.Name); @@ -668,28 +655,46 @@ namespace WixToolset.Core.Burn.Bundles writer.WriteAttributeString("LayoutOnly", "yes"); } + if (!String.IsNullOrEmpty(payload.DownloadUrl)) + { + writer.WriteAttributeString("DownloadUrl", payload.DownloadUrl); + } + switch (payload.Packaging) { case PackagingType.Embedded: // this means it's in a container. + Debug.Assert(BurnConstants.BurnUXContainerName != payload.ContainerRef); + writer.WriteAttributeString("Packaging", "embedded"); writer.WriteAttributeString("SourcePath", payload.EmbeddedId); - - if (BurnConstants.BurnUXContainerName != payload.ContainerRef) - { - writer.WriteAttributeString("Container", payload.ContainerRef); - } + writer.WriteAttributeString("Container", payload.ContainerRef); break; case PackagingType.External: - if (!String.IsNullOrEmpty(payload.DownloadUrl)) - { - writer.WriteAttributeString("DownloadUrl", payload.DownloadUrl); - } - writer.WriteAttributeString("Packaging", "external"); writer.WriteAttributeString("SourcePath", payload.Name); break; } + + writer.WriteEndElement(); + } + + private void WriteBurnManifestUXPayload(XmlTextWriter writer, WixBundlePayloadSymbol payload) + { + Debug.Assert(PackagingType.Embedded == payload.Packaging); + Debug.Assert(BurnConstants.BurnUXContainerName == payload.ContainerRef); + + writer.WriteStartElement("Payload"); + + // TODO: The engine should be updated to not require FileSize, Hash, or Packaging for UX payloads since the values are never used. + writer.WriteAttributeString("Id", payload.Id.Id); + writer.WriteAttributeString("FilePath", payload.Name); + writer.WriteAttributeString("FileSize", payload.FileSize.Value.ToString(CultureInfo.InvariantCulture)); + writer.WriteAttributeString("Hash", payload.Hash); + writer.WriteAttributeString("Packaging", "embedded"); + writer.WriteAttributeString("SourcePath", payload.EmbeddedId); + + writer.WriteEndElement(); } } } diff --git a/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs b/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs index 8f36162..87a63cc 100644 --- a/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs +++ b/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs @@ -44,13 +44,6 @@ namespace WixToolset.Core.Burn.Bundles public void Execute() { - var payloadCount = this.Payloads.Count(); // The number of embedded payloads - - if (!String.IsNullOrEmpty(this.ManifestFile)) - { - ++payloadCount; - } - var cabinetPath = Path.GetFullPath(this.OutputPath); var files = new List(); diff --git a/src/WixToolset.Core/Compile/CompilerPayload.cs b/src/WixToolset.Core/Compile/CompilerPayload.cs index 4c7e843..3f42303 100644 --- a/src/WixToolset.Core/Compile/CompilerPayload.cs +++ b/src/WixToolset.Core/Compile/CompilerPayload.cs @@ -15,8 +15,6 @@ namespace WixToolset.Core public string Description { get; set; } - public string DisplayName { get; set; } - public string DownloadUrl { get; set; } public string Hash { get; set; } @@ -158,7 +156,7 @@ namespace WixToolset.Core if (!String.IsNullOrEmpty(this.DownloadUrl)) { - this.Core.Write(WarningMessages.DownloadUrlNotSupportedForEmbeddedPayloads(this.SourceLineNumbers, this.Id.Id)); + this.Core.Write(WarningMessages.DownloadUrlNotSupportedForBAPayloads(this.SourceLineNumbers, this.Id.Id)); } this.Compressed = YesNoDefaultType.Yes; @@ -174,7 +172,7 @@ namespace WixToolset.Core DownloadUrl = this.DownloadUrl, Compressed = (this.Compressed == YesNoDefaultType.Yes) ? true : (this.Compressed == YesNoDefaultType.No) ? (bool?)false : null, UnresolvedSourceFile = this.SourceFile, // duplicate of sourceFile but in a string column so it won't get resolved to a full path during binding. - DisplayName = this.DisplayName ?? this.ProductName, + DisplayName = this.ProductName, Description = this.Description, Hash = this.Hash, FileSize = this.Size, @@ -245,11 +243,6 @@ namespace WixToolset.Core this.Description = this.Core.GetAttributeValue(this.SourceLineNumbers, attrib); } - public void ParseDisplayName(XAttribute attrib) - { - this.DisplayName = this.Core.GetAttributeValue(this.SourceLineNumbers, attrib); - } - public void ParseDownloadUrl(XAttribute attrib) { this.DownloadUrl = this.Core.GetAttributeValue(this.SourceLineNumbers, attrib); diff --git a/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs b/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs index ff48ee0..dd381df 100644 --- a/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs +++ b/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs @@ -15,6 +15,50 @@ namespace WixToolsetTest.CoreIntegration public class ContainerFixture { + [Fact(Skip = "Test demonstrates failure")] + public void CanBuildWithCustomAttachedContainer() + { + var folder = TestData.Get(@"TestData"); + + using (var fs = new DisposableFileSystem()) + { + var baseFolder = fs.GetFolder(); + var intermediateFolder = Path.Combine(baseFolder, "obj"); + var binFolder = Path.Combine(baseFolder, "bin"); + var bundlePath = Path.Combine(binFolder, "test.exe"); + var baFolderPath = Path.Combine(baseFolder, "ba"); + var extractFolderPath = Path.Combine(baseFolder, "extract"); + + this.BuildMsis(folder, intermediateFolder, binFolder, buildToSubfolder: true); + + var result = WixRunner.Execute(new[] + { + "build", + Path.Combine(folder, "Container", "HarvestIntoAttachedContainer.wxs"), + Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"), + "-bindpath", Path.Combine(folder, "SimpleBundle", "data"), + "-bindpath", binFolder, + "-intermediateFolder", intermediateFolder, + "-o", bundlePath + }); + + result.AssertSuccess(); + + Assert.True(File.Exists(bundlePath)); + + var extractResult = BundleExtractor.ExtractBAContainer(null, bundlePath, baFolderPath, extractFolderPath); + extractResult.AssertSuccess(); + + var payloads = extractResult.SelectManifestNodes("/burn:BurnManifest/burn:Payload"); + Assert.Equal(4, payloads.Count); + var ignoreAttributes = new Dictionary> { { "Payload", new List { "FileSize", "Hash" } } }; + Assert.Equal(@"", payloads[0].GetTestXml(ignoreAttributes)); + Assert.Equal(@"", payloads[1].GetTestXml(ignoreAttributes)); + Assert.Equal(@"", payloads[2].GetTestXml(ignoreAttributes)); + Assert.Equal(@"", payloads[3].GetTestXml(ignoreAttributes)); + } + } + [Fact] public void HarvestedPayloadsArePutInCorrectContainer() { @@ -309,7 +353,7 @@ namespace WixToolsetTest.CoreIntegration } } - private void BuildMsis(string folder, string intermediateFolder, string binFolder) + private void BuildMsis(string folder, string intermediateFolder, string binFolder, bool buildToSubfolder = false) { var result = WixRunner.Execute(new[] { @@ -319,7 +363,7 @@ namespace WixToolsetTest.CoreIntegration Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"), "-bindpath", Path.Combine(folder, "SingleFile", "data"), "-intermediateFolder", intermediateFolder, - "-o", Path.Combine(binFolder, "FirstX86.msi"), + "-o", Path.Combine(binFolder, buildToSubfolder ? "FirstX86" : ".", "FirstX86.msi"), }); result.AssertSuccess(); @@ -332,7 +376,7 @@ namespace WixToolsetTest.CoreIntegration Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"), "-bindpath", Path.Combine(folder, "SingleFile", "data"), "-intermediateFolder", intermediateFolder, - "-o", Path.Combine(binFolder, "FirstX64.msi"), + "-o", Path.Combine(binFolder, buildToSubfolder ? "FirstX64" : ".", "FirstX64.msi"), }); result.AssertSuccess(); diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs new file mode 100644 index 0000000..ec757c5 --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + +