Skip to content

Commit

Permalink
Improve the handling of platform-suffixed target frameworks
Browse files Browse the repository at this point in the history 
We previously (ab)used the TargetFrameworkMoniker of a project as a default/fallback target framework to use whenever the TargetFramework metadata was not present for a particular PackageFile.

This broke down in .NET5+ when platform-suffixed TFs such as "net7.0-windows" now need the target platform version, in addition to the TF< which, furthermore, can no longer be recreated as a nuget framework from the TFM alone, since it also contains the TargetPlatformIdentifier.

So rather than complicating the previous single metadata value with *three* (TFM, TPM and a boolean on whether the original TF had a platform suffix or not), we instead normalize this up-front in all projects via a new target, and set this as PackTargetFramework. We then proceed to use this in a new more intention-revealing metadata item, DefaultTargetFramework, which is now much more obvious than "leveraging" the TFM name.
  • Loading branch information
@kzu
kzu committed Feb 19, 2023
1 parent eca0924 commit d747b62
Show file tree
Hide file tree
Showing 12 changed files with 148 additions and 129 deletions.
6 changes: 3 additions & 3 deletions src/NuGetizer.Tasks/AssignPackagePath.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ ITaskItem EnsurePackagePath(ITaskItem file, IDictionary<string, ITaskItem> kindM

output.SetMetadata(MetadataName.PackageFolder, packageFolder.Replace('\\', '/'));

// NOTE: a declared TargetFramework metadata trumps TargetFrameworkMoniker,
// NOTE: a declared TargetFramework metadata trumps the DefaultTargetFramework
// which is defaulted to that of the project being built.
var targetFramework = output.GetMetadata(MetadataName.TargetFramework);
if (string.IsNullOrEmpty(targetFramework) && frameworkSpecific)
{
var frameworkMoniker = file.GetTargetFrameworkMoniker();
targetFramework = frameworkMoniker.GetShortFrameworkName() ?? "";
var nugetFramework = file.GetNuGetTargetFramework(frameworkSpecific);
targetFramework = nugetFramework?.GetShortFolderName() ?? "";
// At this point we have the correct target framework
output.SetMetadata(MetadataName.TargetFramework, targetFramework);
}
Expand Down
59 changes: 35 additions & 24 deletions src/NuGetizer.Tasks/CreatePackage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,36 +244,43 @@ where PackFolderKind.Dependency.Equals(item.GetMetadata(MetadataName.PackFolder)
{
Id = item.ItemSpec,
Version = VersionRange.Parse(item.GetMetadata(MetadataName.Version)),
TargetFramework = item.GetNuGetTargetFramework(),
// PackFolderKind=Dependency has FrameworkSpecific=true, so pass that along
TargetFramework = item.GetNuGetTargetFramework(true) ?? NuGetFramework.AnyFramework,
Include = item.GetNullableMetadata(MetadataName.PackInclude) ?? item.GetNullableMetadata(MetadataName.IncludeAssets),
Exclude = item.GetNullableMetadata(MetadataName.PackExclude) ?? item.GetNullableMetadata(MetadataName.ExcludeAssets)
};

var definedDependencyGroups = (from dependency in dependencies
group dependency by dependency.TargetFramework into dependenciesByFramework
select new PackageDependencyGroup
(
dependenciesByFramework.Key,
(from dependency in dependenciesByFramework
where dependency.Id != "_._"
group dependency by dependency.Id into dependenciesById
select new PackageDependency
(
dependenciesById.Key,
dependenciesById.Select(x => x.Version).Aggregate(AggregateVersions),
dependenciesById.Select(x => x.Include).Aggregate(default(List<string>), AggregateAssetsFlow),
dependenciesById.Select(x => x.Exclude).Aggregate(default(List<string>), AggregateAssetsFlow)
)).ToList()
)).ToDictionary(p => p.TargetFramework.GetFrameworkString());
let targetFramework = dependenciesByFramework.Key
let packages = (from dependency in dependenciesByFramework
where dependency.Id != "_._"
group dependency by dependency.Id into dependenciesById
select new PackageDependency
(
dependenciesById.Key,
dependenciesById.Select(x => x.Version).Aggregate(AggregateVersions),
dependenciesById.Select(x => x.Include).Aggregate(default(List<string>), AggregateAssetsFlow),
dependenciesById.Select(x => x.Exclude).Aggregate(default(List<string>), AggregateAssetsFlow)
)).ToList()
select new PackageDependencyGroup(targetFramework, packages)
//).ToList();
).ToDictionary(p => p.TargetFramework.GetFrameworkString());

var libframeworks = (from item in Contents
where PackFolderKind.Lib.Equals(item.GetMetadata(MetadataName.PackFolder), StringComparison.OrdinalIgnoreCase) &&
!"all".Equals(item.GetMetadata(MetadataName.PrivateAssets), StringComparison.OrdinalIgnoreCase)
// PackFolderKind=Lib has FrameworkSpecific=true, so pass that along
select item.GetNuGetTargetFramework(true) ?? NuGetFramework.AnyFramework
).ToList();

// include frameworks referenced by libraries, but without dependencies..
foreach (var targetFramework in (from item in Contents
where PackFolderKind.Lib.Equals(item.GetMetadata(MetadataName.PackFolder), StringComparison.OrdinalIgnoreCase) &&
!"all".Equals(item.GetMetadata(MetadataName.PrivateAssets), StringComparison.OrdinalIgnoreCase)
select item.GetNuGetTargetFramework()))
if (!definedDependencyGroups.ContainsKey(targetFramework.GetFrameworkString()))
definedDependencyGroups.Add(targetFramework.GetFrameworkString(),
new PackageDependencyGroup(targetFramework, Array.Empty<PackageDependency>()));
foreach (var targetFramework in libframeworks
.Where(f => !definedDependencyGroups.ContainsKey(f.GetFrameworkString())))
{
definedDependencyGroups.Add(targetFramework.GetFrameworkString(),
new PackageDependencyGroup(targetFramework, Array.Empty<PackageDependency>()));
}

manifest.Metadata.DependencyGroups = definedDependencyGroups.Values;
}
Expand Down Expand Up @@ -381,8 +388,12 @@ where item.GetMetadata(MetadataName.PackFolder) == PackFolderKind.FrameworkRefer
select new FrameworkAssemblyReference
(
item.ItemSpec,
new[] { NuGetFramework.Parse(item.GetTargetFrameworkMoniker().FullName) }
)).Distinct(FrameworkAssemblyReferenceComparer.Default);
// PackFolderKind=FrameworkReference has FrameworkSpecific=true, so pass that along
new[] { item.GetNuGetTargetFramework(true) ?? NuGetFramework.AnyFramework }
))
.ToList()
.Distinct(FrameworkAssemblyReferenceComparer.Default)
.ToList();

