Skip to content

Add reference metadata to indicate that RAR can skip dependency searching #1725

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

Merged
merged 2 commits into from
Nov 29, 2017
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 @@ -195,7 +195,7 @@

<!-- Remove files from copy local that would not be published as they are provided by the platform package -->
<!-- https://github.com/dotnet/sdk/issues/933 tracks a first class feature for this -->
<Target Name="FilterCopyLocal" DependsOnTargets="RunResolvePublishAssemblies" BeforeTargets="ResolveLockFileCopyLocalProjectDeps">
<Target Name="FilterCopyLocal" DependsOnTargets="GetFrameworkPaths;GetReferenceAssemblyPaths;RunResolvePublishAssemblies" BeforeTargets="ResolveLockFileCopyLocalProjectDeps">
Copy link
Member

Choose a reason for hiding this comment

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

Why the new DependsOnTargets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separate commit for separate issue: 83912b8

<ItemGroup>
<_CopyLocalButNotPublished Include="@(AllCopyLocalItems)" Exclude="@(ResolvedAssembliesToPublish)" />
<AllCopyLocalItems Remove="@(_CopyLocalButNotPublished)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ Copyright (c) .NET Foundation. All rights reserved.
<NETBuildExtensionsError Condition="'$(DependsOnNETStandard)' == 'true' AND '$(NETStandardInbox)' != 'true' AND '$(_UsingOldSDK)' == 'true'"
ResourceName="UnsupportedSDKVersionForNetStandard20"/>

<!--
Just like references from packages, RAR should not waste time finding dependencies and related files for libraries added by this
extension. See related MarkPackageReferencesAsExternallyResolved in Microsoft.PackageDependencyResolution.targets.
-->
<PropertyGroup Condition="'$(MarkNETFrameworkExtensionAssembliesAsExternallyResolved)' == ''">
<MarkNETFrameworkExtensionAssembliesAsExternallyResolved>true</MarkNETFrameworkExtensionAssembliesAsExternallyResolved>
</PropertyGroup>

<!--
.NET 4.7.1 has support for .NET Standard 2.0 built-in, so most of the facades aren't necessary. However, the assembly versions of a few set of assemblies
do not yet match the ones shipped in the support package for .NET Standard 2.0 on .NET 4.7 and below. This means that .NET 4.7 or lower libraries might have
Expand Down Expand Up @@ -113,6 +121,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<Reference Include="%(_NETStandardLibraryNETFrameworkLib.FileName)">
<HintPath>%(_NETStandardLibraryNETFrameworkLib.Identity)</HintPath>
<Private>false</Private>
<ExternallyResolved>$(MarkNETFrameworkExtensionAssembliesAsExternallyResolved)</ExternallyResolved>
</Reference>

<ReferenceCopyLocalPaths Include="@(_NETStandardLibraryNETFrameworkLib)" Condition="'%(FileName)' != 'netfx.force.conflicts'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@

<!-- Remove files from copy local that would not be published as they are provided by the platform package -->
<!-- https://github.com/dotnet/sdk/issues/933 tracks a first class feature for this -->
<Target Name="FilterCopyLocal" DependsOnTargets="RunResolvePublishAssemblies" BeforeTargets="ResolveLockFileCopyLocalProjectDeps">
<Target Name="FilterCopyLocal" DependsOnTargets="GetFrameworkPaths;GetReferenceAssemblyPaths;RunResolvePublishAssemblies" BeforeTargets="ResolveLockFileCopyLocalProjectDeps">
<ItemGroup>
<_CopyLocalButNotPublished Include="@(AllCopyLocalItems)" Exclude="@(ResolvedAssembliesToPublish)" />
<AllCopyLocalItems Remove="@(_CopyLocalButNotPublished)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ Copyright (c) .NET Foundation. All rights reserved.
<PreprocessorValue Include="@(NuGetPreprocessorValue)" Exclude="@(PreprocessorValue)" />
</ItemGroup>

<!--
This will prevent RAR from spending time locating dependencies and related files for assemblies
that came from packages. PackageReference should already be promoted to a closure of Reference
items and we are responsible for adding package relates files to CopyLocal items, not RAR. This
is only configurable as a compat opt-out in case skipping the slow RAR code breaks something.
-->
<PropertyGroup Condition="'$(MarkPackageReferencesAsExternallyResolved)' == ''">
<MarkPackageReferencesAsExternallyResolved>true</MarkPackageReferencesAsExternallyResolved>
</PropertyGroup>

<!--
*************************************
3. BUILD TARGETS
Expand Down Expand Up @@ -424,12 +434,22 @@ Copyright (c) .NET Foundation. All rights reserved.
DependsOnTargets="_ComputeLockFileReferences;_ComputeLockFileFrameworks">

<ItemGroup>
<!--
Update Reference items with NuGetPackageId metadata to set ExternallyResolved appropriately.
NetStandard.Library adds its assets in targets this way and not in the standard way that
would get ExternallyResolved set in ResolvedCompileFileDefinitions below.
-->
<Reference Condition="'%(Reference.NuGetPackageId)' != ''">
<ExternallyResolved>$(MarkPackageReferencesAsExternallyResolved)</ExternallyResolved>
</Reference>

<!-- Add framework references from NuGet packages here, so that if there is also a matching reference from a NuGet package,
it will be treated the same as a reference from the project file. -->
<Reference Include="@(ResolvedFrameworkAssemblies)" />

<ResolvedCompileFileDefinitions Update="@(ResolvedCompileFileDefinitions)">
<HintPath>%(FullPath)</HintPath>
<ExternallyResolved>$(MarkPackageReferencesAsExternallyResolved)</ExternallyResolved>
</ResolvedCompileFileDefinitions>
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,39 @@ public GivenThatWeWantToBuildADesktopLibrary(ITestOutputHelper log) : base(log)
{
}

[WindowsOnlyFact]
public void It_gets_implicit_designtime_facades_when_package_reference_uses_system_runtime()
{
// The repro here is very sensitive to the target framework and packages used. This specific case
// net46 using only System.Collections.Immutable v1.4.0 will not pull in System.Runtime from a
// package or from Microsoft.NET.Build.Extensions as a primary reference and so RARs dependency
// walk needs to find it in order for ImplictlyExpandDesignTimeFacades to inject it.

var netFrameworkLibrary = new TestProject()
{
Name = "NETFrameworkLibrary",
TargetFrameworks = "net46",
IsSdkProject = true,
};

netFrameworkLibrary.PackageReferences.Add(new TestPackageReference("System.Collections.Immutable", "1.4.0"));

netFrameworkLibrary.SourceFiles["NETFramework.cs"] = @"
using System.Collections.Immutable;
public class NETFramework
{
public void Method1()
{
ImmutableList<string>.Empty.Add("""");
}
}";

var testAsset = _testAssetsManager.CreateTestProject(netFrameworkLibrary, "FacadesFromTargetFramework").Restore(Log, netFrameworkLibrary.Name);
var buildCommand = new BuildCommand(Log, Path.Combine(testAsset.TestRoot, netFrameworkLibrary.Name));
buildCommand.Execute().Should().Pass();
}


[Fact]
public void It_can_use_HttpClient_and_exchange_the_type_with_a_NETStandard_library()
{
Expand Down Expand Up @@ -372,5 +405,79 @@ public void It_uses_hintpath_when_replacing_simple_name_references(bool useFacad
.metadata["HintPath"]
.Should().Be(correctHttpReference);
}

[WindowsOnlyTheory]
[InlineData(null)]
[InlineData(true)]
[InlineData(false)]
public void It_marks_extension_references_as_externally_resolved(bool? markAsExternallyResolved)
{
var project = new TestProject
{
Name = "NETFrameworkLibrary",
TargetFrameworks = "net462",
IsSdkProject = true
};

var netStandard2Project = new TestProject
{
Name = "NETStandard20Project",
TargetFrameworks = "netstandard2.0",
IsSdkProject = true
};

project.ReferencedProjects.Add(netStandard2Project);

var asset = _testAssetsManager.CreateTestProject(
project,
"ExternallyResolvedExtensions",
markAsExternallyResolved.ToString())
.WithProjectChanges((path, p) =>
{
if (markAsExternallyResolved != null)
{
var ns = p.Root.Name.Namespace;
p.Root.Add(
new XElement(ns + "PropertyGroup",
new XElement(ns + "MarkNETFrameworkExtensionAssembliesAsExternallyResolved",
markAsExternallyResolved)));
}
})
.Restore(Log, project.Name);

var command = new GetValuesCommand(
Log,
Path.Combine(asset.Path, project.Name),
project.TargetFrameworks,
"Reference",
GetValuesCommand.ValueType.Item);

command.MetadataNames.AddRange(new[] { "ExternallyResolved", "HintPath" });
command.Execute().Should().Pass();

int frameworkReferenceCount = 0;
int extensionReferenceCount = 0;
var references = command.GetValuesWithMetadata();

foreach (var (value, metadata) in references)
{
if (metadata["HintPath"] == "")
{
// implicit framework reference (not externally resolved)
metadata["ExternallyResolved"].Should().BeEmpty();
frameworkReferenceCount++;
}
else
{
// reference added by Microsoft.NET.Build.Extensions
metadata["ExternallyResolved"].Should().BeEquivalentTo((markAsExternallyResolved ?? true).ToString());
extensionReferenceCount++;
}
}

// make sure both cases were encountered
frameworkReferenceCount.Should().BeGreaterThan(0);
extensionReferenceCount.Should().BeGreaterThan(0);
}
}
}
49 changes: 49 additions & 0 deletions test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildALibrary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -731,5 +731,54 @@ public void It_can_target_uwp_using_sdk_extras()
.Should()
.Pass();
}

[Theory]
[InlineData(null)]
[InlineData(true)]
[InlineData(false)]
public void It_marks_package_references_as_externally_resolved(bool? markAsExternallyResolved)
{
var project = new TestProject
{
Name = "Library",
TargetFrameworks = "netstandard2.0",
IsSdkProject = true
};

var asset = _testAssetsManager.CreateTestProject(
project,
"ExternallyResolvedPackages",
markAsExternallyResolved.ToString())
.WithProjectChanges((path, p) =>
{
if (markAsExternallyResolved != null)
{
var ns = p.Root.Name.Namespace;
p.Root.Add(
new XElement(ns + "PropertyGroup",
new XElement(ns + "MarkPackageReferencesAsExternallyResolved",
markAsExternallyResolved)));
}
})
.Restore(Log, project.Name);

var command = new GetValuesCommand(
Log,
Path.Combine(asset.Path, project.Name),
project.TargetFrameworks,
"Reference",
GetValuesCommand.ValueType.Item);

command.MetadataNames.Add("ExternallyResolved");
command.Execute().Should().Pass();

var references = command.GetValuesWithMetadata();
references.Should().NotBeEmpty();

foreach (var (value, metadata) in references)
{
metadata["ExternallyResolved"].Should().BeEquivalentTo((markAsExternallyResolved ?? true).ToString());
}
}
}
}