NuGet fixes #734

ghost · 2017-04-07T15:42:39Z

This pull request should fix everything mentioned #569, except for nuget.config customization. Premake now uses api.nuget.org to fetch required information about the packages.

I've tested this against the following NuGet packages:

C++:

boost
boost_filesystem-vc140
boost_system-vc140
sdl2.v140
sdl2.v140.redist
glew.v140
harfbuzz
physfs.v140
freetype2
assimp.v140
assimp.v140.redist
pugixml

C#:

Newtonsoft.Json
NUnit
SSH.NET

There is one problem where older C# packages (published before 2016 or sometime 2016?) might not work because the API response for them doesn't always contain a file listing. There isn't really anything that we can do about this, other than to inform the user and abort.

One thing to also note is that this will increase generation times for projects with NuGet packages. With 12 packages, I was getting generation times of around 20 seconds (api.nuget.org is slow).

We might want to consider caching the API requests to a file. If we had an asynchronous HTTP API, that could also help because then we could run requests in parallel. Any thoughts?

Anyway, I'd appreciate it if people with C# projects could try out this branch before it was merged, preferably with more and different packages (@tritao, @SoleilLapierre, @tvandijck?). I don't have any personal C# projects, so I only have a dummy project to test this with.

tritao · 2017-04-07T15:56:38Z

Sounds great, I'll test it out during the weekend.

tvandijck

So I have one thing that may make this a bit more complicated... but look here:
https://github.com/premake/premake-core/blob/master/premake5.lua#L67

Some teams use premake without curl, at which point the 'http' API is entirely compiled away..

https://github.com/premake/premake-core/blob/master/src/host/premake.c#L100

So I really think this nuget fix is awesome, but it will have to check in some places for this particular case, and basically just error out and say it cannot get the information from the nuget server...

tvandijck · 2017-04-07T16:33:51Z

src/actions/vstudio/vs2010_nuget.lua

-		for prj in p.workspace.eachproject(wks) do
-			for i = 1, #prj.nuget do
-				local package = prj.nuget[i]
+			local response, err, code = http.get(string.format("https://api.nuget.org/v3/registration1/%s/index.json", id:lower()))


I really like this to be configurable, there is other nuget streams out there, and certainly within companies like ours (Blizzard) we host our own internal caching streams, through things like artifactory, and myget...

I just tried this with MyGet, and while it looks like while they implement the same API as api.nuget.org, they don't return a file listing for the packages (at least if they were uploaded by their automatic packages.config tool). There's something weird going on here.

api.nuget.org doesn't return a file listing for all packages either, but from my testing that only seemed to happen with older packages.

If the API doesn't provide us with a file listing, the only other way that I can think of is to have Premake download and extract the packages.

It looks like only "NuGet galleries" report the file listing to us and "NuGet.Server" does not. So, I'm going to see if I can make Premake download and extract the packages so that we can support both.

This would also resolve the issue where api.nuget.org doesn't always return the file listing for whatever reason (and the caching issue I mentioned in the OP).

API's and support aside, let's at least make this configurable... a nuget.serverlist = {}, or something...

Then as for the filelist.. for now, we can simple error... and if we hit it, we can look at implementing whatever is needed to get it to work... Internally here at Blizzard we are using NuGet.Server, so yeah, we're going to hit that, since those implementations only implement the V2 API of nuget, and it seems like you are querying the V3 API.. Which is all right, if we need it, we'll either get to it ourselves, or someone else who needs it will...

tvandijck · 2017-04-10T15:41:45Z

src/base/validation.lua

@@ -219,6 +220,13 @@
 	end


+	function m.NuGetHasHTTP(prj)


that is a pretty smart solution ;) I like it...

ghost · 2017-04-10T17:29:01Z

How's this?

ghost · 2017-04-10T17:30:54Z

I did run into something else when testing this. Some packages, such as MySQL.Data depend on other packages, but this doesn't currently handle those dependencies. So that's going to need some fixing. Should we fix it before merging, or should we do it in a separate PR?

tvandijck · 2017-04-10T17:38:46Z

I'd say, separate PR... I'm sure there will be many more nuget improvements over time...

ghost · 2017-04-10T17:41:13Z

Sure.

SoleilLapierre · 2017-04-10T23:12:30Z

I just tried from c5bf4b9 but I get errors when creating a NuGet dependency on NUnit. If I use:

nuget { "NUnit:3.6.0" }

then I get:

Running action 'vs2015'...
Examining NuGet package source 'https://api.nuget.org/v3/index.json'...
Error: ** Error: NuGet API error (0)
SSL connect error
schannel: failed to retrieve ALPN result

If leave out the :3.6.0 version number, the error becomes:

Running action 'vs2015'...
Generated Zaber.Plugins.sln...
Generated Zaber.Plugins/Zaber.Plugins.csproj...
Error: [string "src/actions/vstudio/vs2010_nuget.lua"]:9: attempt to perform arithmetic on a nil value

I was following the instructions in BUILD.txt to build Premake on Windows using VS2015. "premake5 test" fails with this build.

ghost · 2017-04-11T10:50:46Z

I think we're hitting a libcurl bug here (curl/curl#840), solution would be to update libcurl.

ghost · 2017-04-11T12:18:15Z

I've opened pull request #738 to update libcurl.

ClxS · 2017-04-13T09:50:08Z

src/actions/vstudio/vs2005_csproj.lua

+
+					_x(2, '<Reference Include="%s">', path.getbasename(file))
+					_x(3, '<HintPath>%s</HintPath>', vstudio.path(prj, p.filename(prj.solution, string.format("packages\\%s.%s\\%s", id, packageAPIInfo.verbatimVersion or packageAPIInfo.version, file))))
+					_p(3, '<Private>True</Private>')


One thing to note about this which we've had to fix on our internal version - this doesn't respect NoCopyLocal. On our version we replaced this line with:
local cfg = project.getfirstconfig(prj)
if premake.config.isCopyLocal(prj, package, cfg) then
_p(3, 'True')
else
_p(3, 'False')
end

Thanks for letting me know. I've fixed this in bde50fc.

This was generated on a per-solution basis when one needed to be generated for each project.

For C# projects, we need to get the assembly paths from the NuGet API, instead of assuming that the package only contains one assembly with the same name as the package.

Non-gallery sources (such as NuGet.Server) are currently unsupported due to API limitations.

If a package has a lot of versions, the NuGet API returns multiple "pages" of results, but we only looked at the first "page". This happened with the "NLog" package for example.

tvandijck suggested changes Apr 7, 2017

View reviewed changes

tvandijck approved these changes Apr 10, 2017

View reviewed changes

src/base/validation.lua

@@ -219,6 +220,13 @@

end

function m.NuGetHasHTTP(prj)

Copy link

Contributor

tvandijck Apr 10, 2017

Choose a reason for hiding this comment

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

that is a pretty smart solution ;) I like it...

ClxS reviewed Apr 13, 2017

View reviewed changes

ghost mentioned this pull request Apr 13, 2017

Refactor language flags to go through the language API. #686

Merged

aleksijuvani and others added 17 commits April 14, 2017 13:34

Fix NuGet packages.config generation

b2bbf00

This was generated on a per-solution basis when one needed to be generated for each project.

Fix NuGet packages.config files missing from projects

7746353

Raise error on packages.config conflict

39720da

Validate NuGet package names with the NuGet API

cbcb942

Validate NuGet package versions with the NuGet API

bb992e6

Fix NuGet assembly references in C# projects

c574772

For C# projects, we need to get the assembly paths from the NuGet API, instead of assuming that the package only contains one assembly with the same name as the package.

Fetch NuGet API data on demand, not all at once

bd3c542

Fix C# NuGet tests

3bdf746

Add test for multiple assemblies in C# NuGet packages

df6c1f4

Fix certain NuGet API error messages

34eb9c6

Raise an error if trying to use NuGet without Curl

52b21ee

Add nugetsource API for custom package sources

8286a7b

Non-gallery sources (such as NuGet.Server) are currently unsupported due to API limitations.

Validate NuGet package strings

87f527c

Respect copylocal for NuGet packages

368ef42

Fix not all versions of NuGet packages being found

cf82b2c

If a package has a lot of versions, the NuGet API returns multiple "pages" of results, but we only looked at the first "page". This happened with the "NLog" package for example.

Warn about NuGet package dependencies

067db7a

Merge branch 'master' into nuget-fixes

88d37b1

tvandijck merged commit 473b2f1 into premake:master Apr 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NuGet fixes #734

NuGet fixes #734

ghost commented Apr 7, 2017

tritao commented Apr 7, 2017 •

edited

Loading

tvandijck left a comment

tvandijck Apr 7, 2017

ghost Apr 8, 2017

ghost Apr 8, 2017 •

edited by ghost

Loading

tvandijck Apr 10, 2017

tvandijck Apr 10, 2017

ghost commented Apr 10, 2017

ghost commented Apr 10, 2017

tvandijck commented Apr 10, 2017

ghost commented Apr 10, 2017

SoleilLapierre commented Apr 10, 2017

ghost commented Apr 11, 2017

ghost commented Apr 11, 2017

ClxS Apr 13, 2017

ghost Apr 13, 2017

NuGet fixes #734

NuGet fixes #734

Conversation

ghost commented Apr 7, 2017

tritao commented Apr 7, 2017 • edited Loading

tvandijck left a comment

Choose a reason for hiding this comment

tvandijck Apr 7, 2017

Choose a reason for hiding this comment

ghost Apr 8, 2017

Choose a reason for hiding this comment

ghost Apr 8, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

tvandijck Apr 10, 2017

Choose a reason for hiding this comment

tvandijck Apr 10, 2017

Choose a reason for hiding this comment

ghost commented Apr 10, 2017

ghost commented Apr 10, 2017

tvandijck commented Apr 10, 2017

ghost commented Apr 10, 2017

SoleilLapierre commented Apr 10, 2017

ghost commented Apr 11, 2017

ghost commented Apr 11, 2017

ClxS Apr 13, 2017

Choose a reason for hiding this comment

ghost Apr 13, 2017

Choose a reason for hiding this comment

tritao commented Apr 7, 2017 •

edited

Loading

ghost Apr 8, 2017 •

edited by ghost

Loading