Implement CommandRunner pattern for unified testable package manager operations
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 25/100
Research direction
No file paths or tests are named. Read the existing CommandRunner implementation and the completed YUM and APT migrations first, then use sub-issues #28 and #29 as the active scope for Snap and Flatpak. Done means those migrations, their tests, and the remaining acceptance criteria are complete.
Written by the indexing model from the issue text.
Description
Problem Statement
Currently, package managers use inconsistent command execution patterns, creating architectural and testing issues:
Current State Analysis
- YUM: ✅ Uses CommandRunner (recently migrated, fully tested)
- APT:
Uses CommandBuilder✅ MIGRATED (Issue #27 - PR #26) - Snap: Uses direct
exec.Commandcalls (Issue #28) - Flatpak: Uses direct
exec.Commandcalls (Issue #29)
Issues with Mixed Approaches
- Architectural inconsistency - Different patterns across package managers
- Complex testing - CommandBuilder requires shell script mocking vs simple map-based mocking
- Manual LC_ALL=C setup - Repetitive environment configuration in APT/Snap/Flatpak
- Inconsistent interactive mode - Manual stdin/stdout/stderr handling
- New developer complexity - CommandBuilder requires deep
exec.Cmdknowledge
Decision: Standardize on CommandRunner
Based on comprehensive analysis, CommandRunner is superior to CommandBuilder for this project:
Why CommandRunner Wins
1. Automatic LC_ALL=C Handling
CommandRunner automatically prepends LC_ALL=C for consistent English output across all package managers, with user override capability:
// CommandRunner: Automatic
output, err := runner.RunContext(ctx, "yum", []string{"info", "vim"})
// CommandBuilder: Manual setup required everywhere
cmd := builder.CommandContext(ctx, "yum", "info", "vim")
cmd.Env = append(os.Environ(), "LC_ALL=C", "DEBIAN_FRONTEND=noninteractive")
2. Simplified Testing
// CommandRunner: Simple map-based mocking
mock.AddCommand("yum", []string{"info", "vim"}, []byte("output"), nil)
// CommandBuilder: Complex shell script generation
mock.AddMockResult("yum", []string{"info", "vim"}, &MockResult{
Stdout: []byte("output"), ExitCode: 0,
})
3. Built-in Interactive Support
CommandRunner has dedicated RunInteractive() method that properly handles stdin/stdout/stderr without LC_ALL=C interference.
4. Consistency with Project Goals
From CLAUDE.md: "Use KISS (Keep It Simple and Stupid) and DRY (Don't Repeat Yourself)"
CommandRunner eliminates repetitive LC_ALL=C setup and interactive mode handling across all package managers.
5. Proven Success
YUM's recent migration to CommandRunner shows:
- 100% test coverage maintained
- Cleaner, more readable code
- Robust environment variable handling
- Successful interactive mode support
Solution: Migrate All Package Managers to CommandRunner
CommandRunner Interface
type CommandRunner interface {
// Run executes a command with LC_ALL=C for consistent English output
Run(name string, args ...string) ([]byte, error)
// RunContext executes with context support and LC_ALL=C, plus optional extra env
RunContext(ctx context.Context, name string, args []string, env ...string) ([]byte, error)
// RunInteractive executes in interactive mode with stdin/stdout/stderr passthrough
RunInteractive(ctx context.Context, name string, args []string, env ...string) error
}
Implementation Plan - UPDATED
Migration Progress:
- APT ✅ COMPLETED (Issue #27 - PR #26) - Replace CommandBuilder with CommandRunner
- Snap 🔄 IN PROGRESS (Issue #28) - Replace direct exec.Command with CommandRunner
- Flatpak ⏳ PLANNED (Issue #29) - Replace direct exec.Command with CommandRunner
Benefits
Testing Benefits
- ✅ Simple mocking - Map-based command mocking vs complex shell scripts
- ✅ Environment testing - Built-in environment variable tracking
- ✅ Interactive testing - Dedicated test methods for interactive mode
- ✅ 100% test coverage - All commands can be easily mocked
Architecture Benefits
- ✅ Consistent interface - Same pattern across all package managers
- ✅ Automatic LC_ALL=C - No manual environment setup needed
- ✅ Built-in interactive support - Proper stdin/stdout/stderr handling
- ✅ Simple for new developers - Easy to understand and implement
Code Quality Benefits
- ✅ DRY principle - Eliminates repetitive environment setup
- ✅ KISS principle - Simple interface vs complex CommandBuilder
- ✅ Maintainability - Consistent patterns across codebase
Acceptance Criteria - UPDATED
- APT migrated from CommandBuilder to CommandRunner ✅ (Issue #27 - PR #26)
- Snap migrated from direct exec.Command to CommandRunner (Issue #28)
- Flatpak migrated from direct exec.Command to CommandRunner (Issue #29)
- LC_ALL=C automatically handled across all package managers ✅ (APT+YUM complete)
- Interactive mode works consistently across all package managers ✅ (APT+YUM complete)
- Comprehensive test coverage maintained for all migrations ✅ (APT complete)
- All existing functionality preserved ✅ (APT verified)
- Documentation updated to reflect architectural consistency ✅ (Updated for APT)
Completion Status: ✅ 1/3 package managers completed (APT ✅, Snap ⏳, Flatpak ⏳)
Priority
High Priority - This achieves architectural consistency and leverages the proven CommandRunner interface that's already successful with YUM.
Related Work
- ✅ CommandRunner interface implemented and proven with YUM
- ✅ YUM migration completed successfully (100% test coverage)
- ✅ APT migration completed successfully (Issue #27 - PR #26)
- ✅ MockCommandRunner supports environment variable tracking
- ✅ Interactive mode support built-in and tested
Sub-Issues
This large architectural change has been broken down into manageable sub-issues:
- Issue #27: ✅ APT CommandRunner Migration (COMPLETED by PR #26)
- Issue #28: 🔄 Snap CommandRunner Migration (In Progress)
- Issue #29: ⏳ Flatpak CommandRunner Migration (Planned)
Progress: 1/3 package managers completed. APT migration successful with full test coverage and architectural improvements.
- Dominant language
- Go
- Stars
- 17
- Forks
- 8
- PR merge metrics
- No merged PRs in 30d
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from bluet/syspkg
-
Dependency DashboardOpen
Difficulty 4/5 3-5 days Newbie friendliness 15/100
-
apt-fast manager support.May be free again @bluet claimed this 337 days ago, and no pull request is open. Open
-
testing
Difficulty 3/5 1-2 days Newbie friendliness 45/100
-
Difficulty 5/5 Over a week Newbie friendliness 20/100
-
Difficulty 4/5 3-5 days Newbie friendliness 38/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
modelcontextprotocol/go-sdk#1340 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
MHSanaei/3x-ui#6731 · 1 comment ·
Maintainers usually reply within 1 day
-
seccomp: goToNative values for loong64 and mipsle have no matching nativeToSeccomp keyPossibly taken @ricardobranco777 claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day
-
native-convergence self-host
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
JakeChampion/lang#11408 ·
Maintainers usually reply within 1 day
-
bug
Difficulty 1/5 Under an hour Newbie friendliness 92/100
open-telemetry/opentelemetry-go-compile-instrumentation#1445 ·
Maintainers usually reply within 2 days