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

Merged
merged 3 commits into from
Dec 9, 2024
Merged

Conversation

markoburcul
Copy link
Contributor

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

@markoburcul markoburcul requested a review from jakubgs November 26, 2024 13:31
@markoburcul markoburcul marked this pull request as ready for review November 27, 2024 15:16
Copy link

@jakubgs jakubgs left a comment

Choose a reason for hiding this comment

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

Good stuff.

nix/version.nix Outdated Show resolved Hide resolved
flake.nix Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
Copy link

@jakubgs jakubgs left a comment

Choose a reason for hiding this comment

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

Aside from a few nitpicks and some trailing whitespace this looks very good.

flake.nix Outdated Show resolved Hide resolved
nix/default.nix Show resolved Hide resolved
flake.nix Outdated
Comment on lines 15 to 19
forEach = nixpkgs.lib.genAttrs;
forAllSystems = forEach stableSystems;
pkgsFor = forEach stableSystems (
system: import nixpkgs { inherit system; }
);
Copy link

Choose a reason for hiding this comment

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

Still don't get why this change happened.

nix/default.nix Outdated Show resolved Hide resolved
Comment on lines +29 to +27
codex = build ["all"];
default = codex;
Copy link

Choose a reason for hiding this comment

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

One weird thing I found when testing was that if I specify target with #codex or #default it builds, but if I leave that out then it fails with:

 > nix build --print-build-logs --print-out-paths '.?submodules=1'
warning: Git tree '/home/jakubgs/work/nim-codex' is dirty
codex> Running phase: unpackPhase
codex> unpacking source archive /nix/store/k9ri8k88313z6ggkcm4k95aaapqw351i-source
codex> source root is source
codex> Running phase: patchPhase
codex> Running phase: updateAutotoolsGnuConfigScriptsPhase
codex> Running phase: configurePhase
codex> Running phase: buildPhase
codex> build flags: SHELL=/nix/store/516kai7nl5dxr792c0nzq0jp8m4zvxpi-bash-5.2p32/bin/bash all V=2 USE_SYSTEM_NIM=1
codex> Git submodules not found. Running 'git submodule update --init --recursive'.
codex> 0.1.0-dirty
codex> make: Nothing to be done for 'all'.
codex> Running phase: installPhase
codex> cp: cannot stat 'build/codex': No such file or directory

Surprisingly .?submodules=1# works fine, so that smells like a nix interpreter bug that ignores the submodules arg.

@markoburcul markoburcul force-pushed the create-derivation branch 6 times, most recently from b0281b0 to 817dbb9 Compare December 9, 2024 15:42
Create a structure for nix files. Add the derivation file which is using
system Nim to compile Codex.

Referenced issue: #940

Signed-off-by: markoburcul <[email protected]>
Include commit which allows building circom-compat-ffi using Nix(doesn't
affect current usage of the submodule).

Referenced issue: #940

Signed-off-by: markoburcul <[email protected]>
@markoburcul markoburcul enabled auto-merge December 9, 2024 16:01
@markoburcul markoburcul added this pull request to the merge queue Dec 9, 2024
Merged via the queue into master with commit 0c6784d Dec 9, 2024
17 checks passed
@markoburcul markoburcul deleted the create-derivation branch December 9, 2024 18:39
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.

2 participants