Skip to content

Commit

Permalink
ResolvePackageDependencies produces fewer outputs
Browse files Browse the repository at this point in the history 
From VS 16.7 onwards, the DTB no longer requires items of type
`TargetDefinitions`, `FileDefinitions`, `FileDependencies`.

This commit stops producing them by default. To restore the previous
behaviour, set the `EmitLegacyAssetsFileItems` property to `true`.
  • Loading branch information
@drewnoakes
drewnoakes committed May 12, 2020
1 parent d1a5111 commit 28e7900
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 37 deletions.
49 changes: 36 additions & 13 deletions src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolvePackageDependenciesTask.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ public class GivenAResolvePackageDependenciesTask

[Theory]
[MemberData(nameof(ItemCounts))]
public void ItRaisesLockFileToMSBuildItems(string projectName, int [] counts)
public void ItRaisesLockFileToMSBuildItems(string projectName, int[] counts, bool emitLegacyAssetsFileItems)
{
var task = GetExecutedTaskFromPrefix(projectName, out _);
var task = GetExecutedTaskFromPrefix(projectName, out _, emitLegacyAssetsFileItems);

task.PackageDefinitions .Count().Should().Be(counts[0]);
task.FileDefinitions .Count().Should().Be(counts[1]);
Expand All @@ -39,16 +39,36 @@ public static IEnumerable<object[]> ItemCounts
{
new object[] {
"dotnet.new",
new int[] { 110, 2536, 1, 846, 73 }
new int[] { 110, 2536, 1, 846, 73 },
true
},
new object[] {
"dotnet.new",
new int[] { 110, 0, 0, 846, 0 },
false
},
new object[] {
"simple.dependencies",
new int[] { 113, 2613, 1, 878, 94 },
true
},
new object[] {
"simple.dependencies",
new int[] { 113, 2613, 1, 878, 94}
new int[] { 113, 0, 0, 878, 0 },
false
},
};
}
}

[Fact]
public void ItOmitsLegacyItemsByDefault()
{
var task = new ResolvePackageDependencies();

task.EmitLegacyAssetsFileItems.Should().Be(false);
}

[Theory]
[InlineData("dotnet.new")]
[InlineData("simple.dependencies")]
Expand Down Expand Up @@ -524,7 +544,8 @@ public void ItAddsAnalyzerMetadataAndFileDependencies()
{
ProjectAssetsFile = lockFile.Path,
ProjectPath = null,
ProjectLanguage = projectLanguage // set language
ProjectLanguage = projectLanguage, // set language
EmitLegacyAssetsFileItems = true
};
task.Execute().Should().BeTrue();

Expand Down Expand Up @@ -608,7 +629,8 @@ public void ItFiltersAnalyzersByProjectLanguage()
{
ProjectAssetsFile = lockFile.Path,
ProjectPath = null,
ProjectLanguage = projectLanguage // set language
ProjectLanguage = projectLanguage, // set language
EmitLegacyAssetsFileItems = true
};
task.Execute().Should().BeTrue();

Expand Down Expand Up @@ -766,32 +788,33 @@ public void ItMarksTransitiveProjectReferences()
others.Where(t => t.ItemSpec == "ProjF/1.0.0").Count().Should().Be(1);
}

private static ResolvePackageDependencies GetExecutedTaskFromPrefix(string lockFilePrefix, out LockFile lockFile)
private static ResolvePackageDependencies GetExecutedTaskFromPrefix(string lockFilePrefix, out LockFile lockFile, bool emitLegacyAssetsFileItems = true)
{
lockFile = TestLockFiles.GetLockFile(lockFilePrefix);
return GetExecutedTask(lockFile);
return GetExecutedTask(lockFile, emitLegacyAssetsFileItems);
}

private static ResolvePackageDependencies GetExecutedTaskFromContents(string lockFileContents, out LockFile lockFile)
private static ResolvePackageDependencies GetExecutedTaskFromContents(string lockFileContents, out LockFile lockFile, bool emitLegacyAssetsFileItems = true)
{
lockFile = TestLockFiles.CreateLockFile(lockFileContents);
return GetExecutedTask(lockFile);
return GetExecutedTask(lockFile, emitLegacyAssetsFileItems);
}

private static ResolvePackageDependencies GetExecutedTask(LockFile lockFile)
private static ResolvePackageDependencies GetExecutedTask(LockFile lockFile, bool emitLegacyAssetsFileItems)
{
var resolver = new MockPackageResolver(_packageRoot);

var task = new ResolvePackageDependencies(lockFile, resolver)
{
ProjectAssetsFile = lockFile.Path,
ProjectPath = _projectPath,
ProjectLanguage = null
ProjectLanguage = null,
EmitLegacyAssetsFileItems = emitLegacyAssetsFileItems
};

task.Execute().Should().BeTrue();

return task;
}
}
}
}
64 changes: 42 additions & 22 deletions src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageDependencies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ namespace Microsoft.NET.Build.Tasks
/// </summary>
public sealed class ResolvePackageDependencies : TaskBase
{
/// <summary>
/// Only used if <see cref="EmitLegacyAssetsFileItems"/> is <see langword="true"/>.
/// </summary>
private readonly Dictionary<string, string> _fileTypes = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

private HashSet<string> _projectFileDependencies;
private IPackageResolver _packageResolver;
private LockFile _lockFile;
Expand All @@ -32,6 +36,7 @@ public sealed class ResolvePackageDependencies : TaskBase

/// <summary>
/// All the targets in the lock file.
/// Only populated if <see cref="EmitLegacyAssetsFileItems"/> is <see langword="true"/>.
/// </summary>
[Output]
public ITaskItem[] TargetDefinitions
Expand All @@ -49,7 +54,8 @@ public ITaskItem[] PackageDefinitions
}

/// <summary>
/// All the files in the lock file
/// All the files in the lock file.
/// Only populated if <see cref="EmitLegacyAssetsFileItems"/> is <see langword="true"/>.
/// </summary>
[Output]
public ITaskItem[] FileDefinitions
Expand All @@ -58,7 +64,7 @@ public ITaskItem[] FileDefinitions
}

/// <summary>
/// All the dependencies between packages. Each package has metadata 'ParentPackage'
/// All the dependencies between packages. Each package has metadata 'ParentPackage'
/// to refer to the package that depends on it. For top level packages this value is blank.
/// </summary>
[Output]
Expand All @@ -69,7 +75,8 @@ public ITaskItem[] PackageDependencies

/// <summary>
/// All the dependencies between files and packages, labeled by the group containing
/// the file (e.g. CompileTimeAssembly, RuntimeAssembly, etc.)
/// the file (e.g. CompileTimeAssembly, RuntimeAssembly, etc.).
/// Only populated if <see cref="EmitLegacyAssetsFileItems"/> is <see langword="true"/>.
/// </summary>
[Output]
public ITaskItem[] FileDependencies
Expand Down Expand Up @@ -106,6 +113,12 @@ public string ProjectLanguage
get; set;
}

/// <summary>
/// Setting this property restores pre-16.7 behaviour of populating <see cref="TargetDefinitions"/>,
/// <see cref="FileDefinitions"/> and <see cref="FileDependencies"/> outputs.
/// </summary>
public bool EmitLegacyAssetsFileItems { get; set; } = false;

#endregion

public ResolvePackageDependencies()
Expand Down Expand Up @@ -145,13 +158,12 @@ private void ReadProjectFileDependencies()
// get library and file definitions
private void GetPackageAndFileDefinitions()
{
TaskItem item;
foreach (var package in LockFile.Libraries)
{
var packageName = package.Name;
var packageVersion = package.Version.ToNormalizedString();
string packageId = $"{packageName}/{packageVersion}";
item = new TaskItem(packageId);
var item = new TaskItem(packageId);
item.SetMetadata(MetadataKeys.Name, packageName);
item.SetMetadata(MetadataKeys.Type, package.Type);
item.SetMetadata(MetadataKeys.Version, packageVersion);
Expand All @@ -163,6 +175,11 @@ private void GetPackageAndFileDefinitions()

_packageDefinitions.Add(item);

if (!EmitLegacyAssetsFileItems)
{
continue;
}

foreach (var file in package.Files)
{
if (NuGetUtils.IsPlaceholderFile(file))
Expand Down Expand Up @@ -215,17 +232,19 @@ private void GetPackageAndFileDefinitions()
// get target definitions and package and file dependencies
private void RaiseLockFileTargets()
{
TaskItem item;
foreach (var target in LockFile.Targets)
{
item = new TaskItem(target.Name);
item.SetMetadata(MetadataKeys.RuntimeIdentifier, target.RuntimeIdentifier ?? string.Empty);
item.SetMetadata(MetadataKeys.TargetFrameworkMoniker, target.TargetFramework.DotNetFrameworkName);
item.SetMetadata(MetadataKeys.FrameworkName, target.TargetFramework.Framework);
item.SetMetadata(MetadataKeys.FrameworkVersion, target.TargetFramework.Version.ToString());
item.SetMetadata(MetadataKeys.Type, "target");

_targetDefinitions.Add(item);
if (EmitLegacyAssetsFileItems)
{
TaskItem item = new TaskItem(target.Name);
item.SetMetadata(MetadataKeys.RuntimeIdentifier, target.RuntimeIdentifier ?? string.Empty);
item.SetMetadata(MetadataKeys.TargetFrameworkMoniker, target.TargetFramework.DotNetFrameworkName);
item.SetMetadata(MetadataKeys.FrameworkName, target.TargetFramework.Framework);
item.SetMetadata(MetadataKeys.FrameworkVersion, target.TargetFramework.Version.ToString());
item.SetMetadata(MetadataKeys.Type, "target");

_targetDefinitions.Add(item);
}

// raise each library in the target
GetPackageAndFileDependencies(target);
Expand All @@ -242,15 +261,14 @@ private void GetPackageAndFileDependencies(LockFileTarget target)
.Where(lib => lib.IsTransitiveProjectReference(LockFile, ref _projectFileDependencies))
.Select(pkg => pkg.Name),
StringComparer.OrdinalIgnoreCase);

TaskItem item;

foreach (var package in target.Libraries)
{
string packageId = $"{package.Name}/{package.Version.ToNormalizedString()}";

if (_projectFileDependencies.Contains(package.Name))
{
item = new TaskItem(packageId);
TaskItem item = new TaskItem(packageId);
item.SetMetadata(MetadataKeys.ParentTarget, target.Name); // Foreign Key
item.SetMetadata(MetadataKeys.ParentPackage, string.Empty); // Foreign Key

Expand All @@ -260,8 +278,11 @@ private void GetPackageAndFileDependencies(LockFileTarget target)
// get sub package dependencies
GetPackageDependencies(package, target.Name, resolvedPackageVersions, transitiveProjectRefs);

// get file dependencies on this package
GetFileDependencies(package, target.Name);
if (EmitLegacyAssetsFileItems)
{
// get file dependencies on this package
GetFileDependencies(package, target.Name);
}
}
}

Expand All @@ -272,7 +293,6 @@ private void GetPackageDependencies(
HashSet<string> transitiveProjectRefs)
{
string packageId = $"{package.Name}/{package.Version.ToNormalizedString()}";
TaskItem item;
foreach (var deps in package.Dependencies)
{
if (!resolvedPackageVersions.TryGetValue(deps.Id, out string version))
Expand All @@ -282,7 +302,7 @@ private void GetPackageDependencies(

string depsName = $"{deps.Id}/{version}";

item = new TaskItem(depsName);
TaskItem item = new TaskItem(depsName);
item.SetMetadata(MetadataKeys.ParentTarget, targetName); // Foreign Key
item.SetMetadata(MetadataKeys.ParentPackage, packageId); // Foreign Key

Expand All @@ -308,7 +328,7 @@ private void GetFileDependencies(LockFileTargetLibrary package, string targetNam
string filePath = entry.Item1;
IDictionary<string, string> properties = entry.Item2;

if (NuGetUtils.IsPlaceholderFile(filePath))
if (NuGetUtils.IsPlaceholderFile(filePath) || !EmitLegacyAssetsFileItems)
{
continue;
}
Expand Down
11 changes: 9 additions & 2 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.PackageDependencyResolution.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ Copyright (c) .NET Foundation. All rights reserved.
<NuGetTargetMoniker Condition="'$(NuGetTargetMoniker)' == '' AND '$(UseTargetPlatformAsNuGetTargetMoniker)' != 'true'">$(TargetFrameworkMoniker)</NuGetTargetMoniker>

<EmitAssetsLogMessages Condition="'$(EmitAssetsLogMessages)' == ''">true</EmitAssetsLogMessages>

<!-- Setting this property restores pre-16.7 behaviour of ResolvePackageDependencies to produce
TargetDefinitions, FileDefinitions and FileDependencies items. -->
<EmitLegacyAssetsFileItems Condition="'$(EmitLegacyAssetsFileItems)' == ''">false</EmitLegacyAssetsFileItems>
</PropertyGroup>

<!-- Target Moniker + RID-->
Expand Down Expand Up @@ -183,12 +187,15 @@ Copyright (c) .NET Foundation. All rights reserved.
ProjectPath="$(MSBuildProjectFullPath)"
ProjectAssetsFile="$(ProjectAssetsFile)"
ProjectLanguage="$(Language)"
EmitLegacyAssetsFileItems="$(EmitLegacyAssetsFileItems)"
ContinueOnError="ErrorAndContinue">

<Output TaskParameter="TargetDefinitions" ItemName="TargetDefinitions" />
<Output TaskParameter="PackageDefinitions" ItemName="PackageDefinitions" />
<Output TaskParameter="FileDefinitions" ItemName="FileDefinitions" />
<Output TaskParameter="PackageDependencies" ItemName="PackageDependencies" />

<!-- These outputs only produced when EmitLegacyAssetsFileItems is true -->
<Output TaskParameter="TargetDefinitions" ItemName="TargetDefinitions" />
<Output TaskParameter="FileDefinitions" ItemName="FileDefinitions" />
<Output TaskParameter="FileDependencies" ItemName="FileDependencies" />
</ResolvePackageDependencies>

Expand Down
1 change: 1 addition & 0 deletions src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreApp.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ public void It_restores_only_ridless_tfm()
"netcoreapp2.1", "TargetDefinitions", GetValuesCommand.ValueType.Item)
{
DependsOnTargets = "RunResolvePackageDependencies",
Properties = { { "EmitLegacyAssetsFileItems", "true" } }
};

getValuesCommand
Expand Down
13 changes: 13 additions & 0 deletions src/Tests/Microsoft.NET.TestFramework/Commands/GetValuesCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public enum ValueType
public string Configuration { get; set; }

public List<string> MetadataNames { get; set; } = new List<string>();
public Dictionary<string, string> Properties { get; } = new Dictionary<string, string>();

public GetValuesCommand(ITestOutputHelper log, string projectPath, string targetFramework,
string valueName, ValueType valueType = ValueType.Property)
Expand Down Expand Up @@ -69,8 +70,20 @@ protected override SdkCommandSpec CreateCommand(IEnumerable<string> args)
}
}

string propertiesElement = "";
if (Properties.Count != 0)
{
propertiesElement += "<PropertyGroup>\n";
foreach ((string key, string value) in Properties)
{
propertiesElement += $" <{key}>{value}</{key}>\n";
}
propertiesElement += " </PropertyGroup>";
}

string injectTargetContents =
$@"<Project ToolsVersion=`14.0` xmlns=`http://schemas.microsoft.com/developer/msbuild/2003`>
{propertiesElement}
<Target Name=`WriteValuesToFile` {(ShouldCompile ? $"DependsOnTargets=`{DependsOnTargets}`" : "")}>
<ItemGroup>
<LinesToWrite Include=`{linesAttribute}`/>
Expand Down

0 comments on commit 28e7900

Please sign in to comment.