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

diffoscope: enable bloat by default #121042

Merged
merged 1 commit into from May 22, 2021
Merged

Conversation

gebner
Copy link
Member

@gebner gebner commented Apr 28, 2021

Motivation for this change

nix run nixpkgs.diffoscope -c diffoscope should start a full-featured diffoscope. Using a lot of external tools is literally one of the tagline features of diffoscope: "unpack archives of many kinds and transform various binary formats into more human-readable form".

This PR therefore changes the top-level attribute diffoscope to include the whole kitchen sink. A new diffoscopeMinimal attribute is added with the current feature set and smaller closure size.

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.

@grahamc
Copy link
Member

grahamc commented Apr 28, 2021

If I remember correctly "with bloat" cannot be built on Hydra because part of its build chain creates an output with a significantly large sparse file. However, the NAR format doesn't know about sparse files and the output becomes too large and is rejected.

@gebner
Copy link
Member Author

gebner commented Apr 28, 2021

Ah, that's unfortunate. Do you remember which package this was?

I'd really like hydra to build the full version because the python tests take quite some time to run.

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 121042 at b6d7559 run on x86_64-linux 1

2 packages built successfully:
  • diffoscope
  • diffoscopeMinimal

@gebner
Copy link
Member Author

gebner commented Apr 28, 2021

Another alternative would be to reduce the number of tools disabled by enableBloat = false. Upstream has a much less restrictive list of "huge" tools:

# Set of tools considered "large" their installation size, or too niche in
# their target users.  To be easily excluded from installation if not
# specifically required.
# These are the names of the tools, not package names.
HUGE_TOOLS = {
    "ghc",
    "ocamlobjinfo",
    "llvm-bcanalyzer",
    "llvm-config",
    "llvm-dis",
    "ppudump",
    "javap",
    "ssconvert",
    "apktool",
    "apksigner",
    "pedump",
    "radare2",
    "dumpxsb",
}

@grahamc
Copy link
Member

grahamc commented Apr 28, 2021

Since every dependency of diffoscope with bloat enabled was pre-built by hydra, I'm forced to conclude I am misremembering :)

👍

@gebner gebner merged commit ca0f021 into NixOS:master May 22, 2021
@danielfullmer danielfullmer mentioned this pull request May 23, 2021
9 tasks
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

3 participants