diff --git a/companions/buildspec.go b/companions/buildspec.go index c389380c..0210a286 100644 --- a/companions/buildspec.go +++ b/companions/buildspec.go @@ -5,6 +5,7 @@ import ( "os" "path" "path/filepath" + "regexp" "slices" "strings" ) @@ -112,34 +113,52 @@ func BuildSpecs() ([]BuildSpec, error) { } // Instruction is one logical Dockerfile instruction: its physical lines -// joined where a line ends in the escape character, comment lines inside it -// dropped, and Line the physical line it starts on. +// joined where a line ends in the escape character, comment and blank lines +// inside it dropped, and Line the physical line it starts on. type Instruction struct { Line int Text string } -// Instructions splits a Dockerfile into its logical instructions. A COPY -// written over several lines is one instruction, so its sources on the later -// lines are read with it rather than missed. +// escapeDirective is the `# escape=` parser directive, which may only appear +// among the comment lines at the top of a Dockerfile. +var escapeDirective = regexp.MustCompile(`^#\s*escape\s*=\s*(\S)\s*$`) + +// directive is any parser directive line: `# key=value`. +var directive = regexp.MustCompile(`^#\s*[a-zA-Z]+\s*=`) + +// Instructions splits a Dockerfile into its logical instructions, as BuildKit +// reads them. A COPY written over several lines is one instruction, so its +// sources on the later lines are read with it rather than missed: the line +// continuation is the escape character (`\` unless an `# escape=` directive +// names another), and a blank or comment line inside a continuation neither +// ends nor joins it. func Instructions(dockerfile string) []Instruction { + lines := strings.Split(dockerfile, "\n") + escape := "\\" + for _, raw := range lines { + trimmed := strings.TrimSpace(raw) + if !directive.MatchString(trimmed) { + break // directives end at the first line that is not one + } + if match := escapeDirective.FindStringSubmatch(trimmed); match != nil { + escape = match[1] + } + } var out []Instruction var current []string start := 0 - for number, raw := range strings.Split(dockerfile, "\n") { + for number, raw := range lines { line := strings.TrimRight(raw, " \t\r") trimmed := strings.TrimSpace(line) - if len(current) == 0 && (trimmed == "" || strings.HasPrefix(trimmed, "#")) { - continue - } - if len(current) > 0 && strings.HasPrefix(trimmed, "#") { - continue // a comment inside a continued instruction is not part of it + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue // between instructions or inside a continuation alike } if len(current) == 0 { start = number + 1 } - if strings.HasSuffix(line, "\\") { - current = append(current, strings.TrimSuffix(line, "\\")) + if strings.HasSuffix(line, escape) { + current = append(current, strings.TrimSuffix(line, escape)) continue } current = append(current, line) diff --git a/companions/buildspec_test.go b/companions/buildspec_test.go index 82e28257..b8336176 100644 --- a/companions/buildspec_test.go +++ b/companions/buildspec_test.go @@ -228,6 +228,36 @@ func TestInstructionsJoinContinuedLines(t *testing.T) { require.Equal(t, []string{"a", "b"}, sources) } +// A blank line inside a continuation does not end it: BuildKit skips it, so +// the sources after it belong to the same COPY. +func TestABlankLineInsideAContinuationKeepsTheInstruction(t *testing.T) { + instructions := companions.Instructions("FROM alpine\nCOPY a \\\n\n b /dst/\n") + require.Len(t, instructions, 2) + sources, kind := companions.CopySources(instructions[1].Text) + require.Equal(t, companions.FromContext, kind) + require.Equal(t, []string{"a", "b"}, sources) +} + +// `# escape=` names the continuation character; a backslash is then an +// ordinary character (a Windows path), not a continuation. +func TestTheEscapeDirectiveNamesTheContinuation(t *testing.T) { + dockerfile := "# syntax=docker/dockerfile:1\n# escape=`\nFROM alpine\nCOPY a `\n b C:\\dst\\\nRUN true\n" + instructions := companions.Instructions(dockerfile) + require.Len(t, instructions, 3) + sources, kind := companions.CopySources(instructions[1].Text) + require.Equal(t, companions.FromContext, kind) + require.Equal(t, []string{"a", "b"}, sources) + require.Equal(t, "RUN true", instructions[2].Text) +} + +// A directive after the first instruction is an ordinary comment. +func TestAnEscapeDirectiveAfterAnInstructionIsAComment(t *testing.T) { + instructions := companions.Instructions("FROM alpine\n# escape=`\nCOPY a \\\n b /dst/\n") + require.Len(t, instructions, 2) + sources, _ := companions.CopySources(instructions[1].Text) + require.Equal(t, []string{"a", "b"}, sources) +} + func TestCopySourcesReadsEveryContextCopyShape(t *testing.T) { for line, expected := range map[string]struct { sources []string diff --git a/docs/runbooks/branch-protection.md b/docs/runbooks/branch-protection.md index f7a0e00a..76360aa2 100644 --- a/docs/runbooks/branch-protection.md +++ b/docs/runbooks/branch-protection.md @@ -34,11 +34,14 @@ repositories the rulesets are: Queried, not inferred. Ruleset 24397257, `target: branch`, `enforcement: active`, `bypass_actors: []`, conditions `refs/heads/main`: -- **`required_status_checks`** — `Build`, `Proto`, `Registry cache cold (go)`, - `Registry cache cold (next)`, `Registry cache clean runner (go)`, - `Registry cache clean runner (next)`. Six, not the eight below: - `pnpm source evidence` is derived as safe to require and is **not** in the - live ruleset; `Companion version` joins it once #838 merges. +- **`required_status_checks`** — `Build`, `Companion version`, `Proto`, + `Registry cache cold (go)`, `Registry cache cold (next)`, + `Registry cache clean runner (go)`, `Registry cache clean runner (next)`. + Seven, not the eight below: `pnpm source evidence` is derived as safe to + require and is **not** in the live ruleset. `Companion version` was added + on 2026-10-26 by appending that one context to the live ruleset (GET, + append, PUT), not by applying the payload below, which would also change + the decisions listed under it. - **`strict_required_status_checks_policy: false`** — a pull request that went green against an older `main` can land without being retested. The `"strict": true` recommended below is not what is applied. diff --git a/internal/companionbump/main.go b/internal/companionbump/main.go index b64cd860..d96ed0df 100644 --- a/internal/companionbump/main.go +++ b/internal/companionbump/main.go @@ -96,13 +96,21 @@ func git(args ...string) (string, error) { // changedFiles lists every path that differs between baseRef's merge base and // HEAD. --no-renames lists a moved file at both of its paths: moving a file out -// of an input changes the image as surely as editing it. +// of an input changes the image as surely as editing it. -z keeps each path as +// it is: without it git quotes a path with a space or a non-ASCII byte, and the +// quoted spelling matches no input. func changedFiles(baseRef string) ([]string, error) { - diff, err := git("diff", "--name-only", "--no-renames", baseRef+"...HEAD") + diff, err := git("diff", "--name-only", "--no-renames", "-z", baseRef+"...HEAD") if err != nil { return nil, err } - return strings.Fields(diff), nil + var files []string + for _, file := range strings.Split(diff, "\x00") { + if file != "" { + files = append(files, file) + } + } + return files, nil } func run(baseRef string) ([]string, error) { diff --git a/internal/companionbump/main_test.go b/internal/companionbump/main_test.go index 7c21f4cf..b836b40b 100644 --- a/internal/companionbump/main_test.go +++ b/internal/companionbump/main_test.go @@ -91,28 +91,55 @@ func TestTheCheckoutsInputsCoverTheFacadePlugins(t *testing.T) { require.NotContains(t, byName["codefly"], "bin/linux/${TARGETARCH}/codefly") } +// A path git would quote (non-ASCII, a space) is listed as it is, so it still +// lies under its input. +func TestAChangedPathIsListedAsItIs(t *testing.T) { + gitRepository(t) + file := filepath.Join("companions", "proto", "facades", "python", "données plugin.py") + require.NoError(t, os.MkdirAll(filepath.Dir(file), 0o755)) + require.NoError(t, os.WriteFile(file, []byte("x\n"), 0o644)) + gitRun(t, "add", ".") + gitRun(t, "commit", "-q", "-m", "change") + + changed, err := changedFiles("main") + require.NoError(t, err) + require.Equal(t, []string{"companions/proto/facades/python/données plugin.py"}, changed) + require.NotEmpty(t, touchedInputs(proto.inputs, changed)) +} + +// gitRepository makes a scratch repository the current directory: main holds +// one commit, and the branch "change" is checked out on top of it. +func gitRepository(t *testing.T) { + t.Helper() + t.Chdir(t.TempDir()) + gitRun(t, "init", "-q", "-b", "main") + require.NoError(t, os.WriteFile("README", []byte("base\n"), 0o644)) + gitRun(t, "add", ".") + gitRun(t, "commit", "-q", "-m", "base") + gitRun(t, "checkout", "-q", "-b", "change") +} + +func gitRun(t *testing.T, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Env = append(os.Environ(), "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t", "GIT_CONFIG_GLOBAL=/dev/null") + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v: %s", args, out) +} + // Moving a file out of an input changes the image: git's rename detection // would list only the destination, outside every input, and let it through. func TestAFileMovedOutOfAnInputIsAChangeToIt(t *testing.T) { - dir := t.TempDir() - t.Chdir(dir) - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Env = append(os.Environ(), "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t", "GIT_CONFIG_GLOBAL=/dev/null") - out, err := cmd.CombinedOutput() - require.NoError(t, err, "git %v: %s", args, out) - } - run("init", "-q", "-b", "main") + gitRepository(t) plugin := filepath.Join("companions", "proto", "facades", "python", "plugin.py") require.NoError(t, os.MkdirAll(filepath.Dir(plugin), 0o755)) require.NoError(t, os.WriteFile(plugin, []byte("print('the facade plugin, long enough to be detected as a rename')\n"), 0o644)) - run("add", ".") - run("commit", "-q", "-m", "base") - run("checkout", "-q", "-b", "change") + gitRun(t, "add", ".") + gitRun(t, "commit", "-q", "-m", "add the plugin") + gitRun(t, "branch", "-q", "-f", "main") require.NoError(t, os.MkdirAll("elsewhere", 0o755)) - run("mv", plugin, filepath.Join("elsewhere", "plugin.py")) - run("commit", "-q", "-m", "move") + gitRun(t, "mv", plugin, filepath.Join("elsewhere", "plugin.py")) + gitRun(t, "commit", "-q", "-m", "move") changed, err := changedFiles("main") require.NoError(t, err)