From 4f99c91f4527bf7334e4682358c3e4393f727fcc Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 11:55:39 -0700 Subject: [PATCH 01/13] just a test... --- gomonorepo/git_commands.go | 31 +++++++++++++++++++++++++++---- gomonorepo/module_changes.go | 4 ++-- justfile | 4 ++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index 31b3582..a47df8a 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -67,7 +67,12 @@ func listChangedFiles(ctx context.Context, parent string) ([]string, error) { } // getCurrentBranch returns the current branch name. -func getCurrentBranch(ctx context.Context) (string, error) { +func getCurrentBranch(ctx context.Context) (remote string, branch string, err error) { + // git rev-parse --verify HEAD <-- prints out the current hash + // What about unstaged? + // git rev-parse --abbrev-ref --symbolic-full-name @{upstream} <-- prints out the upstream branch OR fatals with "fatal: no upstream configured ..." + // + stdout, done := GetBuffer() defer done(stdout) stderr, done := GetBuffer() @@ -75,9 +80,27 @@ func getCurrentBranch(ctx context.Context) (string, error) { cmd := exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "HEAD") cmd.Stdout = stdout cmd.Stderr = stderr - err := cmd.Run() + err = cmd.Run() if err != nil { - return "", fmt.Errorf("failed to run git diff: %w\n%s", err, stderr.String()) + return "", "", fmt.Errorf("failed to get branch name: %w\n%s", err, stderr.String()) } - return strings.TrimSpace(stdout.String()), nil + branch = strings.TrimSpace(stdout.String()) + + stdout.Reset() + stderr.Reset() + + cmd = exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}") + cmd.Stdout = stdout + cmd.Stderr = stderr + err = cmd.Run() + if err != nil { + return "", "", fmt.Errorf("failed to get upstream: %w\n%s", err, stderr.String()) + } + + remote = strings.TrimSpace(stdout.String()) + if strings.HasPrefix(remote, "fatal: no") { + remote = "" + } + + return remote, branch, nil } diff --git a/gomonorepo/module_changes.go b/gomonorepo/module_changes.go index eab4a14..ce7ccce 100644 --- a/gomonorepo/module_changes.go +++ b/gomonorepo/module_changes.go @@ -38,11 +38,11 @@ func listAllChangedModulesWithTree( return nil, err } if len(changedFiles) == 0 { - current, err := getCurrentBranch(ctx) + remote, current, err := getCurrentBranch(ctx) if err != nil { return nil, err } - if current == parentCommit { + if current == parentCommit || remote+"/"+current == parentCommit { opts.Infof("Currently on %q with no changes; will run command on all %d moduels.\n", current, len(tree.AllModules)) diff --git a/justfile b/justfile index 4362582..3c1a668 100755 --- a/justfile +++ b/justfile @@ -13,6 +13,10 @@ tidy: _tools-monorepo test: _tools-monorepo gomonorepo test --parent=origin/main --invocationDir={{ CURRENT_DIR }} +# Runs `go test --race ` for all modules in the current directory. +test-g: _tools-monorepo + gomonorepo test --parent=gs/compare-branch-correctly --invocationDir={{ CURRENT_DIR }} + # Runs lint and test for all modules in the current directory. check: lint test From 419deaa63ddb114fea86abd296acc5e01c157059 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 11:58:34 -0700 Subject: [PATCH 02/13] humm --- gomonorepo/git_commands.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index a47df8a..52841a8 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -94,13 +94,9 @@ func getCurrentBranch(ctx context.Context) (remote string, branch string, err er cmd.Stderr = stderr err = cmd.Run() if err != nil { - return "", "", fmt.Errorf("failed to get upstream: %w\n%s", err, stderr.String()) + // noop. there is no remote. + return "", branch, nil } - - remote = strings.TrimSpace(stdout.String()) - if strings.HasPrefix(remote, "fatal: no") { - remote = "" - } - + remote = strings.TrimSuffix(strings.TrimSpace(stdout.String()), "/"+branch) return remote, branch, nil } From 67ec8df5d815da726cac7a26a59009e5407021bb Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:00:31 -0700 Subject: [PATCH 03/13] i think this fixes it --- gomonorepo/git_commands.go | 8 +++----- justfile | 4 ---- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index 52841a8..8c48104 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -68,15 +68,12 @@ func listChangedFiles(ctx context.Context, parent string) ([]string, error) { // getCurrentBranch returns the current branch name. func getCurrentBranch(ctx context.Context) (remote string, branch string, err error) { - // git rev-parse --verify HEAD <-- prints out the current hash - // What about unstaged? - // git rev-parse --abbrev-ref --symbolic-full-name @{upstream} <-- prints out the upstream branch OR fatals with "fatal: no upstream configured ..." - // - stdout, done := GetBuffer() defer done(stdout) stderr, done := GetBuffer() defer done(stderr) + + // Get the current branch / revision name excluding the remote: cmd := exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "HEAD") cmd.Stdout = stdout cmd.Stderr = stderr @@ -89,6 +86,7 @@ func getCurrentBranch(ctx context.Context) (remote string, branch string, err er stdout.Reset() stderr.Reset() + // Get the branch name AND its upstream remote (e.g. origin/main): cmd = exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}") cmd.Stdout = stdout cmd.Stderr = stderr diff --git a/justfile b/justfile index 3c1a668..4362582 100755 --- a/justfile +++ b/justfile @@ -13,10 +13,6 @@ tidy: _tools-monorepo test: _tools-monorepo gomonorepo test --parent=origin/main --invocationDir={{ CURRENT_DIR }} -# Runs `go test --race ` for all modules in the current directory. -test-g: _tools-monorepo - gomonorepo test --parent=gs/compare-branch-correctly --invocationDir={{ CURRENT_DIR }} - # Runs lint and test for all modules in the current directory. check: lint test From 868f480c990f9988e98403e4033046633a1d5e93 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:21:58 -0700 Subject: [PATCH 04/13] lets actually error out of the code properly --- gomonorepo/cmd/gomonorepo/main.go | 1 + gomonorepo/git_commands.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/gomonorepo/cmd/gomonorepo/main.go b/gomonorepo/cmd/gomonorepo/main.go index ee0f2be..5d08073 100644 --- a/gomonorepo/cmd/gomonorepo/main.go +++ b/gomonorepo/cmd/gomonorepo/main.go @@ -51,5 +51,6 @@ func main() { parser.WriteHelp(os.Stdout) } else if err != nil { fmt.Fprint(os.Stderr, err.Error()) + os.Exit(1) } } diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index 8c48104..d69fdc7 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -24,7 +24,7 @@ func getIgnoredDirectories( cmd.Stderr = stderr err := cmd.Run() if err != nil { - return nil, fmt.Errorf("failed to run git diff: %w\n%s", err, stderr.String()) + return nil, fmt.Errorf("failed to get ignored files: %w\n%s", err, stderr.String()) } result := make([]string, 0, 8) From daa1ae902fb1530ebe8630383a37c0a36d30a854 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:22:53 -0700 Subject: [PATCH 05/13] using proper logger now.. --- gomonorepo/cmd/gomonorepo/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gomonorepo/cmd/gomonorepo/main.go b/gomonorepo/cmd/gomonorepo/main.go index 5d08073..7f0cf84 100644 --- a/gomonorepo/cmd/gomonorepo/main.go +++ b/gomonorepo/cmd/gomonorepo/main.go @@ -2,7 +2,6 @@ package main import ( "context" - "fmt" "os" "os/signal" "syscall" @@ -50,7 +49,7 @@ func main() { if fErr, ok := err.(*flags.Error); ok && fErr.Type == flags.ErrHelp { parser.WriteHelp(os.Stdout) } else if err != nil { - fmt.Fprint(os.Stderr, err.Error()) + opts.Errorf("Fatal: %w", err) os.Exit(1) } } From 07762689846ed43c06e3c4c7c7d6b8f407970c2f Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:23:29 -0700 Subject: [PATCH 06/13] dumb --- gomonorepo/cmd/gomonorepo/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/gomonorepo/cmd/gomonorepo/main.go b/gomonorepo/cmd/gomonorepo/main.go index 7f0cf84..4cae892 100644 --- a/gomonorepo/cmd/gomonorepo/main.go +++ b/gomonorepo/cmd/gomonorepo/main.go @@ -50,6 +50,5 @@ func main() { parser.WriteHelp(os.Stdout) } else if err != nil { opts.Errorf("Fatal: %w", err) - os.Exit(1) } } From 55fbea7826f6100a369dc938b93044a2f95787ca Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:26:34 -0700 Subject: [PATCH 07/13] another attempt at error formtting --- gomonorepo/cmd/gomonorepo/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gomonorepo/cmd/gomonorepo/main.go b/gomonorepo/cmd/gomonorepo/main.go index 4cae892..5a545f2 100644 --- a/gomonorepo/cmd/gomonorepo/main.go +++ b/gomonorepo/cmd/gomonorepo/main.go @@ -49,6 +49,6 @@ func main() { if fErr, ok := err.(*flags.Error); ok && fErr.Type == flags.ErrHelp { parser.WriteHelp(os.Stdout) } else if err != nil { - opts.Errorf("Fatal: %w", err) + opts.Errorf("Fatal: %s", err.Error()) } } From 6bbbe81b4edd84e9d558eed29d62f3d20464e7f9 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:34:20 -0700 Subject: [PATCH 08/13] is this a git action issue? --- .github/workflows/pr_validation.workflow.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/pr_validation.workflow.yaml b/.github/workflows/pr_validation.workflow.yaml index 80e0082..81679bf 100644 --- a/.github/workflows/pr_validation.workflow.yaml +++ b/.github/workflows/pr_validation.workflow.yaml @@ -14,6 +14,8 @@ jobs: contents: read steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - id: changes uses: ./.github/actions/detect-changes - if: steps.changes.outputs.go == 'true' From d98f58241fc875e75c9830734195fe2bdda9bcbb Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:37:15 -0700 Subject: [PATCH 09/13] see what happesn --- gomonorepo/git_commands.go | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index d69fdc7..aebb9b2 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -51,6 +51,11 @@ func listChangedFiles(ctx context.Context, parent string) ([]string, error) { cmd.Stderr = stderr err := cmd.Run() if err != nil { + logRemotes(ctx) + // if !patched && strings.HasPrefix(stderr.String(), "fatal: ambiguous argument '"+parent+"'") { + // + // return nil, nil + // } return nil, fmt.Errorf("failed to run git diff: %w\n%s", err, stderr.String()) } @@ -66,6 +71,49 @@ func listChangedFiles(ctx context.Context, parent string) ([]string, error) { return result, nil } +// FIXME: DELETE THIS IF NOT USED +func tryFetchParentRevision(ctx context.Context, parent string) error { + stdout, done := GetBuffer() + defer done(stdout) + stderr, done := GetBuffer() + defer done(stderr) + + _, branch, err := getCurrentBranch(ctx) + if err != nil { + return err + } + // git rev-list --count origin/main..$(git branch --show-current) + cmd := exec.CommandContext(ctx, "git", "rev-list", "--count", parent+".."+branch) + cmd.Stdout = stdout + cmd.Stderr = stderr + err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) + } + return nil +} + +// FIXME: DELETE THIS IF NOT USED +func logRemotes(ctx context.Context) error { + stdout, done := GetBuffer() + defer done(stdout) + stderr, done := GetBuffer() + defer done(stderr) + + cmd := exec.CommandContext(ctx, "git", "remote", "-v") + cmd.Stdout = stdout + cmd.Stderr = stderr + err := cmd.Run() + if err != nil { + return fmt.Errorf("failed to fetch remote revision: %w\n%s", err, stderr.String()) + } + + println("===============") + println(stdout.String()) + println("===============") + return nil +} + // getCurrentBranch returns the current branch name. func getCurrentBranch(ctx context.Context) (remote string, branch string, err error) { stdout, done := GetBuffer() From 26ac265214ed0d34fc8352e7d4e93e48f46c93c4 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:40:54 -0700 Subject: [PATCH 10/13] try this --- gomonorepo/git_commands.go | 44 ++++++++---------------------------- gomonorepo/module_changes.go | 2 +- 2 files changed, 11 insertions(+), 35 deletions(-) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index aebb9b2..409fd2b 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -41,7 +41,7 @@ func getIgnoredDirectories( return result, nil } -func listChangedFiles(ctx context.Context, parent string) ([]string, error) { +func listChangedFiles(ctx context.Context, parent string, patched bool) ([]string, error) { stdout, done := GetBuffer() defer done(stdout) stderr, done := GetBuffer() @@ -51,11 +51,13 @@ func listChangedFiles(ctx context.Context, parent string) ([]string, error) { cmd.Stderr = stderr err := cmd.Run() if err != nil { - logRemotes(ctx) - // if !patched && strings.HasPrefix(stderr.String(), "fatal: ambiguous argument '"+parent+"'") { - // - // return nil, nil - // } + if !patched && strings.HasPrefix(stderr.String(), "fatal: ambiguous argument '"+parent+"'") { + err = tryFetchParentRevision(ctx, parent) + if err != nil { + return nil, err + } + return listChangedFiles(ctx, parent, true) + } return nil, fmt.Errorf("failed to run git diff: %w\n%s", err, stderr.String()) } @@ -71,46 +73,20 @@ func listChangedFiles(ctx context.Context, parent string) ([]string, error) { return result, nil } -// FIXME: DELETE THIS IF NOT USED func tryFetchParentRevision(ctx context.Context, parent string) error { stdout, done := GetBuffer() defer done(stdout) stderr, done := GetBuffer() defer done(stderr) - _, branch, err := getCurrentBranch(ctx) - if err != nil { - return err - } // git rev-list --count origin/main..$(git branch --show-current) - cmd := exec.CommandContext(ctx, "git", "rev-list", "--count", parent+".."+branch) - cmd.Stdout = stdout - cmd.Stderr = stderr - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) - } - return nil -} - -// FIXME: DELETE THIS IF NOT USED -func logRemotes(ctx context.Context) error { - stdout, done := GetBuffer() - defer done(stdout) - stderr, done := GetBuffer() - defer done(stderr) - - cmd := exec.CommandContext(ctx, "git", "remote", "-v") + cmd := exec.CommandContext(ctx, "git", "fetch", parent) cmd.Stdout = stdout cmd.Stderr = stderr err := cmd.Run() if err != nil { - return fmt.Errorf("failed to fetch remote revision: %w\n%s", err, stderr.String()) + return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) } - - println("===============") - println(stdout.String()) - println("===============") return nil } diff --git a/gomonorepo/module_changes.go b/gomonorepo/module_changes.go index ce7ccce..ee84c33 100644 --- a/gomonorepo/module_changes.go +++ b/gomonorepo/module_changes.go @@ -33,7 +33,7 @@ func listAllChangedModulesWithTree( return set.Make(tree.AllModules...), nil } - changedFiles, err := listChangedFiles(ctx, parentCommit) + changedFiles, err := listChangedFiles(ctx, parentCommit, false) if err != nil { return nil, err } From 22968577825277fd6de56c605cb5183a39d87367 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 12:50:50 -0700 Subject: [PATCH 11/13] dobut it --- gomonorepo/git_commands.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index 409fd2b..8d59598 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -84,6 +84,20 @@ func tryFetchParentRevision(ctx context.Context, parent string) error { cmd.Stdout = stdout cmd.Stderr = stderr err := cmd.Run() + if err == nil { + return nil + } + remote, branch, ok := strings.Cut(parent, "/") + if !ok { + return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) + } + + stdout.Reset() + stderr.Reset() + cmd = exec.CommandContext(ctx, "git", "fetch", remote, branch) + cmd.Stdout = stdout + cmd.Stderr = stderr + err = cmd.Run() if err != nil { return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) } From 7e5de891817e5c10289371a572535459c86c57e6 Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 13:00:51 -0700 Subject: [PATCH 12/13] try againnnn --- gomonorepo/git_commands.go | 51 +++++++++++++++++++++++++----------- gomonorepo/module_changes.go | 2 +- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/gomonorepo/git_commands.go b/gomonorepo/git_commands.go index 8d59598..54079ea 100644 --- a/gomonorepo/git_commands.go +++ b/gomonorepo/git_commands.go @@ -41,7 +41,7 @@ func getIgnoredDirectories( return result, nil } -func listChangedFiles(ctx context.Context, parent string, patched bool) ([]string, error) { +func listChangedFiles(ctx context.Context, opts *AppOptions, parent string, patched bool) ([]string, error) { stdout, done := GetBuffer() defer done(stdout) stderr, done := GetBuffer() @@ -52,11 +52,12 @@ func listChangedFiles(ctx context.Context, parent string, patched bool) ([]strin err := cmd.Run() if err != nil { if !patched && strings.HasPrefix(stderr.String(), "fatal: ambiguous argument '"+parent+"'") { + opts.Infof("Parent %q not found locally, attempting to fetch it from remote.\n", parent) err = tryFetchParentRevision(ctx, parent) if err != nil { return nil, err } - return listChangedFiles(ctx, parent, true) + return listChangedFiles(ctx, opts, parent, true) } return nil, fmt.Errorf("failed to run git diff: %w\n%s", err, stderr.String()) } @@ -74,34 +75,52 @@ func listChangedFiles(ctx context.Context, parent string, patched bool) ([]strin } func tryFetchParentRevision(ctx context.Context, parent string) error { + remote, err := getConfiguredRemoteName(ctx) + if err != nil { + return err + } + branch := parent + if strings.HasPrefix(parent, remote) { + branch = strings.TrimPrefix(parent, remote+"/") + } + stdout, done := GetBuffer() defer done(stdout) stderr, done := GetBuffer() defer done(stderr) - // git rev-list --count origin/main..$(git branch --show-current) - cmd := exec.CommandContext(ctx, "git", "fetch", parent) + cmd := exec.CommandContext(ctx, "git", "fetch", remote, branch) cmd.Stdout = stdout cmd.Stderr = stderr - err := cmd.Run() - if err == nil { - return nil - } - remote, branch, ok := strings.Cut(parent, "/") - if !ok { + err = cmd.Run() + if err != nil { return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) } + return nil +} - stdout.Reset() - stderr.Reset() - cmd = exec.CommandContext(ctx, "git", "fetch", remote, branch) +// getConfiguredRemoteName returns the first remote name configured in the git repository. +func getConfiguredRemoteName(ctx context.Context) (string, error) { + stdout, done := GetBuffer() + defer done(stdout) + stderr, done := GetBuffer() + defer done(stderr) + + cmd := exec.CommandContext(ctx, "git", "remote") cmd.Stdout = stdout cmd.Stderr = stderr - err = cmd.Run() + err := cmd.Run() if err != nil { - return fmt.Errorf("failed to fetch parent revision: %w\n%s", err, stderr.String()) + return "", fmt.Errorf("failed to get remote name: %w\n%s", err, stderr.String()) } - return nil + + // Get the first remote name + remotes := strings.Split(stdout.String(), "\n") + if len(remotes) == 0 { + return "", fmt.Errorf("no remotes found") + } + + return remotes[0], nil } // getCurrentBranch returns the current branch name. diff --git a/gomonorepo/module_changes.go b/gomonorepo/module_changes.go index ee84c33..97f7894 100644 --- a/gomonorepo/module_changes.go +++ b/gomonorepo/module_changes.go @@ -33,7 +33,7 @@ func listAllChangedModulesWithTree( return set.Make(tree.AllModules...), nil } - changedFiles, err := listChangedFiles(ctx, parentCommit, false) + changedFiles, err := listChangedFiles(ctx, opts, parentCommit, false) if err != nil { return nil, err } From e104d214d81ef5662ad1724a0b2948cd17defc7f Mon Sep 17 00:00:00 2001 From: Gavin Shriver Date: Tue, 6 May 2025 13:02:20 -0700 Subject: [PATCH 13/13] and if i revert fetch depth? --- .github/workflows/pr_validation.workflow.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/pr_validation.workflow.yaml b/.github/workflows/pr_validation.workflow.yaml index 81679bf..80e0082 100644 --- a/.github/workflows/pr_validation.workflow.yaml +++ b/.github/workflows/pr_validation.workflow.yaml @@ -14,8 +14,6 @@ jobs: contents: read steps: - uses: actions/checkout@v4 - with: - fetch-depth: 0 - id: changes uses: ./.github/actions/detect-changes - if: steps.changes.outputs.go == 'true'