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.

Sign up for GitHub

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

Add Meson/ninja setup hooks #28444

Merged
merged 6 commits into from
Aug 27, 2017
Merged

Add Meson/ninja setup hooks #28444

merged 6 commits into from
Aug 27, 2017

Conversation

lluchs
Copy link
Contributor

@lluchs lluchs commented Aug 21, 2017

Motivation for this change

Lots of packages have been switching to Meson recently. The setup hooks should make updating these packages easier and prevent duplication in configure/build/install scripts.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@lluchs, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sternenseemann, @bjornfor and @Mic92 to be potential reviewers.

@dezgeg
Copy link
Contributor

dezgeg commented Aug 22, 2017

Looks very nice to me! Few comments though: does this one other package benefit/get broken from this setup-hook:

pkgs/applications/video/pitivi/default.nix:, which, meson, ninja
pkgs/applications/video/pitivi/default.nix:    buildInputs = [ which meson ninja pkgconfig gobjectIntrospection ]

What about these using ninja (without meson)?:


pkgs/applications/audio/i-score/default.nix:  ninja,
pkgs/applications/audio/i-score/default.nix:    ninja
pkgs/applications/audio/i-score/default.nix:    ninja
pkgs/applications/graphics/ao/default.nix:{stdenv, fetchgit, cmake, ninja, boost, libpng, glfw3, epoxy, guile, pkgconfig
pkgs/applications/graphics/ao/default.nix:    cmake ninja boost libpng glfw3 epoxy guile pkgconfig mesa libX11 
pkgs/applications/graphics/ao/default.nix:  buildPhase = "ninja";
pkgs/applications/graphics/ao/default.nix:    ninja install
pkgs/applications/misc/llpp/default.nix:{ stdenv, lib, makeWrapper, fetchgit, pkgconfig, ninja, ocaml, findlib, mupdf
pkgs/applications/misc/llpp/default.nix:  nativeBuildInputs = [ pkgconfig makeWrapper ninja ];
pkgs/applications/networking/browsers/chromium/common.nix:{ stdenv, ninja, which, nodejs, fetchurl, gnutar
pkgs/applications/networking/browsers/chromium/common.nix:      ninja which python2Packages.python perl pkgconfig
pkgs/applications/networking/browsers/chromium/common.nix:        ninja -C "${buildPath}"  \
pkgs/applications/networking/browsers/vivaldi/ffmpeg-codecs.nix:, dbus_glib, gtk2, gtk3, libexif, libpulseaudio, libXScrnSaver, ninja, nss
pkgs/applications/networking/browsers/vivaldi/ffmpeg-codecs.nix:    dbus_glib gtk2 gtk3 libexif libpulseaudio libXScrnSaver ninja nss pciutils pkgconfig
pkgs/applications/networking/browsers/vivaldi/ffmpeg-codecs.nix:    ninja -C out/Release -v libffmpeg.so
pkgs/applications/science/logic/lean2/default.nix:, gperftools, ninja, makeWrapper }:
pkgs/applications/science/logic/lean2/default.nix:  buildInputs = [ gmp mpfr cmake python gperftools ninja makeWrapper ];
pkgs/applications/science/logic/lean2/default.nix:    wrapProgram $out/bin/linja --prefix PATH : $out/bin:${ninja}/bin
pkgs/development/compilers/swift/default.nix:, ninja
pkgs/development/compilers/swift/default.nix:    ninja
pkgs/development/libraries/audio/jamomacore/default.nix:{ stdenv, fetchFromGitHub, pkgconfig, alsaLib, portaudio, portmidi, libsndfile, cmake, libxml2,  ninja }:
pkgs/development/libraries/audio/jamomacore/default.nix:  buildInputs = [ pkgconfig alsaLib portaudio portmidi libsndfile cmake libxml2  ninja ];
pkgs/development/libraries/qt-5/5.9/qtwebengine/default.nix:, bison, coreutils, flex, git, gperf, ninja, pkgconfig, python2, which
pkgs/development/libraries/qt-5/5.9/qtwebengine/default.nix:    bison coreutils flex git gperf ninja pkgconfig python2 which
pkgs/development/tools/xcbuild/default.nix:{ stdenv, cmake, fetchFromGitHub, zlib, libxml2, libpng, CoreServices, CoreGraphics, ImageIO, ninja }:
pkgs/development/tools/xcbuild/default.nix:  buildInputs = [ cmake zlib libxml2 libpng ninja ]
pkgs/os-specific/darwin/swift-corefoundation/default.nix:{ stdenv, fetchFromGitHub, python, ninja, libxml2 }:
pkgs/os-specific/darwin/swift-corefoundation/default.nix:  buildInputs = [ ninja python libxml2 ];
pkgs/os-specific/darwin/swift-corefoundation/default.nix:    ninja
pkgs/tools/inputmethods/fcitx-engines/fcitx-mozc/default.nix:{ clangStdenv, fetchFromGitHub, fetchurl, fetchpatch, gyp, which, ninja,
pkgs/tools/inputmethods/fcitx-engines/fcitx-mozc/default.nix:  nativeBuildInputs = [ gyp which ninja python pkgconfig ];
pkgs/tools/inputmethods/ibus-engines/ibus-mozc/default.nix:{ clangStdenv, fetchFromGitHub, which, ninja, python, gyp, pkgconfig, protobuf
pkgs/tools/inputmethods/ibus-engines/ibus-mozc/default.nix:  nativeBuildInputs = [ which ninja python gyp pkgconfig ];
pkgs/tools/system/illum/default.nix:{ stdenv, fetchgit, pkgconfig, ninja, libevdev, libev }:
pkgs/tools/system/illum/default.nix:  buildInputs = [ pkgconfig ninja libevdev libev ];
pkgs/tools/system/illum/default.nix:    ninja

@lluchs
Copy link
Contributor Author

lluchs commented Aug 22, 2017 •
edited

Thanks for the hint, I'm going to try building those packages.

  • pitivi/gst-transcoder: does not have configure/build commands as the repository includes a configure script. Should work just fine with the setup hooks here though (I'll check that)
  • i-score: cmake/ninja, has obsolete preBuild - doesn't build because of some unrelated cmake error
  • ao: cmake/ninja, has obsolete buildPhase
  • llpp: custom build/install
  • chromium/vivaldi: custom build/install
  • lean2: will check, ninja appears to be a runtime dependency and not used for the build
  • swift: custom build/install
  • jamomacore: already fixed
  • qtwebengine: not sure, will have to try building
  • xcbuild: will check, ninja appears to be a runtime dependency
  • swift-corefoundation: custom build/install
  • fcitx-mozc/ibus-mozc: custom build/install
  • illum: has obsolete buildPhase

@yorickvP
Copy link
Contributor

One thing I noticed is that ninja install strips the entire rpath from all the binaries.
I added a patch to the meson I used to build, but there should probably be a better solution (it does this for some kind of reason, I think).

meson.overrideDerivation (attrs: {
  postPatch = ''
    sed -i -e 's|e.fix_rpath(install_rpath)||' mesonbuild/scripts/meson_install.py
  '';
})

@lluchs
Remove obsolete ninja buildPhases
3c22255 
These are all handled by ninja's setup hook.
@lluchs
lean2, xcbuild: fix build with ninja setup hook
e9dd356 
Ninja is a runtime dependency here. However, cmake can generate Ninja
build files as well to satisfy the setup hook.
@lluchs
Copy link
Contributor Author

lluchs commented Aug 22, 2017 •
edited

qtwebengine is still building, but I think they're all working now!

Edit: qtwebengine fails on install, I'll fix that...
Edit2: I added another commit to fix qtwebengine.

@yorickvP I think this is a separate issue as it doesn't really depend on the setup hook.

@globin globin merged commit f0a1ab7 into NixOS:master Aug 27, 2017
@jtojnar
Copy link
Contributor

jtojnar commented Sep 4, 2017

Would it be also possible to set checkPhase to mesontest?

running tests
check flags: -j1 -l1 VERBOSE=y check
make: *** No rule to make target 'check'.  Stop.

@lukateras lukateras mentioned this pull request Sep 25, 2017
Closed
8 tasks
lukateras added a commit to lukateras/nixpkgs that referenced this pull request Sep 25, 2017
@lukateras
meson: do not strip RPATH
259a74a 
NixOS#28444 (comment)
@jtojnar jtojnar mentioned this pull request Sep 27, 2017
Merged
71 tasks
@vcunat
Copy link
Member

vcunat commented Sep 27, 2017

@jtojnar: checkTarget = "mesontest";

@jtojnar
Copy link
Contributor

jtojnar commented Sep 27, 2017 •
edited

@vcunat yes, I am currently adding it manually for GNOME packages but extending the setup hook would be more convenient. I did not make a pull request since I have not been sure if having a separate mesonCheckPhase analogous to mesonConfigurePhase was a good idea and what flags to pass to meson test. I will try to investigate later.

Mic92 pushed a commit that referenced this pull request Oct 15, 2017
@lukateras @Mic92
meson: do not strip RPATH
8f95aef 
#28444 (comment)
@asavah asavah mentioned this pull request Nov 1, 2017
Closed
@orivej orivej mentioned this pull request Dec 3, 2017
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@globin globin globin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@lluchs @mention-bot @dezgeg @yorickvP @jtojnar @vcunat @globin