From 6f0c593f6098ced3b42cd592c1fc47bb4cc02999 Mon Sep 17 00:00:00 2001 From: Christopher Maher Date: Thu, 9 Jul 2026 19:13:21 -0700 Subject: [PATCH] feat(foreman): integrate library for disjoint slice union Go port of the slicer experiment's integrate.sh: union the disjoint slice branches onto the current base on a fresh integration branch and return the union diff for reconciliation. Proves disjointness first (no file changed by two slices -> *OverlapError), then applies each slice's diff onto the base (*ApplyError on conflict, e.g. a slice cut from a stale base) and commits; *EmptyUnionError when no slice changed anything. Owners maps each changed file to its single owning slice. Git is shelled via an injectable GitRunner (exec-backed default), matching Foreman's commandRunner convention. Real-git table tests cover clean disjoint union, overlap rejection (branch not left behind), empty union, and arg validation. Pure library, no controller wiring. Part of #1033 Signed-off-by: Christopher Maher --- pkg/foreman/slicer/integrate.go | 198 +++++++++++++++++++++++++++ pkg/foreman/slicer/integrate_test.go | 159 +++++++++++++++++++++ 2 files changed, 357 insertions(+) create mode 100644 pkg/foreman/slicer/integrate.go create mode 100644 pkg/foreman/slicer/integrate_test.go diff --git a/pkg/foreman/slicer/integrate.go b/pkg/foreman/slicer/integrate.go new file mode 100644 index 00000000..d313d6b2 --- /dev/null +++ b/pkg/foreman/slicer/integrate.go @@ -0,0 +1,198 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package slicer + +import ( + "context" + "fmt" + "os" + "os/exec" + "strings" +) + +// GitRunner runs one git subcommand in dir and returns its combined output. +// Production callers use the exec-backed default; tests inject a stub or run +// against a real temp repo. Matches Foreman's commandRunner convention so the +// library never hard-codes a git transport. +type GitRunner func(ctx context.Context, dir string, args ...string) (string, error) + +// execGitRunner is the production GitRunner, backed by os/exec. +func execGitRunner(ctx context.Context, dir string, args ...string) (string, error) { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + return string(out), err +} + +// IntegrateOptions configures a disjoint-slice union. +type IntegrateOptions struct { + // RepoDir is the git worktree the slice branches are fetched into. + RepoDir string + // Base is the ref every slice was cut from and the union is built on + // (the current upstream tip, not a stale fork default: see #813/#1029). + Base string + // Branch is the integration branch to create at Base and fill with the + // union. + Branch string + // Slices are the slice branch refs to union, in application order. + Slices []string + // Run overrides the git transport. Nil uses the exec-backed default. + Run GitRunner +} + +// IntegrateResult is a successful union. +type IntegrateResult struct { + // Branch is the integration branch that now holds the union commit. + Branch string + // UnionDiff is `git diff Base...Branch`, the input the reconcile step + // hands to its LLM sweep. + UnionDiff string + // Owners maps each changed file to the slice that changed it. Because the + // union is disjoint, every file has exactly one owner. + Owners map[string]string +} + +// OverlapError reports that two slices both changed the same file, so the +// union is not disjoint and cannot be a clean apply. A slice violated its +// assigned file scope; this is real signal, not a transient failure. +type OverlapError struct { + File string + SliceA string + SliceB string +} + +func (e *OverlapError) Error() string { + return fmt.Sprintf("slices %q and %q both change %q (union is not disjoint)", e.SliceA, e.SliceB, e.File) +} + +// ApplyError reports that a slice's diff did not apply cleanly onto the base +// (typically the slice was cut from a stale base and its diff now conflicts). +type ApplyError struct { + Slice string + Output string +} + +func (e *ApplyError) Error() string { + return fmt.Sprintf("slice %q did not apply cleanly onto base: %s", e.Slice, e.Output) +} + +// EmptyUnionError reports that no slice changed any file, so there is nothing +// to integrate. +type EmptyUnionError struct{} + +func (e *EmptyUnionError) Error() string { return "empty union: no slice changed any file" } + +// Integrate unions the disjoint slice branches onto Base on a fresh Branch and +// returns the union diff for reconciliation. It first proves the slices are +// disjoint (no file changed by two slices), then applies each slice's diff onto +// the base and commits. It returns *OverlapError, *ApplyError, or +// *EmptyUnionError for the meaningful failure modes. +func Integrate(ctx context.Context, opts IntegrateOptions) (*IntegrateResult, error) { + if opts.RepoDir == "" || opts.Base == "" || opts.Branch == "" || len(opts.Slices) == 0 { + return nil, fmt.Errorf("integrate: RepoDir, Base, Branch and at least one slice are required") + } + run := opts.Run + if run == nil { + run = execGitRunner + } + + // 1. Disjointness: no file may be changed by two slices. + owners := make(map[string]string) + for _, s := range opts.Slices { + out, err := run(ctx, opts.RepoDir, "diff", "--name-only", opts.Base+"..."+s) + if err != nil { + return nil, fmt.Errorf("integrate: diff --name-only %s...%s: %w: %s", opts.Base, s, err, strings.TrimSpace(out)) + } + for _, f := range nonEmptyLines(out) { + if prev, ok := owners[f]; ok && prev != s { + return nil, &OverlapError{File: f, SliceA: prev, SliceB: s} + } + owners[f] = s + } + } + if len(owners) == 0 { + return nil, &EmptyUnionError{} + } + + // 2. Fresh integration branch at the current base. + if out, err := run(ctx, opts.RepoDir, "checkout", "-B", opts.Branch, opts.Base); err != nil { + return nil, fmt.Errorf("integrate: checkout -B %s %s: %w: %s", opts.Branch, opts.Base, err, strings.TrimSpace(out)) + } + + // 3. Apply each slice's diff onto the base. Disjoint files mean this is a + // clean apply, not a semantic merge. + for _, s := range opts.Slices { + diff, err := run(ctx, opts.RepoDir, "diff", opts.Base+"..."+s) + if err != nil { + return nil, fmt.Errorf("integrate: diff %s...%s: %w: %s", opts.Base, s, err, strings.TrimSpace(diff)) + } + if strings.TrimSpace(diff) == "" { + continue + } + patch, err := writeTempPatch(diff) + if err != nil { + return nil, fmt.Errorf("integrate: stage patch for slice %q: %w", s, err) + } + out, err := run(ctx, opts.RepoDir, "apply", "--index", patch) + _ = os.Remove(patch) + if err != nil { + return nil, &ApplyError{Slice: s, Output: strings.TrimSpace(out)} + } + } + + // 4. Commit the union. + msg := "integrate: union of " + strings.Join(opts.Slices, " ") + if out, err := run(ctx, opts.RepoDir, "commit", "-m", msg); err != nil { + return nil, fmt.Errorf("integrate: commit union: %w: %s", err, strings.TrimSpace(out)) + } + + // 5. The union diff feeds the downstream reconcile step's LLM sweep. + unionDiff, err := run(ctx, opts.RepoDir, "diff", opts.Base+"..."+opts.Branch) + if err != nil { + return nil, fmt.Errorf("integrate: diff %s...%s: %w: %s", opts.Base, opts.Branch, err, strings.TrimSpace(unionDiff)) + } + + return &IntegrateResult{Branch: opts.Branch, UnionDiff: unionDiff, Owners: owners}, nil +} + +// writeTempPatch writes a diff to a temp file and returns its path; callers +// remove it after `git apply`. +func writeTempPatch(content string) (string, error) { + f, err := os.CreateTemp("", "slicer-patch-*.diff") + if err != nil { + return "", err + } + if _, err := f.WriteString(content); err != nil { + _ = f.Close() + return "", err + } + if err := f.Close(); err != nil { + return "", err + } + return f.Name(), nil +} + +// nonEmptyLines splits on newlines and drops blank lines. +func nonEmptyLines(s string) []string { + var out []string + for _, l := range strings.Split(s, "\n") { + if t := strings.TrimSpace(l); t != "" { + out = append(out, t) + } + } + return out +} diff --git a/pkg/foreman/slicer/integrate_test.go b/pkg/foreman/slicer/integrate_test.go new file mode 100644 index 00000000..717e7e92 --- /dev/null +++ b/pkg/foreman/slicer/integrate_test.go @@ -0,0 +1,159 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package slicer + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// git runs a git subcommand in dir, failing the test on error. +func git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } + return string(out) +} + +// readAt reads dir/path, failing the test on error. +func readAt(t *testing.T, dir, path string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(dir, path)) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// writeAt writes content to dir/path, creating parent dirs. +func writeAt(t *testing.T, dir, path, content string) { + t.Helper() + full := filepath.Join(dir, path) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// initRepo makes a temp git repo on branch main with one base commit. +func initRepo(t *testing.T) string { + t.Helper() + dir := t.TempDir() + git(t, dir, "init", "-q", "-b", "main") + git(t, dir, "config", "user.email", "test@example.com") + git(t, dir, "config", "user.name", "Test") + git(t, dir, "config", "commit.gpgsign", "false") + writeAt(t, dir, "base.txt", "base\n") + git(t, dir, "add", "-A") + git(t, dir, "commit", "-qm", "base") + return dir +} + +// sliceBranch creates branch name off main, writes files, commits, returns to main. +func sliceBranch(t *testing.T, dir, name string, files map[string]string) { + t.Helper() + git(t, dir, "checkout", "-q", "-B", name, "main") + for p, c := range files { + writeAt(t, dir, p, c) + } + git(t, dir, "add", "-A") + git(t, dir, "commit", "-qm", name) + git(t, dir, "checkout", "-q", "main") +} + +func TestIntegrate_CleanDisjointUnion(t *testing.T) { + dir := initRepo(t) + // two slices touching distinct files, plus one modifying the base file. + sliceBranch(t, dir, "slice-a", map[string]string{"a/new.txt": "AAA\n"}) + sliceBranch(t, dir, "slice-b", map[string]string{"base.txt": "base\nfrom-b\n"}) + + res, err := Integrate(context.Background(), IntegrateOptions{ + RepoDir: dir, Base: "main", Branch: "integ", Slices: []string{"slice-a", "slice-b"}, + }) + if err != nil { + t.Fatalf("integrate: %v", err) + } + if res.Branch != "integ" { + t.Fatalf("branch = %q, want integ", res.Branch) + } + if res.Owners["a/new.txt"] != "slice-a" || res.Owners["base.txt"] != "slice-b" { + t.Fatalf("owners = %v", res.Owners) + } + // the integration branch holds BOTH slices' contributions. + git(t, dir, "checkout", "-q", "integ") + if got := readAt(t, dir, "a/new.txt"); got != "AAA\n" { + t.Fatalf("a/new.txt = %q", got) + } + if got := readAt(t, dir, "base.txt"); !strings.Contains(got, "from-b") { + t.Fatalf("base.txt = %q, want the slice-b change", got) + } + if !strings.Contains(res.UnionDiff, "a/new.txt") || !strings.Contains(res.UnionDiff, "from-b") { + t.Fatalf("union diff missing a slice's change:\n%s", res.UnionDiff) + } +} + +func TestIntegrate_OverlapRejected(t *testing.T) { + dir := initRepo(t) + // both slices change the SAME file -> not disjoint. + sliceBranch(t, dir, "slice-a", map[string]string{"shared.txt": "from-a\n"}) + sliceBranch(t, dir, "slice-b", map[string]string{"shared.txt": "from-b\n"}) + + _, err := Integrate(context.Background(), IntegrateOptions{ + RepoDir: dir, Base: "main", Branch: "integ", Slices: []string{"slice-a", "slice-b"}, + }) + var overlap *OverlapError + if !errors.As(err, &overlap) { + t.Fatalf("want *OverlapError, got %v", err) + } + if overlap.File != "shared.txt" { + t.Fatalf("overlap file = %q, want shared.txt", overlap.File) + } + // the union branch must not have been left behind on a rejected integration. + if out := git(t, dir, "branch", "--list", "integ"); strings.TrimSpace(out) != "" { + t.Fatalf("integ branch should not exist after overlap, got %q", out) + } +} + +func TestIntegrate_EmptyUnion(t *testing.T) { + dir := initRepo(t) + // pointing a "slice" at main itself yields no changes. + _, err := Integrate(context.Background(), IntegrateOptions{ + RepoDir: dir, Base: "main", Branch: "integ", Slices: []string{"main"}, + }) + var empty *EmptyUnionError + if !errors.As(err, &empty) { + t.Fatalf("want *EmptyUnionError, got %v", err) + } +} + +func TestIntegrate_MissingOptions(t *testing.T) { + _, err := Integrate(context.Background(), IntegrateOptions{Base: "main", Branch: "integ", Slices: []string{"s"}}) + if err == nil { + t.Fatal("want error for missing RepoDir") + } +}