diff --git a/.gitignore b/.gitignore index a5b1187..321d933 100644 --- a/.gitignore +++ b/.gitignore @@ -29,4 +29,7 @@ gs_* .vscode dist -gih_dev \ No newline at end of file +gih_dev + +.worktrees/ +tasks/ \ No newline at end of file diff --git a/gih/main.go b/gih/main.go index fe91067..09e4126 100644 --- a/gih/main.go +++ b/gih/main.go @@ -12,7 +12,7 @@ import ( // set by build var ( - version = "0.15.0" + version = "0.16.0" goversion = "1.26.2" ) @@ -75,7 +75,7 @@ func main() { writer := os.Stdout errWriter := os.Stderr - err := (&syncer.Sync{ + summary, err := (&syncer.Sync{ TargetDir: *targetDir, IgnoreDir: *ignoreDir, TimeOut: *timeout, @@ -86,7 +86,15 @@ func main() { Writer: printer.NewPrinter(writer, errWriter), }).Run() + // Map run outcome to exit codes per README spec: + // 0 - all operations completed successfully + // 1 - setup error (invalid args, no repos, invalid regex/timeout) + // 2 - some repositories failed or timed out if err != nil { - panic(err) + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + if summary != nil && summary.HasFailures() { + os.Exit(2) } } diff --git a/printer/print.go b/printer/print.go index a7d492f..5d1cc97 100644 --- a/printer/print.go +++ b/printer/print.go @@ -5,6 +5,7 @@ import ( "log" "os" "strings" + "sync" "text/template" "github.com/fatih/color" @@ -44,8 +45,24 @@ const repoErrTmpl = ` {{ range .Repos }}- {{ . }} {{ end }}` +// Templates are parsed once at package init. text/template.Template.Execute is +// safe for concurrent use, so the same parsed template can be shared across +// goroutines without re-parsing on every call. +var ( + successTpl = template.Must(template.New("success").Funcs(helpers).Parse(successTmpl)) + errTpl = template.Must(template.New("err").Funcs(helpers).Parse(errTmpl)) + cmdTpl = template.Must(template.New("cmd").Funcs(helpers).Parse(cmdTmpl)) + msgTpl = template.Must(template.New("msg").Funcs(helpers).Parse(msgTmpl)) + msgErrTpl = template.Must(template.New("msgErr").Funcs(helpers).Parse(msgErrTmpl)) + repoErrTpl = template.Must(template.New("repoErr").Funcs(helpers).Parse(repoErrTmpl)) +) + // Printer is struct type Printer struct { + // mu serialises template.Execute calls so that the multiple Write calls + // emitted per Execute don't interleave across goroutines, even when + // writer and errWriter end up at the same TTY. + mu sync.Mutex writer io.Writer errWriter io.Writer } @@ -78,9 +95,9 @@ func (p *Printer) PrintCmd(cmd string, options []string) { Cmd string Ops string } - t := template.Must(template.New("item").Funcs(helpers).Parse(cmdTmpl)) - err := t.Execute(p.writer, cmds{Cmd: cmd, Ops: strings.Join(options, " ")}) - if err != nil { + p.mu.Lock() + defer p.mu.Unlock() + if err := cmdTpl.Execute(p.writer, cmds{Cmd: cmd, Ops: strings.Join(options, " ")}); err != nil { log.Println(err) } } @@ -90,9 +107,9 @@ func (p *Printer) PrintMsg(msg string) { type message struct { Msg string } - t := template.Must(template.New("msg").Funcs(helpers).Parse(msgTmpl)) - err := t.Execute(p.writer, message{Msg: msg}) - if err != nil { + p.mu.Lock() + defer p.mu.Unlock() + if err := msgTpl.Execute(p.writer, message{Msg: msg}); err != nil { log.Println(err) } } @@ -102,9 +119,9 @@ func (p *Printer) PrintMsgErr(msg string) { type message struct { Msg string } - t := template.Must(template.New("msg").Funcs(helpers).Parse(msgErrTmpl)) - err := t.Execute(p.writer, message{Msg: msg}) - if err != nil { + p.mu.Lock() + defer p.mu.Unlock() + if err := msgErrTpl.Execute(p.writer, message{Msg: msg}); err != nil { log.Println(err) } } @@ -115,17 +132,18 @@ func (p *Printer) PrintRepoErr(msg string, repos []string) { Msg string Repos []string } - t := template.Must(template.New("msg").Funcs(helpers).Parse(repoErrTmpl)) - err := t.Execute(p.writer, message{Msg: msg, Repos: repos}) - if err != nil { + p.mu.Lock() + defer p.mu.Unlock() + if err := repoErrTpl.Execute(p.writer, message{Msg: msg, Repos: repos}); err != nil { log.Println(err) } } // Print prints result func (p *Printer) Print(res Result) { - err := t(true).Execute(p.writer, res) - if err != nil { + p.mu.Lock() + defer p.mu.Unlock() + if err := successTpl.Execute(p.writer, res); err != nil { log.Println(err) } } @@ -133,15 +151,9 @@ func (p *Printer) Print(res Result) { // Error prints error func (p *Printer) Error(res Result) { res.Msg = res.Err.Error() - err := t(false).Execute(p.errWriter, res) - if err != nil { + p.mu.Lock() + defer p.mu.Unlock() + if err := errTpl.Execute(p.errWriter, res); err != nil { log.Println(err) } } - -func t(isSuccess bool) *template.Template { - if isSuccess { - return template.Must(template.New("item").Funcs(helpers).Parse(successTmpl)) - } - return template.Must(template.New("item").Funcs(helpers).Parse(errTmpl)) -} diff --git a/printer/print_test.go b/printer/print_test.go new file mode 100644 index 0000000..95c81ca --- /dev/null +++ b/printer/print_test.go @@ -0,0 +1,188 @@ +package printer + +import ( + "bytes" + "errors" + "io" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/matryer/is" +) + +func TestPrinter_PrintMsg_ConcurrentSafe(t *testing.T) { + is := is.New(t) + + var buf bytes.Buffer + p := NewPrinter(&buf, &buf) + + const N = 200 + var wg sync.WaitGroup + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + p.PrintMsg("hello") + }() + } + wg.Wait() + + out := strings.TrimRight(buf.String(), "\n") + lines := strings.Split(out, "\n") + is.Equal(len(lines), N) + for _, l := range lines { + is.True(strings.Contains(l, "hello")) + } +} + +func TestPrinter_Print_ConcurrentSafe(t *testing.T) { + // successTmpl: "{{.Repo}}\n {{.Msg}}\n" — 2 lines per call. If a lock is + // missing here, the two lines from one call can interleave with another + // call's lines, breaking the strict pair structure verified below. + is := is.New(t) + + var buf bytes.Buffer + p := NewPrinter(&buf, &buf) + + const N = 200 + var wg sync.WaitGroup + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + p.Print(Result{Repo: "/path/repo", Msg: "ok"}) + }() + } + wg.Wait() + + lines := strings.Split(buf.String(), "\n") + if len(lines) > 0 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + is.Equal(len(lines), N*2) + + for i := 0; i < len(lines); i += 2 { + is.True(strings.Contains(lines[i], "/path/repo")) + is.True(strings.HasPrefix(lines[i+1], " ")) + is.True(strings.Contains(lines[i+1], "ok")) + } +} + +func TestPrinter_Error_ConcurrentSafe(t *testing.T) { + is := is.New(t) + + var errBuf bytes.Buffer + p := NewPrinter(io.Discard, &errBuf) + + const N = 100 + var wg sync.WaitGroup + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + p.Error(Result{Repo: "/path/r", Err: errors.New("boom")}) + }() + } + wg.Wait() + + lines := strings.Split(errBuf.String(), "\n") + if len(lines) > 0 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + is.Equal(len(lines), N*2) + for i := 0; i < len(lines); i += 2 { + is.True(strings.Contains(lines[i], "/path/r")) + is.True(strings.HasPrefix(lines[i+1], " ")) + is.True(strings.Contains(lines[i+1], "boom")) + } +} + +// concurrencyProbe is a thin io.Writer that records the maximum number of +// concurrent in-flight Write calls. Used to verify that a single mutex covers +// BOTH writer and errWriter in Printer. +type concurrencyProbe struct { + inflight atomic.Int32 + peak atomic.Int32 +} + +func (c *concurrencyProbe) Write(p []byte) (int, error) { + n := c.inflight.Add(1) + for { + cur := c.peak.Load() + if n <= cur { + break + } + if c.peak.CompareAndSwap(cur, n) { + break + } + } + // Tiny sleep widens the race window so that, if Print and Error were + // protected by separate mutexes, the probe would observe inflight >= 2. + time.Sleep(20 * time.Microsecond) + c.inflight.Add(-1) + return len(p), nil +} + +func TestPrinter_SharedMutex_AcrossWriterAndErrWriter(t *testing.T) { + // Wires both writer and errWriter to the same probe. Print uses writer, + // Error uses errWriter. With a single mutex covering both, no two Writes + // can be in-flight at once → peak == 1. With separate mutexes, peak >= 2 + // would be observed with high probability. + is := is.New(t) + + probe := &concurrencyProbe{} + p := NewPrinter(probe, probe) + + const N = 200 + var wg sync.WaitGroup + for i := 0; i < N; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + if i%2 == 0 { + p.Print(Result{Repo: "/r", Msg: "ok"}) + } else { + p.Error(Result{Repo: "/r", Err: errors.New("boom")}) + } + }(i) + } + wg.Wait() + + is.Equal(int(probe.peak.Load()), 1) +} + +func TestPrinter_MixedPrints_ConcurrentSafe(t *testing.T) { + // Smoke test exercising every public Printer method concurrently. Catches + // any lock omission via the race detector even when message-shape tests + // don't apply (e.g. PrintCmd, PrintRepoErr). + var buf bytes.Buffer + p := NewPrinter(&buf, &buf) + + var wg sync.WaitGroup + for i := 0; i < 200; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + switch i % 7 { + case 0: + p.PrintMsg("m") + case 1: + p.PrintMsgErr("e") + case 2: + p.PrintRepoErr("re", []string{"a", "b"}) + case 3: + p.PrintCmd("status", []string{"--short"}) + case 4: + p.Print(Result{Repo: "/path/r", Msg: "ok"}) + case 5: + p.Error(Result{Repo: "/path/r", Err: errors.New("boom")}) + case 6: + p.PrintMsg("another") + } + }(i) + } + wg.Wait() +} diff --git a/syncer/dir.go b/syncer/dir.go index 1475dfc..28df434 100644 --- a/syncer/dir.go +++ b/syncer/dir.go @@ -2,10 +2,14 @@ package syncer import ( "os" + "path/filepath" "strings" ) -// ListDirs returns target directories +// ListDirs returns the names of direct child directories that look like git +// repositories, skipping hidden entries (those whose name starts with "."). +// The order matters for efficiency: HasPrefix is a cheap string check, so we +// filter out hidden directories before paying the os.Stat cost in IsRepo. func ListDirs() (dirs []string, err error) { files, err := os.ReadDir("./") if err != nil { @@ -14,26 +18,25 @@ func ListDirs() (dirs []string, err error) { dirs = make([]string, 0, len(files)) for _, f := range files { - if f.IsDir() && IsRepo(f.Name()) && strings.Index(f.Name(), ".") != 0 { - dirs = append(dirs, f.Name()) + if !f.IsDir() { + continue } + if strings.HasPrefix(f.Name(), ".") { + continue + } + if !IsRepo(f.Name()) { + continue + } + dirs = append(dirs, f.Name()) } - return } -// IsRepo returns check result, the directory whether git repository +// IsRepo reports whether dirName contains a `.git` entry. It accepts both a +// regular repository (where `.git` is a directory) and a git worktree +// (where `.git` is a file pointing at the main repo's git directory), +// because exec'd git commands work in either layout. func IsRepo(dirName string) bool { - files, err := os.ReadDir(dirName) - if err != nil { - return false - } - - for _, f := range files { - if f.Name() == ".git" { - return true - } - } - - return false + _, err := os.Stat(filepath.Join(dirName, ".git")) + return err == nil } diff --git a/syncer/dir_test.go b/syncer/dir_test.go new file mode 100644 index 0000000..03c18b7 --- /dev/null +++ b/syncer/dir_test.go @@ -0,0 +1,64 @@ +package syncer + +import ( + "os" + "path/filepath" + "testing" + + "github.com/matryer/is" +) + +func TestIsRepo(t *testing.T) { + t.Run("regular repo with .git directory", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + dir := t.TempDir() + repo := filepath.Join(dir, "myrepo") + if err := os.MkdirAll(filepath.Join(repo, ".git"), 0o755); err != nil { + t.Fatalf("setup: %v", err) + } + + is.True(IsRepo(repo)) + }) + + t.Run("worktree with .git file", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + dir := t.TempDir() + worktree := filepath.Join(dir, "myworktree") + if err := os.MkdirAll(worktree, 0o755); err != nil { + t.Fatalf("setup: %v", err) + } + // git worktrees place a .git file (not directory) containing + // a `gitdir: ` pointer. The exact contents don't matter for + // IsRepo — only that the entry exists. + gitFile := filepath.Join(worktree, ".git") + if err := os.WriteFile(gitFile, []byte("gitdir: /tmp/main/.git/worktrees/x\n"), 0o644); err != nil { + t.Fatalf("setup: %v", err) + } + + is.True(IsRepo(worktree)) + }) + + t.Run("non-git directory returns false", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + dir := t.TempDir() + plain := filepath.Join(dir, "plain") + if err := os.MkdirAll(plain, 0o755); err != nil { + t.Fatalf("setup: %v", err) + } + + is.True(!IsRepo(plain)) + }) + + t.Run("non-existent directory returns false", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + is.True(!IsRepo(filepath.Join(t.TempDir(), "does-not-exist"))) + }) +} diff --git a/syncer/git.go b/syncer/git.go index 9a52b10..7c952e2 100644 --- a/syncer/git.go +++ b/syncer/git.go @@ -2,12 +2,26 @@ package syncer import ( "bytes" - "fmt" + "context" "io" "os" "os/exec" ) +// Executor abstracts git command execution. Defined as an interface so that +// callers can substitute fakes in tests without spawning real processes. +type Executor interface { + Git(ctx context.Context, command, dir string, args ...string) (msg, errMsg string, err error) +} + +// ExistGit reports whether the `git` executable is available on PATH. +// It returns the error from exec.LookPath when not found, and nil otherwise. +// Unlike the previous Gitter.IsExist, this function has no side effects. +func ExistGit() error { + _, err := exec.LookPath("git") + return err +} + // Gitter is struct type Gitter struct { writer io.Writer @@ -22,26 +36,14 @@ func NewGitter(writer, errWriter io.Writer) *Gitter { } } -// IsExist is check git command -func (*Gitter) IsExist() error { - s, err := exec.LookPath("git") - if err != nil { - return err - } - _, err = fmt.Fprintf(os.Stdout, "%s", s) - if err != nil { - return err - } - return nil -} - -// Git is execute git command -func (g *Gitter) Git(command, dir string, args ...string) (msg, errMsg string, err error) { +// Git is execute git command. The given context is forwarded to exec.CommandContext +// so that cancellation (e.g. per-repo timeout) terminates the underlying process. +func (g *Gitter) Git(ctx context.Context, command, dir string, args ...string) (msg, errMsg string, err error) { wr := new(bytes.Buffer) errWr := new(bytes.Buffer) cmdArgs := append([]string{command}, args...) - cmd := exec.Command("git", cmdArgs...) + cmd := exec.CommandContext(ctx, "git", cmdArgs...) cmd.Dir = dir cmd.Stdin = os.Stdin cmd.Stdout = wr diff --git a/syncer/git_test.go b/syncer/git_test.go index 518d19a..22e7bbd 100644 --- a/syncer/git_test.go +++ b/syncer/git_test.go @@ -1,27 +1,70 @@ package syncer import ( + "context" + "io" "os" + "os/exec" + "strings" "testing" "github.com/matryer/is" ) -func TestIsExist(t *testing.T) { +func TestExistGit_NoStdoutSideEffect(t *testing.T) { + // Cannot use t.Parallel(): mutates global os.Stdout. is := is.New(t) - gi := NewGitter(os.Stdout, os.Stderr) - err := gi.IsExist() + r, w, err := os.Pipe() + is.NoErr(err) + orig := os.Stdout + os.Stdout = w + defer func() { os.Stdout = orig }() + + err = ExistGit() is.NoErr(err) + + if cerr := w.Close(); cerr != nil { + t.Fatalf("close pipe writer: %v", cerr) + } + out, rerr := io.ReadAll(r) + is.NoErr(rerr) + is.Equal(string(out), "") // ExistGit must not write anything to stdout } func TestFetch(t *testing.T) { is := is.New(t) + // bare repo serves as the "remote". Fetching from it never reaches the + // network, so this test is hermetic and CI-stable. + remote := t.TempDir() + runGit(t, remote, "init", "--bare", "-q") + + // working repo: register the bare repo as origin and push one commit so + // there is something for the fetch to discover. + work := t.TempDir() + runGit(t, work, "init", "-q") + runGit(t, work, "config", "user.email", "test@example.invalid") + runGit(t, work, "config", "user.name", "tester") + runGit(t, work, "config", "commit.gpgsign", "false") + runGit(t, work, "commit", "--allow-empty", "-m", "init", "-q") + runGit(t, work, "remote", "add", "origin", remote) + runGit(t, work, "push", "-q", "origin", "HEAD:refs/heads/main") + gi := NewGitter(os.Stdout, os.Stderr) args := []string{"--all", "-p"} - _, _, err := gi.Git("fetch", ".", args...) + _, _, err := gi.Git(context.Background(), "fetch", work, args...) is.NoErr(err) + + // err==nil alone would let a no-op fetch pass. Verify the fetch actually + // populated the remote-tracking ref by resolving refs/remotes/origin/main. + cmd := exec.Command("git", "-C", work, "rev-parse", "refs/remotes/origin/main") + out, rpErr := cmd.CombinedOutput() + if rpErr != nil { + t.Fatalf("rev-parse refs/remotes/origin/main failed: %v\n%s", rpErr, out) + } + sha := strings.TrimSpace(string(out)) + is.True(sha != "") } func TestPull(t *testing.T) { @@ -30,6 +73,17 @@ func TestPull(t *testing.T) { gi := NewGitter(os.Stdout, os.Stderr) args := []string{"--verbose"} - _, _, err := gi.Git("pull", ".", args...) + _, _, err := gi.Git(context.Background(), "pull", ".", args...) is.NoErr(err) } + +// runGit is a fixture helper. Failures here mean the test setup is broken, +// not the system under test, so abort early with t.Fatalf rather than is.NoErr. +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v in %s failed: %v\n%s", args, dir, err, out) + } +} diff --git a/syncer/syncer.go b/syncer/syncer.go index 5a2dfd6..67a4679 100644 --- a/syncer/syncer.go +++ b/syncer/syncer.go @@ -3,9 +3,10 @@ package syncer import ( "context" "fmt" - "os" "path/filepath" "regexp" + "sync" + "sync/atomic" "time" "github.com/pkg/errors" @@ -13,6 +14,20 @@ import ( "golang.org/x/sync/errgroup" ) +// RunSummary reports per-category counts after a Sync.Run completes +// successfully (i.e. the run itself was not aborted by a setup error). +type RunSummary struct { + Succeeded int + Failed int + TimedOut int +} + +// HasFailures reports whether any repository failed or timed out. +// Used by callers to decide between exit code 0 (clean) and 2 (partial). +func (s *RunSummary) HasFailures() bool { + return s.Failed > 0 || s.TimedOut > 0 +} + // Sync is struct type Sync struct { // TimeOut is timeout of performed command on one directory. @@ -37,146 +52,206 @@ type Sync struct { ConNum int // Gitter is the git command executor instance. - Gitter *Gitter + Gitter Executor +} + +// failedRepo carries the repository path together with the error that caused +// the failure so that it can be reported in the final summary. +type failedRepo struct { + Repo string + Err error +} + +// runStats accumulates per-repository outcomes safely from concurrent goroutines. +type runStats struct { + mu sync.Mutex + succeeded []string + failed []failedRepo + timedOut []string +} + +func (s *runStats) addSuccess(r string) { + s.mu.Lock() + defer s.mu.Unlock() + s.succeeded = append(s.succeeded, r) +} + +func (s *runStats) addFailed(r string, e error) { + s.mu.Lock() + defer s.mu.Unlock() + s.failed = append(s.failed, failedRepo{Repo: r, Err: e}) } -// Run is execute logic -func (s *Sync) Run() (err error) { - // - // list target directories - // +func (s *runStats) addTimedOut(r string) { + s.mu.Lock() + defer s.mu.Unlock() + s.timedOut = append(s.timedOut, r) +} + +// summary builds a public RunSummary snapshot of the current stats. +func (s *runStats) summary() *RunSummary { + s.mu.Lock() + defer s.mu.Unlock() + return &RunSummary{ + Succeeded: len(s.succeeded), + Failed: len(s.failed), + TimedOut: len(s.timedOut), + } +} + +// Run discovers repositories, applies filters, and executes the configured +// git command across the matching set. It returns a summary of per-repo +// outcomes, or an error for setup failures (no repositories, invalid regex, +// invalid timeout, etc.). The returned summary is non-nil exactly when err +// is nil; callers can inspect summary.HasFailures() to decide between a +// clean run (exit 0) and a partial-failure run (exit 2). +func (s *Sync) Run() (*RunSummary, error) { dirs, err := ListDirs() if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) + return nil, errors.Wrap(err, "list directories") } if len(dirs) == 0 { - fmt.Fprintf(os.Stderr, "%s\n", "There is no repositories...") - os.Exit(1) + return nil, errors.New("no git repositories found in current directory") } fmt.Printf("repositories are found: (%d)\n", len(dirs)) - s.Writer.PrintCmd(s.Command, s.Options) - // - // compile regex patterns for performance - // + repos, err := s.filterRepos(dirs) + if err != nil { + return nil, err + } + if len(repos) == 0 { + // Filter narrowed down to zero — no work, but not an error. + s.Writer.PrintMsg("No target repositories.") + return &RunSummary{}, nil + } + s.Writer.PrintMsg(fmt.Sprintf("target repositories: (%d)", len(repos))) + + perRepoTimeout, err := time.ParseDuration(s.TimeOut) + if err != nil { + return nil, errors.Wrapf(err, "invalid timeout value: %s", s.TimeOut) + } + + stats := s.execute(context.Background(), repos, perRepoTimeout) + s.printSummary(stats) + return stats.summary(), nil +} + +// filterRepos applies the target/ignore regex patterns to the discovered +// directories. It is split out from Run() so tests can exercise the matching +// logic without scanning the filesystem. +func (s *Sync) filterRepos(dirs []string) ([]string, error) { var ignoreRegex, targetRegex *regexp.Regexp if s.IgnoreDir != "" { - ignoreRegex, err = regexp.Compile(s.IgnoreDir) + re, err := regexp.Compile(s.IgnoreDir) if err != nil { - return errors.Wrapf(err, "invalid ignore directory regex pattern: %s", s.IgnoreDir) + return nil, errors.Wrapf(err, "invalid ignore directory regex pattern: %s", s.IgnoreDir) } + ignoreRegex = re } if s.TargetDir != "" { - targetRegex, err = regexp.Compile(s.TargetDir) + re, err := regexp.Compile(s.TargetDir) if err != nil { - return errors.Wrapf(err, "invalid target directory regex pattern: %s", s.TargetDir) + return nil, errors.Wrapf(err, "invalid target directory regex pattern: %s", s.TargetDir) } + targetRegex = re } - // - // retrieve target repos - // repos := make([]string, 0, len(dirs)) for _, d := range dirs { if ignoreRegex != nil && ignoreRegex.MatchString(d) { continue } - if targetRegex != nil && !targetRegex.MatchString(d) { continue } - repos = append(repos, d) } + return repos, nil +} - if len(repos) == 0 { - s.Writer.PrintMsg("No target repositories.") - return - } - - targetRepoNum := len(repos) - s.Writer.PrintMsg(fmt.Sprintf("target repositories: (%d)", targetRepoNum)) - - // - // execute command - // - eg := errgroup.Group{} +// execute runs the git command across all repos in parallel, throttled by +// ConNum. Each invocation is bounded by perRepoTimeout via a derived context; +// the parent context is *not* timed out so a slow repo never starves the +// remaining ones. +func (s *Sync) execute(parent context.Context, repos []string, perRepoTimeout time.Duration) *runStats { + stats := &runStats{} + total := len(repos) + var done atomic.Int64 start := time.Now() - throttle := make(chan struct{}, s.ConNum) - // set up context - to, err := time.ParseDuration(s.TimeOut) - if err != nil { - fmt.Fprintf(os.Stderr, "%s\n", "invalid timeout value.") - os.Exit(1) + eg := &errgroup.Group{} + if s.ConNum > 0 { + eg.SetLimit(s.ConNum) } - ctx, cancel := context.WithTimeout(context.Background(), to) - defer cancel() - - for i := range repos { - num := i + 1 - r := repos[i] - throttle <- struct{}{} + for _, r := range repos { + r := r eg.Go(func() error { - defer func() { - <-throttle - }() + ctx, cancel := context.WithTimeout(parent, perRepoTimeout) + defer cancel() err := s.execCmd(ctx, r) - if err != nil { - s.Writer.PrintMsgErr(fmt.Sprintf("Failed: %s\n%v", r, err)) - } else { + switch { + case err == nil: + stats.addSuccess(r) s.Writer.PrintMsg(fmt.Sprintf("Success: %s\n", r)) + case errors.Is(ctx.Err(), context.DeadlineExceeded): + stats.addTimedOut(r) + s.Writer.PrintMsgErr(fmt.Sprintf("Timeout: %s", r)) + default: + stats.addFailed(r, err) + s.Writer.PrintMsgErr(fmt.Sprintf("Failed: %s\n%v", r, err)) } - s.Writer.PrintMsg(fmt.Sprintf("Done: %d/%d", num, targetRepoNum)) + n := done.Add(1) + s.Writer.PrintMsg(fmt.Sprintf("Done: %d/%d", n, total)) return nil }) } - - // Handle timeout in a separate goroutine - done := make(chan struct{}) - go func() { - defer close(done) - if err := eg.Wait(); err != nil { - s.Writer.PrintMsgErr(fmt.Sprintf("Error.exists: %v", err)) - } - }() - - select { - case <-done: - // All goroutines completed successfully - case <-ctx.Done(): - s.Writer.PrintMsgErr(fmt.Sprintf("---- Timeouted (%v) ----", time.Since(start).String())) - // returns no error - return - } + _ = eg.Wait() s.Writer.PrintMsg(fmt.Sprintf("All done. (%v)", time.Since(start).Round(time.Millisecond))) - return + return stats } // execCmd is execute git command -func (s *Sync) execCmd(ctx context.Context, d string) (err error) { +func (s *Sync) execCmd(ctx context.Context, d string) error { absPath, err := filepath.Abs(d) if err != nil { - err = errors.Wrapf(err, "get.abs.failed: %s", d) - s.Writer.Error(printer.Result{Err: err}) - return + return errors.Wrapf(err, "get.abs.failed: %s", d) } - msg, errMsg, err := s.Gitter.Git(s.Command, absPath, s.Options...) + msg, errMsg, err := s.Gitter.Git(ctx, s.Command, absPath, s.Options...) if err != nil { - s.Writer.Error(printer.Result{Repo: absPath, Err: errors.Wrapf(err, "%s", errMsg)}) - } else { - s.Writer.Print(printer.Result{Repo: absPath, Msg: msg}) + return errors.Wrapf(err, "%s", errMsg) } + s.Writer.Print(printer.Result{Repo: absPath, Msg: msg}) + return nil +} - return +// printSummary emits the post-run summary: counts, plus the list of failed +// and timed-out repositories. Reuses printer.PrintRepoErr (previously unused). +func (s *Sync) printSummary(stats *runStats) { + stats.mu.Lock() + defer stats.mu.Unlock() + + s.Writer.PrintMsg(fmt.Sprintf( + "Summary: success=%d failed=%d timeout=%d", + len(stats.succeeded), len(stats.failed), len(stats.timedOut), + )) + + if len(stats.failed) > 0 { + repos := make([]string, len(stats.failed)) + for i, f := range stats.failed { + repos[i] = f.Repo + } + s.Writer.PrintRepoErr("Failed repositories:", repos) + } + if len(stats.timedOut) > 0 { + s.Writer.PrintRepoErr("Timed out repositories:", stats.timedOut) + } } diff --git a/syncer/syncer_test.go b/syncer/syncer_test.go new file mode 100644 index 0000000..c0f0a6e --- /dev/null +++ b/syncer/syncer_test.go @@ -0,0 +1,223 @@ +package syncer + +import ( + "context" + "errors" + "io" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/matryer/is" + "github.com/takecy/git-here/printer" +) + +type execFn func(ctx context.Context, command, dir string, args ...string) (string, string, error) + +type fakeExecutor struct{ fn execFn } + +func (f *fakeExecutor) Git(ctx context.Context, command, dir string, args ...string) (string, string, error) { + return f.fn(ctx, command, dir, args...) +} + +func newSyncWithFake(fn execFn, conNum int) *Sync { + return &Sync{ + Command: "status", + ConNum: conNum, + Gitter: &fakeExecutor{fn: fn}, + Writer: printer.NewPrinter(io.Discard, io.Discard), + } +} + +func TestSync_Execute(t *testing.T) { + t.Run("all success", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := newSyncWithFake(func(_ context.Context, _, _ string, _ ...string) (string, string, error) { + return "ok", "", nil + }, 4) + + stats := s.execute(context.Background(), []string{"a", "b", "c"}, time.Second) + is.Equal(len(stats.succeeded), 3) + is.Equal(len(stats.failed), 0) + is.Equal(len(stats.timedOut), 0) + }) + + t.Run("partial failure", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := newSyncWithFake(func(_ context.Context, _, dir string, _ ...string) (string, string, error) { + if filepath.Base(dir) == "b" { + return "", "boom", errors.New("fail") + } + return "ok", "", nil + }, 4) + + stats := s.execute(context.Background(), []string{"a", "b", "c"}, time.Second) + is.Equal(len(stats.succeeded), 2) + is.Equal(len(stats.failed), 1) + is.Equal(filepath.Base(stats.failed[0].Repo), "b") + }) + + t.Run("timeout is classified separately", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := newSyncWithFake(func(ctx context.Context, _, _ string, _ ...string) (string, string, error) { + <-ctx.Done() + return "", "", ctx.Err() + }, 1) + + stats := s.execute(context.Background(), []string{"a"}, 50*time.Millisecond) + is.Equal(len(stats.timedOut), 1) + is.Equal(len(stats.failed), 0) + is.Equal(len(stats.succeeded), 0) + }) + + t.Run("respects ConNum throttle", func(t *testing.T) { + // Time-sensitive: don't run in parallel with the rest of the suite. + is := is.New(t) + + var inflight, peak atomic.Int64 + + // Atomic max via CompareAndSwap loop. The naive + // `if n > peak.Load() { peak.Store(n) }` form races and may miss + // the real peak. + recordPeak := func(n int64) { + for { + cur := peak.Load() + if n <= cur { + return + } + if peak.CompareAndSwap(cur, n) { + return + } + } + } + + s := newSyncWithFake(func(_ context.Context, _, _ string, _ ...string) (string, string, error) { + n := inflight.Add(1) + recordPeak(n) + time.Sleep(20 * time.Millisecond) + inflight.Add(-1) + return "", "", nil + }, 2) + + s.execute(context.Background(), []string{"a", "b", "c", "d", "e"}, time.Second) + + is.True(peak.Load() >= 1) + is.True(peak.Load() <= int64(s.ConNum)) + }) + + t.Run("all timeout when every repo hangs", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := newSyncWithFake(func(ctx context.Context, _, _ string, _ ...string) (string, string, error) { + <-ctx.Done() + return "", "", ctx.Err() + }, 1) + + stats := s.execute(context.Background(), []string{"a", "b", "c"}, 30*time.Millisecond) + is.Equal(len(stats.timedOut), 3) + is.Equal(len(stats.failed), 0) + is.Equal(len(stats.succeeded), 0) + }) +} + +func TestSync_FilterRepos(t *testing.T) { + t.Run("no filters returns all", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &Sync{} + got, err := s.filterRepos([]string{"a", "b", "c"}) + is.NoErr(err) + is.Equal(got, []string{"a", "b", "c"}) + }) + + t.Run("target only", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &Sync{TargetDir: "^api"} + got, err := s.filterRepos([]string{"api-foo", "web-bar", "api-baz"}) + is.NoErr(err) + is.Equal(got, []string{"api-foo", "api-baz"}) + }) + + t.Run("ignore only", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &Sync{IgnoreDir: "^test"} + got, err := s.filterRepos([]string{"test-a", "prod-b", "test-c"}) + is.NoErr(err) + is.Equal(got, []string{"prod-b"}) + }) + + t.Run("target and ignore combined", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &Sync{TargetDir: "service$", IgnoreDir: "deprecated"} + got, err := s.filterRepos([]string{"user-service", "deprecated-service", "web-app"}) + is.NoErr(err) + is.Equal(got, []string{"user-service"}) + }) + + t.Run("invalid target regex returns error", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &Sync{TargetDir: "[invalid("} + _, err := s.filterRepos([]string{"a"}) + is.True(err != nil) + }) + + t.Run("invalid ignore regex returns error", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &Sync{IgnoreDir: "[invalid("} + _, err := s.filterRepos([]string{"a"}) + is.True(err != nil) + }) +} + +func TestRunSummary_HasFailures(t *testing.T) { + t.Run("clean run returns false", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &RunSummary{Succeeded: 5, Failed: 0, TimedOut: 0} + is.Equal(s.HasFailures(), false) + }) + + t.Run("any failure returns true", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &RunSummary{Succeeded: 4, Failed: 1, TimedOut: 0} + is.Equal(s.HasFailures(), true) + }) + + t.Run("any timeout returns true", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &RunSummary{Succeeded: 4, Failed: 0, TimedOut: 1} + is.Equal(s.HasFailures(), true) + }) + + t.Run("empty summary returns false (no work done)", func(t *testing.T) { + t.Parallel() + is := is.New(t) + + s := &RunSummary{} + is.Equal(s.HasFailures(), false) + }) +}