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 nightly rust-analyzer and add rust-src component #735

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

sodiboo
Copy link
Contributor

@sodiboo sodiboo commented Oct 14, 2024

this also improves the application of overlays to be more uniform, in particular this like applying the overlay was just weird and basically wrong (that's not how overlays work)

there's also some subtlety with using selectLatestNightlyWith. from rust-overlay's README:

Note: Don't use rust-bin.nightly.latest. Your build would fail when some components missing on some days. Always use selectLatestNightlyWith instead.

and uhh... i think packages.${system}.rust-nightly maps to rust-bin.nightly.latest? so we were using the one that the README says not to use? the code is slightly hard to read. Either way, this subtlety wasn't causing any issues previously, but is fixed now to prevent it from causing any issues in the future as well.

the main benefit is that we're now using rust-analyzer also from rust-overlay, as well as including the rust-src component which is required for rust-analyzer to actually be useful. this is a required change because rust-analyzer is basically useless in the devshell since #687 because uhh Nobody Thought To Test It. the other stuff in this PR is mostly just refactoring.

cc @getchoo you touched this last and idk if you want to review it or anything, but feel free to leave comments. or don't, i don't know how much you want to be involved in the future

Copy link
Contributor

@getchoo getchoo left a comment

Choose a reason for hiding this comment

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

everything else LGTM

@@ -22,6 +22,130 @@
rust-overlay,
}:
let
niri-package =
{
lib,
Copy link
Contributor Author

@sodiboo sodiboo Oct 15, 2024

Choose a reason for hiding this comment

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

it's easy to miss in the diff but when moving this package declaration up here, i've also separately added the lib parameter so that comes from pkgs.callPackage instead of our outermost inherit (nixpkgs) lib;. this is the only change to the package declaration; the actual contents are identical to what they previously were

Copy link
Contributor

@getchoo getchoo left a comment

Choose a reason for hiding this comment

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

i don't know how much you want to be involved in the future

And feel free to ping me here any time :)

Copy link
Contributor

@getchoo getchoo left a comment

Choose a reason for hiding this comment

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

Almost missed this

this also improves the application of overlays to be more uniform; what
was previously done was just Wrong
@YaLTeR YaLTeR merged commit e4161be into YaLTeR:main Oct 15, 2024
10 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Oct 15, 2024

Thanks

@sodiboo
Copy link
Contributor Author

sodiboo commented Oct 15, 2024

feel free to ping me here any time :)

I think I just might. It'll be nice to have more than one pair of eyes actively overlooking the code.

FluxTape pushed a commit to FluxTape/niri that referenced this pull request Oct 15, 2024
this also improves the application of overlays to be more uniform; what
was previously done was just Wrong
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.

3 participants