Skip to content

Commit

Permalink
Allow exact duplicate input entries when bundling single file (#50476)
Browse files Browse the repository at this point in the history 
SDK can (and will) produce inputs for bundling which contain exact duplicate entries (same source path and same target relative path). Currently publishing such app as non-single-file works just fine (files are overwritten), but publishing it as single-file fails.

Fixing this in the SDK seems like a rather complex problem - see dotnet/sdk#16576 for more details.

There's no harm in allowing exact duplicates and ignoring them when bundling (only one copy of the file is bundled) as that will be the same behavior as non-single-file publish.

If the duplicates are not exact (different source path) then still reject those. SDK currently allows that, but it's very problematic (effectively random output).
  • Loading branch information
@vitek-karas
vitek-karas authored Apr 2, 2021
1 parent 7fb3997 commit 8445e4e
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 7 deletions.
27 changes: 20 additions & 7 deletions src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,6 @@ public string GenerateBundle(IReadOnlyList<FileSpec> fileSpecs)
throw new ArgumentException("Invalid input specification: Must specify the host binary");
}

var bundleRelativePathCollision = fileSpecs.GroupBy(file => file.BundleRelativePath).FirstOrDefault(g => g.Count() > 1);
if (bundleRelativePathCollision != null)
{
string fileSpecPaths = string.Join(", ", bundleRelativePathCollision.Select(file => "'" + file.SourcePath + "'"));
throw new ArgumentException($"Invalid input specification: Found entries {fileSpecPaths} with the same BundleRelativePath '{bundleRelativePathCollision.Key}'");
}

string bundlePath = Path.Combine(OutputDir, HostName);
if (File.Exists(bundlePath))
{
Expand All @@ -275,6 +268,11 @@ public string GenerateBundle(IReadOnlyList<FileSpec> fileSpecs)

BinaryUtils.CopyFile(hostSource, bundlePath);

// Note: We're comparing file paths both on the OS we're running on as well as on the target OS for the app
// We can't really make assumptions about the file systems (even on Linux there can be case insensitive file systems
// and vice versa for Windows). So it's safer to do case sensitive comparison everywhere.
var relativePathToSpec = new Dictionary<string, FileSpec>(StringComparer.Ordinal);

long headerOffset = 0;
using (BinaryWriter writer = new BinaryWriter(File.OpenWrite(bundlePath)))
{
Expand Down Expand Up @@ -305,6 +303,21 @@ public string GenerateBundle(IReadOnlyList<FileSpec> fileSpecs)
continue;
}

if (relativePathToSpec.TryGetValue(fileSpec.BundleRelativePath, out var existingFileSpec))
{
if (!string.Equals(fileSpec.SourcePath, existingFileSpec.SourcePath, StringComparison.Ordinal))
{
throw new ArgumentException($"Invalid input specification: Found entries '{fileSpec.SourcePath}' and '{existingFileSpec.SourcePath}' with the same BundleRelativePath '{fileSpec.BundleRelativePath}'");
}

// Exact duplicate - intentionally skip and don't include a second copy in the bundle
continue;
}
else
{
relativePathToSpec.Add(fileSpec.BundleRelativePath, fileSpec);
}

using (FileStream file = File.OpenRead(fileSpec.SourcePath))
{
FileType targetType = Target.TargetSpecificFileType(type);
Expand Down
80 changes: 80 additions & 0 deletions ...osoft.NET.HostModel.Tests/Microsoft.NET.HostModel.Bundle.Tests/BundlerConsistencyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,34 @@ public void TestWithoutSpecifyingHostFails()
Assert.Throws<ArgumentException>(() => bundler.GenerateBundle(fileSpecs));
}

[Fact]
public void TestWithExactDuplicateEntriesPasses()
{
var fixture = sharedTestState.TestFixture.Copy();

var hostName = BundleHelper.GetHostName(fixture);
var bundleDir = BundleHelper.GetBundleDir(fixture);
var targetOS = BundleHelper.GetTargetOS(fixture.CurrentRid);
var targetArch = BundleHelper.GetTargetArch(fixture.CurrentRid);

// Generate a file specification with duplicate entries
var fileSpecs = new List<FileSpec>();
fileSpecs.Add(new FileSpec(BundleHelper.GetHostPath(fixture), BundleHelper.GetHostName(fixture)));
string appPath = BundleHelper.GetAppPath(fixture);
fileSpecs.Add(new FileSpec(appPath, "rel/app.repeat.dll"));
fileSpecs.Add(new FileSpec(appPath, "rel/app.repeat.dll"));
string systemLibPath = Path.Join(BundleHelper.GetPublishPath(fixture), "System.dll");
fileSpecs.Add(new FileSpec(systemLibPath, "rel/system.repeat.dll"));
fileSpecs.Add(new FileSpec(systemLibPath, "rel/system.repeat.dll"));

Bundler bundler = new Bundler(hostName, bundleDir.FullName, targetOS: targetOS, targetArch: targetArch);
bundler.GenerateBundle(fileSpecs);

// Exact duplicates are not duplicated in the bundle
bundler.BundleManifest.Files.Where(entry => entry.RelativePath.Equals("rel/app.repeat.dll")).Single().Type.Should().Be(FileType.Assembly);
bundler.BundleManifest.Files.Where(entry => entry.RelativePath.Equals("rel/system.repeat.dll")).Single().Type.Should().Be(FileType.Assembly);
}

[Fact]
public void TestWithDuplicateEntriesFails()
{
Expand All @@ -93,6 +121,58 @@ public void TestWithDuplicateEntriesFails()
.And.Contain(BundleHelper.GetAppPath(fixture));
}

[Fact]
public void TestWithCaseSensitiveDuplicateEntriesPasses()
{
var fixture = sharedTestState.TestFixture.Copy();

var hostName = BundleHelper.GetHostName(fixture);
var bundleDir = BundleHelper.GetBundleDir(fixture);
var targetOS = BundleHelper.GetTargetOS(fixture.CurrentRid);
var targetArch = BundleHelper.GetTargetArch(fixture.CurrentRid);

// Generate a file specification with duplicate entries
var fileSpecs = new List<FileSpec>();
fileSpecs.Add(new FileSpec(BundleHelper.GetHostPath(fixture), BundleHelper.GetHostName(fixture)));
fileSpecs.Add(new FileSpec(BundleHelper.GetAppPath(fixture), "rel/app.repeat.dll"));
fileSpecs.Add(new FileSpec(Path.Join(BundleHelper.GetPublishPath(fixture), "System.dll"), "rel/app.Repeat.dll"));

Bundler bundler = new Bundler(hostName, bundleDir.FullName, targetOS: targetOS, targetArch: targetArch);
bundler.GenerateBundle(fileSpecs);

bundler.BundleManifest.Files.Where(entry => entry.RelativePath.Equals("rel/app.repeat.dll")).Single().Type.Should().Be(FileType.Assembly);
bundler.BundleManifest.Files.Where(entry => entry.RelativePath.Equals("rel/app.Repeat.dll")).Single().Type.Should().Be(FileType.Assembly);
}

[Fact]
public void TestWithMultipleDuplicateEntriesFails()
{
var fixture = sharedTestState.TestFixture.Copy();

var hostName = BundleHelper.GetHostName(fixture);
var bundleDir = BundleHelper.GetBundleDir(fixture);
var targetOS = BundleHelper.GetTargetOS(fixture.CurrentRid);
var targetArch = BundleHelper.GetTargetArch(fixture.CurrentRid);

// Generate a file specification with duplicate entries
var fileSpecs = new List<FileSpec>();
fileSpecs.Add(new FileSpec(BundleHelper.GetHostPath(fixture), BundleHelper.GetHostName(fixture)));
string appPath = BundleHelper.GetAppPath(fixture);
fileSpecs.Add(new FileSpec(appPath, "rel/app.repeat.dll"));
fileSpecs.Add(new FileSpec(appPath, "rel/app.repeat.dll"));
string systemLibPath = Path.Join(BundleHelper.GetPublishPath(fixture), "System.dll");
fileSpecs.Add(new FileSpec(appPath, "rel/system.repeat.dll"));
fileSpecs.Add(new FileSpec(systemLibPath, "rel/system.repeat.dll"));

Bundler bundler = new Bundler(hostName, bundleDir.FullName, targetOS: targetOS, targetArch: targetArch);
Assert.Throws<ArgumentException>(() => bundler.GenerateBundle(fileSpecs))
.Message
.Should().Contain("rel/system.repeat.dll")
.And.NotContain("rel/app.repeat.dll")
.And.Contain(appPath)
.And.Contain(systemLibPath);
}

[Fact]
public void TestBaseNameComputation()
{
Expand Down

0 comments on commit 8445e4e

Please sign in to comment.