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

fix loading Nix Flake shell via direnv, update readme #6807

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

jakubgs
Copy link
Member

@jakubgs jakubgs commented Jan 2, 2025

And some other Nix related fixes.

@jakubgs jakubgs added the nix label Jan 2, 2025
@jakubgs jakubgs requested a review from tersec January 2, 2025 13:01
@jakubgs jakubgs self-assigned this Jan 2, 2025
@jakubgs jakubgs changed the title fix hash for Nimbus Nix derivation fix hash for Nimble Nix derivation Jan 2, 2025
@jakubgs jakubgs force-pushed the fix/nix-nimble-direnv branch from 9802295 to 20559b4 Compare January 2, 2025 13:09
@jakubgs jakubgs changed the title fix hash for Nimble Nix derivation fix hash for Nimble Nix derivation, re-add sat src Jan 2, 2025
@jakubgs jakubgs force-pushed the fix/nix-nimble-direnv branch 3 times, most recently from 440e8da to 0e15e01 Compare January 2, 2025 13:54
@jakubgs
Copy link
Member Author

jakubgs commented Jan 2, 2025

There is an issue with Nimble build and missing sat sources:

bin/nim c -o:bin/nimble -d:release --mm:refc --noNimblePath -d:release --skipUserCfg --skipParentCfg --warnings:off --hints:off dist/nimble/src/nimble.nim
/build/source/vendor/nimbus-build-system/vendor/Nim/dist/nimble/src/nimble.nim(14, 13) Error: cannot open file: sat/sat

This is caused by this import:

when defined(nimNimbleBootstrap):
  import ../dist/sat/src/sat/sat
else:
  import sat/sat

https://github.com/nim-lang/nimble/blob/46e2ae13eeb95619a371a74d16efc5aff30ea371/src/nimble.nim#L11-L14

Which hits the else because when I try to set -d:nimNimbleBootstrap=true in NIMFLAGS it doesn't seem to propagate to the build of Nimbus when using the nimbus-build-system. I'm not sure where sat sources are supposed to be for import sat/sat to work.

Copy link

github-actions bot commented Jan 2, 2025

Unit Test Results

       12 files  ±0    1 822 suites  ±0   55m 57s ⏱️ -16s
  5 327 tests ±0    4 980 ✔️ ±0  347 💤 ±0  0 ±0 
29 521 runs  ±0  29 077 ✔️ ±0  444 💤 ±0  0 ±0 

Results for commit 9583ed2. ± Comparison against base commit fb1c3ba.

♻️ This comment has been updated with latest results.

@jakubgs jakubgs force-pushed the fix/nix-nimble-direnv branch from 0e15e01 to bfec79e Compare January 2, 2025 21:24
@jakubgs
Copy link
Member Author

jakubgs commented Jan 2, 2025

I have checked and indeed the hash issues appear to be caused by some Nix caching mechanism which messes with results.

Indeed use of fetchSubmodules = true here should be sufficient to provide sat sources:

fetchSubmodules = true;

I need to research how to avoid these Nix caching confusion.

@jakubgs jakubgs changed the title fix hash for Nimble Nix derivation, re-add sat src fix loading Nix Flake shell via direnv, update readme Jan 2, 2025
@jakubgs jakubgs force-pushed the fix/nix-nimble-direnv branch from bfec79e to 79672bc Compare January 3, 2025 01:34
@tersec
Copy link
Contributor

tersec commented Jan 3, 2025

There is an issue with Nimble build and missing sat sources:

bin/nim c -o:bin/nimble -d:release --mm:refc --noNimblePath -d:release --skipUserCfg --skipParentCfg --warnings:off --hints:off dist/nimble/src/nimble.nim
/build/source/vendor/nimbus-build-system/vendor/Nim/dist/nimble/src/nimble.nim(14, 13) Error: cannot open file: sat/sat

This is caused by this import:

when defined(nimNimbleBootstrap):
  import ../dist/sat/src/sat/sat
else:
  import sat/sat

https://github.com/nim-lang/nimble/blob/46e2ae13eeb95619a371a74d16efc5aff30ea371/src/nimble.nim#L11-L14

Which hits the else because when I try to set -d:nimNimbleBootstrap=true in NIMFLAGS it doesn't seem to propagate to the build of Nimbus when using the nimbus-build-system. I'm not sure where sat sources are supposed to be for import sat/sat to work.

If you see the line 14 error, it's the wrong/old Nimble version. Nimble 0.16.2 attempts to import sat on either line 12 or 14; This code targets Nimble 0.16.4, which attempts to import sat on line 11.

@jakubgs
Copy link
Member Author

jakubgs commented Jan 3, 2025

Indeed, this is the hash caching behavior that we talked about. Did you review the PR?

nix/README.md Outdated Show resolved Hide resolved

mkdir -p .flake-profiles
eval "$(nix print-dev-env --profile ".flake-profiles/profile")"
if ! has nix_direnv_version || ! nix_direnv_version 3.0.6; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there something specific/special about version 3.0.6 or is this a way of pinning a semi-arbitrary version?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the same version we've been using in all of our infra-* repositories for our Nix Flake shell for a few months now:
https://github.com/status-im/infra-template/blob/3c85b88671a8b48c5d267801bf0ad678f0e739d4/.envrc#L1-L3

So it's known to be stable.

@tersec
Copy link
Contributor

tersec commented Jan 3, 2025

Indeed, this is the hash caching behavior that we talked about. Did you review the PR?

Mostly fine/reasonable, a couple questions

@jakubgs jakubgs force-pushed the fix/nix-nimble-direnv branch from 79672bc to 9583ed2 Compare January 3, 2025 09:18
@jakubgs jakubgs merged commit 9583ed2 into unstable Jan 3, 2025
12 checks passed
@jakubgs jakubgs deleted the fix/nix-nimble-direnv branch January 3, 2025 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants