build: create a nix flake for development #411
Conversation
|
Would it be possible to combine or reuse the recipe from https://search.nixos.org/packages?channel=25.11&query=bpfilter ? |
fzakaria
left a comment
fzakaria
left a comment
There was a problem hiding this comment.
Looks reasonable but tough to say without testing it and the workflow exactly.
I would add a .github workflow to build your shell and maybe run a command in it (maybe even the build!) to validate it works.
Yay for Nix!
flake.nix
Outdated
|
|
||
| # Documentation (though this doesn't quite work yet) | ||
| doxygen | ||
| pythonEnv |
There was a problem hiding this comment.
I would add: venvShellHook
I found moving more to venv when possible is better for more "normal" IDE.
venvShellHook to source a Python 3 venv at the venvDir location. A venv is created if it does not yet exist. postVenvCreation can be used to to run commands only after venv is first created.
There was a problem hiding this comment.
I'm still having trouble with make doc - perhaps because nix doesn't have all the packages required and we can't install all these with pip because cmake still requires things like sphinx.
There was a problem hiding this comment.
-DNO_DOCS=1 will disable the Python dependencies (or at least it should!). Does nix allows for pip install ... within the env? That would solve it. We do it for Ubuntu already, as not all the required Python dependencies are packaged.
| ]); | ||
|
|
||
| in { | ||
| devShells.default = pkgs.mkShell { |
There was a problem hiding this comment.
do you plan to build it too?
Feel free to look at NixOS/nixpkgs#474393
I did this for another similar project.
I tried to upstream it d-e-s-o/bpflint#70 but @d-e-s-o thought it was better to just upstream simpler to nixpkgs
There was a problem hiding this comment.
As @pzmarzly pointed out, someone already added it as a nixpkg - https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/bp/bpfilter/package.nix
I'll probably copy parts from their flake for the build stuff.
flake.nix
Outdated
| clang | ||
| clang-tools # clang-tidy, clang-format | ||
| include-what-you-use | ||
| gcc |
There was a problem hiding this comment.
did this work okay having both?
Usually they have conflicts with shared bin symlinks like cc.
There was a problem hiding this comment.
Yeah, it didn't seem to mind. clang is used to compile the codegen BPF progs
There was a problem hiding this comment.
You should probably ensure CMake uses GCC until we move to Clang.
|
|
||
| shellHook = '' | ||
| # Add libbpf headers to include path for clang-tidy | ||
| export CPATH="${pkgs.libbpf}/include''${CPATH:+:$CPATH}" |
There was a problem hiding this comment.
if you add the .dev output of a library, it should already be in the NIX_INCLUDE_DIR and NIX_C_FLAGS etc.. via a hook..
There was a problem hiding this comment.
This is just for clang-tidy. But you're saying I can possibly remove this by using libbpf-dev?
This is so we don't have to rely on manual installation of dependencies to develop/build bpfilter, which can be problematic for many reasons (more details on https://nixos.org/). This also allows development on other linux distros.
|
I offered to pair with @jordalgo on Friday (or sooner)! We can test it on my laptop which is NixOS which makes it very obvious whether it works since only Nix build software works 🙃 |
qdeslandes
left a comment
qdeslandes
left a comment
There was a problem hiding this comment.
Here is my opinion on this as a non-Nix user. I don't mind having support for Nix, if it's useful to some, great.
However, as I don't use it (for now), I don't want the burden of maintaining it, so someone needs to take care of this part (there is a .github/CODEOWNERS file for this).
It must be clear that (for now), Nix is not officially supported. I can commit to solving issues with CMake on Fedora (which is supported), but I can't for Nix (because we support CMake, and I don't know Nix).
Regarding the new files:
- Can
flake.lockandflake.nixbe hidden? They do not apply to everyone, being able to use.flake.lockis a clear message to people building the project. - Is
flake.lockstandard practice? We support Fedora and Ubuntu, not specific package versions, so we might lock some dependencies for which bpfilter builds, but end up broken with the latestbpftoolfor example.
Overall, I think Nix support could be beneficial, especially for CI (to make it simpler, and potentially faster). However, because we support Fedora and Ubuntu, we still need to build for those systems. Any way for Nix to install the versions of the deps distributed in Ubuntu or Fedora? Kinda like what mkosi does.
Also, pulling @SkohTV in, I remember him trying to sell me Nix shell, he might be interested in this PR!
There was a problem hiding this comment.
This should probably have its dedicated workflow.
There was a problem hiding this comment.
As a Nix advocate, I offered to work with @jordalgo to help curate the Nix recipe as best I can.
A good end state will be:
- The ability to build with Nix
nix build
For instance you can have people try out bpfilter via ❯ nix run github:facebok/bpflter on any Nix machine and have it reproducible (not bit-wise) and include it's own libc/loader nicely validated with a reproducible hash in the path.
- The ability to give you a reproducible developer environment
nix develop. Same strong guarantees as the build but taken 1 edge further to the inputs needed to build the package.
If you need to support traditional filesystem hierarchy codebases like Ubuntu, then probably having a duplicate workflow is unavoidable. The primitives are too orthogonal since Nix eschews all the FHS stuff.
Good news is that Nix the package manager is now a Fedora package though officially :P
Cool to see someone up-streamed it to Nixpkgs though already!
4 participants
This is so we don't have to rely on manual installation of dependencies to develop/build bpfilter, which can be problematic for many reasons (more details on https://nixos.org/). This also allows development on other linux distros.