From ff81bdf8cc8f4bd9dc7d65fe47bc96e5b5bef1d5 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 15:12:24 +0100 Subject: [PATCH] Keep git's own words in a fetch's reason: a bare "error:" line no longer stands for the forge's refusal The mirror of jschoubben/car-hunter on ace said only "git -C: error:": git writes a bare "error:" before the forge's message, and the reason kept the first non-empty line. forgekey.Said keeps every line that says something, leaves bare prefixes out, joins them and bounds them at 600 bytes; every git and ssh-keygen reason of buildstep and forgekey goes through it, and the mirror's commands are named by their verb, not by -C. --- go/buildstep/fetch.go | 17 ++++++----------- go/buildstep/merge.go | 2 +- go/buildstep/mirror_test.go | 18 ++++++++++++++++++ go/buildstep/trees.go | 6 +++--- go/forgekey/keykeeper.go | 2 +- go/forgekey/runner.go | 35 +++++++++++++++++++++++++++++++---- go/forgekey/said_test.go | 28 ++++++++++++++++++++++++++++ 7 files changed, 88 insertions(+), 20 deletions(-) create mode 100644 go/forgekey/said_test.go diff --git a/go/buildstep/fetch.go b/go/buildstep/fetch.go index 2715405..4135bdb 100644 --- a/go/buildstep/fetch.go +++ b/go/buildstep/fetch.go @@ -71,7 +71,7 @@ func FetchCommit(ctx context.Context, run forgekey.Runner, dir, remote, commit, full := append([]string{"-c", "credential.helper=", "-C", dir}, args...) r := run(ctx, env, "git", full...) if r.Status != 0 || r.Err != "" { - return "", fmt.Errorf("git %s: %s", args[0], firstLine(r.Err+" "+r.Stderr)) + return "", fmt.Errorf("git %s: %s", args[0], forgekey.Said(r.Err+"\n"+r.Stderr)) } return r.Stdout, nil } @@ -128,7 +128,11 @@ func UpdateMirror(ctx context.Context, run forgekey.Runner, mirror, remote, comm git := func(args ...string) error { r := run(ctx, env, "git", append([]string{"-c", "credential.helper="}, args...)...) if r.Status != 0 || r.Err != "" { - return fmt.Errorf("git %s: %s", args[0], firstLine(r.Err+" "+r.Stderr)) + verb := args[0] + if verb == "-C" && len(args) > 2 { // the mirror's commands are named by what they do, not by -C + verb = args[2] + } + return fmt.Errorf("git %s: %s", verb, forgekey.Said(r.Err+"\n"+r.Stderr)) } return nil } @@ -190,12 +194,3 @@ func checkFetch(o fetchOptions, dir, remote, commit, ref string) error { } return nil } - -func firstLine(s string) string { - for _, l := range strings.Split(s, "\n") { - if l = strings.TrimSpace(l); l != "" { - return l - } - } - return "" -} diff --git a/go/buildstep/merge.go b/go/buildstep/merge.go index deee405..907009e 100644 --- a/go/buildstep/merge.go +++ b/go/buildstep/merge.go @@ -52,7 +52,7 @@ func MergeInto(ctx context.Context, run forgekey.Runner, dir, remote, base, comm full := append([]string{"-c", "credential.helper=", "-C", dir}, args...) r := run(ctx, env, "git", full...) if r.Status != 0 || r.Err != "" { - return "", fmt.Errorf("git %s: %s", args[0], firstLine(r.Err+" "+r.Stderr)) + return "", fmt.Errorf("git %s: %s", args[0], forgekey.Said(r.Err+"\n"+r.Stderr)) } return r.Stdout, nil } diff --git a/go/buildstep/mirror_test.go b/go/buildstep/mirror_test.go index 21c82fc..71b894a 100644 --- a/go/buildstep/mirror_test.go +++ b/go/buildstep/mirror_test.go @@ -104,3 +104,21 @@ func TestUpdateMirrorRefusesBeforeRunning(t *testing.T) { t.Errorf("ran %v", f.calls) } } + +// The forge's refusal reaches the reason in git's own words: the mirror of a repository its account cannot read said +// only "git -C: error:" (2026-10-26), since git writes a bare "error:" line before the forge's message. +func TestUpdateMirrorSaysTheForgesRefusal(t *testing.T) { + f := &fakeGit{} + f.rule = func(args string) (r forgekey.Ran) { + if strings.Contains(args, "fetch") { + r.Status = 128 + r.Stderr = "error: \nerror: User: 22:nox-builder with Key: 30:mesh:ace.mesh-build-go is not authorized to read o/r.\nerror: \nfatal: Could not read from remote repository.\n" + } + return + } + err := UpdateMirror(context.Background(), f.run, filepath.Join(t.TempDir(), "m.git"), "ssh://mesh-forge/o/r.git", sha, "") + if err == nil || !strings.Contains(err.Error(), "nox-builder with Key: 30:mesh:ace.mesh-build-go is not authorized to read o/r.") || + !strings.Contains(err.Error(), "Could not read from remote repository") || !strings.HasPrefix(err.Error(), "git fetch: ") { + t.Errorf("err = %v", err) + } +} diff --git a/go/buildstep/trees.go b/go/buildstep/trees.go index 27d35ed..65a775e 100644 --- a/go/buildstep/trees.go +++ b/go/buildstep/trees.go @@ -36,7 +36,7 @@ var lockFiles = []string{"go.sum", "package-lock.json"} func ModuleTrees(ctx context.Context, run forgekey.Runner, env []string, dir, commit string) (map[string]ModuleTree, error) { r := run(ctx, env, "git", "-C", dir, "ls-files", "--", "module.json", "*/module.json") if r.Status != 0 || r.Err != "" { - return nil, fmt.Errorf("git ls-files: %s", firstLine(r.Err+" "+r.Stderr)) + return nil, fmt.Errorf("git ls-files: %s", forgekey.Said(r.Err+"\n"+r.Stderr)) } trees := map[string]ModuleTree{} for _, l := range strings.Split(r.Stdout, "\n") { @@ -50,7 +50,7 @@ func ModuleTrees(ctx context.Context, run forgekey.Runner, env []string, dir, co } t := run(ctx, env, "git", "-C", dir, "rev-parse", spec) if t.Status != 0 || t.Err != "" { - return nil, fmt.Errorf("git rev-parse %s: %s", spec, firstLine(t.Err+" "+t.Stderr)) + return nil, fmt.Errorf("git rev-parse %s: %s", spec, forgekey.Said(t.Err+"\n"+t.Stderr)) } mt := ModuleTree{Tree: strings.TrimSpace(t.Stdout)} raw, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(l))) @@ -93,7 +93,7 @@ func narrow(ctx context.Context, run forgekey.Runner, env []string, dir, commit, } r := run(ctx, env, "git", append([]string{"-C", dir, "ls-tree", "-z", commit, "--"}, full...)...) if r.Status != 0 || r.Err != "" { - return fmt.Errorf("git ls-tree %s of the module %s: %s", commit, mdir, firstLine(r.Err+" "+r.Stderr)) + return fmt.Errorf("git ls-tree %s of the module %s: %s", commit, mdir, forgekey.Said(r.Err+"\n"+r.Stderr)) } objects := map[string]string{} for _, l := range strings.Split(r.Stdout, "\x00") { diff --git a/go/forgekey/keykeeper.go b/go/forgekey/keykeeper.go index cb75668..7349d88 100644 --- a/go/forgekey/keykeeper.go +++ b/go/forgekey/keykeeper.go @@ -101,7 +101,7 @@ func (k *KeyKeeper) ensure(ctx context.Context) error { } r := k.Run(ctx, nil, "ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", k.Account+"@"+k.Node+" (mesh)", "-f", k.path()) if r.Status != 0 || r.Err != "" { - return fmt.Errorf("ssh-keygen: %s", firstLine(r.Err+" "+r.Stderr)) + return fmt.Errorf("ssh-keygen: %s", Said(r.Err+"\n"+r.Stderr)) } return nil } diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go index 422e868..b989ef1 100644 --- a/go/forgekey/runner.go +++ b/go/forgekey/runner.go @@ -9,6 +9,7 @@ import ( "os/exec" "strings" "time" + "unicode/utf8" ) // Ran is what a command did: its output, its exit status, and why it never ran to an answer. @@ -108,11 +109,37 @@ func stableDir(clean bool, env []string) string { return os.TempDir() } -func firstLine(s string) string { +// SaidBound bounds what Said keeps of a command's words, so a reason stays a line of a step's record and of a log. +const SaidBound = 600 + +// Said is what a command said of its failure in its own words, for a reason: every line that says something, trimmed +// and joined with "; ", bounded at SaidBound bytes. A line that is a bare prefix and nothing else (git writes "error:" +// alone before and after the forge's own message, "remote:" or "fatal:" likewise) is left out, so the cause is never +// cut off behind it: the first line alone of `git fetch` from a repository the account cannot read was "error:". +func Said(s string) string { + var keep []string for _, l := range strings.Split(s, "\n") { - if l = strings.TrimSpace(l); l != "" { - return l + l = strings.TrimSpace(l) + if l == "" || barePrefix(l) { + continue } + keep = append(keep, l) } - return "" + out := strings.Join(keep, "; ") + if len(out) > SaidBound { + cut := SaidBound + for cut > 0 && !utf8.RuneStart(out[cut]) { + cut-- + } + out = out[:cut] + " …" + } + return out +} + +// barePrefix says whether a line is one word and a colon, such as "error:" or "remote:", with nothing after it. +func barePrefix(l string) bool { + if !strings.HasSuffix(l, ":") { + return false + } + return !strings.ContainsAny(strings.TrimSuffix(l, ":"), " \t:") } diff --git a/go/forgekey/said_test.go b/go/forgekey/said_test.go new file mode 100644 index 0000000..c361a97 --- /dev/null +++ b/go/forgekey/said_test.go @@ -0,0 +1,28 @@ +package forgekey + +import ( + "strings" + "testing" +) + +// git's own words survive as the reason: a bare "error:" or "fatal:" line is never the whole reason (the mirror of +// jschoubben/car-hunter "is not used: git -C: error:", 2026-10-26), every line that says something is kept, and the +// words are bounded. +func TestSaidKeepsTheCauseBehindABarePrefix(t *testing.T) { + forge := "error: \nerror: User: 22:nox-builder with Key: 30:mesh:ace.mesh-build-go is not authorized to read jschoubben/car-hunter.\nerror: \nfatal: Could not read from remote repository.\n\nPlease make sure you have the correct access rights\nand the repository exists.\n" + for _, c := range []struct{ name, in, want string }{ + {"forge refusal", forge, "error: User: 22:nox-builder with Key: 30:mesh:ace.mesh-build-go is not authorized to read jschoubben/car-hunter.; fatal: Could not read from remote repository.; Please make sure you have the correct access rights; and the repository exists."}, + {"one line", "fatal: couldn't find remote ref x\n", "fatal: couldn't find remote ref x"}, + {"bare prefixes only", "error:\nremote:\n", ""}, + {"a colon inside stays", " ENOENT \nfatal: bad: thing\n", "ENOENT; fatal: bad: thing"}, + {"empty", "", ""}, + } { + if got := Said(c.in); got != c.want { + t.Errorf("%s: Said = %q, want %q", c.name, got, c.want) + } + } + long := Said(strings.Repeat("é", SaidBound)) + if !strings.HasSuffix(long, " …") || len(long) > SaidBound+len(" …") || !strings.HasPrefix(long, "é") { + t.Errorf("long: %d bytes, ends %q", len(long), long[len(long)-6:]) + } +} -- 2.54.0