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

Remove nix cruft #475

Merged
merged 6 commits into from
Sep 4, 2022
Merged

Remove nix cruft #475

merged 6 commits into from
Sep 4, 2022

Conversation

jurraca
Copy link
Contributor

@jurraca jurraca commented Sep 4, 2022

So, I had added a flake on the fly for development and building using Nix, and haven't kept up with the project as I should have, and it doesn't use an explicit Rust version, nor does it build correctly. After playing with this, a few reasons to change the current setup:

  • flakes are experimental, and probably don't have much of an audience
  • no need to make building more complicated than just using cargo, or supporting multiple build methods
  • a shell.nix is sufficient because it addresses all Nix users irrespective of version/setup: just nix-shell in the project directory and you're set up for development, which is the whole point.
  • in this case it uses rust 1.63 as required via mozilla's rust overlays, which is the most foolproof way to use Rust with Nix.
    Happy to nuke the Nix stuff if you think it's annoying, but the only maintenance here will be bumping rust versions as needed.

@casey
Copy link
Collaborator

casey commented Sep 4, 2022

Nice! I don't think I understand what a shell.nix is. Is it just supposed to provide all the dependencies needed to work on a project?

Some questions:

  • Is it possible to use stable rust, and not require any particular version of rust? This seems better than having to bump the version manually, and just should always build on stable rust. Also, is it possible to include stable rust in a way which is the same as a NixOS installation will get it, so that it doesn't require an additional download?
  • If it's not a good idea to use whatever's stable, can we refer to the rust by version, i.e. 1.63 instead of "stable" with a date?
  • Is gcc required? Or is llvm/clang good enough?
  • What's the convention for what to call these things? moz_overlay_shell is confusing, maybe just-shell, just-overlay, or just-overlay-shell?
  • Users might not want logging on, so I'd remove RUST_LOG

@jurraca
Copy link
Contributor Author

jurraca commented Sep 4, 2022

Is it just supposed to provide all the dependencies needed to work on a project?

yep, exactly

gcc

sorry, old habits die hard

What's the convention for what to call these things?

yea, just-shell is fine, or ord shell, it's really just metadata, you won't be calling the shell by name. However, I did add just to the shell since it's available in nixpkgs.

just should always build on stable rust

trust you here obvs, but I don't follow what just has to do with building, I thought it just ran commands? nvm i misread 🤦

@casey
Copy link
Collaborator

casey commented Sep 4, 2022

yea, just-shell is fine, or ord shell, it's really just metadata, you won't be calling the shell by name. However, I did add just to the shell since it's available in nixpkgs.

Whoops I'm silly, I was confused and thought this issue was in github.com/casey/just 😅

shell.nix Outdated Show resolved Hide resolved
@casey
Copy link
Collaborator

casey commented Sep 4, 2022

Sweet, looks good. Is this ready to merge?

@jurraca
Copy link
Contributor Author

jurraca commented Sep 4, 2022

yep! thanks for the feedback.

@casey casey merged commit 0ed3de8 into ordinals:master Sep 4, 2022
@casey
Copy link
Collaborator

casey commented Sep 4, 2022

Awesome, this is great! I really like how simple it makes getting started.

@jurraca jurraca deleted the remove-nix-cruft branch September 4, 2022 23:24
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.

None yet

2 participants