Skip to content

Commit

Permalink
Automatically pack transitive assets when PrivateAssets=all
Browse files Browse the repository at this point in the history
When using `<PackageReference PrivateAssets=all `, transitive dependencies were not being brough in. This required package authors to reference explicitly each and every package they wanted packaged as private assets. This was particularly annoying for build and analyzer packages, which need to package all their dependencies privately.

This commit leverages the SDK-provided `RunResolvePackageDependencies` which returns the transitive closure of all referenced packages as a list of package>parent list. We use that to add the concept of "implicit package references" that basically share the PrivateAssets=all that brought them in. The inference target then just considers both `@(PackageReference)` as well as `@(ImplicitPackageReference)` to determine the primary output dependencies to pack, but otherwise the existing logic remains unchanged.

This behavior still honors the `Pack=false` on the PackageReference, but also skips the implificly defined packages like NETStandard.Library and Microsoft.NETCore.

Fixes NuGet/Home#5103.
  • Loading branch information
kzu committed Sep 30, 2020
1 parent d9f32b8 commit 12b3cd7
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 12 deletions.
105 changes: 105 additions & 0 deletions src/NuGetizer.Tasks/InferImplicitPackageReference.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;

namespace NuGetizer.Tasks
{
public class InferImplicitPackageReference : Task
{
[Required]
public ITaskItem[] PackageReferences { get; set; } = Array.Empty<ITaskItem>();

[Required]
public ITaskItem[] PackageDependencies { get; set; } = Array.Empty<ITaskItem>();

[Output]
public ITaskItem[] ImplicitPackageReferences { get; set; } = Array.Empty<ITaskItem>();

public override bool Execute()
{
var packages = new ConcurrentDictionary<PackageIdentity, List<PackageIdentity>>();
Func<string, PackageIdentity> parse = value =>
{
var parts = value.Split('/');
return new PackageIdentity(parts[0], parts[1]);
};

// Build the list of parent>child relationships.
foreach (var dependency in PackageDependencies.Where(x => x.ItemSpec.Contains('/')))
{
var identity = parse(dependency.ItemSpec);
var parent = dependency.GetMetadata("ParentPackage");
if (!string.IsNullOrEmpty(parent))
{
packages.GetOrAdd(parse(parent), _ => new List<PackageIdentity>())
.Add(identity);
}
}

var inferred = new HashSet<PackageIdentity>();

foreach (var reference in PackageReferences.Where(x =>
"all".Equals(x.GetMetadata("PrivateAssets"), StringComparison.OrdinalIgnoreCase) &&
// Unless explicitly set to Pack=false
(!x.TryGetBoolMetadata("Pack", out var pack) || pack != false) &&
// NETCore/NETStandard are implicitly defined, we never need to bring them as deps.
!(bool.TryParse(x.GetMetadata("IsImplicitlyDefined"), out var isImplicit) && isImplicit)))
{
var identity = new PackageIdentity(reference.ItemSpec, reference.GetMetadata("Version"));
foreach (var dependency in FindDependencies(identity, packages))
{
inferred.Add(dependency);
}
}

ImplicitPackageReferences = inferred
.Select(x => new TaskItem(
x.Id,
new Dictionary<string, string>
{
{ "Version", x.Version } ,
{ "PrivateAssets", "all" },
}))
.ToArray();

return true;
}

IEnumerable<PackageIdentity> FindDependencies(PackageIdentity identity, IDictionary<PackageIdentity, List<PackageIdentity>> packages)
{
if (packages.TryGetValue(identity, out var dependencies))
{
foreach (var dependency in dependencies)
{
yield return dependency;
foreach (var child in FindDependencies(dependency, packages))
{
yield return child;
}
}
}
}

class PackageIdentity
{
public PackageIdentity(string id, string version)
=> (Id, Version)
= (id, version);

public string Id { get; }
public string Version { get; }

public override bool Equals(object obj)
=> obj is PackageIdentity dependency &&
dependency.Id == Id &&
dependency.Version == Version;

public override int GetHashCode() => Tuple.Create(Id, Version).GetHashCode();

public override string ToString() => Id + "/" + Version;
}
}
}
28 changes: 22 additions & 6 deletions src/NuGetizer.Tasks/NuGetizer.Inference.targets
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Copyright (c) .NET Foundation. All rights reserved.
***********************************************************************************************
-->
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<UsingTask TaskName="NuGetizer.Tasks.InferImplicitPackageReference" AssemblyFile="NuGetizer.Tasks.dll" />

<PropertyGroup>
<!-- The Kind of primary output (build, symbols and doc) set if PackBuildOutput = true -->
Expand Down Expand Up @@ -70,7 +71,8 @@ Copyright (c) .NET Foundation. All rights reserved.
</PropertyGroup>
</Target>

<Target Name="_SetDefaultPackageReferencePack" Condition="'$(BuildOutputKind)' == 'build'" BeforeTargets="InferPrimaryOutputDependencies;InferPackageContents">
<Target Name="_SetDefaultPackageReferencePack" Condition="'$(BuildOutputKind)' == 'build'"
BeforeTargets="InferPrimaryOutputDependencies;InferPackageContents">
<ItemGroup>
<PackageReference Update="@(PackageReference)"
Condition="$([MSBuild]::ValueOrDefault('%(Identity)', '').StartsWith('Microsoft.Build')) and '%(Pack)' != 'true'"
Expand Down Expand Up @@ -154,7 +156,9 @@ Copyright (c) .NET Foundation. All rights reserved.
it also includes mscorlib which we don't need
TBD: maybe include ResolvedFrom=ImplicitlyExpandDesignTimeFacades too? -->
<_InferredPackageFile Include="@(ReferencePath->'%(OriginalItemSpec)')"
Condition="'$(PackFrameworkReferences)' == 'true' and '%(ReferencePath.ResolvedFrom)' == '{TargetFrameworkDirectory}' and '%(ReferencePath.Pack)' != 'false'">
Condition="'$(PackFrameworkReferences)' == 'true' and
'%(ReferencePath.ResolvedFrom)' == '{TargetFrameworkDirectory}' and
'%(ReferencePath.Pack)' != 'false'">
<Kind>FrameworkReference</Kind>
</_InferredPackageFile>
</ItemGroup>
Expand All @@ -174,30 +178,41 @@ Copyright (c) .NET Foundation. All rights reserved.
</ItemGroup>
</Target>

<Target Name="_CollectPrimaryOutputRelatedFiles" DependsOnTargets="BuildOnlySettings;ResolveReferences" Returns="@(_PrimaryOutputRelatedFile)">
<Target Name="_CollectPrimaryOutputDependencies" DependsOnTargets="BuildOnlySettings;ResolveReferences" Returns="@(ImplicitPackageReference)">
<ItemGroup>
<_PrimaryOutputRelatedFile Include="@(ReferencePath);@(_ReferenceRelatedPaths)"
Condition="'%(NuGetPackageId)' != 'NETStandard.Library' and
'%(Facade)' != 'true' and
'%(FrameworkFile)' != 'true' and
'%(Pack)' != 'false'"/>
<_PrivateAssetsPackageReference Include="@(PackageReference -> WithMetadataValue('PrivateAssets', 'all'))"
Condition="'%(PackageReference.IsImplicitlyDefined)' != 'true' and '%(PackageReference.Pack)' != 'false'"/>
</ItemGroup>
<InferImplicitPackageReference Condition="'@(_PrivateAssetsPackageReference)' != '' and '@(PackageDependencies)' != ''"
PackageReferences="@(_PrivateAssetsPackageReference)"
PackageDependencies="@(PackageDependencies)">
<Output TaskParameter="ImplicitPackageReferences" ItemName="ImplicitPackageReference" />
</InferImplicitPackageReference>
</Target>

<Target Name="_ResolvePackageDependencies" Condition="'$(UsingMicrosoftNETSdk)' == 'true'" DependsOnTargets="RunResolvePackageDependencies" />

<Target Name="InferPrimaryOutputDependencies"
Inputs="@(_PrimaryOutputRelatedFile)"
Outputs="%(_PrimaryOutputRelatedFile.NuGetPackageId)"
Returns="@(_InferredPackageFile)"
DependsOnTargets="_CollectPrimaryOutputRelatedFiles">
DependsOnTargets="_ResolvePackageDependencies;_CollectPrimaryOutputDependencies">

<ItemGroup>
<_NuGetPackageId Include="@(_PrimaryOutputRelatedFile -> '%(NuGetPackageId)')" Condition="'%(NuGetPackageId)' != 'NETStandard.Library'" />
</ItemGroup>
<PropertyGroup>
<_NuGetPackageId>@(_NuGetPackageId -> Distinct())</_NuGetPackageId>
</PropertyGroup>
<ItemGroup>
<_PrimaryPackageReference Include="@(PackageReference)" Condition="'$(_NuGetPackageId)' != '' and '%(Identity)' == '$(_NuGetPackageId)'" />
<_PrimaryPackageReference Include="@(PackageReference);@(ImplicitPackageReference)" Condition="'$(_NuGetPackageId)' != '' and '%(Identity)' == '$(_NuGetPackageId)'" />
</ItemGroup>

<PropertyGroup>
<_PrivateAssets>@(_PrimaryPackageReference -> '%(PrivateAssets)')</_PrivateAssets>
<_ShouldPack>@(_PrimaryPackageReference -> '%(Pack)')</_ShouldPack>
Expand All @@ -215,7 +230,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- In this case, we only add files that have a matching path to the private assets value.
i.e. for Mono.Options, PrivateAssets=lib, we'll include the file if its full path contains
'Mono.Options\*\lib\*', meaning the file is a lib. -->
<_InferredPackageFile Include="@(_PrimaryOutputRelatedFile)" Condition="$([System.Text.RegularExpressions.Regex]::IsMatch('%(_PrimaryOutputRelatedFile.FullPath)', '$(_ShouldIncludeAssetsRegex)', 'RegexOptions.IgnoreCase')) == 'true'">
<_InferredPackageFile Include="@(_PrimaryOutputRelatedFile)"
Condition="$([System.Text.RegularExpressions.Regex]::IsMatch('%(_PrimaryOutputRelatedFile.FullPath)', '$(_ShouldIncludeAssetsRegex)', 'RegexOptions.IgnoreCase')) == 'true'">
<Kind>$(BuildOutputKind)</Kind>
<FrameworkSpecific>$(BuildOutputFrameworkSpecific)</FrameworkSpecific>
</_InferredPackageFile>
Expand Down
7 changes: 1 addition & 6 deletions src/NuGetizer.Tasks/NuGetizer.Tasks.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@
<ItemGroup>
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="all" />
<PackageReference Include="Microsoft.Build.Tasks.Core" Version="15.9.20" PrivateAssets="all" />
<PackageReference Include="NuGet.Common" Version="5.7.0" PrivateAssets="all" />
<PackageReference Include="NuGet.Frameworks" Version="5.7.0" PrivateAssets="all" />
<PackageReference Include="NuGet.Packaging" Version="5.7.0" PrivateAssets="all" />
<PackageReference Include="NuGet.ProjectManagement" Version="4.2.0" PrivateAssets="all" />
<PackageReference Include="NuGet.Versioning" Version="5.7.0" PrivateAssets="all" />
<PackageReference Include="ThisAssembly" Version="0.10.6" PrivateAssets="all" />
</ItemGroup>

Expand All @@ -26,9 +23,7 @@
<None Update="@(None)" CopyToOutputDirectory="PreserveNewest" Pack="true" />
<None Update="NuGetizer.MultiTargeting.props" PackagePath="buildMultiTargeting\NuGetizer.props" />
<None Update="NuGetizer.MultiTargeting.targets" PackagePath="buildMultiTargeting\NuGetizer.targets" />
<None Include="NuGetizer.PackageMetadata.targets;dotnet-nugetize.props;dotnet-nugetize.targets"
PackagePath="buildMultiTargeting\%(Filename)%(Extension)"
Pack="true" />
<None Include="NuGetizer.PackageMetadata.targets;dotnet-nugetize.props;dotnet-nugetize.targets" PackagePath="buildMultiTargeting\%(Filename)%(Extension)" Pack="true" />
<None Include="..\..\img\nugetizer-256.png" Link="icon.png" PackagePath="icon.png" />
</ItemGroup>

Expand Down
76 changes: 76 additions & 0 deletions src/NuGetizer.Tests/given_packagereferences.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,82 @@ public void when_privateassets_all_then_packs_library()
}));
}

[Fact]
public void when_privateassets_all_then_packs_transitive_libraries()
{
var result = Builder.BuildProject(@"
<Project Sdk='Microsoft.NET.Sdk'>
<PropertyGroup>
<PackageId>Library</PackageId>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include='Prism.Forms' Version='7.2.0.1422' PrivateAssets='all' />
</ItemGroup>
</Project>",
"GetPackageContents", output);

result.AssertSuccess(output);
Assert.Contains(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Prism.dll",
}));
Assert.Contains(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Prism.Forms.dll",
}));
Assert.Contains(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Core.dll",
}));
Assert.Contains(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Platform.dll",
}));
Assert.Contains(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Xaml.dll",
}));
}

[Fact]
public void when_privateassets_all_and_pack_false_then_does_not_pack_transitively()
{
var result = Builder.BuildProject(@"
<Project Sdk='Microsoft.NET.Sdk'>
<PropertyGroup>
<PackageId>Library</PackageId>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include='Prism.Forms' Version='7.2.0.1422' PrivateAssets='all' Pack='false' />
</ItemGroup>
</Project>",
"GetPackageContents", output);

result.AssertSuccess(output);
Assert.DoesNotContain(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Prism.dll",
}));
Assert.DoesNotContain(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Prism.Forms.dll",
}));
Assert.DoesNotContain(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Core.dll",
}));
Assert.DoesNotContain(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Platform.dll",
}));
Assert.DoesNotContain(result.Items, item => item.Matches(new
{
PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Xaml.dll",
}));
}

[Fact]
public void when_privateassets_pack_false_then_does_not_pack()
{
Expand Down

0 comments on commit 12b3cd7

Please sign in to comment.