BUILD: Add Nix flake with package and devShell #1572

ashgillman · 2025-02-26T02:15:15Z

Nix is a package manager. I've been using it for a long-while, including for STIR/SIRF development. You can declaratively (i.e., using a .nix file) set up the dependencies of a file, and then drop into an environment with it (so a bit like docker to the end user) or install it to your system (like a normal package manager).

So with Nix, a user could run nix shell github:UCL/STIR and have STIR available (after waiting for the build, locally).

Or you can load up the development env, to just load all the dependencies into the environments without having nix run the build. I run nix develop in the directory, then can cd build && make BUILD_TESTS && ctest without having explicitly installed dependencies, like boost, onto the system.

I don't necessarily advocate adoption - it would be tricky to maintain if no-one is using it and I've been AWOL for a long time. This PR is (1) for visibility; so that some new user could copy the .nix files locally (say they want to try STIR and are familiar with Nix), and (2) to let you know I do have it running if it is of interest.

KrisThielemans

I probably should learn more about nix, but ...

I guess there's 3 options:

you maintain it (a good reason to stay around?)
it's cut down as much as possible and you supply minimal instructions and we say it is unmaintained
we don't merge

Does this need to be in the home directory? If not, it'd make option 2 more attractive (dir can have its own README etc)

KrisThielemans · 2025-02-26T07:53:55Z

default.nix

+
+  # This is a hackaround because STIR requires source available at runtime.
+  setSourceRoot = ''
+    actualSourceRoot=;


strange indentation?

''
blah
blah
''
Is a multi-line string. It is customary in Nix to indent multiline scripts to 2 spaces more than the outer scope for readability. The variable setSourceRoot contains a bash snippet to be run, that should set the sourceRoot variable, pointing to the root of the source. This is usually provided, I can't recall what problem I was solving here but happy to play around with it if we're keeping it.

KrisThielemans · 2025-02-26T07:54:45Z

default.nix

+  propagatedBuildInputs = [ swig ]
+    ++ lib.optional buildPython [ python numpy ];
+
+  # This is a hackaround because STIR requires source available at runtime.


really? Which ones? (only registries, which in CMake are now being handled by object libraries, so no longer source)

Yes, registries. That's great, I'll test it out without

KrisThielemans · 2025-02-26T07:55:34Z

default.nix

+   ] ++ lib.optionals (buildPython) [
+    "-DBUILD_SWIG_PYTHON=ON"
+   ] ++ lib.optionals (!stdenv.hostPlatform.isDarwin) [
+    "-DOPENGL_INCLUDE_DIR=${lib.getInclude libGL}/include"  # Borrowed from vtk


used by what?

I believe this is side-stepping a bug or something that should be covered by nixpkgs' vtk, it shouldn't be needed.

STIR build failed in including vtk libs which were trying to include OpenGL. I didn't spend too long digging in, I should run again without this an post the error at a minimum.

KrisThielemans · 2025-02-26T07:56:38Z

default.nix

+  '';
+  postInstall = ''
+    # add scripts to bin
+    find $src/scripts -type f ! -path "*maintenance*" -name "*.sh"  -exec cp -fn {} $out/bin \;


Not all are called *.sh but this should have been done by CMake anyway.

This, again, is likely either outdated or a kludge for a missing script I needed that was missed. I'll run with/without and check the output/bin diff

KrisThielemans · 2025-02-26T07:57:14Z

default.nix

+  meta = with lib; {
+    description = "STIR - Software for Tomographic Image Reconstruction";
+    homepage = http://stir.sourceforge.net;
+    license = with licenses; [ lgpl21 gpl2 free ];  # free = custom PARAPET license


apache 2 these days. Hopefully no gpl stuff around anymore

KrisThielemans · 2025-02-26T07:58:42Z

default.nix

+{ stdenv
+, lib
+, fetchFromGitHub
+, cmake , boost , itk , swig, libGL


who needs libGL?

Used in the line

"-DOPENGL_INCLUDE_DIR=${lib.getInclude libGL}/include" # Borrowed from vtk

See comment there: probably compensating for an issue with the vtk in nixpkgs (used by itk)

KrisThielemans · 2025-02-26T07:59:45Z

default.nix

+  '';
+  cmakeFlags = [
+    "-DBUILD_TESTING=ON"
+    "-DGRAPHICS=PGM"


any reason not to use None? Not sure if anyone is use the pgm support and how long it will survive

No reason, thanks

KrisThielemans · 2025-02-26T08:02:01Z

default.nix

+    "-DSTIR_MPI=OFF"
+    "-DSTIR_OPENMP=${if stdenv.isDarwin then "OFF" else "ON"}"
+   ] ++ lib.optionals (buildLibs) [
+    "-DBUILD_SHARED_LIBS=ON"


I doubt this works.

Why? I'll have to check. I believe I needed it to build SIRF against STIR, but made it optional here in the hopes it builds faster and with a smaller closure.

There's #6 (our almost oldest issue!) . It's possible that it works on Linux, but definitely not on other systems.

You surely don't need it for SIRF (VM, docker, GHA and all builds that I know of don't use it).

It should be a lot faster to build though, so if it works, you could keep it (but change the name of the variable!)

KrisThielemans · 2025-02-26T08:02:37Z

flake.nix

+      packages = forAllSystems (system:
+        let pkgs = nixpkgsFor.${system}; in with pkgs; rec {
+          stir = callPackage ./default.nix { inherit (python3Packages) python numpy; };
+          stir-nolibs = callPackage ./default.nix { buildLibs = false; buildPython = false; };


confusing name for something which actually uses shared libs and no python.

KrisThielemans · 2025-02-26T08:02:59Z

flake.nix

+        let
+          pkgs = nixpkgsFor.${system};
+          # Which STIR package to use
+          package = self.packages.${system}.stir-nolibs;


why this one?

Would you typically want BUILD_SHARED_DEPS=ON in your development env? I turn it off to build faster, though I can't say I acutally compared the build time for STIR recently.

ashgillman · 2025-02-26T10:57:54Z

If you're happy with 1, now I think about it, I'd be quite happy to maintain the nix components in perpetuity. Once set up changes are simple.

Does this need to be in the home directory?

You mean the project root? Good question. Looks like it: NixOS/nix#4414

I think a user would then have to use something like nix shell github:UCL/STIR?dir=nix

ashgillman · 2025-02-26T10:58:54Z

^Except I can't guarantee I check my GitHub notification all the time, there's a good chance I'd need an email :p

KrisThielemans · 2025-02-26T11:05:32Z

I'd be quite happy to maintain the nix components in perpetuity. Once set up changes are simple.

some version stuff at least... unless that can be automated (but I don't see how)

I think a user would then have to use something like nix shell github:UCL/STIR?dir=nix

if that'd work, I'd be happier with that. Not standard of course, but fine.

BLD: Add Nix flake with package and devShell [skip ci]

6e488fe

KrisThielemans requested changes Feb 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUILD: Add Nix flake with package and devShell #1572

BUILD: Add Nix flake with package and devShell #1572

ashgillman commented Feb 26, 2025 •

edited

Loading

KrisThielemans left a comment

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025 •

edited

Loading

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

KrisThielemans Feb 26, 2025

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

KrisThielemans Feb 26, 2025

KrisThielemans Feb 26, 2025

KrisThielemans Feb 26, 2025

ashgillman Feb 26, 2025

ashgillman commented Feb 26, 2025

ashgillman commented Feb 26, 2025

KrisThielemans commented Feb 26, 2025

BUILD: Add Nix flake with package and devShell #1572

Are you sure you want to change the base?

BUILD: Add Nix flake with package and devShell #1572

Conversation

ashgillman commented Feb 26, 2025 • edited Loading

KrisThielemans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashgillman Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashgillman commented Feb 26, 2025

ashgillman commented Feb 26, 2025

KrisThielemans commented Feb 26, 2025

ashgillman commented Feb 26, 2025 •

edited

Loading

ashgillman Feb 26, 2025 •

edited

Loading