Add an option to keep native debug symbols #39203

omajid · 2020-07-13T16:18:45Z

When packaging .NET for Linux distributions, the package builders generally use a different workflow for shipping symbols to users:

The package maintainer builds code with the debug flags (such as -g) to generate full native debug info and symbols.
Nothing is stripped from build by the package maintainer.
The build system (rpmbuild, debuild) removes the debug info (or debug symbols) from the code and creates separate -debuginfo or -debug packages that contain just the debug symbols.
These debug packages are then distributed along with the normal packages using the normal Linux distribution mechanisms. This letsusers install the exact set of debug symbols matching their other package.

To support this workflow in dotnet/runtime, we need to add optional support for not stripping debug symbols. I used it has follows:

CFLAGS=-g CXXFLAGS=-g ./build.sh --keepsymbols true

After this build, the built binaries include all debug symbols.

I can then rely on the distro package build system to identify, strip, package and ship the debug info/symbols separately.

See #3781 and dotnet/source-build#267 for more details on the background and motivation.

For some related fixes, see:

omajid · 2020-07-13T16:22:19Z

This is a bit of a regression from the equivalent support in dotnet/coreclr and dotnet/corefx

CC'ing some folks who were part of the conversation last time around: @dagood @dleeapho @eerhardt @jkotas @tmds @weshaggard

omajid · 2020-07-21T15:58:14Z

Anyone willing to review this?

dagood · 2020-07-21T17:15:42Z

@jkoritzinsky maybe you'd like to review this change--cmake conditions and new build script arg.

dagood

Seems very reasonable. One suggestion and one comment about the context--I'm not particularly familiar with this area.

dagood · 2020-07-21T17:19:06Z

eng/build.sh

@@ -74,6 +74,7 @@ usage()
  echo "  --gcc                      Optional argument to build using gcc in PATH (default)."
  echo "  --gccx.y                   Optional argument to build using gcc version x.y."
  echo "  --portablebuild            Optional argument: set to false to force a non-portable build."
+  echo "  --keepsymbols              Optional argument: set to true to keep symbols/debuginfo in generated binaries."


The help message doesn't mention it takes an argument, but it seems to take true/false and reject nothingness. 😕 But it looks like this is also the case with other settings, so this PR appears to be consistent to me.

Should I take a stab at fixing up the other settings too?

eng/build.sh

tmds

LGTM, thanks!

omajid · 2020-09-21T15:40:23Z

@janvorli hey, can you help me find some reviewers for this PR? Thanks in advance!

janvorli

I would like to ask you to change few things.

janvorli · 2020-09-21T16:38:55Z

eng/build.sh

+      fi
+      passedKeepSymbols="$(echo "$2" | awk '{print tolower($0)}')"
+      if [ "$passedKeepSymbols" = true ]; then
+        cmakeargs="${cmakeargs} -cmakeargs -DCLR_KEEP_SYMBOLS=true"


I would prefer creating a msbuild property for this instead of passing it in the cmakeargs. We do that e.g. for the -portablebuild option, so I'd like to use unified way for all of them. You would then add this option to eng/build-commons.sh, which is used to parse the common options for coreclr, libraries and installer .sh scripts and add the cmake arg there. You'll need to update src/coreclr/runtime.proj, src/installer/corehost/corehost.proj and src/libraries/Native/build-native.proj to convert the msbuild property to the -keepsymbols option passed to the respective .sh scripts.

Does the updated patch match what you had in mind?

janvorli · 2020-09-21T16:40:29Z

eng/build.sh

+      fi
+      passedKeepSymbols="$(echo "$2" | awk '{print tolower($0)}')"
+      if [ "$passedKeepSymbols" = true ]; then
+        cmakeargs="${cmakeargs} -cmakeargs -DCLR_KEEP_SYMBOLS=true"


A nit - based on various cmake args settings in coreclr, I'd prefer this to be named CLR_CMAKE_KEEP_SYMBOLS to keep things unified.

janvorli

LGTM, thank you!

omajid · 2020-09-25T21:35:53Z

@janvorli Does the "keepSymbols" name appear clear enough to you? Should I look at disambiguating it to "keepNativeSymbols" or "keepUnmanagedSymbols" or something similar?

janvorli · 2020-09-25T21:47:31Z

Hmm, keepNativeSymbols sounds better to me.

This is a backport of dotnet/runtime#39203

When packaging .NET for Linux distributions, the package builders generally use a different workflow for shipping symbols to users: 1. The package maintainer builds code with the debug flags (such as `-g`) to generate full native debug info and symbols. 2. Nothing is stripped from build by the package maintainer. 3. The build system (`rpmbuild`, `debuild`) removes the debug info (or debug symbols) from the code and creates separate `-debuginfo` or `-debug` packages that contain just the debug symbols. 4. These debug packages are then distributed along with the normal packages using the normal Linux distribution mechanisms. This lets users install the exact set of debug symbols matching their other package. To support this workflow in dotnet/runtime, we need to add optional support for not stripping debug symbols. I used it has follows: CFLAGS=-g CXXFLAGS=-g ./build.sh --keepnativesymbols true After this build, the built binaries include all debug symbols. I can then rely on the distro package build system to identify, strip, package and ship the debug info/symbols separately. See dotnet#3781 and dotnet/source-build#267 for more details on the background and motivation. For some related fixes, see: - dotnet/coreclr#3445 - dotnet/corefx#24979

omajid · 2020-09-30T13:32:05Z

The CI failures look unrelated to my changes (json tests and mono build issues) but I have rebased this branch onto master to see if that fixes the CI failures.

omajid · 2020-09-30T21:56:01Z

@janvorli Does this still look okay?

janvorli

LGTM with the latest updates.

janvorli · 2020-09-30T23:41:19Z

I'll still re-run the runtime tests to see the runtime (Installer Build and Test coreclr Linux_arm Debug) not to timeout.

Dotnet-GitSync-Bot added the area-Infrastructure-coreclr label Jul 13, 2020

omajid force-pushed the optional-dont-strip-debuginfo branch 2 times, most recently from 51f12b5 to e935340 Compare July 15, 2020 17:02

omajid mentioned this pull request Jul 15, 2020

Packaging-related issues for .NET 5.0 dotnet/source-build#1645

Closed

omajid force-pushed the optional-dont-strip-debuginfo branch from e935340 to 063a014 Compare July 17, 2020 19:19

dagood approved these changes Jul 21, 2020

View reviewed changes

omajid force-pushed the optional-dont-strip-debuginfo branch from 063a014 to 417e787 Compare July 21, 2020 19:05

tmds approved these changes Sep 15, 2020

View reviewed changes

janvorli requested changes Sep 21, 2020

View reviewed changes

omajid force-pushed the optional-dont-strip-debuginfo branch from 417e787 to f23aca6 Compare September 25, 2020 20:55

janvorli approved these changes Sep 25, 2020

View reviewed changes

omajid force-pushed the optional-dont-strip-debuginfo branch from f23aca6 to 2fcdb9d Compare September 28, 2020 15:28

This was referenced Sep 28, 2020

Agents canceling during dotnet builds jaredpar/runfo#47

Closed

Azure Agent timeout dotnet/roslyn#48030

Closed

omajid added a commit to omajid/dotnet-source-build that referenced this pull request Sep 28, 2020

Re-enable building with debug symbols again

f91e59c

This is a backport of dotnet/runtime#39203

omajid mentioned this pull request Sep 28, 2020

Re-enable building with debug symbols again dotnet/source-build#1776

Merged

omajid closed this Sep 30, 2020

omajid reopened this Sep 30, 2020

omajid force-pushed the optional-dont-strip-debuginfo branch from 2fcdb9d to 2ff2c5b Compare September 30, 2020 13:26

janvorli approved these changes Sep 30, 2020

View reviewed changes

jkotas merged commit 2f1694e into dotnet:master Oct 1, 2020

ghost locked as resolved and limited conversation to collaborators Dec 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an option to keep native debug symbols #39203

Add an option to keep native debug symbols #39203

omajid commented Jul 13, 2020

omajid commented Jul 13, 2020

omajid commented Jul 21, 2020

dagood commented Jul 21, 2020

dagood left a comment

dagood Jul 21, 2020

omajid Jul 21, 2020

tmds left a comment

omajid commented Sep 21, 2020

janvorli left a comment

janvorli Sep 21, 2020

omajid Sep 25, 2020

janvorli Sep 21, 2020

omajid Sep 25, 2020

janvorli left a comment

omajid commented Sep 25, 2020

janvorli commented Sep 25, 2020

omajid commented Sep 30, 2020

omajid commented Sep 30, 2020

janvorli left a comment

janvorli commented Sep 30, 2020

Add an option to keep native debug symbols #39203

Add an option to keep native debug symbols #39203

Conversation

omajid commented Jul 13, 2020

omajid commented Jul 13, 2020

omajid commented Jul 21, 2020

dagood commented Jul 21, 2020

dagood left a comment

Choose a reason for hiding this comment

dagood Jul 21, 2020

Choose a reason for hiding this comment

omajid Jul 21, 2020

Choose a reason for hiding this comment

tmds left a comment

Choose a reason for hiding this comment

omajid commented Sep 21, 2020

janvorli left a comment

Choose a reason for hiding this comment

janvorli Sep 21, 2020

Choose a reason for hiding this comment

omajid Sep 25, 2020

Choose a reason for hiding this comment

janvorli Sep 21, 2020

Choose a reason for hiding this comment

omajid Sep 25, 2020

Choose a reason for hiding this comment

janvorli left a comment

Choose a reason for hiding this comment

omajid commented Sep 25, 2020

janvorli commented Sep 25, 2020

omajid commented Sep 30, 2020

omajid commented Sep 30, 2020

janvorli left a comment

Choose a reason for hiding this comment

janvorli commented Sep 30, 2020