Help and advice for my first NixOS package

Help
1

Hello,

I am developing a numerical calculation tool and I want to package it for NixOS
https://github.com/nelson-lang/nelson/blob/8c32ac68caa4e842793e833d4d0cc8572af1598c/default.nix

I would like some help and advice on this first package

Does it seem correct to you or did I miss something?

Thanks for your contribution and advice

2

Looks alright for a start! I have some comments:

  • nativeBuildInputs is for build inputs that need to run during compilation. Runtime dependencies, such as boost, openssl, zlib and friends should be in buildInputs.

    In practice this will only cause issues if you’re not compiling locally, so you don’t see the problems if you get it wrong.

  • Things like gcc are automatically included by mkDerivation, so you can remove that dependency. This helps make it easier to build your package on other platforms.

  • I see you’re manually running cmake in the buildPhase. cmake is intended to be invoked in the configure phase, and nixpkgs has a hook that is enabled by default if you put cmake in nativeBuildInputs to do just that: nixpkgs/pkgs/by-name/cm/cmake/setup-hook.sh at 2d2b5e26d45f565be79d61727bf1306e41d3fe58 · NixOS/nixpkgs · GitHub

    I suspect you’re using buildPhase because using configurePhase broke that, and therefore the nixpkgs integration. Instead, you should be using the cmakeFlags variable, and not calling cmake by hand at all - buildPhase (and installPhase) should then be left empty, makeDerivation will call make correctly for you.

    By defining your own phase you override a lot of nixpkgs defaults, which are important for cross-platform support, and sometimes even just for adhering to your NixOS config.

  • Even if you did want to override nixpkgs defaults, using $(nproc) is not a good idea for nix packages. You’re likely to build lots of software concurrently, adding that can make memory pressure cause build failures, or at least swamp the kernel with process management.

  • While you can define a package in a default.nix like this, it’s better to make a top-level default.nix that calls your package in a different file using callPackage and to list each dependency as a separate function input. This allows users to use .override to change specific input dependencies, and makes it easier to upstream your package into nixpkgs (or to extend your custom package set.

    See e.g. my dotfiles for an example of how that works: dotfiles/pkgs/default.nix at a4ba1914e337f50e70b0a5c81dc6334e1ae16273 · TLATER/dotfiles · GitHub

    Though you’d need to change your nix-build command a bit, depending on how you build your attrset. You can also just do this for a simple one-package repo:

    { pkgs ? import <nixpkgs> { } }: pkgs.callPackage ./nelson.nix { }

  • The empty checkPhase is superfluous, doCheck already disables it

  • For the maintainer thing to work, you’d need to be in the nixpkgs maintainer list. I’d suggest just leaving that empty for downstream stuff.

  • Try using nixfmt on your code :slight_smile:

4 Likes
3

To add, the preferred way would be to use $NIX_BUILD_CORES only if necessary which would use the cores nix setting.

But usually, most builders will take care of this for you, if you just set enableParallelBuilding = true; (which for some builders is even the default already).

More on callPackage here:

https://nix.dev/tutorials/callpackage

2 Likes
4

Thanks to all for your advice :slight_smile: (applied)

5

Nice! I took a look and there are some small misunderstandings to still clean up:

  • callPackage is only useful for allowing overrides if you let it do its job, which is to automatically take the correct libraries out of pkgs and make them arguments. So instead of this, it should look something like this:

    {
   zlib,
   boost,
   openssl,
   mpi,
   hdf5,
   curl,
   libgit2,
   libsndfile,
   # ... rest cut because this is an example
}:

pkgs.stdenv.mkDerivation {
  buildInputs = [
    zlib
    boost
    openssl
    mpi
    hdf5
    curl
    libgit2
    libsndfile
    # ... again, cutting the rest

    The point of this is that with this definition someone could use e.g. nelson.override { openssl = pkgs.openssl_1_1; } or something, and get a useable build out of that (assuming the APIs are compatible). With your current code, only the entirety of pkgs can be replaced, which is not very useful.

  • wrapQappsHook should be in nativeBuildInputs and I’m fairly sure eigen should be in buildInputs. gnumake is part of the stdenv and should be omitted.

  • DCMAKE_INSTALL_PREFIX and DCMAKE_RELEASE_TYPE are already defined for you, so you should not set them.

Other than that, looks pretty much like a perfect nix package to me.

2 Likes
6

I updated nelson.nix with your advice.
Thanks and happy new year

2 Likes
7

Very close, but still no cigar - you need to remove the pkgs from everything, otherwise you’ll still use the pkgs variable, and any overrides to other arguments will be ignored: nelson/nelson.nix at dece98981e07b244a7b86aaf0e7fe714d74ea3cb · nelson-lang/nelson · GitHub

Ideally, you don’t have pkgs in your function args at all - callPackage will put anything you ask for in your args (including, e.g. lib or stdenv), so there’s no need for it.

8

Thanks
Comments applied https://github.com/nelson-lang/nelson/pull/1318

1 Like