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

Composable phases #296544

Open
SomeoneSerge opened this issue Mar 17, 2024 · 4 comments
Open

Composable phases #296544

SomeoneSerge opened this issue Mar 17, 2024 · 4 comments
Labels
6.topic: architecture Relating to code and API architecture of Nixpkgs 6.topic: stdenv Standard environment

Comments

@SomeoneSerge
Copy link
Contributor

SomeoneSerge commented Mar 17, 2024

"Compose implementations of the same phase"

Issue description

Setup hooks (cmake, meson, pypa-build, etc) defining generic implementations for the common phases (configurePhase, buildPhase, installPhase, etc) weren't designed to be composed: cmake's setup-hook only injects its implementation of the configurePhase if the "slot" hasn't been occupied yet by someone else (e.g. meson). Moreover, since pre- and post-hooks are usually executed by the phase implementations (rather than by the caller), one couldn't "compose" different implementations of the same phase even manually without running the same hooks twice.

The particular issue with runHooks could be addressed by somehow moving the runHooks to generic/setup.sh, but more importantly we need a discussion on how to better accommodate these situations, and on how far to go in doing so (the "composition" of conflicting phases will likely always be manual)

Use-cases

dust3r

For instance, it's currently hard to reuse the functionality of cmakeBuildPhase and pypaBuildPhase in the same derivation. Here's a mildly cursed example of a package generating a setup.py using cmake where the phases have been composed "manually": https://github.com/SomeoneSerge/dust3r.nix/blob/4fd7d61310f2289ba9843c9854aaaf59837eafbd/nix/magnum-bindings/package.nix#L41-L56.

conda (libmambapy)

Here's a fresh example of a PR handling the non-trivial interaction of cmake and pypa-build: https://github.com/NixOS/nixpkgs/pull/296461/files#diff-dffa404ae86470475e60f1461b64526cb09a4bc5409a2e3d225fb262cbd81505R30-R33. This demonstrates that the tools nixpkgs offers are lacking for this kind of use-cases.

opencv

Nixpkgs' opencv expression is notorious:

+ lib.optionalString enablePython ''
pushd $NIX_BUILD_TOP/$sourceRoot/modules/python/package
python -m pip wheel --verbose --no-index --no-deps --no-clean --no-build-isolation --wheel-dir dist .
pushd dist
python -m pip install ./*.whl --no-index --no-warn-script-location --prefix="$out" --no-cache
# the cv2/__init__.py just tries to check provide "nice user feedback" if the installation is bad
# however, this also causes infinite recursion when used by other packages
rm -r $out/${pythonPackages.python.sitePackages}/cv2

cuda-samples

Here's a monorepo-like project with a top-level Makefile and some of the subprojects using CMake:

Prior art

TBD

@SomeoneSerge SomeoneSerge changed the title Make buildPhases (&c) composable Compose implementations of the same phase Mar 17, 2024
@SomeoneSerge SomeoneSerge added the 6.topic: stdenv Standard environment label Mar 17, 2024
@lolbinarycat
Copy link
Contributor

possible solution: replace buildPhase=pypaBuildPhase with postBuildHooks+=(pypaBuildPhase)

would likely need other changes too.

@SomeoneSerge
Copy link
Contributor Author

SomeoneSerge commented Mar 17, 2024

possible solution: replace buildPhase=pypaBuildPhase with postBuildHooks+=(pypaBuildPhase)
would likely need other changes too.

Yes it would, because as is this should result in an infinite recursion (pypaBuildPhase executes runHook postBuild, which runs pypaBuildPhase)

We need a different kind of primitives in terms of which to implement the xxxBuildPhases, so the primitive could be reused outside "common" phases

@lolbinarycat
Copy link
Contributor

do we need a different primitive, or do we just need to remove the runHook?

@SomeoneSerge SomeoneSerge added the 6.topic: architecture Relating to code and API architecture of Nixpkgs label Mar 19, 2024
@SomeoneSerge
Copy link
Contributor Author

do we need a different primitive, or do we just need to remove the runHook?

All of the other phases (including those potentially implemented out of tree) presumably use the same pattern

@SomeoneSerge SomeoneSerge changed the title Compose implementations of the same phase Compsable phases Aug 23, 2024
@SomeoneSerge SomeoneSerge changed the title Compsable phases Composable phases Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: architecture Relating to code and API architecture of Nixpkgs 6.topic: stdenv Standard environment
Projects
None yet
Development

No branches or pull requests

2 participants