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

Proposal of snupkg restore functionality and debug improvement #10899

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

BlackGad
Copy link
Contributor

@BlackGad BlackGad commented May 27, 2021

Design for #10793

@JonDouglas
Copy link
Contributor

JonDouglas commented Jun 1, 2021

Thank you so much for the contribution @BlackGad! We will keep this proposal open for the next couple of weeks to gather upvotes and feedback from the community. We'll then review it as a team and get back to you shortly. Thanks again in the meantime!

If you're seeing this message, please 👍 or provide your feedback on this proposal in this PR as to why we should or shouldn't do this.

Thank you everyone!

@BlackGad
Copy link
Contributor Author

BlackGad commented Jun 1, 2021

Is it reasonable to advertise this PR for community? Maybe you can invite some via the twitter?

@JonDouglas
Copy link
Contributor

@BlackGad I literally just tweeted a minute before your comment :) https://twitter.com/_JonDouglas/status/1399788019851407374

@BlackGad
Copy link
Contributor Author

Well :) Is it time to continue or no?

@JonDouglas
Copy link
Contributor

@BlackGad I have scheduled a review for this week & we will update this post shortly! Sorry for the delay as we're slightly behind this time of year.

@BlackGad
Copy link
Contributor Author

@BlackGad I have scheduled a review for this week & we will update this post shortly! Sorry for the delay as we're slightly behind this time of year.

Nice to hear this. If you need any explanations or details fill free to ask in any form

@BlackGad
Copy link
Contributor Author

Did you manage to do a review?

@BlackGad
Copy link
Contributor Author

BlackGad commented Jul 6, 2021

@JonDouglas congratulations once again! :) When you will be able to review this proposal?

@JonDouglas
Copy link
Contributor

@loic-sharma will you be able to help drive this forward and share thoughts about the review with @BlackGad while I'm out?

@BlackGad
Copy link
Contributor Author

BlackGad commented Jul 8, 2021

@JonDouglas , @loic-sharma guys?

@BlackGad
Copy link
Contributor Author

BlackGad commented Jul 12, 2021

@JonDouglas traditional Monday ping.

@loic-sharma
Copy link
Contributor

loic-sharma commented Jul 17, 2021

Hello, apologies for the late response. Thank you for creating this proposal! I definitely agree that our .NET debugging story is incomplete and needs work 😅

I'm not sure I fully understand your scenario just yet. Could you explain why you need to manually fetch your symbols instead of letting the debugger get them for you? Is it for improved exceptions at runtime?

I'm also curious as to the benefits of adding "symbol gathering" to NuGet restore. If we provided a separate standalone tool, would that work for you?

@BlackGad
Copy link
Contributor Author

Details described in issue but there is a brief overview

Debugging

  1. Debugging is not easy thing :)
  2. Currently to manage debugging - build tools generates additional symbols near the generic output for future hints for the debug tool
  3. Common symbols discovery mechanism for the MS libraries(applications) described here

Abstract example

As far as I understand you are Web developer so will try to explain in this domain (not accurate example I must say).
To present STATIC web page for the user developer must:

Web example diagram

web_example

  1. Create web page
    • Html with DOM structure
    • Appropriate css styling
  2. Deliver to client side
    • Pack html to some kind of blob (1 on diagram)
    • Pack css to some kind of blob (2 on diagram)
    • Transfer to the client side
    • Unpack html blob (3 on diagram)
    • Unpack css blob (4 on diagram)
  3. Display web page (5 on diagram)
    • Make sure client application understand how to discover html
    • Make sure client application understand how to discover css
    • Make sure client application understand how to link them and display

Blobs - byte array
Client application - some kind of web engine (Browser magic box)

Generic application development

Now lets change labels on the diagram

dev_example

  1. Create application
    • Application binaries
    • Debug symbols binaries
  2. Deliver to client side
    • Pack application binaries to nupkg (1 on diagram)
    • Pack symbols to snupkg (2 on diagram)
    • Publish to NuGet feed
    • Install on the client side
    • Unpack application binaries (3 on diagram)
    • Unpack symbols (4 on diagram)
  3. Run application in debug mode (5 on diagram)
    • Make sure runtime understand how to discover application binaries
    • Make sure runtime understand how to discover symbols binaries
    • Make sure debug runtime understand how to link them and display appropriate information

So what NuGet currently can do:

  1. NuGet package framework able to pack generic output (nupkg) and debug symbols (snupkg) into separated packages
  2. NuGet framework able to publish generic output and the debug symbols to remote NuGet feeds
  3. NuGet framework able to restore generic output packages to target system from the NuGet feed (remote AND local)

Basically it is artifacts split and publish mechanisms. Also there is only half of delivery mechanism implementation right now (nupkg only).

What NuGet missing

  1. Ability to restore snupkg in same way how it do with the generic ones
  2. snupkg delivery mechanism for automatic artifacts merge (place symbols near generic output) to enable one of the options of generic symbols discovery mechanism.

Important

  1. NuGet framework symbols restore procedure must be flexible and not attached to generic binaries source NuGet feed. (Then you can use local file system repository as a source for the symbols for binaries from remote NuGet feed)
  2. Existing toolchains must be extended for symbols restore (separate tool will add additional complexity)
  3. NuGet protocol API must be extended for symbols restore (all 3-th party application will have easy way to support standard restore symbols mechanism)
  4. NuGet protocol must be end to end tool for pack, publish, delivery and unpack stages.

Notes

It is a brief overview without technical details. This PR and initial issue have additional information about technical implementation, API definitions, problem areas so must refer you to them :)

This PR: #10899
Initial issue: #10793
PR text itself: https://github.com/BlackGad/Home/blob/dev/proposed/2021/NuspecRestore.md

And the answer

I'm not sure I fully understand your scenario just yet. Could you explain why you need to manually fetch your symbols instead of letting the debugger get them for you? Is it for improved exceptions at runtime?

Current debugging mechanism assumes that you MUST have remote NuGet feed which also hosting Symbol server. Local file system feeds are not supported. There is no easy way to fill symbols manually explanation on real case (We are the legion that requires this %))

I'm also curious as to the benefits of adding "symbol gathering" to NuGet restore. If we provided a separate standalone tool, would that work for you?

One more spike?) snupkg is the same nupkg. All procedures that can be applied to nupkg can be applied to snupkg (discovery, security, install etc). Also NuGet already have API to create and publish snupkg so will be at least strange decision to use external tool for install and use.

@BlackGad
Copy link
Contributor Author

@loic-sharma
Real world is cruel :) Often frameworks used in different from authors idea way.

Simplest solution - application have no additional component references so it uses required nuget packages directly.

Complex solutions requires additional packed modules and this modules require another internal modules so on. Real build often 1 hour CI pipeline build. End product bug means bug in some separate component and to debug and fix you need to have ability rapidly change and merge components locally.

I already said at the very beginning that this functionality is first step to manage above situation %)

@loic-sharma
Copy link
Contributor

This seems like an awesome feature to improve user's debugging/error experiences. I created a prototype for this proposal: https://github.com/loic-sharma/symbols

How this works:

  1. Listen to the build for assemblies that are written to the output directory without corresponding symbols (LINK)
  2. Pass the paths to these assemblies to a tool that download symbols (LINK)
  3. For each assembly, try and download its corresponding symbols from nuget.org's symbol server (LINK) using the symbol query protocol.

My findings:

  1. Instead of restoring .snupkgs, it is simpler to download individual symbols
  2. This doesn't require any special NuGet integration. It can be done entirely by the .NET SDK

Some open questions:

  1. How do we configure what symbol servers to download symbols from?
  2. Should this feature be opt-in? Or should it be on by default?

I'll ping some folks at Microsoft to get their thoughts on this idea.

@BlackGad
Copy link
Contributor Author

Instead of restoring .snupkgs, it is simpler to download individual symbols

This will brake security for signed snupkg's :( #10793 (comment)

Should this feature be opt-in? Or should it be on by default?

Symbols restore option must be opt-in only. Symbols restore process must not impact nupkg restore option and also must be controllable.

Also you faced with one of issues that I collect earlier in source issue. #10793 (comment)

I insist on same behavior for snupkg as for the nupkg. Current internal protocol has implementation for web and local resources (feeds) discovery with solved transfer/auth/security features. If snupkg will behave in different way - you will be forced to solve same features separately. snupkg is the same nupkg with some content and metadata filters. Also there are already implemented caching and restore mechanisms (for VS old and new project structures).

There is no easy road for this feature because different parts of it impact NuGet, VS, NuGet registry teams :)
I introduce a plan (see v2 - 1) that will impact ONLY NuGet team at the very beginning (stage 1). Then it will require other cases implementation.

@loic-sharma
Copy link
Contributor

loic-sharma commented Jul 19, 2021

This will brake security for signed snupkg's :( #10793 (comment)

Why would it break security? This is how your debugger works today. The assembly contains a checksum for its corresponding symbols (LINK). So once you download symbols, you can check it hasn't been tampered by recalculating its checksum.

FYI, nuget.org does not verify .snupkg's signatures today. We do, however, check that the uploaded symbols match assemblies previously uploaded to nuget.org.

Symbols restore option must be opt-in only. Symbols restore process must not impact nupkg restore option and also must be controllable.

Yup, that's my thinking as well.

I insist on same behavior for snupkg as for the nupkg. Current internal protocol has implementation for web and local resources (feeds) discovery with solved transfer/auth/security features. If snupkg will behave in different way - you will be forced to solve same features separately. snupkg is the same nupkg with some content and metadata filters. Also there are already implemented caching and restore mechanisms (for VS old and new project structures).

I see where you're coming from. That said, symbols have been around for decades and changing protocol details would mean losing decades worth of tooling and integrations. This is not a decision we should make lightly.

The following problems are already solved for symbols:

  • Transfer/restore - The Simple Symbol Query Protocol (SSQP) is to download symbols
  • Security - The assembly contains a checksum for its corresponding symbols. In other words, you can trust a symbol if you already trust its corresponding assembly.
  • Caching - Your machine has a symbol store cache.

That only leaves authentication. NuGet doesn't bring much to the table here as out of the box it only supports HTTP basic authentication and client certificate authentication. Hence, I don't think there's a strong case yet to move away from the existing symbol server protocol.

@BlackGad
Copy link
Contributor Author

Ok, I already mention earlier that it is first step for local debugging. If we add support for snupkg into the protocol - than we can manage main thing that I need for proper local debugging of composite applications. All features above cover more than I need, but purpose is organic for current implementation state of protocol.

Idea behind all of this is to call command line to end product for proper assemblies and symbols substitution. Because nupkg become not only assemblies blob.

Can't find my initial case, it is possible that it was missed somewhere in other messages.

Real case

Lets imagine we have multilayered product. Every layer it is some kind of nupkg.

  1. Package that contains basic logic - Base.dll
  2. Package that contains some device driver specific logic (which uses Base.dll as well)
  3. Package that contains web UI resources and msbuild scripts for proper output folder delivery
  4. Final product that hosts web UI with katana, implements service, WPF ui etc.

Now client report a bug. After the research we understand that bug can be in Base.dll. To determine this we need restore symbols and debug in code issue line by line. Our nuget feed do not hosts symbol server. Ok we download symbols as an artifact and copy pdb files into proper location near the assemblies (file by file for every platform) then determine that bug there, so we need to fix it.

We clone repo for base.dll and trying to fix an issue. To test, that issue was fixed we compile new base nuget package.

Current behavior

  1. If we are lucky this is legacy project with v2 project structure. Why lucky? Because that protocol version can work with local packages folder - so we can substitute inside it content directly. Just unpacking nupkg and snupkg folder to currently used by product base nuget version. Manually or with very custom scripts, CLI utils.
  2. If we are not lucky we go to the nuget global cache folder find for currently used version. Backup it and do the same thing from 1. Then restore cached content. But wait it is only nupkg content, because in new structure satellite files will not be copied automatically. So it must be trick inside product msbuild project or manual operation for all platforms.
  3. Launch product and ensure bug fixed.
  4. Commit base repo code and wait for ci build.
  5. Update base NuGet package version and commit to product repo

Note: Package version will be available only after CI build. Base NuGet package content cannot be copied to output folder because of automatic build output files replacement. We are using 3.5, 4 and 5.0 net frameworks in same time.

Now imagine that bug was inside package with device driver logic because some issues inside base.dll. We will receive dependency tree. And now we need to work with 3 repos waterfall substitution.

Wished behavior

  1. Run nuget.exe with limited configuration where the only feeds will be output folders of required nuget packages with no cache option and force reinstall
  2. Launch product and ensure bug fixed.
  3. Commit base repo code and wait for ci build.
  4. Update base NuGet package version and commit to product repo

This is simplified developer flow. Ideally it would be 2 command lines. But, hope, it is a future. Not relevant to this purpose.

@BlackGad
Copy link
Contributor Author

And again. You insist that symbols restore handled by SSQP. What to do when you have no symbol server?

Current builds already have assemblies and symbols in right place for debug. NuGet selects binaries and pack them to package, NuGet selects symbols and pack them to package as well, NuGet can install packed nupkg, but Nuget cannot install packed symbols! From my prospective it is the same situation like brief scheme above for web but at stage css unpack we tell browser - hey bro your css in another castle. Decide yourself how to fetch and link it yourself ;) and it is not my business that technically it similar to already delivered HTML

@papeh
Copy link

papeh commented Feb 7, 2022

My company manages shared dependencies for our desktop apps using NuGet. We like to ship symbols to our users so that automated error reports contain line numbers in crash stacks. This requires downloading symbols at build time. So we would find this feature useful, although most of our products have migrated to the new .

We're using this hack when we collect the files to copy to our output directory:

<Target Name="CollectAssemblyAndPdbPaths">
	<ItemGroup>
		<NugetPacks Include="Package1"><Version>1.0</Version><Path>lib/net461/*.*</Path></NugetPacks>
	</ItemGroup>
	<DownloadFile Address="https://www.nuget.org/api/v2/symbolpackage/%(NugetPacks.Identity)/%(NugetPacks.Version)"
		DownloadsDir="$(SymbolsDir)" LocalFilename="%(NugetPacks.Identity).snupkg" />
	<Unzip ZipFilename="$(SymbolsDir)/%(NugetPacks.Identity).snupkg" ToDir="$(SymbolsDir)/%(NugetPacks.Identity).%(NugetPacks.Version)" />
	<ItemGroup>
		<NugetFiles Include="$(PackagesDir)/%(NugetPacks.Identity).%(NugetPacks.Version)/%(NugetPacks.Path)" />
		<NugetFiles Include="$(SymbolsDir)/%(NugetPacks.Identity).%(NugetPacks.Version)/%(NugetPacks.Path)" />
	</ItemGroup>
</Target>

@jeffkl jeffkl added the Community PRs (and linked Issues) created by someone not in the NuGet team label May 3, 2022
@ghost ghost added the Status:No recent activity No recent activity. label Sep 26, 2023
@ghost
Copy link

ghost commented Sep 26, 2023

This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

@donnie-msft
Copy link
Contributor

Hi, we have removed our "proposals" folder, so please move this proposal to the "accepted" folder.
See the update here, and let me know if you have questions/concerns: https://github.com/NuGet/Home/blob/b18b5cc1507df04ea9785f8ba613b1ceb2ad93ea/meta/README.md#what-happens-to-a-proposal
Thanks!

@ghost ghost removed the Status:No recent activity No recent activity. label Nov 21, 2023
@ghost ghost added the Status:No recent activity No recent activity. label Dec 22, 2023
@ghost
Copy link

ghost commented Dec 22, 2023

This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 330 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PRs (and linked Issues) created by someone not in the NuGet team Status:No recent activity No recent activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants