Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AssetTargetFallback #1436

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -295,21 +295,26 @@ private async Task<PackageSpec> GetPackageSpecAsync(ISettings settings)
.GetPackageReferencesAsync(targetFramework, CancellationToken.None))
.ToList();

var packageTargetFallback = _vsProjectAdapter.PackageTargetFallback?.Split(new[] { ';' })
var splitChars = new[] { ';' };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static readonly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mixed on this. I'm not sure that the benefit of making this static out weighs creating this every time the assembly is loaded.


var packageTargetFallback = _vsProjectAdapter.PackageTargetFallback?.Split(splitChars)
.Select(NuGetFramework.Parse)
.ToList();
.ToList()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it wasn't covered there before, what happens in case of empty or invalid entries in the list?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NuGetFramework.Parse returns the Unsupported framework since it isn't unusual for future frameworks to come in. For fallback frameworks there is a check to verify that unsupported isn't passed and it will throw.

?? new List<NuGetFramework>();

var assetTargetFallback = _vsProjectAdapter.AssetTargetFallback?.Split(splitChars)
.Select(NuGetFramework.Parse)
.ToList()
?? new List<NuGetFramework>();

var projectTfi = new TargetFrameworkInformation
{
FrameworkName = targetFramework,
Dependencies = packageReferences,
Imports = packageTargetFallback ?? new List<NuGetFramework>()
};

if ((projectTfi.Imports?.Count ?? 0) > 0)
{
projectTfi.FrameworkName = new FallbackFramework(projectTfi.FrameworkName, packageTargetFallback);
}
// Apply fallback settings
AssetTargetFallbackUtility.ApplyFramework(projectTfi, packageTargetFallback, assetTargetFallback);

// Build up runtime information.
var runtimes = await _vsProjectAdapter.GetRuntimeIdentifiersAsync();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -161,6 +161,14 @@ public string PackageTargetFallback
}
}

public string AssetTargetFallback
{
get
{
return BuildProperties.GetPropertyValue(ProjectBuildProperties.AssetTargetFallback);
}
}

public EnvDTE.Project Project => _dteProject.Value;

public string ProjectId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public sealed class VsSolutionRestoreService : IVsSolutionRestoreService, IVsSol
private const string RestorePackagesPath = nameof(RestorePackagesPath);
private const string RestoreSources = nameof(RestoreSources);
private const string RestoreFallbackFolders = nameof(RestoreFallbackFolders);
private const string AssetTargetFallback = nameof(AssetTargetFallback);


private static readonly Version Version20 = new Version(2, 0, 0, 0);
Expand Down Expand Up @@ -268,7 +269,7 @@ private static PackageSpec ToPackageSpec(ProjectNames projectNames, IVsProjectRe
.ToList(),
},
RuntimeGraph = GetRuntimeGraph(projectRestoreInfo),
RestoreSettings = new ProjectRestoreSettings() { HideWarningsAndErrors = true }
RestoreSettings = new ProjectRestoreSettings() { HideWarningsAndErrors = true }
};

