-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Publish Single-file: Smart settings for IncludeSymbolsInSingleFile #3685
Comments
CC: @Viir |
CC: @dasMulli |
@lpereira In .net 5, will the apphost validate the dependencies listed in the deps.json file at startup? CC: @vitek-karas |
I'm still not sure if the pdb in the deps.json isn't a bug that should be fixed in the first place. Though this likely needs Nuget involvement.. @nguerrera @dsplaisted thoughts? |
I think it is a bug. We have related discussion about being able to query for pds so that we can copy them. It's ironic that we copy them for native, but not managed. Feels like there's an exclusion of pdbs only in managed case. |
I believe that technically it's a bug to put a pdb in the native files of a package. NuGet interprets anything in the folder as a native file, which is how it flows through to the deps.json. It seems like it is natural to put pdbs in the native folder though, so if we want to support that we could update the NuGet convention, or apply a PDB convention on top of it in the SDK. |
I think it is perfectly natural for authors to include artefacts that help for debugging or error tracking. For managed code, I usually suggest deploying PDBs with everything to get "good" stack traces. Tools like Sentry even help tracking down the GIT commit that caused error to occur based on stack trace symbolificatiton. But "native" really means native assets so that may also mean runtime-specific scripts and assets.. |
To double down: Does |
Great points. At the end of the day, we have to uniformly decide if something is a symbol file or not. If we decide it is not a symbol file, but an arbitrary native asset, then it should be in deps.json AND deployed irrespective of IncludeSymbolsInSingleFile. And if we decide it is a symbol file, then it should not be in deps.json AND included in single file if and only if IncludeSymbolsInSingleFile is true. NuGet and the assets files should be the source of truth for what files in package are what kind of thing. If we decide to use extension heuristics for this, then they should go in nuget. SDK should respect what NuGet tells us. I will combine this issue and the issue of not copying managed PDBs into one feature request for nuget. |
I've been thinking about this, and I might open the deps in apphost so the validation is performed right at the beginning, but keep them open -- the runtime can then just use the open file descriptors instead. In a way, the check now becomes more robust while still improving the performance, by cutting a lot of unneeded stat(2) calls. (Need to sleep on this for a little while longer, though.) |
I believe this is related to #1458 |
With the previous version, got this error when trying to run elm-fullstack.exe: ---- Error: An assembly specified in the application dependencies manifest (elm-fullstack.deps.json) was not found: package: 'LibGit2Sharp.NativeBinaries', version: '2.0.306' path: 'runtimes/win-x64/native/git2-106a5f2.pdb' For context, see: + libgit2/libgit2sharp#1754 (comment) + dotnet/sdk#3685
…build 20191122.1 (#3685) - Microsoft.NET.Sdk.Razor - 3.0.2-servicing.19572.1
The single-file build functionality in .NET 5 did not work well with earlier versions of libgit2sharp. Let's see if our build works with this version and .NET 5 or .NET 6. For discussion of the errors we got when trying .NET 5 with earlier versions of libgit2sharp, see libgit2/libgit2sharp#1857, libgit2/libgit2sharp.nativebinaries#111, dotnet/sdk#3685, and dotnet/runtime#36590
The single-file build functionality in .NET 5 did not work well with earlier versions of libgit2sharp. Let's see if our build works with this version and .NET 5 or .NET 6. For discussion of the errors we got when trying .NET 5 with earlier versions of libgit2sharp, see libgit2/libgit2sharp#1857, libgit2/libgit2sharp.nativebinaries#111, libgit2/libgit2sharp.nativebinaries#119, dotnet/sdk#3685, and dotnet/runtime#36590
The single-file build functionality in .NET 5 did not work well with earlier versions of libgit2sharp. Let's see if our build works with this version and .NET 5 or .NET 6. For discussion of the errors we got when trying .NET 5 with earlier versions of libgit2sharp, see libgit2/libgit2sharp#1857, libgit2/libgit2sharp.nativebinaries#111, libgit2/libgit2sharp.nativebinaries#119, dotnet/sdk#3685, and dotnet/runtime#36590
Marking completed as we now have a way of including the pdb in the nuget package. Reopen if that's not sufficient for this. |
When publishing an app as a single file, the default setting is to not include PDB files in the output bundle by default (and instead leave them as separate files).
However, this setting is unsuitable for some apps, where the
deps.json
file includes a dependency on the PDB file. This causes runtime-crash of the app when theapphost
validates the contents against the deps.json manifest. For example:PowerShell/PowerShell#10266
https://github.com/Viir/bots/tree/adapt-for-single-file-exe-publish
The inclusion of PDB files as a dependency is almost always an error, but an app (and its dependencies/nuget packages) are not required to associate the same semantics with file-extensions.
So, if the deps.json writer finds a PDB file in the manifest, the SDK should default the
IncludeSymbolsInSingleFile
setting to true. The SDK should also issue a warning about this change, so that PDBs are not silently included -- unintentionally increasing the size of published apps.The text was updated successfully, but these errors were encountered: