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

Modify Release to RelWithDebInfo for built wheels #201

Merged
merged 3 commits into from
Dec 21, 2021
Merged

Conversation

mlxd
Copy link
Member

@mlxd mlxd commented Dec 21, 2021

Context: Debugging with pypi supplied wheels is currently difficult due to the missing debug info. This PR ensure deub info is returned to the dynamic libraries by setting Release to RelWithDebInfo. This allows use of Lightning through debuggers, such as gdb, and lldb.

Description of the Change: Release build type defaults to RelWithDebInfo.

Benefits: Allows debugging symbols to be included in distributed binaries.

Possible Drawbacks: Larger wheel sizes.

Related GitHub Issues:

@mlxd mlxd requested a review from chaeyeunpark December 21, 2021 13:34
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Dec 21, 2021

Codecov Report

Merging #201 (ea0b967) into master (17d895f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #201   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files           4        4           
  Lines         278      278           
=======================================
  Hits          277      277           
  Misses          1        1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17d895f...ea0b967. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2021

Test Report (C++) on Ubuntu

       1 files  ±0         1 suites  ±0   0s ⏱️ ±0s
   378 tests ±0     378 ✔️ ±0  0 💤 ±0  0 ±0 
2 233 runs  ±0  2 233 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit ea0b967. ± Comparison against base commit 17d895f.

♻️ This comment has been updated with latest results.

@mlxd mlxd requested a review from maliasadi December 21, 2021 14:06
Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement! Using RelWithDebInfo instead of Release increases the wheel size, but the code runs still fast and optimize, and has debug information included; thus a great deal to trace and fix bugs efficiently. Thanks @mlxd!

@mlxd mlxd merged commit 772bf0e into master Dec 21, 2021
@mlxd mlxd deleted the release_info branch December 21, 2021 14:45
chaeyeunpark added a commit that referenced this pull request Jan 12, 2022
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 this pull request may close these issues.

2 participants