[per-OS Packages] in devbox add, allow multiple --platform/--exclude-platform flag values (#1363)

## Summary

This PR enables calling `--platform` with multiple values with `devbox
add`.

It gives the user an option to user either or both (1) comma-separated
flag values (2) multiple flag invocations. Like:
`--platform <platform>,<platform>...,<platform> --platform <platform>
--platform <platform>`

Ditto for `--exclude-platform`

## How was it tested?

Augmented testscript unit test

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

@@ -2,14 +2,13 @@ package devconfig
 
 import (
 	"encoding/json"
-	"os"
-	"strings"
 
 	"github.com/pkg/errors"
+	"github.com/samber/lo"
 	orderedmap "github.com/wk8/go-ordered-map/v2"
+	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/searcher"
-	"go.jetpack.io/devbox/internal/ux"
 	"golang.org/x/exp/slices"
 )
 
@@ -59,29 +58,21 @@ 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 {
+// 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
+	}
+	if err := nix.EnsureValidPlatform(platforms...); 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
-				}
-			}
-
-			// Add the platform if it's not already present
-			if !alreadyPresent {
-				pkg.Platforms = append(pkg.Platforms, platform)
-			}
+			// Append if the platform is not already present
+			pkg.Platforms = lo.Uniq(append(pkg.Platforms, platforms...))
 
 			// Adding any platform will restrict installation to it, so
 			// the ExcludedPlatforms are no longer needed
@@ -96,35 +87,27 @@ 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
-				}
-			}
-
-			if !alreadyPresent {
-				pkg.ExcludedPlatforms = append(pkg.ExcludedPlatforms, platform)
-			}
+			pkg.ExcludedPlatforms = lo.Uniq(append(pkg.ExcludedPlatforms, platforms...))
 			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,
+				return usererr.New(
+					"cannot exclude any platform for package %s because it already has `platforms` defined. "+
+						"Please delete the `platforms` for this package from devbox.json and re-try.",
+					pkg.VersionedName(),
 				)
 			}
 

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 {

internal/nix/nix.go

@@ -195,15 +195,24 @@ var nixPlatforms = []string{
 	"armv7l-linux",
 }
 
-// EnsureValidPlatform returns an error if the platform is not supported by nix.
+// ensureValidPlatform returns an error if the platform is not supported by nix.
 // https://nixos.org/manual/nix/stable/installation/supported-platforms.html
-func EnsureValidPlatform(platform string) error {
-	for _, p := range nixPlatforms {
-		if p == platform {
-			return nil
+func EnsureValidPlatform(platforms ...string) error {
+	ensureValid := func(platform string) error {
+		for _, p := range nixPlatforms {
+			if p == platform {
+				return nil
+			}
 		}
+		return usererr.New("Unsupported platform: %s. Valid platforms are: %v", platform, nixPlatforms)
 	}
-	return usererr.New("Unsupported platform: %s. Valid platforms are: %v", platform, nixPlatforms)
+
+	for _, p := range platforms {
+		if err := ensureValid(p); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // Warning: be careful using the bins in default/bin, they won't always match bins

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"]
+    }
+  }
+}