-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Basic NuGet package investigation #420
base: master
Are you sure you want to change the base?
Conversation
Might be good to get a TODO list going for this - from what I can remember in Discord:
Glad to see the debugging experience is there though! |
3edd08e
to
5f61cee
Compare
@@ -1,7 +1,6 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
<PropertyGroup> | |||
<TargetFrameworks>netstandard2.0;net7.0</TargetFrameworks> | |||
<Platforms>x64</Platforms> | |||
<TargetFrameworks>net40;netstandard2.0;net7.0</TargetFrameworks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this will break due to the reasons listed here: #355 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to work fine for me... maybe it was a fluke? SDL2# is set up the same way https://github.com/flibitijibibo/SDL2-CS/blob/master/SDL2-CS.Core.csproj#L3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VS 2022 and OmniSharp are forcing our hand here, since Intellisense has a very bad time when referencing projects that target both Framework and Core.
@flibitijibibo Is there any way to repro the issue you saw? I'd be curious to follow up the root cause issue of this
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change cosmetic or is it required for NuGet to work? Similar to GitHub's preview I'm okay with the plaintext formatting here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filename in the package has to end with .md
or NuGet will reject it. If you really like the plaintext preview on GitHub, I suppose I can make a separate README.md
file that'll be used for the package.
5f61cee
to
cd03fb9
Compare
cd03fb9
to
597f83f
Compare
<PropertyGroup> | ||
<IncludeSymbols>true</IncludeSymbols> | ||
<SymbolPackageFormat>snupkg</SymbolPackageFormat> | ||
<EmbedUntrackedSources>true</EmbedUntrackedSources> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way to use the source is using the GitHub source link:
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
see documentation. This does a package reference to the project though, but it's how you can point to the GitHub code directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been using the .NET 8 SDK, which imports SourceLink automatically. You're right, though, that the package is needed for earlier SDKs, and the GitHub Actions images probably won't have .NET 8 for a while.
All EmbedUntrackedSources
does here is embed AssemblyInfo.g.cs
(and probably some other equally meaningless files), I can probably remove it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's pretty cool! I wasn't sure if .NET 8 SDK pulled the GitHub Source Link but that does seem to be the case. EmbedUntrackedSources
is probably okay here.
the GitHub Actions images probably won't have .NET 8 for a while
I think we can just specify the .NET 8 SDK in the GitHub Actions (8.0.x
should pull the RC1 version).
Hey @reflectronic, what's the status of this? What needs to be done to push it forward? I think having FNA via Nuget which just works is amazing. No need to clone a repo, clone submodules and either download pre-compiled libraries from a "random" link or compiling/installing them on my own. Similarly for an update. |
The PR has bitrotted somewhat—some minor tweaks would be required to get it to build again. Otherwise, I believe it's functionally complete: the package it builds contains FNA.dll, debugging symbols, and native libraries. And, of course, running and debugging FNA works in my (limited) testing. I think what's next is testing it with larger projects to make sure things work as expected. And, of course, maintainers need to make a call on whether they are interested in supporting this or not. |
This PR is meant to investigate and discuss what a NuGet package for FNA would look like. I figured it would be most helpful to put up something concrete, so we can see how it works in practice and so on.
The below investigation shows what the development experience might look like with the FNA package, pending availability of the .NET 8 SDK (which has changed a couple of these behaviors).
Build experience
Add the FNA package to your project:
For a portable build (
dotnet build
), the build output looks like:The auto-generated
Game.deps.json
file contains a relative path to each native library, keyed by platform. This information is used by the runtime when resolving P/Invokes, allowing these build outputs to run unmodified on any supported operating system.For a platform-specific build (e.g.
dotnet build -r win-x64
), the build output is more traditional-looking:Need to investigate on Mac (I haven't handled the MoltenVK setup yet).
.NET Framework
When using a legacy-style project file with .NET Framework, native libraries and FNA.dll.config are copied next to the executable based on the platform target. If you target AnyCPU, no native libraries are copied. I considered adding a warning about this, but I think it's an easy enough issue to diagnose.
When using an SDK-style project file with .NET Framework, the behavior is similar. Though, naturally, there's a minor regression: when the platform target is AnyCPU, win-x86 binaries will be copied to the build output. (Prefer32Bit is not enabled by default, so the program will probably crash on startup.) This can be fixed by setting up solution configurations or using
<UseCurrentRuntimeIdentifier>true</UseCurrentRuntimeIdentifier>
, so this is probably fine. (This could also fix itself in a future version of the .NET SDK.)When using packages.config (the "old way" of using NuGet), native libraries are not copied at all (it does not understand the
runtimes
folder). The easiest solution is probably just to advise against using packages.config.Native AOT and Single File
Both still function as they did before. We should consider packaging an Rd.xml with the NuGet package so that loading content works out-of-the-box. (Though, my understanding is that Rd.xml is not officially supported with NativeAOT and may be removed entirely in the future, so we may want to look into alternative ways of preserving the needed code.)
The fnalibs package doesn't come with import libraries, so DirectPInvoke can't be used on Windows without building fnalibs from source. Unless it's hard for you to get them with your toolchain, it would be nice to just have import libraries along with the prebuilt binaries.
Unfortunately,
ExcludeFromSingleFile
doesn't work onPackageReference
s. It's possible to hack something together that will exclude FNA.dll anyway, but this really should just work. I will look into what it takes to support this in the .NET SDK.Miscellaneous
To copy FNA.dll to build output, but not fnalibs:
To copy fnalibs to build output, but not FNA.dll:
Debug experience
NuGet supports providing symbols for packages, either by:
.NET also supports acquiring sources for external libraries while debugging, either by:
Right now the build is configured to use Source Link with a symbol package. (The .NET SDK automatically detects version control information to populate Source Link and package metadata, again pending availability of the .NET 8 SDK.) However, we can use any other combination if that's desired.
In any case, Visual Studio is able to find/automatically download symbols and source when debugging.
(Note that the full path of ContentManager.cs here points to a temporary file, downloaded by the VS debugger from github.com, rather than my local copy of FNA.)
Unfortunately, there's no way for the build to download the symbols and copy them to the build output. In addition, Native AOT doesn't support using source link in the unmanaged CodeView/DWARF debug info. Both are possible for the .NET SDK to implement in the future, and both can be worked around using embedded PDBs (at the cost of much larger assembly sizes, which would make their way into published games).
Building the packages
To create
FNA.nupkg
: