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

flake: use dream2nix within nix-cargo-integration #1758

Merged

Conversation

the-mikedavis
Copy link
Member

closes #1757

possibly connects #1647 but I don't have an ARM machine to test on. @jdahm would you mind trying nix run github:the-mikedavis/helix/665e01b4639f87de1f33a1cfa99c97d6a3549fd0? I suspect the tree-sitter gcc failure is unrelated but it's worth a shot.

builds on @yusdacra's wonderful work - I just added back the shell part. I watched that presentation on dream2nix and I'm pretty excited 😄

@the-mikedavis
Copy link
Member Author

I'm not sure whether this should be a draft PR or not, since it's a branch of nix-cargo-integration and also I see many of these when running nix flake check (albeit now nix flake check succeeds, it was failing for me on master):

trace: 

The dream-lock.json for input 'helix-term' doesn't exist or is outdated.
...Falling back to on-the-fly evaluation (possibly slow).
To speed up future evalutations run once:
  nix run .#resolve

(and the helix flake doesn't appear to export a resolve app). Seems like it's not a big deal though, when running this flake I have syntax highlighting, the base16 themes, etc. - seems to be behaving well.

@the-mikedavis the-mikedavis changed the title use dream2nix-only branch of nix-cargo-integration flake: use dream2nix-only branch of nix-cargo-integration Mar 5, 2022
@yusdacra
Copy link
Contributor

yusdacra commented Mar 5, 2022

FYI, I merged the branch sometime ago, so you should be able to use master instead :)

And the trace warnings shouldn't cause anything to break. I *think" they were already removed from dream2nix, but I'm not sure.

@the-mikedavis the-mikedavis changed the title flake: use dream2nix-only branch of nix-cargo-integration flake: use dream2nix within nix-cargo-integration Mar 5, 2022
@jdahm
Copy link
Contributor

jdahm commented Mar 6, 2022

@the-mikedavis thanks for tagging me and jumping on this.

I have a local copy, so I did a gh pr checkout 1758. I've been waiting for direnv to finish rebuilding the shell ever since ;) Seems like something might be amiss. Separately I ran nix run github:the-mikedavis/helix/7b19f8b6d86f178a9fe558ffbe7e32312c5572b6 and same thing, it's hanging.

I'm confused why this adds helix as an input, but I've only switched to using flakes about a month ago so I'm still learning. What was the reason for doing that?

@jdahm
Copy link
Contributor

jdahm commented Mar 6, 2022

I suspect the tree-sitter gcc failure is unrelated but it's worth a shot.

I tracked that down to that nix derivations are using gcc as the underlying C compiler, even though on MacOS it needs to be using clang. I haven't had time to figure out how to remedy that. I was hoping using dream2nix would solve it.

@the-mikedavis
Copy link
Member Author

the-mikedavis commented Mar 6, 2022

I'm confused why this adds helix as an input

It's a whole controversy in Nix 🙃

IIRC it went something like: flakes added support for fetching submodules for the self input. That worked well for helix but broke some other packages, so it was reverted out of nix and hasn't been added back (yet). So we take helix as an input to be able to fetch the submodules (with the submodules = true; part). When flakes are fetching inputs it doesn't have any output though so it looks like it hangs. I think if you just have faith, eventually the download will finish 😄

#1659 will address this and make it much faster. Plus I have some ideas to beat the dead horse after that :)

nix derivations are using gcc as the underlying C compiler, even though on MacOS it needs to be using clang

Ah hmm, I wonder how tree-sitter handles this - we're currently using very similar code to what they do. We should be able to customize that in the flake part of #1659 pretty easily though

@jdahm
Copy link
Contributor

jdahm commented Mar 6, 2022

When flakes are fetching inputs it doesn't have any output though so it looks like it hangs. I think if you just have faith, eventually the download will finish 😄

Thanks. It finished and started building. I am getting the error:

error: builder for '/nix/store/is1ig25qjbbyzbsiaba5ild655fd28mj-helix-term-deps-0.6.0.drv' failed with exit code 1;
       last 7 log lines:
       > cargoArtifacts not set, will not reuse any cargo artifacts
       > unpacking sources
       > unpacking source archive /nix/store/c4zqry7wfsqy4cxn8pvn02pbc54jm585-dummy-src
       > source root is dummy-src
       > patching sources
       > Executing configureCargoCommonVars
       > mkdir: cannot create directory '/build': Operation not permitted

This could be because I'm using sandboxed nix builds (with useSandbox = true).

@the-mikedavis
Copy link
Member Author

bah all that waiting for another error 😅

I am on nixos where I think the sandboxing is the default, so I think it might be something darwin-specific? I might be able to borrow an M1 mac and do some testing. I suspect the builder for a crates deps is trying to access $out before it gets created but I'm not very familiar with this. I'll take a look 🧐

@yusdacra
Copy link
Contributor

yusdacra commented Mar 6, 2022

When flakes are fetching inputs it doesn't have any output though so it looks like it hangs. I think if you just have faith, eventually the download will finish smile

Thanks. It finished and started building. I am getting the error:

error: builder for '/nix/store/is1ig25qjbbyzbsiaba5ild655fd28mj-helix-term-deps-0.6.0.drv' failed with exit code 1;
       last 7 log lines:
       > cargoArtifacts not set, will not reuse any cargo artifacts
       > unpacking sources
       > unpacking source archive /nix/store/c4zqry7wfsqy4cxn8pvn02pbc54jm585-dummy-src
       > source root is dummy-src
       > patching sources
       > Executing configureCargoCommonVars
       > mkdir: cannot create directory '/build': Operation not permitted

This could be because I'm using sandboxed nix builds (with useSandbox = true).

Could you try using the temp-fixes branch of nix-cargo-integration? I attempted to fix it there. If that works, I'll make a PR to dream2nix.

@the-mikedavis
Copy link
Member Author

Ah that would make sense if darwin is using something other than /build for the build dir. Works on my machine! Thanks for your help here @yusdacra :)

@jdahm
Copy link
Contributor

jdahm commented Mar 6, 2022

@yusdacra

Could you try using the temp-fixes branch of nix-cargo-integration? I attempted to fix it there. If that works, I'll make a PR to dream2nix.

Yes, that avoids the error that /build cannot be created. Back to the gcc error on darwin. Thanks for jumping on it!

@the-mikedavis
Copy link
Member Author

Hmm it will probably be easier to debug this with #1659 than without. Does

nix run github:the-mikedavis/helix/5476c897804fb907dd34b76b5db5fdae4a2eda2d

build for you @jdahm? If not,

nix run github:the-mikedavis/helix/58fc60358df6899a63f52b4eb44d021aad547ed0

might? The second one is forcing clang.

@jdahm
Copy link
Contributor

jdahm commented Mar 7, 2022

It looks like I might have been slightly wrong about the current error. It seems like GCC is fine, it's actually a problem with range of an integer (?). First error here:

  running: "gcc" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-arch" "arm64" "-I" "src" "-I" "include" "-Wall" "-Wextra" "-std=c99" "-Wno-unused-parameter" "-o" "/private/tmp/nix-build-helix-term-deps-0.6.0.drv-0/dummy-src/target/release/build/tree-sitter-6dced0abe38c135b/out/src/lib.o" "-c" "src/lib.c"
  cargo:warning=/private/tmp/nix-build-helix-term-deps-0.6.0.drv-0/ccxlRFKK.s:1904:15: error: index must be an integer in range [-256, 255].
  cargo:warning=        ldr     x0, [x0, ___stack_chk_guard];momd

Entire log here:
bgyrzvdm5bs31hcricnn83ppsa09milk-helix-term-deps-0.6.0.log

@the-mikedavis I ran both those and neither got past that error. It looks like both those used gcc, but the log I attached is from the first. Thanks for putting them up to try!

@yusdacra
Copy link
Contributor

yusdacra commented Mar 7, 2022

Could you try using the temp-fixes branch of nix-cargo-integration? I attempted to fix it there. If that works, I'll make a PR to dream2nix.

Yes, that avoids the error that /build cannot be created. Back to the gcc error on darwin. Thanks for jumping on it!

That's good to hear. The PR that fixes the issue is merged to dream2nix now, and I have updated nix-cargo-integration master branch so it can be used again.

@the-mikedavis
Copy link
Member Author

With some looking around, I see NixOS/nixpkgs#127608 (comment) for the ___stack_chk_guard. Seems like it should be possible to disable it. I'll try out some changes to the flake

@the-mikedavis
Copy link
Member Author

Ok I've got some more commits to try out @jdahm! (Btw no rush on these, and thanks for all your testing :)

  • 3e529faab543d38fd32b2e901af3a8898913cabb disables stackprotector for the helix-term build
  • 9aa026c0cc9bb89ac60086ad618278e20f7c32e9 then also disables stackprotector for the grammars

From path in the error message though

nix-build-helix-term-deps-0.6.0.drv-0

I suspect that this might not fix it. We might need a way to add arguments to the deps builder (here in dream2nix I think), but if any of the nix run github:the-mikedavis/helix/<rev>s for the above revs work, it'd be great to not have to figure all that out

@yusdacra
Copy link
Contributor

yusdacra commented Mar 7, 2022

We might need a way to add arguments to the deps builder (here in dream2nix I think), but if any of the nix run github:the-mikedavis/helix/<rev>s for the above revs work, it'd be great to not have to figure all that out

This should be possible via just adding a helix-term-deps override in crateOverrides.

@the-mikedavis
Copy link
Member Author

Oh sweet! In that case I have another revision for testing: d4f326a2b7155dbf28c596807c238fbb2ad90439

@jdahm
Copy link
Contributor

jdahm commented Mar 7, 2022

Thanks for continuing to hack on this!

3e529fa

Same error:

error: builder for '/nix/store/kykgsgrrbpl4sf54a1a70z558hg03a9v-helix-term-deps-0.6.0.drv' failed with exit code 101;
       last 10 log lines:
       > warning:         ldr     x0, [x0, ___stack_chk_guard];momd
       > warning:                          ^
       > warning: /private/tmp/nix-build-helix-term-deps-0.6.0.drv-0/ccDHwWgb.s:39946:15: error: index must be an integer in range [-256, 255].

9aa026c

Also, similar error.

d4f326a

This one almost completes, but hits a link error at the very last step! Log here.

@the-mikedavis
Copy link
Member Author

Ah so close! This is starting to seem like an issue upstream in cc-rs. Sounds similar to

I'm starting to run out of ideas but we might be able to fiddle with some of the CC setup in helix-loader/src/grammars.rs. This should be easier to work with after #1659 is merged (I think it's pretty close to being merged). I'm in the queue to try out AWS's M1 EC2s, but if you're intested in hacking on this in the meantime, you should be able to clone and use nix develop to get cargo and everything set up. I'm not sure a regular cargo build would work on M1 given these errors, but if it does, it might indicate that the issue can be fixed in dream2nix/rust-overlay/cargo-nix-integration rather than need changes to any rust in helix.

https://github.com/nix-community/dream2nix is a fairly new and
cool-looking project for adapting upstream package manager outputs
(lockfiles mostly it would seem) for nix.

This should improve the ability to cross-compile. As a more concrete
measure of improvement, `nix flake check' now succeeds 🎉
@archseer archseer merged commit f31e85a into helix-editor:master Mar 8, 2022
@the-mikedavis the-mikedavis deleted the md-dream2nix-flake-refactor branch March 8, 2022 05:14
@jdahm
Copy link
Contributor

jdahm commented Mar 8, 2022

@the-mikedavis Thanks again. I checked and a clone of helix builds without error when using cargo not driven by the nix flake. So, as you say I'll do some digging into dream2nix/rust-overlay/cargo-nix-integration when I find time.

@colemickens
Copy link

Should I be concerned with this:

The dream-lock.json for input 'helix-term' doesn't exist or is outdated.
...Falling back to on-the-fly evaluation (possibly slow).

Also, thank you for all the work on this from all folks!

@yusdacra
Copy link
Contributor

yusdacra commented Mar 8, 2022

Should I be concerned with this:

The dream-lock.json for input 'helix-term' doesn't exist or is outdated.
...Falling back to on-the-fly evaluation (possibly slow).

Also, thank you for all the work on this from all folks!

No, that shouldn't matter in this case. The translator for Cargo.lock in dream2nix is fast enough in my experience. I think there should be a way to disable those warnings, but afaik right now they should not pop up after seeing them once (although im not sure how long that lasts).

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.

nix - update flake.nix to better support cross-system eval
5 participants