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

Jump to bottom

mpd: Add support for meson options #87940

Closed
wants to merge 3 commits into from
Closed

mpd: Add support for meson options #87940

wants to merge 3 commits into from

Conversation

doronbehar
Copy link
Contributor

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 (:heart: @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
mpd: use pkg-config instead of pkgconfig
34ea477
@doronbehar
mpd: format nativeBuildInputs
de0ed61
@doronbehar
mpd: Enable support for declarative options
6670bce 
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

@GrahamcOfBorg build mpd mpd-small mpd-full

@GrahamcOfBorg test mpd.aarch64-linux

@GrahamcOfBorg test mpd.x86_64-linux

@ofborg ofborg bot requested review from fpletz, astsmtl, ehmry and tobim May 16, 2020 12:46
@jtojnar
Copy link
Contributor

jtojnar commented May 16, 2020 •
edited

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

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 mpd-meson-options branch May 16, 2020 14:11
@jpotier jpotier mentioned this pull request Jun 6, 2020
Merged
10 tasks
@doronbehar doronbehar mentioned this pull request Sep 26, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz Awaiting requested review from fpletz

@astsmtl astsmtl Awaiting requested review from astsmtl

@ehmry ehmry Awaiting requested review from ehmry

@tobim tobim Awaiting requested review from tobim

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@doronbehar @jtojnar