Skip to content

Derive nix profile from flake: attempt 2 #1724

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

Merged
merged 2 commits into from
Jan 17, 2024
Merged

Conversation

savil
Copy link
Collaborator

@savil savil commented Jan 12, 2024

Summary

This PR is a re-write of #1692. See motivation and discussion there.

Motivation
This PR seeks to unify our nix print-dev-env and nix profile steps. Other benefits:

Previously, these would be two separate processes: we'd manually do nix profile install.
Now, we get the buildInputPaths from nix print-dev-env and install those, so the nix profile is derived from the flake.

Implementation notes

  1. In ensureStateIsUpToDate, I had to modify the logic in Derive nix profile from flake: attempt 2 #1724: call d.installPackages that creates plugin files, installs runx and nix packages. Also, move all lockfile operations to the end for all modes, instead of just for recomputeState == true.
  2. The nix build operation now happens with the [1/3] [email protected]: Success output that previously would happen with nix profile install.
  3. Instead of nixprofiles.NixProfileInstall, we first install via nix build to ensure the package is installed in the local nix store. We then call devbox.syncNixProfile to get the buildInputs from devbox.computeEnv and nix profile install $buildInputs.

How was it tested?

  • CICD tests pass
  • Did basic sanity checking with devbox add hello cowsay and devbox rm hello cowsay.
  • Used the Devbox binary for development for a while.

To verify that we can install packages in /nix/store during devbox add, and then use that during offline in devbox shell:

  1. Do devbox add [email protected]
  2. Do devbox shell and exit, or just devbox install.
  3. Turn Wifi off, and then open devbox shell and verify that the newly installed version of python is used.
    BEFORE: we could skip step 2, because we directly installed packages in the nix profile
    AFTER: step 2 is required because we re-evaluate the flake's environment and derive nix profile from it.

Copy link
Collaborator Author

savil commented Jan 12, 2024

@savil savil force-pushed the savil/refactor-ensureStateUpToDate branch from 3e1be89 to 5cbd4a9 Compare January 12, 2024 04:42
@savil savil force-pushed the savil/profile-from-flake-2 branch 3 times, most recently from 7e4e95d to b9a4588 Compare January 12, 2024 16:54
@savil savil force-pushed the savil/refactor-ensureStateUpToDate branch from 5cbd4a9 to 6788172 Compare January 12, 2024 23:58
@savil savil force-pushed the savil/profile-from-flake-2 branch from b9a4588 to 062cc7f Compare January 12, 2024 23:58
@savil savil force-pushed the savil/profile-from-flake-2 branch 3 times, most recently from 9fe6bea to b0dd286 Compare January 13, 2024 00:59
@savil
Copy link
Collaborator Author

savil commented Jan 13, 2024

@mikeland73 @gcurtis I think this is 95% there. The insecure example was failing which I think I've now addressed 🤞🏾

PTAL and let me know what you think.

@savil savil requested review from gcurtis and mikeland73 January 13, 2024 01:00
@savil savil marked this pull request as ready for review January 13, 2024 01:01
@savil savil force-pushed the savil/profile-from-flake-2 branch from 336ce68 to 1f0aead Compare January 16, 2024 18:24
Comment on lines +87 to +89
func (i *NixProfileListItem) StorePaths() []string {
return i.nixStorePaths
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Export instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since all the fields in this struct are private, I wanted to maintain that constraint. Makes it easier to reason about.

recomputeState := mode == ensure || d.IsEnvEnabled()
if recomputeState {
if err := d.recomputeState(ctx, mode); err != nil {
if mode == install || mode == update || mode == ensure {
Copy link
Contributor

Choose a reason for hiding this comment

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

!= remove?

@savil savil force-pushed the savil/profile-from-flake-2 branch from 1f0aead to 62f5cb2 Compare January 16, 2024 22:25
Base automatically changed from savil/refactor-ensureStateUpToDate to main January 16, 2024 23:05
@savil savil force-pushed the savil/profile-from-flake-2 branch from 62f5cb2 to 7fc9961 Compare January 16, 2024 23:08
@savil savil merged commit 03a1c2b into main Jan 17, 2024
@savil savil deleted the savil/profile-from-flake-2 branch January 17, 2024 01:14
mikeland73 added a commit that referenced this pull request Jan 18, 2024
mikeland73 added a commit that referenced this pull request Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants