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

mpd: Add support for meson options #87940

Closed
wants to merge 3 commits into from

Conversation

@doronbehar
Copy link
Contributor

doronbehar commented May 16, 2020

Motivation for this change

I wanted to have documentation installed. Then I learned it's handled by the meson option -Ddocumentation=true and then I realized that only meson "features" handled by the already wonderfully written derivation (❤️ @tobim for #57608 ).

Things done

I added support for declarative meson options support similar to that of features. I also enabled tests by default although that's not enabled by default in upstream's meson_options.txt.

I also added another mpd-full derivation which will be available for everyone that will want an mpd derivation with documentation.

  • 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.
doronbehar added 3 commits May 16, 2020
Add sphinx and doxygen as build inputs if documentation is enabled.
Add gtest and zip as inputs if tests is enabled.
Add a patch needed to make the tests not fail.
Run tests by default.
@doronbehar
Copy link
Contributor Author

doronbehar commented May 16, 2020

@GrahamcOfBorg build mpd mpd-small mpd-full

@GrahamcOfBorg test mpd.aarch64-linux

@GrahamcOfBorg test mpd.x86_64-linux

@jtojnar
Copy link
Contributor

jtojnar commented May 16, 2020

There were several threads discussing whether this is something we should do, as opposed to providing a reasonable defaults and having people who want to change them use the standard mechanisms for overriding derivations (overrideAttrs). The main question is whether this is something large enough fraction of users will want to justify the added maintenance burden (expression harder to read, the option list getting out of sync with upstream…).

Cannot find them at the moment.

Edit: #51766 (review)

@doronbehar
Copy link
Contributor Author

doronbehar commented May 16, 2020

I see. TBH I only wanted to get the docs, so maybe using split outputs is better. Closing in favor of #87949 .

@doronbehar doronbehar closed this May 16, 2020
@doronbehar doronbehar deleted the doronbehar:mpd-meson-options branch May 16, 2020
@jpotier jpotier mentioned this pull request Jun 6, 2020
2 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.