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 Package Download API #5919

Open
zhili1208 opened this issue Sep 21, 2017 · 17 comments
Open

NuGet Package Download API #5919

zhili1208 opened this issue Sep 21, 2017 · 17 comments
Labels
Functionality:SDK The NuGet client packages published to nuget.org Priority:2 Issues for the current backlog. Type:Feature
Milestone

Comments

@zhili1208
Copy link
Contributor

Initial Spec: https://github.com/NuGet/Home/wiki/%5BSpec%5D-Package-download-API

@zhili1208
Copy link
Contributor Author

@jeffkl -jeff, I wrote a initial spec for this API, Please provide feedback.

@jeffkl
Copy link
Contributor

jeffkl commented Sep 22, 2017

I think it would be better if the method had this signature:

public static string DownloadPackage(
  string packageId,
  string version,
  ISettings settings,
  ILogger logger)

From my perspective, I just need NuGet to acquire the package and tell me where it put it. I think taking an ISettings which would be more flexible.

I also don't care about target framework for this particular case as the packages won't have any TFM-specific folders. In my opinion, this API is just requesting that NuGet download the package to where ever NuGet would put it. I do not want to tell NuGet where to put the package. That seems like legacy packages.config behavior.

I'll also need logging so passing in an ILogger is a necessity.

We do not need an MSBuild task for this API. It can just exist in the object model and I'll be implementing an SdkResolver which runs before targets and tasks execute.

Does NuGet need to support downloading multiple packages in one call?

At this time no, but perhaps the API should take a list? That would save us from having to add this functionality later.

Does NuGet need to provide a way to find all dependency packages after the top package downloaded?

I'm on the fence on this one. SDK packages can't currently have dependencies so I don't need this right now but I can see in the future needing to make that work.

@isaacabraham
Copy link

Can we also see how this is documented in terms of the actual HTTP API (if there are any changes planned that impact that)?

@zhili1208
Copy link
Contributor Author

@jeffkl Why you need ISettings ? it only contains NuGet.Config, I think this interface should be used only in nuget, it's not for public use.

@zhili1208
Copy link
Contributor Author

@isaacabraham I don't think there are any changes that impact actual HTTP API.

@jeffkl
Copy link
Contributor

jeffkl commented Sep 22, 2017

Passing ISettings would make things a little more flexible for the caller than just passing in a root directory. But either one works. My code will be restoring a package for a particular project so I could also pass in that path and your API could use that as the root directory for creating an ISettings object.

@emgarten
Copy link
Member

There are a lot of pitfalls around creating ISettings, unless you really need to customize this you are better off just passing the path as you described.

@jeffkl
Copy link
Contributor

jeffkl commented Dec 13, 2017

@zhili1208 did you make any progress on this? I'm working to get the SDK resolver in the next release

@emgarten
Copy link
Member

@jeffkl when do you need this by, would you give a date? This has been on hold since it wasn't needed yet.

@jeffkl
Copy link
Contributor

jeffkl commented Dec 13, 2017

I'm starting on the SDK resolver in MSBuild this week. The earlier we get this the better but realistically it could be mid January. I need to find the release schedule to be sure. Do you think this is a big effort for your team or have a rough ETA?

@emgarten
Copy link
Member

@zhili1208 was working on this but is out until Jan.

//cc @rrelyea for the timeline on this

@jeffkl
Copy link
Contributor

jeffkl commented Dec 13, 2017

Great I'll sync up with you guys first week of January to lock down on a timeline.. Thanks!

@natemcmaster
Copy link

👀 I'm very interested in this API too. We implemented an MSBuild task in the aspnetcore build system that looks basically like this, but would love to see a first-class MSBuild task for it, as mentioned in https://github.com/NuGet/Home/wiki/%5BSpec%5D-Package-download-API#msbuild-task.

@ghost
Copy link

ghost commented Feb 4, 2018

Any updates on this one? Can we have any visibility what is going on, what is missing, tasks, blocker, something we can help with? Please SHARE to get HELP

@wli3
Copy link

wli3 commented Mar 1, 2018

To support dotnetTool, this API should be able to

  1. filter TFM, RID for tool package type
  2. allow Microsoft.NETCore.Platforms to be added
  3. additional feed etc.
    ...
    ...

There has been a lot of work done for restore target. I feel the requirement moved further than a cross plat nuget.exe install to an msbuild restore target or equivalent API that don't need a temp project.

@nkolev92 @KathleenDollard

@wli3
Copy link

wli3 commented Mar 1, 2018

If restore target eventually call a c# API. Edit + Expose such API maybe better. So CLI can have a proper exception object and don't need to deal with text stream. @peterhuene @livarcocc

@nkolev92
Copy link
Member

nkolev92 commented Mar 1, 2018

@wli3 As I mentioned in the other issue, I'm not sure this is the appropriate direction for the dotnet tools.

I'm not a fan of expanding the scope of a "download API", to fit reqs that specifically require restore. (compat checking, runtime graph eval)
This API is not supposed to replace restore.

The C# API thing is a also a tricky one, because that'd effectively introduce a new public surface area for NuGet, and that surface area is not something that is written with that in mind.

If logging/language is the only issue, we can certainly look into fixing that.
The error code in question, is a DotnetTool specific error code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:SDK The NuGet client packages published to nuget.org Priority:2 Issues for the current backlog. Type:Feature
Projects
None yet
Development

No branches or pull requests

9 participants