NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

Include restoring of symbols in dotnet restore #9667

Open tmat opened 4 years ago

tmat commented 4 years ago

dotnet restore currently does not have a switch that would restore PDBs published on symbol servers (e.g. via snupkg) to nuget package cache. There are at least two scenarios for which such feature would be valuable:

1) Restoring local packages

Scenario steps:

2) Offline development

Scenario steps:

The above scenarios should work regardless of whether PDBs are included in nupkg or in a separate snupkg.

Blockers

Fixing https://github.com/dotnet/sdk/issues/1458: “New project system doesn't copy PDBs from packages” is a prerequisite for the above scenarios, but it only works if the symbols are in the nupkg. This fix would make sure that when the PDB is in the package cache next to the dll it will get copied to the output directory, where the debugger can find it.

Proposal

Add --symbols:[all|local] switch to dotnet restore with default value being local.

This would turn on symbol restore from local directories (local) and symbol servers (all = local + servers). The list of symbol servers would be listed in nuget.config. By default if nuget.org feed is listed nuget would also include nuget.org symbol server.

When restoring packages from a local feed NuGet would check if there is a snupkg next to each restored nupkg. If so it should restore the package to the same directory it restored the corresponding nupkg (effectively merging the content of both packages). Since snupkg directory layout mirrors the corresponding nupkg the result would the same layout in the nuget cache as it would be if the nupkg contained the PDBs.

When restoring packages from remote feeds and --symbols:all is specified, nuget would pull PDBs from the symbol server. The restore operation would read the debug directory entry of each DLL included in the restored package that doesn't have PDB next to it already and query the configured symbol servers for the corresponding PDB using Simple Symbol Query Protocol. It would then store the PDB next to the dll in the package cache. Again the result would be the same as if the package contained the PDB.

Fetching sources for scenario [2] (outside of nuget scope)

Symbols are required for debugging dependencies but not sufficient. To get the scenario [2] working end-to-end another tool would be needed that would fetch the sources from the repository the package was built from (this information is available in nuspec repository metadata) and store them locally. In addition, the debugger would need to be able to redirect Source Link links to the local clone. The debugger does not have such capability today. Perhaps this could be implemented by running a local HTTP proxy.

Further possible improvements (lower priority)

Some scenarios, like crash dump debugging require PDBs to be present in the local symbol cache. The debuggers are not aware of NuGet cache and thus having PDBs only in the nuget cache isn't sufficient. It would however be possible for dotnet restore --symbols to also link the downloaded PDBs to debugger cache to make this scenario work. If an additional argument is passed, say dotnet restore --symbols --debug-cache <path> restore would link the downloaded PDBs to the specified debugger cache directory.

tmat commented 4 years ago

@clairernovotny @davidfowl @rrelyea @anangaur @jinujoseph @chuckries

loic-sharma commented 4 years ago

The spec Publishing and Consuming Symbols and Source for Debugging mentions:

To enable debugging offline, when the symbol server is not available, we propose to develop a tool (msbuild task and dotnet tool) that downloads symbols from the symbol server for all binaries a given project depends on. This tool would enumerate all dependencies of the project similarly to dotnet restore. It would download symbols to the local symbol cache where debuggers can find them.

That approach seems less intrusive than adding functionality to dotnet restore. Could you elaborate why you'd prefer adding this to dotnet restore instead?

When restoring packages from a local feed NuGet would check if there is a snupkg next to each restored nupkg. If so it should restore the package to the same directory it restored the corresponding nupkg (effectively merging the content of both packages).

From my understanding, we don't support publishing .snupkg files to a local folder feed. This would likely be another blocking prerequisite. Please let me know if I misunderstood something here though :)

tmat commented 4 years ago

hat approach seems less intrusive than adding functionality to dotnet restore. Could you elaborate why you'd prefer adding this to dotnet restore instead?

I'm not sure why would it be intrusive. It's an optional parameter, the default behavior doesn't change. Another command/tool would work too, but it would likely need to reimplement some of the logic that restore does and would also require devs to know about another thing.

From my understanding, we don't support publishing .snupkg files to a local folder feed

Isn't it just copying the snupkg file to a directory?

loic-sharma commented 4 years ago

I'm not sure why would it be intrusive. It's an optional parameter, the default behavior doesn't change.

Maybe intrusive was the wrong word. I agree that your proposal wouldn't break the existing customer experience. However, I'm concerned that this proposal may "complicate" NuGet restore into being an even more complex beast. I would push to keep this functionality as a separate tool unless there's strong value in making this a first-class NuGet experience.

... but it would likely need to reimplement some of the logic that restore does and would also require devs to know about another thing.

From my understanding, this tool could be implemented without needing restore logic:

  1. One could read the project.assets.json file in the obj folder, or...
  2. One could enumerate the assemblies in the output directory and download any missing symbols
tmat commented 4 years ago

Reading the info from project.assets.json would work. The tool needs to work with and understand packages, since we want to store the PDBs in the package cache. This is so that they don't need to be downloaded every time the build output directory is cleaned.

So yes, it could be a tool that runs after restore and uses restore's outputs as its inputs. Still, I believe it would be more convenient for the user if this operation was part of restore (definitely the implementation could be architecturally isolated from the rest of the restore to not make restore a complex monolith).

As proposed the default call to dotnet restore would copy PDBs from snupkgs published to local feeds as it it cheap to do so. --symbols:all would only be needed to download symbols from symbol server.

JonDouglas commented 3 years ago

We may be able to consider this in .NET 6, so I'm going to link it to https://github.com/NuGet/Home/issues/9783 for now given the pre-req is included for now.

akovac35 commented 3 years ago

Speaking from production experience: we have a growing number of internal nugets. Using dotnet restore does not load symbols from our internal package server, so the reslease builds do not contain line numbers when exceptions are thrown from those packages.

So there exists a very good reason for adding this function as a switch.

EDIT: for anyone reading this, a possible solution regarding exception line numbers for custom packages is to simply build them with symbols in the package itself:

<PropertyGroup>    
  <DebugSymbols>true</DebugSymbols>
  <DebugType>embedded</DebugType>

    <!-- Only enable the following if the line numbers mismatch -->
    <!--<Optimize>false</Optimize>-->

    <!--
      Additional properties which may impact how printed line numbers match the source code line numbers are listed here:
      https://docs.microsoft.com/en-us/dotnet/core/run-time-config/compilation
    -->
</PropertyGroup>
loic-sharma commented 3 years ago

I created a hacky prototype that hooks into your build and tries to download missing symbols: https://github.com/loic-sharma/symbols

yufeih commented 1 year ago

Add another use cases here: documentation tools such as docfx can use source links in PDB files to generate clickable API links to 3rd party dependency's source code hosted on GitHub. Restoring, locating and handling PDB package formats (embedded or .snupkg) could be done in the dotnet restore command, allow other tools to easily consume PDB files and source link map by checking the side by side PDB file.