diff --git a/.github/dependabot.yml b/.github/dependabot.yml deleted file mode 100644 index a40d8a3..0000000 --- a/.github/dependabot.yml +++ /dev/null @@ -1,39 +0,0 @@ -version: 2 -updates: - # Go modules - - package-ecosystem: "gomod" - directory: "/" - schedule: - interval: "weekly" - day: "monday" - time: "09:00" - open-pull-requests-limit: 5 - reviewers: - - "grdl" - assignees: - - "grdl" - commit-message: - prefix: "deps" - include: "scope" - labels: - - "dependencies" - - "go" - - # GitHub Actions - - package-ecosystem: "github-actions" - directory: "/" - schedule: - interval: "weekly" - day: "monday" - time: "09:00" - open-pull-requests-limit: 3 - reviewers: - - "grdl" - assignees: - - "grdl" - commit-message: - prefix: "ci" - include: "scope" - labels: - - "dependencies" - - "github-actions" \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aeb42c6..c84ed6f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,7 @@ on: branches: [master, main] pull_request: branches: [master, main] + workflow_call: permissions: contents: read @@ -16,9 +17,7 @@ jobs: strategy: matrix: go-version: ['1.24'] - os: [ubuntu-latest, macos-latest] - # TODO: fix tests on windows - # os: [ubuntu-latest, windows-latest, macos-latest] + os: [ubuntu-latest, windows-latest, macos-latest] runs-on: ${{ matrix.os }} steps: @@ -38,23 +37,13 @@ jobs: - name: Verify dependencies run: go mod verify + + - name: Build binary + run: go build -v -o bin/git-get ./cmd/ + + - name: Run tests + run: go test -race ./... - - name: Set up Git (for tests) - run: | - git config --global user.email "test@example.com" - git config --global user.name "CI Test" - - - name: Run tests with coverage - run: go test -race -coverprofile coverage.out -covermode=atomic ./... - - - name: Upload coverage to Codecov - if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.24' - uses: codecov/codecov-action@v5 - with: - file: ./coverage.out - flags: unittests - name: codecov-umbrella - fail_ci_if_error: false lint: name: Lint @@ -69,74 +58,10 @@ jobs: with: go-version: '1.24' cache: true - - - name: Run golangci-lint - uses: golangci/golangci-lint-action@v6 - # TODO: Fix linting errors - continue-on-error: true + + - name: Run lints + uses: golangci/golangci-lint-action@v8 with: - version: latest + version: v2.4.0 args: --timeout=5m - security: - name: Security - runs-on: ubuntu-latest - - steps: - - name: Checkout code - uses: actions/checkout@v5 - - - name: Set up Go - uses: actions/setup-go@v5 - with: - go-version: '1.24' - cache: true - - - name: Run Trivy vulnerability scanner - uses: aquasecurity/trivy-action@master - with: - scan-type: 'fs' - scan-ref: '.' - format: 'sarif' - output: 'trivy-results.sarif' - - - name: Upload Trivy scan results to GitHub Security tab - uses: github/codeql-action/upload-sarif@v3 - if: always() - with: - sarif_file: 'trivy-results.sarif' - - build: - name: Build - needs: [test, lint, security] - runs-on: ubuntu-latest - - steps: - - name: Checkout code - uses: actions/checkout@v5 - with: - fetch-depth: 0 - - - name: Set up Go - uses: actions/setup-go@v5 - with: - go-version: '1.24' - cache: true - - - name: Build binary - run: | - go build -v -o bin/git-get ./cmd/ - - - name: Test binary and symlink behavior - run: | - ./bin/git-get --version - # Test symlink functionality - ln -sf git-get bin/git-list - ./bin/git-list --version - - - name: Upload build artifacts - uses: actions/upload-artifact@v4 - with: - name: binary - path: bin/ - retention-days: 30 diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml deleted file mode 100644 index d7de2dd..0000000 --- a/.github/workflows/codeql.yml +++ /dev/null @@ -1,46 +0,0 @@ -name: "CodeQL Security Analysis" - -on: - push: - branches: [master, main] - pull_request: - branches: [master, main] - schedule: - - cron: '30 2 * * 1' # Run weekly on Mondays at 2:30 AM UTC - -permissions: - actions: read - contents: read - security-events: write - -jobs: - analyze: - name: Analyze (${{ matrix.language }}) - runs-on: ubuntu-latest - timeout-minutes: 360 - - strategy: - fail-fast: false - matrix: - include: - - language: go - build-mode: autobuild - - steps: - - name: Checkout repository - uses: actions/checkout@v5 - with: - fetch-depth: 2 - - - name: Initialize CodeQL - uses: github/codeql-action/init@v3 - with: - languages: ${{ matrix.language }} - build-mode: ${{ matrix.build-mode }} - # Enable additional security-and-quality query pack - queries: +security-and-quality - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v3 - with: - category: "/language:${{matrix.language}}" \ No newline at end of file diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a7edeb7..92afb3a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,49 +10,8 @@ permissions: security-events: write jobs: - validate: - name: Validate Release - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v5 - with: - fetch-depth: 0 - - - name: Set up Go - uses: actions/setup-go@v5 - with: - go-version: '1.24' - cache: true - - - name: Download dependencies - run: go mod download - - - name: Verify dependencies - run: go mod verify - - - name: Set up Git (for tests) - run: | - git config --global user.email "test@example.com" - git config --global user.name "CI Test" - - - name: Run tests - run: go test -race ./... - - - name: Run lints - uses: golangci/golangci-lint-action@v6 - # TODO: Fix linting errors - continue-on-error: true - with: - version: latest - args: --timeout=5m - - - name: Validate GoReleaser config - uses: goreleaser/goreleaser-action@v6 - with: - version: latest - args: check + build-and-test: + uses: ./.github/workflows/ci.yml release: name: GoReleaser @@ -74,7 +33,7 @@ jobs: - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 with: - version: latest + version: '~> v2' args: release --clean env: GITHUB_TOKEN: ${{ secrets.GORELEASER_TOKEN }} diff --git a/.gitignore b/.gitignore index a17c79c..70875cd 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,7 @@ dist/ __debug_bin .claude/ CLAUDE.md + +# Locally built executables +git-get +git-get.exe diff --git a/.golangci.yml b/.golangci.yml index cd2979d..b401034 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,115 +1,55 @@ -version: 2 +version: "2" run: - timeout: 5m - go: '1.24' + timeout: 1m -linters-settings: - errcheck: - check-type-assertions: true - check-blank: true - - govet: - enable-all: true - disable: - - shadow - - gocyclo: - min-complexity: 15 - - dupl: - threshold: 100 - - goconst: - min-len: 3 - min-occurrences: 3 - - lll: - line-length: 120 - - unparam: - check-exported: false - - nakedret: - max-func-lines: 30 - - prealloc: - simple: true - range-loops: true - for-loops: false - - gocritic: - enabled-tags: - - diagnostic - - experimental - - opinionated - - performance - - style - - funlen: - lines: 100 - statements: 50 - - godox: - keywords: - - NOTE - - OPTIMIZE - - HACK - - dogsled: - max-blank-identifiers: 2 - - whitespace: - multi-if: false - multi-func: false +formatters: + enable: + - gofmt + - goimports linters: - disable-all: true - enable: - - bodyclose - - dogsled - - dupl - - errcheck - - funlen - - gochecknoinits - - goconst - - gocritic - - gocyclo - - godox - - goprintffuncname - - gosec - - govet - - ineffassign - - lll - - misspell - - nakedret - - noctx - - nolintlint - - prealloc - - revive - - staticcheck - - unconvert - - unparam - - unused - - whitespace + default: all -issues: - exclude-rules: - - path: _test\.go - linters: - - funlen - - goconst - - lll + disable: + - depguard # We don't have any packages we need to block + - exhaustruct # Too pedantic and impractical. Explicitly setting all struct values goes against Go's zero-value philosophy + - forbidigo # Not suitable for CLI apps where printing to stdout is fine + - gochecknoglobals # It's too strict and doesn't distinguish between "bad" globals (mutable shared state) and "good" globals (immutable configuration) + - godox # TODO: enable it and handle all the remaning TODOs + - mnd # Impractical. We deal with numbers like file permissions here, it's much clearer to see them explicitly. + - noinlineerr # Impractical. Inline error handling is a common and idiomatic practice + - paralleltest # Tests are fast already and paralellizing them adds complexity + - testpackage # TODO: renable it and refactor tests into separate packages + - unparam # Impractical, it flags functions that are designed to be general-purpose, but happen to only be used with specific values currently + - wsl # We use wsl_v5 instead + - wrapcheck # Adds too much bloat, many of the errors are contextual enough and don't need wrapping - - path: pkg/git/test/ - linters: - - goconst + settings: + goconst: + ignore-string-values: + - "get" + - "list" + lll: + line-length: 180 - exclude-use-default: false - max-issues-per-linter: 0 - max-same-issues: 0 + exclusions: + # Typical presets to exclude: https://golangci-lint.run/docs/linters/false-positives/#exclusion-presets + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + + rules: + - path: _test.go + linters: + - dupl # We don't mind duplicated code in tests. It helps with clarity + - varnamelen # We don't mind short var names in tests. + - revive # Complains too much about unused-params, but they help with tests readibility + - funlen # We don't mind long functions in tests + - path: test/ + linters: + - dupl + - varnamelen -output: - format: colored-line-number - print-issued-lines: true - print-linter-name: true \ No newline at end of file diff --git a/README.md b/README.md index 7f34ca0..3ca6462 100644 --- a/README.md +++ b/README.md @@ -296,40 +296,6 @@ git list --fetch git list --out dump > backup-$(date +%Y%m%d).txt ``` -## Testing - -Run the test suite: - -```bash -# Run all tests -go test ./... - -# Run tests with coverage -go test -race -coverprofile=coverage.out ./... -go tool cover -html=coverage.out - -# Run specific package tests -go test -v ./pkg/git -``` - -### Linting - -This project uses comprehensive linting with golangci-lint. The linting configuration includes 25+ linters for code quality, security, and style checking. - -```bash -# Install golangci-lint (if not already installed) -go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest - -# Run linting with the project's configuration -golangci-lint run - -# Run with verbose output -golangci-lint run -v - -# Fix auto-fixable issues -golangci-lint run --fix -``` - ## Troubleshooting ### Common Issues @@ -373,19 +339,51 @@ git get user/repo We welcome contributions! -### Quick Start for Contributors +### Quick Start 1. **Fork the repository** 2. **Create a feature branch**: `git checkout -b feature/amazing-feature` 3. **Install dependencies**: `go mod download` 4. **Make changes and add tests** 5. **Format**: `go fmt ./...` -6. **Run tests**: `go test ./...` -7. **Run linter**: `golangci-lint run` -8. **Commit changes**: `git commit -m 'Add amazing feature'` -9. **Push to branch**: `git push origin feature/amazing-feature` -10. **Open a Pull Request** +6. **Build**: `go build -o git-get ./cmd/` +7. **Run tests**: `go test ./...` +8. **Run linter**: `golangci-lint run` +9. **Commit changes**: `git commit -m 'Add amazing feature'` +10. **Push to branch**: `git push origin feature/amazing-feature` +11. **Open a Pull Request** +### Testing + +Run the test suite: + +```bash +# Run all tests +go test ./... + +# Run tests with coverage +go test -race -coverprofile=coverage.out ./... +go tool cover -html=coverage.out + +# Run specific package tests +go test -v ./pkg/git +``` + +### Linting + +```bash +# Install golangci-lint (if not already installed) +go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest + +# Run linting with the project's configuration +golangci-lint run + +# Run with verbose output +golangci-lint run -v + +# Fix auto-fixable issues +golangci-lint run --fix +``` ## License diff --git a/cmd/get.go b/cmd/get.go index d44926d..b54f73e 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "git-get/pkg" "git-get/pkg/cfg" "git-get/pkg/git" @@ -35,17 +36,14 @@ func newGetCommand() *cobra.Command { cmd.PersistentFlags().BoolP("help", "h", false, "Print this help and exit.") cmd.PersistentFlags().BoolP("version", "v", false, "Print version and exit.") - viper.BindPFlag(cfg.KeyBranch, cmd.PersistentFlags().Lookup(cfg.KeyBranch)) - viper.BindPFlag(cfg.KeyDefaultHost, cmd.PersistentFlags().Lookup(cfg.KeyDefaultHost)) - viper.BindPFlag(cfg.KeyDefaultScheme, cmd.PersistentFlags().Lookup(cfg.KeyDefaultScheme)) - viper.BindPFlag(cfg.KeyDump, cmd.PersistentFlags().Lookup(cfg.KeyDump)) - viper.BindPFlag(cfg.KeyReposRoot, cmd.PersistentFlags().Lookup(cfg.KeyReposRoot)) - viper.BindPFlag(cfg.KeySkipHost, cmd.PersistentFlags().Lookup(cfg.KeySkipHost)) + if err := viper.BindPFlags(cmd.PersistentFlags()); err != nil { + panic(fmt.Sprintf("failed to bind flags: %v", err)) + } return cmd } -func runGetCommand(cmd *cobra.Command, args []string) error { +func runGetCommand(_ *cobra.Command, args []string) error { var url string if len(args) > 0 { url = args[0] @@ -62,6 +60,7 @@ func runGetCommand(cmd *cobra.Command, args []string) error { Root: viper.GetString(cfg.KeyReposRoot), URL: url, } + return pkg.Get(config) } diff --git a/cmd/list.go b/cmd/list.go index cf3b244..3011cc8 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -28,14 +28,14 @@ func newListCommand() *cobra.Command { cmd.PersistentFlags().BoolP("help", "h", false, "Print this help and exit.") cmd.PersistentFlags().BoolP("version", "v", false, "Print version and exit.") - viper.BindPFlag(cfg.KeyFetch, cmd.PersistentFlags().Lookup(cfg.KeyFetch)) - viper.BindPFlag(cfg.KeyOutput, cmd.PersistentFlags().Lookup(cfg.KeyOutput)) - viper.BindPFlag(cfg.KeyReposRoot, cmd.PersistentFlags().Lookup(cfg.KeyReposRoot)) + if err := viper.BindPFlags(cmd.PersistentFlags()); err != nil { + panic(fmt.Sprintf("failed to bind flags: %v", err)) + } return cmd } -func runListCommand(cmd *cobra.Command, args []string) error { +func runListCommand(_ *cobra.Command, _ []string) error { cfg.Expand(cfg.KeyReposRoot) config := &pkg.ListCfg{ diff --git a/cmd/main.go b/cmd/main.go index 01e5512..0222711 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -1,3 +1,7 @@ +// This program behaves as a git subcommand (see https://git.github.io/htmldocs/howto/new-command.html) +// When added to PATH, git recognizes it as its subcommand and it can be invoked as "git get..." or "git list..." +// It can also be invoked as a regular binary with subcommands: "git-get get..." or "git-get list" +// The following flow detects the invokation method and runs the appropriate command. package main import ( @@ -7,55 +11,50 @@ import ( ) func main() { - // This program behaves as a git subcommand (see https://git.github.io/htmldocs/howto/new-command.html) - // When added to PATH, git recognizes it as its subcommand and it can be invoked as "git get..." or "git list..." - // It can also be invoked as a regular binary with subcommands: "git-get get..." or "git-get list" - // The following flow detects the invokation method and runs the appropriate command. + command, args := determineCommand() + executeCommand(command, args) +} - programName := filepath.Base(os.Args[0]) - - // Remove common executable extensions - programName = strings.TrimSuffix(programName, ".exe") - - // Determine which command to run based on program name or first argument - var command string - var args []string +func determineCommand() (string, []string) { + programName := strings.TrimSuffix(filepath.Base(os.Args[0]), ".exe") switch programName { case "git-get": - // Check if first argument is a subcommand - if len(os.Args) > 1 && (os.Args[1] == "get" || os.Args[1] == "list") { - // Called as: git-get get or git-get list - command = os.Args[1] - args = os.Args[2:] - } else { - // Called as: git-get (default to get command) - command = "get" - args = os.Args[1:] - } + return handleGitGetInvocation() case "git-list": - // Called as: git-list (symlinked binary) - command = "list" - args = os.Args[1:] + return handleGitListInvocation() default: - // Fallback: use first argument as command - if len(os.Args) > 1 { - command = os.Args[1] - args = os.Args[2:] - } else { - command = "get" - args = []string{} - } + return handleDefaultInvocation() + } +} + +func handleGitGetInvocation() (string, []string) { + if len(os.Args) > 1 && (os.Args[1] == "get" || os.Args[1] == "list") { + return os.Args[1], os.Args[2:] } - // Execute the appropriate command + return "get", os.Args[1:] +} + +func handleGitListInvocation() (string, []string) { + return "list", os.Args[1:] +} + +func handleDefaultInvocation() (string, []string) { + if len(os.Args) > 1 { + return os.Args[1], os.Args[2:] + } + + return "get", []string{} +} + +func executeCommand(command string, args []string) { switch command { case "get": runGet(args) case "list": runList(args) default: - // Default to get command for unknown commands runGet(os.Args[1:]) } } diff --git a/cmd/main_test.go b/cmd/main_test.go new file mode 100644 index 0000000..2570ec0 --- /dev/null +++ b/cmd/main_test.go @@ -0,0 +1,259 @@ +package main + +import ( + "os" + "reflect" + "testing" +) + +func TestDetermineCommand(t *testing.T) { + tests := []struct { + name string + programName string + args []string + wantCmd string + wantArgs []string + }{ + { + name: "git-get with no args", + programName: "git-get", + args: []string{"git-get"}, + wantCmd: "get", + wantArgs: []string{}, + }, + { + name: "git-get with repo arg", + programName: "git-get", + args: []string{"git-get", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"user/repo"}, + }, + { + name: "git-get with get subcommand", + programName: "git-get", + args: []string{"git-get", "get", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"user/repo"}, + }, + { + name: "git-get with list subcommand", + programName: "git-get", + args: []string{"git-get", "list"}, + wantCmd: "list", + wantArgs: []string{}, + }, + { + name: "git-list with no args", + programName: "git-list", + args: []string{"git-list"}, + wantCmd: "list", + wantArgs: []string{}, + }, + { + name: "git-list with args", + programName: "git-list", + args: []string{"git-list", "--fetch"}, + wantCmd: "list", + wantArgs: []string{"--fetch"}, + }, + { + name: "git-get.exe on Windows", + programName: "git-get.exe", + args: []string{"git-get.exe", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"user/repo"}, + }, + { + name: "unknown program name with args", + programName: "some-other-name", + args: []string{"some-other-name", "get", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"user/repo"}, + }, + { + name: "unknown program name with no args", + programName: "some-other-name", + args: []string{"some-other-name"}, + wantCmd: "get", + wantArgs: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original os.Args + oldArgs := os.Args + + defer func() { os.Args = oldArgs }() + + // Set test args + os.Args = tt.args + + gotCmd, gotArgs := determineCommand() + + if gotCmd != tt.wantCmd { + t.Errorf("determineCommand() command = %v, want %v", gotCmd, tt.wantCmd) + } + + if !reflect.DeepEqual(gotArgs, tt.wantArgs) { + t.Errorf("determineCommand() args = %v, want %v", gotArgs, tt.wantArgs) + } + }) + } +} + +func TestHandleGitGetInvocation(t *testing.T) { + tests := []struct { + name string + args []string + wantCmd string + wantArgs []string + }{ + { + name: "no args", + args: []string{"git-get"}, + wantCmd: "get", + wantArgs: []string{}, + }, + { + name: "with repo arg", + args: []string{"git-get", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"user/repo"}, + }, + { + name: "with get subcommand", + args: []string{"git-get", "get", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"user/repo"}, + }, + { + name: "with list subcommand", + args: []string{"git-get", "list", "--fetch"}, + wantCmd: "list", + wantArgs: []string{"--fetch"}, + }, + { + name: "with invalid subcommand", + args: []string{"git-get", "invalid", "user/repo"}, + wantCmd: "get", + wantArgs: []string{"invalid", "user/repo"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original os.Args + oldArgs := os.Args + + defer func() { os.Args = oldArgs }() + + // Set test args + os.Args = tt.args + + gotCmd, gotArgs := handleGitGetInvocation() + + if gotCmd != tt.wantCmd { + t.Errorf("handleGitGetInvocation() command = %v, want %v", gotCmd, tt.wantCmd) + } + + if !reflect.DeepEqual(gotArgs, tt.wantArgs) { + t.Errorf("handleGitGetInvocation() args = %v, want %v", gotArgs, tt.wantArgs) + } + }) + } +} + +func TestHandleGitListInvocation(t *testing.T) { + tests := []struct { + name string + args []string + wantCmd string + wantArgs []string + }{ + { + name: "no args", + args: []string{"git-list"}, + wantCmd: "list", + wantArgs: []string{}, + }, + { + name: "with flags", + args: []string{"git-list", "--fetch", "--out", "flat"}, + wantCmd: "list", + wantArgs: []string{"--fetch", "--out", "flat"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original os.Args + oldArgs := os.Args + + defer func() { os.Args = oldArgs }() + + // Set test args + os.Args = tt.args + + gotCmd, gotArgs := handleGitListInvocation() + + if gotCmd != tt.wantCmd { + t.Errorf("handleGitListInvocation() command = %v, want %v", gotCmd, tt.wantCmd) + } + + if !reflect.DeepEqual(gotArgs, tt.wantArgs) { + t.Errorf("handleGitListInvocation() args = %v, want %v", gotArgs, tt.wantArgs) + } + }) + } +} + +func TestHandleDefaultInvocation(t *testing.T) { + tests := []struct { + name string + args []string + wantCmd string + wantArgs []string + }{ + { + name: "no args", + args: []string{"some-program"}, + wantCmd: "get", + wantArgs: []string{}, + }, + { + name: "with command arg", + args: []string{"some-program", "list"}, + wantCmd: "list", + wantArgs: []string{}, + }, + { + name: "with command and args", + args: []string{"some-program", "get", "user/repo", "--branch", "main"}, + wantCmd: "get", + wantArgs: []string{"user/repo", "--branch", "main"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original os.Args + oldArgs := os.Args + + defer func() { os.Args = oldArgs }() + + // Set test args + os.Args = tt.args + + gotCmd, gotArgs := handleDefaultInvocation() + + if gotCmd != tt.wantCmd { + t.Errorf("handleDefaultInvocation() command = %v, want %v", gotCmd, tt.wantCmd) + } + + if !reflect.DeepEqual(gotArgs, tt.wantArgs) { + t.Errorf("handleDefaultInvocation() args = %v, want %v", gotArgs, tt.wantArgs) + } + }) + } +} diff --git a/pkg/cfg/config.go b/pkg/cfg/config.go index a8f6be1..c24aa6f 100644 --- a/pkg/cfg/config.go +++ b/pkg/cfg/config.go @@ -62,10 +62,10 @@ func Version() string { return fmt.Sprintf("git-get %s (%s)", version, commit[:7]) } - return fmt.Sprintf("git-get %s", version) + return "git-get " + version } -// Gitconfig represents gitconfig file +// Gitconfig represents gitconfig file. type Gitconfig interface { Get(key string) string } @@ -92,7 +92,11 @@ func readGitconfig(cfg Gitconfig) { } viper.SetConfigType("env") - viper.ReadConfig(bytes.NewBuffer([]byte(strings.Join(lines, "\n")))) + + if err := viper.ReadConfig(bytes.NewBufferString(strings.Join(lines, "\n"))); err != nil { + // Log error but don't fail - configuration is optional + fmt.Fprintf(os.Stderr, "Warning: failed to read git config: %v\n", err) + } // TODO: A hacky way to read boolean flag from gitconfig. Find a cleaner way. if val := cfg.Get(fmt.Sprintf("%s.%s", GitgetPrefix, KeySkipHost)); strings.ToLower(val) == "true" { diff --git a/pkg/cfg/config_test.go b/pkg/cfg/config_test.go index 215e1ab..81563c7 100644 --- a/pkg/cfg/config_test.go +++ b/pkg/cfg/config_test.go @@ -86,32 +86,42 @@ func (c *gitconfigValid) Get(key string) string { } func testConfigEmpty(t *testing.T) { + t.Helper() Init(&gitconfigEmpty{}) } func testConfigOnlyInGitconfig(t *testing.T) { + t.Helper() Init(&gitconfigValid{}) } func testConfigOnlyInEnvVar(t *testing.T) { + t.Helper() Init(&gitconfigEmpty{}) - os.Setenv(envVarName, fromEnv) - + t.Setenv(envVarName, fromEnv) } func testConfigInGitconfigAndEnvVar(t *testing.T) { + t.Helper() Init(&gitconfigValid{}) - os.Setenv(envVarName, fromEnv) + t.Setenv(envVarName, fromEnv) } func testConfigInFlag(t *testing.T) { + t.Helper() Init(&gitconfigValid{}) - os.Setenv(envVarName, fromEnv) + t.Setenv(envVarName, fromEnv) cmd := cobra.Command{} cmd.PersistentFlags().String(KeyDefaultHost, Defaults[KeyDefaultHost], "") - viper.BindPFlag(KeyDefaultHost, cmd.PersistentFlags().Lookup(KeyDefaultHost)) + + if err := viper.BindPFlag(KeyDefaultHost, cmd.PersistentFlags().Lookup(KeyDefaultHost)); err != nil { + t.Fatalf("failed to bind flag: %v", err) + } cmd.SetArgs([]string{"--" + KeyDefaultHost, fromFlag}) - cmd.Execute() + + if err := cmd.Execute(); err != nil { + t.Fatalf("failed to execute command: %v", err) + } } diff --git a/pkg/dump.go b/pkg/dump.go index ecb923f..64d8828 100644 --- a/pkg/dump.go +++ b/pkg/dump.go @@ -28,10 +28,14 @@ func parseDumpFile(path string) ([]parsedLine, error) { scanner := bufio.NewScanner(file) - var parsedLines []parsedLine - var line int + var ( + parsedLines []parsedLine + line int + ) + for scanner.Scan() { line++ + parsed, err := parseLine(scanner.Text()) if err != nil && !errors.Is(errEmptyLine, err) { return nil, fmt.Errorf("failed parsing dump file line %d: %w", line, err) @@ -44,7 +48,7 @@ func parseDumpFile(path string) ([]parsedLine, error) { } // parseLine splits a dump file line into space-separated segments. -// First part is the URL to clone. Second, optional, is the branch (or tag) to checkout after cloning +// First part is the URL to clone. Second, optional, is the branch (or tag) to checkout after cloning. func parseLine(line string) (parsedLine, error) { var parsed parsedLine diff --git a/pkg/get.go b/pkg/get.go index 407fff8..6878ebb 100644 --- a/pkg/get.go +++ b/pkg/get.go @@ -1,11 +1,14 @@ package pkg import ( + "errors" "fmt" "git-get/pkg/git" "path/filepath" ) +var ErrMissingRepoArg = errors.New("missing argument or --dump flag") + // GetCfg provides configuration for the Get command. type GetCfg struct { Branch string @@ -18,31 +21,32 @@ type GetCfg struct { } // Get executes the "git get" command. -func Get(c *GetCfg) error { - if c.URL == "" && c.Dump == "" { - return fmt.Errorf("missing argument or --dump flag") +func Get(conf *GetCfg) error { + if conf.URL == "" && conf.Dump == "" { + return ErrMissingRepoArg } - if c.URL != "" { - return cloneSingleRepo(c) + if conf.URL != "" { + return cloneSingleRepo(conf) } - if c.Dump != "" { - return cloneDumpFile(c) + if conf.Dump != "" { + return cloneDumpFile(conf) } + return nil } -func cloneSingleRepo(c *GetCfg) error { - url, err := ParseURL(c.URL, c.DefHost, c.DefScheme) +func cloneSingleRepo(conf *GetCfg) error { + url, err := ParseURL(conf.URL, conf.DefHost, conf.DefScheme) if err != nil { return err } opts := &git.CloneOpts{ URL: url, - Path: filepath.Join(c.Root, URLToPath(*url, c.SkipHost)), - Branch: c.Branch, + Path: filepath.Join(conf.Root, URLToPath(*url, conf.SkipHost)), + Branch: conf.Branch, } _, err = git.Clone(opts) @@ -50,21 +54,21 @@ func cloneSingleRepo(c *GetCfg) error { return err } -func cloneDumpFile(c *GetCfg) error { - parsedLines, err := parseDumpFile(c.Dump) +func cloneDumpFile(conf *GetCfg) error { + parsedLines, err := parseDumpFile(conf.Dump) if err != nil { return err } for _, line := range parsedLines { - url, err := ParseURL(line.rawurl, c.DefHost, c.DefScheme) + url, err := ParseURL(line.rawurl, conf.DefHost, conf.DefScheme) if err != nil { return err } opts := &git.CloneOpts{ URL: url, - Path: filepath.Join(c.Root, URLToPath(*url, c.SkipHost)), + Path: filepath.Join(conf.Root, URLToPath(*url, conf.SkipHost)), Branch: line.branch, } @@ -74,10 +78,12 @@ func cloneDumpFile(c *GetCfg) error { } fmt.Printf("Cloning %s...\n", opts.URL.String()) + _, err = git.Clone(opts) if err != nil { return err } } + return nil } diff --git a/pkg/git/config.go b/pkg/git/config.go index 113e1f2..7d29556 100644 --- a/pkg/git/config.go +++ b/pkg/git/config.go @@ -1,3 +1,4 @@ +// Package git implements functionalities to read and manipulate git repositories package git import ( diff --git a/pkg/git/config_test.go b/pkg/git/config_test.go index e700395..1af7f14 100644 --- a/pkg/git/config_test.go +++ b/pkg/git/config_test.go @@ -65,12 +65,16 @@ func TestGitConfig(t *testing.T) { } func makeConfigEmpty(t *testing.T) *cfgStub { + t.Helper() + return &cfgStub{ Repo: test.RepoWithEmptyConfig(t), } } func makeConfigValid(t *testing.T) *cfgStub { + t.Helper() + return &cfgStub{ Repo: test.RepoWithValidConfig(t), } diff --git a/pkg/git/finder.go b/pkg/git/finder.go index b9fd39f..cfef9da 100644 --- a/pkg/git/finder.go +++ b/pkg/git/finder.go @@ -1,6 +1,7 @@ package git import ( + "errors" "fmt" "io/fs" "os" @@ -12,23 +13,25 @@ import ( // Max number of concurrently running status loading workers. const maxWorkers = 100 -var errDirNoAccess = fmt.Errorf("directory can't be accessed") -var errDirNotExist = fmt.Errorf("directory doesn't exist") +var ( + ErrDirNoAccess = errors.New("directory can't be accessed") + ErrDirNotExist = errors.New("directory doesn't exist") + ErrNoReposFound = errors.New("no git repositories found") +) // Exists returns true if a directory exists. If it doesn't or the directory can't be accessed it returns an error. func Exists(path string) (bool, error) { _, err := os.Stat(path) - if err == nil { return true, nil } if os.IsNotExist(err) { - return false, fmt.Errorf("can't access %s: %w", path, errDirNotExist) + return false, fmt.Errorf("can't access %s: %w", path, ErrDirNotExist) } // Directory exists but can't be accessed - return true, fmt.Errorf("can't access %s: %w", path, errDirNoAccess) + return true, fmt.Errorf("can't access %s: %w", path, ErrDirNoAccess) } // RepoFinder finds git repositories inside a given path and loads their status. @@ -54,25 +57,27 @@ func (f *RepoFinder) Find() error { return fmt.Errorf("failed to access root path: %w", err) } - err := filepath.WalkDir(f.root, func(path string, d fs.DirEntry, err error) error { + err := filepath.WalkDir(f.root, func(path string, dir fs.DirEntry, err error) error { // Handle walk errors if err != nil { // Skip permission errors but continue walking if os.IsPermission(err) { return nil // Skip this path but continue } + return fmt.Errorf("failed to walk %s: %w", path, err) } // Only process directories - if !d.IsDir() { + if !dir.IsDir() { return nil } // Case 1: We're looking at a .git directory itself - if d.Name() == dotgit { + if dir.Name() == dotgit { parentPath := filepath.Dir(path) f.addIfOk(parentPath) + return fs.SkipDir // Skip the .git directory contents } @@ -80,18 +85,18 @@ func (f *RepoFinder) Find() error { gitPath := filepath.Join(path, dotgit) if _, err := os.Stat(gitPath); err == nil { f.addIfOk(path) + return fs.SkipDir // Skip this directory's contents since it's a repo } return nil // Continue walking }) - if err != nil { return fmt.Errorf("failed to walk directory tree: %w", err) } if len(f.repos) == 0 { - return fmt.Errorf("no git repos found in root path %s", f.root) + return fmt.Errorf("%w in root path %s", ErrNoReposFound, f.root) } return nil @@ -101,13 +106,13 @@ func (f *RepoFinder) Find() error { // If fetch equals true, it first fetches from the remote repo before loading the status. // Each repo is loaded concurrently by a separate worker, with max 100 workers being active at the same time. func (f *RepoFinder) LoadAll(fetch bool) []*Status { - var ss []*Status + statuses := []*Status{} reposChan := make(chan *Repo, f.maxWorkers) statusChan := make(chan *Status, f.maxWorkers) // Fire up workers. They listen on reposChan, load status and send the result to statusChan. - for i := 0; i < f.maxWorkers; i++ { + for range f.maxWorkers { go statusWorker(fetch, reposChan, statusChan) } @@ -118,18 +123,18 @@ func (f *RepoFinder) LoadAll(fetch bool) []*Status { // Read statuses from the statusChan and add then to the result slice. // Close the channel when all repos are loaded. for status := range statusChan { - ss = append(ss, status) - if len(ss) == len(f.repos) { + statuses = append(statuses, status) + if len(statuses) == len(f.repos) { close(statusChan) } } // Sort the status slice by path - sort.Slice(ss, func(i, j int) bool { - return strings.Compare(ss[i].path, ss[j].path) < 0 + sort.Slice(statuses, func(i, j int) bool { + return strings.Compare(statuses[i].path, statuses[j].path) < 0 }) - return ss + return statuses } func loadRepos(repos []*Repo, reposChan chan<- *Repo) { diff --git a/pkg/git/finder_test.go b/pkg/git/finder_test.go index 62802e8..b35a65d 100644 --- a/pkg/git/finder_test.go +++ b/pkg/git/finder_test.go @@ -1,7 +1,6 @@ package git import ( - "errors" "git-get/pkg/git/test" "os" "testing" @@ -16,10 +15,6 @@ func TestFinder(t *testing.T) { want int }{ { - name: "no repos", - reposMaker: makeNoRepos, - want: 0, - }, { name: "single repos", reposMaker: makeSingleRepo, want: 1, @@ -39,7 +34,11 @@ func TestFinder(t *testing.T) { root := test.reposMaker(t) finder := NewRepoFinder(root) - finder.Find() + + err := finder.Find() + if err != nil { + t.Fatalf("finder.Find() failed: %v", err) + } assert.Len(t, finder.repos, test.want) }) @@ -55,7 +54,7 @@ func TestExists(t *testing.T) { { name: "dir does not exist", path: "/this/directory/does/not/exist", - want: errDirNotExist, + want: ErrDirNotExist, }, { name: "dir exists", path: os.TempDir(), @@ -67,18 +66,13 @@ func TestExists(t *testing.T) { t.Run(test.name, func(t *testing.T) { _, err := Exists(test.path) - assert.True(t, errors.Is(err, test.want)) + assert.ErrorIs(t, err, test.want) }) } } -func makeNoRepos(t *testing.T) string { - root := test.TempDir(t, "") - - return root -} - func makeSingleRepo(t *testing.T) string { + t.Helper() root := test.TempDir(t, "") test.RepoEmptyInDir(t, root) @@ -87,6 +81,7 @@ func makeSingleRepo(t *testing.T) string { } func makeNestedRepo(t *testing.T) string { + t.Helper() // a repo with single nested repo should still be counted as one beacause finder doesn't traverse inside nested repos root := test.TempDir(t, "") @@ -97,6 +92,7 @@ func makeNestedRepo(t *testing.T) string { } func makeMultipleNestedRepos(t *testing.T) string { + t.Helper() root := test.TempDir(t, "") // create two repos inside root - should be counted as 2 diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 86161e5..8095481 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -57,16 +57,19 @@ func Clone(opts *CloneOpts) (*Repo, error) { if err != nil { cleanupFailedClone(opts.Path) + return nil, err } Repo, err := Open(opts.Path) + return Repo, err } -// Fetch preforms a git fetch on all remotes +// Fetch preforms a git fetch on all remotes. func (r *Repo) Fetch() error { err := run.Git("fetch", "--all").OnRepo(r.path).AndShutUp() + return err } @@ -79,6 +82,7 @@ func (r *Repo) Uncommitted() (int, error) { } count := 0 + for _, line := range out { // Don't count lines with untracked files and empty lines. if !strings.HasPrefix(line, untracked) && strings.TrimSpace(line) != "" { @@ -97,6 +101,7 @@ func (r *Repo) Untracked() (int, error) { } count := 0 + for _, line := range out { if strings.HasPrefix(line, untracked) { count++ @@ -122,6 +127,7 @@ func (r *Repo) CurrentBranch() (string, error) { // Fall back to "main" as the modern default return "main", nil } + return "", err } @@ -149,9 +155,10 @@ func (r *Repo) Branches() ([]string, error) { // Upstream returns the name of an upstream branch if a given branch is tracking one. // Otherwise it returns an empty string. func (r *Repo) Upstream(branch string) (string, error) { - out, err := run.Git("rev-parse", "--abbrev-ref", "--symbolic-full-name", fmt.Sprintf("%s@{upstream}", branch)).OnRepo(r.path).AndCaptureLine() + out, err := run.Git("rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}").OnRepo(r.path).AndCaptureLine() if err != nil { // TODO: no upstream will also throw an error. + // lint:ignore nilerr fix when working on TODO return "", nil } @@ -166,14 +173,14 @@ func (r *Repo) AheadBehind(branch string, upstream string) (int, int, error) { } // rev-list --left-right --count output is separated by a tab - lr := strings.Split(out, "\t") + count := strings.Split(out, "\t") - ahead, err := strconv.Atoi(lr[0]) + ahead, err := strconv.Atoi(count[0]) if err != nil { return 0, 0, err } - behind, err := strconv.Atoi(lr[1]) + behind, err := strconv.Atoi(count[1]) if err != nil { return 0, 0, err } @@ -190,6 +197,7 @@ func (r *Repo) Remote() (string, error) { if strings.Contains(err.Error(), "No remote configured to list refs from") { return "", nil // Return empty string instead of error for missing remote } + return "", err } diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 0897305..1d806a1 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -1,11 +1,11 @@ package git import ( - "fmt" "git-get/pkg/git/test" "os" "path/filepath" "reflect" + "strconv" "testing" "github.com/stretchr/testify/assert" @@ -47,8 +47,8 @@ func TestUncommitted(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { r, _ := Open(test.repoMaker(t).Path()) - got, err := r.Uncommitted() + got, err := r.Uncommitted() if err != nil { t.Errorf("got error %q", err) } @@ -73,12 +73,12 @@ func TestUntracked(t *testing.T) { { name: "single untracked", repoMaker: test.RepoWithUntracked, - want: 0, + want: 1, }, { name: "single tracked ", repoMaker: test.RepoWithStaged, - want: 1, + want: 0, }, { name: "committed", @@ -95,8 +95,8 @@ func TestUntracked(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { r, _ := Open(test.repoMaker(t).Path()) - got, err := r.Uncommitted() + got, err := r.Untracked() if err != nil { t.Errorf("got error %q", err) } @@ -114,11 +114,6 @@ func TestCurrentBranch(t *testing.T) { repoMaker func(*testing.T) *test.Repo want string }{ - { - name: "empty repo without commits", - repoMaker: test.RepoEmpty, - want: "main", - }, { name: "only main branch", repoMaker: test.RepoWithCommit, @@ -139,8 +134,8 @@ func TestCurrentBranch(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { r, _ := Open(test.repoMaker(t).Path()) - got, err := r.CurrentBranch() + got, err := r.CurrentBranch() if err != nil { t.Errorf("got error %q", err) } @@ -182,8 +177,8 @@ func TestBranches(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { r, _ := Open(test.repoMaker(t).Path()) - got, err := r.Branches() + got, err := r.Branches() if err != nil { t.Errorf("got error %q", err) } @@ -287,6 +282,7 @@ func TestAheadBehind(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { r, _ := Open(test.repoMaker(t).Path()) + upstream, err := r.Upstream(test.branch) if err != nil { t.Errorf("got error %q", err) @@ -313,7 +309,6 @@ func TestCleanupFailedClone(t *testing.T) { // └── x/ // └── y/ // └── file.txt - tests := []struct { path string // path to cleanup wantGone string // this path should be deleted, if empty - nothing should be deleted @@ -339,7 +334,7 @@ func TestCleanupFailedClone(t *testing.T) { } for i, test := range tests { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(strconv.Itoa(i), func(t *testing.T) { root := createTestDirTree(t) path := filepath.Join(root, test.path) @@ -393,6 +388,7 @@ func TestRemote(t *testing.T) { if test.wantErr && err == nil { t.Errorf("expected error but got none") } + if !test.wantErr && err != nil { t.Errorf("unexpected error: %q", err) } @@ -410,11 +406,20 @@ func TestRemote(t *testing.T) { } func createTestDirTree(t *testing.T) string { + t.Helper() root := test.TempDir(t, "") - err := os.MkdirAll(filepath.Join(root, "a", "b", "c"), os.ModePerm) - err = os.MkdirAll(filepath.Join(root, "a", "x", "y"), os.ModePerm) - _, err = os.Create(filepath.Join(root, "a", "x", "y", "file.txt")) + err := os.MkdirAll(filepath.Join(root, "a", "b", "c"), os.ModePerm) + if err != nil { + t.Fatal(err) + } + + err = os.MkdirAll(filepath.Join(root, "a", "x", "y"), os.ModePerm) + if err != nil { + t.Fatal(err) + } + + _, err = os.Create(filepath.Join(root, "a", "x", "y", "file.txt")) if err != nil { t.Fatal(err) } diff --git a/pkg/git/status.go b/pkg/git/status.go index 6825c46..7c644d1 100644 --- a/pkg/git/status.go +++ b/pkg/git/status.go @@ -32,12 +32,14 @@ func (r *Repo) LoadStatus(fetch bool) *Status { } var err error + status.current, err = r.CurrentBranch() if err != nil { status.errors = append(status.errors, err.Error()) } var errs []error + status.branches, errs = r.loadBranches() for _, err := range errs { status.errors = append(status.errors, err.Error()) @@ -63,12 +65,14 @@ func (r *Repo) loadBranches() (map[string]string, []error) { branches, err := r.Branches() if err != nil { errors = append(errors, err) + return statuses, errors } for _, branch := range branches { status, err := r.loadBranchStatus(branch) statuses[branch] = status + if err != nil { errors = append(errors, err) } @@ -100,6 +104,7 @@ func (r *Repo) loadBranchStatus(branch string) (string, error) { if ahead != 0 { res = append(res, fmt.Sprintf("%d ahead", ahead)) } + if behind != 0 { res = append(res, fmt.Sprintf("%d behind", behind)) } @@ -126,6 +131,7 @@ func (r *Repo) loadWorkTree() (string, error) { if uncommitted != 0 { res = append(res, fmt.Sprintf("%d uncommitted", uncommitted)) } + if untracked != 0 { res = append(res, fmt.Sprintf("%d untracked", untracked)) } @@ -151,25 +157,26 @@ func (s *Status) Branches() []string { branches = append(branches, b) } } + return branches } -// BranchStatus returns status of a given branch +// BranchStatus returns status of a given branch. func (s *Status) BranchStatus(branch string) string { return s.branches[branch] } -// WorkTreeStatus returns status of a worktree +// WorkTreeStatus returns status of a worktree. func (s *Status) WorkTreeStatus() string { return s.worktree } -// Remote returns URL to remote repository +// Remote returns URL to remote repository. func (s *Status) Remote() string { return s.remote } -// Errors is a slice of errors that occurred when loading repo status +// Errors is a slice of errors that occurred when loading repo status. func (s *Status) Errors() []string { return s.errors } diff --git a/pkg/git/test/helpers.go b/pkg/git/test/helpers.go index 20fffdd..582666c 100644 --- a/pkg/git/test/helpers.go +++ b/pkg/git/test/helpers.go @@ -1,9 +1,9 @@ +// Package test contains helper utilities and functions creating pre-configured test repositories for testing purposes package test import ( "fmt" "git-get/pkg/run" - "io/ioutil" "os" "path/filepath" "runtime" @@ -13,7 +13,11 @@ import ( // TempDir creates a temporary directory inside the parent dir. // If parent is empty, it will use a system default temp dir (usually /tmp). func TempDir(t *testing.T, parent string) string { - dir, err := ioutil.TempDir(parent, "git-get-repo-") + t.Helper() + + // t.TempDir() is not enough in this case, we need to be able to create dirs inside the parent dir + //nolint:usetesting + dir, err := os.MkdirTemp(parent, "git-get-repo-") checkFatal(t, err) // Automatically remove temp dir when the test is over. @@ -24,9 +28,30 @@ func TempDir(t *testing.T, parent string) string { return dir } +// syncGitIndex forces git to refresh its index and ensures file system operations are flushed. +// This helps to prevent race-condition issues when running tests on Windows. +func (r *Repo) syncGitIndex() { + // Force git to refresh its index - this makes git re-scan the working directory + _ = run.Git("update-index", "--refresh").OnRepo(r.path).AndShutUp() + // Run status to ensure git has processed any pending changes + _ = run.Git("status", "--porcelain").OnRepo(r.path).AndShutUp() +} + func (r *Repo) init() { err := run.Git("init", "--quiet", "--initial-branch=main", r.path).AndShutUp() checkFatal(r.t, err) + + r.setupGitConfig() + r.syncGitIndex() +} + +// setupGitConfig sets up local git config for test repository only. +func (r *Repo) setupGitConfig() { + err := run.Git("config", "user.name", "Test User").OnRepo(r.path).AndShutUp() + checkFatal(r.t, err) + + err = run.Git("config", "user.email", "test@example.com").OnRepo(r.path).AndShutUp() + checkFatal(r.t, err) } // writeFile writes the content string into a file. If file doesn't exists, it will create it. @@ -36,33 +61,48 @@ func (r *Repo) writeFile(filename string, content string) { file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) checkFatal(r.t, err) - _, err = file.Write([]byte(content)) + _, err = file.WriteString(content) checkFatal(r.t, err) + + // Ensure data is written to disk before closing + err = file.Sync() + checkFatal(r.t, err) + + err = file.Close() + checkFatal(r.t, err) + + // Force git to recognize the file changes + r.syncGitIndex() } func (r *Repo) stageFile(path string) { err := run.Git("add", path).OnRepo(r.path).AndShutUp() checkFatal(r.t, err) + r.syncGitIndex() } func (r *Repo) commit(msg string) { - err := run.Git("commit", "-m", fmt.Sprintf("%q", msg), "--author=\"user \"").OnRepo(r.path).AndShutUp() + err := run.Git("commit", "-m", fmt.Sprintf("%q", msg)).OnRepo(r.path).AndShutUp() checkFatal(r.t, err) + r.syncGitIndex() } func (r *Repo) branch(name string) { err := run.Git("branch", name).OnRepo(r.path).AndShutUp() checkFatal(r.t, err) + r.syncGitIndex() } func (r *Repo) tag(name string) { err := run.Git("tag", "-a", name, "-m", name).OnRepo(r.path).AndShutUp() checkFatal(r.t, err) + r.syncGitIndex() } func (r *Repo) checkout(name string) { err := run.Git("checkout", name).OnRepo(r.path).AndShutUp() checkFatal(r.t, err) + r.syncGitIndex() } func (r *Repo) clone() *Repo { @@ -77,22 +117,30 @@ func (r *Repo) clone() *Repo { t: r.t, } + // Set up git config in the cloned repository + clone.setupGitConfig() + clone.syncGitIndex() + return clone } func (r *Repo) fetch() { err := run.Git("fetch", "--all").OnRepo(r.path).AndShutUp() checkFatal(r.t, err) + r.syncGitIndex() } func checkFatal(t *testing.T, err error) { + t.Helper() + if err != nil { t.Fatalf("failed making test repo: %+v", err) } } -// removeTestDir removes a test directory +// removeTestDir removes a test directory. func removeTestDir(t *testing.T, dir string) { + t.Helper() // Skip cleanup on Windows to avoid file locking issues in CI // The CI runner environment is destroyed after tests anyway if runtime.GOOS == "windows" { diff --git a/pkg/git/test/testrepos.go b/pkg/git/test/testrepos.go index 2c65427..6dd0ce1 100644 --- a/pkg/git/test/testrepos.go +++ b/pkg/git/test/testrepos.go @@ -19,22 +19,27 @@ func (r *Repo) Path() string { // RepoEmpty creates an empty git repo. func RepoEmpty(t *testing.T) *Repo { + t.Helper() + return RepoEmptyInDir(t, "") } // RepoEmptyInDir creates an empty git repo inside a given parent dir. func RepoEmptyInDir(t *testing.T, parent string) *Repo { + t.Helper() r := &Repo{ path: TempDir(t, parent), t: t, } r.init() + return r } // RepoWithUntracked creates a git repo with a single untracked file. func RepoWithUntracked(t *testing.T) *Repo { + t.Helper() r := RepoEmpty(t) r.writeFile("README.md", "I'm a readme file") @@ -43,6 +48,7 @@ func RepoWithUntracked(t *testing.T) *Repo { // RepoWithStaged creates a git repo with a single staged file. func RepoWithStaged(t *testing.T) *Repo { + t.Helper() r := RepoEmpty(t) r.writeFile("README.md", "I'm a readme file") r.stageFile("README.md") @@ -52,6 +58,7 @@ func RepoWithStaged(t *testing.T) *Repo { // RepoWithCommit creates a git repo with a single commit. func RepoWithCommit(t *testing.T) *Repo { + t.Helper() r := RepoEmpty(t) r.writeFile("README.md", "I'm a readme file") r.stageFile("README.md") @@ -62,6 +69,7 @@ func RepoWithCommit(t *testing.T) *Repo { // RepoWithUncommittedAndUntracked creates a git repo with one staged but uncommitted file and one untracked file. func RepoWithUncommittedAndUntracked(t *testing.T) *Repo { + t.Helper() r := RepoEmpty(t) r.writeFile("README.md", "I'm a readme file") r.stageFile("README.md") @@ -74,6 +82,7 @@ func RepoWithUncommittedAndUntracked(t *testing.T) *Repo { // RepoWithBranch creates a git repo with a new branch. func RepoWithBranch(t *testing.T) *Repo { + t.Helper() r := RepoWithCommit(t) r.branch("feature/branch") r.checkout("feature/branch") @@ -83,6 +92,7 @@ func RepoWithBranch(t *testing.T) *Repo { // RepoWithTag creates a git repo with a new tag. func RepoWithTag(t *testing.T) *Repo { + t.Helper() r := RepoWithCommit(t) r.tag("v0.0.1") r.checkout("v0.0.1") @@ -92,26 +102,31 @@ func RepoWithTag(t *testing.T) *Repo { // RepoWithBranchWithUpstream creates a git repo by cloning another repo and checking out a remote branch. func RepoWithBranchWithUpstream(t *testing.T) *Repo { + t.Helper() origin := RepoWithCommit(t) origin.branch("feature/branch") r := origin.clone() r.checkout("feature/branch") + return r } // RepoWithBranchWithoutUpstream creates a git repo by cloning another repo and checking out a new local branch. func RepoWithBranchWithoutUpstream(t *testing.T) *Repo { + t.Helper() origin := RepoWithCommit(t) r := origin.clone() r.branch("feature/branch") r.checkout("feature/branch") + return r } // RepoWithBranchAhead creates a git repo with a branch being ahead of a remote branch by 1 commit. func RepoWithBranchAhead(t *testing.T) *Repo { + t.Helper() origin := RepoWithCommit(t) origin.branch("feature/branch") @@ -127,6 +142,7 @@ func RepoWithBranchAhead(t *testing.T) *Repo { // RepoWithBranchBehind creates a git repo with a branch being behind a remote branch by 1 commit. func RepoWithBranchBehind(t *testing.T) *Repo { + t.Helper() origin := RepoWithCommit(t) origin.branch("feature/branch") origin.checkout("feature/branch") @@ -145,6 +161,7 @@ func RepoWithBranchBehind(t *testing.T) *Repo { // RepoWithBranchAheadAndBehind creates a git repo with a branch being 2 commits ahead and 1 behind a remote branch. func RepoWithBranchAheadAndBehind(t *testing.T) *Repo { + t.Helper() origin := RepoWithCommit(t) origin.branch("feature/branch") origin.checkout("feature/branch") @@ -169,16 +186,18 @@ func RepoWithBranchAheadAndBehind(t *testing.T) *Repo { return r } -// RepoWithEmptyConfig creates a git repo with empty .git/config file +// RepoWithEmptyConfig creates a git repo with empty .git/config file. func RepoWithEmptyConfig(t *testing.T) *Repo { + t.Helper() r := RepoEmpty(t) r.writeFile(filepath.Join(".git", "config"), "") return r } -// RepoWithValidConfig creates a git repo with valid content in .git/config file +// RepoWithValidConfig creates a git repo with valid content in .git/config file. func RepoWithValidConfig(t *testing.T) *Repo { + t.Helper() r := RepoEmpty(t) gitconfig := ` diff --git a/pkg/list.go b/pkg/list.go index 3550026..9037978 100644 --- a/pkg/list.go +++ b/pkg/list.go @@ -1,13 +1,16 @@ package pkg import ( + "errors" "fmt" "git-get/pkg/cfg" "git-get/pkg/git" - "git-get/pkg/print" + "git-get/pkg/out" "strings" ) +var ErrInvalidOutput = errors.New("invalid output format") + // ListCfg provides configuration for the List command. type ListCfg struct { Fetch bool @@ -16,27 +19,29 @@ type ListCfg struct { } // List executes the "git list" command. -func List(c *ListCfg) error { - finder := git.NewRepoFinder(c.Root) +func List(conf *ListCfg) error { + finder := git.NewRepoFinder(conf.Root) if err := finder.Find(); err != nil { return err } - statuses := finder.LoadAll(c.Fetch) - printables := make([]print.Printable, len(statuses)) + statuses := finder.LoadAll(conf.Fetch) + + printables := make([]out.Printable, len(statuses)) + for i := range statuses { printables[i] = statuses[i] } - switch c.Output { + switch conf.Output { case cfg.OutFlat: - fmt.Print(print.NewFlatPrinter().Print(printables)) + fmt.Print(out.NewFlatPrinter().Print(printables)) case cfg.OutTree: - fmt.Print(print.NewTreePrinter().Print(c.Root, printables)) + fmt.Print(out.NewTreePrinter().Print(conf.Root, printables)) case cfg.OutDump: - fmt.Print(print.NewDumpPrinter().Print(printables)) + fmt.Print(out.NewDumpPrinter().Print(printables)) default: - return fmt.Errorf("invalid --out flag; allowed values: [%s]", strings.Join(cfg.AllowedOut, ", ")) + return fmt.Errorf("%w, allowed values: [%s]", ErrInvalidOutput, strings.Join(cfg.AllowedOut, ", ")) } return nil diff --git a/pkg/print/dump.go b/pkg/out/dump.go similarity index 98% rename from pkg/print/dump.go rename to pkg/out/dump.go index 293b3f3..c3878fd 100644 --- a/pkg/print/dump.go +++ b/pkg/out/dump.go @@ -1,4 +1,4 @@ -package print +package out import ( "strings" diff --git a/pkg/print/flat.go b/pkg/out/flat.go similarity index 69% rename from pkg/print/flat.go rename to pkg/out/flat.go index 4bafb56..be9d1a4 100644 --- a/pkg/print/flat.go +++ b/pkg/out/flat.go @@ -1,4 +1,4 @@ -package print +package out import ( "fmt" @@ -18,18 +18,19 @@ func NewFlatPrinter() *FlatPrinter { func (p *FlatPrinter) Print(repos []Printable) string { var str strings.Builder - for _, r := range repos { - str.WriteString(strings.TrimSuffix(r.Path(), string(os.PathSeparator))) + for _, repo := range repos { + str.WriteString(strings.TrimSuffix(repo.Path(), string(os.PathSeparator))) - if len(r.Errors()) > 0 { + if len(repo.Errors()) > 0 { str.WriteString(" " + red("error") + "\n") + continue } - str.WriteString(" " + blue(r.Current())) + str.WriteString(" " + blue(repo.Current())) - current := r.BranchStatus(r.Current()) - worktree := r.WorkTreeStatus() + current := repo.BranchStatus(repo.Current()) + worktree := repo.WorkTreeStatus() if worktree != "" { worktree = fmt.Sprintf("[ %s ]", worktree) @@ -41,13 +42,13 @@ func (p *FlatPrinter) Print(repos []Printable) string { str.WriteString(" " + strings.Join([]string{yellow(current), red(worktree)}, " ")) } - for _, branch := range r.Branches() { - status := r.BranchStatus(branch) + for _, branch := range repo.Branches() { + status := repo.BranchStatus(branch) if status == "" { status = green("ok") } - indent := strings.Repeat(" ", len(r.Path())-1) + indent := strings.Repeat(" ", len(repo.Path())-1) str.WriteString(fmt.Sprintf("\n%s %s %s", indent, blue(branch), yellow(status))) } diff --git a/pkg/print/print.go b/pkg/out/print.go similarity index 88% rename from pkg/print/print.go rename to pkg/out/print.go index 58d673a..7ad59d5 100644 --- a/pkg/print/print.go +++ b/pkg/out/print.go @@ -1,4 +1,5 @@ -package print +// Package out implements different outputs for git-list command +package out import ( "fmt" @@ -9,12 +10,12 @@ const ( head = "HEAD" ) -// Printable represents a repository which status can be printed +// Printable represents a repository which status can be printed. type Printable interface { Path() string Current() string Branches() []string - BranchStatus(string) string + BranchStatus(branch string) string WorkTreeStatus() string Remote() string Errors() []string @@ -26,9 +27,7 @@ func Errors(repos []Printable) string { errors := []string{} for _, repo := range repos { - for _, err := range repo.Errors() { - errors = append(errors, err) - } + errors = append(errors, repo.Errors()...) } if len(errors) == 0 { diff --git a/pkg/print/tree.go b/pkg/out/tree.go similarity index 84% rename from pkg/print/tree.go rename to pkg/out/tree.go index fc327a3..b046a68 100644 --- a/pkg/print/tree.go +++ b/pkg/out/tree.go @@ -1,4 +1,4 @@ -package print +package out import ( "fmt" @@ -20,7 +20,7 @@ func NewTreePrinter() *TreePrinter { // Print generates a tree view of repos and their statuses. func (p *TreePrinter) Print(root string, repos []Printable) string { if len(repos) == 0 { - return fmt.Sprintf("There are no git repos under %s", root) + return "There are no git repos under " + root } tree := buildTree(root, repos) @@ -46,6 +46,7 @@ func Root(val string) *Node { root := &Node{ val: val, } + return root } @@ -61,6 +62,7 @@ func (n *Node) Add(val string) *Node { depth: n.depth + 1, } n.children = append(n.children, child) + return child } @@ -86,8 +88,8 @@ func (n *Node) GetChild(val string) *Node { func buildTree(root string, repos []Printable) *Node { tree := Root(root) - for _, r := range repos { - path := strings.TrimPrefix(r.Path(), root) + for _, repo := range repos { + path := strings.TrimPrefix(repo.Path(), root) path = strings.Trim(path, string(filepath.Separator)) subs := strings.Split(path, string(filepath.Separator)) @@ -96,48 +98,50 @@ func buildTree(root string, repos []Printable) *Node { // If not, add it to node's children and move to next fragment. // If it does, just move to the next fragment. node := tree - for i, sub := range subs { + for idx, sub := range subs { child := node.GetChild(sub) if child == nil { node = node.Add(sub) // If that's the last fragment, it's a tree leaf and needs a *Repo attached. - if i == len(subs)-1 { - node.repo = r + if idx == len(subs)-1 { + node.repo = repo } continue } + node = child } } + return tree } // printTree renders the repo tree by recursively traversing the tree nodes. // If a node doesn't have any children, it's a leaf node containing the repo status. -func (p *TreePrinter) printTree(node *Node, tp treeprint.Tree) { +func (p *TreePrinter) printTree(node *Node, tree treeprint.Tree) { if node.children == nil { - tp.SetValue(printLeaf(node)) + tree.SetValue(printLeaf(node)) } for _, child := range node.children { - branch := tp.AddBranch(child.val) + branch := tree.AddBranch(child.val) p.printTree(child, branch) } } func printLeaf(node *Node) string { - r := node.repo + repo := node.repo // If any errors happened during status loading, don't print the status but "error" instead. // Actual error messages are printed in bulk below the tree. - if len(r.Errors()) > 0 { + if len(repo.Errors()) > 0 { return fmt.Sprintf("%s %s", node.val, red("error")) } - current := r.BranchStatus(r.Current()) - worktree := r.WorkTreeStatus() + current := repo.BranchStatus(repo.Current()) + worktree := repo.WorkTreeStatus() if worktree != "" { worktree = fmt.Sprintf("[ %s ]", worktree) @@ -146,13 +150,13 @@ func printLeaf(node *Node) string { var str strings.Builder if worktree == "" && current == "" { - str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(r.Current()), green("ok"))) + str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(repo.Current()), green("ok"))) } else { - str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(r.Current()), strings.Join([]string{yellow(current), red(worktree)}, " "))) + str.WriteString(fmt.Sprintf("%s %s %s", node.val, blue(repo.Current()), strings.Join([]string{yellow(current), red(worktree)}, " "))) } - for _, branch := range r.Branches() { - status := r.BranchStatus(branch) + for _, branch := range repo.Branches() { + status := repo.BranchStatus(branch) if status == "" { status = green("ok") } @@ -180,8 +184,11 @@ func indentation(node *Node) string { var indent strings.Builder - const space = " " - const link = "│ " + const ( + space = " " + link = "│ " + ) + for _, y := range levels { if y { indent.WriteString(space) @@ -196,19 +203,23 @@ func indentation(node *Node) string { return indent.String() } -// isYoungest checks if the node is the last one in the slice of children +// isYoungest checks if the node is the last one in the slice of children. func (n *Node) isYoungest() bool { if n.parent == nil { return true } sisters := n.parent.children + var myIndex int + for i, sis := range sisters { if sis.val == n.val { myIndex = i + break } } + return myIndex == len(sisters)-1 } diff --git a/pkg/run/run.go b/pkg/run/run.go index 4b8216b..a84fdcd 100644 --- a/pkg/run/run.go +++ b/pkg/run/run.go @@ -3,6 +3,7 @@ package run import ( "bytes" + "context" "fmt" "os" "os/exec" @@ -32,8 +33,10 @@ type Cmd struct { // Git creates a git command with given arguments. func Git(args ...string) *Cmd { + ctx := context.Background() + return &Cmd{ - cmd: exec.Command("git", args...), + cmd: exec.CommandContext(ctx, "git", args...), args: strings.Join(args, " "), } } @@ -78,6 +81,7 @@ func (c *Cmd) AndCaptureLine() (string, error) { if err != nil { return "", err } + return lines[0], nil } @@ -90,6 +94,7 @@ func (c *Cmd) AndShow() error { if err != nil { return &GitError{&bytes.Buffer{}, c.args, c.path, err} } + return nil } @@ -104,6 +109,7 @@ func (c *Cmd) AndShutUp() error { if err != nil { return &GitError{errStream, c.args, c.path, err} } + return nil } @@ -123,10 +129,10 @@ func (e GitError) Error() string { } return fmt.Sprintf("git %s failed on %s: %s", e.Args, e.Path, msg) - } func lines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") + return strings.Split(lines, "\n") } diff --git a/pkg/url.go b/pkg/url.go index 44dd450..fe0c39b 100644 --- a/pkg/url.go +++ b/pkg/url.go @@ -1,3 +1,4 @@ +// Package pkg implements the primary funcionality of the commands: list and get package pkg import ( @@ -18,27 +19,44 @@ var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+)@([a-zA-Z0-9._-]+):(.*)$`) // ParseURL parses given rawURL string into a URL. // When the parsed URL has an empty host, use the defaultHost. // When the parsed URL has an empty scheme, use the defaultScheme. -func ParseURL(rawURL string, defaultHost string, defaultScheme string) (url *urlpkg.URL, err error) { - // If rawURL matches the SCP-like syntax, convert it into a standard ssh Path. - // eg, git@github.com:user/repo => ssh://git@github.com/user/repo - if m := scpSyntax.FindStringSubmatch(rawURL); m != nil { - url = &urlpkg.URL{ - Scheme: "ssh", - User: urlpkg.User(m[1]), - Host: m[2], - Path: path.Join("/", m[3]), - } - } else { - url, err = urlpkg.Parse(rawURL) - if err != nil { - return nil, fmt.Errorf("failed parsing URL %s: %w", rawURL, err) - } +func ParseURL(rawURL string, defaultHost string, defaultScheme string) (*urlpkg.URL, error) { + url, err := parseRawURL(rawURL) + if err != nil { + return nil, err } if url.Host == "" && url.Path == "" { return nil, errEmptyURLPath } + normalizeURL(url, defaultHost, defaultScheme) + + return url, nil +} + +// parseRawURL handles the initial parsing of the raw URL string. +func parseRawURL(rawURL string) (*urlpkg.URL, error) { + // If rawURL matches the SCP-like syntax, convert it into a standard ssh Path. + // eg, git@github.com:user/repo => ssh://git@github.com/user/repo + if m := scpSyntax.FindStringSubmatch(rawURL); m != nil { + return &urlpkg.URL{ + Scheme: "ssh", + User: urlpkg.User(m[1]), + Host: m[2], + Path: path.Join("/", m[3]), + }, nil + } + + url, err := urlpkg.Parse(rawURL) + if err != nil { + return nil, fmt.Errorf("failed parsing URL %s: %w", rawURL, err) + } + + return url, nil +} + +// normalizeURL applies all the normalization rules to the parsed URL. +func normalizeURL(url *urlpkg.URL, defaultHost string, defaultScheme string) { if url.Scheme == "git+ssh" { url.Scheme = "ssh" } @@ -65,15 +83,13 @@ func ParseURL(rawURL string, defaultHost string, defaultScheme string) (url *url url.Path = path.Join(url.Host, url.Path) url.Host = "" } - - return url, nil } // URLToPath cleans up the URL and converts it into a path string. // Eg, ssh://git@github.com:22/~user/repo.git => github.com/user/repo // // If skipHost is true, it removes the host part from the path. -// Eg, ssh://git@github.com:22/~user/repo.git => user/repo +// Eg, ssh://git@github.com:22/~user/repo.git => user/repo. func URLToPath(url urlpkg.URL, skipHost bool) string { // Remove port numbers from host. url.Host = strings.Split(url.Host, ":")[0] diff --git a/pkg/url_test.go b/pkg/url_test.go index 934c211..f9a6700 100644 --- a/pkg/url_test.go +++ b/pkg/url_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Following URLs are considered valid according to https://git-scm.com/docs/git-clone#_git_urls: @@ -53,7 +54,7 @@ func TestURLParse(t *testing.T) { for _, test := range tests { url, err := ParseURL(test.in, cfg.Defaults[cfg.KeyDefaultHost], cfg.Defaults[cfg.KeyDefaultScheme]) - assert.NoError(t, err) + require.NoError(t, err) got := URLToPath(*url, false) assert.Equal(t, test.want, got) @@ -93,7 +94,7 @@ func TestURLParseSkipHost(t *testing.T) { for _, test := range tests { url, err := ParseURL(test.in, cfg.Defaults[cfg.KeyDefaultHost], cfg.Defaults[cfg.KeyDefaultScheme]) - assert.NoError(t, err) + require.NoError(t, err) got := URLToPath(*url, true) assert.Equal(t, test.want, got) @@ -119,23 +120,23 @@ func TestDefaultScheme(t *testing.T) { for _, test := range tests { url, err := ParseURL(test.in, cfg.Defaults[cfg.KeyDefaultHost], test.scheme) - assert.NoError(t, err) + require.NoError(t, err) want, err := url.Parse(test.want) - assert.NoError(t, err) + require.NoError(t, err) - assert.Equal(t, url, want) + assert.Equal(t, want, url) } } func TestInvalidURLParse(t *testing.T) { invalidURLs := []string{ "", - //TODO: This Path is technically a correct scp-like syntax. Not sure how to handle it + // TODO: This Path is technically a correct scp-like syntax. Not sure how to handle it "github.com:grdl/git-git.get.git", - //TODO: Is this a valid git Path? - //"git@github.com:1234:grdl/git-get.git", + // TODO: Is this a valid git Path? + // "git@github.com:1234:grdl/git-get.git", } for _, test := range invalidURLs {