Skip to content

Commit fbe3273

Browse files
authored
[plugins] Fix rm for packages with __remove_trigger_package (#1940)
## Summary Fixes issue where user could not remove package that is replaced by a flake in plugin (uses `__remove_trigger_package`) Fixes #1936 We need two different functions for packages: * `AllPackages` which is used for instalation and lock file. Includes packages added/removed by plugins. * `TopLevelPackages` is only what is in root devbox.json ## How was it tested? * Tested manually * Added php to rm unit test
1 parent 00e4954 commit fbe3273

File tree

5 files changed

+22
-15
lines changed

5 files changed

+22
-15
lines changed

internal/devbox/devbox.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func (d *Devbox) ConfigHash() (string, error) {
176176

177177
buf := bytes.Buffer{}
178178
buf.WriteString(h)
179-
for _, pkg := range d.ConfigPackages() {
179+
for _, pkg := range d.AllPackages() {
180180
buf.WriteString(pkg.Hash())
181181
}
182182
for _, pluginConfig := range d.cfg.IncludedPluginConfigs() {
@@ -1033,21 +1033,27 @@ func (d *Devbox) PackageNames() []string {
10331033
return d.cfg.PackagesVersionedNames()
10341034
}
10351035

1036-
// ConfigPackages returns the packages that are defined in devbox.json
1037-
// NOTE: the return type is different from devconfig.Packages
1038-
func (d *Devbox) ConfigPackages() []*devpkg.Package {
1036+
// AllPackages returns the packages that are defined in devbox.json and
1037+
// recursively added by plugins.
1038+
// NOTE: This will not return packages removed by their plugin with the
1039+
// __remove_trigger_package field.
1040+
func (d *Devbox) AllPackages() []*devpkg.Package {
10391041
return devpkg.PackagesFromConfig(d.cfg.Packages(), d.lockfile)
10401042
}
10411043

1044+
func (d *Devbox) TopLevelPackages() []*devpkg.Package {
1045+
return devpkg.PackagesFromConfig(d.cfg.Root.TopLevelPackages(), d.lockfile)
1046+
}
1047+
10421048
// InstallablePackages returns the packages that are to be installed
10431049
func (d *Devbox) InstallablePackages() []*devpkg.Package {
1044-
return lo.Filter(d.ConfigPackages(), func(pkg *devpkg.Package, _ int) bool {
1050+
return lo.Filter(d.AllPackages(), func(pkg *devpkg.Package, _ int) bool {
10451051
return pkg.IsInstallable()
10461052
})
10471053
}
10481054

10491055
func (d *Devbox) HasDeprecatedPackages() bool {
1050-
for _, pkg := range d.ConfigPackages() {
1056+
for _, pkg := range d.AllPackages() {
10511057
if pkg.IsLegacy() {
10521058
return true
10531059
}
@@ -1060,7 +1066,7 @@ func (d *Devbox) findPackageByName(name string) (*devpkg.Package, error) {
10601066
return nil, errors.New("package name cannot be empty")
10611067
}
10621068
results := map[*devpkg.Package]bool{}
1063-
for _, pkg := range d.ConfigPackages() {
1069+
for _, pkg := range d.TopLevelPackages() {
10641070
if pkg.Raw == name || pkg.CanonicalName() == name {
10651071
results[pkg] = true
10661072
}

internal/devbox/docgen/docgen.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ func GenerateReadme(
5656
"Scripts": devbox.Config().Scripts(),
5757
"EnvVars": devbox.Config().Env(),
5858
"InitHook": devbox.Config().InitHook(),
59-
"Packages": devbox.ConfigPackages(),
59+
"Packages": devbox.TopLevelPackages(),
60+
// TODO add includes
6061
})
6162
}
6263

internal/devbox/packages.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ func (d *Devbox) updateLockfile(recomputeState bool) error {
294294
d.lockfile.Tidy()
295295

296296
// Update lockfile with new packages that are not to be installed
297-
for _, pkg := range d.ConfigPackages() {
297+
for _, pkg := range d.AllPackages() {
298298
if err := pkg.EnsureUninstallableIsInLockfile(); err != nil {
299299
return err
300300
}

internal/devbox/update.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (d *Devbox) inputsToUpdate(
8181
opts devopt.UpdateOpts,
8282
) ([]*devpkg.Package, error) {
8383
if len(opts.Pkgs) == 0 {
84-
return d.ConfigPackages(), nil
84+
return d.AllPackages(), nil
8585
}
8686

8787
var pkgsToUpdate []*devpkg.Package

testscripts/rm/add-rm.test.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
exec devbox init
22

3-
exec devbox add hello vim cowsay
3+
exec devbox add hello vim cowsay php
44
json.superset devbox.json all.json
55

6-
exec devbox rm vim hello
6+
exec devbox rm vim hello php
77
json.superset devbox.json cowsay.json
88

9-
exec devbox add vim hello vim hello vim hello vim hello cowsay
9+
exec devbox add vim hello vim hello vim hello vim hello cowsay php php
1010
json.superset devbox.json all.json
1111

12-
exec devbox rm vim hello cowsay cowsay
12+
exec devbox rm vim hello cowsay cowsay php
1313
json.superset devbox.json empty.json
1414

1515
-- all.json --
1616
{
17-
"packages": ["hello@latest", "vim@latest", "cowsay@latest"]
17+
"packages": ["hello@latest", "vim@latest", "cowsay@latest", "php@latest"]
1818
}
1919

2020
-- cowsay.json --

0 commit comments

Comments
 (0)