From b03449f3d1ebb0fc116f3446ea749f3bbc69f8a9 Mon Sep 17 00:00:00 2001 From: Rob Stolarz Date: Fri, 14 Aug 2026 10:33:21 -0700 Subject: [PATCH] ci migrate: stop rewriting .github/ references to .depot/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `depot ci migrate` copied `.github/actions/` into `.depot/actions/` and then rewrote every `.github/(actions|workflows)` reference it could find — in the workflow YAML, in YAML comments, and in the copied action sources — to point at `.depot/`. The rewrite is redundant. Depot resolves a local `.github/` reference wherever it leads: the IR converter maps `.github/` to `.depot/` when it resolves an action path, and accepts either prefix when resolving a reusable workflow call. So the migrated file runs identically whether or not the paths were edited. It is also actively harmful. Only `.github/actions` and the selected workflows are copied, so a rewritten reference to anything that stayed behind — a helper script, a config file, a workflow the user chose not to migrate — dangled after migration. Rewriting a `sparse-checkout` entry stopped git materializing `.github/` at all. And a rewrite fires on any textual match, so a regex in an action's source that deliberately matched the repo's real `.github` directory silently stopped matching. Dropping the pass removes all three failure modes and leaves the migrated workflow reading like the file the author wrote. The copy into `.depot/actions/` stays: the compile-time checkout is sparse to `.depot/`, so the action directories do have to exist there. Refs DEP-5812, DEP-5306. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/ci/transform/transform.go | 259 +--------- pkg/ci/transform/transform_test.go | 765 ++--------------------------- pkg/cmd/ci/migrate.go | 30 +- 3 files changed, 55 insertions(+), 999 deletions(-) diff --git a/pkg/ci/transform/transform.go b/pkg/ci/transform/transform.go index fe779afd..e98cc44f 100644 --- a/pkg/ci/transform/transform.go +++ b/pkg/ci/transform/transform.go @@ -3,10 +3,6 @@ package transform import ( "bytes" "fmt" - "net/http" - "os" - "path/filepath" - "regexp" "strings" "github.com/depot/cli/pkg/ci/compat" @@ -14,11 +10,6 @@ import ( "gopkg.in/yaml.v3" ) -func isBinary(data []byte) bool { - contentType := http.DetectContentType(data) - return !strings.HasPrefix(contentType, "text/") -} - // ChangeType categorizes a transformation change. type ChangeType int @@ -26,7 +17,6 @@ const ( ChangeRunsOn ChangeType = iota // runs-on label was remapped ChangeTriggerRemoved // Unsupported trigger was removed ChangeJobDisabled // Entire job was commented out - ChangePathRewritten // .github/ path was rewritten to .depot/ ) // ChangeRecord describes a single change made during transformation. @@ -46,10 +36,12 @@ type TransformResult struct { // TransformWorkflow applies Depot CI migration transformations to a workflow. // It uses the parsed WorkflowFile for structural info and the CompatibilityReport // to identify issues, then transforms the raw YAML bytes. -// migratedWorkflows is a set of workflow relative paths (e.g., "ci.yml") that were -// selected for migration. When non-nil, only references to these workflows are rewritten. -// When nil, all .github/workflows/ references are rewritten. Actions are always rewritten. -func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.CompatibilityReport, migratedWorkflows map[string]bool) (*TransformResult, error) { +// +// Local `.github/` references — `uses: ./.github/actions/foo`, reusable workflow +// calls, script paths in `run:` blocks — are deliberately left as written. Depot +// resolves them wherever they lead, so rewriting them to `.depot/` would only add +// churn to the migrated file and break references to anything that stayed behind. +func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.CompatibilityReport) (*TransformResult, error) { var doc yaml.Node if err := yaml.Unmarshal(raw, &doc); err != nil { return nil, fmt.Errorf("failed to parse YAML: %w", err) @@ -77,11 +69,7 @@ func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.Comp runsOnChanges := transformRunsOn(root, disabledJobs) changes = append(changes, runsOnChanges...) - // 4. Rewrite .github/ path references to .depot/ - pathChanges := transformGitHubPaths(root, migratedWorkflows) - changes = append(changes, pathChanges...) - - // 5. Marshal the node tree back to bytes + // 4. Marshal the node tree back to bytes var buf bytes.Buffer enc := yaml.NewEncoder(&buf) enc.SetIndent(2) @@ -92,7 +80,7 @@ func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.Comp output := buf.Bytes() - // 6. Post-process: comment out disabled jobs in text + // 5. Post-process: comment out disabled jobs in text if len(disabledJobs) > 0 { var disableChanges []ChangeRecord output, disableChanges = commentOutDisabledJobs(output, disabledJobs) @@ -107,7 +95,7 @@ func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.Comp } } - // 7. Prepend header comment + // 6. Prepend header comment header := buildHeaderComment(wf, changes) output = append([]byte(header), output...) @@ -325,228 +313,6 @@ func transformRunsOnNode(node *yaml.Node, jobName string) []ChangeRecord { return changes } -// transformGitHubPaths walks all nodes and rewrites local .github/ references to .depot/ -// in both scalar values and YAML comments (HeadComment, LineComment, FootComment). -// Remote references like org/repo/.github/workflows/reusable.yml@ref are left untouched. -func transformGitHubPaths(node *yaml.Node, migratedWorkflows map[string]bool) []ChangeRecord { - rewrote := false - rewrite := func(s string) string { - result, changed := rewriteGitHubPaths(s, migratedWorkflows) - if changed { - rewrote = true - } - return result - } - walkNodes(node, func(n *yaml.Node) { - if n.Kind == yaml.ScalarNode { - n.Value = rewrite(n.Value) - } - if n.HeadComment != "" { - n.HeadComment = rewrite(n.HeadComment) - } - if n.LineComment != "" { - n.LineComment = rewrite(n.LineComment) - } - if n.FootComment != "" { - n.FootComment = rewrite(n.FootComment) - } - }) - if !rewrote { - return nil - } - return []ChangeRecord{{ - Type: ChangePathRewritten, - Detail: "Rewrote .github/ path references to .depot/", - }} -} - -var ( - // githubPathRe matches .github/actions or .github/workflows references. - githubPathRe = regexp.MustCompile(`\.github/(actions|workflows)`) - - // remoteRefRe matches owner/repo/.github/(actions|workflows) patterns. - // The char class [a-zA-Z0-9_.-] naturally excludes expression characters ($, {, }), - // so expression-expanded paths like "${{ workspace }}/.github/" won't match. - remoteRefRe = regexp.MustCompile(`[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/\.github/(?:actions|workflows)`) - - // pathTailRe captures a file path segment after a / delimiter, stopping at - // whitespace or shell metacharacters. - pathTailRe = regexp.MustCompile(`^[^\s"'();|&=]+`) -) - -// rewriteGitHubPaths replaces local .github/{actions,workflows} references with -// .depot/ equivalents in a single pass. Remote repo references (org/repo/.github/...), -// URLs, and non-migrated .github/ files (dependabot.yml, CODEOWNERS, etc.) are preserved. -// -// migratedWorkflows controls workflow filtering: when non-nil, only references to -// workflows in the set are rewritten. When nil, all workflow references are rewritten. -// Actions are always rewritten (the entire .github/actions/ directory is copied). -func rewriteGitHubPaths(s string, migratedWorkflows map[string]bool) (string, bool) { - candidates := githubPathRe.FindAllStringSubmatchIndex(s, -1) - if len(candidates) == 0 { - return s, false - } - - remoteSpans := remoteRefRe.FindAllStringIndex(s, -1) - - var b strings.Builder - last := 0 - changed := false - - for _, m := range candidates { - start, end := m[0], m[1] - subdir := s[m[2]:m[3]] - - if !shouldRewrite(s, start, end, subdir, migratedWorkflows, remoteSpans) { - continue - } - - b.WriteString(s[last:start]) - b.WriteString(".depot/" + subdir) - last = end - changed = true - } - - if !changed { - return s, false - } - b.WriteString(s[last:]) - return b.String(), true -} - -// boundaryChars are characters that can validly precede ".github/" as a path reference. -// Prevents matching inside longer names like "myapp.github/actions". -const boundaryChars = "/ \t\n\"'();|&=" - -// shouldRewrite decides whether a .github/(actions|workflows) match at [start:end] -// should be replaced with .depot/. -func shouldRewrite(s string, start, end int, subdir string, migratedWorkflows map[string]bool, remoteSpans [][]int) bool { - if start > 0 && !strings.ContainsRune(boundaryChars, rune(s[start-1])) { - return false - } - - // Must not continue into a longer dir name (e.g., ".github/actions-custom") - if end < len(s) && isPathChar(s[end]) { - return false - } - - // Skip matches inside URLs. This is a procedural backward scan rather than a - // regex pre-pass because URL patterns overlap heavily with the paths we want to - // match, and a backward scan for "://" is simpler than managing overlapping spans. - if isURL(s, start) { - return false - } - - // Skip remote repo refs (owner/repo/.github/...) detected by remoteRefRe. - // Deep filesystem paths (/home/.../repo/.github/) are distinguished by checking - // whether the match is preceded by /, which indicates a deeper path, not a remote ref. - for _, span := range remoteSpans { - if start >= span[0] && start < span[1] { - if span[0] == 0 || s[span[0]-1] != '/' { - return false - } - } - } - - // Partial migration: for workflows, only rewrite references to selected files. - // Bare directory refs (e.g., "ls .github/workflows") are skipped when filtering - // is active since the directory still partially lives at .github/. - if subdir == "workflows" && migratedWorkflows != nil { - if end >= len(s) || s[end] != '/' { - return false - } - tail := pathTailRe.FindString(s[end+1:]) - if tail == "" || !migratedWorkflows[tail] { - return false - } - } - - return true -} - -// isPathChar returns true for characters that can continue a directory name, -// distinguishing ".github/actions/..." from ".github/actions-custom/...". -func isPathChar(b byte) bool { - return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || - (b >= '0' && b <= '9') || b == '-' || b == '_' || b == '.' -} - -// isURL scans backward from idx to check if the .github/ match appears inside a -// URL (contains "://" before the match within the same token). This is procedural -// rather than a regex pre-pass because URL patterns overlap with the paths we're -// rewriting, and a backward scan for "://" is simpler than managing span overlaps. -func isURL(s string, idx int) bool { - for i := idx - 1; i >= 0; i-- { - c := s[i] - if c == ' ' || c == '\t' || c == '\n' || c == '"' || c == '\'' || - c == ';' || c == '|' || c == '&' || c == '(' || c == ')' || c == '=' { - return false - } - if i >= 2 && s[i-2:i+1] == "://" { - return true - } - } - return false -} - -// walkNodes recursively visits all nodes in a YAML tree. -func walkNodes(node *yaml.Node, fn func(*yaml.Node)) { - if node == nil { - return - } - fn(node) - for _, child := range node.Content { - walkNodes(child, fn) - } -} - -// RewriteGitHubPathsInDir walks a directory and rewrites .github/ → .depot/ references -// in all text files. Binary files and symlinks are skipped. Original file permissions -// are preserved. This is used for copied action files that aren't processed through -// the full YAML transform pipeline. -// -// migratedWorkflows controls workflow path filtering — pass nil to rewrite all -// .github/workflows/ references (full migration), or a set of relative filenames -// (e.g., {"ci.yml": true}) to only rewrite references to those specific workflows. -// Actions (.github/actions/) are always rewritten regardless of this parameter. -func RewriteGitHubPathsInDir(dir string, migratedWorkflows map[string]bool) (int, error) { - rewritten := 0 - err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - if d.IsDir() || d.Type()&os.ModeSymlink != 0 { - return nil - } - - info, err := d.Info() - if err != nil { - return fmt.Errorf("failed to stat %s: %w", path, err) - } - - raw, err := os.ReadFile(path) - if err != nil { - return fmt.Errorf("failed to read %s: %w", path, err) - } - - if isBinary(raw) { - return nil - } - - result, changed := rewriteGitHubPaths(string(raw), migratedWorkflows) - if !changed { - return nil - } - - if err := os.WriteFile(path, []byte(result), info.Mode().Perm()); err != nil { - return fmt.Errorf("failed to write %s: %w", path, err) - } - rewritten++ - return nil - }) - return rewritten, err -} - // commentOutDisabledJobs does a text-level pass to comment out entire job blocks. func commentOutDisabledJobs(content []byte, disabledJobs map[string]disabledJobInfo) ([]byte, []ChangeRecord) { if len(disabledJobs) == 0 { @@ -652,7 +418,6 @@ func summarizeChanges(changes []ChangeRecord) []string { var runsOnOrder []runsOnKey var lines []string - hasPathRewrite := false for _, c := range changes { switch c.Type { case ChangeRunsOn: @@ -662,17 +427,11 @@ func summarizeChanges(changes []ChangeRecord) []string { runsOnOrder = append(runsOnOrder, key) } runsOnCounts[key]++ - case ChangePathRewritten: - hasPathRewrite = true default: lines = append(lines, c.Detail) } } - if hasPathRewrite { - lines = append(lines, "Rewrote .github/ path references to .depot/") - } - // Check if all runs-on changes are standard GitHub → Depot mappings allStandard := true for _, key := range runsOnOrder { diff --git a/pkg/ci/transform/transform_test.go b/pkg/ci/transform/transform_test.go index 250e53ff..bd849a41 100644 --- a/pkg/ci/transform/transform_test.go +++ b/pkg/ci/transform/transform_test.go @@ -1,9 +1,6 @@ package transform import ( - "bytes" - "os" - "path/filepath" "strings" "testing" @@ -31,7 +28,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -70,7 +67,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -114,7 +111,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -151,7 +148,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -181,7 +178,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -222,7 +219,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -261,7 +258,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -307,7 +304,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -355,7 +352,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -405,7 +402,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -447,7 +444,7 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -463,265 +460,27 @@ jobs: } } -func TestTransformWorkflow_RewritesLocalActionPaths(t *testing.T) { +// Local .github/ references are resolved by Depot wherever they lead, so migrate +// leaves every form of them exactly as written — action refs, reusable workflow +// calls, sparse-checkout inputs, script paths, and paths inside comments. +func TestTransformWorkflow_PreservesGitHubPaths(t *testing.T) { raw := []byte(`name: CI +# Helpers live in .github/actions/ on: push jobs: build: runs-on: depot-ubuntu-latest steps: - uses: actions/checkout@v4 + with: + sparse-checkout: | + .github/actions + .github/workflows - uses: ./.github/actions/setup-node - - uses: ./.github/actions/deploy -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - result, err := TransformWorkflow(raw, wf, report, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - if strings.Contains(content, "./.github/actions/") { - t.Errorf("expected ./.github/actions/ to be rewritten, got:\n%s", content) - } - if !strings.Contains(content, "./.depot/actions/setup-node") { - t.Errorf("expected ./.depot/actions/setup-node, got:\n%s", content) - } - if !strings.Contains(content, "./.depot/actions/deploy") { - t.Errorf("expected ./.depot/actions/deploy, got:\n%s", content) - } - // Remote actions should be untouched - if !strings.Contains(content, "actions/checkout@v4") { - t.Errorf("expected remote action unchanged, got:\n%s", content) - } - - pathRewriteCount := 0 - for _, c := range result.Changes { - if c.Type == ChangePathRewritten { - pathRewriteCount++ - } - } - if pathRewriteCount != 1 { - t.Errorf("expected exactly 1 ChangePathRewritten (deduplicated), got %d", pathRewriteCount) - } -} - -func TestTransformWorkflow_RewritesBareGitHubPaths(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: .github/actions/my-action - - uses: .github/workflows/reusable.yml -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - result, err := TransformWorkflow(raw, wf, report, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - if !strings.Contains(content, ".depot/actions/my-action") { - t.Errorf("expected .depot/actions/my-action, got:\n%s", content) - } - if !strings.Contains(content, ".depot/workflows/reusable.yml") { - t.Errorf("expected .depot/workflows/reusable.yml, got:\n%s", content) - } -} - -func TestTransformWorkflow_PreservesNonMigratedGitHubPaths(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: actions/checkout@v4 + - uses: .github/actions/deploy + - uses: ./.github/workflows/reusable.yml + - run: .github/workflows/scripts/build.sh # see .github/actions/deploy - run: cat .github/dependabot.yml - - run: cp .github/config.json dist/ - - run: ls .github/ISSUE_TEMPLATE/ - - uses: ./.github/actions/setup -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - result, err := TransformWorkflow(raw, wf, report, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - // Non-migrated paths should be preserved - if !strings.Contains(content, ".github/dependabot.yml") { - t.Errorf("expected .github/dependabot.yml preserved, got:\n%s", content) - } - if !strings.Contains(content, ".github/config.json") { - t.Errorf("expected .github/config.json preserved, got:\n%s", content) - } - if !strings.Contains(content, ".github/ISSUE_TEMPLATE/") { - t.Errorf("expected .github/ISSUE_TEMPLATE/ preserved, got:\n%s", content) - } - // Migrated path should be rewritten - if !strings.Contains(content, "./.depot/actions/setup") { - t.Errorf("expected ./.depot/actions/setup, got:\n%s", content) - } -} - -func TestTransformWorkflow_SkipsRemoteRepoGitHubPaths(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: org/repo/.github/workflows/reusable.yml@main - - uses: ./.github/actions/local-action -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - result, err := TransformWorkflow(raw, wf, report, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - // Remote reference should be untouched - if !strings.Contains(content, "org/repo/.github/workflows/reusable.yml@main") { - t.Errorf("expected remote repo reference unchanged, got:\n%s", content) - } - // Local reference should be rewritten - if !strings.Contains(content, "./.depot/actions/local-action") { - t.Errorf("expected local action rewritten, got:\n%s", content) - } -} - -func TestTransformWorkflow_PathRewriteInHeader(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: ./.github/actions/setup -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - result, err := TransformWorkflow(raw, wf, report, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - if !strings.Contains(content, "Rewrote .github/ path references to .depot/") { - t.Errorf("expected path rewrite note in header, got:\n%s", content) - } - // Source line in header should still reference original .github/ path - if !strings.Contains(content, "# Source: .github/workflows/ci.yml") { - t.Errorf("expected original source path in header, got:\n%s", content) - } -} - -func TestTransformWorkflow_RewritesYAMLComments(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: actions/checkout@v4 - # Run the setup from .github/actions/setup - - uses: ./.github/actions/setup # local action at .github/actions/setup -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - result, err := TransformWorkflow(raw, wf, report, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - // Head comment should be rewritten - if strings.Contains(content, "# Run the setup from .github/actions/setup") { - t.Errorf("expected head comment .github/ path rewritten, got:\n%s", content) - } - if !strings.Contains(content, ".depot/actions/setup") { - t.Errorf("expected .depot/actions/setup in comments, got:\n%s", content) - } - // Line comment should be rewritten - if strings.Contains(content, "# local action at .github/actions/setup") { - t.Errorf("expected line comment .github/ path rewritten, got:\n%s", content) - } -} - -func TestTransformWorkflow_ExpressionExpandedPaths(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: actions/checkout@v4 - - run: ${{ github.workspace }}/.github/actions/setup/run.sh - - run: $GITHUB_WORKSPACE/.github/actions/build/compile.sh `) wf := &migrate.WorkflowFile{ @@ -734,471 +493,29 @@ jobs: } report := compat.AnalyzeWorkflow(wf) - result, err := TransformWorkflow(raw, wf, report, nil) + result, err := TransformWorkflow(raw, wf, report) if err != nil { t.Fatalf("unexpected error: %v", err) } content := string(result.Content) - if !strings.Contains(content, "}}/.depot/actions/setup/run.sh") { - t.Errorf("expected expression-expanded path rewritten, got:\n%s", content) - } - if !strings.Contains(content, "WORKSPACE/.depot/actions/build/compile.sh") { - t.Errorf("expected env var path rewritten, got:\n%s", content) - } -} - -func TestTransformWorkflow_PartialMigration(t *testing.T) { - raw := []byte(`name: CI -on: push -jobs: - build: - runs-on: depot-ubuntu-latest - steps: - - uses: ./.github/workflows/reusable-build.yml - - uses: ./.github/workflows/reusable-deploy.yml - - uses: ./.github/actions/setup -`) - - wf := &migrate.WorkflowFile{ - Path: ".github/workflows/ci.yml", - Name: "CI", - Triggers: []string{"push"}, - Jobs: []migrate.JobInfo{ - {Name: "build", RunsOn: "depot-ubuntu-latest"}, - }, - } - report := compat.AnalyzeWorkflow(wf) - - // Only reusable-build.yml was migrated, not reusable-deploy.yml - migratedWorkflows := map[string]bool{ - "ci.yml": true, - "reusable-build.yml": true, - } - - result, err := TransformWorkflow(raw, wf, report, migratedWorkflows) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - content := string(result.Content) - // Migrated workflow should be rewritten - if !strings.Contains(content, "./.depot/workflows/reusable-build.yml") { - t.Errorf("expected migrated workflow rewritten, got:\n%s", content) - } - // Non-migrated workflow should be preserved - if !strings.Contains(content, "./.github/workflows/reusable-deploy.yml") { - t.Errorf("expected non-migrated workflow preserved, got:\n%s", content) - } - // Actions are always rewritten regardless of workflow filtering - if !strings.Contains(content, "./.depot/actions/setup") { - t.Errorf("expected action always rewritten, got:\n%s", content) - } -} - -func TestRewriteGitHubPaths(t *testing.T) { - tests := []struct { - name string - input string - want string - changed bool - }{ - { - name: "explicit local action path", - input: "./.github/actions/setup-node", - want: "./.depot/actions/setup-node", - changed: true, - }, - { - name: "explicit local workflow path", - input: "./.github/workflows/reusable.yml", - want: "./.depot/workflows/reusable.yml", - changed: true, - }, - { - name: "bare local action path", - input: ".github/actions/setup", - want: ".depot/actions/setup", - changed: true, - }, - { - name: "bare local workflow path", - input: ".github/workflows/helper.yml", - want: ".depot/workflows/helper.yml", - changed: true, - }, - { - name: "non-migrated github file preserved", - input: "cat .github/dependabot.yml", - want: "cat .github/dependabot.yml", - changed: false, - }, - { - name: "non-migrated github directory preserved", - input: "ls .github/ISSUE_TEMPLATE/", - want: "ls .github/ISSUE_TEMPLATE/", - changed: false, - }, - { - name: "non-migrated config preserved", - input: "cp .github/config.json dist/", - want: "cp .github/config.json dist/", - changed: false, - }, - { - name: "remote repo reference preserved", - input: "org/repo/.github/workflows/reusable.yml@main", - want: "org/repo/.github/workflows/reusable.yml@main", - changed: false, - }, - { - name: "remote repo action preserved", - input: "org/repo/.github/actions/shared@v2", - want: "org/repo/.github/actions/shared@v2", - changed: false, - }, - { - name: "no github reference", - input: "actions/checkout@v4", - want: "actions/checkout@v4", - changed: false, - }, - { - name: "multiple local references", - input: "./.github/actions/a and ./.github/actions/b", - want: "./.depot/actions/a and ./.depot/actions/b", - changed: true, - }, - { - name: "mixed local and remote", - input: "./.github/actions/local org/repo/.github/workflows/remote.yml@v1", - want: "./.depot/actions/local org/repo/.github/workflows/remote.yml@v1", - changed: true, - }, - { - name: "mixed migrated and non-migrated", - input: "./.github/actions/setup && cat .github/config.json", - want: "./.depot/actions/setup && cat .github/config.json", - changed: true, - }, - { - name: "bare actions directory no trailing slash", - input: "ls .github/actions", - want: "ls .depot/actions", - changed: true, - }, - { - name: "bare workflows directory no trailing slash", - input: "ls .github/workflows", - want: "ls .depot/workflows", - changed: true, - }, - { - name: "explicit bare directory no trailing slash", - input: "ls ./.github/actions", - want: "ls ./.depot/actions", - changed: true, - }, - { - name: "similar directory name not rewritten", - input: ".github/actions-custom/setup.sh", - want: ".github/actions-custom/setup.sh", - changed: false, - }, - { - name: "similar directory name with dot prefix not rewritten", - input: "./.github/actions-custom/setup.sh", - want: "./.github/actions-custom/setup.sh", - changed: false, - }, - { - name: "expression-expanded path rewritten", - input: "${{ github.workspace }}/.github/actions/setup/run.sh", - want: "${{ github.workspace }}/.depot/actions/setup/run.sh", - changed: true, - }, - { - name: "env var path rewritten", - input: "$GITHUB_WORKSPACE/.github/actions/build/compile.sh", - want: "$GITHUB_WORKSPACE/.depot/actions/build/compile.sh", - changed: true, - }, - { - name: "absolute path rewritten", - input: "/home/runner/work/repo/repo/.github/workflows/ci.yml", - want: "/home/runner/work/repo/repo/.depot/workflows/ci.yml", - changed: true, - }, - { - name: "part of longer name not rewritten", - input: "myapp.github/actions/setup", - want: "myapp.github/actions/setup", - changed: false, - }, - { - name: "CODEOWNERS preserved", - input: "cat .github/CODEOWNERS", - want: "cat .github/CODEOWNERS", - changed: false, - }, - { - name: "FUNDING.yml preserved", - input: ".github/FUNDING.yml", - want: ".github/FUNDING.yml", - changed: false, - }, - { - name: "github URL preserved", - input: "https://github.com/org/repo/tree/main/.github/actions/setup", - want: "https://github.com/org/repo/tree/main/.github/actions/setup", - changed: false, - }, - { - name: "github URL with workflows preserved", - input: "see https://github.com/org/repo/blob/main/.github/workflows/ci.yml for details", - want: "see https://github.com/org/repo/blob/main/.github/workflows/ci.yml for details", - changed: false, - }, - { - name: "URL before shell delimiter does not block rewrite", - input: "curl https://api.com/status;.github/actions/setup/run.sh", - want: "curl https://api.com/status;.depot/actions/setup/run.sh", - changed: true, - }, - { - name: "subshell boundary rewritten", - input: "$(.github/actions/setup/version.sh)", - want: "$(.depot/actions/setup/version.sh)", - changed: true, - }, - { - name: "assignment boundary rewritten", - input: "CONFIG=.github/actions/setup/config.json", - want: "CONFIG=.depot/actions/setup/config.json", - changed: true, - }, - { - name: "pipe boundary rewritten", - input: "cat file|.github/actions/setup/run.sh", - want: "cat file|.depot/actions/setup/run.sh", - changed: true, - }, - { - name: "semicolon boundary rewritten", - input: "cd repo;.github/actions/setup/run.sh", - want: "cd repo;.depot/actions/setup/run.sh", - changed: true, - }, - { - name: "parenthesized remote ref preserved", - input: "(org/repo/.github/actions/setup)", - want: "(org/repo/.github/actions/setup)", - changed: false, - }, - { - name: "parenthesized local ref rewritten", - input: "(.github/actions/setup/run.sh)", - want: "(.depot/actions/setup/run.sh)", - changed: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, changed := rewriteGitHubPaths(tt.input, nil) - if got != tt.want { - t.Errorf("rewriteGitHubPaths(%q) = %q, want %q", tt.input, got, tt.want) - } - if changed != tt.changed { - t.Errorf("rewriteGitHubPaths(%q) changed = %v, want %v", tt.input, changed, tt.changed) - } - }) - } -} - -func TestRewriteGitHubPaths_WorkflowFiltering(t *testing.T) { - migrated := map[string]bool{"ci.yml": true, "build.yml": true} - - tests := []struct { - name string - input string - want string - changed bool - }{ - { - name: "migrated workflow rewritten", - input: "./.github/workflows/ci.yml", - want: "./.depot/workflows/ci.yml", - changed: true, - }, - { - name: "non-migrated workflow preserved", - input: "./.github/workflows/deploy.yml", - want: "./.github/workflows/deploy.yml", - changed: false, - }, - { - name: "bare workflows dir preserved when filtering", - input: "ls .github/workflows", - want: "ls .github/workflows", - changed: false, - }, - { - name: "actions always rewritten regardless", - input: "./.github/actions/setup", - want: "./.depot/actions/setup", - changed: true, - }, - { - name: "semicolon-terminated migrated workflow rewritten", - input: "source .github/workflows/ci.yml;echo done", - want: "source .depot/workflows/ci.yml;echo done", - changed: true, - }, - { - name: "pipe-terminated migrated workflow rewritten", - input: "cat .github/workflows/ci.yml|grep foo", - want: "cat .depot/workflows/ci.yml|grep foo", - changed: true, - }, - { - name: "ampersand-terminated migrated workflow rewritten", - input: "cat .github/workflows/build.yml&&echo ok", - want: "cat .depot/workflows/build.yml&&echo ok", - changed: true, - }, - { - name: "semicolon-terminated non-migrated workflow preserved", - input: "cat .github/workflows/deploy.yml;echo done", - want: "cat .github/workflows/deploy.yml;echo done", - changed: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, changed := rewriteGitHubPaths(tt.input, migrated) - if got != tt.want { - t.Errorf("rewriteGitHubPaths(%q) = %q, want %q", tt.input, got, tt.want) - } - if changed != tt.changed { - t.Errorf("rewriteGitHubPaths(%q) changed = %v, want %v", tt.input, changed, tt.changed) - } - }) - } -} - -func TestRewriteGitHubPathsInDir(t *testing.T) { - dir := t.TempDir() - - // Composite action with local .github/ reference - actionDir := filepath.Join(dir, "setup") - if err := os.MkdirAll(actionDir, 0755); err != nil { - t.Fatal(err) - } - actionYAML := `name: Setup -description: Setup the project -runs: - using: composite - steps: - - uses: ./.github/actions/install-deps - - run: echo "done" -` - if err := os.WriteFile(filepath.Join(actionDir, "action.yml"), []byte(actionYAML), 0644); err != nil { - t.Fatal(err) - } - - // Shell script with execute bit referencing migrated and non-migrated .github/ paths - script := `#!/bin/bash -cp .github/actions/shared/config.sh . -cat .github/dependabot.yml -` - scriptPath := filepath.Join(actionDir, "setup.sh") - if err := os.WriteFile(scriptPath, []byte(script), 0755); err != nil { - t.Fatal(err) - } - - // Binary file with .github/actions bytes — should be skipped - binaryContent := []byte("some\x00binary\x00.github/actions/setup data") - if err := os.WriteFile(filepath.Join(actionDir, "tool.wasm"), binaryContent, 0644); err != nil { - t.Fatal(err) - } - - // File with no .github/ references (should be untouched) - clean := `name: Clean Action -runs: - using: node20 - main: index.js -` - cleanDir := filepath.Join(dir, "clean") - if err := os.MkdirAll(cleanDir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(cleanDir, "action.yml"), []byte(clean), 0644); err != nil { - t.Fatal(err) - } - - rewritten, err := RewriteGitHubPathsInDir(dir, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if rewritten != 2 { - t.Errorf("expected 2 files rewritten, got %d", rewritten) - } - - // Verify action.yml was rewritten - content, _ := os.ReadFile(filepath.Join(actionDir, "action.yml")) - if strings.Contains(string(content), "./.github/actions/") { - t.Errorf("expected .github/actions reference rewritten in action.yml, got:\n%s", content) - } - if !strings.Contains(string(content), "./.depot/actions/install-deps") { - t.Errorf("expected .depot/actions reference in action.yml, got:\n%s", content) - } - - // Verify shell script: migrated path rewritten, non-migrated path preserved - scriptContent, _ := os.ReadFile(scriptPath) - if !strings.Contains(string(scriptContent), ".depot/actions/shared/config.sh") { - t.Errorf("expected .github/actions/ rewritten in setup.sh, got:\n%s", scriptContent) - } - if !strings.Contains(string(scriptContent), ".github/dependabot.yml") { - t.Errorf("expected .github/dependabot.yml preserved in setup.sh, got:\n%s", scriptContent) - } - - // Verify script preserved its execute permission - scriptInfo, err := os.Stat(scriptPath) - if err != nil { - t.Fatal(err) - } - if scriptInfo.Mode().Perm()&0111 == 0 { - t.Errorf("expected setup.sh to retain execute permission, got %v", scriptInfo.Mode().Perm()) - } - - // Verify binary file was not modified - binaryResult, _ := os.ReadFile(filepath.Join(actionDir, "tool.wasm")) - if !bytes.Equal(binaryResult, binaryContent) { - t.Errorf("expected binary file to be untouched") - } - - // Verify clean file was not touched - cleanContent, _ := os.ReadFile(filepath.Join(cleanDir, "action.yml")) - if string(cleanContent) != clean { - t.Errorf("expected clean file unchanged, got:\n%s", cleanContent) - } - - // Verify symlinks are skipped: create a symlink and confirm it's not followed - symlinkTarget := filepath.Join(actionDir, "action.yml") - symlinkPath := filepath.Join(cleanDir, "link.yml") - if err := os.Symlink(symlinkTarget, symlinkPath); err != nil { - t.Fatal(err) - } - // Re-run after adding symlink — count should stay at 0 since action.yml is already rewritten - rewritten2, err := RewriteGitHubPathsInDir(dir, nil) - if err != nil { - t.Fatalf("unexpected error on second pass: %v", err) - } - if rewritten2 != 0 { - t.Errorf("expected 0 files rewritten on second pass (symlink should be skipped), got %d", rewritten2) + if strings.Contains(content, ".depot/") { + t.Errorf("expected no .depot/ rewrites, got:\n%s", content) + } + for _, want := range []string{ + "./.github/actions/setup-node", + ".github/actions/deploy", + "./.github/workflows/reusable.yml", + ".github/workflows/scripts/build.sh", + ".github/dependabot.yml", + ".github/actions\n", + ".github/workflows\n", + "# Helpers live in .github/actions/", + "# see .github/actions/deploy", + } { + if !strings.Contains(content, want) { + t.Errorf("expected %q preserved, got:\n%s", want, content) + } } } diff --git a/pkg/cmd/ci/migrate.go b/pkg/cmd/ci/migrate.go index c3a68d85..41eedd42 100644 --- a/pkg/cmd/ci/migrate.go +++ b/pkg/cmd/ci/migrate.go @@ -531,34 +531,14 @@ func workflows(opts migrateOptions) error { return fmt.Errorf("failed to inspect .depot directory: %w", err) } - // Copy .github/actions/ to .depot/actions/ + // Copy .github/actions/ to .depot/actions/. Depot's compile-time checkout is + // sparse to .depot/, so the actions have to exist there even though the + // workflow references still point at .github/actions/ — those are resolved + // wherever they lead, so they're left exactly as the author wrote them. if _, err := migrate.CopyGitHubToDepot(workDir, []string{"actions"}, copyMode); err != nil { return fmt.Errorf("failed to copy GitHub CI files: %w", err) } - // Build set of migrated workflow relative paths for selective rewriting. - // When all workflows are selected, pass nil so all .github/workflows/ references - // (including bare directory refs) are rewritten. - var migratedWorkflows map[string]bool - if len(selectedWorkflows) < len(workflows) { - migratedWorkflows = make(map[string]bool, len(selectedWorkflows)) - for _, wf := range selectedWorkflows { - relPath, err := filepath.Rel(workflowsDir, wf.Path) - if err != nil { - return fmt.Errorf("failed to resolve relative path for %s: %w", wf.Path, err) - } - migratedWorkflows[filepath.ToSlash(relPath)] = true - } - } - - // Rewrite .github/ references in copied action files - depotActionsDir := filepath.Join(depotDir, "actions") - if info, err := os.Stat(depotActionsDir); err == nil && info.IsDir() { - if _, err := transform.RewriteGitHubPathsInDir(depotActionsDir, migratedWorkflows); err != nil { - return fmt.Errorf("failed to rewrite paths in action files: %w", err) - } - } - // Transform and write each workflow depotWorkflowsDir := filepath.Join(depotDir, "workflows") if err := os.MkdirAll(depotWorkflowsDir, 0755); err != nil { @@ -579,7 +559,7 @@ func workflows(opts migrateOptions) error { } report := compat.AnalyzeWorkflow(wf) - result, err := transform.TransformWorkflow(raw, wf, report, migratedWorkflows) + result, err := transform.TransformWorkflow(raw, wf, report) if err != nil { return fmt.Errorf("failed to transform %s: %w", filepath.Base(wf.Path), err) }