[global] Unify nix profile install #695
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gcurtis
approved these changes
Feb 27, 2023
mikeland73
force-pushed
the
landau/set-priority-unify
branch
from
203260f
to
eb9162a
Compare
savil
reviewed
Feb 27, 2023
internal/impl/packages.go
Outdated
| return "5" | ||
| } | ||
| } | ||
| return "4" |
There was a problem hiding this comment.
This should be the opposite: the raw packages from devbox.json get highest priority compared to packages from elsewhere like global packages.
the lower the number, the higher the priority....
internal/impl/global.go
Outdated
| stepMsg := fmt.Sprintf("[%d/%d] %s", stepNum, total, pkg) | ||
| if err := nix.ProfileInstall(&nix.ProfileInstallArgs{ | ||
| CustomStepMessage: stepMsg, | ||
| NixpkgsCommit: plansdk.DefaultNixpkgsCommit, |
There was a problem hiding this comment.
shouldn't this take the nixpkgs commit hash from the global devbox.json?
| if err := ensureNixpkgsPrefetched(args.Writer, args.NixpkgsCommit); err != nil { | ||
| return err | ||
| } | ||
| stepMsg := args.Package |
There was a problem hiding this comment.
stepMsg := fmt.Sprintf("Installing %s\n", args.Package)
This was referenced Feb 27, 2023
mikeland73
added a commit
that referenced
this pull request
Feb 28, 2023
## Summary Fixes minor double error issue. (This was pulled out of #695) ## How was it tested? ```bash devbox global add hello hello is now installed Warning: the devbox global profile is not in your $PATH. Add the following line to your shell's rcfile (e.g., ~/.bashrc or ~/.zshrc) and restart your shell to fix this: eval "$(devbox global shellenv)" ```
mikeland73
added a commit
that referenced
this pull request
Feb 28, 2023
## Summary I'm running into some weird CICD resource issue on #695 so I'm splitting that up into smaller PRs. ## How was it tested? ```bash devbox global add go_1_20 devbox add go_1_19 devbox shell ```
mikeland73
changed the title
mikeland73
force-pushed
the
landau/set-priority-unify
branch
from
d53fd41
to
c946b74
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This fixes a few global issues:
It does so by refactoring some code around:
nix profile install, packageInstallIgnore, checking if nixpkgs is installed)What this does not fix: If we install a very large package (e.g.
bazel) we block oncmd.Run()and don't print any output. This affects all ways of installing.How was it tested?
Also tried changing hashes on global devbox.json, installing conflicting binaries, adding and removing large packages (e.g. pulumi, bazel, etc). Bazel was a bad experience.