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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cargo missing in dev env #51

Merged
merged 3 commits into from
Apr 18, 2024
Merged

Fix cargo missing in dev env #51

merged 3 commits into from
Apr 18, 2024

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Apr 18, 2024

No idea how I didn't notice it missing before..

Must've come from somewhere else 馃し

Turns out lazyDerivation (added in 3f361a4) ended up filtering out .nativeBuildInputs and co., which mkShell's inputsFrom relies on to figure out what to use for the environment.. 馃う

@infinisil infinisil requested a review from a team as a code owner April 18, 2024 04:47
@willbush
Copy link
Member

willbush commented Apr 18, 2024

Ah strange. I don't have it installed globally either. In my forked copy of this repo, in older branches, I have cargo so it's coming from somewhere else in the derivation somehow.

@philiptaron
Copy link
Contributor

Isn't it coming from inputsFrom = [ results.build ];?

lazyDerivation ended up filtering out .nativeBuildInputs and co.,
which mkShell's inputsFrom relies on to figure out what to use for the environment..
@infinisil infinisil changed the title Add cargo to dev env Fix cargo missing in dev env Apr 18, 2024
@infinisil
Copy link
Member Author

Figured it out, lazyDerivation was the problem, see the commit message 馃う

Copy link
Contributor

@philiptaron philiptaron 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 think lazyDerivation is doing too much for us. The promise of being less messy now seems mostly untrue. This is more code and less easy to understand than the simple exposure of the original derivation.

@infinisil
Copy link
Member Author

Yeah agreed 馃槄

Maybe let's just not bother trying to make nix-build work, we can still use nix-build -A build

I'll update it shortly

The lazyDerivation part to try to make it less messy ends up just making the code messy instead,
let's just drop it and not worry about making `nix-build` without arguments work.

Instead `nix-build -A build` works, and this also keeps the resulting attribute set cleaner in
general.
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Yeah, let's do this.

@infinisil infinisil merged commit ff3553e into main Apr 18, 2024
2 checks passed
@infinisil infinisil deleted the cargo branch April 18, 2024 17: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

3 participants