From 4195493c1436dadf46e83296d8a409d272754cae Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 15:38:01 +0100 Subject: [PATCH 1/8] test(cmd/gortex): pin rendered service unit files byte for byte The renderLaunchdPlist and renderSystemdUnit tests check fragments: a key/value pair, an ExecStart= line, the absence of an Environment= line. They would not notice a renderer that moved a newline, changed an indent or dropped the trailing newline, which launchd and systemd would load differently or not at all. Pin both unit files whole, with and without captured environment variables, including the XML escapes in the plist and the systemd quoting and %-escaping of Environment= and log paths. The expected output is what the current text/template rendering produces. --- cmd/gortex/daemon_service_render_test.go | 130 +++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 cmd/gortex/daemon_service_render_test.go diff --git a/cmd/gortex/daemon_service_render_test.go b/cmd/gortex/daemon_service_render_test.go new file mode 100644 index 000000000..1e2ecdc9c --- /dev/null +++ b/cmd/gortex/daemon_service_render_test.go @@ -0,0 +1,130 @@ +package main + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// The render tests in daemon_service_test.go check fragments. These pin +// whole unit files, byte for byte, so a change to how they are rendered +// cannot move a newline, an indent or an escape unnoticed. + +// launchdDefaultPATH is the plist PATH when the installing shell's PATH +// contributes nothing: the launchd defaults, joined with the host's list +// separator. +func launchdDefaultPATH() string { + return strings.Join([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"}, string(os.PathListSeparator)) +} + +func TestRenderLaunchdPlist_Golden(t *testing.T) { + const head = ` + + + + Label + com.zzet.gortex + ProgramArguments + + /opt/a&b/gortex + daemon + start + + RunAtLoad + + KeepAlive + + SuccessfulExit + + + StandardOutPath + /var/o'neil/<logs>/daemon.stderr.log + StandardErrorPath + /var/o'neil/<logs>/daemon.stderr.log + EnvironmentVariables + + PATH + ` + const tail = ` + + + +` + for _, tc := range []struct { + name string + env []serviceEnvVar + vars string + }{ + {name: "no env"}, + { + name: "env", + env: []serviceEnvVar{ + {Key: "XDG_CONFIG_HOME", Value: `/srv/a&b/"config"`}, + {Key: "GORTEX_DAEMON_LOGFILE", Value: "/srv/a&b/daemon.log"}, + }, + vars: ` + XDG_CONFIG_HOME + /srv/a&b/"config" + GORTEX_DAEMON_LOGFILE + /srv/a&b/daemon.log`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("PATH", "") + out, err := renderLaunchdPlist("com.zzet.gortex", "/opt/a&b/gortex", "/var/o'neil//daemon.stderr.log", tc.env) + require.NoError(t, err) + require.Equal(t, head+launchdDefaultPATH()+""+tc.vars+tail, out) + assertWellFormedXML(t, out) + }) + } +} + +func TestRenderSystemdUnit_Golden(t *testing.T) { + const head = `[Unit] +Description=Gortex code intelligence daemon +Documentation=https://github.com/zzet/gortex +After=network.target + +[Service] +Type=simple +ExecStart=/home/u/.local/bin/gortex daemon start` + const tail = ` +Restart=on-failure +RestartSec=2 +StandardOutput=append:/home/u/100%% logs/daemon.stderr.log +StandardError=append:/home/u/100%% logs/daemon.stderr.log + +[Install] +WantedBy=default.target +` + for _, tc := range []struct { + name string + env []serviceEnvVar + vars string + }{ + {name: "no env"}, + { + name: "env", + env: []serviceEnvVar{ + {Key: "XDG_CACHE_HOME", Value: "/home/u/.cache"}, + {Key: "XDG_DATA_HOME", Value: "/home/u/has space"}, + {Key: "XDG_CONFIG_HOME", Value: `/home/u/"q"\x`}, + {Key: "GORTEX_DAEMON_LOGFILE", Value: "/home/u/100% logs/daemon.log"}, + }, + vars: ` +Environment=XDG_CACHE_HOME=/home/u/.cache +Environment="XDG_DATA_HOME=/home/u/has space" +Environment="XDG_CONFIG_HOME=/home/u/\"q\"\\x" +Environment="GORTEX_DAEMON_LOGFILE=/home/u/100%% logs/daemon.log"`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("PATH", "") + out, err := renderSystemdUnit("/home/u/.local/bin/gortex", "/home/u/100% logs/daemon.stderr.log", tc.env) + require.NoError(t, err) + require.Equal(t, head+tc.vars+tail, out) + }) + } +} From 0cb0a7c995945c8c14188c55f917ccbc8654cdde Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 16:05:05 +0100 Subject: [PATCH 2/8] perf(cmd/gortex): render service unit files without text/template renderLaunchdPlist and renderSystemdUnit filled the launchd plist and the systemd unit with text/template. Executing a template evaluates each field reference through reflect.Value.MethodByName with a name known only at run time. The compiler flags such a call, and once one is reachable the linker stops pruning methods: any exported method of any reachable type might be looked up, so it keeps all of them. These two renderers were the only reachable template execution in cmd/gortex. Without them the binary drops from 315,899,314 to 309,820,674 bytes (-6,078,640, -1.92%), nearly all of it methods nothing calls and more than half of the release-size growth since v0.64.10 tracked in #1116. That was measured on a stripped darwin/arm64 build with the linux release's build tags plus -trimpath (-trimpath -tags netgo,osusergo,static_link -ldflags "-s -w -X ..."), not on the linux release binaries. Projected onto those, main would be about 1.70% (linux/amd64) and 1.39% (linux/arm64) larger than v0.64.10; the linux release build has to confirm that. Render with fmt over the same document text instead. The output is byte-identical: the golden tests from the previous commit and the existing render tests pass unchanged, and a one-off differential run against the template renderers over 20,000 generated inputs (XML and systemd metacharacters, percent signs, quotes, backslashes, newlines, template-like text) matched exactly. The functions keep their (string, error) signatures in this commit so callers and tests stay as they are; the error is now always nil. --- cmd/gortex/daemon_service.go | 83 ++++++++++++++---------------------- 1 file changed, 32 insertions(+), 51 deletions(-) diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index 663408f26..786d2eff1 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "encoding/xml" "errors" "fmt" @@ -12,7 +11,6 @@ import ( "runtime" "slices" "strings" - "text/template" "github.com/spf13/cobra" @@ -255,15 +253,18 @@ func runDaemonServiceStatus(cmd *cobra.Command, _ []string) error { // EnvironmentVariables carries the installing shell's PATH with missing // Homebrew / system defaults appended, plus any XDG_* overrides in effect at // install time, and GORTEX_DAEMON_LOGFILE — see servicePath and xdgServiceEnv. +// +// The verbs take, in order: label, executable, log path (twice), PATH, and +// the rendered / lines of the other variables. const launchdPlistTemplate = ` Label - {{.Label}} + %s ProgramArguments - {{.Exe}} + %s daemon start @@ -275,17 +276,13 @@ const launchdPlistTemplate = ` StandardOutPath - {{.LogPath}} + %s StandardErrorPath - {{.LogPath}} + %s EnvironmentVariables PATH - {{.Path}} -{{- range .EnvVars}} - {{.Key}} - {{.Value}} -{{- end}} + %s%s @@ -294,25 +291,18 @@ const launchdPlistTemplate = ` // renderLaunchdPlist fills launchdPlistTemplate. String values are // XML-escaped so a path containing an XML metacharacter can't produce a // malformed, unloadable plist. +// +// Unit files are rendered with fmt, not text/template: executing a template +// looks fields up by reflection, which makes the linker keep every exported +// method of every reachable type (see scripts/ci-dumpdep). func renderLaunchdPlist(label, exe, logPath string, env []serviceEnvVar) (string, error) { - data := struct { - Label, Exe, LogPath, Path string - EnvVars []serviceEnvVar - }{ - Label: xmlEscape(label), - Exe: xmlEscape(exe), - LogPath: xmlEscape(logPath), - Path: xmlEscape(servicePath([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"})), - EnvVars: make([]serviceEnvVar, len(env)), - } - for i, e := range env { - data.EnvVars[i] = serviceEnvVar{Key: e.Key, Value: xmlEscape(e.Value)} - } - var buf bytes.Buffer - if err := template.Must(template.New("plist").Parse(launchdPlistTemplate)).Execute(&buf, data); err != nil { - return "", err + var vars strings.Builder + for _, e := range env { + fmt.Fprintf(&vars, "\n %s\n %s", e.Key, xmlEscape(e.Value)) } - return buf.String(), nil + logPath = xmlEscape(logPath) + path := servicePath([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"}) + return fmt.Sprintf(launchdPlistTemplate, xmlEscape(label), xmlEscape(exe), logPath, logPath, xmlEscape(path), vars.String()), nil } func launchdPlistPath() (string, error) { @@ -625,6 +615,9 @@ func noteRunningDaemon(w io.Writer, daemonRunning func() bool) { // PATH, XDG_* overrides, and GORTEX_DAEMON_LOGFILE at install time so the // daemon resolves the same paths as the installing shell — see servicePath // and xdgServiceEnv. +// +// The verbs take, in order: executable, the rendered Environment= lines, and +// the log path (twice). const systemdUnitTemplate = `[Unit] Description=Gortex code intelligence daemon Documentation=https://github.com/zzet/gortex @@ -632,14 +625,11 @@ After=network.target [Service] Type=simple -ExecStart={{.Exe}} daemon start -{{- range .EnvVars}} -Environment={{.}} -{{- end}} +ExecStart=%s daemon start%s Restart=on-failure RestartSec=2 -StandardOutput=append:{{.LogPath}} -StandardError=append:{{.LogPath}} +StandardOutput=append:%s +StandardError=append:%s [Install] WantedBy=default.target @@ -651,24 +641,15 @@ func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) (string, error) if path := servicePath(nil); path != "" { env = append([]serviceEnvVar{{Key: "PATH", Value: path}}, env...) } - data := struct { - Exe, LogPath string - EnvVars []string - }{ - Exe: exe, - // systemd expands % specifiers in StandardOutput= and StandardError= - // paths too, so escape them as systemdEnvValue does. - LogPath: strings.ReplaceAll(logPath, "%", "%%"), - EnvVars: make([]string, len(env)), - } - for i, e := range env { - data.EnvVars[i] = systemdEnvValue(e.Key + "=" + e.Value) - } - var buf bytes.Buffer - if err := template.Must(template.New("unit").Parse(systemdUnitTemplate)).Execute(&buf, data); err != nil { - return "", err + var vars strings.Builder + for _, e := range env { + vars.WriteString("\nEnvironment=") + vars.WriteString(systemdEnvValue(e.Key + "=" + e.Value)) } - return buf.String(), nil + // systemd expands % specifiers in StandardOutput= and StandardError= + // paths too, so escape them as systemdEnvValue does. + logPath = strings.ReplaceAll(logPath, "%", "%%") + return fmt.Sprintf(systemdUnitTemplate, exe, vars.String(), logPath, logPath), nil } func systemdUnitPath() (string, error) { From 8a77b1abfc6656ffb56a8a1ba0ee0806829419b6 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 16:05:11 +0100 Subject: [PATCH 3/8] ci: fail when a reachable reflect method lookup stops method pruning One reachable call to reflect.Value.Method, or to MethodByName with a non-constant name, makes the linker keep every exported method of every reachable type. Executing a text/template or html/template makes that call. Nothing in a diff shows the cost: the template rendering removed in the previous commit held about 6 MB of the stripped binary. scripts/ci-dumpdep reads the reachability listing a -dumpdep link prints and fails when a reachable symbol carries the linker's flag, or when text/template's executor is reachable (Execute is usually inlined into its caller, so the executor is what the listing names). For each one it prints the chain of symbols that reached it, ending at the first-party caller. It fails closed: input without a reachable main.main is rejected, with its first lines echoed, and so is a listing in which no symbol carries the flag every real program has. A failed or skipped link, or a listing format the tool cannot read, cannot pass as clean. The build-linux-static job now relinks the release build with -dumpdep and pipes the listing through the tool under pipefail. Compilation is cached from the job's build step, so the step costs one link; locally a darwin/arm64 relink with -dumpdep took 3-5 s and printed ~36 MB. build-native does the same on macOS and Windows, linked the way release.yml links those binaries: no build tags, and on Windows the mingw runtime linked in statically. A wiring test keeps the step in both jobs. On the parent of the previous commit the check reports 10 symbols, starting with text/template.(*Template).execute reached through runDaemonInstallService -> installLaunchd -> renderLaunchdPlist; on this tree it passes. --- .github/workflows/ci.yml | 28 ++++++ docs/ci-testing.md | 5 +- scripts/ci-dumpdep/main.go | 148 ++++++++++++++++++++++++++++++++ scripts/ci-dumpdep/main_test.go | 137 +++++++++++++++++++++++++++++ 4 files changed, 316 insertions(+), 2 deletions(-) create mode 100644 scripts/ci-dumpdep/main.go create mode 100644 scripts/ci-dumpdep/main_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6089f7081..b6b146378 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,6 +131,18 @@ jobs: with: go-version-file: go.mod - run: go build -o gortex ./cmd/gortex/ + # The unused-method check from build-linux-static, linked the way + # release.yml links these platforms' binaries: no build tags, and on + # windows the mingw runtime linked in statically. Compilation is cached + # from the build above, so this costs one more link. + - name: Keep unused methods prunable + shell: bash + run: | + set -o pipefail + extld= + if [ "$RUNNER_OS" = Windows ]; then extld=-extldflags=-static; fi + go build -ldflags "-s -w $extld -dumpdep" \ + -o "$RUNNER_TEMP/gortex-dumpdep" ./cmd/gortex/ 2>&1 | go run ./scripts/ci-dumpdep smoke: needs: plan @@ -183,6 +195,22 @@ jobs: file gortex-static sh scripts/verify-static-elf.sh gortex-static + # Executing a text/template or html/template, like any other reflect + # method lookup by a run-time name, makes the linker keep every exported + # method of every reachable type: ~6 MB of the stripped binary when the + # service unit files were rendered with text/template. Relink (the + # compile above is cached) with -dumpdep, which lists what the linker + # keeps and why; scripts/ci-dumpdep fails with the call chain that + # reaches such a lookup. + - name: Keep unused methods prunable + env: + CGO_ENABLED: "1" + run: | + set -o pipefail + go build -tags netgo,osusergo,static_link \ + -ldflags '-s -w -extldflags "-static" -dumpdep' \ + -o /dev/null ./cmd/gortex/ 2>&1 | go run ./scripts/ci-dumpdep + # The static-only code path (custom grammars declining instead of # dlopen'ing) is compiled out of every other job, so it needs its tests # run under the tag or they never execute anywhere. diff --git a/docs/ci-testing.md b/docs/ci-testing.md index d3304050b..9ac4ec869 100644 --- a/docs/ci-testing.md +++ b/docs/ci-testing.md @@ -93,8 +93,9 @@ A small Linux race smoke uses the existing production wiring tests `TestDaemon_EndToEnd_ToolHandlerCanReturnItsDispatchSlot` and `TestComposeWiresTheLeaseManagerIntoTheRESTSurface` to supplement imports with runtime handler/transport coverage. All-OS default builds remain enabled (Linux -in the existing `build-onnx` job), along with static/tagged builds, complete lint, -module tidiness, CGO flag checks, benchmarks and the separate security workflows. +in the existing `build-onnx` job), along with static/tagged builds, a link check +that unused methods stay prunable (`scripts/ci-dumpdep`), complete lint, module +tidiness, CGO flag checks, benchmarks and the separate security workflows. Only full-suite Linux coverage goes to Codecov. Affected-run coverage is kept in explicitly named `affected-coverage-*` artifacts and is never presented as diff --git a/scripts/ci-dumpdep/main.go b/scripts/ci-dumpdep/main.go new file mode 100644 index 000000000..db13316b7 --- /dev/null +++ b/scripts/ci-dumpdep/main.go @@ -0,0 +1,148 @@ +// ci-dumpdep fails when a binary links code that makes the Go linker keep +// every exported method of every reachable type. +// +// The linker's dead-code pass drops methods that nothing can call. A call to +// reflect.Value.Method, or to MethodByName with a name that is not a +// compile-time constant, defeats it: the linker cannot know which method will +// be asked for, so it keeps all of them. Executing a text/template or +// html/template makes exactly that call to evaluate fields. Rendering the +// daemon's service unit files with text/template once kept about 6 MB of +// otherwise unused methods in the stripped gortex binary. +// +// It reads the reachability listing a -dumpdep link prints: +// +// go build -ldflags=-dumpdep -o /dev/null ./cmd/gortex/ 2>&1 | go run ./scripts/ci-dumpdep +// +// Each "parent -> symbol" line marks symbol reachable from parent. The linker +// appends to a function the compiler flagged as such a +// method lookup, and to a type converted to an interface. +// Every real program has such types, so a listing without that flag is not +// one this tool can read: it is rejected rather than reported clean. +package main + +import ( + "bufio" + "errors" + "fmt" + "io" + "os" + "strings" +) + +const ( + reflectMethodFlag = "" + usedInIfaceFlag = "" +) + +// templateExecution is the function every text/template and html/template +// execution runs through; Execute and ExecuteTemplate are wrappers the +// compiler may inline into their caller. It always reaches the flagged +// lookup, and reporting it puts the caller that executes a template at the +// end of the chain rather than template internals. +const templateExecution = "text/template.(*Template).execute" + +// finding is a reachable symbol that keeps all exported methods, with the +// chain of symbols that reached it, root first. +type finding struct { + symbol string + path []string +} + +// symbolName drops the attribute flags -dumpdep appends to a symbol. +func symbolName(s string) string { + for _, flag := range []string{reflectMethodFlag, usedInIfaceFlag} { + s = strings.TrimSuffix(s, flag) + } + return strings.TrimSuffix(s, " ") +} + +// maxOtherLines bounds how much non-listing input, such as a build failure, +// is echoed back when the input turns out not to be a link listing. +const maxOtherLines = 100 + +// check reads a -dumpdep listing and returns the reachable symbols that make +// the linker keep every exported method. Lines that are not dependency edges, +// such as the package header go build prints, are skipped. +func check(r io.Reader) ([]finding, error) { + parent := map[string]string{} + var offenders, other []string + flagged := 0 + scanner := bufio.NewScanner(r) + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for scanner.Scan() { + from, to, ok := strings.Cut(scanner.Text(), " -> ") + if !ok { + if len(other) < maxOtherLines { + other = append(other, scanner.Text()) + } + continue + } + if strings.HasSuffix(to, usedInIfaceFlag) { + flagged++ + } + name := symbolName(to) + if _, seen := parent[name]; !seen { + parent[name] = symbolName(from) + } + if strings.HasSuffix(to, reflectMethodFlag) || name == templateExecution { + offenders = append(offenders, name) + } + } + if err := scanner.Err(); err != nil { + return nil, err + } + if _, ok := parent["main.main"]; !ok { + msg := "main.main is not in the input; pipe in the output of a -dumpdep link of a command" + if len(other) > 0 { + msg += ". The input began:\n" + strings.Join(other, "\n") + } + return nil, errors.New(msg) + } + // Without the attribute flags a could not be seen + // either, so a listing without them must not pass as clean. + if flagged == 0 { + return nil, errors.New("no symbol carries the " + usedInIfaceFlag + " flag, so this is not a -dumpdep listing this tool can read") + } + findings := make([]finding, 0, len(offenders)) + for _, name := range offenders { + path := []string{name} + for at := name; len(path) <= len(parent); { + from, ok := parent[at] + if !ok || from == "_" { + break + } + path = append(path, from) + at = from + } + for i, j := 0, len(path)-1; i < j; i, j = i+1, j-1 { + path[i], path[j] = path[j], path[i] + } + findings = append(findings, finding{symbol: name, path: path}) + } + return findings, nil +} + +func run(stdin io.Reader, stdout, stderr io.Writer) int { + findings, err := check(stdin) + if err != nil { + fmt.Fprintf(stderr, "ci-dumpdep: %v\n", err) + return 1 + } + if len(findings) == 0 { + fmt.Fprintln(stdout, "No reachable dynamic method lookup: the linker can drop unused methods.") + return 0 + } + fmt.Fprintf(stderr, "ci-dumpdep: %d reachable symbols make the linker keep every exported method of every reachable type.\n", len(findings)) + fmt.Fprintln(stderr, "Render text with fmt or strings rather than text/template or html/template, and look methods up by constant name.") + for _, f := range findings { + fmt.Fprintf(stderr, "\n%s, reached by:\n", f.symbol) + for _, hop := range f.path { + fmt.Fprintf(stderr, " %s\n", hop) + } + } + return 1 +} + +func main() { + os.Exit(run(os.Stdin, os.Stdout, os.Stderr)) +} diff --git a/scripts/ci-dumpdep/main_test.go b/scripts/ci-dumpdep/main_test.go new file mode 100644 index 000000000..2214a3da2 --- /dev/null +++ b/scripts/ci-dumpdep/main_test.go @@ -0,0 +1,137 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// prunedLink is an excerpt of a -dumpdep link in which the linker can still +// drop unused methods. MethodByName with a constant name is fine: the compiler +// records the name, so only matching methods are kept. +const prunedLink = `# github.com/zzet/gortex/cmd/gortex +_ -> runtime.main +_ -> main.main +main.main -> main.runDaemonInstallService +main.runDaemonInstallService -> main.renderSystemdUnit +main.renderSystemdUnit -> fmt.Fprintf +main.renderSystemdUnit -> type:main.serviceEnvVar +google.golang.org/protobuf/internal/descfmt.formatListOpt -> reflect.Value.MethodByName +` + +func runDumpdep(t *testing.T, input string) (int, string, string) { + t.Helper() + var stdout, stderr bytes.Buffer + code := run(strings.NewReader(input), &stdout, &stderr) + return code, stdout.String(), stderr.String() +} + +func TestDumpdepAcceptsALinkThatPrunesMethods(t *testing.T) { + code, stdout, stderr := runDumpdep(t, prunedLink) + if code != 0 || stderr != "" || !strings.Contains(stdout, "the linker can drop unused methods") { + t.Fatalf("clean link rejected: exit=%d\nstdout=%s\nstderr=%s", code, stdout, stderr) + } +} + +func TestDumpdepRejectsReachableMethodLookups(t *testing.T) { + input := prunedLink + `main.main -> main.installLaunchd +main.installLaunchd -> main.renderLaunchdPlist +main.renderLaunchdPlist -> text/template.(*Template).execute +text/template.(*Template).execute -> text/template.(*state).walk +text/template.(*state).walk -> text/template.(*state).evalField +text/template.(*state).evalField -> reflect.Value.MethodByName +type:reflect.Value -> reflect.(*Value).Method +` + code, stdout, stderr := runDumpdep(t, input) + if code == 0 || stdout != "" { + t.Fatalf("reachable method lookups accepted: exit=%d\nstdout=%s\nstderr=%s", code, stdout, stderr) + } + for _, want := range []string{ + "ci-dumpdep: 3 reachable symbols", + // Execute is usually inlined, so its caller reaches execute directly + // and the chain shows which first-party code runs a template. + "\ntext/template.(*Template).execute, reached by:\n" + + " main.main\n main.installLaunchd\n main.renderLaunchdPlist\n text/template.(*Template).execute\n", + "\ntext/template.(*state).evalField, reached by:\n" + + " main.main\n main.installLaunchd\n main.renderLaunchdPlist\n text/template.(*Template).execute\n" + + " text/template.(*state).walk\n text/template.(*state).evalField\n", + "\nreflect.(*Value).Method, reached by:\n type:reflect.Value\n reflect.(*Value).Method\n", + } { + if !strings.Contains(stderr, want) { + t.Errorf("report is missing %q:\n%s", want, stderr) + } + } + if strings.Contains(stderr, "MethodByName, reached by") { + t.Errorf("the lookup function itself is not flagged; only its dynamic callers are:\n%s", stderr) + } +} + +func TestDumpdepRejectsInputThatIsNotALinkListing(t *testing.T) { + for _, tc := range []struct{ name, input, echoed string }{ + {"empty", "", ""}, + // A failed build must still show why it failed. + {"build failure", "# github.com/zzet/gortex/cmd/gortex\n./main.go:1:1: expected 'package', found 'EOF'\n", + "The input began:\n# github.com/zzet/gortex/cmd/gortex\n./main.go:1:1: expected 'package', found 'EOF'\n"}, + {"library", "_ -> runtime.main\n", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + code, stdout, stderr := runDumpdep(t, tc.input) + if code == 0 || stdout != "" || !strings.Contains(stderr, "main.main is not in the input") || + !strings.Contains(stderr, tc.echoed) { + t.Fatalf("non-listing accepted: exit=%d\nstdout=%s\nstderr=%s", code, stdout, stderr) + } + }) + } +} + +// TestDumpdepRejectsAListingWithoutFlags guards against a listing format the +// tool cannot read: if the linker stopped appending attribute flags, a +// would go unseen and every link would pass. +func TestDumpdepRejectsAListingWithoutFlags(t *testing.T) { + input := strings.ReplaceAll(prunedLink, " ", "") + code, stdout, stderr := runDumpdep(t, input) + if code == 0 || stdout != "" || !strings.Contains(stderr, "not a -dumpdep listing") { + t.Fatalf("listing without flags accepted: exit=%d\nstdout=%s\nstderr=%s", code, stdout, stderr) + } +} + +// TestCIWorkflowChecksTheReleaseLink keeps the check wired to a -dumpdep link +// with the release build tags and link flags of each platform, with a +// pipeline that fails when the link does. +func TestCIWorkflowChecksTheReleaseLink(t *testing.T) { + data, err := os.ReadFile(filepath.Join("..", "..", ".github", "workflows", "ci.yml")) + if err != nil { + t.Fatal(err) + } + text := strings.ReplaceAll(string(data), "\r\n", "\n") + for name, wants := range map[string][]string{ + "build-linux-static": { + "go build -tags netgo,osusergo,static_link", + "-dumpdep", + "-o /dev/null ./cmd/gortex/ 2>&1 | go run ./scripts/ci-dumpdep", + }, + // darwin and windows: no tags; windows links its runtime statically. + "build-native": { + "shell: bash", + "extld=-extldflags=-static", + `go build -ldflags "-s -w $extld -dumpdep"`, + "./cmd/gortex/ 2>&1 | go run ./scripts/ci-dumpdep", + }, + } { + _, job, found := strings.Cut(text, "\n "+name+":\n") + if !found { + t.Fatalf("ci.yml has no %s job", name) + } + if next := regexp.MustCompile(`\n [a-z][a-z0-9-]*:\n`).FindStringIndex(job); next != nil { + job = job[:next[0]] + } + for _, want := range append(wants, "set -o pipefail") { + if !strings.Contains(job, want) { + t.Errorf("%s is missing %q", name, want) + } + } + } +} From 42fa91280e2554e1e688f8626fdc394bcc3ebaf8 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 18:15:05 +0100 Subject: [PATCH 4/8] test(cmd/gortex): drop fragment render tests the golden tests cover The NoXDG, PropagatesXDG and EscapesXML tests for the launchd plist and the NoXDG and PropagatesXDG tests for the systemd unit each check a few substrings of a rendered file. The golden tests render the same cases, with and without captured variables, XML escapes, systemd quoting and an empty PATH, and compare the whole file, so every fragment these checked is pinned there too. The PATH capture, empty PATH and daemon log override tests stay: they exercise how the installing environment reaches the renderers, which the golden tests hold fixed (empty PATH, variables passed in directly). --- cmd/gortex/daemon_service_render_test.go | 7 +- cmd/gortex/daemon_service_test.go | 116 ----------------------- 2 files changed, 4 insertions(+), 119 deletions(-) diff --git a/cmd/gortex/daemon_service_render_test.go b/cmd/gortex/daemon_service_render_test.go index 1e2ecdc9c..1c18b3c57 100644 --- a/cmd/gortex/daemon_service_render_test.go +++ b/cmd/gortex/daemon_service_render_test.go @@ -8,9 +8,10 @@ import ( "github.com/stretchr/testify/require" ) -// The render tests in daemon_service_test.go check fragments. These pin -// whole unit files, byte for byte, so a change to how they are rendered -// cannot move a newline, an indent or an escape unnoticed. +// These pin whole unit files, byte for byte, so a change to how they are +// rendered cannot move a newline, an indent or an escape unnoticed. The +// render tests in daemon_service_test.go check how the installing shell's +// PATH and daemon log override reach them. // launchdDefaultPATH is the plist PATH when the installing shell's PATH // contributes nothing: the launchd defaults, joined with the host's list diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index b8fc9a8dd..ca25b039b 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -31,122 +31,6 @@ func assertWellFormedXML(t *testing.T, s string) { } } -// TestRenderLaunchdPlist_NoXDG proves the launchd plist is valid and -// carries the substitutions it must — binary path, log path, label — -// and that with no XDG override captured it has no extra env entries -// (the pre-fix baseline behaviour, unchanged for the common case). -func TestRenderLaunchdPlist_NoXDG(t *testing.T) { - out, err := renderLaunchdPlist( - "com.zzet.gortex", - "/usr/local/bin/gortex", - "/Users/testuser/.gortex/cache/daemon.stderr.log", - nil, - ) - require.NoError(t, err) - - assert.Contains(t, out, "Label\n com.zzet.gortex") - assert.Contains(t, out, "/usr/local/bin/gortex") - assert.Contains(t, out, "daemon") - assert.Contains(t, out, "start") - assert.Contains(t, out, "RunAtLoad") - assert.Contains(t, out, "KeepAlive") - assert.Contains(t, out, "StandardOutPath\n /Users/testuser/.gortex/cache/daemon.stderr.log") - assert.Contains(t, out, "StandardErrorPath\n /Users/testuser/.gortex/cache/daemon.stderr.log") - // Homebrew paths must be on PATH so launchd can find a Homebrew- - // installed gortex binary even when the LaunchAgent env is minimal. - assert.Contains(t, out, "/opt/homebrew/bin") - assert.NotContains(t, out, "XDG_", "no env entries when none captured") - assertWellFormedXML(t, out) -} - -// TestRenderLaunchdPlist_PropagatesXDG is the regression test for the -// service-ignores-XDG bug: when an XDG override is in effect at install -// time it must be baked into the plist so the supervised daemon resolves -// the same paths as the installing shell instead of falling back to -// ~/.gortex. -func TestRenderLaunchdPlist_PropagatesXDG(t *testing.T) { - env := []serviceEnvVar{ - {Key: "XDG_CONFIG_HOME", Value: "/Users/testuser/.config"}, - {Key: "XDG_DATA_HOME", Value: "/Users/testuser/.local/share"}, - } - out, err := renderLaunchdPlist( - "com.zzet.gortex", - "/usr/local/bin/gortex", - "/Users/testuser/.config/gortex/daemon.stderr.log", - env, - ) - require.NoError(t, err) - - assert.Contains(t, out, "XDG_CONFIG_HOME\n /Users/testuser/.config") - assert.Contains(t, out, "XDG_DATA_HOME\n /Users/testuser/.local/share") - // PATH must still be present alongside the captured XDG vars. - assert.Contains(t, out, "PATH") - assertWellFormedXML(t, out) -} - -// TestRenderLaunchdPlist_EscapesXML guards against a home path with an -// XML metacharacter producing a malformed, unloadable plist. -func TestRenderLaunchdPlist_EscapesXML(t *testing.T) { - env := []serviceEnvVar{{Key: "XDG_CONFIG_HOME", Value: "/Users/a&b/.config"}} - out, err := renderLaunchdPlist("com.zzet.gortex", "/opt/a&b/gortex", "/log&path", env) - require.NoError(t, err) - - assert.Contains(t, out, "/Users/a&b/.config") - assert.Contains(t, out, "/opt/a&b/gortex") - assert.NotContains(t, out, "a&b", "raw ampersand must be escaped") - assertWellFormedXML(t, out) -} - -// TestRenderSystemdUnit_NoXDG confirms the Type=simple + Restart=on-failure -// contract and that no Environment= line is emitted when nothing was -// captured. -func TestRenderSystemdUnit_NoXDG(t *testing.T) { - t.Setenv("PATH", "") - out, err := renderSystemdUnit( - "/home/u/.local/bin/gortex", - "/home/u/.gortex/cache/daemon.stderr.log", - nil, - ) - require.NoError(t, err) - - assert.Contains(t, out, "ExecStart=/home/u/.local/bin/gortex daemon start") - assert.Contains(t, out, "Type=simple") - assert.Contains(t, out, "Restart=on-failure") - assert.Contains(t, out, "StandardOutput=append:/home/u/.gortex/cache/daemon.stderr.log") - assert.Contains(t, out, "StandardError=append:/home/u/.gortex/cache/daemon.stderr.log") - assert.Contains(t, out, "WantedBy=default.target") - assert.NotContains(t, out, "Environment=", "no env line when none captured") -} - -// TestRenderSystemdUnit_PropagatesXDG is the Linux analog of the launchd -// regression test, and also covers systemd quoting for a value with a -// space. -func TestRenderSystemdUnit_PropagatesXDG(t *testing.T) { - env := []serviceEnvVar{ - {Key: "XDG_CACHE_HOME", Value: "/home/u/.cache"}, - {Key: "XDG_DATA_HOME", Value: "/home/u/has space"}, - } - out, err := renderSystemdUnit( - "/home/u/.local/bin/gortex", - "/home/u/.cache/gortex/daemon.stderr.log", - env, - ) - require.NoError(t, err) - - assert.Contains(t, out, "Environment=XDG_CACHE_HOME=/home/u/.cache") - // An assignment containing whitespace is double-quoted as a whole, - // because systemd recognizes a quote only at the start of an item. - assert.Contains(t, out, `Environment="XDG_DATA_HOME=/home/u/has space"`) - - // Environment lines must sit inside [Service], ahead of [Install]. - svcStart := strings.Index(out, "[Service]") - installStart := strings.Index(out, "[Install]") - require.Greater(t, svcStart, -1) - require.Greater(t, installStart, svcStart) - svc := out[svcStart:installStart] - assert.Contains(t, svc, "Environment=XDG_CACHE_HOME=") -} - // TestXDGServiceEnv_OnlyAbsoluteSet pins the capture contract: only XDG // vars that are set to an absolute path are propagated. Unset and // relative values are ignored (the latter per the XDG spec, matching From 7b6d86d78f959c966b1dfd3fec90498149924021 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 18:15:26 +0100 Subject: [PATCH 5/8] refactor(cmd/gortex): return the rendered unit files without an error renderLaunchdPlist and renderSystemdUnit returned an error only because executing a template can fail. Rendered with fmt they cannot, so the error was always nil and installLaunchd and installSystemd carried a "render plist"/"render unit" failure branch that never ran. Return the string alone and drop those branches and the NoError checks in the render tests. --- cmd/gortex/daemon_service.go | 18 ++++++------------ cmd/gortex/daemon_service_render_test.go | 6 ++---- cmd/gortex/daemon_service_test.go | 18 ++++++------------ 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index 786d2eff1..152e9aa68 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -295,14 +295,14 @@ const launchdPlistTemplate = ` // Unit files are rendered with fmt, not text/template: executing a template // looks fields up by reflection, which makes the linker keep every exported // method of every reachable type (see scripts/ci-dumpdep). -func renderLaunchdPlist(label, exe, logPath string, env []serviceEnvVar) (string, error) { +func renderLaunchdPlist(label, exe, logPath string, env []serviceEnvVar) string { var vars strings.Builder for _, e := range env { fmt.Fprintf(&vars, "\n %s\n %s", e.Key, xmlEscape(e.Value)) } logPath = xmlEscape(logPath) path := servicePath([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"}) - return fmt.Sprintf(launchdPlistTemplate, xmlEscape(label), xmlEscape(exe), logPath, logPath, xmlEscape(path), vars.String()), nil + return fmt.Sprintf(launchdPlistTemplate, xmlEscape(label), xmlEscape(exe), logPath, logPath, xmlEscape(path), vars.String()) } func launchdPlistPath() (string, error) { @@ -327,10 +327,7 @@ func installLaunchd(w io.Writer, exe string) error { return err } stderrLog := daemon.StderrLogPath(logFile) - plist, err := renderLaunchdPlist(daemonServiceName, exe, stderrLog, xdgServiceEnv(logFile)) - if err != nil { - return fmt.Errorf("render plist: %w", err) - } + plist := renderLaunchdPlist(daemonServiceName, exe, stderrLog, xdgServiceEnv(logFile)) if err := os.WriteFile(path, []byte(plist), 0o644); err != nil { return fmt.Errorf("write plist: %w", err) } @@ -637,7 +634,7 @@ WantedBy=default.target // renderSystemdUnit fills systemdUnitTemplate, quoting Environment= // assignments that need it. -func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) (string, error) { +func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) string { if path := servicePath(nil); path != "" { env = append([]serviceEnvVar{{Key: "PATH", Value: path}}, env...) } @@ -649,7 +646,7 @@ func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) (string, error) // systemd expands % specifiers in StandardOutput= and StandardError= // paths too, so escape them as systemdEnvValue does. logPath = strings.ReplaceAll(logPath, "%", "%%") - return fmt.Sprintf(systemdUnitTemplate, exe, vars.String(), logPath, logPath), nil + return fmt.Sprintf(systemdUnitTemplate, exe, vars.String(), logPath, logPath) } func systemdUnitPath() (string, error) { @@ -674,10 +671,7 @@ func installSystemd(w io.Writer, exe string) error { return err } stderrLog := daemon.StderrLogPath(logFile) - unit, err := renderSystemdUnit(exe, stderrLog, xdgServiceEnv(logFile)) - if err != nil { - return fmt.Errorf("render unit: %w", err) - } + unit := renderSystemdUnit(exe, stderrLog, xdgServiceEnv(logFile)) if err := os.WriteFile(path, []byte(unit), 0o644); err != nil { return fmt.Errorf("write unit: %w", err) } diff --git a/cmd/gortex/daemon_service_render_test.go b/cmd/gortex/daemon_service_render_test.go index 1c18b3c57..dc1997bad 100644 --- a/cmd/gortex/daemon_service_render_test.go +++ b/cmd/gortex/daemon_service_render_test.go @@ -74,8 +74,7 @@ func TestRenderLaunchdPlist_Golden(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Setenv("PATH", "") - out, err := renderLaunchdPlist("com.zzet.gortex", "/opt/a&b/gortex", "/var/o'neil//daemon.stderr.log", tc.env) - require.NoError(t, err) + out := renderLaunchdPlist("com.zzet.gortex", "/opt/a&b/gortex", "/var/o'neil//daemon.stderr.log", tc.env) require.Equal(t, head+launchdDefaultPATH()+""+tc.vars+tail, out) assertWellFormedXML(t, out) }) @@ -123,8 +122,7 @@ Environment="GORTEX_DAEMON_LOGFILE=/home/u/100%% logs/daemon.log"`, } { t.Run(tc.name, func(t *testing.T) { t.Setenv("PATH", "") - out, err := renderSystemdUnit("/home/u/.local/bin/gortex", "/home/u/100% logs/daemon.stderr.log", tc.env) - require.NoError(t, err) + out := renderSystemdUnit("/home/u/.local/bin/gortex", "/home/u/100% logs/daemon.stderr.log", tc.env) require.Equal(t, head+tc.vars+tail, out) }) } diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index ca25b039b..52e72c7dc 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -110,8 +110,7 @@ func TestRenderLaunchdPlist_CapturesPATH(t *testing.T) { custom := filepath.Join(t.TempDir(), "a&b", "100% tools") t.Setenv("PATH", custom) t.Setenv("GORTEX_DAEMON_HTTP_TOKEN", "must-not-be-captured") - out, err := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", "/log", nil) - require.NoError(t, err) + out := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", "/log", nil) want := strings.ReplaceAll(custom, "&", "&") + sep + strings.Join([]string{"/usr/local/bin", "/opt/homebrew/bin", "/usr/bin", "/bin"}, sep) @@ -124,8 +123,7 @@ func TestRenderLaunchdPlist_CapturesPATH(t *testing.T) { func TestRenderLaunchdPlist_EmptyPATH(t *testing.T) { t.Setenv("PATH", "") - out, err := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", "/log", nil) - require.NoError(t, err) + out := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", "/log", nil) // This is exactly the old PATH on launchd's Unix host; adapt only the // path-list separator when these render tests run on Windows. @@ -142,8 +140,7 @@ func TestRenderSystemdUnit_CapturesPATH(t *testing.T) { second := filepath.Join(root, "bin") t.Setenv("PATH", first+sep+second) t.Setenv("GORTEX_DAEMON_HTTP_TOKEN", "must-not-be-captured") - out, err := renderSystemdUnit("/bin/gortex", "/log", nil) - require.NoError(t, err) + out := renderSystemdUnit("/bin/gortex", "/log", nil) // systemd quotes whitespace and escapes backslashes within a quoted // value (relevant to this native fixture when tests run on Windows). @@ -160,8 +157,7 @@ func TestRenderSystemdUnit_EmptyPATH(t *testing.T) { for _, path := range []string{"", ".", string(os.PathListSeparator) + "relative/bin"} { t.Run(path, func(t *testing.T) { t.Setenv("PATH", path) - out, err := renderSystemdUnit("/bin/gortex", "/log", nil) - require.NoError(t, err) + out := renderSystemdUnit("/bin/gortex", "/log", nil) assert.NotContains(t, out, "PATH=") }) } @@ -276,8 +272,7 @@ func TestRenderLaunchdPlist_DaemonLogOverride(t *testing.T) { for _, value := range []string{"", "/custom/a&b/daemon.log"} { t.Run(value, func(t *testing.T) { t.Setenv("GORTEX_DAEMON_LOGFILE", value) - out, err := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", daemon.StderrLogFilePath(), xdgServiceEnv(daemon.LogFilePath())) - require.NoError(t, err) + out := renderLaunchdPlist("com.zzet.gortex", "/bin/gortex", daemon.StderrLogFilePath(), xdgServiceEnv(daemon.LogFilePath())) assertWellFormedXML(t, out) if value == "" { assert.NotContains(t, out, "GORTEX_DAEMON_LOGFILE") @@ -293,8 +288,7 @@ func TestRenderSystemdUnit_DaemonLogOverride(t *testing.T) { for _, value := range []string{"", "/custom/100% logs/daemon.log"} { t.Run(value, func(t *testing.T) { t.Setenv("GORTEX_DAEMON_LOGFILE", value) - out, err := renderSystemdUnit("/bin/gortex", daemon.StderrLogFilePath(), xdgServiceEnv(daemon.LogFilePath())) - require.NoError(t, err) + out := renderSystemdUnit("/bin/gortex", daemon.StderrLogFilePath(), xdgServiceEnv(daemon.LogFilePath())) if value == "" { assert.NotContains(t, out, "GORTEX_DAEMON_LOGFILE") } else { From 6cd72e98e908e47b31a37c6efa6c0e43bac51ae0 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 16:05:19 +0100 Subject: [PATCH 6/8] test(graph): pin StubKind and measure its per-call cost StubKind had no direct tests: a few packages spot-check IsStub on one id each. The resolver calls IsStub on every name-lookup candidate, and a profile of a large cold index put StubKind at 13% of CPU, most of it in runtime.concatstring2 building "::" for each candidate kind. Pin the stub-id grammar in a table (kind as the first or second segment, only when "::" follows it, empty repo prefix, near-miss kinds, third-segment kinds), and check StubKind against a copy of the current implementation on every prefix of generated ids, so a faster version has to agree at each truncation point around the separators. BenchmarkStubKind covers a first-party id (the common candidate), a per-repo stub and a legacy stub. On the current implementation (Apple M5 Pro, GOMAXPROCS=2) they take ~72, ~54 and ~33 ns/op with no heap allocation: the concatenations use a stack buffer, so the cost is the calls, not garbage. --- internal/graph/stub_test.go | 120 ++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 internal/graph/stub_test.go diff --git a/internal/graph/stub_test.go b/internal/graph/stub_test.go new file mode 100644 index 000000000..3e171103b --- /dev/null +++ b/internal/graph/stub_test.go @@ -0,0 +1,120 @@ +package graph + +import ( + "strings" + "testing" +) + +// stubKindByConcatenation is the original StubKind, which built +// k+"::" for every candidate kind on every call. It is the reference +// StubKind must keep agreeing with. +func stubKindByConcatenation(id string) string { + kinds := []string{StubKindStdlib, StubKindExternalCall, StubKindBuiltin, StubKindModule} + for _, k := range kinds { + if strings.HasPrefix(id, k+"::") { + return k + } + } + first := strings.Index(id, "::") + if first < 0 { + return "" + } + rest := id[first+2:] + for _, k := range kinds { + if strings.HasPrefix(rest, k+"::") { + return k + } + } + return "" +} + +// TestStubKind pins the stub-id grammar: a kind counts only as the +// first or second "::"-separated segment, and only when "::" follows it. +func TestStubKind(t *testing.T) { + t.Parallel() + + cases := []struct{ id, kind string }{ + // Legacy form, no repo prefix. + {"stdlib::fmt::Errorf", StubKindStdlib}, + {"external_call::requests.get", StubKindExternalCall}, + {"builtin::go::len", StubKindBuiltin}, + {"module::lodash", StubKindModule}, + // Per-repo form. + {"gortex::stdlib::fmt::Errorf", StubKindStdlib}, + {"gortex::external_call::requests.get", StubKindExternalCall}, + {"tree-sitter-dart::builtin::dart::print", StubKindBuiltin}, + {"gortex::module::lodash", StubKindModule}, + // The kind segment may end the id, and the repo prefix may be empty. + {"stdlib::", StubKindStdlib}, + {"gortex::module::", StubKindModule}, + {"::builtin::go::len", StubKindBuiltin}, + // A leading kind wins over a second-segment one. + {"builtin::stdlib::x", StubKindBuiltin}, + // Not stubs. + {"", ""}, + {"stdlib", ""}, + {"stdlib:", ""}, + {"stdlib:fmt", ""}, + {"gortex::stdlib", ""}, + {"gortex::stdlib:fmt", ""}, + {"Stdlib::fmt", ""}, + {"stdlibs::fmt", ""}, + {"xmodule::lodash", ""}, + {"a::b::stdlib::fmt", ""}, + {"gortex/internal/graph/stub.go::StubKind", ""}, + {"unresolved::Foo", ""}, + } + for _, c := range cases { + if got := StubKind(c.id); got != c.kind { + t.Errorf("StubKind(%q) = %q, want %q", c.id, got, c.kind) + } + if got := IsStub(c.id); got != (c.kind != "") { + t.Errorf("IsStub(%q) = %v, want %v", c.id, got, c.kind != "") + } + } +} + +// TestStubKindMatchesConcatenation checks StubKind against the +// concatenating original on every prefix of ids built from each kind, +// near-miss kinds and repo prefixes, so every truncation point around +// the "::" separators is covered. +func TestStubKindMatchesConcatenation(t *testing.T) { + t.Parallel() + + heads := []string{"", "gortex::", "::", ":", "a::b::", "stdlib::", "gortex/x.go:"} + kinds := []string{ + StubKindStdlib, StubKindExternalCall, StubKindBuiltin, StubKindModule, + "stdli", "stdlibs", "external", "Builtin", "unresolved", + } + tails := []string{"::fmt::Errorf", ":::x", "::", ":", "", "::module::x"} + for _, head := range heads { + for _, kind := range kinds { + for _, tail := range tails { + id := head + kind + tail + for i := 0; i <= len(id); i++ { + if got, want := StubKind(id[:i]), stubKindByConcatenation(id[:i]); got != want { + t.Errorf("StubKind(%q) = %q, want %q", id[:i], got, want) + } + } + } + } + } +} + +// BenchmarkStubKind measures the per-candidate stub check the resolver +// runs while filtering name-lookup results, which are mostly +// first-party ids. +func BenchmarkStubKind(b *testing.B) { + for _, bc := range []struct{ name, id string }{ + {"first_party", "gortex/internal/resolver/resolver.go::withoutAttributedTerminals"}, + {"repo_stub", "gortex::external_call::requests.get"}, + {"legacy_stub", "module::lodash"}, + } { + b.Run(bc.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + StubKind(bc.id) + } + }) + } +} From 70c88e70f7c6b3c7c5d9bdc7b7c7f1de3dcd37d5 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 16:05:27 +0100 Subject: [PATCH 7/8] perf(graph): match stub kinds without building "::" per call StubKind looped over the four kinds and tested strings.HasPrefix(id, k+"::"), twice over for prefixed ids: up to eight run-time string concatenations for every id that is not a stub, which is most of what the resolver's IsStub filter sees. Match each kind in a switch on constant "::" prefixes instead. The concatenation is folded at compile time and the comparisons are inlined, so no call remains. The order (stdlib, external_call, builtin, module) and the grammar are unchanged; the table and the every-prefix comparison against the previous implementation from the previous commit pass. BenchmarkStubKind, 10 runs each (Apple M5 Pro, GOMAXPROCS=2): first_party ~72 ns/op -> ~8.1 ns/op repo_stub ~54 ns/op -> ~7.2 ns/op legacy_stub ~33 ns/op -> ~2.1 ns/op still with 0 B/op and 0 allocs/op. --- internal/graph/stub.go | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/internal/graph/stub.go b/internal/graph/stub.go index ba3573fdf..a5888af18 100644 --- a/internal/graph/stub.go +++ b/internal/graph/stub.go @@ -73,11 +73,9 @@ func IsStub(id string) bool { // We match by looking for one of the known kind segments // anywhere in the first two "::"-separated positions. func StubKind(id string) string { - for _, k := range stubKinds { - // Without repo prefix: "::..." - if strings.HasPrefix(id, k+"::") { - return k - } + // Without repo prefix: "::..." + if k := stubKindSegment(id); k != "" { + return k } // With repo prefix: "::::..." // Find the second "::" segment. @@ -85,23 +83,27 @@ func StubKind(id string) string { if first < 0 { return "" } - rest := id[first+2:] - for _, k := range stubKinds { - if strings.HasPrefix(rest, k+"::") { - return k - } - } - return "" + return stubKindSegment(id[first+2:]) } -// stubKinds is the closed set of stub categories. Ordered by -// expected frequency so the lookup loop bails early in the -// common case. -var stubKinds = []string{ - StubKindStdlib, - StubKindExternalCall, - StubKindBuiltin, - StubKindModule, +// stubKindSegment returns the stub kind s starts with as a whole +// "::" segment, or "" if it starts with none. The resolver runs +// IsStub on every name-lookup candidate, so each "::" is a +// constant expression rather than a per-call k+"::" concatenation. +// Cases are ordered by expected frequency so the common case bails +// early. +func stubKindSegment(s string) string { + switch { + case strings.HasPrefix(s, StubKindStdlib+"::"): + return StubKindStdlib + case strings.HasPrefix(s, StubKindExternalCall+"::"): + return StubKindExternalCall + case strings.HasPrefix(s, StubKindBuiltin+"::"): + return StubKindBuiltin + case strings.HasPrefix(s, StubKindModule+"::"): + return StubKindModule + } + return "" } // IsStdlibStub etc are convenience predicates that don't make From 9a10f77d21d03daa18e9c64e21ab3785f436c7a7 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sun, 1 Nov 2026 19:38:23 +0100 Subject: [PATCH 8/8] test(cmd/gortex): use neutral service fixture paths --- cmd/gortex/daemon_service_render_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/gortex/daemon_service_render_test.go b/cmd/gortex/daemon_service_render_test.go index dc1997bad..ea3a1563f 100644 --- a/cmd/gortex/daemon_service_render_test.go +++ b/cmd/gortex/daemon_service_render_test.go @@ -89,12 +89,12 @@ After=network.target [Service] Type=simple -ExecStart=/home/u/.local/bin/gortex daemon start` +ExecStart=/srv/u/.local/bin/gortex daemon start` const tail = ` Restart=on-failure RestartSec=2 -StandardOutput=append:/home/u/100%% logs/daemon.stderr.log -StandardError=append:/home/u/100%% logs/daemon.stderr.log +StandardOutput=append:/srv/u/100%% logs/daemon.stderr.log +StandardError=append:/srv/u/100%% logs/daemon.stderr.log [Install] WantedBy=default.target @@ -108,21 +108,21 @@ WantedBy=default.target { name: "env", env: []serviceEnvVar{ - {Key: "XDG_CACHE_HOME", Value: "/home/u/.cache"}, - {Key: "XDG_DATA_HOME", Value: "/home/u/has space"}, - {Key: "XDG_CONFIG_HOME", Value: `/home/u/"q"\x`}, - {Key: "GORTEX_DAEMON_LOGFILE", Value: "/home/u/100% logs/daemon.log"}, + {Key: "XDG_CACHE_HOME", Value: "/srv/u/.cache"}, + {Key: "XDG_DATA_HOME", Value: "/srv/u/has space"}, + {Key: "XDG_CONFIG_HOME", Value: `/srv/u/"q"\x`}, + {Key: "GORTEX_DAEMON_LOGFILE", Value: "/srv/u/100% logs/daemon.log"}, }, vars: ` -Environment=XDG_CACHE_HOME=/home/u/.cache -Environment="XDG_DATA_HOME=/home/u/has space" -Environment="XDG_CONFIG_HOME=/home/u/\"q\"\\x" -Environment="GORTEX_DAEMON_LOGFILE=/home/u/100%% logs/daemon.log"`, +Environment=XDG_CACHE_HOME=/srv/u/.cache +Environment="XDG_DATA_HOME=/srv/u/has space" +Environment="XDG_CONFIG_HOME=/srv/u/\"q\"\\x" +Environment="GORTEX_DAEMON_LOGFILE=/srv/u/100%% logs/daemon.log"`, }, } { t.Run(tc.name, func(t *testing.T) { t.Setenv("PATH", "") - out := renderSystemdUnit("/home/u/.local/bin/gortex", "/home/u/100% logs/daemon.stderr.log", tc.env) + out := renderSystemdUnit("/srv/u/.local/bin/gortex", "/srv/u/100% logs/daemon.stderr.log", tc.env) require.Equal(t, head+tc.vars+tail, out) }) }