10 files changed, 185 insertions, 11 deletions

diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs
index 93c617d9..4b3d554a 100644
--- a/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs
+++ b/src/WixToolset.Core.WindowsInstaller/Bind/BindDatabaseCommand.cs
@@ -364,7 +364,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
             Dictionary<MediaSymbol, IEnumerable<FileFacade>> filesByCabinetMedia;
             IEnumerable<FileFacade> uncompressedFiles;
             {
-                var order = new OptimizeFileFacadesOrderCommand(fileFacades);
+                var order = new OptimizeFileFacadesOrderCommand(this.BackendHelper, this.PathResolver, section, platform, fileFacades);
                 order.Execute();
 
                 fileFacades = order.FileFacades;
diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs
index 6943d345..e96dfd91 100644
--- a/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs
+++ b/src/WixToolset.Core.WindowsInstaller/Bind/OptimizeFileFacadesOrderCommand.cs
@@ -4,34 +4,112 @@ namespace WixToolset.Core.WindowsInstaller.Bind
 {
     using System;
     using System.Collections.Generic;
+    using System.Linq;
     using WixToolset.Core.Bind;
+    using WixToolset.Data;
+    using WixToolset.Data.Symbols;
+    using WixToolset.Extensibility.Data;
+    using WixToolset.Extensibility.Services;
 
     internal class OptimizeFileFacadesOrderCommand
     {
-        public OptimizeFileFacadesOrderCommand(List<FileFacade> fileFacades)
+        public OptimizeFileFacadesOrderCommand(IBackendHelper helper, IPathResolver pathResolver, IntermediateSection section, Platform platform, List<FileFacade> fileFacades)
         {
+            this.BackendHelper = helper;
+            this.PathResolver = pathResolver;
+            this.Section = section;
+            this.Platform = platform;
             this.FileFacades = fileFacades;
         }
 
         public List<FileFacade> FileFacades { get; private set; }
 
+        private IBackendHelper BackendHelper { get; }
+
+        private IPathResolver PathResolver { get; }
+
+        private IntermediateSection Section { get; }
+
+        private Platform Platform { get; }
+
         public List<FileFacade> Execute()
         {
-            this.FileFacades.Sort(FileFacadeOptimizer.Instance);
+            var canonicalComponentTargetPaths = this.ComponentTargetPaths();
+
+            this.FileFacades.Sort(new FileFacadeOptimizer(canonicalComponentTargetPaths));
 
             return this.FileFacades;
         }
 
+        private Dictionary<string, string> ComponentTargetPaths()
+        {
+            var directories = this.ResolveDirectories();
+
+            var canonicalPathsByDirectoryId = new Dictionary<string, string>();
+            foreach (var component in this.Section.Symbols.OfType<ComponentSymbol>())
+            {
+                var directoryPath = this.PathResolver.GetCanonicalDirectoryPath(directories, null, component.DirectoryRef, this.Platform);
+                canonicalPathsByDirectoryId.Add(component.Id.Id, directoryPath);
+            }
+
+            return canonicalPathsByDirectoryId;
+        }
+
+        private Dictionary<string, IResolvedDirectory> ResolveDirectories()
+        {
+            var targetPathsByDirectoryId = new Dictionary<string, IResolvedDirectory>();
+
+            // Get the target paths for all directories.
+            foreach (var directory in this.Section.Symbols.OfType<DirectorySymbol>())
+            {
+                var resolvedDirectory = this.BackendHelper.CreateResolvedDirectory(directory.ParentDirectoryRef, directory.Name);
+                targetPathsByDirectoryId.Add(directory.Id.Id, resolvedDirectory);
+            }
+
+            return targetPathsByDirectoryId;
+        }
+
         private class FileFacadeOptimizer : IComparer<FileFacade>
         {
-            public static readonly FileFacadeOptimizer Instance = new FileFacadeOptimizer();
+            public FileFacadeOptimizer(Dictionary<string, string> componentTargetPaths)
+            {
+                this.ComponentTargetPaths = componentTargetPaths;
+            }
+
+            private Dictionary<string, string> ComponentTargetPaths { get; }
 
             public int Compare(FileFacade x, FileFacade y)
             {
-                // TODO: Sort these facades even smarter by directory path and component id 
-                //       and maybe file size or file extension and other creative ideas to
-                //       get optimal install speed out of MSI.
-                return String.Compare(x.ComponentRef, y.ComponentRef, StringComparison.Ordinal);
+                // First group files by DiskId.
+                var compare = x.DiskId.CompareTo(y.DiskId);
+
+                if (compare != 0)
+                {
+                    return compare;
+                }
+
+                // Next try to group files by target install directory.
+                if (this.ComponentTargetPaths.TryGetValue(x.ComponentRef, out var canonicalX) &&
+                    this.ComponentTargetPaths.TryGetValue(y.ComponentRef, out var canonicalY))
+                {
+                    compare = String.Compare(canonicalX, canonicalY, StringComparison.Ordinal);
+
+                    if (compare != 0)
+                    {
+                        return compare;
+                    }
+                }
+
+                // TODO: Consider sorting these facades even smarter by file size or file extension
+                //       or other creative ideas to get optimal install speed out of MSI.
+                compare = String.Compare(x.FileName, y.FileName, StringComparison.Ordinal);
+
+                if (compare != 0)
+                {
+                    return compare;
+                }
+
+                return String.Compare(x.Id, y.Id, StringComparison.Ordinal);
             }
         }
     }
diff --git a/src/WixToolset.Core/Linker.cs b/src/WixToolset.Core/Linker.cs
index 431ba4c7..e0af89ba 100644
--- a/src/WixToolset.Core/Linker.cs
+++ b/src/WixToolset.Core/Linker.cs
@@ -198,8 +198,10 @@ namespace WixToolset.Core
                 }
 
                 // Report duplicates that would ultimately end up being primary key collisions.
-                var reportDupes = new ReportConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, resolve.ResolvedSections);
-                reportDupes.Execute();
+                {
+                    var reportDupes = new ReportConflictingSymbolsCommand(this.Messaging, find.PossibleConflicts, resolve.ResolvedSections);
+                    reportDupes.Execute();
+                }
 
                 if (this.Messaging.EncounteredError)
                 {
diff --git a/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs b/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs
index 5aef148e..ad62dea6 100644
--- a/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/CabFixture.cs
@@ -42,7 +42,7 @@ namespace WixToolsetTest.CoreIntegration
                 var fileRows = fileTable.Select(r => new FileRow(r)).OrderBy(f => f.Sequence).ToList();
 
                 Assert.Equal(new[] { 1, 2 }, fileRows.Select(f => f.Sequence).ToArray());
-                Assert.Equal(new[] { "test.txt", "Notepad.exe" }, fileRows.Select(f => f.Name).ToArray());
+                Assert.Equal(new[] { "Notepad.exe", "test.txt" }, fileRows.Select(f => f.Name).ToArray());
 
                 var files = Query.GetCabinetFiles(cabPath);
                 Assert.Equal(fileRows.Select(f => f.Id).ToArray(), files.Select(f => f.Name).ToArray());
diff --git a/src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs b/src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs
new file mode 100644
index 00000000..de18e30c
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/MediaFixture.cs
@@ -0,0 +1,62 @@
+// 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.CoreIntegration
+{
+    using System.IO;
+    using System.Linq;
+    using WixBuildTools.TestSupport;
+    using WixToolset.Core.TestPackage;
+    using WixToolset.Data;
+    using Xunit;
+
+    public class MediaFixture
+    {
+        [Fact]
+        public void CanBuildMultiMedia()
+        {
+            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\test.msi");
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "Media", "MultiMedia.wxs"),
+                    "-bindpath", Path.Combine(folder, "Media", "data"),
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", msiPath
+                });
+
+                result.AssertSuccess();
+
+                var intermediate = Intermediate.Load(Path.Combine(baseFolder, @"bin\test.wixpdb"));
+                var section = intermediate.Sections.Single();
+
+                var mediaSymbols = section.Symbols.OfType<WixToolset.Data.Symbols.MediaSymbol>().OrderBy(m => m.DiskId).ToList();
+                var fileSymbols = section.Symbols.OfType<WixToolset.Data.Symbols.FileSymbol>().OrderBy(f => f.Sequence).ToList();
+                Assert.Equal(1, mediaSymbols[0].DiskId);
+                Assert.Equal(2, mediaSymbols[0].LastSequence);
+                Assert.Equal(2, mediaSymbols[1].DiskId);
+                Assert.Equal(4, mediaSymbols[1].LastSequence);
+                Assert.Equal(new[]
+                {
+                    "a1.txt",
+                    "a2.txt",
+                    "b1.txt",
+                    "b2.txt",
+                }, fileSymbols.Select(f => f.Name).ToArray());
+                Assert.Equal(new[]
+                {
+                    1,
+                    2,
+                    3,
+                    4,
+                }, fileSymbols.Select(f => f.Sequence).ToArray());
+            }
+        }
+    }
+}
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs
new file mode 100644
index 00000000..8a555bda
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/MultiMedia.wxs
@@ -0,0 +1,28 @@
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Package Name="~MultiMedia" Codepage="1252" Language="1033" Version="1.0.0.0" Manufacturer="Example Corporation"
+             UpgradeCode="12E4699F-E774-4D05-8A01-5BDD41BBA127">
+
+        <MajorUpgrade DowngradeErrorMessage="A newer version of [ProductName] is already installed." />
+
+        <Media Id="1" Cabinet="cab1.cab" />
+        <Media Id="2" Cabinet="cab2.cab" />
+        
+        <Feature Id="ProductFeature" Title="MsiPackageTitle">
+            <Component Directory="ProgramFilesFolder:\~MultiMedia" DiskId="1">
+                <File Source="a1.txt" />
+            </Component>
+
+            <Component Directory="ProgramFilesFolder:\~MultiMedia" DiskId="1">
+                <File Source="a2.txt" />
+            </Component>
+
+            <Component Directory="ProgramFilesFolder:\~MultiMedia" DiskId="2">
+                <File Source="b2.txt" />
+            </Component>
+
+            <Component Directory="ProgramFilesFolder:\~MultiMedia" DiskId="2">
+                <File Source="b1.txt" />
+            </Component>
+        </Feature>
+    </Package>
+</Wix>
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt
new file mode 100644
index 00000000..ad9cdcb5
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a1.txt
@@ -0,0 +1 @@
+This is a1.txt
\ No newline at end of file
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt
new file mode 100644
index 00000000..d5de23de
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/a2.txt
@@ -0,0 +1 @@
+This is a2.txt
\ No newline at end of file
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt
new file mode 100644
index 00000000..88bc4a56
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b1.txt
@@ -0,0 +1 @@
+This is b1.txt
\ No newline at end of file
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt
new file mode 100644
index 00000000..38525276
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Media/data/b2.txt
@@ -0,0 +1 @@
+This is b2.txt
\ No newline at end of file