premake / premake-core

Premake
https://premake.github.io/
BSD 3-Clause "New" or "Revised" License
3.22k stars 620 forks source link

Nuget keyword not respecting nuget.config, project location, actual lib names #569

Open SoleilLapierre opened 8 years ago

SoleilLapierre commented 8 years ago

Hi. I'm currently evaluating premake 5 as a possible new build system at work. So far I'm liking it lots.

I'm having some trouble getting nuget dependencies working though. The first thing I tried to do with it was add a dependency on NUnit 3.4.1 in one project of a vs2015 multi-project solution. I realize it's bleeding edge days for premake 5 and I may have to invest some effort into making it work, but the user guide says if something's not working as you expect, submit a ticket. Also from reading some of your dev discussions and pull requests I think this probably touches on larger questions.

So here goes. :)

1) There appear to be assumptions that a NuGet package contains only one relevant lib and that it has the same name as the package. These are not always the case, but as far as I can tell the .nupkg file doesn't actually tell you which files you want; it might actually be correct to just find and use all the DLLs in the appropriate subdirectory of the package.

2) The content of the generated packages.config file is correct, but it is placed next to the .sln file rather than the .csproj that actually has the dependency.

3) The properties added to the .csproj are not correct; in addition to #1 above it adds dependencies for every .net framework version, and fails to add the file include for packages.config. Because of this the package will never be downloaded and the build will fail.

4) If you have a nuget.config file set up so that all your packages download to one nice clean place, that location is not respected. See http://docs.nuget.org/release-notes/nuget-2.1#Specify_%e2%80%98packages%e2%80%99_Folder_Location - there are several places this config can go. (Actually, I'd be totally cool with premake generating the nunit.config file next to the .sln as long as it allowed specifying extra package sources as well as a custom repository path.)

Here are the diffs to the .csproj file if I add the NuGet package manually, using the solution generated from the attached repro case with the nuget line commented out. This is what should happen. Also note that it's using the .net 4.5 version even though I use 4.6, because there is no 4.6 version in the NuGet package.

42a43,47
>     <Reference Include="nunit.framework, Version=3.4.1.0, Culture=neutral, PublicKeyToken=2638cd05610744eb, processorArchitecture=MSIL">
>       <SpecificVersion>False</SpecificVersion>
>       <HintPath>..\..\..\nuget_packages\downloaded\NUnit.3.4.1\lib\net45\nunit.framework.dll</HintPath>
>       <Private>True</Private>
>     </Reference>
57a63,65
>   </ItemGroup>
>   <ItemGroup>
>     <None Include="packages.config" />

And here is what actually happens:

43a44,54
>     <Reference Include="NUnit">
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net10\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net10\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net11\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net11\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net20\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net20\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net30\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net30\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net35\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net35\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net40\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net40\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net45\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net45\NUnit.dll</HintPath>
>       <HintPath Condition="Exists('..\..\packages\NUnit.3.4.1\lib\net46\NUnit.dll')">..\..\packages\NUnit.3.4.1\lib\net46\NUnit.dll</HintPath>
>       <Private>True</Private>
>     </Reference>

GitHub won't let me attach a zip file, so for now my repro case example is here: https://drive.google.com/open?id=0B6BLRF-3H7q1ckZtZTg3N3BzUEk - I'm using premake5 built from master/head with VS2015 on Windows 7.

(My next question after getting this working would be, how can I get a path into a NuGet package so I can invoke executables that are distributed that way?)

Also: Hi, Tom! :)

tvandijck commented 8 years ago

Hey Soleil... fancy that ;)

So the nuget "support" was recently added, and is clearly not sufficient for all use-cases... I ran into your number 1 issue myself with the CommandLineParser package. So it is definitely a work in progress and will require community support to get it up to snuff..

I noticed number 2 and 3 as well. Not familiar with 4.

As for the last question... I have no idea, I've never used nuget like that, doesn't the nuget package itself setup paths to include?

Unfortunately here at Blizzard we're not using premake for C# as much, and so haven't really had any incentive to work on that part of premake. My suggestion would be to make it do what you want it to do and send in a pull request here on Github. We can use all the help we can get. @aleksijuvani originally worked on this, maybe he has any ideas as well.

ghost commented 8 years ago

Unfortunately here at Blizzard we're not using premake for C# as much, and so haven't really had any incentive to work on that part of premake.

It's kind of the same thing for me, really. I haven't used C# for years. C++ support was a priority for me. I was kind of hoping for some people to test it before it got merged.

I did do some rudimentary testing, but it looks like there are a lot of discrepancies between NuGet for C++ and C# that I didn't account for.

ghost commented 8 years ago

2) The content of the generated packages.config file is correct, but it is placed next to the .sln file rather than the .csproj that actually has the dependency.

Does C# want a separate packages.config file for each project? For C++, even in multi-project solutions only a single packages.config is generated by Visual Studio.

3) The properties added to the .csproj are not correct; in addition to #1 above it adds dependencies for every .net framework version

That's not entirely accurate. It should only add a single reference, but with HintPaths for every .NET Framework version. From my testing, Visual Studio only uses the latest HintPath where the condition is true, so the reference should always point to the newest compatible version.

I know this doesn't match Visual Studio's output, but doing so is virtually impossible.

Visual Studio adds the closest .NET Framework version that the package supports as a reference. Premake would need to download the package first to see what .NET Framework versions it supports. Visual Studio already has this information because it has the package downloaded before adding the reference.

and fails to add the file include for packages.config. Because of this the package will never be downloaded and the build will fail.

I'm pretty sure that this worked back when I was testing this. C++ doesn't require the packages.config to be included in the project, so it might also be that I'm just remembering this wrong. In any case, it should be a quick fix.

1) There appear to be assumptions that a NuGet package contains only one relevant lib and that it has the same name as the package. These are not always the case, but as far as I can tell the .nupkg file doesn't actually tell you which files you want; it might actually be correct to just find and use all the DLLs in the appropriate subdirectory of the package.

This is problematic for the same reasons that _#_3 is problematic. We would need to download the whole package before we know what DLL files it contains. Surely there's a better way to do this?

Actually, I'd be totally cool with premake generating the nunit.config file next to the .sln as long as it allowed specifying extra package sources as well as a custom repository path.

I agree.

SoleilLapierre commented 8 years ago

doesn't the nuget package itself setup paths to include?

NuGet packages include a .nuget file which is just a zip containing information about the package, but from a casual inspection it doesn't look like it specifies which of the files are meaningful. Maybe it can't since that's a user-subjective determination to some extent.

Does C# want a separate packages.config file for each project?

Yes. I'm not sure it requires that, but in multi-project solutions where I've manually added different NuGet dependencies to different projects, it has generated one per project (note the project files were all in different directories though). I think it is possible to have a single packages.config and have each project include the same one by using the right paths, but it would be undesirable for every assembly generated by those projects to have the same references to the DLLs in the NuGet packages.

It should only add a single reference, but with HintPaths for every .NET Framework version.

You're right; I overlooked that distinction.

We would need to download the whole package before we know what DLL files it contains. Surely there's a better way to do this?

I would hope so.

I did do some rudimentary testing, but it looks like there are a lot of discrepancies between NuGet for C++ and C# that I didn't account for.

No worries! I'm really happy that you did the ground work for this.

OK, I will try to learn more about how to consume NuGet packages for C# projects, and tinker with premake locally. This will have to be a back burner project for now, so no promises about timing.

ghost commented 8 years ago

NuGet packages include a .nuget file which is just a zip containing information about the package, but from a casual inspection it doesn't look like it specifies which of the files are meaningful. Maybe it can't since that's a user-subjective determination to some extent.

C++ packages contain targets files that tell Visual Studio which binaries to link and what include directories to use. They can be used to specify preprocessor options and other flags as well. I don't think C# packages use targets files though. If they did, this would be trivial.

ghost commented 8 years ago

I did some digging and it looks like NuGet provides an undocumented API for retrieving info about packages. For C# packages, this includes the contents of the package. Take a look at this:

This would require an internet connection when generating the project files though. That might be an issue? If we document it, perhaps not?

tvandijck commented 8 years ago

Well... we do have http requests available through curl... simply call http.get("....") or http.download("..."). So it's not impossible for premake to access this, but it would definitely be the first use of it's kind in premake itself... so far http support has been an optional thing for premake, and people have opted for disabling it... in which case the nuget support would also have to be disabled, or give an error that it can't do it's work.

I have no objection... but not sure how other people think about it.. besides... you already need an internet connection to open the project, otherwise nuget wouldn't be able to get it's stuff.

tritao commented 8 years ago

What @tvandijck said. Internet access is needed to use NuGet itself anyway...

ghost commented 8 years ago

I've done some work on this (https://github.com/premake/premake-core/compare/master...aleksijuvani:nuget-fixes), but there's still some more work to be done before this works as well as I'd like it to:

At some point we should also:

I think I have some free time now, so I'm going to take a look at this stuff, and hopefully I'll be able to submit a pull request soon.

SoleilLapierre commented 8 years ago

Thanks for the update - I haven't had any time to look into this myself.

As for the custom package sources and path, an alternative would be to put those in a hand-authored NuGet.config in the same location as the generated solution. It would be nicer to have premake optionally generate that file though.

tritao commented 8 years ago

Seems like there will be a new syntax for NuGet project references in VS project files.

See the "NuGet Package references" section at https://blogs.msdn.microsoft.com/dotnet/2016/10/19/net-core-tooling-in-visual-studio-15/.

ghost commented 8 years ago

Interesting. That will definitely make things a lot easier when generating project files for Visual Studio 15. I wonder if they've done anything similar with the C++ NuGet. Do we have support in Premake for Visual Studio 15 yet?

tvandijck commented 8 years ago

Do we have support in Premake for Visual Studio 15 yet?

no, but that's like a copy paste of one file to get it going, it's more that supporting the specific details of VS'15 would be more work.... same for 2015 and the other versions, we keep encountering this minute details and differences...

SoleilLapierre commented 7 years ago

Just a note for anyone else who may be encountering this issue and need a quick fix: I've worked around it fairly easily by using Paket (https://fsprojects.github.io/Paket/index.html) to manage NuGet dependencies as a post-process after Premake.

In short, I use premake to generate projects and solutions without any NuGet references, then Paket to add the references to the generated projects. It's not perfect because Paket doesn't let me specify where the packages get downloaded to, and the references are not visible in VS2015's built-in NuGet package manager.

tvandijck commented 7 years ago

Just a note as well.... Starting with VS2017, this has become a lot easier, since a nuget reference is just a

<PackageReference include="somename" Version="version" />

http://blog.nuget.org/20170316/NuGet-now-fully-integrated-into-MSBuild.html

No longer a need to look at any kind of metadata, since msbuild/visual studio already does all that work.

So... I wonder if we should actually go through the effort of fixing this for VS2015, or rather just say.... VS2015 premake has no nuget support, we'll support it in VS2017+

And then we just need someone to actually fix the VS2017 target to correctly create these PackageReferences, and make sure we give an error for VS2015, but other then that this seems the easier road to getting this to work.

ghost commented 7 years ago

I've been thinking about this as well.

I'm on the fence about it. It wouldn't be that much work to fix it, but I don't like the idea of having to add a JSON parser to Premake just to parse NuGet API responses (or project generation requiring an internet connection for that matter).

In any case, the C++ NuGet support for VS2015 is working fine (as far as I can tell), so ideally we would leave that in. It does have the minor gripe of having to specify the exact package version (e.g. 1.2 won't work, will have to specify 1.2.0.0 etc.)

tritao commented 7 years ago

I think having a JSON parser inside Premake would be pretty nice for other things such as consuming web APIs, in fact I tried it some months ago using a binding for a C JSON library.

We want to use Nuget Premake support in our projects but had these issues. We're thinking of dropping VS2015 support and depending on VS2017 because of this, but would be nice to support VS2015 too if it's not too much work.

tvandijck commented 7 years ago

We just added: http://regex.info/blog/lua/json

it's a single pure lua json parser: https://github.com/Blizzard/premake-core/blob/master/modules/blizzard/json.lua

It's not the fastest in the world I guess, but sufficient for most of our cases, and we indeed use it for parsing web api calls.

ghost commented 7 years ago

If we included a JSON library in Premake for this purpose, should we also expose it to end-users as an API (such as json.decode and json.encode)?

tvandijck commented 7 years ago

well... the one I mentioned above is just pure lua... so if we loaded that as per it's example in the link, then there would be a JSON object in _G, which you can just call using:

local lua_value = JSON:decode(raw_json_text) -- decode example

local raw_json_text    = JSON:encode(lua_table_or_value)        -- encode example
local pretty_json_text = JSON:encode_pretty(lua_table_or_value) -- "pretty printed" version
tristam92 commented 1 year ago

So, about this issue. It's still a thing, do you guys have a plans to fix it, or it will marked as will not fix?

tristam92 commented 1 year ago

From my understanding, there is a potential to even generate NuGet,config file, however it's tied to prj object, which in reality is an error. NuGet.config according to https://learn.microsoft.com/en-us/nuget/consume-packages/configuring-nuget-behavior should be solution/workspace level file. So it's generation in premake done incorrectly right now. Another thing that can be improved in that way is addition of param to workspace, which will change default location of packages, with NuGet.config. And this location can be then used to generate proper path in prj generation for C++/C#.