Refuse a range the shallow clone's depth does not reach

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,31 +70,79 @@ 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)
 	base := r.git("rev-parse", "HEAD")
 	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
+}

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=<n> 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,