From a6075d84e802b9a72ed6da64ed396d40e65c506d Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Tue, 3 Oct 2023 16:08:36 -0700 Subject: [PATCH 1/2] [add] Print messages about actions taken, or not taken. --- internal/devconfig/packages.go | 13 +++++++++++-- internal/impl/packages.go | 25 +++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/internal/devconfig/packages.go b/internal/devconfig/packages.go index 8f969594f78..366856e259b 100644 --- a/internal/devconfig/packages.go +++ b/internal/devconfig/packages.go @@ -2,7 +2,10 @@ package devconfig import ( "encoding/json" + "fmt" + "io" "slices" + "strings" "github.com/pkg/errors" "github.com/samber/lo" @@ -70,7 +73,7 @@ func (pkgs *Packages) Remove(versionedName string) { } // AddPlatforms adds a platform to the list of platforms for a given package -func (pkgs *Packages) AddPlatforms(versionedname string, platforms []string) error { +func (pkgs *Packages) AddPlatforms(writer io.Writer, versionedname string, platforms []string) error { if len(platforms) == 0 { return nil } @@ -94,6 +97,10 @@ func (pkgs *Packages) AddPlatforms(versionedname string, platforms []string) err pkg.VersionedName(), ) } + fmt.Fprintf(writer, + "Added platform %s to package %s\n", strings.Join(platforms, ", "), + pkg.VersionedName(), + ) pkgs.jsonKind = jsonMap pkg.kind = regular @@ -105,7 +112,7 @@ func (pkgs *Packages) AddPlatforms(versionedname string, platforms []string) err } // ExcludePlatforms adds a platform to the list of excluded platforms for a given package -func (pkgs *Packages) ExcludePlatforms(versionedName string, platforms []string) error { +func (pkgs *Packages) ExcludePlatforms(writer io.Writer, versionedName string, platforms []string) error { if len(platforms) == 0 { return nil } @@ -127,6 +134,8 @@ func (pkgs *Packages) ExcludePlatforms(versionedName string, platforms []string) pkg.VersionedName(), ) } + fmt.Fprintf(writer, "Excluded platform %s for package %s\n", strings.Join(platforms, ", "), + pkg.VersionedName()) pkgs.jsonKind = jsonMap pkg.kind = regular diff --git a/internal/impl/packages.go b/internal/impl/packages.go index b340f76cd1b..d33cc89a98e 100644 --- a/internal/impl/packages.go +++ b/internal/impl/packages.go @@ -38,6 +38,9 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, ctx, task := trace.NewTask(ctx, "devboxAdd") defer task.End() + // Track which packages had no changes so we can report that to the user. + unchangedPackageNames := []string{} + // Only add packages that are not already in config. If same canonical exists, // replace it. pkgs := devpkg.PackageFromStrings(lo.Uniq(pkgsNames), d.lockfile) @@ -53,6 +56,8 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, if slices.Contains(existingPackageNames, pkg.Versioned()) { // But we still need to add to addedPackageNames. See its comment. addedPackageNames = append(addedPackageNames, pkg.Versioned()) + unchangedPackageNames = append(unchangedPackageNames, pkg.Versioned()) + fmt.Fprintf(d.stderr, "Package %q already in devbox.json\n", pkg.Versioned()) continue } @@ -88,15 +93,16 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, return usererr.New("Package %s not found", pkg.Raw) } + fmt.Fprintf(d.stderr, "Adding package %q to devbox.json\n", packageNameForConfig) d.cfg.Packages.Add(packageNameForConfig) addedPackageNames = append(addedPackageNames, packageNameForConfig) } for _, pkg := range addedPackageNames { - if err := d.cfg.Packages.AddPlatforms(pkg, platforms); err != nil { + if err := d.cfg.Packages.AddPlatforms(d.stderr, pkg, platforms); err != nil { return err } - if err := d.cfg.Packages.ExcludePlatforms(pkg, excludePlatforms); err != nil { + if err := d.cfg.Packages.ExcludePlatforms(d.stderr, pkg, excludePlatforms); err != nil { return err } } @@ -110,6 +116,11 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, if err != nil { return err } + // TODO: Now that config packages can have fields, + // we should set this in the config, not the lockfile. + if !p.AllowInsecure { + fmt.Fprintf(d.stderr, "Allowing insecure for %s\n", name) + } p.AllowInsecure = true } } @@ -134,6 +145,16 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, } } + if len(platforms) == 0 && len(excludePlatforms) == 0 && !d.allowInsecureAdds { + if len(unchangedPackageNames) == 1 { + fmt.Fprintf(d.stderr, "Package %q was already in devbox.json and was not modified\n", unchangedPackageNames[0]) + } else if len(unchangedPackageNames) > 1 { + fmt.Fprintf(d.stderr, "Packages %s were already in devbox.json and were not modified\n", + strings.Join(unchangedPackageNames, ", "), + ) + } + } + return nil } From 56520ca11dc3a0b4a6085267a2392b6e3e60e9c1 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Tue, 3 Oct 2023 17:40:07 -0700 Subject: [PATCH 2/2] use ux.Info --- internal/devconfig/packages.go | 6 +++--- internal/impl/packages.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/devconfig/packages.go b/internal/devconfig/packages.go index 366856e259b..008a43884b3 100644 --- a/internal/devconfig/packages.go +++ b/internal/devconfig/packages.go @@ -2,7 +2,6 @@ package devconfig import ( "encoding/json" - "fmt" "io" "slices" "strings" @@ -13,6 +12,7 @@ import ( "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" ) type jsonKind int @@ -97,7 +97,7 @@ func (pkgs *Packages) AddPlatforms(writer io.Writer, versionedname string, platf pkg.VersionedName(), ) } - fmt.Fprintf(writer, + ux.Finfo(writer, "Added platform %s to package %s\n", strings.Join(platforms, ", "), pkg.VersionedName(), ) @@ -134,7 +134,7 @@ func (pkgs *Packages) ExcludePlatforms(writer io.Writer, versionedName string, p pkg.VersionedName(), ) } - fmt.Fprintf(writer, "Excluded platform %s for package %s\n", strings.Join(platforms, ", "), + ux.Finfo(writer, "Excluded platform %s for package %s\n", strings.Join(platforms, ", "), pkg.VersionedName()) pkgs.jsonKind = jsonMap diff --git a/internal/impl/packages.go b/internal/impl/packages.go index d33cc89a98e..0869cc01a39 100644 --- a/internal/impl/packages.go +++ b/internal/impl/packages.go @@ -57,7 +57,7 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, // But we still need to add to addedPackageNames. See its comment. addedPackageNames = append(addedPackageNames, pkg.Versioned()) unchangedPackageNames = append(unchangedPackageNames, pkg.Versioned()) - fmt.Fprintf(d.stderr, "Package %q already in devbox.json\n", pkg.Versioned()) + ux.Finfo(d.stderr, "Package %q already in devbox.json\n", pkg.Versioned()) continue } @@ -93,7 +93,7 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, return usererr.New("Package %s not found", pkg.Raw) } - fmt.Fprintf(d.stderr, "Adding package %q to devbox.json\n", packageNameForConfig) + ux.Finfo(d.stderr, "Adding package %q to devbox.json\n", packageNameForConfig) d.cfg.Packages.Add(packageNameForConfig) addedPackageNames = append(addedPackageNames, packageNameForConfig) } @@ -147,9 +147,9 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string, if len(platforms) == 0 && len(excludePlatforms) == 0 && !d.allowInsecureAdds { if len(unchangedPackageNames) == 1 { - fmt.Fprintf(d.stderr, "Package %q was already in devbox.json and was not modified\n", unchangedPackageNames[0]) + ux.Finfo(d.stderr, "Package %q was already in devbox.json and was not modified\n", unchangedPackageNames[0]) } else if len(unchangedPackageNames) > 1 { - fmt.Fprintf(d.stderr, "Packages %s were already in devbox.json and were not modified\n", + ux.Finfo(d.stderr, "Packages %s were already in devbox.json and were not modified\n", strings.Join(unchangedPackageNames, ", "), ) }