Skip to content

Commit

Permalink
[Config Packages] in devbox add, allow multiple --platform/--exclude-…
Browse files Browse the repository at this point in the history
…platform flag values
  • Loading branch information
savil committed Aug 11, 2023
1 parent 0dbee1d commit 8790b47
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 51 deletions.
2 changes: 1 addition & 1 deletion devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 9 additions & 9 deletions internal/boxcli/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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...)
}
58 changes: 28 additions & 30 deletions internal/devconfig/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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, ", "),
)
}

Expand Down
14 changes: 5 additions & 9 deletions internal/impl/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/impl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
56 changes: 55 additions & 1 deletion testscripts/add/add_platforms.test.txt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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": [
Expand Down Expand Up @@ -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"]
}
}
}

0 comments on commit 8790b47

Please sign in to comment.