return packageSpec;
Expand Down Expand Up @@ -313,7 +314,7 @@ private static string[] HandleClear(string[] input)
{
if (input.Any(e => StringComparer.OrdinalIgnoreCase.Equals(Clear, e)))
{
return new string[] { Clear};
return new string[] { Clear };
}

return input;
Expand Down Expand Up @@ -372,21 +373,16 @@ private static TargetFrameworkInformation ToTargetFrameworkInformation(
FrameworkName = NuGetFramework.Parse(targetFrameworkInfo.TargetFrameworkMoniker)
};

var ptf = GetPropertyValueOrNull(targetFrameworkInfo.Properties, PackageTargetFallback);
if (!string.IsNullOrEmpty(ptf))
{
var fallbackList = MSBuildStringUtility.Split(ptf)
.Select(NuGetFramework.Parse)
.ToList();
var ptf = MSBuildStringUtility.Split(GetPropertyValueOrNull(targetFrameworkInfo.Properties, PackageTargetFallback))
.Select(NuGetFramework.Parse)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indenting.

.ToList();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same question here about empty/invalid entries.


tfi.Imports = fallbackList;
var atf = MSBuildStringUtility.Split(GetPropertyValueOrNull(targetFrameworkInfo.Properties, AssetTargetFallback))
.Select(NuGetFramework.Parse)
.ToList();

// Update the PackageSpec framework to include fallback frameworks
if (tfi.Imports.Count != 0)
{
tfi.FrameworkName = new FallbackFramework(tfi.FrameworkName, fallbackList);
}
}
// Update TFI with fallback properties
AssetTargetFallbackUtility.ApplyFramework(tfi, ptf, atf);

if (targetFrameworkInfo.PackageReferences != null)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
Expand Down Expand Up @@ -39,6 +39,11 @@ public interface IVsProjectAdapter
/// </summary>
string PackageTargetFallback { get; }

/// <summary>
/// AssetTargetFallback project property
/// </summary>
string AssetTargetFallback { get; }

/// <summary>
/// Restore Packages Path DTE property
/// </summary>
Expand Down
9 changes: 7 additions & 2 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -635,13 +635,18 @@ Copyright (c) .NET Foundation. All rights reserved.
TaskParameter="RestoreGraphItems"
ItemName="_RestoreGraphEntry" />
</GetRestorePackageReferencesTask>


<PropertyGroup>
<_CombinedFallbacks>$(PackageTargetFallback);$(AssetTargetFallback)</_CombinedFallbacks>
</PropertyGroup>

<!-- Write out target framework information -->
<ItemGroup Condition=" '$(RestoreProjectStyle)' == 'PackageReference' AND '$(PackageTargetFallback)' != '' ">
<ItemGroup Condition=" '$(RestoreProjectStyle)' == 'PackageReference' AND '$(_CombinedFallbacks)' != '' ">
<_RestoreGraphEntry Include="$([System.Guid]::NewGuid())">
<Type>TargetFrameworkInformation</Type>
<ProjectUniqueName>$(MSBuildProjectFullPath)</ProjectUniqueName>
<PackageTargetFallback>$(PackageTargetFallback)</PackageTargetFallback>
<AssetTargetFallback>$(AssetTargetFallback)</AssetTargetFallback>
<TargetFramework>$(TargetFramework)</TargetFramework>
</_RestoreGraphEntry>
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,28 @@ private bool IsCompatible(CompatibilityData compatibilityData)
{
// A package is compatible if it has...
return
compatibilityData.TargetLibrary.RuntimeAssemblies.Count > 0 || // Runtime Assemblies, or
compatibilityData.TargetLibrary.CompileTimeAssemblies.Count > 0 || // Compile-time Assemblies, or
compatibilityData.TargetLibrary.FrameworkAssemblies.Count > 0 || // Framework Assemblies, or
compatibilityData.TargetLibrary.ContentFiles.Count > 0 || // Shared content
compatibilityData.TargetLibrary.ResourceAssemblies.Count > 0 || // Resources (satellite package)
compatibilityData.TargetLibrary.Build.Count > 0 || // Build
compatibilityData.TargetLibrary.BuildMultiTargeting.Count > 0 || // Cross targeting build
HasCompatibleAssets(compatibilityData.TargetLibrary) ||
!compatibilityData.Files.Any(p =>
p.StartsWith("ref/", StringComparison.OrdinalIgnoreCase)
|| p.StartsWith("lib/", StringComparison.OrdinalIgnoreCase)); // No assemblies at all (for any TxM)
}

/// <summary>
/// Check if the library contains assets.
/// </summary>
internal static bool HasCompatibleAssets(LockFileTargetLibrary targetLibrary)
{
// A package is compatible if it has...
return
targetLibrary.RuntimeAssemblies.Count > 0 || // Runtime Assemblies, or
targetLibrary.CompileTimeAssemblies.Count > 0 || // Compile-time Assemblies, or
targetLibrary.FrameworkAssemblies.Count > 0 || // Framework Assemblies, or
targetLibrary.ContentFiles.Count > 0 || // Shared content
targetLibrary.ResourceAssemblies.Count > 0 || // Resources (satellite package)
targetLibrary.Build.Count > 0 || // Build
targetLibrary.BuildMultiTargeting.Count > 0; // Cross targeting build
}

private CompatibilityData GetCompatibilityData(RestoreTargetGraph graph, LibraryIdentity libraryId)
{
LockFileTargetLibrary targetLibrary = null;
Expand Down
22 changes: 17 additions & 5 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/LockFileBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ public LockFile CreateLockFile(LockFile previousLockFile,

var libraries = lockFile.Libraries.ToDictionary(lib => Tuple.Create(lib.Name, lib.Version));

var warnForImports = project.TargetFrameworks.Any(framework => framework.Warn);
var librariesWithWarnings = new HashSet<LibraryIdentity>();

var rootProjectStyle = project.RestoreMetadata?.ProjectStyle ?? ProjectStyle.Unknown;
Expand All @@ -175,8 +174,12 @@ public LockFile CreateLockFile(LockFile previousLockFile,

var flattenedFlags = IncludeFlagUtils.FlattenDependencyTypes(_includeFlagGraphs, project, targetGraph);

var fallbackFramework = target.TargetFramework as FallbackFramework;
var warnForImportsOnGraph = warnForImports && fallbackFramework != null;
// Check if warnings should be displayed for the current framework.
var tfi = project.GetTargetFramework(targetGraph.Framework);

var warnForImportsOnGraph = tfi.Warn
&& (target.TargetFramework is FallbackFramework
|| target.TargetFramework is AssetTargetFallbackFramework);

foreach (var graphItem in targetGraph.Flattened.OrderBy(x => x.Key))
{
Expand Down Expand Up @@ -231,7 +234,7 @@ public LockFile CreateLockFile(LockFile previousLockFile,
// Log warnings if the target library used the fallback framework
if (warnForImportsOnGraph && !librariesWithWarnings.Contains(library))
{
var nonFallbackFramework = new NuGetFramework(fallbackFramework);
var nonFallbackFramework = new NuGetFramework(target.TargetFramework);

var targetLibraryWithoutFallback = LockFileUtils.CreateLockFileTargetLibrary(
libraries[Tuple.Create(library.Name, library.Version)],
Expand All @@ -248,7 +251,7 @@ public LockFile CreateLockFile(LockFile previousLockFile,
var message = string.Format(CultureInfo.CurrentCulture,
Strings.Log_ImportsFallbackWarning,
libraryName,
string.Join(", ", fallbackFramework.Fallback),
GetFallbackFrameworkString(target.TargetFramework),
nonFallbackFramework);

var logMessage = RestoreLogMessage.CreateWarning(
Expand Down Expand Up @@ -277,6 +280,15 @@ public LockFile CreateLockFile(LockFile previousLockFile,
return lockFile;
}

private static string GetFallbackFrameworkString(NuGetFramework framework)
{
var frameworks = (framework as AssetTargetFallbackFramework)?.Fallback
?? (framework as FallbackFramework)?.Fallback
?? new List<NuGetFramework>();

return string.Join(", ", frameworks);
}

private static void AddProjectFileDependenciesForSpec(PackageSpec project, LockFile lockFile)
{
// Use empty string as the key of dependencies shared by all frameworks
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using NuGet.Common;

namespace NuGet.Commands
{
/// <summary>
/// Holds an <see cref="IRestoreLogMessage"/> and returns the message for the exception.
/// </summary>
public class RestoreCommandException : Exception, ILogMessageException
{
private readonly IRestoreLogMessage _logMessage;

public RestoreCommandException(IRestoreLogMessage logMessage)
: base(logMessage?.Message)
{
_logMessage = logMessage ?? throw new ArgumentNullException(nameof(logMessage));
}

public ILogMessage AsLogMessage()
{
return _logMessage;
}
}
}
Loading