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

feat: add devenv.flakeArgs to avoid --no-pure-eval #1418

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Atry
Copy link
Contributor

@Atry Atry commented Sep 5, 2024

This PR avoids --no-pure-eval in devenv-devShell.nix, in favor of "--override-input" "devenv-root" "path:.devenv/state/pwd".

Atry added a commit to Atry/nix-ml-ops that referenced this pull request Sep 5, 2024
@Atry
Copy link
Contributor Author

Atry commented Sep 10, 2024

@domenkozar What do you think?

@Nebucatnetzer
Copy link
Contributor

Nebucatnetzer commented Sep 11, 2024

I like the idea of making devenv more pure but I would avoid putting necessary steps into the .envrc file.
This way direnv becomes a dependency, currently it's just a nice tool to have.
In CI I would then either need to have direnv present or make the state directory manually.
The the moment it its enough to just run nix develop --no-pure-eval --command some-test-command

BTW. isn't devenv-root at .devenv and devenv-state would be .devenv/state?

@Atry
Copy link
Contributor Author

Atry commented Sep 11, 2024

I like the idea of making devenv more pure but I would avoid putting necessary steps into the .envrc file. This way direnv becomes a dependency, currently it's just a nice tool to have. In CI I would then either need to have direnv present or make the state directory manually. The the moment it its enough to just run nix develop --no-pure-eval --command some-test-command

This PR did not affect nix develop --no-pure-eval --command some-test-command behavior in general. It just makes devenv up in flake base projects pure. It does not affect devenv up in devenv.yaml based projects because they have a different implementation of devenv.

Since it changed devenv up, I updated the PR in 56f801a to keep supporting nix develop --no-pure-eval --command devenv up for flake projects that do not include an .envrc, so the direnv is not a dependency any more.

BTW. isn't devenv-root at .devenv and devenv-state would be .devenv/state?

Yes

@Atry
Copy link
Contributor Author

Atry commented Sep 11, 2024

@domenkozar Potentially devenv.yaml based projects could also utilize the .devenv/state/pwd file to avoid --no-pure-eval, but it is out of the scope of this PR.

@Nebucatnetzer
Copy link
Contributor

This PR did not affect nix develop --no-pure-eval --command some-test-command behavior in general.

We have some PHP tests that write session files to .devenv/state/php-fpm so they wouldn't have worked anymore.
Thank you for updating it :).

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.

2 participants