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

nix: make derivation and update shell #1003

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ ifeq ($(OS),Windows_NT)
ARCH = arm64
endif
else
UNAME_P := $(shell uname -p)
UNAME_P := $(shell uname -m)
ifneq ($(filter $(UNAME_P), i686 i386 x86_64),)
ARCH = x86_64
endif
Expand Down
37 changes: 36 additions & 1 deletion flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 22 additions & 19 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,36 @@

inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.05";
circom-compat.url = "github:codex-storage/circom-compat-ffi";
Copy link

Choose a reason for hiding this comment

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

We can't reference it by relative path? Might be possible:

};

outputs = { self, nixpkgs }:
outputs = { self, nixpkgs, circom-compat}:
let
supportedSystems = [
stableSystems = [
"x86_64-linux" "aarch64-linux"
"x86_64-darwin" "aarch64-darwin"
];
forAllSystems = f: nixpkgs.lib.genAttrs supportedSystems (system: f system);
pkgsFor = forAllSystems (system: import nixpkgs { inherit system; });
forEach = nixpkgs.lib.genAttrs;
forAllSystems = forEach stableSystems;
pkgsFor = forEach stableSystems (
system: import nixpkgs { inherit system; }
);
Comment on lines +15 to +19
Copy link

Choose a reason for hiding this comment

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

Not sure why this change happened.

in rec {
devShells = forAllSystems (system: let
pkgs = pkgsFor.${system};
inherit (pkgs) lib stdenv mkShell;
in {
default = mkShell.override { stdenv = pkgs.gcc11Stdenv; } {
buildInputs = with pkgs; [
# General
git pkg-config openssl lsb-release
# Build
rustc cargo nimble gcc11 cmake nim-unwrapped-1
# Libraries
gmp llvmPackages.openmp
# Tests
nodejs_18
];
packages = forAllSystems (system: let
circomCompatPkg = circom-compat.packages.${system}.default;
buildTarget = pkgsFor.${system}.callPackage ./nix/default.nix {
inherit stableSystems;
inherit circomCompatPkg;
Comment on lines +24 to +25
Copy link

Choose a reason for hiding this comment

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

Suggested change
inherit stableSystems;
inherit circomCompatPkg;
inherit stableSystems circomCompatPkg;

src = self;
};
build = targets: buildTarget.override { inherit targets; };
in rec {
codex = build ["all"];
default = codex;
});

devShells = forAllSystems (system: {
default = pkgsFor.${system}.callPackage ./nix/shell.nix { };
});
Comment on lines +34 to 36
Copy link

Choose a reason for hiding this comment

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

We could just use the package itself to get necessary tools, and add others using buildInputes:

Suggested change
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.

};
}
29 changes: 29 additions & 0 deletions nix/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Usage

## Shell

A development shell can be started using:
```sh
nix develop
```

## Building

To build a Codex you can use:
```sh
nix build '.?submodules=1#codex'
```
The `?submodules=1` part should eventually not be necessary.
For more details see:
https://github.com/NixOS/nix/issues/4423

It can be also done without even cloning the repo:
```sh
nix build 'github:codex-storage/nim-codex?submodules=1'
```

## Running

```sh
nix run 'github:codex-storage/nim-codex?submodules=1'
```
86 changes: 86 additions & 0 deletions nix/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
{
pkgs ? import <nixpkgs> { },
src ? ../.,
targets ? ["all"],
# Options: 0,1,2
verbosity ? 0,
# Use system Nim compiler instead of building it with nimbus-build-system
useSystemNim ? true,
commit ? builtins.substring 0 7 (src.rev or "dirty"),
# These are the only platforms tested in CI and considered stable.
stableSystems ? [
"x86_64-linux" "aarch64-linux"
"x86_64-darwin" "aarch64-darwin"
],
circomCompatPkg
Copy link

Choose a reason for hiding this comment

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

We could add a default fallback here too:

Suggested change
circomCompatPkg
circomCompatPkg ? (
builtins.getFlake "github:codex-storage/circom-compat-ffi"
).packages.${builtins.currentSystem}.default

}:

let
inherit (pkgs) stdenv lib writeScriptBin callPackage;

revision = lib.substring 0 8 (src.rev or "dirty");
in pkgs.stdenv.mkDerivation rec {
Copy link

Choose a reason for hiding this comment

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

Shouldn't we be enforcing GCC11?

Suggested change
in pkgs.stdenv.mkDerivation rec {
in pkgs.gcc11Stdenv.mkDerivation rec {


pname = "codex";

version = "${callPackage ./version.nix {}}-${revision}";

inherit src;

# Dependencies that should only exist in the build environment.
nativeBuildInputs = let
# Fix for Nim compiler calling 'git rev-parse' and 'lsb_release'.
fakeGit = writeScriptBin "git" "echo ${version}";
fakeLsbRelease = writeScriptBin "lsb_release" "echo nix";
Copy link

Choose a reason for hiding this comment

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

Why are you using this instead of lsb-release package?

# Fix for the nim-circom-compat-ffi package that is built with cargo.
fakeCargo = writeScriptBin "cargo" "echo ${version}";
in
with pkgs; [
gcc11
Copy link

Choose a reason for hiding this comment

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

This should be done using gcc11Stdenv.mkDerivation.

cmake
curl
bash
Comment on lines +41 to +42
Copy link

Choose a reason for hiding this comment

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

Bash is a part of stdenv, and curl makes no sense.

pkg-config
openssl
gmp
Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

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

Aren't we already adding this in buildInputs?

nimble
which
nim-unwrapped-1
circomCompatPkg
fakeGit
fakeCargo
fakeLsbRelease
];

# Disable CPU optmizations that make binary not portable.
NIMFLAGS = "-d:disableMarchNative -d:git_revision_override=${revision}";
# Avoid Nim cache permission errors.
XDG_CACHE_HOME = "/tmp";

makeFlags = targets ++ [
"V=${toString verbosity}"
"USE_SYSTEM_NIM=${if useSystemNim then "1" else "0"}"
];

# Dependencies that should exist in the runtime environment.
buildInputs = with pkgs; [
openssl
gmp
];

configurePhase = ''
patchShebangs . > /dev/null
'';

installPhase = ''
mkdir -p $out/bin
cp build/codex $out/bin/
'';

meta = with pkgs.lib; {
description = "Codex storage system";
homepage = "https://github.com/codex-storage/nim-codex";
license = licenses.mit;
platforms = stableSystems;
};
}
15 changes: 15 additions & 0 deletions nix/shell.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{ pkgs ? import <nixpkgs> {}}:

pkgs.mkShell.override { stdenv = pkgs.gcc11Stdenv; } {

buildInputs = with pkgs; [
# General
git pkg-config openssl lsb-release
# Build
rustc cargo nimble gcc11 cmake nim-unwrapped-1
# Libraries
gmp llvmPackages.openmp
# Tests
nodejs_18
];
}
15 changes: 15 additions & 0 deletions nix/tools.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{ pkgs ? import <nixpkgs> { } }:

let

inherit (pkgs.lib) fileContents last splitString flatten remove;
inherit (builtins) map match;
in {
findKeyValue = regex: sourceFile:
let
linesFrom = file: splitString "\n" (fileContents file);
matching = regex: lines: map (line: match regex line) lines;
extractMatch = matches: last (flatten (remove null matches));
in
extractMatch (matching regex (linesFrom sourceFile));
}
9 changes: 9 additions & 0 deletions nix/version.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{ pkgs ? import <nixpkgs> { } }:

let
tools = pkgs.callPackage ./tools.nix {};
source = ../codex.nimble;

version = tools.findKeyValue "version = \"([0-9]+\.[0-9]+\.[0-9]+)\"" source;
in
"${version}"
Comment on lines +3 to +9
Copy link

Choose a reason for hiding this comment

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

The version variable is pointless, and so it interpolating it after the let/in block.

Suggested change
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.

2 changes: 1 addition & 1 deletion vendor/nim-circom-compat