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.

Sign up for GitHub

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

Jump to bottom

Use PackageDownload #3111

Merged
merged 5 commits into from Apr 16, 2019
Merged

Use PackageDownload #3111

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f5de7e0
Use PackageDownload
dsplaisted Apr 10, 2019
55f4ead
Update tests to use PackageDownload
dsplaisted Apr 11, 2019
23cbbf4
Download AppHost packs for RIDs specified in RuntimeIdentifiers
dsplaisted Apr 12, 2019
42323e3
Honor TargetingPackVersion on FrameworkReference when looking in targ…
dsplaisted Apr 15, 2019
65582d4
Code review feedback: code formatting
dsplaisted Apr 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/ResolveAppHosts.cs
View file
Open in desktop
Expand Up @@ -19,6 +19,8 @@ public class ResolveAppHosts : TaskBase

public string AppHostRuntimeIdentifier { get; set; }

public string [] OtherRuntimeIdentifiers { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, extra space between string and []


public string RuntimeFrameworkVersion { get; set; }

public ITaskItem[] PackAsToolShimRuntimeIdentifiers { get; set; } = Array.Empty<ITaskItem>();
Expand Down Expand Up @@ -130,6 +132,25 @@ protected override void ExecuteCore()
PackAsToolShimAppHostPacks = packAsToolShimAppHostPacks.ToArray();
}

if (OtherRuntimeIdentifiers != null)
{
foreach (var otherRuntimeIdentifier in OtherRuntimeIdentifiers)
{
// Download any apphost packages for other runtime identifiers.
// This allows you to specify the list of RIDs in RuntimeIdentifiers and only restore once,
// and then build for each RuntimeIdentifier without restoring separately.

// We discard the return value, and pass in some bogus data that won't be used, because
// we won't use the assets from the apphost pack in this build.
GetHostItem(otherRuntimeIdentifier,
knownAppHostPacksForTargetFramework,
packagesToDownload,
hostNameWithoutExtension: "unused",
itemName: "unused",
isExecutable: true);
}
}

if (packagesToDownload.Any())
{
PackagesToDownload = packagesToDownload.ToArray();
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/Microsoft.NET.Build.Tasks/ResolveFrameworkReferences.cs
View file
Open in desktop
Expand Up @@ -112,7 +112,7 @@ protected override void ExecuteCore()
string targetingPackPath = null;
if (!string.IsNullOrEmpty(TargetingPackRoot))
{
targetingPackPath = Path.Combine(TargetingPackRoot, knownFrameworkReference.TargetingPackName, knownFrameworkReference.TargetingPackVersion);
targetingPackPath = Path.Combine(TargetingPackRoot, knownFrameworkReference.TargetingPackName, targetingPackVersion);
}
if (targetingPackPath != null && Directory.Exists(targetingPackPath))
{
Expand Down
20 changes: 16 additions & 4 deletions ...Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.TargetingPackResolution.targets
View file
Open in desktop
Expand Up @@ -95,7 +95,7 @@ Copyright (c) .NET Foundation. All rights reserved.
TargetLatestRuntimePatch="$(TargetLatestRuntimePatch)"
EnableTargetingPackDownload="$(EnableTargetingPackDownload)">

<Output TaskParameter="PackagesToDownload" ItemName="_PackageReferenceToAdd" />
<Output TaskParameter="PackagesToDownload" ItemName="_PackageToDownload" />
<Output TaskParameter="RuntimeFrameworks" ItemName="RuntimeFramework" />
<Output TaskParameter="TargetingPacks" ItemName="TargetingPack" />
<Output TaskParameter="RuntimePacks" ItemName="RuntimePack" />
Expand All @@ -107,22 +107,34 @@ Copyright (c) .NET Foundation. All rights reserved.
TargetFrameworkVersion="$(_TargetFrameworkVersionWithoutV)"
TargetingPackRoot="$(NetCoreTargetingPackRoot)"
AppHostRuntimeIdentifier="$(AppHostRuntimeIdentifier)"
OtherRuntimeIdentifiers="$(RuntimeIdentifiers)"
RuntimeFrameworkVersion="$(RuntimeFrameworkVersion)"
PackAsToolShimRuntimeIdentifiers="@(_PackAsToolShimRuntimeIdentifiers)"
DotNetAppHostExecutableNameWithoutExtension="$(_DotNetAppHostExecutableNameWithoutExtension)"
DotNetComHostLibraryNameWithoutExtension="$(_DotNetComHostLibraryNameWithoutExtension)"
RuntimeGraphPath="$(BundledRuntimeIdentifierGraphFile)"
KnownAppHostPacks="@(KnownAppHostPack)">

<Output TaskParameter="PackagesToDownload" ItemName="_PackageReferenceToAdd" />
<Output TaskParameter="PackagesToDownload" ItemName="_PackageToDownload" />
<Output TaskParameter="AppHost" ItemName="AppHostPack" />
<Output TaskParameter="ComHost" ItemName="ComHostPack" />
<Output TaskParameter="PackAsToolShimAppHostPacks" ItemName="PackAsToolShimAppHostPack" />

</ResolveAppHosts>

<ItemGroup>
<PackageReference Include="@(_PackageReferenceToAdd)"
<PropertyGroup Condition="'$(UsePackageDownload)' == ''">
<UsePackageDownload Condition=" '$(MSBuildRuntimeType)' == 'Core'">true</UsePackageDownload>
<UsePackageDownload Condition=" '$(MSBuildRuntimeType)' != 'Core'">false</UsePackageDownload>
</PropertyGroup>

<ItemGroup Condition="'$(UsePackageDownload)' == 'true'">
<PackageDownload Include="@(_PackageToDownload)">
Copy link

Choose a reason for hiding this comment

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

just help me understanding this nuget feature, we use PackageDownload to download, but i didn't find the code to dig (i think it is in)nuget cache.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the GetPackageDirectory task. It doesn't change with this PR, with PackageDownload, the packages are downloaded to the same place that the would have been with PackageReference.

<Version>[%(_PackageToDownload.Version)]</Version>
</PackageDownload>
</ItemGroup>

<ItemGroup Condition="'$(UsePackageDownload)' != 'true'">
<PackageReference Include="@(_PackageToDownload)"
IsImplicitlyDefined="true"
PrivateAssets="all"
ExcludeAssets="all" />
Expand Down
30 changes: 17 additions & 13 deletions src/Tests/Microsoft.NET.Build.Tests/GivenFrameworkReferences.cs
View file
Open in desktop
Expand Up @@ -22,7 +22,9 @@ public GivenFrameworkReferences(ITestOutputHelper log) : base(log)
{
}

[WindowsOnlyFact]
// Tests in this class are currently Core MSBuild only, as they check for PackageDownload items,
// which are currently only used in Core MSBuild
[CoreMSBuildAndWindowsOnlyFact]
public void Multiple_frameworks_are_written_to_runtimeconfig_when_there_are_multiple_FrameworkReferences()
{
var testProject = new TestProject()
Expand Down Expand Up @@ -84,7 +86,7 @@ public static void Main(string [] args)
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App", "Microsoft.WindowsDesktop.App");
}

[Fact]
[CoreMSBuildOnlyFact]
public void The_build_fails_when_there_is_an_unknown_FrameworkReference()
{
var testProject = new TestProject()
Expand Down Expand Up @@ -123,7 +125,7 @@ public void The_build_fails_when_there_is_an_unknown_FrameworkReference()
;
}

[Theory]
[CoreMSBuildOnlyTheory]
[InlineData("netcoreapp2.1", false)]
[InlineData("netcoreapp3.0", true)]
public void KnownFrameworkReferencesOnlyApplyToCorrectTargetFramework(string targetFramework, bool shouldPass)
Expand Down Expand Up @@ -166,7 +168,7 @@ public void KnownFrameworkReferencesOnlyApplyToCorrectTargetFramework(string tar
.And.HaveStdOutContaining("Microsoft.AspNetCore.App");
}
}
[Fact]
[CoreMSBuildOnlyFact]
public void TargetingPackDownloadCanBeDisabled()
{
var testProject = new TestProject()
Expand Down Expand Up @@ -203,7 +205,7 @@ public void TargetingPackDownloadCanBeDisabled()
.HaveStdOutContaining("NETSDK1073");
}

[Fact]
[CoreMSBuildOnlyFact]
public void BuildFailsIfRuntimePackIsNotAvailableForRuntimeIdentifier()
{
var testProject = new TestProject()
Expand Down Expand Up @@ -242,7 +244,7 @@ public void BuildFailsIfRuntimePackIsNotAvailableForRuntimeIdentifier()
.HaveStdOutContaining("1 Error(s)");
}

[Fact]
[CoreMSBuildOnlyFact]
public void BuildFailsIfInvalidRuntimeIdentifierIsSpecified()
{
var testProject = new TestProject()
Expand All @@ -269,7 +271,7 @@ public void BuildFailsIfInvalidRuntimeIdentifierIsSpecified()
.HaveStdOutContaining("1 Error(s)");
}

[Fact]
[CoreMSBuildOnlyFact]
public void BuildFailsIfRuntimePackHasNotBeenRestored()
{
var testProject = new TestProject()
Expand Down Expand Up @@ -310,7 +312,7 @@ public void BuildFailsIfRuntimePackHasNotBeenRestored()

}

[Fact]
[CoreMSBuildOnlyFact]
public void RuntimeFrameworkVersionCanBeSpecifiedOnFrameworkReference()
{
var testProject = new TestProject();
Expand Down Expand Up @@ -342,7 +344,7 @@ public void RuntimeFrameworkVersionCanBeSpecifiedOnFrameworkReference()
resolvedVersions.AppHostPack["AppHost"].Should().Be("3.0.0-runtimeframeworkversion-property");
}

[Fact]
[CoreMSBuildOnlyFact]
public void RuntimeFrameworkVersionCanBeSpecifiedViaProperty()
{
var testProject = new TestProject();
Expand All @@ -365,7 +367,7 @@ public void RuntimeFrameworkVersionCanBeSpecifiedViaProperty()
resolvedVersions.AppHostPack["AppHost"].Should().Be(runtimeFrameworkVersion);
}

[Theory]
[CoreMSBuildOnlyTheory]
[InlineData(true)]
[InlineData(false)]
public void TargetLatestPatchCanBeSpecifiedOnFrameworkReference(bool attributeValue)
Expand Down Expand Up @@ -401,7 +403,7 @@ public void TargetLatestPatchCanBeSpecifiedOnFrameworkReference(bool attributeVa
resolvedVersions.AppHostPack["AppHost"].Should().Be("3.0.0-apphostversion");
}

[Theory]
[CoreMSBuildOnlyTheory]
[InlineData(true)]
[InlineData(false)]
public void TargetLatestPatchCanBeSpecifiedViaProperty(bool propertyValue)
Expand All @@ -427,7 +429,7 @@ public void TargetLatestPatchCanBeSpecifiedViaProperty(bool propertyValue)
resolvedVersions.AppHostPack["AppHost"].Should().Be("3.0.0-apphostversion");
}

[Fact]
[CoreMSBuildOnlyFact]
public void TargetingPackVersionCanBeSpecifiedOnFrameworkReference()
{
var testProject = new TestProject();
Expand Down Expand Up @@ -499,7 +501,7 @@ public void TargetingPackVersionCanBeSpecifiedOnFrameworkReference()
<Target Name=`WriteResolvedVersions` DependsOnTargets=`PrepareForBuild;ResolveFrameworkReferences`>
<ItemGroup>
<LinesToWrite Include=`RuntimeFramework%09%(RuntimeFramework.Identity)%09%(RuntimeFramework.Version)`/>
<LinesToWrite Include=`PackageDownload%09%(_PackageReferenceToAdd.Identity)%09%(_PackageReferenceToAdd.Version)`/>
<LinesToWrite Include=`PackageDownload%09%(PackageDownload.Identity)%09%(PackageDownload.Version)`/>
<LinesToWrite Include=`TargetingPack%09%(TargetingPack.Identity)%09%(TargetingPack.PackageVersion)`/>
<LinesToWrite Include=`RuntimePack%09%(RuntimePack.Identity)%09%(RuntimePack.PackageVersion)`/>
<LinesToWrite Include=`AppHostPack%09%(AppHostPack.Identity)%09%(AppHostPack.PackageVersion)`/>
Expand Down Expand Up @@ -558,7 +560,9 @@ public static ResolvedVersionInfo ParseFrom(string path)
dict = versionInfo.RuntimeFramework;
break;
case "PackageDownload":
// PackageDownload versions are enclosed in [brackets]
dict = versionInfo.PackageDownload;
version = version.Substring(1, version.Length - 2);
break;
case "TargetingPack":
dict = versionInfo.TargetingPack;
Expand Down