Skip to content
This repository has been archived by the owner on Jun 12, 2024. It is now read-only.

feat: rust template #26

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

liketechnik
Copy link

@liketechnik liketechnik commented May 6, 2024

As discussed on the forum (Brainstorming: What should Rust support look like?), this PR adds a rust template.

This PR is far from it's final form, with most stuff being either simply unpolished yet or up for discussion (many items on the lists taken from @getchoo's excellent suggestions):

Unpolished:

  • drop nix-filter as a dependency
  • better compose cross compilation toolchains
    • the current use of a function to both instantiate a toolchain and build a package is a bit messy
    • clearly separating supported cross targets, the instantiation of toolchains, and the overriding of a package using the standard toolchain from nixpkgs would allow for easier iteration
    • a good example of what i mean can be found in one of my repos here. it’s mainly used to create multi arch docker images from any arm or x64 machine on macos or linux
    • more callPackage in general would be good here

Discussion:

  • location
    • I opted for a rust sub-folder for now; although I think we landed on something like lang-tooling/rust in the discussion
    • we also considered SIG specific template repos, but ultimately decided against them, regarding discoverability and the still to be determined exact structure of SIG repos
    • considering there's already a c folder too, rust should be fine
  • use nixpkgs’ rust toolchain where possible and reasonable
    • for a standard build, there isn’t much reason to use fenix in the first place
    • this would make fenix another optional dependency, allowing for only depending on nixpkgs
    • the rust mirrors fenix and similar use are also very slow for users in countries such as china. hearing from people i know, being able to take advantage of local cache mirrors is much faster and more convenient
    • does it makes sense to add a second template showing how to use custom fenix / oxalica/rust-overlay / ... rust toolchains?
    • if we want a second template, I'd like to remove fenix last, so that I can just copy this template for the fenix one
  • RUST_SRC_PATH = "${pkgs.rustPlatform.rustLibSrc}" should be set in devShells with rust-analyer per Rust - NixOS Wiki 1
    • I believe this is unnecessary due to the inclusion of rust-src as part of the components that make up the dev shell's rust toolchain
  • examples of checks would be nice
    • from the same project i linked before, i have a rustfmt check here for example
    • I've added the rustfmt check
    • I've also tried adding clippy, but that failed with an obscure "Permission denied" error
  • the structure needs some reworking
    • this is mainly a nitpick about the requirement of some paths to use string concatenation. this is ugly and does have a (very minimal) performance impact in comparison to atomic expressions (i.e., standard paths)
    • agreed - should the files simply keep their current names and the string concatenations replaced by their paths?
  • the template is quite opinionated in some additional places too:
    • justfile for common tasks
    • formatting (tabs instead of rustfmt defaults; further customisation of rustfmt.toml)

I'll work on the 'unpolished' parts before requesting reviews, so that we can focus on the 'discussion' part.

@Sigmanificient Sigmanificient mentioned this pull request May 6, 2024
2 tasks
@liketechnik
Copy link
Author

So, the basic polishing stuff should be done for now.

I've amended the original to-do list with in-progress comments at the end of each to-do item; please make sure to re-read those to not miss anything.

@getchoo I would really appreciate an review from you here, you're nix code is a lot more idiomatic than mine :) I think you should also be able to push to this PR (at least I have checked GH's "allow edits from maintainers" option), so if you want, you can also just apply the restructuring without me having to understand every detail right (i.e. cut short the potential review back-and-forth between what you expect and what I make of it).

@liketechnik liketechnik requested a review from getchoo May 16, 2024 16:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant