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

Include restoring of symbols in dotnet restore #9667

Open
Tracked by #10846
tmat opened this issue Jun 10, 2020 · 9 comments
Open
Tracked by #10846

Include restoring of symbols in dotnet restore #9667

tmat opened this issue Jun 10, 2020 · 9 comments

Comments

@tmat
Copy link

tmat commented Jun 10, 2020

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:

  • Clone repository that has Source Link enabled on one machine, make changes, push the changes to source server, build and pack.
  • Copy the resulting packages over to another machine.
  • On this machine restore a project that depends on these packages, has local directory as a feed in its nuget.config.
  • F5 in VS/VS Code and able to step into the source code of the package.
  1. Offline development

Scenario steps:

  • Clone repository and restore packages.
  • Disconnect from the network.
  • Build and debug your application and its dependencies.

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

Blockers

Fixing dotnet/sdk#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
Copy link
Author

tmat commented Jun 10, 2020

@loic-sharma
Copy link
Contributor

loic-sharma commented Jun 23, 2020

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
Copy link
Author

tmat commented Jun 23, 2020

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
Copy link
Contributor

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
Copy link
Author

tmat commented Jun 24, 2020

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
Copy link
Contributor

We may be able to consider this in .NET 6, so I'm going to link it to #9783 for now given the pre-req is included for now.

@akovac35
Copy link

akovac35 commented Mar 12, 2021

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
Copy link
Contributor

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

@yufeih
Copy link

yufeih commented Mar 25, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants