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.

Sign up for GitHub

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

Jump to bottom

fix: let nixConfig be used at build step #206

Closed
wants to merge 1 commit into from
Closed

fix: let nixConfig be used at build step #206

wants to merge 1 commit into from

Conversation

soupglasses
Copy link

Allows nix build to become flake aware. This is important to allow nixConfig based changes to be used at build time. For example when setting extra-substituters through the flake.nix file.

Partially fixes: #204, this PR does not fix the remote builder.

@soupglasses
fix: let nixConfig be used at build step
e9ca51d 
Allows `nix build` to become flake aware. This is important to allow
nixConfig based changes to be used at build time. For example when
setting `extra-substituters` through the flake.nix file.
balsoft
balsoft requested changes Apr 19, 2023
View reviewed changes
Copy link
Member

@balsoft balsoft left a comment •
edited

Choose a reason for hiding this comment

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

This evaluates the derivation twice. This will result in quite a massive slow-down for many deployments.

We should either fix Nix evaluation cache to handle this properly, or not evaluate the derivation path when first evaluating the deploy output.

@soupglasses
Copy link
Author

That is some good spotting! I had not thought about that.

Handling nixConfig has ended up being much more of a pain than first thought. The only other idea that i have is running something similar to nix eval --json --file flake.nix nixConfig at the data step, using it to populate a nix-config list that could be applied with --option. This should not evaluate anything under the flake inputs and flake outputs for this second pass.

Originally wanted to avoid this as re-implementing how nix build works natively i feel could have small unexpected changes that could be hard to reason and debug, as well as there not being an easy way to handle the per permission toggle that nixConfig follows.

@soupglasses
Copy link
Author

Dropping this PR due to my Rust knowledge not being up for the job. If anyone else wants to pick this up, feel free to.

@soupglasses soupglasses closed this May 2, 2023
@soupglasses soupglasses mentioned this pull request May 2, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@balsoft balsoft balsoft requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

deploy-rs builds derivations without considering flake attribute nixConfig
2 participants
@soupglasses @balsoft