[Config Packages] in devbox add, allow multiple --platform/--exclude-platform flag values

Author: savil

devbox.go

@@ -19,7 +19,7 @@ type Devbox interface {
 	// Add adds Nix packages to the config so that they're available in the devbox
 	// environment. It validates that the Nix packages exist, and install them.
 	// Adding duplicate packages is a no-op.
-	Add(ctx context.Context, platform, excludePlatform string, pkgs ...string) error
+	Add(ctx context.Context, platforms, excludePlatforms []string, pkgs ...string) error
 	Config() *devconfig.Config
 	ProjectDir() string
 	// Generate creates the directory of Nix files and the Dockerfile that define

internal/boxcli/add.go

@@ -18,10 +18,10 @@ import (
 const toSearchForPackages = "To search for packages, use the `devbox search` command"
 
 type addCmdFlags struct {
-	config          configFlags
-	allowInsecure   bool
-	platform        string
-	excludePlatform string
+	config           configFlags
+	allowInsecure    bool
+	platforms        []string
+	excludePlatforms []string
 }
 
 func addCmd() *cobra.Command {
@@ -53,11 +53,11 @@ func addCmd() *cobra.Command {
 	command.Flags().BoolVar(
 		&flags.allowInsecure, "allow-insecure", false,
 		"allow adding packages marked as insecure.")
-	command.Flags().StringVar(
-		&flags.platform, "platform", "",
+	command.Flags().StringSliceVarP(
+		&flags.platforms, "platform", "p", []string{},
 		"add packages to run on only this platform.")
-	command.Flags().StringVar(
-		&flags.excludePlatform, "exclude-platform", "",
+	command.Flags().StringSliceVarP(
+		&flags.excludePlatforms, "exclude-platform", "e", []string{},
 		"exclude packages from a specific platform.")
 
 	return command
@@ -73,5 +73,5 @@ func addCmdFunc(cmd *cobra.Command, args []string, flags addCmdFlags) error {
 		return errors.WithStack(err)
 	}
 
-	return box.Add(cmd.Context(), flags.platform, flags.excludePlatform, args...)
+	return box.Add(cmd.Context(), flags.platforms, flags.excludePlatforms, args...)
 }

internal/devconfig/packages.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+	"github.com/samber/lo"
 	orderedmap "github.com/wk8/go-ordered-map/v2"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/searcher"
@@ -59,30 +60,28 @@ func (pkgs *Packages) Remove(versionedName string) {
 	})
 }
 
-// AddPlatform adds a platform to the list of platforms for a given package
-func (pkgs *Packages) AddPlatform(versionedname, platform string) error {
-	if err := nix.EnsureValidPlatform(platform); err != nil {
-		return errors.WithStack(err)
+// AddPlatforms adds a platform to the list of platforms for a given package
+func (pkgs *Packages) AddPlatforms(versionedname string, platforms []string) error {
+	if len(platforms) == 0 {
+		return nil
+	}
+	for _, platform := range platforms {
+		if err := nix.EnsureValidPlatform(platform); err != nil {
+			return errors.WithStack(err)
+		}
 	}
 
 	name, version := parseVersionedName(versionedname)
 	for idx, pkg := range pkgs.Collection {
 		if pkg.name == name && pkg.Version == version {
 
-			// Check if the platform is already present
-			alreadyPresent := false
-			for _, existing := range pkg.Platforms {
-				if existing == platform {
-					alreadyPresent = true
-					break
+			for _, platform := range platforms {
+				// Append if the platform is not already present
+				if !lo.SomeBy(pkg.Platforms, func(p string) bool { return p == platform }) {
+					pkg.Platforms = append(pkg.Platforms, platform)
 				}
 			}
 
-			// Add the platform if it's not already present
-			if !alreadyPresent {
-				pkg.Platforms = append(pkg.Platforms, platform)
-			}
-
 			// Adding any platform will restrict installation to it, so
 			// the ExcludedPlatforms are no longer needed
 			pkg.ExcludedPlatforms = nil
@@ -96,35 +95,34 @@ func (pkgs *Packages) AddPlatform(versionedname, platform string) error {
 	return errors.Errorf("package %s not found", versionedname)
 }
 
-// ExcludePlatform adds a platform to the list of excluded platforms for a given package
-func (pkgs *Packages) ExcludePlatform(versionedName, platform string) error {
-	if err := nix.EnsureValidPlatform(platform); err != nil {
-		return errors.WithStack(err)
+// ExcludePlatforms adds a platform to the list of excluded platforms for a given package
+func (pkgs *Packages) ExcludePlatforms(versionedName string, platforms []string) error {
+	if len(platforms) == 0 {
+		return nil
+	}
+	for _, platform := range platforms {
+		if err := nix.EnsureValidPlatform(platform); err != nil {
+			return errors.WithStack(err)
+		}
 	}
 
 	name, version := parseVersionedName(versionedName)
 	for idx, pkg := range pkgs.Collection {
 		if pkg.name == name && pkg.Version == version {
 
-			// Check if the platform is already present
-			alreadyPresent := false
-			for _, existing := range pkg.ExcludedPlatforms {
-				if existing == platform {
-					alreadyPresent = true
-					break
+			for _, platform := range platforms {
+				// Append if the platform is not already present
+				if !lo.SomeBy(pkg.ExcludedPlatforms, func(p string) bool { return p == platform }) {
+					pkg.ExcludedPlatforms = append(pkg.ExcludedPlatforms, platform)
 				}
 			}
-
-			if !alreadyPresent {
-				pkg.ExcludedPlatforms = append(pkg.ExcludedPlatforms, platform)
-			}
 			if len(pkg.Platforms) > 0 {
 				ux.Finfo(
 					os.Stderr,
 					"Excluding a platform for %[1]s is a bit redundant because it will only be installed on: %[2]v. "+
 						"Consider removing the `platform` field from %[1]s's definition in your devbox."+
 						"json if you intend for %[1]s to be installed on all platforms except %[3]s.\n",
-					versionedName, strings.Join(pkg.Platforms, ", "), platform,
+					versionedName, strings.Join(pkg.Platforms, ", "), strings.Join(platforms, ", "),
 				)
 			}
 

internal/impl/packages.go

@@ -34,7 +34,7 @@ import (
 // Add adds the `pkgs` to the config (i.e. devbox.json) and nix profile for this
 // devbox project
 // nolint:revive // warns about cognitive complexity
-func (d *Devbox) Add(ctx context.Context, platform, excludePlatform string, pkgsNames ...string) error {
+func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, pkgsNames ...string) error {
 	ctx, task := trace.NewTask(ctx, "devboxAdd")
 	defer task.End()
 
@@ -88,15 +88,11 @@ func (d *Devbox) Add(ctx context.Context, platform, excludePlatform string, pkgs
 	}
 
 	for _, pkg := range addedPackageNames {
-		if platform != "" {
-			if err := d.cfg.Packages.AddPlatform(pkg, platform); err != nil {
-				return err
-			}
+		if err := d.cfg.Packages.AddPlatforms(pkg, platforms); err != nil {
+			return err
 		}
-		if excludePlatform != "" {
-			if err := d.cfg.Packages.ExcludePlatform(pkg, excludePlatform); err != nil {
-				return err
-			}
+		if err := d.cfg.Packages.ExcludePlatforms(pkg, excludePlatforms); err != nil {
+			return err
 		}
 	}
 

internal/impl/update.go

@@ -34,7 +34,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
 			// Calling Add function with the original package names, since
 			// Add will automatically append @latest if search is able to handle that.
 			// If not, it will fallback to the nixpkg format.
-			if err := d.Add(ctx, "" /*platform*/, "" /*excludePlatform*/, pkg.Raw); err != nil {
+			if err := d.Add(ctx, nil /*platforms*/, nil /*excludePlatforms*/, pkg.Raw); err != nil {
 				return err
 			}
 		} else {

testscripts/add/add_platforms.test.txt

@@ -1,6 +1,7 @@
 # Testscript for exercising adding packages
 
-# exec devbox init
+#### Part 1: Adding with a single platform or exclude-platform
+
 exec devbox install
 ! exec rg --version
 ! exec vim --version
@@ -17,6 +18,14 @@ exec devbox add vim --exclude-platform x86_64-linux
 
 json.superset devbox.json expected_devbox2.json
 
+#### Part 2: Adding with multiple platforms or exclude-platforms
+
+exec devbox add hello --platform x86_64-darwin,x86_64-linux --platform aarch64-darwin
+json.superset devbox.json expected_devbox3.json
+
+exec devbox add cowsay --exclude-platform x86_64-darwin,x86_64-linux --exclude-platform aarch64-darwin
+json.superset devbox.json expected_devbox4.json
+
 -- devbox.json --
 {
   "packages": [
@@ -52,3 +61,48 @@ json.superset devbox.json expected_devbox2.json
     }
   }
 }
+
+-- expected_devbox3.json --
+
+{
+  "packages": {
+    "hello": "",
+    "cowsay": "latest",
+    "ripgrep": {
+      "version": "latest",
+      "platforms": ["x86_64-darwin", "x86_64-linux"]
+    },
+    "vim": {
+      "version": "latest",
+      "excluded_platforms": ["x86_64-linux"]
+    },
+    "hello": {
+        "version": "latest",
+        "platforms": ["x86_64-darwin", "x86_64-linux", "aarch64-darwin"]
+    }
+  }
+}
+
+-- expected_devbox4.json --
+
+{
+  "packages": {
+    "hello": "",
+    "cowsay": {
+      "version": "latest",
+      "excluded_platforms": ["x86_64-darwin", "x86_64-linux", "aarch64-darwin"]
+    },
+    "ripgrep": {
+      "version": "latest",
+      "platforms": ["x86_64-darwin", "x86_64-linux"]
+    },
+    "vim": {
+      "version": "latest",
+      "excluded_platforms": ["x86_64-linux"]
+    },
+    "hello": {
+        "version": "latest",
+        "platforms": ["x86_64-darwin", "x86_64-linux", "aarch64-darwin"]
+    }
+  }
+}