New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIXBUG4187 - Melt doesn't export Binary or Icon tables #192

Merged
merged 3 commits into from Jan 31, 2015

Conversation

Projects
None yet
2 participants
@stunney
Contributor

stunney commented Jan 6, 2015

New -xn switch used in conjunction with -x to export files using the Dark style top level directory structure of
File
Binary
Icon

The contents of the File directory will match what melt exported before so there will be a minimal breaking change if people decide to utilize this switch.

stunney added some commits Dec 31, 2014

Melt changes to extract Binary table contents
Will help with WIXBUG4187.

Still to do.  Accept BinaryRef in PatchFamily node properly and the have
torch generate the transforms properly for pyro.
WIXBUG4187
Melt now has -xn switch used in conjunction with -x <path> to export
files from the install package in the same parent folder structure as
Dark (File, Binary, Icon top-level directories).  This fixes the
inability for patches to catch changes in those tables using the pure
wix patching method.
@barnson

View changes

Show outdated Hide outdated src/DTF/Libraries/WindowsInstaller.Package/InstallPackage.cs
@@ -190,7 +190,7 @@ public void ExtractFiles()
public void ExtractFiles(ICollection<string> fileKeys)
{
this.ProcessFilesByMediaDisk(fileKeys,
new ProcessFilesOnOneMediaDiskHandler(this.ExtractFilesOnOneMediaDisk));
new ProcessFilesOnOneMediaDiskHandler(this.ExtractFilesOnOneMediaDisk));

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

whitespace

@barnson

barnson Jan 10, 2015

Member

whitespace

This comment has been minimized.

@stunney

stunney Jan 30, 2015

Contributor

Bah! VS has been doing this to me a lot lately (I'm sure it is something that I am doing wrong, just don't know WHAT)!

@stunney

stunney Jan 30, 2015

Contributor

Bah! VS has been doing this to me a lot lately (I'm sure it is something that I am doing wrong, just don't know WHAT)!

@barnson

View changes

Show outdated Hide outdated src/DTF/Libraries/WindowsInstaller.Package/InstallPackage.cs
/// <returns>A <see cref="M:System.Colletions.Generic.IDictionary`1{string,string}"/> of the filename/fullpath key value pairs of the directory given.</returns>
public virtual IDictionary<string, string> GetFilePaths(string path, ICollection<string> names = null)
{
IDictionary<string, string> retval = new Dictionary<string, string>(100);

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

Needs a more meaningful name. "map"?

@barnson

barnson Jan 10, 2015

Member

Needs a more meaningful name. "map"?

This comment has been minimized.

@stunney

stunney Jan 30, 2015

Contributor

You are referring to "retval" being changed to "map" or something like that? :)

@stunney

stunney Jan 30, 2015

Contributor

You are referring to "retval" being changed to "map" or something like that? :)

This comment has been minimized.

@barnson

barnson Jan 31, 2015

Member

Yeah, just needs to be 10 percent more meaningful. :)

@barnson

barnson Jan 31, 2015

Member

Yeah, just needs to be 10 percent more meaningful. :)

@barnson

View changes

Show outdated Hide outdated src/DTF/Libraries/WindowsInstaller.Package/InstallPackage.cs
/// <param name="tableName">The name of the table to extract binary data from. Valid values are "Binary" and "Icon" or a custom table with Name and Data columns of type string and stream respectively.</param>
/// <param name="path">The path to extract the files to. The path must exist before calling this method.</param>
[SuppressMessage("Microsoft.Globalization", "CA1308:NormalizeStringsToUppercase")]
public virtual void ExtractFilesInBinaryTable(ICollection<string> names, string tableName, string path)

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

Also used for Icon table. Maybe "ExtractBinaryFiles"? Actually, I recommend moving this to Melt -- it's useful functionality but right now works only for tables that have binary fields in the second column. DTF should be more general-purpose.

@barnson

barnson Jan 10, 2015

Member

Also used for Icon table. Maybe "ExtractBinaryFiles"? Actually, I recommend moving this to Melt -- it's useful functionality but right now works only for tables that have binary fields in the second column. DTF should be more general-purpose.

This comment has been minimized.

@stunney

stunney Jan 30, 2015

Contributor

I will move to Melt but wouldn't this be handy for custom tables as well in the future that follow the same schema?

@stunney

stunney Jan 30, 2015

Contributor

I will move to Melt but wouldn't this be handy for custom tables as well in the future that follow the same schema?

This comment has been minimized.

@barnson

barnson Jan 31, 2015

Member

In DTF, I'd want something more generic, so it didn't require the exact same column definitions. (i.e., pass in the column).

@barnson

barnson Jan 31, 2015

Member

In DTF, I'd want something more generic, so it didn't require the exact same column definitions. (i.e., pass in the column).

@barnson

View changes

