-
-
Notifications
You must be signed in to change notification settings - Fork 54
Add a clean nix/nixos development enviroment #163
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
Conversation
fb7c730
to
ba36a01
Compare
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
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 |
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. |
I will rephase as it's my fault originally as I was rushing this pr before I go to asleep.
Yes, but when you use release tags the versions are pinned by github. |
Is this completed? |
Almost, Just need to fix the github action caches then it's ready to merge. |
@Sweattypalms Okay i've done all of my changes, Ready to merge if you wanted to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff --git a/flake.nix b/flake.nix
index cd51e48..4c69f92 100644
--- a/flake.nix
+++ b/flake.nix
@@ -1,10 +1,15 @@
{
inputs = {
nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
- flake-parts.url = "github:hercules-ci/flake-parts";
- flake-compat = {
- url = "github:edolstra/flake-compat";
- flake = false;
+ systems.url = "github:nix-systems/default";
+ flake-compat.url = "github:edolstra/flake-compat";
+ treefmt-nix = {
+ url = "github:numtide/treefmt-nix";
+ inputs.nixpkgs.follows = "nixpkgs";
+ };
+ flake-parts = {
+ url = "github:hercules-ci/flake-parts";
+ inputs.nixpkgs-lib.follows = "nixpkgs";
};
rust-overlay = {
url = "github:oxalica/rust-overlay";
@@ -14,13 +19,19 @@
outputs =
inputs@{
+ systems,
flake-parts,
+ treefmt-nix,
nixpkgs,
rust-overlay,
...
}:
flake-parts.lib.mkFlake { inherit inputs; } {
- systems = nixpkgs.lib.systems.flakeExposed;
+ systems = import systems;
+
+ imports = [
+ treefmt-nix.flakeModule
+ ];
perSystem =
{
@@ -28,51 +39,27 @@
system,
...
}:
+ let
+ rustToolchain = pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml;
+ in
{
- formatter = nixpkgs.legacyPackages.${system}.nixfmt-rfc-style;
_module.args.pkgs = import inputs.nixpkgs {
inherit system;
- overlays = [
- rust-overlay.overlays.default
- (self: super: {
- rustToolchain =
- let
- rust = super.rust-bin;
- in
- if builtins.pathExists ./rust-toolchain.toml then
- rust.fromRustupToolchainFile ./rust-toolchain.toml
- else if builtins.pathExists ./rust-toolchain then
- rust.fromRustupToolchainFile ./rust-toolchain
- else
- rust.nightly.latest.default;
- })
- ];
- config = { };
+ overlays = [ rust-overlay.overlays.default ];
};
- # Used to check formatting for nix specificly
- checks.fmt-check =
- pkgs.runCommand "format-check"
- {
- src = ./.;
- doCheck = true;
- nativeBuildInputs = [
- pkgs.nixfmt-rfc-style
- ];
- }
- ''
- nixfmt --check .
- touch $out
- '';
+ treefmt = {
+ projectRootFile = "flake.lock";
+ programs.nixfmt.enable = true;
+ };
devShells.default = pkgs.mkShell {
- packages = with pkgs; [
+ packages = [
rustToolchain
- pkg-config
- openssl
+ pkgs.pkg-config
+ pkgs.openssl
];
};
-
};
};
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I suggest to use
numtide/treefmt-nix
to create a formatter. - I also suggest to use
nix-systems/default
to reference available systems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use nix-systems can I ask because nixpkgs.lib.systems.flakeExposed
does basically the same thing but with an extra dependancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<nixpkgs>/lib/systems/flake-systems.nix
This file contains not only x86_64-linux
and aarch64-linux
, but also i686-linux
, powerpc64le-linux
and riscv64-linux
. Are we even needed to support powerpc64le-linux
& riscv64-linux
systems?
The nix-systems/default
contains only these systems. If we don't support Darwin systems, we can use nix-systems/default-linux
.
# github:nix-systems/default
[
"aarch64-darwin"
"aarch64-linux"
"x86_64-darwin"
"x86_64-linux"
]
# github:nix-systems/default-linux
[
"aarch64-linux"
"x86_64-linux"
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want support for windows, linux and mac, with linux and mac being able to work on ARM64 systems. I have no idea how nix works so if you guys can make that work and are happy to maintain it, I'll merge it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ReCore-sys
Hi. For now, I created a sample branch for @Eveeifyeve , to check my above patch. Please check the commit. If you want the way to check the commit, Feel free to ask me!!
(Now the commit is not include the GitHub Actions, because I don't know that @ReCore-sys really want to check via Nix flakes)
haruki7049@44a9302
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. For now I'd rather not have a CI step for nix flakes since I'm already fighting tooth and nail to get the windows build times to a reasonable level. Either @Eveeifyeve can include your changes or you are welcome to open your own PR and I'll merge that instead, whatever works out best for you guys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that is better for another pr and GitHub actions don't seem right for this job and I am actually removing this or changing it to not open prs but make a commit straight. I have let a lone 30+ prs from GitHub actions updating flakes it does get cluttered very quick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. @haruki7049 if you wanna open another PR without the CI modifications I'll merge that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ReCore-sys @Eveeifyeve
#185
Created the PR. Please check.
Merged @haruki7049's PR instead |
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,
cargo build --release
and then use the bin produced.so this is a followup from #125
How has this been tested?
Local using direnv and
nix develop
as well asnix flake check
plus I have also built in a flake check for each pr that is created.Screenshots (if appropriate):
Types of changes
Checklist: