Merge pull request #12 from gogrlx/cd/cql-review

fix: CQL review — correctness, safety, and standards

apk/apk_linux.go

@@ -96,18 +96,22 @@ func update(ctx context.Context) error {
 
 func list(ctx context.Context) ([]snack.Package, error) {
 	cmd := exec.CommandContext(ctx, "apk", "list", "--installed")
-	out, err := cmd.CombinedOutput()
+	var stderr strings.Builder
+	cmd.Stderr = &stderr
+	out, err := cmd.Output()
 	if err != nil {
-		return nil, fmt.Errorf("apk list: %w", err)
+		return nil, fmt.Errorf("apk list: %s: %w", strings.TrimSpace(stderr.String()), err)
 	}
 	return parseListInstalled(string(out)), nil
 }
 
 func search(ctx context.Context, query string) ([]snack.Package, error) {
 	cmd := exec.CommandContext(ctx, "apk", "search", "-v", query)
-	out, err := cmd.CombinedOutput()
+	var stderr strings.Builder
+	cmd.Stderr = &stderr
+	out, err := cmd.Output()
 	if err != nil {
-		return nil, fmt.Errorf("apk search: %w", err)
+		return nil, fmt.Errorf("apk search: %s: %w", strings.TrimSpace(stderr.String()), err)
 	}
 	results := parseSearch(string(out))
 	if len(results) == 0 {

apt/capabilities_linux.go

@@ -37,37 +37,43 @@ func latestVersion(ctx context.Context, pkg string) (string, error) {
 }
 
 func listUpgrades(ctx context.Context) ([]snack.Package, error) {
-	cmd := exec.CommandContext(ctx, "apt", "list", "--upgradable")
-	cmd.Env = append(os.Environ(), "LANG=C")
+	// Use apt-get --just-print upgrade instead of `apt list --upgradable`
+	// because `apt` has unstable CLI output not intended for scripting.
+	cmd := exec.CommandContext(ctx, "apt-get", "--just-print", "upgrade")
+	cmd.Env = append(os.Environ(), "LANG=C", "DEBIAN_FRONTEND=noninteractive")
 	out, err := cmd.Output()
 	if err != nil {
-		return nil, fmt.Errorf("apt list --upgradable: %w", err)
+		return nil, fmt.Errorf("apt-get --just-print upgrade: %w", err)
 	}
 	var pkgs []snack.Package
 	for _, line := range strings.Split(string(out), "\n") {
 		line = strings.TrimSpace(line)
-		if line == "" || strings.HasPrefix(line, "Listing...") {
+		// Lines starting with "Inst " indicate upgradable packages.
+		// Format: "Inst pkg [old-ver] (new-ver repo [arch])"
+		if !strings.HasPrefix(line, "Inst ") {
 			continue
 		}
-		// Format: "pkg/source version arch [upgradable from: old-version]"
-		slashIdx := strings.Index(line, "/")
-		if slashIdx < 0 {
-			continue
-		}
-		name := line[:slashIdx]
-		rest := line[slashIdx+1:]
-		fields := strings.Fields(rest)
+		line = strings.TrimPrefix(line, "Inst ")
+		fields := strings.Fields(line)
 		if len(fields) < 2 {
 			continue
 		}
+		name := fields[0]
+		// Find the new version in parentheses
+		parenStart := strings.Index(line, "(")
+		parenEnd := strings.Index(line, ")")
+		if parenStart < 0 || parenEnd < 0 {
+			continue
+		}
+		verFields := strings.Fields(line[parenStart+1 : parenEnd])
+		if len(verFields) < 1 {
+			continue
+		}
 		p := snack.Package{
 			Name:      name,
-			Version:   fields[1],
+			Version:   verFields[0],
 			Installed: true,
 		}
-		if len(fields) > 2 {
-			p.Arch = fields[2]
-		}
 		pkgs = append(pkgs, p)
 	}
 	return pkgs, nil
@@ -361,9 +367,7 @@ func listKeys(ctx context.Context) ([]string, error) {
 
 	// List keyring files
 	matches, _ := filepath.Glob("/etc/apt/keyrings/*.gpg")
-	for _, m := range matches {
-		keys = append(keys, m)
-	}
+	keys = append(keys, matches...)
 	ascMatches, _ := filepath.Glob("/etc/apt/keyrings/*.asc")
 	keys = append(keys, ascMatches...)
 

capabilities.go

@@ -4,14 +4,14 @@ package snack
 // Useful for grlx to determine what operations are available before
 // attempting them.
 type Capabilities struct {
-	VersionQuery    bool
-	Hold            bool
-	Clean           bool
-	FileOwnership   bool
-	RepoManagement  bool
-	KeyManagement   bool
-	Groups          bool
-	NameNormalize   bool
+	VersionQuery   bool
+	Hold           bool
+	Clean          bool
+	FileOwnership  bool
+	RepoManagement bool
+	KeyManagement  bool
+	Groups         bool
+	NameNormalize  bool
 }
 
 // GetCapabilities probes a Manager for all optional interface support.

dpkg/dpkg_test.go

@@ -1,6 +1,7 @@
 package dpkg
 
 import (
+	"context"
 	"testing"
 
 	"github.com/gogrlx/snack"
@@ -73,14 +74,14 @@ func TestNew(t *testing.T) {
 
 func TestUpgradeUnsupported(t *testing.T) {
 	d := New()
-	if err := d.Upgrade(nil); err != snack.ErrUnsupportedPlatform {
+	if err := d.Upgrade(context.TODO()); err != snack.ErrUnsupportedPlatform {
 		t.Errorf("expected ErrUnsupportedPlatform, got %v", err)
 	}
 }
 
 func TestUpdateUnsupported(t *testing.T) {
 	d := New()
-	if err := d.Update(nil); err != snack.ErrUnsupportedPlatform {
+	if err := d.Update(context.TODO()); err != snack.ErrUnsupportedPlatform {
 		t.Errorf("expected ErrUnsupportedPlatform, got %v", err)
 	}
 }

errors.go

@@ -28,4 +28,8 @@ var (
 	// ErrManagerNotFound is returned by detect when no supported package
 	// manager can be found on the system.
 	ErrManagerNotFound = errors.New("no supported package manager found")
+
+	// ErrDaemonNotRunning is returned when a package manager's required
+	// daemon (e.g. snapd) is not running.
+	ErrDaemonNotRunning = errors.New("package manager daemon is not running")
 )

snap/snap_linux.go

@@ -13,8 +13,12 @@ import (
 )
 
 func available() bool {
-	_, err := exec.LookPath("snap")
-	return err == nil
+	if _, err := exec.LookPath("snap"); err != nil {
+		return false
+	}
+	// Verify snapd is running by checking snap version (requires daemon).
+	cmd := exec.Command("snap", "version")
+	return cmd.Run() == nil
 }
 
 func run(ctx context.Context, args []string) (string, error) {

types.go

@@ -69,7 +69,7 @@ func WithReinstall() Option {
 
 // Repository represents a configured package repository.
 type Repository struct {
-	ID       string `json:"id"`                 // unique identifier
+	ID       string `json:"id"`                  // unique identifier
 	Name     string `json:"name,omitempty"`      // human-readable name
 	URL      string `json:"url"`                 // repository URL
 	Enabled  bool   `json:"enabled"`             // whether the repo is active