nix: make derivation and update shell #1003
Conversation
Create a structure for nix files. Add the derivation file which is using system Nim to compile Codex. Move shell definition to a specific file. Referenced issue: #940 Signed-off-by: markoburcul <[email protected]>
Signed-off-by: markoburcul <[email protected]>
Signed-off-by: markoburcul <[email protected]>
markoburcul
force-pushed
the
create-derivation
branch
from
fadb206
to
a5bc0bd
Compare
| let | ||
| tools = pkgs.callPackage ./tools.nix {}; | ||
| source = ../codex.nimble; | ||
|
|
||
| version = tools.findKeyValue "version = \"([0-9]+\.[0-9]+\.[0-9]+)\"" source; | ||
| in | ||
| "${version}" |
There was a problem hiding this comment.
The version variable is pointless, and so it interpolating it after the let/in block.
| let | |
| tools = pkgs.callPackage ./tools.nix {}; | |
| source = ../codex.nimble; | |
| version = tools.findKeyValue "version = \"([0-9]+\.[0-9]+\.[0-9]+)\"" source; | |
| in | |
| "${version}" | |
| let | |
| tools = pkgs.callPackage ./tools.nix {}; | |
| in | |
| tools.findKeyValue "version = \"([0-9]+\.[0-9]+\.[0-9]+)\"" ../codex.nimble; |
Honestly, this is so short you could just put this directly in nix/default.nix.
| @@ -3,33 +3,36 @@ | |||
|
|
|||
| inputs = { | |||
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.05"; | |||
| circom-compat.url = "github:codex-storage/circom-compat-ffi"; | |||
There was a problem hiding this comment.
We can't reference it by relative path? Might be possible:
| inherit stableSystems; | ||
| inherit circomCompatPkg; |
There was a problem hiding this comment.
| inherit stableSystems; | |
| inherit circomCompatPkg; | |
| inherit stableSystems circomCompatPkg; |
| nativeBuildInputs = let | ||
| # Fix for Nim compiler calling 'git rev-parse' and 'lsb_release'. | ||
| fakeGit = writeScriptBin "git" "echo ${version}"; | ||
| fakeLsbRelease = writeScriptBin "lsb_release" "echo nix"; |
There was a problem hiding this comment.
Why are you using this instead of lsb-release package?
| devShells = forAllSystems (system: { | ||
| default = pkgsFor.${system}.callPackage ./nix/shell.nix { }; | ||
| }); |
There was a problem hiding this comment.
We could just use the package itself to get necessary tools, and add others using buildInputes:
| devShells = forAllSystems (system: { | |
| default = pkgsFor.${system}.callPackage ./nix/shell.nix { }; | |
| }); | |
| devShells = forAllSystems (system: let | |
| pkgs = pkgsFor.${system}; | |
| in { | |
| default = pkgs.mkShell { | |
| inputsFrom = [ | |
| packages.${system}.codex | |
| circom-compat.packages.${system}.default | |
| ]; | |
| buildInputs = with pkgs; [ git nodejs_18 ]; | |
| }; | |
| }); |
Are we even using this llvmPackages.openmp thing? I don't see it in the build. Built fine without it on my host.
| "x86_64-linux" "aarch64-linux" | ||
| "x86_64-darwin" "aarch64-darwin" | ||
| ], | ||
| circomCompatPkg |
There was a problem hiding this comment.
We could add a default fallback here too:
| circomCompatPkg | |
| circomCompatPkg ? ( | |
| builtins.getFlake "github:codex-storage/circom-compat-ffi" | |
| ).packages.${builtins.currentSystem}.default |
| curl | ||
| bash |
There was a problem hiding this comment.
Bash is a part of stdenv, and curl makes no sense.
| openssl | ||
| gmp |
There was a problem hiding this comment.
Aren't we already adding this in buildInputs?
| inherit (pkgs) stdenv lib writeScriptBin callPackage; | ||
|
|
||
| revision = lib.substring 0 8 (src.rev or "dirty"); | ||
| in pkgs.stdenv.mkDerivation rec { |
There was a problem hiding this comment.
Shouldn't we be enforcing GCC11?
| in pkgs.stdenv.mkDerivation rec { | |
| in pkgs.gcc11Stdenv.mkDerivation rec { |
| fakeCargo = writeScriptBin "cargo" "echo ${version}"; | ||
| in | ||
| with pkgs; [ | ||
| gcc11 |
There was a problem hiding this comment.
This should be done using gcc11Stdenv.mkDerivation.
Create a structure for nix files. Add the derivation file which is using system Nim to compile Codex. Move shell definition to a specific file.
Referenced issue: #940