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 to disable downloading a runtime pack when RID is set #14926

Closed
safern opened this issue Dec 10, 2020 · 2 comments · Fixed by #15944
Closed

Add support to disable downloading a runtime pack when RID is set #14926

safern opened this issue Dec 10, 2020 · 2 comments · Fixed by #15944
Milestone

Comments

@safern
Copy link
Member

safern commented Dec 10, 2020

Currently the SDK provides a hook to disable downloading the targeting pack: EnableTargetingPackDownload, however if a RuntimeIdentifier is set it attempts to download a runtime pack even if that property is set to false.

This has caused trouble in dotnet/runtime where we build a targeting/runtime pack (as a directory not nupkg) and then we override the PackageDirectory to point to the corresponding directory, so we never need to download either the targeting or runtime pack. However, now that we already moved our target framework to use net6.0 we had to add a KnownTargetFramework for net6.0 and we set the RuntimeFrameworkVersion to 6.0.0, since that runtime package doesn't exist in any of the feeds, we hit errors like:

/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102: Unable to find package Microsoft.NETCore.App.Runtime.browser-wasm with version (= 6.0.0)
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 437 version(s) in dotnet5 [ Nearest version: 6.0.0-alpha.1.20420.3 ]
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 396 version(s) in dotnet6 [ Nearest version: 6.0.0-alpha.1.20610.6 ]
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 8 version(s) in nuget.org [ Nearest version: 5.0.1 ]
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 0 version(s) in dotnet-eng
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 0 version(s) in dotnet-tools
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 0 version(s) in dotnet5-transport
/Users/santifdezm/repos/runtime/src/libraries/Microsoft.CSharp/tests/Microsoft.CSharp.Tests.csproj : error NU1102:   - Found 0 version(s) in dotnet6-transport

So we have had to add a workaround in a couple of places already to remove the package download with something like:

<Target Name="RemoveRuntimePackFromDownloadItem"
          Condition="'$(_UseLocalTargetingRuntimePack)' == 'true' and '$(RuntimeIdentifier)' != ''"
          AfterTargets="ProcessFrameworkReferences">
  <ItemGroup>
    <PackageDownload Remove="@(PackageDownload)"
                                       Condition="$([System.String]::Copy('%(Identity)').StartsWith('Microsoft.NETCore.App.Runtime'))" />
  </ItemGroup>
</Target>

It would be great if we could just set something like EnableRuntimePackDownload=false and this is skipped:

if (RuntimeIdentifiers != null)
{
foreach (var runtimeIdentifier in RuntimeIdentifiers)
{
if (processedPrimaryRuntimeIdentifier && runtimeIdentifier == this.RuntimeIdentifier)
{
// We've already processed this RID
continue;
}
// Pass in null for the runtimePacks list, as for these runtime identifiers we only want to
// download the runtime packs, but not use the assets from them
ProcessRuntimeIdentifier(runtimeIdentifier, runtimePackForRuntimeIDProcessing, runtimePackVersion, additionalFrameworkReferencesForRuntimePack: null,
unrecognizedRuntimeIdentifiers, unavailableRuntimePacks, runtimePacks: null, packagesToDownload, isTrimmable, includeInPackageDownload);
}
}

cc: @dsplaisted @ViktorHofer @ericstj @Anipik

@marcpopMSFT marcpopMSFT added the untriaged Request triage from a team member label Dec 11, 2020
@marcpopMSFT marcpopMSFT self-assigned this Dec 11, 2020
@marcpopMSFT marcpopMSFT added needs team triage Requires a full team discussion and removed untriaged Request triage from a team member labels Dec 11, 2020
@marcpopMSFT marcpopMSFT removed their assignment Dec 11, 2020
@marcpopMSFT marcpopMSFT added this to the 6.0.1xx milestone Dec 16, 2020
@marcpopMSFT marcpopMSFT removed the needs team triage Requires a full team discussion label Dec 16, 2020
@marcpopMSFT
Copy link
Member

@safern I assume just disabling the download isn't enough as work would need to be done to pick up the runtime from the local build. We can put it in our backlog for 6.0 but don't know when we'll get to it but if ya'll want to contribute a PR, we'd be happy to take alook.

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 16, 2020

Right, in addition to disable the downloading of the runtime packs, we hook up the locally built runtime pack. I'm willing to send a PR, the change should be trivial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants