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 for ClientKit #2

Open
JeroMiya opened this issue Apr 5, 2015 · 9 comments
Open

NuGet package for ClientKit #2

JeroMiya opened this issue Apr 5, 2015 · 9 comments

Comments

@JeroMiya
Copy link
Contributor

JeroMiya commented Apr 5, 2015

From a request in this issue:
OSVR/OSVR-Unity#6

Task:

  • Create a NuGet package for Managed-OSVR.

Open questions:

  • What to name it? Perhaps "OSVR.ClientKit"?
  • Does the NuGet package include the OSVR-Core native binaries, or should those be distributed in a separate NuGet package?
  • Do we need post-install scripts to get the native dlls added to the target project as content so that they are copied to the output directory correctly?
@JeroMiya
Copy link
Contributor Author

JeroMiya commented Apr 5, 2015

OK, it's a bit late, and I'm still a little woozy from some flu medicine, but I made a couple of failed attempts at this. Documenting:

Attempt 1: From ClientKit.csproj:

Results:

  • Only the .net 2.0 framework version of the managed dll was packaged.
  • No native dlls were packaged.

Attempt 2: Copying files into convention-based directory structure

  • Using the conventions documented here: https://docs.nuget.org/Create/Enforced-Package-Conventions copied files into a new /test folder.
  • Copied ClientKit.nuspec from attempt 1 above into /test.
  • Filled in all of the token values in ClientKit.nuspec with static values (because otherwise it complains that it has no values for them).
  • Put files from /build/bin/net20/Release (excluding sample app binaries) into /test/lib/net20, including a subfolder /test/lib/net20/x64 (probably not the right thing to do, but we'll go with it for now and see what happens).
  • Put files from /build/bin/net45/Release (excluding sample app binaries) into /test/lib/net45, again including the x64 subfolder.
  • nuget pack from /test as CWD.

Results:

Package was created successfully without warnings/errors. When opening up the nupkg all the binaries for net20 and net45 seemed to be there. However, this is the error I got when attempting to install the package into a .net 4.5 project:

Installing 'OSVR.ClientKit 0.1.5570.40638'.
Successfully installed 'OSVR.ClientKit 0.1.5570.40638'.
Adding 'OSVR.ClientKit 0.1.5570.40638' to ConsoleApplication1.
Uninstalling 'OSVR.ClientKit 0.1.5570.40638'.
Successfully uninstalled 'OSVR.ClientKit 0.1.5570.40638'.
Install failed. Rolling back...
Failed to add reference to 'msvcp120'.

Next steps:

  • research packaging of native DLLs with NuGet packages for best practices/etc... This seems to be where it's breaking down.
  • Write a batch file that 1) builds net20 and net45 release binaries, 2) copies all files into /test in their proper place (probably name it something other than /test), and 3) packs the nupkg file.

@rpavlik
Copy link
Member

rpavlik commented Apr 6, 2015

Thanks for looking into this - I was hoping you would.

Not sure how cross-platform-usable msbuild files are, but the main root one (that is an alternative to using the .sln file) is where I anticipated a "make a NuGet-convention directory structure" target/script would go. The other item is that we could make a target in that file that overrides properties so that it builds directly into the correct directory structure.

The 'msvcp120' and the other similarly named file is just the VS2013 C/C++ runtime DLLs - we bundled it with the native library to keep people from needing to install the redist, but it can be left out and just simply state "hey, you have to have the redist". That might be all it takes to get that package working properly

Note that the line here: https://github.com/OSVR/Managed-OSVR/blob/master/ClientKit/ClientKit.cs#L111 and the one a bit later for 32-bit gives some flexibility in where the native libraries are w.r.t. the managed assembly.

@JeroMiya
Copy link
Contributor Author

JeroMiya commented Apr 8, 2015

OK, I have this working:
JeroMiya@bb76dc0

I was able to install this NuGet package locally into a console application project and copy any of the command line sample code into the new project and they run. I'm basing my implementation on how libgit2 packages their native library, except I'm not separating the managed and native assemblies into two NuGet packages. Not sure why we'd need to do that.

I do need some things from the OSVR team:

The metadata is specified in the NuGetPackaging/OSVR.ClientKit.nuspec file.

For reference:
libgit2/libgit2sharp#974

@JeroMiya
Copy link
Contributor Author

JeroMiya commented Apr 8, 2015

FYI: I have that section of NuGetPackaging/build/OSVR.ClientKit.props commented out, but this method would hypothetically allow us to distribute native binaries for Mac/Linux on mono as well as for Windows.

@rpavlik
Copy link
Member

rpavlik commented Apr 8, 2015

Thanks! I do think eventually we would want to separate out the native binaries, for either convenience (not having to rev the managed OSVR package with every revision of the native binaries) and/or NuGet for C++ (which is even more of a mysterious unicorn to me but sounded like a good idea when I read about it in some MSVC blog).

Will add a few commit comments inline, but go ahead and open a pull request, and/or let me know when you're ready for me to merge that.

@JeroMiya
Copy link
Contributor Author

JeroMiya commented Apr 9, 2015

I'll look into updating the metadata per your line notes and submit a pull request this evening.

As for the icon, 32x32 will do, but I see most packages are using 64x64. I think it shows 32x32 in the Visual Studio UI but 64x64 in the web site listing on nuget.org. Also, the one you pointed to will do for now, but you may want to copy it somewhere and name it something with 'nuget' in the name. Otherwise if you're editing that site you may not know that the icon is being used for the NuGet package and move/delete/change it by mistake.

@rpavlik
Copy link
Member

rpavlik commented Apr 9, 2015

So the NuGet docs said 32x32, which is why I did that. I've since committed a 64x64 to the repo and used rawgit.com to generate a stable, CDN-backed URL for it.

Thanks!

@JeroMiya
Copy link
Contributor Author

Hmm, I was basing it off of some examples from NuGet.org. Apparently the spec says 32x32, but if the image is larger the UI is supposed to scale down to 32x32. Meanwhile the NuGet.org website itself displays it at several resolutions, even up to 128x128:
https://nuget.codeplex.com/workitem/2453

JSON.Net uses a 64x64 image:
http://www.newtonsoft.com/content/images/nugeticon.png

@JeroMiya
Copy link
Contributor Author

Pull request made: #4

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

No branches or pull requests

2 participants