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

API / endpoint for snupkg's #7912

Open
clairernovotny opened this issue Mar 24, 2020 · 7 comments
Open

API / endpoint for snupkg's #7912

clairernovotny opened this issue Mar 24, 2020 · 7 comments

Comments

@clairernovotny
Copy link

@clairernovotny clairernovotny commented Mar 24, 2020

We need an API / metadata on the package that points to a snupkg, if the package has one.

The scenario is that tooling may need to download all symbols for a NuGet package as part of validation steps, such as ensuring the Source Link information is properly embedded.

Currently the only option is to use the CDN endpoint, and check with a HEAD: https://globalcdn.nuget.org/symbol-packages/{lowercase ID}.{lowercase normalized version}.snupkg

There's no guarantees around this endpoint, however.

Downloading each PDB via the symbol server API is less efficient as each pdb needs to be calculated and downloaded, if published, individually. For validation scenarios, all pdb's would be required anyway.

--

There's another scenario where people find it generally useful to see what's in the snupkg package, downloading and opening with a tool like NuGet Package Explorer.

--

NuGet Package Explorer intends on helping people know if packages are properly Source Linked and thus needs to perform all these steps: NuGetPackageExplorer/NuGetPackageExplorer#904

@loic-sharma

This comment has been minimized.

Copy link
Member

@loic-sharma loic-sharma commented Mar 27, 2020

This is a great idea! I would love to have a similar "debuggable" indicator on nuget.org and Visual Studio. I think at the bare minimum we could add a documented service index entry for the .snupkg resource.

Downloading each PDB via the symbol server API is less efficient as each pdb needs to be calculated and downloaded, if published, individually.

Even with the .snupkg API, wouldn't you need to do this regardless? A package may have .DLLs whose symbols are on the Microsoft symbol server but not in its corresponding symbol package.

/cc @cristinamanum who worked on the symbols feature.

@clairernovotny

This comment has been minimized.

Copy link
Author

@clairernovotny clairernovotny commented Mar 27, 2020

I think it's important to define what dll's are relevant to be validated. For now, in NPE, I'm just checking ones in lib and runtimes. I'm not validating files in tools, etc. Therefore, I expect all the files to be present in the snupkg and not need to check any symbol servers.

I've just released 5.6.18 of NPE with support for Source Link validation. I only support snupkg checking if the package came from NuGet.org and I think that's a huge issue and limitation.

@loic-sharma

This comment has been minimized.

Copy link
Member

@loic-sharma loic-sharma commented Mar 27, 2020

I only support snupkg checking if the package came from NuGet.org and I think that's a huge issue and limitation.

Yup that's very fair.

I've just released 5.6.18 of NPE

I just played around with the latest version, it works like a charm! :)

I think it's important to define what dll's are relevant to be validated. For now, in NPE, I'm just checking ones in lib and runtimes. I'm not validating files in tools, etc. Therefore, I expect all the files to be present in the snupkg and not need to check any symbol servers.

Even with that lib and runtimes folder limitation, I think you would still need to fallback to symbol servers in certain scenarios. For example, Microsoft.Extensions.Logging.Abstractions has published its symbols on the Microsoft symbol server and does not have a corresponding .snupkg on nuget.org.

image

@clairernovotny

This comment has been minimized.

Copy link
Author

@clairernovotny clairernovotny commented Mar 27, 2020

Good catch re Microsoft.Extensions.Logging.Abstractions. I was really hoping to avoid poling symbol servers as that's a whole other game.

@loic-sharma

This comment has been minimized.

Copy link
Member

@loic-sharma loic-sharma commented Mar 27, 2020

I was really hoping to avoid poling symbol servers as that's a whole other game.

I've found this repository invaluable: https://github.com/dotnet/symstore

I took a crack at this problem a while back when I was trying to prototype a similar feature. Maybe it might help! https://github.com/loic-sharma/NuGet.Quality/blob/6fcb1d2e47568a721d1d6fdf62b161c22ab2ad8a/Program.cs#L162-L188

@clairernovotny

This comment has been minimized.

Copy link
Author

@clairernovotny clairernovotny commented Mar 27, 2020

@loic-sharma I just published 5.6.23, which checks the Microsoft symbol server for Microsoft files. It fixes this.

I saw your TODO about ensuring symbols match the PE file... any leads on that?

@loic-sharma

This comment has been minimized.

Copy link
Member

@loic-sharma loic-sharma commented Mar 27, 2020

I saw your TODO about ensuring symbols match the PE file... any leads on that?

Yes nuget.org does this validation at .snupkg ingestion. The steps are:

  1. Get the checksum entries from the DLL (see this)
  2. Find a PDB that corresponds to the DLL
  3. Check that PDB's hash matches at least one checksum entry from the DLL (see this).
    1. For each checksum entry from DLL:
      1. Grab the algorithm name from the checksum entry
      2. Calculate the PDB's content hash using that hash algorithm name (see this)
      3. If that PDB content hash matches a checksum entry, pass! (see this) 😀
    2. If no PDB content hash matches the checksum entry, fail! 😠

Please let me know if you have any questions

EDIT: By the way, I believe this checksum entry stuff only works with portable PDBs. I believe DLLs don't have checksum entries for Windows PDBs.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.