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

add nix flake #30

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

add nix flake #30

wants to merge 4 commits into from

Conversation

nebunebu
Copy link

@nebunebu nebunebu commented Aug 4, 2024

adds files flake.nix and package.nix and updates README.md with explanation on how to run with nix and how to install on NixOS.

@yanganto
Copy link
Contributor

Could you put a dev shell for the project? It will be nice for anyone want to join the project.

@nebunebu
Copy link
Author

@yanganto I added a devShell with the local build of serie, and also rustfmt and rustup

@yanganto
Copy link
Contributor

yanganto commented Aug 14, 2024

If we want to build a nix package for serie, we do not need to add flake here. We can make a PR on https://github.com/NixOS/nixpkgs/, which is an official way.

Why do people use nix/flake for their projects?
Because we want to make the build, the development, and the CI server in the same environment.

For a Rust project, we will keep the Rust version, fmt, clippy, and all the dependencies the same from the build system, and the dev shell for development, and use it in CI.

The flake is introduced here, and what do we want for this project?
The build and the development environment are not the same, so it is not reproducible and not beneficial to serie in this PR.

@nebunebu
Copy link
Author

It's already been added to nixpkgs. I appreciate the consideration you gave to this contribution. Feel free to close the request at your leisure.

@alerque
Copy link
Contributor

alerque commented Aug 14, 2024

Having a flake serves a completely different purpose as having a package in nixpkgs. Both can be used by and end user just to run an app, but they are fundamentally different and having both is a good thing. The nix package is great to have both on Nix OS and anywhere nix will run as a way to run a pre-approved packaged version (often prebuilt in a cache somewhere). The Flake is useful to be able to execute the repository as if it was the app at any point in history (tag, branch, commit hash, etc.). This is great for testing PRs, checking out a new feature before it is merged or packaged, etc. Additionally having the dev shell feature in the flake makes it easy to contribute back to the project by making sure to have all the development, testing, and linting tooling handy. The project itself can choose to use it or not, but for contributors that use Nix it's quite handy.

@yanganto
Copy link
Contributor

yanganto commented Aug 16, 2024

@alerque
Yes, I agree with you. Flake provides a consistent environment for all builds (and easy to use branch, tag, and event with a commit). I love combining the Rust with Nix to enhance a project. I am not against flake here, but the strong power of nix and flake needs you to lock things correctly. The thing I point out is that the current implementation is not correct, and let me clarify this in details.

The following are issues if you use rustup in dev shell, and it does not help and makes issues.

First, rustup will download rust and cargo, and it will have a side effect, no one can guarantee any version of rust downloaded by rustup can run this project, such that Nix loses its reproducibility guarantee. In other words, the rust to build the package is locked in flake.lock, but the rust in the dev shell is not the same thing, so it will not be reproducible.

Second, if a system already has cargo managed by another existing rustup, the rustup of the dev shell will get conflict with it. Because rustup will not install things inside the nix shell but out side of it. This will conflict if you develop more than one Rust projects with nix dev shell using rustup.

Last, installation of rustup is easy with the one-line command

curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh

It is no a good reason for a developer to install nix and get rustup only, the developer will want the exact rust and all dependencies for the project from the nix dev shell.

flake.nix Outdated
packages = [
inputs.self.packages.${system}.serie
pkgs.rustfmt
pkgs.rustup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the comments, the dev shell should use the pinned version of Rust from the flake, not rustup.

@nebunebu
Copy link
Author

@alerque @yanganto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants