-
Notifications
You must be signed in to change notification settings - Fork 230
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: implement outdated command #2497
Conversation
This allow to present all new versions available for the pkg installed. It is a lot useful because otherwise you need to check this manually.
thanks for sending a PR! |
For sure it is something we can move under another menu, no problem! |
I think this might be best to move under |
internal/devbox/packages.go
Outdated
result, err := searcher.Client().Search(ctx, pkg.CanonicalName()) | ||
if err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gcurtis does this reliably give us the latest version of the package?
For example, when I run it with go
I get:
devbox ls --outdated
The following packages can be updated:
* go 1.23.4 -> 1.24rc2
which doesn't seem to be what we want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is correct, is the latest version available in the store :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But its a release-candidate. In devbox update
we wouldn't actually update you to that version if you had go@latest
} | ||
} | ||
|
||
return outdatedPackages, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guerinoni I think we need this function to be rewritten as so. I created this patch via git diff --patch
, so you could try applying it: https://gist.github.com/savil/7a55e959c5eb5c5e240f57dedd5fd7ad
copying here for clarity:
type UpdateVersion struct {
Current string
Latest string
}
// Outdated returns a map of package names to their available latest version.
func (d *Devbox) Outdated(ctx context.Context) (map[string]UpdateVersion, error) {
ctx, task := trace.NewTask(ctx, "devboxOutdated")
defer task.End()
lockfile := d.Lockfile()
outdatedPackages := map[string]UpdateVersion{}
for _, pkg := range d.AllPackages() {
// For non-devbox packages, like flakes or runx, we can skip for now
if !pkg.IsDevboxPackage {
continue
}
lockPackage, err := lockfile.FetchResolvedPackage(pkg.Versioned())
if err != nil {
return nil, errors.Wrap(err, "failed to fetch resolved package")
}
existingLockPackage := lockfile.Packages[pkg.Raw]
if lockPackage.Version == existingLockPackage.Version {
continue
}
outdatedPackages[pkg.Versioned()] = UpdateVersion{Current: existingLockPackage.Version, Latest: lockPackage.Version}
}
return outdatedPackages, nil
}
wdyt @gcurtis @mikeland73 ?
The benefit of this is that it also works for @latest
. For example, when I run this on the devbox repo itself, I see:
$ devbox run build
$ dist/devbox ls --outdated
The following packages can be updated:
* go@latest 1.23.0 -> 1.23.4
* runx:golangci/golangci-lint@latest v1.60.2 -> v1.63.4
I don't think the runx
one should be getting printed, but its cause we return true for IsDevboxPackage
for runx packages!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I aligned with this patch :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not runx? The versions are correct. Ideally we could also do it for flakes as well but our current flake update mechanism is a bit broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh good point, I forgot runx updating works properly. So all good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nits, but this LGTM
defer task.End() | ||
|
||
lockfile := d.Lockfile() | ||
outdatedPackages := map[string]UpdateVersion{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, would prefer to use a slice over map. We don't need random access or deduplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gcurtis may not like this but we could just return []lo.Tuple2[*lockfile.Package, *lockfile.Package]
that way we don't have to create a new struct at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm I don't mind the struct. Its nice to have the names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The map
is just a convenience for the name. Otherwise you'd have to add a Name
field to the struct. That works too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How you prefer guys, here map doesn't create any performance problems, it is just a matter of some seconds to run the command, I chose readability over little optimization
} | ||
} | ||
|
||
return outdatedPackages, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not runx? The versions are correct. Ideally we could also do it for flakes as well but our current flake update mechanism is a bit broken.
Summary
This allow to present all new versions available for the pkg installed. It is a lot useful because otherwise you need to check this manually.
How was it tested?
In a different repository, this is the output.