manifest.Metadata.FrameworkReferences = frameworkReferences;
}
Expand Down
45 changes: 6 additions & 39 deletions src/NuGetizer.Tasks/Extensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,55 +85,22 @@ public static Manifest GetManifest(this IPackageCoreReader packageReader)
}
}

public static NuGetFramework GetNuGetTargetFramework(this ITaskItem taskItem)
public static NuGetFramework? GetNuGetTargetFramework(this ITaskItem taskItem, bool? frameworkSpecific = default)
{
if (bool.TryParse(taskItem.GetMetadata(MetadataName.FrameworkSpecific), out var frameworkSpecific) &&
!frameworkSpecific)
return NuGetFramework.AnyFramework;
if (bool.TryParse(taskItem.GetMetadata(MetadataName.FrameworkSpecific), out var fws))
frameworkSpecific = fws;

var metadataValue = taskItem.GetMetadata(MetadataName.TargetFramework);
if (string.IsNullOrEmpty(metadataValue))
metadataValue = taskItem.GetMetadata(MetadataName.TargetFrameworkMoniker);

if (!string.IsNullOrEmpty(metadataValue))
return NuGetFramework.Parse(metadataValue);
else
if (frameworkSpecific != true)
return NuGetFramework.AnyFramework;
}

public static FrameworkName GetTargetFramework(this ITaskItem taskItem)
{
var metadataValue = taskItem.GetMetadata(MetadataName.TargetFramework);
if (string.IsNullOrEmpty(metadataValue))
metadataValue = taskItem.GetMetadata(MetadataName.TargetFrameworkMoniker);
metadataValue = taskItem.GetMetadata(MetadataName.DefaultTargetFramework);

if (!string.IsNullOrEmpty(metadataValue))
return new FrameworkName(NuGetFramework.Parse(metadataValue).DotNetFrameworkName);
return NuGetFramework.Parse(metadataValue);
else
return NullFramework;
}

public static FrameworkName GetTargetFrameworkMoniker(this ITaskItem item)
{
var value = item.GetMetadata(MetadataName.TargetFrameworkMoniker);
// \o/: Turn .NETPortable,Version=v5.0 into .NETPlatform,Version=v5.0, hardcoded for now?
// TODO: should be able to get .NETStandard,Version=v1.x from the item metadata somehow.

return string.IsNullOrEmpty(value) ?
NullFramework :
new FrameworkName(value);
}

public static string GetShortFrameworkName(this FrameworkName frameworkName)
{
if (frameworkName == null || frameworkName == NullFramework)
return null;

// In this case, NuGet returns portable50, is that correct?
//if (frameworkName.Identifier == ".NETPortable" && frameworkName.Version.Major == 5 && frameworkName.Version.Minor == 0)
// return "dotnet";

return NuGetFramework.Parse(frameworkName.FullName).GetShortFolderName();
}

public static void LogErrorCode(this TaskLoggingHelper log, string code, string message, params object[] messageArgs) =>
Expand Down
8 changes: 7 additions & 1 deletion src/NuGetizer.Tasks/MetadataName.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,15 @@ public static class MetadataName
/// </summary>
public const string FrameworkSpecific = nameof(FrameworkSpecific);

/// <summary>
/// The target framework of an item.
/// </summary>
public const string TargetFramework = nameof(TargetFramework);

public const string TargetFrameworkMoniker = nameof(TargetFrameworkMoniker);
/// <summary>
/// The original (and therefore default) target framework of the project that declared an item.
/// </summary>
public const string DefaultTargetFramework = nameof(DefaultTargetFramework);

/// <summary>
/// Available optional metadata values of contentFiles.
Expand Down
7 changes: 4 additions & 3 deletions src/NuGetizer.Tasks/NuGetizer.Inference.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<GetPackageContentsDependsOn>
$(GetPackageContentsDependsOn);
_SetDefaultPackageReferencePack;
_SetPackTargetFramework;
InferPackageContents
</GetPackageContentsDependsOn>
</PropertyGroup>
Expand Down Expand Up @@ -303,14 +304,12 @@ Copyright (c) .NET Foundation. All rights reserved.
@(_SatelliteDllsProjectOutputGroupOutput -> '%(FinalOutputPath)')">
<PackFolder>$(PackFolder)</PackFolder>
<FrameworkSpecific>$(BuildOutputFrameworkSpecific)</FrameworkSpecific>
<TargetFramework Condition="'$(IsPackagingProject)' != 'true' or '$(BuildOutputFrameworkSpecific)' == 'true'">$(TargetFramework)</TargetFramework>
</_InferredProjectOutput>

<_InferredProjectOutput Include="@(DebugSymbolsProjectOutputGroupOutput -> '%(FinalOutputPath)')"
Condition="'$(PackSymbols)' != 'false'">
<PackFolder>$(PackFolder)</PackFolder>
<FrameworkSpecific>$(BuildOutputFrameworkSpecific)</FrameworkSpecific>
<TargetFramework Condition="'$(IsPackagingProject)' != 'true' or '$(BuildOutputFrameworkSpecific)' == 'true'">$(TargetFramework)</TargetFramework>
</_InferredProjectOutput>

<_InferredPackageFile Include="@(_InferredProjectOutput -> Distinct())" />
Expand All @@ -324,6 +323,7 @@ Copyright (c) .NET Foundation. All rights reserved.
'%(PackageReference.PrivateAssets)' != 'all' and
'%(PackageReference.Pack)' != 'false'">
<PackFolder>Dependency</PackFolder>
<!--<FrameworkSpecific Condition="'$(BuildOutputFrameworkSpecific)' == 'true'">true</FrameworkSpecific>-->
</_InferredPackageFile>

<!-- We can't use %(FrameworkFile)==true because it's not defined for raw file references and
Expand All @@ -346,7 +346,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<Source>Implicit</Source>
<PackageId Condition="'$(IsPackable)' == 'true'">$(PackageId)</PackageId>
<Platform>$(Platform)</Platform>
<TargetFrameworkMoniker Condition="'$(IsPackagingProject)' != 'true' or '$(BuildOutputFrameworkSpecific)' == 'true'">$(TargetFrameworkMoniker)</TargetFrameworkMoniker>
<OriginalTargetFramework>$(PackTargetFramework)</OriginalTargetFramework>
<DefaultTargetFramework Condition="'$(IsPackagingProject)' != 'true' or '$(BuildOutputFrameworkSpecific)' == 'true'">$(PackTargetFramework)</DefaultTargetFramework>
</PackageFile>
</ItemGroup>
</Target>
Expand Down
6 changes: 4 additions & 2 deletions src/NuGetizer.Tasks/NuGetizer.MultiTargeting.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,16 @@ Copyright (c) .NET Foundation. All rights reserved.
<Output ItemName="_PackageContent" TaskParameter="TargetOutputs" />
</MSBuild>

<!-- _AddPackageManifest adds per-TFM, so deduplicate and remove the TFM -->
<!-- _AddPackageManifest adds per-TFM, so deduplicate and remove the TF(M) -->
<ItemGroup Condition="'$(IsPackable)' == 'true'">
<_PackageMetadataContent Include="@(_PackageContent -> WithMetadataValue('PackFolder', 'Metadata'))" />
<_PackageContent Remove="@(_PackageMetadataContent)" />
<_PackageContent Include="@(_PackageMetadataContent -> Distinct())">
<AdditionalProperties />
<Platform>$(Platform)</Platform>
<TargetFrameworkMoniker />
<TargetFramework />
<DefaultTargetFramework />
<OriginalTargetFramework />
</_PackageContent>
</ItemGroup>

Expand Down
5 changes: 4 additions & 1 deletion src/NuGetizer.Tasks/NuGetizer.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- Don't show package files in project explorer by default -->
<Visible>false</Visible>
<TargetFramework />
<TargetFrameworkMoniker />
<!-- The (pack) target framework of the originating project -->
<OriginalTargetFramework />
<!-- The determined default (pack) target framework of an inferred item -->
<DefaultTargetFramework />
</PackageFile>
<PackageReference>
<!-- See https://github.com/NuGet/Home/wiki/PackageReference-Specification -->
Expand Down
Loading

0 comments on commit d747b62

Please sign in to comment.