From 283745f99e5e6007cf1138ef0e0edc9ec4d51f01 Mon Sep 17 00:00:00 2001 From: closer-claudio Date: Fri, 30 Oct 2026 01:31:32 -0700 Subject: [PATCH] Refuse a range the shallow clone's depth does not reach A --range git could not resolve was an error in a whole clone but, in a shallow one, fell back to validating what the clone had, from its oldest commit: a depth of one validated the head commit alone and passed, so the commits a pull request added went unseen. The range asked for is the contract; an unresolvable one is an error in a shallow clone too, and the error names the end the clone does not have and what to fetch (fetch-depth: 0, git fetch --deepen or --unshallow). A shallow clone deep enough for the range runs as before. Co-Authored-By: Claude Fable 5.1 Signed-off-by: closer-claudio --- cmd/git-validation/contract_decided_test.go | 70 ++++++++++++++++---- git/commits.go | 72 ++++++++++++--------- 2 files changed, 101 insertions(+), 41 deletions(-) diff --git a/cmd/git-validation/contract_decided_test.go b/cmd/git-validation/contract_decided_test.go index e9f0f8b..738d4f5 100644 --- a/cmd/git-validation/contract_decided_test.go +++ b/cmd/git-validation/contract_decided_test.go @@ -4,8 +4,8 @@ package main_test // behaviour before them: the short-subject rule counts characters, not // bytes, so a subject of 89 non-ASCII characters passes and one of 90 // fails; and a --range git cannot resolve is an error naming the range, -// with nothing validated, where a shallow clone whose history does not -// reach the range's start still runs over what it has. +// with nothing validated, in a shallow clone whose history does not reach +// the range's start as well, where the error says what to fetch. import ( "os" @@ -70,11 +70,13 @@ func TestContractUnresolvableRangeIsAnError(t *testing.T) { } } -// A shallow clone's history may not reach the range's start a CI -// configuration names; the run then covers what the clone has, as before. +// A shallow clone whose history does not reach the range's start a CI +// configuration names is refused like any unresolvable range, nothing +// validated, and the error says what to fetch; it used to run over what the +// clone had, so a depth of one validated the head commit alone and passed. // //nolint:paralleltest // newRepo sets the environment with t.Setenv -func TestContractShallowCloneRunsOverWhatItHas(t *testing.T) { +func TestContractShallowCloneRefusesARangeOutOfReach(t *testing.T) { r := newRepo(t) r.commit("a", "a\n", "Root", true) @@ -82,19 +84,65 @@ func TestContractShallowCloneRunsOverWhatItHas(t *testing.T) { r.commit("b", "b\n", "Second", true) r.commit("c", "c\n", "Third", true) - shallow := filepath.Join(t.TempDir(), "shallow") + shallow := shallowClone(t, r.dir, "1") - clone := exec.CommandContext(t.Context(), "git", "clone", "-q", "--depth", "1", "file://"+r.dir, shallow) - if out, err := clone.CombinedOutput(); err != nil { - t.Fatalf("shallow clone: %v\n%s", err, out) + cmd := exec.CommandContext(t.Context(), binary, "--dir", shallow, "--range", base+"..HEAD") + cmd.Dir = shallow + cmd.Env = os.Environ() + + var stderr strings.Builder + + cmd.Stderr = &stderr + + out, err := cmd.Output() + if err == nil { + t.Fatalf("a range past the clone's depth validated what the clone has:\n%s", out) } + if strings.Contains(string(out), " * ") { + t.Errorf("commits were validated under a range past the clone's depth:\n%s", out) + } + + for _, want := range []string{base + " is not in it", "shallow clone", "fetch-depth: 0", "--unshallow"} { + if !strings.Contains(stderr.String(), want) { + t.Errorf("the error does not say %q:\n%s", want, stderr.String()) + } + } +} + +// A shallow clone deep enough for the range runs as a whole clone does. +// +//nolint:paralleltest // newRepo sets the environment with t.Setenv +func TestContractShallowCloneWithTheRangeInReachRuns(t *testing.T) { + r := newRepo(t) + + r.commit("a", "a\n", "Root", true) + base := r.git("rev-parse", "HEAD") + r.commit("b", "b\n", "Second", true) + r.commit("c", "c\n", "Third", true) + + shallow := shallowClone(t, r.dir, "3") + cmd := exec.CommandContext(t.Context(), binary, "--dir", shallow, "--range", base+"..HEAD") cmd.Dir = shallow cmd.Env = os.Environ() out, err := cmd.Output() - if err != nil || !strings.Contains(string(out), "PASS") { - t.Errorf("shallow clone: %v\n%s", err, out) + if err != nil || strings.Count(string(out), "PASS") != 2 { + t.Errorf("shallow clone deep enough for the range: %v\n%s", err, out) } } + +// shallowClone clones dir with the given depth and returns the clone's path. +func shallowClone(t *testing.T, dir, depth string) string { + t.Helper() + + shallow := filepath.Join(t.TempDir(), "shallow") + + clone := exec.CommandContext(t.Context(), "git", "clone", "-q", "--depth", depth, "file://"+dir, shallow) + if out, err := clone.CombinedOutput(); err != nil { + t.Fatalf("shallow clone: %v\n%s", err, out) + } + + return shallow +} diff --git a/git/commits.go b/git/commits.go index b40ad8f..085d867 100644 --- a/git/commits.go +++ b/git/commits.go @@ -48,54 +48,66 @@ func Commits(commitrange string) ([]CommitEntry, error) { return commits, nil } -// errRange is a --range git cannot resolve in a repository whose history is -// whole: a typo, or a ref that is not there. It names the range. +// errRange is a --range git cannot resolve: a typo, a ref that is not +// there, or a start past the depth of a shallow clone. It names the range. var errRange = errors.New("the commit range does not resolve") -// checkRevList returns the range to validate. A range git resolves is -// itself. One it does not resolve is an error in a repository with its -// whole history; in a shallow clone, the CI case, the range's start may lie -// past the depth fetched, and the run then covers what the clone has, from -// its oldest commit. Before, every unresolvable range took that fallback, -// so a typo validated the whole history and passed. +// checkRevList returns the range to validate: the range itself, once git +// resolves it. One git does not resolve is an error. Before, it validated +// what the repository had, so a typo, or a shallow clone whose depth did not +// reach the range's start, validated the whole history and passed. In a +// shallow clone the error says what to fetch. func checkRevList(commitrange string) (string, error) { cmdArgs := []string{"git", "rev-list", commitrange} _, err := command(cmdArgs[1:]...).Output() if err == nil { - // no issues, return now return commitrange, nil } - if !shallowRepository() { - // The git command that failed is named, and its exit unwraps, as - // every git failure here does; git's own words follow. - detail := "" + // The git command that failed is named, and its exit unwraps, as every + // git failure here does; git's own words follow. + detail := "" - if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && len(exitErr.Stderr) > 0 { - detail = " (" + strings.TrimSpace(string(exitErr.Stderr)) + ")" + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && len(exitErr.Stderr) > 0 { + detail = " (" + strings.TrimSpace(string(exitErr.Stderr)) + ")" + } + + if shallowRepository() { + detail += "; the repository is a shallow clone" + + // A typo and an end past the depth look the same from here: both + // are not in the clone. + switch missing := missingEnds(commitrange); len(missing) { + case 0: + case 1: + detail += " and " + missing[0] + " is not in it" + default: + detail += " and " + strings.Join(missing, " and ") + " are not in it" } - return "", fmt.Errorf("%w: %s: %w%s", errRange, strings.Join(cmdArgs, " "), err, detail) + detail += ": fetch more history (actions/checkout fetch-depth: 0, git fetch --deepen= or --unshallow)" } - cmdArgs = []string{"git", "log", "--pretty=oneline", "--no-show-signature"} + return "", fmt.Errorf("%w: %s: %w%s", errRange, strings.Join(cmdArgs, " "), err, detail) +} - output, err := command(cmdArgs[1:]...).Output() - if err != nil { - return "", fmt.Errorf("%s: %w", strings.Join(cmdArgs, " "), err) - } - // This "output" is now the list of available commits and short description. - // We want the last commit hash only.. (i.e. `| tail -n1 | awk '{ print $1 }'`) - chunks := strings.Split(strings.TrimSpace(string(output)), "\n") - if len(chunks) == 1 { - commit, _, _ := strings.Cut(chunks[0], " ") - return commit, nil - } +// missingEnds returns the ends of a range (a..b, a...b, or a single revision) +// that do not name a commit the repository has. +func missingEnds(commitrange string) []string { + var missing []string + + for end := range strings.SplitSeq(strings.ReplaceAll(commitrange, "...", ".."), "..") { + if end == "" { + continue + } - lastCommit, _, _ := strings.Cut(chunks[len(chunks)-1], " ") + if err := command("rev-parse", "--verify", "--quiet", end+"^{commit}").Run(); err != nil { + missing = append(missing, end) + } + } - return lastCommit + "..HEAD", nil + return missing } // shallowRepository reports whether the repository was cloned with a depth,