Review round 1 for the Node build module: registry for other @novox packages and refused bundles (1489), toolchain.reported stream (1483, 1488), re-pin to mesh-sdk 36d6f774; delta part 4 of 4

This commit is contained in:
2026-10-20 21:12:10 +00:00
parent 4a1804cda3
commit 00fa5e74d0
9 changed files with 210 additions and 31 deletions
@@ -45,6 +45,15 @@ type FetchResult struct {
// Modules are the TypeScript modules' directories found, and Installed those whose own packages were installed.
Modules []string `json:"modules"`
Installed []string `json:"installed"`
// Refused are the bundles whose packages were not installed, each with why (a module naming packages beyond the SDK
// with no package-lock.json). The fetch passes; a build of such a bundle fails naming it (issue 1459).
Refused []Refusal `json:"refused"`
}
// Refusal is a module a fetch did not install the packages of.
type Refusal struct {
Module string `json:"module"`
Reason string `json:"reason"`
}
func (m *Module) fetchTool() StepTool {
@@ -93,13 +102,16 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) {
if err != nil {
return nil, err
}
res := FetchResult{Workspace: id, Commit: commit, Modules: []string{}, Installed: []string{}}
res := FetchResult{Workspace: id, Commit: commit, Modules: []string{}, Installed: []string{}, Refused: []Refusal{}}
for _, d := range dirs {
res.Modules = append(res.Modules, d)
did, err := m.installOwn(ctx, root, filepath.Join(tree, filepath.FromSlash(d)), d)
did, refused, err := m.installOwn(ctx, root, filepath.Join(tree, filepath.FromSlash(d)), d)
if err != nil {
return nil, err
}
if refused != "" {
res.Refused = append(res.Refused, Refusal{Module: d, Reason: refused})
}
if did {
res.Installed = append(res.Installed, d)
}
@@ -165,27 +177,45 @@ func ownDependencies(dir string) (names []string, pkg map[string]any, err error)
return names, pkg, nil
}
// installOwn installs a module's own packages into its directory, or does nothing for a module that has none.
func (m *Module) installOwn(ctx context.Context, root, dir, rel string) (bool, error) {
// lockProblem says why a module's own packages cannot be installed: it names packages beyond the SDK and has no
// package-lock.json. Empty when it names none or has its lock.
func lockProblem(dir, rel string) (string, error) {
names, _, err := ownDependencies(dir)
if err != nil || len(names) == 0 {
return "", err
}
if _, err := os.Stat(filepath.Join(dir, "package-lock.json")); err != nil {
return fmt.Sprintf("%s names packages beyond the SDK and has no package-lock.json (%s): commit one", rel, strings.Join(names, ", ")), nil
}
return "", nil
}
// installOwn installs a module's own packages into its directory, or does nothing for a module that has none. A module
// that names packages and has no lock is not installed and not an error: the reason is answered, the fetch passes and
// the build of that bundle fails naming it.
func (m *Module) installOwn(ctx context.Context, root, dir, rel string) (installed bool, refused string, err error) {
names, pkg, err := ownDependencies(dir)
if err != nil || len(names) == 0 {
return false, err
return false, "", err
}
for _, n := range names {
if strings.HasPrefix(n, "@novox/") {
return false, fmt.Errorf("%s depends on %s, and a fetch holds no credential for the mesh's package registry "+
"(ADR 0466 §4), so it cannot be installed", rel, n)
}
if why, err := lockProblem(dir, rel); why != "" || err != nil {
return false, why, err
}
lock, err := os.ReadFile(filepath.Join(dir, "package-lock.json"))
if err != nil {
return false, fmt.Errorf("%s names packages beyond the SDK (%s) and has no package-lock.json: commit one, and "+
"the step installs exactly what it names", rel, strings.Join(names, ", "))
return false, "", err
}
if m.Toolchain == nil || m.Toolchain.Registry == nil {
return false, "", fmt.Errorf("the module has no package registry to install %s from", rel)
}
reg, err := m.Toolchain.Registry()
if err != nil {
return false, "", err
}
// A scratch copy, so the module's package.json and lock are never rewritten (dependencies.go installSteps).
scratch := filepath.Join(root, "npm-"+strings.NewReplacer("/", "-", ".", "root").Replace(rel))
if err := os.MkdirAll(scratch, 0o700); err != nil {
return false, err
return false, "", err
}
for _, k := range []string{"dependencies", "peerDependencies", "optionalDependencies"} {
if d, ok := pkg[k].(map[string]any); ok {
@@ -196,28 +226,34 @@ func (m *Module) installOwn(ctx context.Context, root, dir, rel string) (bool, e
delete(pkg, "scripts")
raw, _ := json.Marshal(pkg)
if err := os.WriteFile(filepath.Join(scratch, "package.json"), raw, 0o600); err != nil {
return false, err
return false, "", err
}
if err := os.WriteFile(filepath.Join(scratch, "package-lock.json"), lock, 0o600); err != nil {
return false, err
return false, "", err
}
// The registry is given through a config file of its own, never on the command line, and removed after.
npmrc := filepath.Join(scratch, ".npmrc-install")
if err := os.WriteFile(npmrc, []byte(reg.Npmrc()), 0o600); err != nil {
return false, "", err
}
env := append(m.baseEnv(), "npm_config_cache="+filepath.Join(m.Toolchain.Paths.Cache, "npm"))
r := m.Run(ctx, env, "npm", "ci", "--omit=dev", "--omit=peer", "--ignore-scripts", "--no-bin-links",
"--no-audit", "--no-fund", "--prefix", scratch)
"--no-audit", "--no-fund", "--prefix", scratch, "--userconfig", npmrc)
os.Remove(npmrc)
if why := ranFailed(r); why != "" {
return false, fmt.Errorf("installing the packages of %s (%s): %s", rel, strings.Join(names, ", "), why)
return false, "", fmt.Errorf("installing the packages of %s (%s): %s", rel, strings.Join(names, ", "), why)
}
nm := filepath.Join(scratch, "node_modules")
if err := removeSDKCopies(nm); err != nil {
return false, err
return false, "", err
}
if err := os.RemoveAll(filepath.Join(dir, "node_modules")); err != nil {
return false, err
return false, "", err
}
if err := os.Rename(nm, filepath.Join(dir, "node_modules")); err != nil {
return false, err
return false, "", err
}
return true, os.RemoveAll(scratch)
return true, "", os.RemoveAll(scratch)
}
// removeSDKCopies removes every copy of the SDK the install pulled in: the toolchain's is the pin.
@@ -137,10 +137,74 @@ func TestAModuleNamingLodashWithALockIsInstalledWithoutTheSdkAndWithoutALockFail
t.Errorf("the scratch install is left in the workspace: %v", scratch)
}
delete(files, "modules/m/package-lock.json")
m, _ = fetchMachine(t, files, nil)
if _, err := m.fetch(context.Background(), fetchArgs); err == nil || !strings.Contains(err.Error(), "modules/m") || !strings.Contains(err.Error(), "package-lock.json") {
t.Errorf("without a lock: %v, want the failure naming modules/m", err)
}
// 1489 (the 1459 settlement): a module naming packages with no package-lock.json does not fail the fetch: it passes,
// installs nothing for that module, and its result lists the refused bundle with its reason; a build of that bundle
// fails naming it.
func TestAModuleWithoutALockIsRefusedInTheResultNotTheFetch(t *testing.T) {
files := typeScriptModule(`{"dependencies":{"@novox/mesh-sdk":"^0.1.6","lodash":"^4"}}`)
files["modules/ok/module.json"] = `{"module":"ok","build":{"artifacts":[{"name":"tools","kind":"bundle","language":"typescript","entrypoints":["tools/index.js"]}]}}`
files["modules/ok/package.json"] = `{"dependencies":{"@novox/mesh-sdk":"^0.1.6"}}`
m, f := fetchMachine(t, files, nil)
out, err := m.fetch(context.Background(), fetchArgs)
if err != nil {
t.Fatalf("a module without a lock failed the fetch: %v", err)
}
r := out.(FetchResult)
if len(npmCalls(f)) != 0 || len(r.Installed) != 0 || len(r.Modules) != 2 {
t.Errorf("result = %+v, npm calls %d", r, len(npmCalls(f)))
}
if len(r.Refused) != 1 || r.Refused[0].Module != "modules/m" ||
!strings.Contains(r.Refused[0].Reason, "names packages beyond the SDK and has no package-lock.json") || !strings.Contains(r.Refused[0].Reason, "lodash") {
t.Fatalf("refused = %+v, want modules/m with its reason", r.Refused)
}
// the build of that bundle fails naming it, without running a compiler.
f.calls = nil
_, err = m.build(context.Background(), map[string]any{"workspace": r.Workspace, "module": "modules/m", "artifact": "tools"})
if err == nil || !strings.Contains(err.Error(), "modules/m") || !strings.Contains(err.Error(), "no package-lock.json") || len(f.calls) != 0 {
t.Errorf("build = %v after %d commands, want the refusal naming modules/m and nothing run", err, len(f.calls))
}
}
// 1489 (review 4): a bundle depending on another @novox package is installed, the mesh's registry given to npm ci
// through --userconfig, as the toolchain's install gives it.
func TestAnotherNovoxPackageIsInstalledFromTheRegistryThroughAUserconfig(t *testing.T) {
files := typeScriptModule(`{"dependencies":{"@novox/other":"^1","lodash":"^4"}}`)
files["modules/m/package-lock.json"] = `{}`
m, f := fetchMachine(t, files, map[string]string{"@novox/other/index.js": "o", "lodash/index.js": "l"})
m.Toolchain.Registry = func() (Registry, error) {
return Registry{URL: "https://git.novox.be/api/packages/novox/npm/", Username: "mesh-build-node", Password: "pw"}, nil
}
var npmrc string
inner := f.effect
f.effect = func(c call) {
inner(c)
for i, a := range c.args {
if c.name == "npm" && a == "--userconfig" {
raw, _ := os.ReadFile(c.args[i+1])
npmrc = string(raw)
}
}
}
out, err := m.fetch(context.Background(), fetchArgs)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(npmrc, "@novox:registry=https://git.novox.be/api/packages/novox/npm/") || !strings.Contains(npmrc, ":_auth=") {
t.Errorf("npm ci was given the config %q", npmrc)
}
if strings.Contains(strings.Join(npmCalls(f)[0].args, " "), "pw") {
t.Errorf("the credential is on npm's command line")
}
r := out.(FetchResult)
if _, err := os.Stat(filepath.Join(m.Work, r.Workspace, "tree", "modules", "m", "node_modules", "@novox", "other", "index.js")); err != nil {
t.Errorf("@novox/other was not installed: %v", err)
}
for _, e := range mustReadDir(t, filepath.Join(m.Work, r.Workspace)) {
if strings.HasPrefix(e, "npm-") {
t.Errorf("the scratch install (and its credential file) is left: %s", e)
}
}
}
@@ -71,8 +71,10 @@ func newMachine(t *testing.T) (*Module, *fakeMachine, string) {
home, cache, work := t.TempDir(), t.TempDir(), t.TempDir()
f := &fakeMachine{now: time.Unix(1_000_000, 0), cost: map[string]time.Duration{}, answer: map[string]forgekey.Ran{}}
m := &Module{Node: "ace", Recipe: Recipe{Steps: "1", Node: "v26.10.0", Lock: "lock"},
Toolchain: &Toolchain{Paths: Paths{Home: home, Cache: cache}},
Work: work, Run: f.run, Clock: f.clock, Ctx: context.Background()}
Toolchain: &Toolchain{Paths: Paths{Home: home, Cache: cache}, Registry: func() (Registry, error) {
return Registry{URL: "https://git.novox.be/api/packages/novox/npm/"}, nil
}},
Work: work, Run: f.run, Clock: f.clock, Ctx: context.Background()}
// The toolchain directory as npm ci leaves it, with no bin links.
nm := filepath.Join(m.toolchainDir(), "node_modules")
for _, p := range []string{"typescript/bin/tsc", "esbuild/bin/esbuild", "@esbuild/linux-x64/bin/esbuild",
@@ -43,6 +43,7 @@ func main() {
Probe: ProbeNode,
Toolchain: &Toolchain{Paths: paths, Run: forgekey.ExecRunnerClean(0), Registry: registry},
Toolchains: stdio.State(delivery.ToolchainsState).Put,
Emit: func(t string, b any) error { return stdio.Emit(t, b) },
Steps: stdio.State(delivery.StepsState).Put,
Work: WorkDir,
Run: forgekey.ExecRunnerClean(0),
@@ -36,6 +36,9 @@ type Module struct {
Clock func() time.Time
// Toolchains and Steps are the Put of the states of those names.
Toolchains, Steps buildstep.Put
// Emit tells the bus an event (stdio.Emit); nil emits nothing. The guard emits toolchain.reported after each write of
// the toolchains key, so the indexer, which holds no timer, learns that the bucket exists (issue 1444).
Emit func(eventType string, body any) error
// Ctx is the process's context, which a step runs under (the tool call's is cut at 30 s, the step's is not).
Ctx context.Context
Say func(format string, a ...any)
@@ -51,7 +54,7 @@ func (m *Module) recipe() Recipe {
}
func (m *Module) guard(r Recipe) buildstep.Guard {
return buildstep.Guard{Language: "typescript", Want: r.Node, Recipe: r.Version(), Probe: m.Probe, Node: m.Node, Put: m.Toolchains}
return buildstep.Guard{Language: "typescript", Want: r.Node, Recipe: r.Version(), Probe: m.Probe, Node: m.Node, Put: m.reportingPut()}
}
// Check reads the installed toolchain, writes <node>.typescript of the toolchains state and answers nil, or the
@@ -176,3 +179,21 @@ func pathOrDefault() string {
}
return defaultPath
}
// ToolchainReported is the event the guard emits after it writes the toolchains key.
const ToolchainReported = "toolchain.reported"
// reportingPut is the toolchains Put that then emits toolchain.reported with the node and the language. A write that
// failed emits nothing; an emit that failed is said and does not refuse the step.
func (m *Module) reportingPut() buildstep.Put {
return func(key string, value any) (uint64, error) {
rev, err := m.Toolchains(key, value)
if err != nil || m.Emit == nil {
return rev, err
}
if eerr := m.Emit(ToolchainReported, map[string]string{"node": m.Node, "language": "typescript"}); eerr != nil && m.Say != nil {
m.Say("the event %s could not be emitted: %v", ToolchainReported, eerr)
}
return rev, nil
}
}
@@ -60,6 +60,9 @@ func (m *Module) esbuild() string {
// provide copies the toolchain's packages to <root>/node_modules unless they are there.
func (m *Module) provide(root string) error {
if err := m.writeBin(filepath.Join(root, "bin")); err != nil {
return err
}
dest := filepath.Join(root, "node_modules")
if _, err := os.Stat(dest); err == nil {
return nil
@@ -67,6 +70,52 @@ func (m *Module) provide(root string) error {
return copyPackages(filepath.Join(m.toolchainDir(), "node_modules"), dest)
}
// binDir is the workspace's bin directory: small shell scripts, files and not links, that call the toolchain's tsc and
// esbuild, so a package's scripts (`tsc`, `esbuild` by name) find them. npm ci runs with --no-bin-links, which leaves
// the toolchain no .bin directory of its own.
func binDir(root string) string { return filepath.Join(root, "bin") }
func (m *Module) writeBin(dir string) error {
if err := os.MkdirAll(dir, 0o755); err != nil {
return err
}
for name, body := range map[string]string{
"tsc": "exec node '" + m.tsc() + "' \"$@\"\n",
"esbuild": "exec '" + m.esbuild() + "' \"$@\"\n",
} {
path := filepath.Join(dir, name)
if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body), 0o755); err != nil {
return err
}
if err := os.Chmod(path, 0o755); err != nil {
return err
}
}
return nil
}
// copyTree copies a directory of regular files and directories, keeping permission bits, except a build's output and
// the repository's own .git. Anything else in it (a link) is an error: the mesh makes no symlink.
func copyTree(from, to string) error {
return filepath.WalkDir(from, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
rel, _ := filepath.Rel(from, path)
if d.IsDir() && (rel == ".mesh-build" || rel == ".git") {
return filepath.SkipDir
}
switch {
case d.IsDir():
return os.MkdirAll(filepath.Join(to, rel), 0o755)
case d.Type().IsRegular():
return copyFile(path, filepath.Join(to, rel))
default:
return fmt.Errorf("%s is neither a file nor a directory, and the build copies only those", path)
}
})
}
// copyPackages copies the packages of a node_modules directory except the toolchain's own tools. It copies regular
// files and directories only and keeps their permission bits; anything else in the source is an error.
func copyPackages(from, to string) error {
+1 -1
View File
@@ -2,4 +2,4 @@ module meshbuildnode
go 1.26.0
require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020203945-1572e50e0ef0
require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020204626-36d6f7745c8d
+2 -2
View File
@@ -1,2 +1,2 @@
git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020203945-1572e50e0ef0 h1:mr9HyARwzeZT7FxFHv1Xc2z7QErvDZ/F/pBMIpjGSd0=
git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020203945-1572e50e0ef0/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY=
git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020204626-36d6f7745c8d h1:mmOCJphjQN15jNVh+sMQqnmJ3UiYltiuQRg/zw3wFtw=
git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020204626-36d6f7745c8d/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY=
+6
View File
@@ -18,6 +18,12 @@
"build_node_status",
"build_node_health"
],
"streams": [
{
"name": "toolchains-node",
"events": ["toolchain.**"]
}
],
"requires": [
"forge-account",
"npm-package-registry"