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 a clean nix/nixos development enviroment #163

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

Conversation

Eveeifyeve
Copy link

@Eveeifyeve Eveeifyeve commented Jan 21, 2025

Check description.

Description

Adds a nix/nixos development with automation though github actions with

Motivation and Context

This change is required because there is currently no default dev enviroment for nix/nixos It solves having an environment to use when developing ferrum.

There was previously a pr for adding a flake here, but a follow-up is needed because was a couple problems,

  1. it uses flake utils which is a non module based system which means it makes it harder to make things compatible plus is a non nix way of doing.
  2. building the package inside of the devshell is not really required as the user can run cargo build --release and then use the bin produced.
  3. it's more important to have the cargo lock file checked in for reproducability but also other packages managers and ci for referencing the version and is reccomended by the rust team here even if it's a binary or library.

so this is a followup from #125

How has this been tested?

Local using direnv and nix develop as well as nix flake check plus I have also built in a flake check for each pr that is created.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (restructuring code, without changing its behavior)
  • Add repository changes

Checklist:

  • My code follows the code style of this project.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Clippy passes with no warnings.

@Eveeifyeve Eveeifyeve force-pushed the nix-flake branch 2 times, most recently from fb7c730 to ba36a01 Compare January 21, 2025 15:25
flake.nix Outdated Show resolved Hide resolved
make flake check action check all systems

added assignment to assign myself to flake update prs.

fix actions to use determinsys

switch nix formatting check to flake check

fix formatting for nix
@nicholaschiasson
Copy link

I think you got things backwards with the Cargo.lock. You want to check your Cargo.lock into version control for binaries, not libraries.

@Eveeifyeve
Copy link
Author

Eveeifyeve commented Jan 24, 2025

I think you got things backwards with the Cargo.lock. You want to check your Cargo.lock into version control for binaries, not libraries.

No I think it's okay to have it in version control even if it's a binary & libraries see more at https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control

@nicholaschiasson
Copy link

nicholaschiasson commented Jan 25, 2025

No I think it's okay to have it in version control even if it's a binary & libraries see more at https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control

Indeed, maybe to rephrase: it is more important, if anything, to check in the Cargo.lock for binaries, than for libraries. I was pointing it out because you stated in the PR description the opposite.

Also, and you're free to disagree on this of course, but I would have thought the Cargo.lock would be important for a nix setup (flake or not) given the mission statement of nix/nixpkgs/nixos being to guarantee reproducibility. For this reason, I kind of thought you would want the dependency versions pinned if it makes sense to do so (which it does since this repo outputs a binary). I'm definitely not a nix expert though so please feel free to share your thoughts on this.

@Eveeifyeve
Copy link
Author

Eveeifyeve commented Jan 25, 2025

No I think it's okay to have it in version control even if it's a binary & libraries see more at https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control

Indeed, maybe to rephrase: it is more important, if anything, to check in the Cargo.lock for binaries, than for libraries. I was pointing it out because you stated in the PR description the opposite.

I will rephase as it's my fault originally as I was rushing this pr before I go to asleep.

Also, and you're free to disagree on this of course, but I would have thought the Cargo.lock would be important for a nix setup (flake or not) given the mission statement of nix/nixpkgs/nixos being to guarantee reproducibility. For this reason, I kind of thought you would want the dependency versions pinned if it makes sense to do so (which it does since this repo outputs a binary). I'm definitely not a nix expert though so please feel free to share your thoughts on this.

Yes, but when you use release tags the versions are pinned by github.
I also forgot to enable it.

@Sweattypalms
Copy link
Member

Is this completed?

@Eveeifyeve
Copy link
Author

Is this completed?

Almost, Just need to fix the github action caches then it's ready to merge.

@Eveeifyeve
Copy link
Author

Eveeifyeve commented Feb 2, 2025

@Sweattypalms Okay i've done all of my changes, Ready to merge if you wanted to.

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.

4 participants