Skip to content

Commit

Permalink
Make pack inference more flexible and avoid repeating code
Browse files Browse the repository at this point in the history 
Adding support for EmbeddedResource packing (as supported by SDK pack) would have involved more copy/paste of exisitng MSBuild code, and that was becoming a red herring for maintainability.

Taking inspiration from the awesome flexibility of the roslyn editor config generation from MSBuild (see https://github.com/dotnet/roslyn/blob/master/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets#L122-L154), we can now make it not only straightforward to add new item types for pack inference, but do so in a simple and maintainable way.

The remaining built-in content type items will be added in separate commits to showcase how that will be done moving forward, based on the items at https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#including-content-in-a-package.
  • Loading branch information
@kzu
kzu committed Oct 1, 2020
1 parent 27af2f4 commit 9ebaa40
Show file tree
Hide file tree
Showing 6 changed files with 225 additions and 90 deletions.
2 changes: 1 addition & 1 deletion src/NuGetizer.Tasks/AssignPackagePath.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ ITaskItem EnsurePackagePath(ITaskItem file, IDictionary<string, ITaskItem> kindM

// If the kind is known but it isn't mapped to a folder inside the package, we're done.
// Special-case None kind since that means 'leave it wherever it lands' ;)
if (string.IsNullOrEmpty(packageFolder) && kind != PackageItemKind.None)
if (string.IsNullOrEmpty(packageFolder) && !kind.Equals(PackageItemKind.None, StringComparison.OrdinalIgnoreCase))
return output;

// Special case for contentFiles, since they can also provide a codeLanguage metadata
Expand Down
153 changes: 79 additions & 74 deletions src/NuGetizer.Tasks/NuGetizer.Inference.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<PackContent Condition="'$(PackContent)' == ''">true</PackContent>
<!-- Whether to include @(None) items with CopyToOutputDirectory != '' in the package -->
<PackNone Condition="'$(PackNone)' == ''">false</PackNone>
<!-- Whether to include @(None) items with CopyToOutputDirectory != '' in the package -->
<PackEmbeddedResource Condition="'$(PackEmbeddedResource)' == ''">false</PackEmbeddedResource>
<!-- Whether to include @(BuiltProjectOutputGroupOutput), @(DocumentationProjectOutputGroupOutput) and @(SatelliteDllsProjectOutputGroupOutput) items in the package -->
<PackBuildOutput Condition="'$(PackBuildOutput)' == '' and '$(IsPackagingProject)' != 'true'">true</PackBuildOutput>
<!-- Whether to include @(DebugSymbolsProjectOutputGroupOutput) items in the package -->
Expand All @@ -35,15 +37,34 @@ Copyright (c) .NET Foundation. All rights reserved.
<_OutputFullPath Condition="'$(_OutputFullPath)' == ''">$(MSBuildProjectDirectory.TrimEnd('\'))\$(OutputPath)</_OutputFullPath>
</PropertyGroup>

<PropertyGroup>
<GetPackageContentsDependsOn>
$(GetPackageContentsDependsOn);
_BuildOutputFrameworkSpecific;
_SetDefaultPackageReferencePack;
InferPackageContents
</GetPackageContentsDependsOn>
</PropertyGroup>

<ItemDefinitionGroup Label="Inference Defaults">
<Compile>
<!-- For Compile, the CodeLanguage should default to the project's default source extension -->
<CodeLanguage>$(DefaultLanguageSourceExtension.TrimStart('.'))</CodeLanguage>
<Pack Condition="'$(PackCompile)' == 'true'">true</Pack>
<DefaultKind>content</DefaultKind>
</Compile>
<Content>
<Pack Condition="'$(PackContent)' == true">true</Pack>
<DefaultKind>content</DefaultKind>
</Content>
<EmbeddedResource>
<Pack Condition="'$(PackEmbeddedResource)' == true">true</Pack>
<DefaultKind>content</DefaultKind>
</EmbeddedResource>
<None>
<Pack Condition="'$(PackNone)' == true">true</Pack>
<DefaultKind>none</DefaultKind>
</None>
<InferenceCandidate>
<DefaultKind />
<Pack />
<PackagePath />
<Kind />
<ShouldPack />
</InferenceCandidate>
</ItemDefinitionGroup>

<!-- Extend some built-in items with metadata we use in our inference targets -->
<ItemDefinitionGroup>
<PackageReference>
Expand All @@ -61,12 +82,18 @@ Copyright (c) .NET Foundation. All rights reserved.
<NuGetPackageId />
<Pack />
</_ReferenceRelatedPaths>
<Compile>
<CodeLanguage />
</Compile>
</ItemDefinitionGroup>

<Target Name="_BuildOutputFrameworkSpecific" Condition="'$(BuildOutputFrameworkSpecific)' == ''" Returns="$(BuildOutputFrameworkSpecific)">
<PropertyGroup>
<GetPackageContentsDependsOn>
$(GetPackageContentsDependsOn);
_SetBuildOutputFrameworkSpecific;
_SetDefaultPackageReferencePack;
InferPackageContents
</GetPackageContentsDependsOn>
</PropertyGroup>

<Target Name="_SetBuildOutputFrameworkSpecific" Condition="'$(BuildOutputFrameworkSpecific)' == ''" Returns="$(BuildOutputFrameworkSpecific)">
<!-- Determine whether primary output is framework specific -->
<ItemGroup>
<_BuildOutputKindFrameworkSpecific Include="@(PackageItemKind->'%(FrameworkSpecific)')" Condition="'%(Identity)' == '$(BuildOutputKind)'" />
Expand All @@ -85,91 +112,69 @@ Copyright (c) .NET Foundation. All rights reserved.
</ItemGroup>
</Target>

<ItemGroup>
<PackInference Include="Compile;Content;EmbeddedResource;None" />
</ItemGroup>

<Target Name="InferPackageContents" DependsOnTargets="$(InferPackageContentsDependsOn)" Returns="@(PackageFile)">
<!-- For Compile, the CodeLanguage should default to the project's default source extension -->
<ItemGroup Condition="'$(PackCompile)' == 'true'">
<Compile Update="@(Compile)" Condition="'%(CodeLanguage)' == ''">
<CodeLanguage>$(DefaultLanguageSourceExtension.TrimStart('.'))</CodeLanguage>
</Compile>
<ItemGroup>
<InferenceCandidate Include="@(%(PackInference.Identity))" />
<InferenceCandidate>
<ShouldPack Condition="('%(Pack)' == 'true' or '%(PackagePath)' != '' or '%(Kind)' != '') and '%(Pack)' != 'false'">true</ShouldPack>
</InferenceCandidate>
</ItemGroup>

<!-- Ensure TargetPath -->
<AssignTargetPath Files="@(Compile)" RootFolder="$(MSBuildProjectDirectory)"
Condition="'%(Compile.Pack)' == 'true' or
'%(Compile.PackagePath)' != '' or
'%(Compile.Kind)' != '' or
('$(PackCompile)' == 'true' and '%(Compile.Pack)' != 'false')">
<!-- NOTE: we infer Compile with PackCompile=true just like we infer Content.
Main difference is that PackCompile is false by default. Opting in triggers
same rules as content: if CopyToOutputDirectory is present, the file becomes
part of the build output kind, otherwise, it becomes contentFiles.
-->
<Output TaskParameter="AssignedFiles" ItemName="_ContentToInfer" />
</AssignTargetPath>

<AssignTargetPath Files="@(Content)" RootFolder="$(MSBuildProjectDirectory)"
Condition="'%(Content.Pack)' == 'true' or
'%(Content.PackagePath)' != '' or
'%(Content.Kind)' != '' or
('$(PackContent)' == 'true' and '%(Content.Pack)' != 'false')">
<Output TaskParameter="AssignedFiles" ItemName="_ContentToInfer" />
<AssignTargetPath Files="@(InferenceCandidate)" RootFolder="$(MSBuildProjectDirectory)"
Condition="'%(ShouldPack)' == 'true'">
<Output TaskParameter="AssignedFiles" ItemName="_InferenceCandidateWithTargetPath" />
</AssignTargetPath>

<AssignTargetPath Files="@(None)" RootFolder="$(MSBuildProjectDirectory)"
Condition="'%(None.Pack)' == 'true' or
'%(None.PackagePath)' != '' or
'%(None.Kind)' != '' or
('$(PackNone)' == 'true' and '%(None.Pack)' != 'false')">
<Output TaskParameter="AssignedFiles" ItemName="_NoneToInfer" />
</AssignTargetPath>
<ItemGroup Label="PackInference">
<_InferenceCandidateWithTargetPath Condition="'%(Kind)' == ''">
<!-- Items that are copied to the output directory adopt the kind of the build output -->
<Kind Condition="'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' != '' or
'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' != 'Never'">$(BuildOutputKind)</Kind>
<!-- Otherwise they cake on whichever is the default for their item type, as defined by their PackInference item -->
<Kind Condition="'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' == '' or
'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' == 'Never'">%(_InferenceCandidateWithTargetPath.DefaultKind)</Kind>
</_InferenceCandidateWithTargetPath>

<!-- Items that are copied to the output directory are included from the target path -->
<_InferredPackageFile Include="@(_InferenceCandidateWithTargetPath -> '$(_OutputFullPath)\%(TargetPath)')"
Condition="'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' != '' and
'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' != 'Never'" />
<!-- Otherwise, they are included from the source location -->
<_InferredPackageFile Include="@(_InferenceCandidateWithTargetPath->'%(FullPath)')"
Condition="'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' == '' or
'%(_InferenceCandidateWithTargetPath.CopyToOutputDirectory)' == 'Never'" />
</ItemGroup>

<ItemGroup>
<ItemGroup Label="BuildOutput Inference" Condition="'$(PackBuildOutput)' == 'true'">
<!-- Unfortunately, even with https://github.com/Microsoft/msbuild/pull/1115, when multi-targeting
.NETFramework, the desktop WinFX.targets are imported which don't have the fix, so we need to
do it "the old way" for this particular output group -->
<_SatelliteDllsProjectOutputGroupOutput Include="@(SatelliteDllsProjectOutputGroupOutput)" FinalOutputPath="'%(FullPath)')" />
<_SatelliteDllsProjectOutputGroupOutput Include="@(SatelliteDllsProjectOutputGroupOutput)"
FinalOutputPath="'%(FullPath)')" />

<_InferredProjectOutput Include="@(BuiltProjectOutputGroupOutput -> '%(FinalOutputPath)');
@(BuiltProjectOutputGroupKeyOutput -> '%(FinalOutputPath)');
@(DocumentationProjectOutputGroupOutput -> '%(FinalOutputPath)');
@(_SatelliteDllsProjectOutputGroupOutput -> '%(FinalOutputPath)')"
Condition="'$(PackBuildOutput)' == 'true'">
@(_SatelliteDllsProjectOutputGroupOutput -> '%(FinalOutputPath)')">
<Kind>$(BuildOutputKind)</Kind>
<FrameworkSpecific>$(BuildOutputFrameworkSpecific)</FrameworkSpecific>
</_InferredProjectOutput>

<_InferredProjectOutput Include="@(DebugSymbolsProjectOutputGroupOutput -> '%(FinalOutputPath)')"
Condition="'$(PackBuildOutput)' == 'true' and '$(PackSymbols)' != 'false'">
Condition="'$(PackSymbols)' != 'false'">
<Kind>$(BuildOutputKind)</Kind>
<FrameworkSpecific>$(BuildOutputFrameworkSpecific)</FrameworkSpecific>
</_InferredProjectOutput>

<_InferredPackageFile Include="@(_InferredProjectOutput -> Distinct())" />
</ItemGroup>

<!-- NOTE: Content is opt-out (must have Pack=false to exclude if PackContent=true) -->
<!-- Stuff that is copied to output should be included from that output location -->
<_InferredPackageFile Include="@(_ContentToInfer->'$(_OutputFullPath)\%(TargetPath)')"
Condition="'%(_ContentToInfer.CopyToOutputDirectory)' != '' and '%(_ContentToInfer.CopyToOutputDirectory)' != 'Never'">
<Kind Condition="'%(_ContentToInfer.Kind)' == ''">$(BuildOutputKind)</Kind>
</_InferredPackageFile>
<!-- Otherwise, include from source location and default to content -->
<_InferredPackageFile Include="@(_ContentToInfer->'%(FullPath)')"
Condition="'%(_ContentToInfer.CopyToOutputDirectory)' == '' or '%(_ContentToInfer.CopyToOutputDirectory)' == 'Never'">
<Kind Condition="'%(_ContentToInfer.Kind)' == ''">Content</Kind>
</_InferredPackageFile>

<!-- NOTE: None is also opt-out (must have Pack=false to exclude if PackNone=true, but this property defaults to false) -->
<!-- Likewise, include from target path if it's copied, from source path otherwise -->
<_InferredPackageFile Include="@(_NoneToInfer->'$(_OutputFullPath)\%(TargetPath)')"
Condition="'%(_NoneToInfer.CopyToOutputDirectory)' != '' and '%(_NoneToInfer.CopyToOutputDirectory)' != 'Never'">
<Kind Condition="'%(_NoneToInfer.Kind)' == ''">$(BuildOutputKind)</Kind>
</_InferredPackageFile>
<_InferredPackageFile Include="@(_NoneToInfer->'%(FullPath)')"
Condition="'%(_NoneToInfer.CopyToOutputDirectory)' == '' or '%(_NoneToInfer.CopyToOutputDirectory)' == 'Never'">
<Kind Condition="'%(_NoneToInfer.Kind)' == ''">None</Kind>
</_InferredPackageFile>

<_InferredPackageFile Include="@(PackageReference)"
<ItemGroup Label="References Inference">
<_InferredPackageFile Include="@(PackageReference)"
Condition="'%(PackageReference.Identity)' != 'NuGetizer' and
'%(PackageReference.Identity)' != 'NETStandard.Library' and
'%(PackageReference.PrivateAssets)' != 'all' and
Expand Down
24 changes: 15 additions & 9 deletions src/NuGetizer.Tests/Builder.NuGetizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,9 @@ public static TargetResult BuildProject(
File.WriteAllText(Path.Combine(scenarioDir, file.name), file.contents);
}

var openLog = OpenBuildLogAttribute.IsActive;
try
{
OpenBuildLogAttribute.IsActive = false;
using (var disable = OpenBuildLogAttribute.Disable())
BuildScenario(scenarioName, target: "Restore", output: output, verbosity: verbosity)
.AssertSuccess(output);
}
finally
{
OpenBuildLogAttribute.IsActive = openLog;
}

return BuildScenario(scenarioName, target: target, output: output, verbosity: verbosity);
}
Expand Down Expand Up @@ -231,6 +223,7 @@ public static bool IsActive
}
}
}
public static IDisposable Disable() => new DisposableOpenBinlog();

public override void Before(MethodInfo methodUnderTest)
{
Expand All @@ -243,6 +236,19 @@ public override void After(MethodInfo methodUnderTest)
IsActive = false;
base.After(methodUnderTest);
}

class DisposableOpenBinlog : IDisposable
{
bool isActive;

public DisposableOpenBinlog()
{
isActive = IsActive;
IsActive = false;
}

public void Dispose() => IsActive = isActive;
}
}

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion src/NuGetizer.Tests/given_a_library_with_content.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class given_a_library_with_content
public given_a_library_with_content(ITestOutputHelper output)
{
this.output = output;
using var disable = OpenBuildLogAttribute.Disable();
Builder.BuildScenario(nameof(given_a_library_with_content), target: "Restore")
.AssertSuccess(output);
}
Expand Down Expand Up @@ -137,7 +138,7 @@ public void content_no_copy_is_content_files_anylang_tfm_specific()
}));
}

[Fact]
[Fact]
public void content_no_copy_is_included_from_source()
{
var result = Builder.BuildScenario(nameof(given_a_library_with_content), new
Expand Down
5 changes: 3 additions & 2 deletions src/NuGetizer.Tests/given_a_library_with_private_assets_reference.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ public class given_a_library_with_private_assets_reference
[Fact]
public void when_getting_package_contents_then_contains_private_assets_as_primary_output()
{
Builder.BuildScenario(nameof(given_a_library_with_private_assets_reference), target: "Restore", output: output)
.AssertSuccess(output);
using (var disable = OpenBuildLogAttribute.Disable())
Builder.BuildScenario(nameof(given_a_library_with_private_assets_reference), target: "Restore", output: output)
.AssertSuccess(output);

var result = Builder.BuildScenario(nameof(given_a_library_with_private_assets_reference), output: output);

Expand Down
Loading

0 comments on commit 9ebaa40

Please sign in to comment.