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

Nuget restore/package download C# API #10430

Open
wli3 opened this issue Jan 6, 2021 · 12 comments
Open

Nuget restore/package download C# API #10430

wli3 opened this issue Jan 6, 2021 · 12 comments
Labels
Functionality:Restore Functionality:SDK The NuGet client packages published to nuget.org Partner:DotNet Priority:2 Issues for the current backlog. Type:Feature

Comments

@wli3
Copy link

wli3 commented Jan 6, 2021

This was raised during development of dotnet tools. The decision was "try to use a temp project and restore the package for now". However, after using this mechanism for 2 years. We still wish to have a C# API to do restore, especially considering we would reuse the dotnet tool install component to install dotnet SDK workloads. Dotnet SDK workloads installation experience needs to be bulletproof or the user will be blocked using Xamarin.

Issues for existing temp project approach

  1. Multiple IO operation and process hops caused unreliability and they are very hard to reproduce and resolve. First a file needs to be written to the disk. And a restore in a separate process need to write more files. The following 2 issues are likely caused by race conditions. However, I cannot reproduce them. The cleanup of the temporary files is also tricky.

    a. .NET 5: Unable to install some .NET tools (dotnet tool install) · Issue #14547 · dotnet/sdk (github.com)

    b. Failed to install tool package ... · Issue #11459 · dotnet/sdk (github.com)

  2. Hard-to-act-on error messages. Because the communication between SDK and nuget restore is via the command line. When error happens, SDK can only redirect the output of nuget and SDK cannot give a more specific guidance for the user to act on. That results the long list of "probably this happened." https://docs.microsoft.com/en-us/dotnet/core/tools/troubleshoot-usage-issues#executable-file-not-found

  3. Environment leak in. Because there is an explicit msbuild run. Environment variables get leaked in. There is no good way to prevent them.

    a. `dotnet tool install ...` fails if environment variable RESTOREPROJECTS is set · Issue #10798 · dotnet/sdk (github.com)

  4. Escape unescaped madness. Because the communication is done via the command line. SDK need to carefully escape/unescape characters. It is not trivial and error prone.

Requirement

It should be a replacement for the temp project. It should select correct TFM and generate asset.json. However, if we can get an API that can simply download package to a specific location, that would fit net6.0 requirement. But it won't help us making tools experience better.

@wli3
Copy link
Author

wli3 commented Jan 6, 2021

@nkolev92 @aortiz-msft we hope to discuss this in next Monday sync meeting

@nkolev92
Copy link
Member

nkolev92 commented Jan 7, 2021

I think the requirements for the workload download are not equivalent and as such need different solutions.

Tools rely on detecting configuration, but also doing asset selection, while it's not clear that workload download will need any type of asset selection.

I think migrating to use tool restore without a project file is possible, but we need to evaluate the work.

A more generic API for downloading also technically exists, but there's many gotchas depending on the requirements.

However, if we can get an API that can simply download package to a specific location, that would fit net6.0 requirement

I think that's not enough for tools. You need asset selection, which is why restore was the preferred implementation.

@wli3 Before the meeting, it'd be helpful if you can describe the workload scenarios in more detail.

@nkolev92 nkolev92 added Functionality:Restore Functionality:SDK The NuGet client packages published to nuget.org Type:Feature labels Jan 7, 2021
@wli3
Copy link
Author

wli3 commented Jan 7, 2021

Agree. Workload scenario has less requirement than tools. For workloads, it need to support input of: package id, version range, location to restore to, RestoreRootConfigDirectory -- path where nuget starts to probe configuration (like nuget config), optional path for nuget config, sources urls as an array. This is what I can think of so far. It does not require nuget to filter assets and produce asset.json like dotnet tools. And we should have C# exception when error happens so SDK could do recovery or issue helpful messages.

The main goal is to download nuget packages while no need for SDK team to reimplement http retry, caching and supports for custom/multiple/local feed etc.

However, it is highly desired that it also supports tools scenario. Or we will have 2 different package download system (3 if you count the package download in msbuild) inside SDK.

@nkolev92
Copy link
Member

nkolev92 commented Jan 8, 2021

Thanks for clarifying the scenario @wli3, that helps.

@dsplaisted
Copy link

As part of the API, it would be nice to have a way to report download progress.

@nkolev92
Copy link
Member

nkolev92 commented Jan 8, 2021

As part of the API, it would be nice to have a way to report download progress.

We have log reporting for individual files, but not for progress on a single file.

@aortiz-msft aortiz-msft added the Priority:2 Issues for the current backlog. label Jan 21, 2021
@aortiz-msft aortiz-msft added this to the Sprint 182 - 2021.01.11 milestone Jan 21, 2021
@zivkan
Copy link
Member

zivkan commented Feb 1, 2021

@wli3 @dsplaisted In a meeting, regarding the "tool download" scenario, we suggested trying to existing APIs, just as the NuGetSdkResolver does. Did anyone have time to try it out yet?

Regarding download progress (particularly for VS for Mac), via email we suggested the caller implementing their own IHttpRetryHandler. I can create a sample if that helps.

I'm trying to plan my sprint, and estimate how much time I'll need on this issue.

@wli3
Copy link
Author

wli3 commented Feb 1, 2021

@zivkan yes. I am trying it out. Although some build fire of my team caused 2 weeks of delay. https://github.com/dotnet/sdk/compare/master...wli3:try-nuget-csharpapi?expand=1

I find I need to copy paste to create HttpRetryHandlerWithProgress.cs , but I can override DownloadTimeoutStreamWithLog.cs without too much duplication.

On that front, other than it looks ugly and you need to maintain these API's stability, I don't see action required from you.

However, looks like we need to solve the authentication issue (dotnet restore --interactive). Could you give me some guidance on how to implement authentication (talk with the auth plug-in) using this API?

@zivkan zivkan modified the milestones: Sprint 2021-02, Sprint 2021-03 Mar 1, 2021
@zivkan
Copy link
Member

zivkan commented Mar 23, 2021

Given dotnet/sdk#16443 was merged, I'm going to close this issue.

Unfortunately the changes requested were complex and would have taken a lot of effort for the NuGet team to implement, so it was faster for the SDK team to use NuGet's APIs "lightly" and implement the required functionality themselves.

I hope we (NuGet) can do better in the future, but right now there are many other things to prioritize.

@zivkan zivkan closed this as completed Mar 23, 2021
@wli3
Copy link
Author

wli3 commented Mar 24, 2021

@zivkan we can close this issue but note dotnet/sdk#16443 is about 5% of the work. We merge it so that we can work on different implementation threads in parallel. We have a lot of "remining" tasks dotnet/sdk#16470. And we will send emails ask for detail usage when we get to it. So please still allocate time for replying these emails.

@zivkan
Copy link
Member

zivkan commented Mar 24, 2021

Thanks for the reminder. I'm reopening for sprint planning purposes, as suggested.

@zivkan zivkan reopened this Mar 24, 2021
@zivkan zivkan modified the milestones: Sprint 2021-03, Sprint 2021-04 Apr 1, 2021
@gbaydin
Copy link

gbaydin commented Apr 11, 2021

As part of the API, it would be nice to have a way to report download progress.

May I please bring the following issues to your attention? My interpretation of the conversation above is that this is relevant for the code you're working on. The issue is the lack of a very basic progress bar (or any indicator of progress) during the download of individual files (imagine dotnet restore hanging for 30 mins without printing a single thing). This is a big shortcoming in the user experience compared with package managers of other ecosystems (python, etc.) which provide this progress information.

#4346
dotnet/sdk#14813

@zivkan zivkan modified the milestones: Sprint 2021-04, Sprint 2021-05 Apr 26, 2021
@zivkan zivkan removed the Priority:2 Issues for the current backlog. label May 24, 2021
@zivkan zivkan added the Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. label May 24, 2021
@zivkan zivkan removed this from the Sprint 2021-05 milestone May 24, 2021
@zivkan zivkan removed their assignment May 24, 2021
@zkat zkat mentioned this issue Jul 1, 2021
5 tasks
@zkat zkat mentioned this issue Jul 12, 2021
8 tasks
@nkolev92 nkolev92 added Priority:2 Issues for the current backlog. and removed Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. labels Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Restore Functionality:SDK The NuGet client packages published to nuget.org Partner:DotNet Priority:2 Issues for the current backlog. Type:Feature
Projects
None yet
Development

No branches or pull requests

8 participants