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 switch to disable apphost pack restore #29137

Merged
merged 6 commits into from
Nov 21, 2022
Merged

Conversation

ViktorHofer
Copy link
Member

Contributes to dotnet/runtime#58109
Mimics #15944

In dotnet/runtime we live build targeting packs, runtime packs and app host packs. For the former two, switches already exist to disable nuget restore:

  • EnableTargetingPackDownload
  • EnableRuntimePackDownload

The latter one doesn't yet have such a switch and therefore is always restored by NuGet. In the cases where want to use the live built apphost pack, restore fails, i.e. from a runtime build:

artifacts/bin/trimmingTests/projects/Microsoft.Extensions.DependencyInjection.TrimmingTests/ActivatorUtilitiesTests/osx-x64/project.csproj(0,0): error NU1102: (NETCORE_ENGINEERING_TELEMETRY=Build) Unable to find package Microsoft.NETCore.App.Host.osx-x64 with version (= 8.0.0)

Contributes to dotnet/runtime#58109

In dotnet/runtime we live build targeting packs, runtime packs and app host packs. For the former two, switches already exist to disable nuget restore:
- EnableTargetingPackDownload
- EnableRuntimePackDownload

The latter one doesn't yet have such a switch and therefore is always restored by NuGet. In the cases where want to use the live built apphost pack, restore fails, i.e. from a runtime build:

`artifacts/bin/trimmingTests/projects/Microsoft.Extensions.DependencyInjection.TrimmingTests/ActivatorUtilitiesTests/osx-x64/project.csproj(0,0): error NU1102: (NETCORE_ENGINEERING_TELEMETRY=Build) Unable to find package Microsoft.NETCore.App.Host.osx-x64 with version (= 8.0.0)`
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ViktorHofer ViktorHofer marked this pull request as draft November 20, 2022 11:00
@ViktorHofer ViktorHofer marked this pull request as ready for review November 21, 2022 11:05
@ViktorHofer ViktorHofer reopened this Nov 21, 2022
@ViktorHofer ViktorHofer enabled auto-merge (squash) November 21, 2022 11:53
@ViktorHofer ViktorHofer merged commit bb61c2e into main Nov 21, 2022
@ViktorHofer ViktorHofer deleted the ViktorHofer-patch-1 branch November 21, 2022 13:09
@@ -54,6 +54,8 @@ public class ResolveAppHosts : TaskBase
public bool NuGetRestoreSupported { get; set; } = true;

public string NetCoreTargetingPackRoot { get; set; }

public bool EnableAppHostPackDownload { get; set; } = true;
Copy link
Member

Choose a reason for hiding this comment

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

Normally we set the default value for something like this in an MSBuild property. I'm not entirely sure that this works correctly if the MSBuild property is empty, though if it didn't work there would probably some test failures.

Copy link
Member Author

@ViktorHofer ViktorHofer Nov 21, 2022

Choose a reason for hiding this comment

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

I tested that and it works as expected. Also, I just mimicked the NuGetRestoreSupported property which also sets a default property value and is already used.

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 this pull request may close these issues.

3 participants