Update Process Compose to 0.85.0 #1820
Conversation
| @@ -14,7 +14,7 @@ import ( | |||
| "github.com/f1bonacc1/process-compose/src/types" | |||
| ) | |||
|
|
|||
| type processStates = types.ProcessStates | |||
| type processStates = types.ProcessesState | |||
There was a problem hiding this comment.
is this still the same type?
There was a problem hiding this comment.
Should be. Everything still seems to compile and work as expected.
| @@ -32,14 +35,66 @@ func (d *Devbox) addDevboxUtilityPackage(ctx context.Context, pkgName string) er | |||
| return err | |||
| } | |||
|
|
|||
| return nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ | |||
There was a problem hiding this comment.
If nixprofile.ProfileInstall and nixprofile.ProfileInstallArgs are no longer used you should remove.
There was a problem hiding this comment.
I was a little confused about having both nixprofile and nix.profile. Is nix.profile the one we should keep using? Can we completely remove nixprofile?
There was a problem hiding this comment.
nixprofile.ProfileInstall wraps nix.ProfileInstall printing a "Step message" and showing nicer error messages. I think for your use case, using nix.ProfileInstall is fine.
In 06fd5ff @savil switched from using nixprofile.ProfileInstall to nix.ProfileInstall because he handles the messaging outside of the function call. In that case we can get rid of nixprofile.ProfileInstall. @savil please confirm
internal/devbox/util.go
Outdated
| profileString, err := nix.ProfileList(d.stderr, utilityProfilePath, true) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err = json.Unmarshal([]byte(profileString), &utilProfile); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
You should use nixprofile.ProfileListItems which handles legacy nix outputs. It may also simplify finding the index.
Edit: actually, you can use ProfileListNameOrIndex to find the index.
There was a problem hiding this comment.
I'm not really sure why we have nixprofile and nix.profile in the source code, but both of those methods you mentioned seem to require that we have a lockfile, which does not exist in the case of the utility profile (since we're manipulating the profile directly, instead of using Devbox).
I think we'd need to either refactor those methods to not require a profile, or refactor the utility profile to use Devbox.
There was a problem hiding this comment.
Went with using ProfileListItems to get the list, and then doing a quick match based on unlockedReference == installable. This should work for utility packages that we install from a Flake reference and not from a store path, which is currently the case for all utility packages.
| @@ -32,14 +35,66 @@ func (d *Devbox) addDevboxUtilityPackage(ctx context.Context, pkgName string) er | |||
| return err | |||
| } | |||
|
|
|||
| return nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ | |||
There was a problem hiding this comment.
nixprofile.ProfileInstall wraps nix.ProfileInstall printing a "Step message" and showing nicer error messages. I think for your use case, using nix.ProfileInstall is fine.
In 06fd5ff @savil switched from using nixprofile.ProfileInstall to nix.ProfileInstall because he handles the messaging outside of the function call. In that case we can get rid of nixprofile.ProfileInstall. @savil please confirm
internal/nix/profiles.go
Outdated
| @@ -142,6 +142,16 @@ func readManifest(profilePath string) (manifest, error) { | |||
|
|
|||
| const DefaultPriority = 5 | |||
|
|
|||
| type NixProfile struct { | |||
internal/nix/nixprofile/profile.go
Outdated
| if args.Lockfile != nil { | ||
| for _, item := range items { | ||
| if item.Matches(args.Package, args.Lockfile) { | ||
| return item.NameOrIndex(), nil | ||
| } |
There was a problem hiding this comment.
revert? This function won't really work for flakes without the lockfile.
I think we can fix this dependency, but out of scope for this PR.
internal/devbox/util.go
Outdated
| func findUtilPackage(installable string, profile []*nixprofile.NixProfileListItem) int { | ||
| for i := range profile { | ||
| if profile[i].MatchesInstallable(installable) { | ||
| return i | ||
| } | ||
| } | ||
| return -1 | ||
| } |
There was a problem hiding this comment.
I think it's probably best to keep this function in removeDevboxUtilityPackage.
As written, this is not really limited to util packages.
So
for i, profileItem := range profile {
if profileItem.MatchesInstallable(installable) {
return nix.ProfileRemove(utilityProfilePath, fmt.Sprint(i))
}
}
return nilThere was a problem hiding this comment.
Thanks, that's much better
Summary
Devbox will now install process-compose 0.85.0 if an older version (or no version) is installed on the user's system
If an older version of process compose was installed in the utility profile, Devbox will remove that older version first. This PR provides a general way to remove packages (either from Flakes or Nixpkgs) from the utils profile
This PR also fixes a bug where using
devbox services upwith the--configflag would cause process compose to fail.This PR doesn't add any new 0.85.0 features, though that could be a follow up.
How was it tested?