From abe5f7dc17f5d45af036a0370a6d5697f83dba35 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 03:11:06 +0200 Subject: [PATCH] Say a build source only for the trunk's head, and hold what C, assembly and a new go.mod reach (hq ADR 0267, review) A hand build of an older trunk commit said a closure lacking what was imported since, and the planner would have mapped the next merge onto it. C and assembly beside Go may include files below their directory, and a go.mod made above a package moves it out of its module: each is now held. --- internal/builder/build_source_test.go | 40 +++++++++++++++++++++----- internal/builder/builder.go | 36 +++++++++++++++++++---- internal/builder/gosource.go | 41 +++++++++++++++++++++++++++ internal/builder/gosource_test.go | 22 ++++++++++++-- internal/builder/source.go | 22 +++++++++++++- 5 files changed, 146 insertions(+), 15 deletions(-) diff --git a/internal/builder/build_source_test.go b/internal/builder/build_source_test.go index 56a2cd1d..5c1fdc48 100644 --- a/internal/builder/build_source_test.go +++ b/internal/builder/build_source_test.go @@ -16,12 +16,17 @@ import ( type onTrunk struct { *recorded off bool + // behind is a commit on the trunk that is not its head: an older commit built by hand. + behind bool } func (o onTrunk) run(ctx context.Context, dir, name string, args ...string) (string, error) { if name == "git" && len(args) > 0 && args[0] == "symbolic-ref" { return "origin/main\n", nil } + if name == "git" && len(args) > 1 && args[0] == "rev-parse" && args[1] == "origin/main" && o.behind { + return "feedfacefeedfacefeedfacefeedfacefeedface\n", nil + } if name == "git" && len(args) > 0 && args[0] == "merge-base" && o.off { return "", os.ErrNotExist } @@ -47,14 +52,14 @@ const aProxy = `{"module":"route-proxy","version":"1", "context":{"repository":"https://forge.invalid/ctl.git","ref":"main"}}, {"name":"trust","kind":"upstream","from":"alpine@sha256:` + "3333333333333333333333333333333333333333333333333333333333333333" + `"}]}}` -func buildTheProxy(t *testing.T, context_ map[string]string, off bool) (Result, *recorded, string) { +func buildTheProxy(t *testing.T, context_ map[string]string, off bool, behind ...bool) (Result, *recorded, string) { t.Helper() r := &recorded{ contents: map[string]string{"modules/route-proxy/" + ManifestName: aProxy, "modules/route-proxy/Dockerfile": "FROM scratch\nCOPY . .\n", "modules/route-proxy/README.md": "x"}, secondary: map[string]map[string]string{"https://forge.invalid/ctl.git": context_}, } workspace := t.TempDir() - got, err := Build(context.Background(), onTrunk{r, off}.run, r, + got, err := Build(context.Background(), onTrunk{r, off, len(behind) > 0 && behind[0]}.run, r, "https://forge.invalid/catalogue.git", "modules/route-proxy", "", workspace, nil, Npmrc{}, GitCredential{}, nil) if err != nil { t.Fatal(err) @@ -112,12 +117,33 @@ func TestAnImageCompilingGoIsHandedItsBuildSourceAndSaysIt(t *testing.T) { } } -// A build off the trunk says no build source: the planner maps a merge onto the trunk's. -func TestABuildOffTheTrunkSaysNoBuildSource(t *testing.T) { +// A build off the trunk, or of a trunk commit that is not its head, says no build source: the planner maps +// a merge onto the trunk head's, and an older commit's closure lacks what was imported since. +func TestABuildOffTheTrunksHeadSaysNoBuildSource(t *testing.T) { got, _, _ := buildTheProxy(t, aSharedRepository("one", "1"), true) if len(got.Sources) != 0 { t.Fatalf("a build off the trunk said %+v", got.Sources) } + got, _, _ = buildTheProxy(t, aSharedRepository("one", "1"), false, true) + if len(got.Sources) != 0 { + t.Fatalf("a build of an older trunk commit said %+v", got.Sources) + } +} + +// An archive of the module's whole directory holds every file of it. +func TestAnArchiveOfTheWholeDirectoryHoldsIt(t *testing.T) { + manifest := `{"module":"look","version":"1","build":{"artifacts":[{"name":"all","kind":"archive","from":"."}]}, + "resources":[{"id":"files","type":"archive","path":"/opt/look","artifact":"all"}]}` + r := &recorded{contents: map[string]string{"modules/look/" + ManifestName: manifest, "modules/look/a/b.css": "x"}} + got, err := Build(context.Background(), onTrunk{recorded: r}.run, r, + "https://forge.invalid/catalogue.git", "modules/look", "", t.TempDir(), nil, Npmrc{}, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if len(got.Sources) != 1 || !SourceHolds(got.Sources[0].Paths, "modules/look/a/b.css") || + SourceHolds(got.Sources[0].Paths, "modules/other/x") { + t.Fatalf("sources %+v", got.Sources) + } } // A recipe reading past its build source fails, naming what it could not find — in the real docker build; @@ -128,7 +154,7 @@ func TestAnImageCompilingANonexistentPackageFails(t *testing.T) { "Dockerfile": "FROM scratch\n"}, secondary: map[string]map[string]string{"https://forge.invalid/ctl.git": aSharedRepository("one", "1")}, } - _, err := Build(context.Background(), onTrunk{r, false}.run, r, + _, err := Build(context.Background(), onTrunk{recorded: r}.run, r, "https://forge.invalid/catalogue.git", "", "", t.TempDir(), nil, Npmrc{}, GitCredential{}, nil) if err == nil || !strings.Contains(err.Error(), "nowhere") { t.Fatalf("a package that is not there built: %v", err) @@ -147,7 +173,7 @@ func TestAGoBundleSaysItsClosureAndAnUntoldImageNothing(t *testing.T) { r.contents[k] = v } held := map[string]string{"mesh-tools-go/build": "registry.invalid/mesh-tools-go/build@sha256:" + strings.Repeat("b", 64)} - got, err := Build(context.Background(), onTrunk{r, false}.run, r, + got, err := Build(context.Background(), onTrunk{recorded: r}.run, r, "https://forge.invalid/ctl.git", "", "", t.TempDir(), held, Npmrc{}, GitCredential{}, nil) if err != nil { t.Fatal(err) @@ -165,7 +191,7 @@ func TestAGoBundleSaysItsClosureAndAnUntoldImageNothing(t *testing.T) { untold := `{"module":"ctl","version":"1","build":{"artifacts":[ {"name":"server","kind":"image","from":"Dockerfile"}]}}` r = &recorded{contents: map[string]string{ManifestName: untold, "Dockerfile": "FROM scratch\n"}} - got, err = Build(context.Background(), onTrunk{r, false}.run, r, + got, err = Build(context.Background(), onTrunk{recorded: r}.run, r, "https://forge.invalid/ctl.git", "", "", t.TempDir(), nil, Npmrc{}, GitCredential{}, nil) if err != nil { t.Fatal(err) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 098ab18b..2c38e005 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -320,10 +320,11 @@ func build(ctx context.Context, run Runner, publish Publisher, if fingerprint == "" { say("source", "no source fingerprint: %s", orNoTree(src.unpinned)) } - // **Only a trunk build says its build source** (novox/hq ADR 0267): the planner maps the next merge onto - // the build source of the trunk's last build, and a branch's closure is not the trunk's. + // **Only a build of the trunk's head says its build source** (novox/hq ADR 0267): the planner maps the + // next merge onto the build source of the newest build, and a branch's closure — or an older trunk + // commit's, built by hand — is not the trunk's. Nor does a build whose context was not its trunk's head. var sources []BuildSource - if trunk != "" && onTrunk { + if trunk != "" && onTrunk && src.contextsAtHead && atTrunkHead(ctx, run, tree, commit, trunk) { sources = src.buildSources() for _, s := range sources { where := "its own repository" @@ -380,6 +381,28 @@ func trunkOf(ctx context.Context, run Runner, clone, commit string) (string, boo return trunk, err == nil } +// atTrunkHead is whether a clone's commit is its trunk's head as the clone holds it. +func atTrunkHead(ctx context.Context, run Runner, clone, commit, trunk string) bool { + head, err := run(ctx, clone, "git", "rev-parse", "origin/"+trunk) + if err != nil { + return false + } + at, err := run(ctx, clone, "git", "rev-parse", commit) + if err != nil { + return false + } + return strings.TrimSpace(head) != "" && strings.TrimSpace(head) == strings.TrimSpace(at) +} + +// cleanEntry is a path of the module's directory as an entry: cleaned, relative, `.` for the directory. +func cleanEntry(p string) string { + c := strings.Trim(path.Clean("/"+filepath.ToSlash(p)), "/") + if c == "" { + return "." + } + return c +} + // orNoTree is why a build has no source fingerprint, for its log. func orNoTree(why string) string { if why == "" { @@ -684,6 +707,9 @@ func one(ctx context.Context, run Runner, publish Publisher, src.contexts[a.Name] = t } buildDir = cloned + if t, _ := trunkOf(ctx, run, cloned, "HEAD"); t == "" || !atTrunkHead(ctx, run, cloned, "HEAD", t) { + src.contextOffHead() + } } // docker build accepts -f outside the context it is given; the recipe stays exactly where it was // read from and validated against, absolute so a context elsewhere does not change which file @@ -722,7 +748,7 @@ func one(ctx context.Context, run Runner, publish Publisher, src.readWhole(*a.Context) } if a.Compiles != "" || a.Context != nil { - src.ownHas(a.From) + src.ownHas(cleanEntry(a.From)) } else { src.ownWhole() } @@ -844,7 +870,7 @@ func one(ctx context.Context, run Runner, publish Publisher, return catalogue.Built{Name: a.Name, Kind: a.Kind, Reference: reference}, nil case catalogue.ArtifactArchive: - src.ownHas(strings.Trim(path.Clean("/"+filepath.ToSlash(a.From)), "/") + "/**") + src.ownHas(cleanEntry(a.From) + "/**") body, err := pack(filepath.Join(tree, a.From)) if err != nil { return catalogue.Built{}, fmt.Errorf("%s: packing %s failed: %w", module, a.Name, err) diff --git a/internal/builder/gosource.go b/internal/builder/gosource.go index c7015887..80d363cb 100644 --- a/internal/builder/gosource.go +++ b/internal/builder/gosource.go @@ -185,6 +185,10 @@ func GoBuildSource(root, pkg string) ([]string, error) { } seen[dir] = true entries[goPackageDirEntry(dir)] = true + // A go.mod made between the module's root and a package moves that package out of the module. + for up := dir; up != modRoot && up != "." && up != "/" && !strings.HasPrefix(up, "../"); up = path.Dir(up) { + file(up, "go.mod") + } listing, err := os.ReadDir(filepath.Join(root, filepath.FromSlash(dir))) if err != nil { // A package that is not there fails the build that imports it; its directory is held, so @@ -193,6 +197,18 @@ func GoBuildSource(root, pkg string) ([]string, error) { } for _, f := range listing { name := f.Name() + // **C and assembly beside Go** may include files from below the package's directory: the + // directory is held whole, and an include reaching above it is refused. + if !f.IsDir() && nativeSource(name) { + entries[strings.TrimPrefix(dir+"/**", "./")] = true + if dir == "." { + entries["**"] = true + } + if err := includesStayWithin(filepath.Join(root, filepath.FromSlash(dir), name)); err != nil { + return nil, fmt.Errorf("%s: %w", path.Join(dir, name), err) + } + continue + } if f.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { continue } @@ -359,6 +375,31 @@ func goFileReads(file string) (imports, embeds []string, err error) { return imports, embeds, scanner.Err() } +// nativeSource is a file the Go command compiles or links beside Go: C, C++, Objective-C, Fortran, +// assembly, their headers and a system object. +func nativeSource(name string) bool { + switch strings.ToLower(path.Ext(name)) { + case ".c", ".h", ".cc", ".cpp", ".cxx", ".hh", ".hpp", ".hxx", ".m", ".s", ".sx", ".f", ".f90", ".for", ".syso": + return true + } + return false +} + +// includesStayWithin refuses a native source whose #include names a path above its directory. +func includesStayWithin(file string) error { + body, err := os.ReadFile(file) + if err != nil { + return err + } + for _, line := range strings.Split(string(body), "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "#") && strings.Contains(line, "include") && strings.Contains(line, "..") { + return errors.New("it includes a path outside its directory: " + line) + } + } + return nil +} + // embedPatterns splits a //go:embed line's patterns: separated by spaces, each possibly quoted. func embedPatterns(s string) ([]string, error) { var out []string diff --git a/internal/builder/gosource_test.go b/internal/builder/gosource_test.go index d51394fd..034ed4f9 100644 --- a/internal/builder/gosource_test.go +++ b/internal/builder/gosource_test.go @@ -47,7 +47,7 @@ var aProgram = map[string]string{ "lib/lib.go": "package lib\n\nconst X = 1\n", "lib/lib_plan9.go": "//go:build plan9\n\npackage lib\n\nimport _ \"example.com/fix/plan9only\"\n", "lib/lib_test.go": "package lib\n\nimport _ \"example.com/fix/testonly\"\n", - "lib/lib_amd64.s": "", + "emb/emb_amd64.s": "", "lib/sub/sub.go": "package sub\n", "plan9only/p.go": "package plan9only\n", "testonly/t.go": "package testonly\n", @@ -74,7 +74,7 @@ func TestAGoProgramsBuildSourceIsItsImportClosure(t *testing.T) { {"cmd/prog/main.go", true, "the program itself"}, {"cmd/prog/helper.go", true, "a file added to the program's package"}, {"lib/lib.go", true, "a package it imports"}, - {"lib/lib_amd64.s", true, "assembly beside a package's Go"}, + {"emb/emb_amd64.s", true, "assembly beside a package's Go"}, {"lib/lib_plan9.go", true, "a file one system builds"}, {"plan9only/p.go", true, "what a file one system builds imports"}, {"emb/static/index.html", true, "an embedded file"}, @@ -172,6 +172,8 @@ func TestABuildSourceThatCannotBeReadIsRefused(t *testing.T) { "go.mod": "module x\n\nreplace y => ../y\n", "p/main.go": "package main\n\nimport _ \"y\"\n"}, "p", "outside"}, {"a cgo header outside the directory", map[string]string{ "go.mod": "module x\n", "p/main.go": "package main\n\n// #include \"../h/h.h\"\nimport \"C\"\n"}, "p", "cgo"}, + {"an assembly include above its directory", map[string]string{"go.mod": "module x\n", "p/main.go": "package main\n", + "p/a_amd64.s": "#include \"../h/textflag.h\"\n"}, "p", "outside"}, {"a file that does not parse", map[string]string{"go.mod": "module x\n", "p/main.go": "package main\n\nimport (\n"}, "p", "main.go"}, {"a package that leaves the tree", map[string]string{"go.mod": "module x\n"}, "../elsewhere", "leaves"}, @@ -286,3 +288,19 @@ func TestThisRepositorysProgramsHaveBuildSourcesOfTheirOwn(t *testing.T) { } } } + +// C or assembly beside Go holds its package's directory whole — an include may name a file below it — and a +// go.mod made between the module's root and a package is a change. +func TestNativeSourcesHoldTheirDirectoryWhole(t *testing.T) { + got, err := GoBuildSource(aGoTree(t, map[string]string{"go.mod": "module x\n", "cmd/p/main.go": "package main\n\nimport _ \"x/lib/asm\"\n", + "lib/asm/a.go": "package asm\n", "lib/asm/a_amd64.s": "#include \"inc/textflag.h\"\n", "lib/asm/inc/textflag.h": ""}), "cmd/p") + if err != nil { + t.Fatal(err) + } + for file, want := range map[string]bool{"lib/asm/inc/textflag.h": true, "lib/asm/a_amd64.s": true, "lib/go.mod": true, + "lib/asm/go.mod": true, "cmd/go.mod": true, "other/go.mod": false} { + if SourceHolds(got, file) != want { + t.Errorf("%s: held %v, wanted %v (%v)", file, !want, want, got) + } + } +} diff --git a/internal/builder/source.go b/internal/builder/source.go index a5f54beb..7f14e9fe 100644 --- a/internal/builder/source.go +++ b/internal/builder/source.go @@ -61,11 +61,22 @@ type sourceInputs struct { // read is, per context (repository and ref), the build source read there; nil for one read whole. read map[string]map[string]bool readAs map[string]catalogue.ArtifactContext + // contextsAtHead is false once a context was cloned at something other than its trunk's head: its + // closure is not the one the next merge there meets, and the build says no build source. + contextsAtHead bool +} + +// contextOffHead says a context was not its trunk's head. +func (s *sourceInputs) contextOffHead() { + if s != nil { + s.contextsAtHead = false + } } func newSourceInputs(module string) *sourceInputs { return &sourceInputs{module: module, contexts: map[string]string{}, toolchains: map[string]string{}, - own: map[string]bool{}, read: map[string]map[string]bool{}, readAs: map[string]catalogue.ArtifactContext{}} + own: map[string]bool{}, read: map[string]map[string]bool{}, readAs: map[string]catalogue.ArtifactContext{}, + contextsAtHead: true} } // ownHas adds entries, relative to the module's directory, to its build source in its own repository. @@ -156,6 +167,15 @@ func (s *sourceInputs) buildSources() []BuildSource { // withPrefix is an entry relative to the module's directory made relative to its repository's root. func withPrefix(prefix, entry string) string { + switch { + case entry == "." || entry == "./": + entry = "" + case entry == "**" || entry == "./**" || entry == "/**": + if prefix == "" { + return "**" + } + return prefix + "/**" + } entry = strings.TrimPrefix(entry, "./") if prefix == "" { if entry == "" {