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

Allow disabling the AssetTargetFallback dependency resolution via an environment variable #4703

Merged
merged 6 commits into from
Jun 28, 2022
Merged
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
Expand Up @@ -34,6 +34,7 @@ public class SourceRepositoryDependencyProvider : IRemoteDependencyProvider
private bool _ignoreFailedSources;
private bool _ignoreWarning;
private bool _isFallbackFolderSource;
private bool _useLegacyAssetTargetFallbackBehavior;

private readonly ConcurrentDictionary<LibraryRangeCacheKey, AsyncLazy<LibraryDependencyInfo>> _dependencyInfoCache
= new ConcurrentDictionary<LibraryRangeCacheKey, AsyncLazy<LibraryDependencyInfo>>();
Expand Down Expand Up @@ -86,13 +87,33 @@ public SourceRepositoryDependencyProvider(
/// is <c>null</c>.</exception>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="logger" /> is <c>null</c>.</exception>
public SourceRepositoryDependencyProvider(
SourceRepository sourceRepository,
ILogger logger,
SourceCacheContext cacheContext,
bool ignoreFailedSources,
bool ignoreWarning,
LocalPackageFileCache fileCache,
bool isFallbackFolderSource)
SourceRepository sourceRepository,
ILogger logger,
SourceCacheContext cacheContext,
bool ignoreFailedSources,
bool ignoreWarning,
LocalPackageFileCache fileCache,
bool isFallbackFolderSource) :
this(sourceRepository,
logger,
cacheContext,
ignoreFailedSources,
ignoreWarning,
fileCache,
isFallbackFolderSource,
environmentVariableReader: EnvironmentVariableWrapper.Instance)
{
}

internal SourceRepositoryDependencyProvider(
SourceRepository sourceRepository,
ILogger logger,
SourceCacheContext cacheContext,
bool ignoreFailedSources,
bool ignoreWarning,
LocalPackageFileCache fileCache,
bool isFallbackFolderSource,
IEnvironmentVariableReader environmentVariableReader)
{
_sourceRepository = sourceRepository ?? throw new ArgumentNullException(nameof(sourceRepository));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
Expand All @@ -101,6 +122,7 @@ public SourceRepositoryDependencyProvider(
_ignoreWarning = ignoreWarning;
_packageFileCache = fileCache;
_isFallbackFolderSource = isFallbackFolderSource;
_useLegacyAssetTargetFallbackBehavior = MSBuildStringUtility.IsTrue(environmentVariableReader.GetEnvironmentVariable("NUGET_USE_LEGACY_ASSET_TARGET_FALLBACK_DEPENDENCY_RESOLUTION"));
}

/// <summary>
Expand Down Expand Up @@ -470,7 +492,7 @@ public async Task<IPackageDownloader> GetPackageDownloaderAsync(
return null;
}

private static IEnumerable<LibraryDependency> GetDependencies(
private IEnumerable<LibraryDependency> GetDependencies(
FindPackageByIdDependencyInfo packageInfo,
NuGetFramework targetFramework)
{
Expand All @@ -488,13 +510,17 @@ private static IEnumerable<LibraryDependency> GetDependencies(
dependencyGroup = NuGetFrameworkUtility.GetNearest(packageInfo.DependencyGroups, dualCompatibilityFramework.SecondaryFramework, item => item.TargetFramework);
}

// FrameworkReducer.GetNearest does not consider ATF since it is used for more than just compat
if (dependencyGroup == null &&
targetFramework is AssetTargetFallbackFramework assetTargetFallbackFramework)
if (!_useLegacyAssetTargetFallbackBehavior)
{
dependencyGroup = NuGetFrameworkUtility.GetNearest(packageInfo.DependencyGroups,
assetTargetFallbackFramework.AsFallbackFramework(),
item => item.TargetFramework);
// FrameworkReducer.GetNearest does not consider ATF since it is used for more than just compat

if (dependencyGroup == null &&
targetFramework is AssetTargetFallbackFramework assetTargetFallbackFramework)
{
dependencyGroup = NuGetFrameworkUtility.GetNearest(packageInfo.DependencyGroups,
assetTargetFallbackFramework.AsFallbackFramework(),
item => item.TargetFramework);
}
}

if (dependencyGroup != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,21 @@ private readonly Dictionary<string, ExternalProjectReference> _externalProjectsB

private readonly ILogger _logger;

private readonly bool _useLegacyAssetTargetFallbackBehavior;

public PackageSpecReferenceDependencyProvider(
IEnumerable<ExternalProjectReference> externalProjects,
ILogger logger)
ILogger logger) :
this(externalProjects,
logger,
environmentVariableReader: EnvironmentVariableWrapper.Instance)
{
}

internal PackageSpecReferenceDependencyProvider(
IEnumerable<ExternalProjectReference> externalProjects,
ILogger logger,
IEnvironmentVariableReader environmentVariableReader)
{
if (externalProjects == null)
{
Expand Down Expand Up @@ -68,6 +80,7 @@ public PackageSpecReferenceDependencyProvider(
_externalProjectsByUniqueName.Add(project.UniqueName, project);
}
}
_useLegacyAssetTargetFallbackBehavior = MSBuildStringUtility.IsTrue(environmentVariableReader.GetEnvironmentVariable("NUGET_USE_LEGACY_ASSET_TARGET_FALLBACK_DEPENDENCY_RESOLUTION"));
}

public bool SupportsType(LibraryDependencyTarget libraryType)
Expand Down Expand Up @@ -155,7 +168,7 @@ public Library GetLibrary(LibraryRange libraryRange, NuGetFramework targetFramew
return library;
}

private static void AddLibraryProperties(Library library, PackageSpec packageSpec, NuGetFramework targetFramework)
private void AddLibraryProperties(Library library, PackageSpec packageSpec, NuGetFramework targetFramework)
{
var projectStyle = packageSpec.RestoreMetadata?.ProjectStyle ?? ProjectStyle.Unknown;

Expand Down Expand Up @@ -219,10 +232,13 @@ private List<LibraryDependency> GetDependenciesFromSpecRestoreMetadata(PackageSp
// Get the nearest framework
var referencesForFramework = packageSpec.GetRestoreMetadataFramework(targetFramework);

if (referencesForFramework.FrameworkName == null &&
targetFramework is AssetTargetFallbackFramework assetTargetFallbackFramework)
if (!_useLegacyAssetTargetFallbackBehavior)
{
referencesForFramework = packageSpec.GetRestoreMetadataFramework(assetTargetFallbackFramework.AsFallbackFramework());
if (referencesForFramework.FrameworkName == null &&
targetFramework is AssetTargetFallbackFramework assetTargetFallbackFramework)
{
referencesForFramework = packageSpec.GetRestoreMetadataFramework(assetTargetFallbackFramework.AsFallbackFramework());
}
}

// Ensure that this project is compatible
Expand Down Expand Up @@ -318,7 +334,7 @@ private List<LibraryDependency> GetDependenciesFromExternalReference(
return dependencies;
}

internal static List<LibraryDependency> GetSpecDependencies(
internal List<LibraryDependency> GetSpecDependencies(
PackageSpec packageSpec,
NuGetFramework targetFramework)
{
Expand All @@ -332,9 +348,12 @@ internal static List<LibraryDependency> GetSpecDependencies(
// Add framework specific dependencies
var targetFrameworkInfo = packageSpec.GetTargetFramework(targetFramework);

if (targetFrameworkInfo.FrameworkName == null && targetFramework is AssetTargetFallbackFramework atfFramework)
if (!_useLegacyAssetTargetFallbackBehavior)
{
targetFrameworkInfo = packageSpec.GetTargetFramework(atfFramework.AsFallbackFramework());
if (targetFrameworkInfo.FrameworkName == null && targetFramework is AssetTargetFallbackFramework atfFramework)
{
targetFrameworkInfo = packageSpec.GetTargetFramework(atfFramework.AsFallbackFramework());
}
}

dependencies.AddRange(targetFrameworkInfo.Dependencies);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -17,6 +18,7 @@
using NuGet.Protocol.Core.Types;
using NuGet.Test.Utility;
using NuGet.Versioning;
using Test.Utility;
using Xunit;

namespace NuGet.Commands.Test
Expand Down Expand Up @@ -772,6 +774,131 @@ public async Task FindLibraryAsync_WhenASourceIsInaccessible_AndFailuresAreIgnor
secondTestLogger.ShowWarnings().Should().Contain("NU1801");
}

[Fact]
public async Task GetDependenciesAsync_WhenPackageHasVariousDependencyGroups_CorrectFrameworkIsSelected()
{
// Arrange
var testLogger = new TestLogger();
var cacheContext = new SourceCacheContext();
var findResource = new Mock<FindPackageByIdResource>();
var net46 = FrameworkConstants.CommonFrameworks.Net46;
var netstandard20 = FrameworkConstants.CommonFrameworks.NetStandard20;

var packageDependencyGroups = new List<PackageDependencyGroup>();
var net46group = new PackageDependencyGroup(net46, new PackageDependency[] { new PackageDependency("full.framework", VersionRange.Parse("1.0.0")) });
var netstandardGroup = new PackageDependencyGroup(netstandard20, new PackageDependency[] { new PackageDependency("netstandard", VersionRange.Parse("1.0.0")) });

packageDependencyGroups.Add(net46group);
packageDependencyGroups.Add(netstandardGroup);

findResource.Setup(s => s.GetDependencyInfoAsync(
It.IsAny<string>(),
It.IsAny<NuGetVersion>(),
It.IsAny<SourceCacheContext>(),
It.IsAny<ILogger>(),
It.IsAny<CancellationToken>()))
.ReturnsAsync(new FindPackageByIdDependencyInfo(
new PackageIdentity("x", NuGetVersion.Parse("1.0.0-beta")),
packageDependencyGroups,
Enumerable.Empty<FrameworkSpecificGroup>()));

var source = new Mock<SourceRepository>();
source.Setup(s => s.GetResourceAsync<FindPackageByIdResource>())
.ReturnsAsync(findResource.Object);
source.SetupGet(s => s.PackageSource)
.Returns(new PackageSource("http://test/index.json"));

var provider = new SourceRepositoryDependencyProvider(
source.Object,
testLogger,
cacheContext,
ignoreFailedSources: true,
ignoreWarning: true,
fileCache: null,
isFallbackFolderSource: false,
new TestEnvironmentVariableReader(new Dictionary<string, string>()));

// Act
var library = await provider.GetDependenciesAsync(
new LibraryIdentity("x", NuGetVersion.Parse("1.0.0-beta"), LibraryType.Package),
FrameworkConstants.CommonFrameworks.Net472,
cacheContext,
testLogger,
CancellationToken.None);
// Assert
library.Dependencies.Should().HaveCount(1);
var dependencies = library.Dependencies.Single();
dependencies.Name.Should().Be("full.framework");
}

[Theory]
[InlineData("true", false)]
[InlineData("blbla", true)]
public async Task GetDependenciesAsync_WhenPackageIsSelectedWithAssetTargetFallback_AndLegacyDependencyResolutionVariableIsSpecified_CorrectDependenciesAreSelected(string envValue, bool areDependenciesSelected)
{
// Arrange
var testLogger = new TestLogger();
var cacheContext = new SourceCacheContext();
var findResource = new Mock<FindPackageByIdResource>();
var wrapper = new TestEnvironmentVariableReader(new Dictionary<string, string>
{
{ "NUGET_USE_LEGACY_ASSET_TARGET_FALLBACK_DEPENDENCY_RESOLUTION", envValue }
});
var net472 = FrameworkConstants.CommonFrameworks.Net472;
var net60 = FrameworkConstants.CommonFrameworks.Net60;
var inputFramework = new AssetTargetFallbackFramework(net60, new List<NuGetFramework> { net472 });

var packageDependencyGroups = new List<PackageDependencyGroup>();
var net472Group = new PackageDependencyGroup(net472, new PackageDependency[] { new PackageDependency("full.framework", VersionRange.Parse("1.0.0")) });

packageDependencyGroups.Add(net472Group);

findResource.Setup(s => s.GetDependencyInfoAsync(
It.IsAny<string>(),
It.IsAny<NuGetVersion>(),
It.IsAny<SourceCacheContext>(),
It.IsAny<ILogger>(),
It.IsAny<CancellationToken>()))
.ReturnsAsync(new FindPackageByIdDependencyInfo(
new PackageIdentity("x", NuGetVersion.Parse("1.0.0-beta")),
packageDependencyGroups,
Enumerable.Empty<FrameworkSpecificGroup>()));

var source = new Mock<SourceRepository>();
source.Setup(s => s.GetResourceAsync<FindPackageByIdResource>())
.ReturnsAsync(findResource.Object);
source.SetupGet(s => s.PackageSource)
.Returns(new PackageSource("http://test/index.json"));

var provider = new SourceRepositoryDependencyProvider(
source.Object,
testLogger,
cacheContext,
ignoreFailedSources: true,
ignoreWarning: true,
fileCache: null,
isFallbackFolderSource: false,
wrapper);

// Act
var library = await provider.GetDependenciesAsync(
new LibraryIdentity("x", NuGetVersion.Parse("1.0.0-beta"), LibraryType.Package),
inputFramework,
cacheContext,
testLogger,
CancellationToken.None);
// Assert
if (areDependenciesSelected)
{
library.Dependencies.Should().HaveCount(1);
var dependencies = library.Dependencies.Single();
dependencies.Name.Should().Be("full.framework");
} else
{
library.Dependencies.Should().HaveCount(0);
}
}

private sealed class SourceRepositoryDependencyProviderTest : IDisposable
{
internal TestLogger Logger { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@

using System.Collections.Generic;
using System.Linq;
using FluentAssertions;
using NuGet.Commands.Test;
using NuGet.Common;
using NuGet.Frameworks;
using NuGet.LibraryModel;
using NuGet.Test.Utility;
using NuGet.Versioning;
using Test.Utility;
using Xunit;

namespace NuGet.ProjectModel.Test
Expand Down Expand Up @@ -40,8 +43,9 @@ public void GetSpecDependencies_AddsCentralPackageVersionsIfDefined(bool cpvmEna
var dependencyGraphSpec = CreateDependencyGraphSpecWithCentralDependencies(cpvmEnabled, CentralPackageTransitivePinningEnabled, tfi);
var packSpec = dependencyGraphSpec.Projects[0];

var dependencyProvider = new PackageSpecReferenceDependencyProvider(new List<ExternalProjectReference>(), NullLogger.Instance);
// Act
var dependencies = PackageSpecReferenceDependencyProvider.GetSpecDependencies(packSpec, tfi.FrameworkName);
var dependencies = dependencyProvider.GetSpecDependencies(packSpec, tfi.FrameworkName);

// Assert
if (cpvmEnabled && CentralPackageTransitivePinningEnabled)
Expand Down Expand Up @@ -72,6 +76,32 @@ public void GetSpecDependencies_AddsCentralPackageVersionsIfDefined(bool cpvmEna
}
}

[Theory]
[InlineData(null, 1)]
[InlineData("true", 0)]
public void GetSpecDependencies_WithAssetTargetFallback_AndDependencyResolutionVariableSpecified_ReturnsCorrectDependencies(string assetTargetFallbackEnvironmentVariableValue, int dependencyCount)
{
// Arrange
var net60Framework = FrameworkConstants.CommonFrameworks.Net60;
var net472Framework = FrameworkConstants.CommonFrameworks.Net472;
var packageSpec = ProjectTestHelpers.GetPackageSpec(rootPath: "C:\\", projectName: "A", framework: net472Framework.GetShortFolderName(), dependencyName: "x");

var envVarWrapper = new TestEnvironmentVariableReader(new Dictionary<string, string> { { "NUGET_USE_LEGACY_ASSET_TARGET_FALLBACK_DEPENDENCY_RESOLUTION", assetTargetFallbackEnvironmentVariableValue } });
var dependencyProvider = new PackageSpecReferenceDependencyProvider(new List<ExternalProjectReference>(), NullLogger.Instance, envVarWrapper);
var assetTargetFallback = new AssetTargetFallbackFramework(net60Framework, new List<NuGetFramework> { net472Framework });
// Act

var dependencies = dependencyProvider.GetSpecDependencies(packageSpec, assetTargetFallback);

// Assert
dependencies.Should().HaveCount(dependencyCount);

if (dependencyCount > 0)
{
dependencies[0].Name.Should().Be("x");
}
}

private static TargetFrameworkInformation CreateTargetFrameworkInformation(List<LibraryDependency> dependencies, List<CentralPackageVersion> centralVersionsDependencies, bool cpvmEnabled)
{
NuGetFramework nugetFramework = new NuGetFramework("net40");
Expand Down