Show outdated Hide outdated src/DTF/Libraries/WindowsInstaller.Package/InstallPackage.cs
FileInfo binaryFile = new FileInfo(Path.Combine(path, binaryKey));
using (FileStream fs = binaryFile.Create())
{
for (int a = binaryData.ReadByte(); a != -1; a = binaryData.ReadByte()) fs.WriteByte((byte)a);

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

Not terribly concerned with perf in a tool like this but without buffering this could be unpleasant for bigger CA binaries. But all the easy stream copy methods are in .NET v4.

@barnson

barnson Jan 10, 2015

Member

Not terribly concerned with perf in a tool like this but without buffering this could be unpleasant for bigger CA binaries. But all the easy stream copy methods are in .NET v4.

@barnson

View changes

Show outdated Hide outdated src/tools/melt/MeltStrings.resx
(defaults to output files path)
-xn export contents in much the same manner as dark.exe

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

Don't want version references if we can avoid. Recommend "export contents of File, Binary, and Icon tables to subdirectories to support patching all types of files."

@barnson

barnson Jan 10, 2015

Member

Don't want version references if we can avoid. Recommend "export contents of File, Binary, and Icon tables to subdirectories to support patching all types of files."

@barnson

View changes

Show outdated Hide outdated src/tools/melt/melt.cs
/// <param name="package">The installer database to rip from.</param>
/// <param name="exportPath">The full path where files will be exported to.</param>
/// <param name="tableName">The name of the table to export.</param>
private void MeltBinaryTable(ref Pdb inputPdb, InstallPackage package, string exportPath, string tableName)

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

Only needs to be "ref" if you're replacing the object itself.

@barnson

barnson Jan 10, 2015

Member

Only needs to be "ref" if you're replacing the object itself.

@barnson

View changes

Show outdated Hide outdated src/tools/melt/melt.cs
/// <param name="tableName">The name of the table to export.</param>
private void MeltBinaryTable(ref Pdb inputPdb, InstallPackage package, string exportPath, string tableName)
{
if(string.IsNullOrEmpty(tableName))

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

DTF's different but WiX style is a space between keyword and opening parens. (Not for function invocations, though.)

@barnson

barnson Jan 10, 2015

Member

DTF's different but WiX style is a space between keyword and opening parens. (Not for function invocations, though.)

@barnson

View changes

Show outdated Hide outdated src/tools/melt/melt.cs
@@ -34,6 +34,7 @@ namespace Microsoft.Tools.WindowsInstallerXml.Tools
public sealed class Melt
{
private string exportBasePath;
private bool exportToWiX4Format;

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

As above. Maybe "exportToSubdirectories"?

@barnson

barnson Jan 10, 2015

Member

As above. Maybe "exportToSubdirectories"?

@barnson

View changes

Show outdated Hide outdated src/tools/melt/melt.cs
Table pdbTable = inputPdb.Output.Tables[tableName];
if (null == pdbTable)
{
throw new ArgumentException(string.Format("Table by the name of {0} was not found in the input PDB.", tableName), "tableAndSubDirectoryName");

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

This will crash with an exception if a table doesn't exist. It doesn't really matter if a particular table doesn't exist, so I suggest just returning if the table's null. (If it mattered, you could make it a bool method and return false.)

@barnson

barnson Jan 10, 2015

Member

This will crash with an exception if a table doesn't exist. It doesn't really matter if a particular table doesn't exist, so I suggest just returning if the table's null. (If it mattered, you could make it a bool method and return false.)

@barnson

View changes

Show outdated Hide outdated src/tools/melt/melt.cs
throw new ArgumentException(string.Format("Table by the name of {0} was not found in the input PDB.", tableName), "tableAndSubDirectoryName");
}
if (!Directory.Exists(exportPath)) Directory.CreateDirectory(exportPath);

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

You can skip the Exists check; Directory.CreateDirectory does that for you.

@barnson

barnson Jan 10, 2015

Member

You can skip the Exists check; Directory.CreateDirectory does that for you.

@barnson

View changes

Show outdated Hide outdated src/tools/melt/melt.cs
if(this.exportToWiX4Format)
{
exportBinaryPath = Path.Combine(outputDirectory, @"Binary");

This comment has been minimized.

@barnson

barnson Jan 10, 2015

Member

Don't need @ strings.

@barnson

barnson Jan 10, 2015

Member

Don't need @ strings.

This comment has been minimized.

@stunney

stunney Jan 30, 2015

Contributor

Just habbit :) I'll remove them.

@stunney

stunney Jan 30, 2015

Contributor

Just habbit :) I'll remove them.

@barnson

This comment has been minimized.

Show comment
Hide comment
@barnson

barnson Jan 10, 2015

Member

Sorry for the nitpicking but it's my job. 😄

Member

barnson commented Jan 10, 2015

Sorry for the nitpicking but it's my job. 😄

@stunney

This comment has been minimized.

Show comment
Hide comment
@stunney

stunney Jan 30, 2015

Contributor

I like nitpicking 👍

Contributor

stunney commented Jan 30, 2015

I like nitpicking 👍

WIXBUG4187 - Round2
Updates based on Bob Arnson's insightful feedback.

@barnson barnson merged commit 0ed35b5 into wixtoolset:develop Jan 31, 2015

@barnson

This comment has been minimized.

Show comment
Hide comment
@barnson

barnson Jan 31, 2015

Member

I added a history.md entry for you (and cleaned up some whitespace :) ). Thanks for working through it!

Member

barnson commented Jan 31, 2015

I added a history.md entry for you (and cleaned up some whitespace :) ). Thanks for working through it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment