devbox add fallthrough and fallback for packages not handled by search #1264
Conversation
This file contains hidden or 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
mikeland73
reviewed
Jul 10, 2023
There was a problem hiding this comment.
This is a great and simple start, but I think there a few issues we may need to solve before we merge (or ship?)
- Adding fall-through package causes the warning
Warning: Your devbox.json contains packages in legacy format. Please run devbox update to update your devbox.json.to be shown on subsequent commands. - Running
devbox updatewill add@latestto the packages indevbox.jsonwhich will cause them to fail on the next run/shell. (Adding@latestwas our upgrade path for old configs)
Possible solution:
- Add "source" field to lock file packages. Source can be devbox-search, nixpkgs. etc. Only show legacy warning is source if unknown and package is in legacy format.
- When doing
devbox updatedon't update stuff with nixpkgs source. If lockfile/source is missing then check if package exists in search before appending the@latest.
LucilleH
force-pushed
the
lucille--nixpkg-fallback
branch
from
50b7687 to
6c60238
Compare
LucilleH
force-pushed
the
lucille--nixpkg-fallback
branch
from
6c60238 to
fca5809
Compare
mikeland73
approved these changes
Jul 11, 2023
| @@ -403,7 +403,7 @@ func (p *Package) Versioned() string { | |||
| } | |||
|
|
|||
| func (p *Package) IsLegacy() bool { | |||
| return p.isDevboxPackage() && !p.isVersioned() | |||
| return p.isDevboxPackage() && !p.isVersioned() && p.lockfile.Source(p.Raw) == "" | |||
There was a problem hiding this comment.
Another possible API could be
p.lockfile.Get(p.Raw).Source() to have lock packages themselves respond to source()
(and just default Source() to be empty string if not set or if package is nil)
There was a problem hiding this comment.
Oh I like p.lockfile.Get(p.Raw).Source() better. I'll follow up with a separate PR
LucilleH
added a commit
that referenced
this pull request
Jul 11, 2023
## Summary Update lockfile api for package source as suggested in #1264 (comment) ## How was it tested? devbox run build devbox update
mikeland73
added a commit
that referenced
this pull request
Sep 8, 2023
…1463) ## Summary Restores functionality implemented by #1264 and refactored in #1279 which was accidentally broken by #1437 tldr: the `FillNarInfoCache` function breaks if packages are not in search, but we support non-search packages via fallback (.e.g. stdenv.cc.cc.lib) I'm keeping surface area as small as possible, but as a follow up I think we can remove the `devpkg.FillNarInfoCache` calls from `impl.Add` if we simply ignore the`IsInBinaryCache` errors in [ValidateExists](https://github.com/jetpack-io/devbox/blob/main/internal/devpkg/validation.go#L10-L26) or remove `IsInBinaryCache` from that function completely (I'm not convinced it is needed, @savil @ipince correct me if I'm wrong) ## How was it tested? `devbox add stdenv.cc.cc.lib`
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
devbox addfallthrough and fallback for packages not handled by search.This PR does not do the following:
nixpkgcommit field indevbox.jsonimmediately?)I'd like to address those two things above if needed in a separate PR as a follow up.
Example lockfile:
How was it tested?
In the Tensorflow example: