diff --git a/pkg/ci/transform/preserve.go b/pkg/ci/transform/preserve.go new file mode 100644 index 00000000..65df82c9 --- /dev/null +++ b/pkg/ci/transform/preserve.go @@ -0,0 +1,599 @@ +package transform + +import ( + "fmt" + "strings" + + "github.com/depot/cli/pkg/ci/compat" + "github.com/depot/cli/pkg/ci/migrate" + "gopkg.in/yaml.v3" +) + +// transformInPlace applies changes to the original YAML and reports whether all +// required edits were safely expressible as source edits. +func transformInPlace(s *source, root *yaml.Node, disabledJobs map[string]disabledJobInfo) ([]byte, []ChangeRecord, bool) { + var edits []edit + var changes []ChangeRecord + + for _, plan := range []func(*source, *yaml.Node, map[string]disabledJobInfo) ([]edit, []ChangeRecord, bool){ + planTriggerEdits, + planRunsOnEdits, + planDisabledJobEdits, + } { + planEdits, planChanges, ok := plan(s, root, disabledJobs) + if !ok { + return nil, nil, false + } + edits = append(edits, planEdits...) + changes = append(changes, planChanges...) + } + + out, err := applyEdits(s.text, edits) + if err != nil { + return nil, nil, false + } + + // Splicing text cannot be trusted on inspection alone: if the result no + // longer parses, some extent was wrong and re-encoding is the safe answer. + var check yaml.Node + if err := yaml.Unmarshal([]byte(out), &check); err != nil { + return nil, nil, false + } + + return []byte(out), changes, true +} + +// documentEnd returns the exclusive line bound for the document. +func documentEnd(s *source) int { + return s.lineCount() + 1 +} + +// findMappingEntry finds a mapping pair and its index. +func findMappingEntry(mapping *yaml.Node, key string) (pairIndex int, keyNode, valNode *yaml.Node) { + if mapping.Kind != yaml.MappingNode { + return -1, nil, nil + } + for i := 0; i+1 < len(mapping.Content); i += 2 { + k := mapping.Content[i] + if k.Kind == yaml.ScalarNode && k.Value == key { + return i / 2, k, mapping.Content[i+1] + } + } + return -1, nil, nil +} + +// startLineOf finds an entry's line, including same-indent comments above it. +func startLineOf(s *source, line, indent int) (int, bool) { + got, hasContent := s.indentOf(line) + if !hasContent || got != indent { + return 0, false + } + for line > 1 && s.isCommentLine(line-1, indent) { + line-- + } + return line, true +} + +// boundAfter finds the first line after an entry, preserving separating blanks. +func boundAfter(s *source, next *yaml.Node, nextIndent int, firstLine, outerBound int) int { + bound := outerBound + if next != nil { + bound = next.Line + for bound > 1 && s.isCommentLine(bound-1, nextIndent) { + bound-- + } + } + for bound-1 > firstLine { + if _, hasContent := s.indentOf(bound - 1); hasContent { + break + } + bound-- + } + return bound +} + +// mappingEntryRange returns the source line range for a mapping entry. +func mappingEntryRange(s *source, mapping *yaml.Node, pairIndex, outerBound int) (first, bound int, ok bool) { + key := mapping.Content[2*pairIndex] + indent := key.Column - 1 + first, ok = startLineOf(s, key.Line, indent) + if !ok { + return 0, 0, false + } + var next *yaml.Node + if 2*(pairIndex+1) < len(mapping.Content) { + next = mapping.Content[2*(pairIndex+1)] + } + return first, boundAfter(s, next, indent, first, outerBound), true +} + +// sequenceItemRange returns the source line range for a block sequence item. +func sequenceItemRange(s *source, seq *yaml.Node, index, outerBound int) (first, bound int, ok bool) { + item := seq.Content[index] + indent, hasContent := s.indentOf(item.Line) + if !hasContent { + return 0, 0, false + } + first, ok = startLineOf(s, item.Line, indent) + if !ok { + return 0, 0, false + } + var next *yaml.Node + if index+1 < len(seq.Content) { + next = seq.Content[index+1] + } + return first, boundAfter(s, next, indent, first, outerBound), true +} + +// deleteEntryLines removes an entry without leaving an accidental double gap. +func deleteEntryLines(s *source, first, bound int) (edit, bool) { + if first > 1 && bound <= s.lineCount() { + _, afterHasContent := s.indentOf(bound) + _, beforeHasContent := s.indentOf(first - 1) + if !afterHasContent && !beforeHasContent { + first-- + } + } + return deleteLines(s, first, bound) +} + +// deleteLines returns an edit removing the given line range. +func deleteLines(s *source, first, bound int) (edit, bool) { + start, ok := s.lineStart(first) + if !ok { + return edit{}, false + } + end, ok := s.lineStart(bound) + if !ok { + return edit{}, false + } + if end < start { + return edit{}, false + } + return edit{start: start, end: end, text: ""}, true +} + +// planTriggerEdits removes unsupported triggers and records the reason. +func planTriggerEdits(s *source, root *yaml.Node, _ map[string]disabledJobInfo) ([]edit, []ChangeRecord, bool) { + pairIndex, onKey, onVal := findMappingEntry(root, "on") + if onKey == nil { + // yaml.v3 may decode bare `on` as boolean true + pairIndex, onKey, onVal = findMappingEntry(root, "true") + } + if onKey == nil || onVal == nil { + return nil, nil, true + } + + var edits []edit + var changes []ChangeRecord + var notes []string + + removed := func(trigger string) { + rule := compat.TriggerRules[trigger] + notes = append(notes, fmt.Sprintf("Removed unsupported trigger: %s. %s", trigger, rule.Note)) + changes = append(changes, ChangeRecord{ + Type: ChangeTriggerRemoved, + Detail: fmt.Sprintf("Removed unsupported trigger %q", trigger), + }) + } + + onBound := boundAfter(s, nextSibling(root, pairIndex), onKey.Column-1, onKey.Line, documentEnd(s)) + + switch onVal.Kind { + case yaml.ScalarNode: + if !isUnsupportedTrigger(onVal.Value) { + return nil, nil, true + } + start, ok := s.nodeOffset(onVal) + if !ok { + return nil, nil, false + } + _, end, ok := scalarExtent(s.text, onVal, start, false) + if !ok { + return nil, nil, false + } + edits = append(edits, edit{start: start, end: end, text: "{}"}) + removed(onVal.Value) + + case yaml.SequenceNode: + var drop []int + for i, item := range onVal.Content { + if item.Kind == yaml.ScalarNode && isUnsupportedTrigger(item.Value) { + drop = append(drop, i) + } + } + if len(drop) == 0 { + return nil, nil, true + } + + if onVal.Style&yaml.FlowStyle != 0 { + // Reuse kept source tokens so their quoting survives. + e, ok := rebuildFlowSequence(s, onVal, drop) + if !ok { + return nil, nil, false + } + edits = append(edits, e) + } else if len(drop) == len(onVal.Content) { + collapse, ok := replaceValueWithEmptyMap(s, onKey, onVal, onBound) + if !ok { + return nil, nil, false + } + edits = append(edits, collapse...) + } else { + for _, i := range drop { + first, bound, ok := sequenceItemRange(s, onVal, i, onBound) + if !ok { + return nil, nil, false + } + e, ok := deleteEntryLines(s, first, bound) + if !ok { + return nil, nil, false + } + edits = append(edits, e) + } + } + for _, i := range drop { + removed(onVal.Content[i].Value) + } + + case yaml.MappingNode: + if onVal.Style&yaml.FlowStyle != 0 { + // Re-encoding handles flow mappings safely. + for i := 0; i+1 < len(onVal.Content); i += 2 { + if k := onVal.Content[i]; k.Kind == yaml.ScalarNode && isUnsupportedTrigger(k.Value) { + return nil, nil, false + } + } + return nil, nil, true + } + + var drop []int + pairs := len(onVal.Content) / 2 + for p := 0; p < pairs; p++ { + if k := onVal.Content[2*p]; k.Kind == yaml.ScalarNode && isUnsupportedTrigger(k.Value) { + drop = append(drop, p) + } + } + if len(drop) == 0 { + return nil, nil, true + } + + if len(drop) == pairs { + collapse, ok := replaceValueWithEmptyMap(s, onKey, onVal, onBound) + if !ok { + return nil, nil, false + } + edits = append(edits, collapse...) + } else { + for _, p := range drop { + first, bound, ok := mappingEntryRange(s, onVal, p, onBound) + if !ok { + return nil, nil, false + } + e, ok := deleteEntryLines(s, first, bound) + if !ok { + return nil, nil, false + } + edits = append(edits, e) + } + } + for _, p := range drop { + removed(onVal.Content[2*p].Value) + } + + default: + return nil, nil, true + } + + if len(notes) > 0 { + e, ok := insertCommentsAbove(s, onKey.Line, onKey.Column-1, notes) + if !ok { + return nil, nil, false + } + edits = append(edits, e) + } + + return edits, changes, true +} + +// nextSibling returns the key node after pairIndex. +func nextSibling(mapping *yaml.Node, pairIndex int) *yaml.Node { + if pairIndex < 0 || 2*(pairIndex+1) >= len(mapping.Content) { + return nil + } + return mapping.Content[2*(pairIndex+1)] +} + +// rebuildFlowSequence replaces a flow sequence while preserving kept tokens. +func rebuildFlowSequence(s *source, seq *yaml.Node, drop []int) (edit, bool) { + start, ok := s.nodeOffset(seq) + if !ok || start >= len(s.text) || s.text[start] != '[' { + return edit{}, false + } + + dropped := make(map[int]bool, len(drop)) + for _, i := range drop { + dropped[i] = true + } + + var kept []string + lastEnd := start + 1 + for i, item := range seq.Content { + itemStart, ok := s.nodeOffset(item) + if !ok { + return edit{}, false + } + _, itemEnd, ok := scalarExtent(s.text, item, itemStart, true) + if !ok { + return edit{}, false + } + lastEnd = itemEnd + if !dropped[i] { + kept = append(kept, s.text[itemStart:itemEnd]) + } + } + + closing := strings.IndexByte(s.text[lastEnd:], ']') + if closing < 0 { + return edit{}, false + } + end := lastEnd + closing + 1 + + text := "{}" + if len(kept) > 0 { + text = "[" + strings.Join(kept, ", ") + "]" + } + return edit{start: start, end: end, text: text}, true +} + +// replaceValueWithEmptyMap collapses a block value to `{}` while preserving the +// key line's trailing comment. +func replaceValueWithEmptyMap(s *source, key, val *yaml.Node, bound int) ([]edit, bool) { + keyStart, ok := s.nodeOffset(key) + if !ok { + return nil, false + } + _, keyEnd, ok := scalarExtent(s.text, key, keyStart, false) + if !ok { + return nil, false + } + colon := keyEnd + for colon < len(s.text) && isSpaceByte(s.text[colon]) { + colon++ + } + if colon >= len(s.text) || s.text[colon] != ':' { + return nil, false + } + + if val.Line == key.Line { + return nil, false // inline collection; re-encoding handles it + } + + del, ok := deleteLines(s, key.Line+1, bound) + if !ok { + return nil, false + } + return []edit{ + {start: colon + 1, end: colon + 1, text: " {}"}, + del, + }, true +} + +// insertCommentsAbove inserts same-indent comments immediately above a line. +func insertCommentsAbove(s *source, line, indent int, notes []string) (edit, bool) { + at, ok := s.lineStart(line) + if !ok { + return edit{}, false + } + pad := strings.Repeat(" ", indent) + var b strings.Builder + for _, note := range notes { + b.WriteString(pad) + b.WriteString("# ") + b.WriteString(note) + b.WriteString("\n") + } + return edit{start: at, end: at, text: b.String()}, true +} + +func isUnsupportedTrigger(trigger string) bool { + rule, ok := compat.TriggerRules[trigger] + return ok && rule.Supported == compat.Unsupported +} + +// planRunsOnEdits remaps runs-on labels and annotates each replacement. +func planRunsOnEdits(s *source, root *yaml.Node, disabledJobs map[string]disabledJobInfo) ([]edit, []ChangeRecord, bool) { + _, _, jobsVal := findMappingEntry(root, "jobs") + if jobsVal == nil || jobsVal.Kind != yaml.MappingNode { + return nil, nil, true + } + + var edits []edit + var changes []ChangeRecord + // Group notes by line so a flow sequence gets one trailing comment. + noteLines := make([]int, 0, 4) + notesByLine := make(map[int][]string) + tokenEndByLine := make(map[int]int) + + for i := 0; i+1 < len(jobsVal.Content); i += 2 { + jobKey := jobsVal.Content[i] + jobVal := jobsVal.Content[i+1] + if jobKey.Kind != yaml.ScalarNode || jobVal.Kind != yaml.MappingNode { + continue + } + if _, disabled := disabledJobs[jobKey.Value]; disabled { + continue + } + + _, _, runsOnVal := findMappingEntry(jobVal, "runs-on") + if runsOnVal == nil { + continue + } + + var items []*yaml.Node + flow := false + switch runsOnVal.Kind { + case yaml.ScalarNode: + items = []*yaml.Node{runsOnVal} + case yaml.SequenceNode: + items = runsOnVal.Content + flow = runsOnVal.Style&yaml.FlowStyle != 0 + default: + continue + } + + for _, item := range items { + if item.Kind != yaml.ScalarNode { + continue + } + original := item.Value + newLabel, changed, reason := migrate.MapLabel(original) + if !changed { + continue + } + + start, ok := s.nodeOffset(item) + if !ok { + return nil, nil, false + } + _, end, ok := scalarExtent(s.text, item, start, flow) + if !ok { + return nil, nil, false + } + edits = append(edits, edit{start: start, end: end, text: quoteLike(item, newLabel)}) + + if _, seen := notesByLine[item.Line]; !seen { + noteLines = append(noteLines, item.Line) + } + notesByLine[item.Line] = append(notesByLine[item.Line], fmt.Sprintf("was: %s. %s", original, reason)) + if end > tokenEndByLine[item.Line] { + tokenEndByLine[item.Line] = end + } + + changes = append(changes, ChangeRecord{ + Type: ChangeRunsOn, + JobName: jobKey.Value, + Detail: fmt.Sprintf("Changed runs-on from %q to %q in job %q", original, newLabel, jobKey.Value), + }) + } + } + + for _, line := range noteLines { + e, ok := annotateLine(s, line, tokenEndByLine[line], notesByLine[line]) + if !ok { + return nil, nil, false + } + edits = append(edits, e) + } + + return edits, changes, true +} + +// quoteLike keeps the source scalar's quoting style when safe. +func quoteLike(n *yaml.Node, value string) string { + switch { + case n.Style&yaml.DoubleQuotedStyle != 0 && !strings.ContainsAny(value, "\"\\\n"): + return `"` + value + `"` + case n.Style&yaml.SingleQuotedStyle != 0 && !strings.ContainsAny(value, "'\n"): + return "'" + value + "'" + default: + return value + } +} + +// commentSafe prevents source text from escaping a generated YAML comment. +func commentSafe(notes []string) []string { + flatten := func(r rune) rune { + if r == '\n' || r == '\r' { + return ' ' + } + return r + } + out := make([]string, len(notes)) + for i, note := range notes { + out[i] = strings.Map(flatten, note) + } + return out +} + +// annotateLine appends notes unless the line already has a comment. +func annotateLine(s *source, line, tokenEnd int, notes []string) (edit, bool) { + notes = commentSafe(notes) + if s.commentStart(line, tokenEnd) >= 0 { + indent, hasContent := s.indentOf(line) + if !hasContent { + return edit{}, false + } + return insertCommentsAbove(s, line, indent, notes) + } + at, ok := s.contentEnd(line) + if !ok { + return edit{}, false + } + return edit{start: at, end: at, text: " # " + strings.Join(notes, " ")}, true +} + +// planDisabledJobEdits comments out jobs migration cannot correct. +func planDisabledJobEdits(s *source, root *yaml.Node, disabledJobs map[string]disabledJobInfo) ([]edit, []ChangeRecord, bool) { + if len(disabledJobs) == 0 { + return nil, nil, true + } + + jobsIndex, jobsKey, jobsVal := findMappingEntry(root, "jobs") + if jobsVal == nil || jobsVal.Kind != yaml.MappingNode { + return nil, nil, false + } + jobsBound := boundAfter(s, nextSibling(root, jobsIndex), jobsKey.Column-1, jobsKey.Line, documentEnd(s)) + + var edits []edit + var changes []ChangeRecord + + pairs := len(jobsVal.Content) / 2 + for p := 0; p < pairs; p++ { + jobKey := jobsVal.Content[2*p] + if jobKey.Kind != yaml.ScalarNode { + continue + } + info, disabled := disabledJobs[jobKey.Value] + if !disabled { + continue + } + + first, bound, ok := mappingEntryRange(s, jobsVal, p, jobsBound) + if !ok { + return nil, nil, false + } + start, ok := s.lineStart(first) + if !ok { + return nil, nil, false + } + end, ok := s.lineStart(bound) + if !ok { + return nil, nil, false + } + + indent := strings.Repeat(" ", jobKey.Column-1) + var b strings.Builder + fmt.Fprintf(&b, "%s# DISABLED: %s\n", indent, info.Reason) + for line := first; line < bound; line++ { + text := s.line(line) + if strings.TrimSpace(text) == "" { + b.WriteString("\n") + continue + } + b.WriteString(indent) + b.WriteString("# ") + b.WriteString(text) + b.WriteString("\n") + } + + edits = append(edits, edit{start: start, end: end, text: b.String()}) + changes = append(changes, ChangeRecord{ + Type: ChangeJobDisabled, + JobName: jobKey.Value, + Detail: fmt.Sprintf("Disabled job %q: %s", jobKey.Value, info.Reason), + }) + } + + return edits, changes, true +} diff --git a/pkg/ci/transform/preserve_test.go b/pkg/ci/transform/preserve_test.go new file mode 100644 index 00000000..35070ad6 --- /dev/null +++ b/pkg/ci/transform/preserve_test.go @@ -0,0 +1,616 @@ +package transform + +import ( + "strings" + "testing" + + "github.com/depot/cli/pkg/ci/compat" + "github.com/depot/cli/pkg/ci/migrate" + "gopkg.in/yaml.v3" +) + +// body strips the generated header for byte-level assertions. +func body(t *testing.T, content string) string { + t.Helper() + idx := strings.Index(content, "\n\n") + if idx < 0 { + t.Fatalf("no header found in:\n%s", content) + } + return content[idx+2:] +} + +func runsOnNote(t *testing.T, label string) (string, string) { + t.Helper() + newLabel, changed, reason := migrate.MapLabel(label) + if !changed { + t.Fatalf("expected MapLabel(%q) to remap", label) + } + return newLabel, "# was: " + label + ". " + reason +} + +// fidelityWorkflow includes formatting and trailing whitespace that yaml.v3 +// re-encoding loses. +var fidelityWorkflow = strings.ReplaceAll(`name: CI + +# Build and test everything. +on: + push: + branches: ['main'] + +env: + LOG_LEVEL: "debug" + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Build + run: | + set -euo pipefail + make build + + make test +`, "", " ") + +func TestTransformWorkflow_PreservesFormatting(t *testing.T) { + raw := []byte(fidelityWorkflow) + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntu-latest"}}, + } + + result, err := TransformWorkflow(raw, wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + newLabel, note := runsOnNote(t, "ubuntu-latest") + want := strings.Replace(fidelityWorkflow, + "runs-on: ubuntu-latest", + "runs-on: "+newLabel+" "+note, 1) + + got := body(t, string(result.Content)) + if got != want { + t.Errorf("transformed body is not the input with runs-on remapped\n--- want ---\n%s\n--- got ---\n%s", want, got) + } +} + +func TestTransformWorkflow_NoChangesLeavesBodyIdentical(t *testing.T) { + raw := `name: CI + +on: + push: + branches: [main] + +jobs: + build: + runs-on: depot-ubuntu-latest + + steps: + - run: | + echo one + + echo two +` + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "depot-ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(result.Changes) != 0 { + t.Fatalf("expected no changes, got %v", result.Changes) + } + if got := body(t, string(result.Content)); got != raw { + t.Errorf("body was rewritten\n--- want ---\n%s\n--- got ---\n%s", raw, got) + } +} + +func TestTransformWorkflow_TriggerRemovalKeepsBlankLines(t *testing.T) { + tests := []struct { + name string + raw string + want string + }{ + { + name: "block mapping", + raw: `name: CI + +on: + push: + branches: [main] + + release: + types: [published] + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + want: `name: CI + +on: + push: + branches: [main] + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + }, + { + name: "block sequence", + raw: `name: CI + +on: + - push + - release + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + want: `name: CI + +on: + - push + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + }, + { + name: "flow sequence keeps quoting", + raw: `name: CI + +on: ['push', release] + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + want: `name: CI + +on: ['push'] + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + }, + { + name: "every trigger unsupported", + raw: `name: CI + +on: + release: + types: [published] + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + want: `name: CI + +on: {} + +jobs: + build: + runs-on: depot-ubuntu-latest +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push", "release"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "depot-ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(tt.raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got := body(t, string(result.Content)) + note := "# Removed unsupported trigger: release. " + compat.TriggerRules["release"].Note + "\n" + want := strings.Replace(tt.want, "on:", note+"on:", 1) + if got != want { + t.Errorf("unexpected body\n--- want ---\n%s\n--- got ---\n%s", want, got) + } + }) + } +} + +func TestTransformWorkflow_DisablesJobAtFourSpaceIndent(t *testing.T) { + raw := []byte(`name: CI +on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: make build + + deploy: + runs-on: [self-hosted, linux] + strategy: + matrix: + env: [staging, prod] + steps: + - run: make deploy + + publish: + runs-on: ubuntu-latest + steps: + - run: make publish +`) + + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{ + {Name: "build", RunsOn: "ubuntu-latest"}, + {Name: "deploy", RunsOn: "self-hosted,linux", HasMatrix: true}, + {Name: "publish", RunsOn: "ubuntu-latest"}, + }, + } + + result, err := TransformWorkflow(raw, wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !result.HasCritical { + t.Fatalf("expected HasCritical, changes: %v", result.Changes) + } + + content := string(result.Content) + if !strings.Contains(content, " # DISABLED:") { + t.Errorf("expected DISABLED marker at the job's own indent, got:\n%s", content) + } + for _, want := range []string{ + " # deploy:", + " # runs-on: [self-hosted, linux]", + " # env: [staging, prod]", + } { + if !strings.Contains(content, want) { + t.Errorf("expected %q in commented-out job, got:\n%s", want, content) + } + } + + newLabel, _ := runsOnNote(t, "ubuntu-latest") + if n := strings.Count(content, "runs-on: "+newLabel); n != 2 { + t.Errorf("expected 2 live remapped labels, got %d:\n%s", n, content) + } + if strings.Contains(content, "# runs-on: "+newLabel) { + t.Errorf("disabled job should not have been remapped, got:\n%s", content) + } +} + +func TestTransformWorkflow_RemapsRunsOnSequenceInPlace(t *testing.T) { + raw := `name: CI + +on: push + +jobs: + build: + runs-on: [ubuntu-latest] + steps: + - run: make build +` + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + newLabel, note := runsOnNote(t, "ubuntu-latest") + want := strings.Replace(raw, "runs-on: [ubuntu-latest]", "runs-on: ["+newLabel+"] "+note, 1) + if got := body(t, string(result.Content)); got != want { + t.Errorf("unexpected body\n--- want ---\n%s\n--- got ---\n%s", want, got) + } +} + +func TestTransformWorkflow_KeepsExistingLineComment(t *testing.T) { + raw := `name: CI +on: push +jobs: + build: + runs-on: ubuntu-latest # pinned deliberately + steps: + - run: make build +` + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + newLabel, note := runsOnNote(t, "ubuntu-latest") + want := strings.Replace(raw, + " runs-on: ubuntu-latest # pinned deliberately", + " "+note+"\n runs-on: "+newLabel+" # pinned deliberately", 1) + if got := body(t, string(result.Content)); got != want { + t.Errorf("unexpected body\n--- want ---\n%s\n--- got ---\n%s", want, got) + } +} + +func TestTransformWorkflow_KeepsLabelQuotingStyle(t *testing.T) { + tests := []struct{ name, original, want string }{ + {name: "double quoted", original: `"ubuntu-latest"`, want: `"depot-ubuntu-latest"`}, + {name: "single quoted", original: `'ubuntu-latest'`, want: `'depot-ubuntu-latest'`}, + {name: "plain", original: `ubuntu-latest`, want: `depot-ubuntu-latest`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + raw := "name: CI\non: push\njobs:\n build:\n runs-on: " + tt.original + "\n steps:\n - run: make build\n" + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, note := runsOnNote(t, "ubuntu-latest") + want := strings.Replace(raw, "runs-on: "+tt.original, "runs-on: "+tt.want+" "+note, 1) + if got := body(t, string(result.Content)); got != want { + t.Errorf("unexpected body\n--- want ---\n%s\n--- got ---\n%s", want, got) + } + }) + } +} + +func TestTransformWorkflow_CollapsedTriggerKeepsLineComment(t *testing.T) { + raw := `name: CI +on: # only cut releases + release: + types: [published] +jobs: + build: + runs-on: depot-ubuntu-latest +` + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"release"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "depot-ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := "name: CI\n# Removed unsupported trigger: release. " + compat.TriggerRules["release"].Note + "\n" + + `on: {} # only cut releases +jobs: + build: + runs-on: depot-ubuntu-latest +` + if got := body(t, string(result.Content)); got != want { + t.Errorf("unexpected body\n--- want ---\n%s\n--- got ---\n%s", want, got) + } +} + +func TestTransformWorkflow_RemapsAfterMultibyteOnSameLine(t *testing.T) { + raw := `name: CI + +on: push + +jobs: + build: + runs-on: [ubuntü-runner, ubuntu-latest] + steps: + - run: make build +` + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntü-runner,ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + firstLabel, firstNote := runsOnNote(t, "ubuntü-runner") + secondLabel, secondNote := runsOnNote(t, "ubuntu-latest") + want := strings.Replace(raw, + "runs-on: [ubuntü-runner, ubuntu-latest]", + "runs-on: ["+firstLabel+", "+secondLabel+"] "+firstNote+" "+strings.TrimPrefix(secondNote, "# "), 1) + if got := body(t, string(result.Content)); got != want { + t.Errorf("unexpected body\n--- want ---\n%s\n--- got ---\n%s", want, got) + } +} + +func TestTransformWorkflow_HandlesCRLF(t *testing.T) { + lf := "name: CI\n\non: push\n\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: make build\n" + raw := strings.ReplaceAll(lf, "\n", "\r\n") + + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + newLabel, note := runsOnNote(t, "ubuntu-latest") + want := strings.Replace(raw, "runs-on: ubuntu-latest", "runs-on: "+newLabel+" "+note, 1) + if got := body(t, string(result.Content)); got != want { + t.Errorf("unexpected body\n--- want ---\n%q\n--- got ---\n%q", want, got) + } + + var parsed map[string]any + if err := yaml.Unmarshal(result.Content, &parsed); err != nil { + t.Errorf("transformed CRLF workflow does not parse: %v", err) + } +} + +func TestTransformWorkflow_FallsBackWhenExtentUnknown(t *testing.T) { + raw := []byte(strings.Replace(fidelityWorkflow, + "runs-on: ubuntu-latest", "runs-on: &label ubuntu-latest", 1)) + + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "ubuntu-latest"}}, + } + + result, err := TransformWorkflow(raw, wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + newLabel, _ := runsOnNote(t, "ubuntu-latest") + content := string(result.Content) + if !strings.Contains(content, newLabel) { + t.Errorf("expected label remapped even on the fallback path, got:\n%s", content) + } + + if strings.Contains(body(t, content), "\n\n") { + t.Errorf("expected re-encoding to drop blank lines, got:\n%s", content) + } + if !strings.Contains(content, `\n`) { + t.Errorf("expected re-encoding to collapse the run block into an escaped string, got:\n%s", content) + } + if strings.Contains(content, "run: |") { + t.Errorf("expected the run block style to be lost on re-encoding, got:\n%s", content) + } +} + +func TestAnnotateLineFlattensNewlinesInNotes(t *testing.T) { + s := newSource([]byte("jobs:\n build:\n runs-on: x\n")) + + e, ok := annotateLine(s, 3, 0, []string{"was: self-hosted\ninjected: pwned. Nonstandard runner."}) + if !ok { + t.Fatalf("annotateLine refused a line it should annotate") + } + if strings.ContainsAny(e.text, "\n\r") { + t.Errorf("note text carries a line break, which would end the comment: %q", e.text) + } + if !strings.Contains(e.text, "injected: pwned") { + t.Errorf("the note's content should be kept, only flattened: %q", e.text) + } +} + +func TestTransformWorkflow_NewlineInLabelStaysInComment(t *testing.T) { + raw := "name: CI\non: push\njobs:\n build:\n runs-on: \"self-hosted\\ninjected: pwned\"\n steps:\n - run: make build\n" + + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "self-hosted"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var out map[string]any + if err := yaml.Unmarshal(result.Content, &out); err != nil { + t.Fatalf("migrated workflow does not parse: %v\n%s", err, result.Content) + } + if _, injected := out["injected"]; injected { + t.Errorf("label content materialized as a top-level key:\n%s", result.Content) + } + if len(out) != 3 { + t.Errorf("expected exactly name, on and jobs, got %v:\n%s", out, result.Content) + } +} + +func TestTransformWorkflow_RewritesPathsWithoutReformatting(t *testing.T) { + raw := strings.ReplaceAll(`name: CI + +on: + push: + branches: [main] + +jobs: + build: + runs-on: depot-ubuntu-latest + + steps: + - uses: ./.github/actions/setup + + - uses: acme/toolkit/.github/actions/probe@v1 + + - name: Build + run: | + ./.github/actions/build.sh + + make test +`, "", " ") + + wf := &migrate.WorkflowFile{ + Path: ".github/workflows/ci.yml", + Name: "CI", + Triggers: []string{"push"}, + Jobs: []migrate.JobInfo{{Name: "build", RunsOn: "depot-ubuntu-latest"}}, + } + + result, err := TransformWorkflow([]byte(raw), wf, compat.AnalyzeWorkflow(wf), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := strings.ReplaceAll(raw, "./.github/actions/", "./.depot/actions/") + if got := body(t, string(result.Content)); got != want { + t.Errorf("expected only the local paths rewritten\n--- want ---\n%s\n--- got ---\n%s", want, got) + } + + if !strings.Contains(string(result.Content), "acme/toolkit/.github/actions/probe@v1") { + t.Errorf("remote reference should not be rewritten, got:\n%s", result.Content) + } + + var rewrote bool + for _, c := range result.Changes { + if c.Type == ChangePathRewritten { + rewrote = true + } + } + if !rewrote { + t.Errorf("expected a ChangePathRewritten record, got %v", result.Changes) + } +} diff --git a/pkg/ci/transform/textedit.go b/pkg/ci/transform/textedit.go new file mode 100644 index 00000000..1d26f477 --- /dev/null +++ b/pkg/ci/transform/textedit.go @@ -0,0 +1,284 @@ +package transform + +import ( + "fmt" + "sort" + "strings" + "unicode/utf8" + + "gopkg.in/yaml.v3" +) + +// source indexes the original YAML so node positions can become byte offsets. +type source struct { + text string + // lineStarts[i] is the byte offset of the first byte of line i+1. + lineStarts []int +} + +func newSource(raw []byte) *source { + text := string(raw) + starts := []int{0} + for i := 0; i < len(text); i++ { + if text[i] == '\n' { + starts = append(starts, i+1) + } + } + return &source{text: text, lineStarts: starts} +} + +func (s *source) lineCount() int { + return len(s.lineStarts) +} + +// lineStart returns the byte offset where a line begins. One past the last line +// is accepted as the end of the text. +func (s *source) lineStart(line int) (int, bool) { + if line < 1 || line > len(s.lineStarts)+1 { + return 0, false + } + if line == len(s.lineStarts)+1 { + return len(s.text), true + } + return s.lineStarts[line-1], true +} + +// lineEnd returns the byte offset just past a line's content. +func (s *source) lineEnd(line int) (int, bool) { + start, ok := s.lineStart(line) + if !ok { + return 0, false + } + end := len(s.text) + if line < len(s.lineStarts) { + end = s.lineStarts[line] - 1 // drop the \n + } + if end < start { + end = start + } + return end, true +} + +// line returns a line without its line terminator. +func (s *source) line(line int) string { + start, ok := s.lineStart(line) + if !ok { + return "" + } + end, ok := s.lineEnd(line) + if !ok { + return "" + } + return strings.TrimSuffix(s.text[start:end], "\r") +} + +// offset converts a YAML line and character column into a byte offset. +func (s *source) offset(line, col int) (int, bool) { + start, ok := s.lineStart(line) + if !ok || col < 1 { + return 0, false + } + end, ok := s.lineEnd(line) + if !ok { + return 0, false + } + off := start + for c := 1; c < col; c++ { + if off >= end { + return 0, false + } + _, size := utf8.DecodeRuneInString(s.text[off:]) + off += size + } + return off, true +} + +// nodeOffset returns the byte offset where a node begins. +func (s *source) nodeOffset(n *yaml.Node) (int, bool) { + if n == nil { + return 0, false + } + return s.offset(n.Line, n.Column) +} + +// indentOf returns a line's leading whitespace width and whether it has content. +func (s *source) indentOf(line int) (indent int, hasContent bool) { + text := s.line(line) + trimmed := strings.TrimLeft(text, " \t") + if trimmed == "" { + return 0, false + } + return len(text) - len(trimmed), true +} + +// isCommentLine reports whether a line is a comment at the given indent. +func (s *source) isCommentLine(line, indent int) bool { + got, hasContent := s.indentOf(line) + if !hasContent || got != indent { + return false + } + return strings.HasPrefix(strings.TrimLeft(s.line(line), " \t"), "#") +} + +// edit replaces the byte range [start,end); equal bounds insert text. +type edit struct { + start, end int + text string +} + +// applyEdits splices non-overlapping edits into the text. +func applyEdits(text string, edits []edit) (string, error) { + sorted := make([]edit, len(edits)) + copy(sorted, edits) + sort.SliceStable(sorted, func(i, j int) bool { + if sorted[i].start != sorted[j].start { + return sorted[i].start < sorted[j].start + } + return sorted[i].end < sorted[j].end + }) + + var b strings.Builder + last := 0 + for _, e := range sorted { + if e.start < 0 || e.end < e.start || e.end > len(text) { + return "", fmt.Errorf("edit [%d,%d) out of range for %d bytes", e.start, e.end, len(text)) + } + if e.start < last { + return "", fmt.Errorf("edit [%d,%d) overlaps a previous edit ending at %d", e.start, e.end, last) + } + b.WriteString(text[last:e.start]) + b.WriteString(e.text) + last = e.end + } + b.WriteString(text[last:]) + return b.String(), nil +} + +// scalarExtent returns a safely delimited scalar token, or false when callers +// should use the re-encoding fallback. flow marks flow-collection delimiters. +func scalarExtent(text string, n *yaml.Node, start int, flow bool) (int, int, bool) { + if n == nil || n.Kind != yaml.ScalarNode || n.Anchor != "" || n.Alias != nil { + return 0, 0, false + } + if n.Style&(yaml.LiteralStyle|yaml.FoldedStyle|yaml.TaggedStyle) != 0 { + return 0, 0, false + } + if start < 0 || start >= len(text) { + return 0, 0, false + } + + var end int + switch { + case n.Style&yaml.DoubleQuotedStyle != 0: + if text[start] != '"' { + return 0, 0, false + } + i := start + 1 + for i < len(text) && text[i] != '\n' { + if text[i] == '\\' { + i += 2 + continue + } + if text[i] == '"' { + end = i + 1 + break + } + i++ + } + + case n.Style&yaml.SingleQuotedStyle != 0: + if text[start] != '\'' { + return 0, 0, false + } + i := start + 1 + for i < len(text) && text[i] != '\n' { + if text[i] == '\'' { + if i+1 < len(text) && text[i+1] == '\'' { + i += 2 + continue + } + end = i + 1 + break + } + i++ + } + + default: // plain + i := start + for i < len(text) { + c := text[i] + if c == '\n' { + break + } + // A comment starts only after whitespace. + if c == '#' && i > start && (text[i-1] == ' ' || text[i-1] == '\t') { + break + } + // Stop before a mapping key's colon. + if c == ':' && (i+1 >= len(text) || isSpaceByte(text[i+1]) || text[i+1] == '\n') { + break + } + if flow && (c == ',' || c == ']' || c == '}' || c == ':') { + break + } + i++ + } + end = i + for end > start && isSpaceByte(text[end-1]) { + end-- + } + } + + if end <= start || end > len(text) { + return 0, 0, false + } + if !scalarSourceIs(text[start:end], n.Value) { + return 0, 0, false + } + return start, end, true +} + +// scalarSourceIs confirms an extracted token has the node's value. +func scalarSourceIs(token, want string) bool { + var doc yaml.Node + if err := yaml.Unmarshal([]byte(token), &doc); err != nil { + return false + } + if doc.Kind != yaml.DocumentNode || len(doc.Content) != 1 { + return false + } + got := doc.Content[0] + return got.Kind == yaml.ScalarNode && got.Value == want +} + +// commentStart returns a trailing comment offset, or -1 when absent. +func (s *source) commentStart(line, from int) int { + end, ok := s.lineEnd(line) + if !ok || from < 0 || from > end { + return -1 + } + if idx := strings.IndexByte(s.text[from:end], '#'); idx >= 0 { + return from + idx + } + return -1 +} + +// contentEnd returns where a trailing comment can be appended. +func (s *source) contentEnd(line int) (int, bool) { + start, ok := s.lineStart(line) + if !ok { + return 0, false + } + end, ok := s.lineEnd(line) + if !ok { + return 0, false + } + for end > start && isSpaceByte(s.text[end-1]) { + end-- + } + return end, true +} + +func isSpaceByte(b byte) bool { + return b == ' ' || b == '\t' || b == '\r' +} diff --git a/pkg/ci/transform/textedit_test.go b/pkg/ci/transform/textedit_test.go new file mode 100644 index 00000000..ce5f0280 --- /dev/null +++ b/pkg/ci/transform/textedit_test.go @@ -0,0 +1,178 @@ +package transform + +import ( + "testing" + + "gopkg.in/yaml.v3" +) + +func TestSourceOffsetCountsRunesNotBytes(t *testing.T) { + s := newSource([]byte("a: ünïcode\nb: two\n")) + + off, ok := s.offset(1, 5) + if !ok { + t.Fatal("offset failed") + } + if got := s.text[off]; got != 'n' { + t.Errorf("offset(1, 5) = byte %d (%q), want the 'n' after the multibyte rune", off, got) + } + + if off, ok := s.offset(2, 4); !ok || s.text[off:off+3] != "two" { + t.Errorf("offset(2, 4) = %d, ok=%v", off, ok) + } + if _, ok := s.offset(2, 99); ok { + t.Error("expected a column past the end of the line to fail") + } +} + +func TestSourceLineStartAcceptsOnePastTheEnd(t *testing.T) { + s := newSource([]byte("one\ntwo\n")) + if got := s.lineCount(); got != 3 { + t.Fatalf("lineCount = %d, want 3 (the empty line after the final newline)", got) + } + if off, ok := s.lineStart(s.lineCount() + 1); !ok || off != len(s.text) { + t.Errorf("lineStart(lineCount+1) = %d, ok=%v, want %d", off, ok, len(s.text)) + } + if _, ok := s.lineStart(s.lineCount() + 2); ok { + t.Error("expected a line two past the end to fail") + } +} + +func TestApplyEdits(t *testing.T) { + const text = "hello world" + + got, err := applyEdits(text, []edit{ + {start: 6, end: 11, text: "there"}, + {start: 0, end: 5, text: "goodbye"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "goodbye there" { + t.Errorf("got %q, want %q", got, "goodbye there") + } + + got, err = applyEdits(text, []edit{ + {start: 5, end: 5, text: ","}, + {start: 5, end: 5, text: " and"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "hello, and world" { + t.Errorf("got %q, want %q", got, "hello, and world") + } + + if _, err := applyEdits(text, []edit{ + {start: 0, end: 6, text: "x"}, + {start: 3, end: 8, text: "y"}, + }); err == nil { + t.Error("expected overlapping edits to be rejected") + } + + if _, err := applyEdits(text, []edit{{start: 0, end: 99, text: "x"}}); err == nil { + t.Error("expected an out-of-range edit to be rejected") + } +} + +func TestScalarExtent(t *testing.T) { + tests := []struct { + name string + doc string + path string + want string + }{ + {name: "plain", doc: "k: ubuntu-latest\n", path: "k", want: "ubuntu-latest"}, + {name: "plain with trailing comment", doc: "k: ubuntu-latest # note\n", path: "k", want: "ubuntu-latest"}, + {name: "plain with trailing space", doc: "k: ubuntu-latest \n", path: "k", want: "ubuntu-latest"}, + {name: "plain containing a hash", doc: "k: build#1\n", path: "k", want: "build#1"}, + {name: "double quoted", doc: `k: "a: b # c"` + "\n", path: "k", want: `"a: b # c"`}, + {name: "double quoted with escape", doc: `k: "say \"hi\""` + "\n", path: "k", want: `"say \"hi\""`}, + {name: "single quoted", doc: "k: 'it''s here'\n", path: "k", want: "'it''s here'"}, + {name: "literal block refused", doc: "k: |\n line one\n", path: "k", want: ""}, + {name: "folded block refused", doc: "k: >\n line one\n", path: "k", want: ""}, + {name: "anchored refused", doc: "k: &a ubuntu-latest\n", path: "k", want: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var doc yaml.Node + if err := yaml.Unmarshal([]byte(tt.doc), &doc); err != nil { + t.Fatalf("fixture does not parse: %v", err) + } + _, _, val := findMappingEntry(doc.Content[0], tt.path) + if val == nil { + t.Fatalf("key %q not found", tt.path) + } + + s := newSource([]byte(tt.doc)) + start, ok := s.nodeOffset(val) + if !ok { + t.Fatal("nodeOffset failed") + } + start, end, ok := scalarExtent(s.text, val, start, false) + if tt.want == "" { + if ok { + t.Errorf("expected refusal, got %q", s.text[start:end]) + } + return + } + if !ok { + t.Fatal("scalarExtent refused a scalar it should have delimited") + } + if got := s.text[start:end]; got != tt.want { + t.Errorf("got %q, want %q", got, tt.want) + } + }) + } +} + +func TestScalarExtentStopsAtKeyColon(t *testing.T) { + const doc = "on:\n push: {}\n" + var node yaml.Node + if err := yaml.Unmarshal([]byte(doc), &node); err != nil { + t.Fatalf("fixture does not parse: %v", err) + } + _, key, _ := findMappingEntry(node.Content[0], "on") + if key == nil { + t.Fatal("key not found") + } + + s := newSource([]byte(doc)) + start, ok := s.nodeOffset(key) + if !ok { + t.Fatal("nodeOffset failed") + } + start, end, ok := scalarExtent(s.text, key, start, false) + if !ok { + t.Fatal("scalarExtent refused a plain key") + } + if got := s.text[start:end]; got != "on" { + t.Errorf("got %q, want %q", got, "on") + } +} + +func TestScalarExtentRefusesAMisdelimitedToken(t *testing.T) { + s := newSource([]byte("k: ubuntu-latest\n")) + node := &yaml.Node{Kind: yaml.ScalarNode, Value: "something-else", Line: 1, Column: 4} + start, _ := s.nodeOffset(node) + if _, _, ok := scalarExtent(s.text, node, start, false); ok { + t.Error("expected a token that does not reparse to the node's value to be refused") + } +} + +func TestIsCommentLine(t *testing.T) { + s := newSource([]byte("# top\n # nested\nkey: 1\n\n")) + if !s.isCommentLine(1, 0) { + t.Error("line 1 is a comment at indent 0") + } + if s.isCommentLine(2, 0) { + t.Error("line 2's comment is at indent 2, not 0") + } + if !s.isCommentLine(2, 2) { + t.Error("line 2 is a comment at indent 2") + } + if s.isCommentLine(3, 0) || s.isCommentLine(4, 0) { + t.Error("neither a key line nor a blank line is a comment line") + } +} diff --git a/pkg/ci/transform/transform.go b/pkg/ci/transform/transform.go index fe779afd..c96e5d24 100644 --- a/pkg/ci/transform/transform.go +++ b/pkg/ci/transform/transform.go @@ -49,7 +49,21 @@ type TransformResult struct { // 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. +// +// Changes are spliced into the original YAML when safe; unsupported shapes use +// the existing node-tree fallback. func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.CompatibilityReport, migratedWorkflows map[string]bool) (*TransformResult, error) { + var changes []ChangeRecord + + // Rewrite paths before parsing so source positions still match the edited text. + if rewritten, changed := rewriteGitHubPaths(string(raw), migratedWorkflows); changed { + raw = []byte(rewritten) + changes = append(changes, ChangeRecord{ + Type: ChangePathRewritten, + Detail: "Rewrote .github/ path references to .depot/", + }) + } + var doc yaml.Node if err := yaml.Unmarshal(raw, &doc); err != nil { return nil, fmt.Errorf("failed to parse YAML: %w", err) @@ -64,40 +78,19 @@ func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.Comp return nil, fmt.Errorf("expected mapping at root, got %d", root.Kind) } - var changes []ChangeRecord - - // 1. Transform triggers - triggerChanges := transformTriggers(root) - changes = append(changes, triggerChanges...) - - // 2. Identify jobs that need to be disabled (uncorrectable issues) + // Jobs with uncorrectable issues get commented out rather than corrected, + // and are skipped by the passes that would otherwise edit them. disabledJobs := findDisabledJobs(wf, report) - // 3. Transform runs-on labels (skip disabled jobs) - 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 - var buf bytes.Buffer - enc := yaml.NewEncoder(&buf) - enc.SetIndent(2) - if err := enc.Encode(&doc); err != nil { - return nil, fmt.Errorf("failed to marshal YAML: %w", err) - } - enc.Close() - - output := buf.Bytes() - - // 6. Post-process: comment out disabled jobs in text - if len(disabledJobs) > 0 { - var disableChanges []ChangeRecord - output, disableChanges = commentOutDisabledJobs(output, disabledJobs) - changes = append(changes, disableChanges...) + output, editChanges, ok := transformInPlace(newSource(raw), root, disabledJobs) + if !ok { + var err error + output, editChanges, err = transformByReencoding(&doc, root, disabledJobs) + if err != nil { + return nil, err + } } + changes = append(changes, editChanges...) hasCritical := false for _, c := range changes { @@ -107,7 +100,6 @@ func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.Comp } } - // 7. Prepend header comment header := buildHeaderComment(wf, changes) output = append([]byte(header), output...) @@ -118,6 +110,31 @@ func TransformWorkflow(raw []byte, wf *migrate.WorkflowFile, report *compat.Comp }, nil } +// transformByReencoding is the existing correctness fallback for unsupported +// source shapes; it may reformat the file. +func transformByReencoding(doc, root *yaml.Node, disabledJobs map[string]disabledJobInfo) ([]byte, []ChangeRecord, error) { + var changes []ChangeRecord + changes = append(changes, transformTriggers(root)...) + changes = append(changes, transformRunsOn(root, disabledJobs)...) + + var buf bytes.Buffer + enc := yaml.NewEncoder(&buf) + enc.SetIndent(2) + if err := enc.Encode(doc); err != nil { + return nil, nil, fmt.Errorf("failed to marshal YAML: %w", err) + } + enc.Close() + + output := buf.Bytes() + if len(disabledJobs) > 0 { + var disableChanges []ChangeRecord + output, disableChanges = commentOutDisabledJobs(output, disabledJobs) + changes = append(changes, disableChanges...) + } + + return output, changes, nil +} + // transformTriggers removes unsupported triggers from the on: block. func transformTriggers(root *yaml.Node) []ChangeRecord { var changes []ChangeRecord @@ -325,41 +342,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)`) @@ -489,17 +471,6 @@ func isURL(s string, idx int) bool { 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