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

treewide: add a debug-output to a few packages #80751

Closed
wants to merge 8 commits into from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Feb 21, 2020

Motivation for this change

After learning about the optional debug-output I realized that only a few packages provide debug symbols. This PR adds the debug-output to an arbitrary list of packages to spread the usage of this feature.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jtojnar
Copy link
Contributor

jtojnar commented Feb 24, 2020

There were concerns about this taking too much space in binary cache so for now we have only added it to libraries whose rebuild would be too much effort for user, not leaf packages.

@Ma27
Copy link
Member Author

Ma27 commented Feb 24, 2020

That's actually a fair argument, however I guess that the packages I changed are rather popular (and not that many), so the outcome wins here - at least in my opinion.

@edolstra do the concerns you raised back in 2016 still apply? (tbh I'm not sure about what the current upstream infrastructure is able to withstand). If needed I'm also happy to provide more detailed information about the per-package impact on storage.

@edolstra
Copy link
Member

I don't think this is a good idea. It's one thing to provide debug symbols for some widely used libraries (e.g. Qt), but who really needs debug symbols for htop?

@Ma27
Copy link
Member Author

Ma27 commented Mar 13, 2020

@edolstra so your main issue with this is that htop (and similar pkgs in here) aren't something you usually need to debug?

The main purpose here is that I wanted to add debug symbols to a few packages I frequently use to be able to easily debug e.g. core-dumps. But I agree that this list is fairly arbitrary and we may want to select a few more (or different) packages.

But may I ask if there's any reason not to enable a debug-output on (more or less) every derivation in nixpkgs? Being able to load debug-symbols for a (possibly broken) package (if needed) from the binary cache is IMHO an incredible useful feature :)

@lheckemann
Copy link
Member

@Ma27 binary cache size: #18530

@Ma27
Copy link
Member Author

Ma27 commented Mar 24, 2020

@lheckemann I'm aware of that discussion, it's been referenced by @jtojnar already. I mainly asked since I don't know how much of the binary-cache size can be used to serve debug-symbols of packages.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 24, 2020

I think Eelco’s point was that the storage requirements would outweigh costs of occasional rebuilding of leaf packages. Especially since the cache keeps build artefacts indefinitely.

Estimates are in the thread: #18530 (comment)

@Ma27
Copy link
Member Author

Ma27 commented Mar 29, 2020

I think Eelco’s point was that the storage requirements would outweigh costs of occasional rebuilding of leaf packages. Especially since the cache keeps build artefacts indefinitely.

Fair enough 👍

@Ma27 Ma27 closed this Mar 29, 2020
@Ma27 Ma27 deleted the debug-outputs branch March 29, 2020 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants