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

Best way to debug OC packages #1521

Closed
alberthajdu opened this issue Feb 23, 2018 · 16 comments · Fixed by #1544
Closed

Best way to debug OC packages #1521

alberthajdu opened this issue Feb 23, 2018 · 16 comments · Fixed by #1544
Milestone

Comments

@alberthajdu
Copy link

What's the best way to debug OC packages when used from the MyGet source (https://www.myget.org/gallery/orchardcore-preview)?
I've found this symbol server doc http://docs.myget.org/docs/reference/symbols but it seems like the orchardcore-preview feed doesn't have a symbol source.

A documentation regarding this topic would be useful too.

@alberthajdu
Copy link
Author

@sebastienros Any thoughts on this? This could really boost the developer experience and could help identify bugs more quickly.

@sfmskywalker
Copy link
Member

I'm also very interested in this. Currently I have to basically copy my custom modules to the OrchardCore full-source solution and troubleshoot from there. It works, but increasingly inefficient as our application's complexity increases.

@jtkech
Copy link
Member

jtkech commented Feb 27, 2018

I think we just need to include pdb files when packing modules. There is this line in NuGet.Build.Tasks.Pack.targets.

<AllowedOutputExtensionsInSymbolsPackageBuildOutputFolder>.pdb; .mdb; $(AllowedOutputExtensionsInPackageBuildOutputFolder); $(AllowedOutputExtensionsInSymbolsPackageBuildOutputFolder)</AllowedOutputExtensionsInSymbolsPackageBuildOutputFolder>

So maybe we just need to set this property (with a very long name) to true. See here where @jnm2 suggests a shorter property name IncludeSymbolsInPackage.

I will look into this tomorrow.

@jnm2
Copy link

jnm2 commented Feb 27, 2018

Beyond including the pdbs in the nupkg, you probably also want to GitLink (or SourceLink or embed) your source files if you aren't already. That makes it so much easier for people to step into source- they don't need to have the source on their hard disk and browse to each file. You might be interested in this comparison of the available options, especially the final nupkg size comparison lower down.

@sfmskywalker
Copy link
Member

That would be a dream come true.

@jtkech
Copy link
Member

jtkech commented Mar 1, 2018

I have a solution that works, at least on my machine, i will commit it soon so that you can try it.
@jnm2 thanks for pointing me in the right direction, right now i opted to embed source files.

@sebastienros
Copy link
Member

MVC uses SourceLink

@alberthajdu
Copy link
Author

@jtkech you are awesome. One more thing I think it would be useful to mention it in the docs so people would be aware of this thing. And it's basic but it could be helpful if we mention how to enable it (uncheck "Enable Just My Code" in the debugging options).

@jnm2
Copy link

jnm2 commented Mar 6, 2018

@alberthajdu Library consumers using the new csproj format will also have to make sure the PDBs are copied to the build output directory for the time being, in addition to unchecking Enable Just My Code. (I highly recommend Just My Code Toggle by Sam Harwell if you switch back and forth a lot.)

Basically, everything in https://github.com/nunit/docs/wiki/Debugger-Source-Stepping except step 2 which is specific to GitLink.

@jtkech
Copy link
Member

jtkech commented Mar 7, 2018

@alberthajdu thanks for the feedback and happy to know it works for you.

@jnm2 about copying pdb files, as i remember i didn't have any issue with this.

<PropertyGroup>
  <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOut...);.pdb</AllowedOut...>
</PropertyGroup>

<!-- In a next release we will just use a new '$(EmbedAllSources)' property -->
<Target Name="PopulateEmbeddedFiles" AfterTargets="BeforeCompile" BeforeTargets="CoreCompile">
  <ItemGroup>
    <EmbeddedFiles Include="@(Compile)" />
  </ItemGroup>
</Target>

Moreover now, for a given library, we also embed the pdb file itself in the assembly.

<PropertyGroup>
  <DebugType>embedded</DebugType>
</PropertyGroup>

@alberthajdu i didn't need to uncheck Enable Just My Code but the following options were also checked.

  • Enable Just My Code
  • Enable Source Link
  • Require source files to exactly match the original version
    Seems to be also related to embedded sources files

You're right about the doc but could you re-try with these options?

@jnm2
Copy link

jnm2 commented Mar 7, 2018

Ah, so DebugType embedded will mean it will work for consumers without them having to do the PDB copy workaround. It also means that applications that depend on OC will have no choice but to distribute all that PDB and source inside the OC dll with their app. I think typically people prefer not to deploy debug symbols along with their application.

@jtkech
Copy link
Member

jtkech commented Mar 7, 2018

Good point thanks. I naively thought it could not be an issue. Are you talking about dll sizes and / or performance? I didn't see any too big increase of sizes and no impact on performance.

@alberthajdu
Copy link
Author

@jtkech the mentioned options are checked for me. I think they are checked by default.

@jnm2
Copy link

jnm2 commented Mar 7, 2018

@jtkech It will only affect DLL size. (IP rights aren't an issue since this is open source. 😊) What was the final size? For NUnit, it would have been 3.8 times the original size and we weren't impressed with that.

@jtkech
Copy link
Member

jtkech commented Mar 9, 2018

@alberthajdu okay thanks, so the only difference is that i didn't need to uncheck Just My Code, maybe because, for testing, packages were generated on my machine and outputed in a local feed.

@jnm2 okay cool. For most of the libraries it was less than twice, sometimes a little more. After analyzing the sizes of dll, pdb, and nupkg, it seems that a pdb is already compressed when it is embedded in a dll.

@scil
Copy link
Contributor

scil commented Dec 18, 2021

There is a non-best but effecient way #10603

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

Successfully merging a pull request may close this issue.

6 participants