[plugins] Ensure plugin packages are in nix profile

examples/development/php/php8.1/devbox.lock

@@ -18,9 +18,9 @@
     },
     "[email protected]": {
       "last_modified": "2023-05-01T16:53:22Z",
-      "plugin_version": "0.0.1",
+      "plugin_version": "0.0.2",
       "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#php",
       "version": "8.1.18"
     }
   }
-}
+}

internal/impl/devbox.go

@@ -138,7 +138,7 @@ func (d *Devbox) Config() *devconfig.Config {
 }
 
 func (d *Devbox) ConfigHash() (string, error) {
-	pkgHashes := lo.Map(d.PackagesAsInputs(), func(i *devpkg.Package, _ int) string { return i.Hash() })
+	pkgHashes := lo.Map(d.Packages(), func(i *devpkg.Package, _ int) string { return i.Hash() })
 	includeHashes := lo.Map(d.Includes(), func(i plugin.Includable, _ int) string { return i.Hash() })
 	h, err := d.cfg.Hash()
 	if err != nil {
@@ -402,7 +402,7 @@ func (d *Devbox) GenerateDevcontainer(ctx context.Context, force bool) error {
 			redact.Safe(filepath.Base(devContainerPath)), err)
 	}
 	// generate devcontainer.json
-	err = generate.CreateDevcontainer(ctx, devContainerPath, d.Packages())
+	err = generate.CreateDevcontainer(ctx, devContainerPath, d.PackageNames())
 	if err != nil {
 		return redact.Errorf("error generating devcontainer.json in <project>/%s: %w",
 			redact.Safe(filepath.Base(devContainerPath)), err)
@@ -484,7 +484,7 @@ func (d *Devbox) saveCfg() error {
 
 func (d *Devbox) Services() (services.Services, error) {
 	pluginSvcs, err := d.pluginManager.GetServices(
-		d.PackagesAsInputs(),
+		d.Packages(),
 		d.cfg.Include,
 	)
 	if err != nil {
@@ -824,7 +824,7 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	// We still need to be able to add env variables to non-service binaries
 	// (e.g. ruby). This would involve understanding what binaries are associated
 	// to a given plugin.
-	pluginEnv, err := d.pluginManager.Env(d.PackagesAsInputs(), d.cfg.Include, env)
+	pluginEnv, err := d.pluginManager.Env(d.Packages(), d.cfg.Include, env)
 	if err != nil {
 		return nil, err
 	}
@@ -927,12 +927,26 @@ func (d *Devbox) nixFlakesFilePath() string {
 }
 
 // Packages returns the list of Packages to be installed in the nix shell.
-func (d *Devbox) Packages() []string {
+func (d *Devbox) PackageNames() []string {
 	return d.cfg.Packages.VersionedNames()
 }
 
-func (d *Devbox) PackagesAsInputs() []*devpkg.Package {
-	return devpkg.PackageFromStrings(d.Packages(), d.lockfile)
+func (d *Devbox) Packages() []*devpkg.Package {
+	return devpkg.PackageFromStrings(d.PackageNames(), d.lockfile)
+}
+
+// AllPackages returns user packages and plugin packages concatenated in
+// correct order
+func (d *Devbox) AllPackages() ([]*devpkg.Package, error) {
+	userPackages := d.Packages()
+	pluginPackages, err := d.PluginManager().PluginPackages(userPackages)
+	if err != nil {
+		return nil, err
+	}
+	// We prioritize plugin packages so that the php plugin works. Not sure
+	// if this is behavior we want for user plugins. We may need to add an optional
+	// priority field to the config.
+	return append(pluginPackages, userPackages...), nil
 }
 
 func (d *Devbox) Includes() []plugin.Includable {
@@ -946,7 +960,7 @@ func (d *Devbox) Includes() []plugin.Includable {
 }
 
 func (d *Devbox) HasDeprecatedPackages() bool {
-	for _, pkg := range d.PackagesAsInputs() {
+	for _, pkg := range d.Packages() {
 		if pkg.IsLegacy() {
 			return true
 		}

internal/impl/flakes.go

@@ -14,7 +14,7 @@ func (d *Devbox) getLocalFlakesDirs() []string {
 	localFlakeDirs := []string{}
 
 	// searching through installed packages to get location of local flakes
-	for _, pkg := range d.Packages() {
+	for _, pkg := range d.PackageNames() {
 		// filtering local flakes packages
 		if strings.HasPrefix(pkg, "path:") {
 			pkgDirAndName, _ := strings.CutPrefix(pkg, "path:")

internal/impl/packages.go

@@ -196,6 +196,13 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 		fmt.Fprintln(d.writer, "Ensuring packages are installed.")
 	}
 
+	// Create plugin directories first because packages might need them
+	for _, pkg := range d.Packages() {
+		if err := d.PluginManager().Create(pkg); err != nil {
+			return err
+		}
+	}
+
 	if err := d.syncPackagesToProfile(ctx, mode); err != nil {
 		return err
 	}
@@ -255,15 +262,6 @@ func (d *Devbox) addPackagesToProfile(ctx context.Context, mode installMode) err
 		return nil
 	}
 
-	// If packages are in profile but nixpkgs has been purged, the experience
-	// will be poor when we try to run print-dev-env. So we ensure nixpkgs is
-	// prefetched for all relevant packages (those not in binary cache).
-	for _, input := range d.PackagesAsInputs() {
-		if err := input.EnsureNixpkgsPrefetched(d.writer); err != nil {
-			return err
-		}
-	}
-
 	pkgs, err := d.pendingPackagesForInstallation(ctx)
 	if err != nil {
 		return err
@@ -273,11 +271,21 @@ func (d *Devbox) addPackagesToProfile(ctx context.Context, mode installMode) err
 		return nil
 	}
 
+	// If packages are in profile but nixpkgs has been purged, the experience
+	// will be poor when we try to run print-dev-env. So we ensure nixpkgs is
+	// prefetched for all relevant packages (those not in binary cache).
+	for _, input := range pkgs {
+		if err := input.EnsureNixpkgsPrefetched(d.writer); err != nil {
+			return err
+		}
+	}
+
 	var msg string
 	if len(pkgs) == 1 {
 		msg = fmt.Sprintf("Installing package: %s.", pkgs[0])
 	} else {
-		msg = fmt.Sprintf("Installing %d packages: %s.", len(pkgs), strings.Join(pkgs, ", "))
+		pkgNames := lo.Map(pkgs, func(p *devpkg.Package, _ int) string { return p.Raw })
+		msg = fmt.Sprintf("Installing %d packages: %s.", len(pkgs), strings.Join(pkgNames, ", "))
 	}
 	fmt.Fprintf(d.writer, "\n%s\n\n", msg)
 
@@ -295,7 +303,7 @@ func (d *Devbox) addPackagesToProfile(ctx context.Context, mode installMode) err
 		if err := nixprofile.ProfileInstall(&nixprofile.ProfileInstallArgs{
 			CustomStepMessage: stepMsg,
 			Lockfile:          d.lockfile,
-			Package:           pkg,
+			Package:           pkg.Raw,
 			ProfilePath:       profileDir,
 			Writer:            d.writer,
 		}); err != nil {
@@ -368,32 +376,36 @@ func (d *Devbox) tidyProfile(ctx context.Context) error {
 // devbox.json or global devbox.json but are not yet installed in the nix
 // profile. It maintains the order of packages as specified by
 // Devbox.packages() (higher priority first)
-func (d *Devbox) pendingPackagesForInstallation(ctx context.Context) ([]string, error) {
+func (d *Devbox) pendingPackagesForInstallation(ctx context.Context) ([]*devpkg.Package, error) {
 	defer trace.StartRegion(ctx, "pendingPackages").End()
 
 	profileDir, err := d.profilePath()
 	if err != nil {
 		return nil, err
 	}
 
-	pending := []string{}
+	pending := []*devpkg.Package{}
 	list, err := nixprofile.ProfileListItems(d.writer, profileDir)
 	if err != nil {
 		return nil, err
 	}
-	for _, input := range d.PackagesAsInputs() {
+	packages, err := d.AllPackages()
+	if err != nil {
+		return nil, err
+	}
+	for _, pkg := range packages {
 		_, err := nixprofile.ProfileListIndex(&nixprofile.ProfileListIndexArgs{
 			List:       list,
 			Lockfile:   d.lockfile,
 			Writer:     d.writer,
-			Input:      input,
+			Input:      pkg,
 			ProfileDir: profileDir,
 		})
 		if err != nil {
 			if !errors.Is(err, nix.ErrPackageNotFound) {
 				return nil, err
 			}
-			pending = append(pending, input.Raw)
+			pending = append(pending, pkg)
 		}
 	}
 	return pending, nil
@@ -416,7 +428,7 @@ func (d *Devbox) extraPackagesInProfile(ctx context.Context) ([]*nixprofile.NixP
 	if err != nil {
 		return nil, err
 	}
-	devboxInputs := d.PackagesAsInputs()
+	devboxInputs := d.Packages()
 
 	if len(devboxInputs) == len(profileItems) {
 		// Optimization: skip comparison if number of packages are the same. This only works

internal/impl/update.go

@@ -75,7 +75,7 @@ func (d *Devbox) inputsToUpdate(pkgs ...string) ([]*devpkg.Package, error) {
 		pkgsToUpdate = append(pkgsToUpdate, found)
 	}
 	if len(pkgsToUpdate) == 0 {
-		pkgsToUpdate = d.Packages()
+		pkgsToUpdate = d.PackageNames()
 	}
 
 	return devpkg.PackageFromStrings(pkgsToUpdate, d.lockfile), nil

internal/lock/interfaces.go

@@ -6,7 +6,7 @@ package lock
 type devboxProject interface {
 	ConfigHash() (string, error)
 	NixPkgsCommitHash() string
-	Packages() []string
+	PackageNames() []string
 	ProjectDir() string
 }
 

internal/lock/lockfile.go

@@ -141,7 +141,7 @@ func IsLegacyPackage(pkg string) bool {
 // Tidy ensures that the lockfile has the set of packages corresponding to the devbox.json config.
 // It gets rid of older packages that are no longer needed.
 func (f *File) Tidy() {
-	f.Packages = lo.PickByKeys(f.Packages, f.devboxProject.Packages())
+	f.Packages = lo.PickByKeys(f.Packages, f.devboxProject.PackageNames())
 }
 
 // IsUpToDateAndInstalled returns true if the lockfile is up to date and the

internal/nix/profiles.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/pkg/errors"
 	"go.jetpack.io/devbox/internal/boxcli/usererr"
+	"go.jetpack.io/devbox/internal/debug"
 	"go.jetpack.io/devbox/internal/redact"
 )
 
@@ -59,6 +60,7 @@ func ProfileInstall(writer io.Writer, profilePath string, installable string) er
 	cmd.Stdout = writer
 	cmd.Stderr = writer
 
+	debug.Log("running command: %s\n", cmd)
 	return cmd.Run()
 }
 

internal/plugin/manager.go

@@ -15,7 +15,7 @@ type Manager struct {
 }
 
 type devboxProject interface {
-	Packages() []string
+	PackageNames() []string
 	ProjectDir() string
 }
 
@@ -45,7 +45,7 @@ func (m *Manager) ApplyOptions(opts ...managerOption) {
 	}
 }
 
-func (m *Manager) PluginInputs(inputs []*devpkg.Package) ([]*devpkg.Package, error) {
+func (m *Manager) PluginPackages(inputs []*devpkg.Package) ([]*devpkg.Package, error) {
 	result := []*devpkg.Package{}
 	for _, input := range inputs {
 		config, err := getConfigIfAny(input, m.ProjectDir())

internal/plugin/plugin.go

@@ -163,7 +163,7 @@ func (m *Manager) createFile(
 		"DevboxDirRoot":        filepath.Join(m.ProjectDir(), devboxDirName),
 		"DevboxProfileDefault": filepath.Join(m.ProjectDir(), nix.ProfilePath),
 		"PackageAttributePath": attributePath,
-		"Packages":             m.Packages(),
+		"Packages":             m.PackageNames(),
 		"System":               system,
 		"URLForInput":          urlForInput,
 		"Virtenv":              filepath.Join(virtenvPath, name),

internal/shellgen/flake_plan.go

@@ -22,13 +22,6 @@ func newFlakePlan(ctx context.Context, devbox devboxer) (*flakePlan, error) {
 	ctx, task := trace.NewTask(ctx, "devboxFlakePlan")
 	defer task.End()
 
-	// Create plugin directories first because inputs might depend on them
-	for _, pkg := range devbox.PackagesAsInputs() {
-		if err := devbox.PluginManager().Create(pkg); err != nil {
-			return nil, err
-		}
-	}
-
 	for _, included := range devbox.Config().Include {
 		// This is a slightly weird place to put this, but since includes can't be
 		// added via command and we need them to be added before we call
@@ -41,18 +34,11 @@ func newFlakePlan(ctx context.Context, devbox devboxer) (*flakePlan, error) {
 		}
 	}
 
-	userPackages := devbox.PackagesAsInputs()
-	pluginPackages, err := devbox.PluginManager().PluginInputs(userPackages)
+	packages, err := devbox.AllPackages()
 	if err != nil {
 		return nil, err
 	}
-	// We prioritize plugin packages so that the php plugin works. Not sure
-	// if this is behavior we want for user plugins. We may need to add an optional
-	// priority field to the config.
-	packages := append(pluginPackages, userPackages...)
-
 	flakeInputs := flakeInputs(ctx, packages)
-
 	nixpkgsInfo := getNixpkgsInfo(devbox.Config().NixPkgsCommitHash())
 
 	// This is an optimization. Try to reuse the nixpkgs info from the flake

internal/shellgen/scripts.go

@@ -23,7 +23,8 @@ const HooksFilename = ".hooks"
 type devboxer interface {
 	Config() *devconfig.Config
 	Lockfile() *lock.File
-	PackagesAsInputs() []*devpkg.Package
+	Packages() []*devpkg.Package
+	AllPackages() ([]*devpkg.Package, error)
 	PluginManager() *plugin.Manager
 	ProjectDir() string
 }
@@ -44,7 +45,7 @@ func WriteScriptsToFiles(devbox devboxer) error {
 
 	// Write all hooks to a file.
 	written := map[string]struct{}{} // set semantics; value is irrelevant
-	pluginHooks, err := plugin.InitHooks(devbox.PackagesAsInputs(), devbox.ProjectDir())
+	pluginHooks, err := plugin.InitHooks(devbox.Packages(), devbox.ProjectDir())
 	if err != nil {
 		return errors.WithStack(err)
 	}