fix(gitextractor): keep deepening shallow clones past merge boundaries

backend/plugins/gitextractor/parser/clone_gitcli.go

@@ -18,12 +18,14 @@ limitations under the License.
 package parser
 
 import (
+	"bytes"
 	"encoding/base64"
 	"fmt"
 	"net/url"
 	"os"
 	"os/exec"
 	"path"
+	"strconv"
 	"strings"
 	"time"
 
@@ -37,6 +39,10 @@ import (
 var _ RepoCloner = (*GitcliCloner)(nil)
 var ErrNoData = errors.NotModified.New("No data to be collected")
 
+// maxExtraDeepenRounds caps the extra deepen rounds in deepenUntilSince. The
+// depth doubles every round, so up to 1023 extra generations are covered.
+const maxExtraDeepenRounds = 10
+
 // CloneRepoConfig is the configuration for the CloneRepo method
 // the subtask should run in Full Sync mode whenever the configuration is changed
 type CloneRepoConfig struct {
@@ -200,6 +206,7 @@ func (g *GitcliCloner) CloneRepo() errors.Error {
 		if err := g.deepen(); err != nil {
 			return err
 		}
+		g.deepenUntilSince()
 		g.success = true
 	}
 	return nil
@@ -231,6 +238,76 @@ func (g *GitcliCloner) deepen() errors.Error {
 	return nil
 }
 
+// deepenUntilSince fetches more history for as long as a shallow boundary
+// commit is newer than g.since, doubling the depth each round (1, 2, 4, ...).
+//
+// Git makes a commit shallow (cuts it off from all of its parents) when any of
+// its parents is older than --shallow-since. For a merge commit whose
+// feature-branch parent is old, this also hides newer commits on its other
+// parent, and a single --deepen=1 can still leave them without their own first
+// parent. The collectors skip commits whose first parent is missing, and the
+// next incremental run starts after them, so they would never be collected.
+// See https://github.com/apache/devlake/issues/9189
+func (g *GitcliCloner) deepenUntilSince() {
+	depth := 1
+	for round := 0; ; round++ {
+		newest, err := g.newestShallowCommitTime()
+		if err != nil {
+			g.logger.Warn(err, "failed to read the shallow boundary")
+			return
+		}
+		if newest.Before(*g.since) {
+			return
+		}
+		if round == maxExtraDeepenRounds {
+			g.logger.Warn(nil, "shallow boundary is still newer than %s after %d extra deepen rounds", g.since.Format(time.RFC3339), maxExtraDeepenRounds)
+			return
+		}
+		if err := g.gitFetch(fmt.Sprintf("--deepen=%d", depth)); err != nil {
+			g.logger.Warn(err, "failed to deepen the cloned repo")
+			return
+		}
+		depth *= 2
+	}
+}
+
+// newestShallowCommitTime returns the newest committer time of the shallow
+// boundary commits present in the local repo, or the zero time if there are none.
+func (g *GitcliCloner) newestShallowCommitTime() (time.Time, errors.Error) {
+	// all clones are bare, so the shallow file is in the repo dir itself
+	shallow, e := os.ReadFile(path.Join(g.localDir, "shallow"))
+	if os.IsNotExist(e) {
+		return time.Time{}, nil
+	}
+	if e != nil {
+		return time.Time{}, errors.Convert(e)
+	}
+	// the shallow file may list commits that were not fetched; `git log` fails on those
+	objects, err := g.gitOutput(shallow, "cat-file", "--batch-check=%(objectname) %(objecttype)")
+	if err != nil {
+		return time.Time{}, err
+	}
+	var commits bytes.Buffer
+	for _, line := range strings.Split(objects, "\n") {
+		if sha, objectType, ok := strings.Cut(line, " "); ok && objectType == "commit" {
+			commits.WriteString(sha + "\n")
+		}
+	}
+	if commits.Len() == 0 {
+		return time.Time{}, nil
+	}
+	// --no-walk lists the given commits newest first by committer time
+	newest, err := g.gitOutput(commits.Bytes(), "log", "--no-walk", "--stdin", "-n1", "--format=%ct")
+	if err != nil {
+		return time.Time{}, err
+	}
+	seconds, e := strconv.ParseInt(strings.TrimSpace(newest), 10, 64)
+	if e != nil {
+		return time.Time{}, errors.Convert(e)
+	}
+	return time.Unix(seconds, 0), nil
+}
+
 func (g *GitcliCloner) shallowClone() errors.Error {
 	// to fetch newly added commits from ALL branches, we need to the following guide:
 	//    https://stackoverflow.com/questions/23708231/git-shallow-clone-clone-depth-misses-remote-branches
@@ -318,14 +395,38 @@ func (g *GitcliCloner) gitCmd(gitcmd string, args ...string) errors.Error {
 }
 
 func (g *GitcliCloner) git(env []string, dir string, gitcmd string, args ...string) errors.Error {
+	return g.execCommand(g.gitCommand(env, dir, gitcmd, args...))
+}
+
+// gitCommand builds a git command with the global config args (auth, credential helper).
+func (g *GitcliCloner) gitCommand(env []string, dir string, gitcmd string, args ...string) *exec.Cmd {
 	g.logger.Debug("git %s %v", gitcmd, sanitizeArgs(args)) // CWE-532: sanitize before logging
 	cmdArgs := append([]string{}, g.globalConfigArgs...)
 	cmdArgs = append(cmdArgs, gitcmd)
 	cmdArgs = append(cmdArgs, args...)
 	cmd := exec.CommandContext(g.ctx.GetContext(), "git", cmdArgs...)
 	cmd.Env = env
 	cmd.Dir = dir
-	return g.execCommand(cmd)
+	return cmd
+}
+
+// gitOutput runs a git command in the cloned repo and returns its stdout.
+//
+// In a partial clone (--filter=blob:none, used when SkipCommitStat is set),
+// looking up a missing object makes git fetch it from origin. GIT_NO_LAZY_FETCH
+// prevents that where it is supported (git 2.45+ and some backports); with older
+// git the fetch uses the same auth and proxy settings as the other fetches.
+func (g *GitcliCloner) gitOutput(stdin []byte, gitcmd string, args ...string) (string, errors.Error) {
+	cmd := g.gitCommand(g.syncEnvs, g.localDir, gitcmd, args...)
+	cmd.Env = append(cmd.Environ(), "GIT_NO_LAZY_FETCH=1")
+	cmd.Stdin = bytes.NewReader(stdin)
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+	output, e := cmd.Output()
+	if e != nil {
+		return "", errors.Default.New(fmt.Sprintf("git %s in %s failed: %s", gitcmd, cmd.Dir, generateErrMsg(stderr.Bytes(), e)))
+	}
+	return string(output), nil
 }
 
 func (g *GitcliCloner) execCommand(cmd *exec.Cmd) errors.Error {

backend/plugins/gitextractor/parser/clone_gitcli_test.go

@@ -0,0 +1,157 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package parser
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/apache/devlake/helpers/unithelper"
+	mockplugin "github.com/apache/devlake/mocks/core/plugin"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// runGit runs git in dir with a fixed identity (and date, if given), and returns its trimmed stdout.
+func runGit(t *testing.T, dir, date string, args ...string) string {
+	t.Helper()
+	cmd := exec.Command("git", args...)
+	cmd.Dir = dir
+	cmd.Env = append(os.Environ(),
+		"GIT_AUTHOR_NAME=DevLake Test", "GIT_AUTHOR_EMAIL=test@devlake.apache.org",
+		"GIT_COMMITTER_NAME=DevLake Test", "GIT_COMMITTER_EMAIL=test@devlake.apache.org",
+	)
+	if date != "" {
+		cmd.Env = append(cmd.Env, "GIT_AUTHOR_DATE="+date, "GIT_COMMITTER_DATE="+date)
+	}
+	output, err := cmd.Output()
+	var stderr []byte
+	if exitErr, ok := err.(*exec.ExitError); ok {
+		stderr = exitErr.Stderr
+	}
+	require.NoError(t, err, "git %v: %s", args, stderr)
+	return strings.TrimSpace(string(output))
+}
+
+// buildMergeBoundaryRepo creates a bare repo with the history from issue #9189:
+//
+//	R ---- Q ---- P ---- M   main
+//	 \                  /
+//	  F ---------------+     feature
+//
+// The previous incremental run started at 12:00, after F (10:00) and before
+// Q (13:00), P (14:00) and M (16:00).
+func buildMergeBoundaryRepo(t *testing.T) (dir string, commits map[string]string) {
+	t.Helper()
+	dir = filepath.Join(t.TempDir(), "origin.git")
+	require.NoError(t, os.Mkdir(dir, 0o755))
+	runGit(t, dir, "", "init", "-q", "--bare")
+	runGit(t, dir, "", "symbolic-ref", "HEAD", "refs/heads/main")
+	tree := runGit(t, dir, "", "mktree")
+	commit := func(date, message string, parents ...string) string {
+		args := []string{"commit-tree", tree, "-m", message}
+		for _, parent := range parents {
+			args = append(args, "-p", parent)
+		}
+		return runGit(t, dir, date, args...)
+	}
+	commits = map[string]string{}
+	commits["R"] = commit("2024-01-01T00:00:00Z", "R")
+	commits["F"] = commit("2024-01-02T10:00:00Z", "F", commits["R"])
+	commits["Q"] = commit("2024-01-02T13:00:00Z", "Q", commits["R"])
+	commits["P"] = commit("2024-01-02T14:00:00Z", "P", commits["Q"])
+	commits["M"] = commit("2024-01-02T16:00:00Z", "M", commits["P"], commits["F"])
+	runGit(t, dir, "", "update-ref", "refs/heads/main", commits["M"])
+	runGit(t, dir, "", "update-ref", "refs/heads/feature", commits["F"])
+	return dir, commits
+}
+
+func newTestCloner(t *testing.T, remoteDir string, since time.Time) *GitcliCloner {
+	t.Helper()
+	ctx := new(mockplugin.SubTaskContext)
+	ctx.On("GetContext").Return(context.Background())
+	return &GitcliCloner{
+		ctx:       ctx,
+		taskData:  &GitExtractorTaskData{Options: &GitExtractorOptions{}},
+		logger:    unithelper.DummyLogger(),
+		since:     &since,
+		remoteUrl: "file://" + remoteDir,
+		localDir:  filepath.Join(t.TempDir(), "clone.git"),
+	}
+}
+
+func hasObject(dir, sha string) bool {
+	cmd := exec.Command("git", "cat-file", "-e", sha)
+	cmd.Dir = dir
+	return cmd.Run() == nil
+}
+
+func TestIncrementalCloneKeepsCommitsBehindMergeBoundary(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git is not installed")
+	}
+	origin, commits := buildMergeBoundaryRepo(t)
+	cloner := newTestCloner(t, origin, time.Date(2024, 1, 2, 12, 0, 0, 0, time.UTC))
+
+	require.NoError(t, cloner.CloneRepo())
+
+	// Q, P and M are newer than since. Each must be fetched together with its
+	// first parent, otherwise CollectCommits skips it and no later run collects it.
+	for _, name := range []string{"Q", "P", "M"} {
+		assert.True(t, hasObject(cloner.localDir, commits[name]), "%s should be fetched", name)
+	}
+	assert.True(t, hasObject(cloner.localDir, commits["R"]), "R, the first parent of Q, should be fetched")
+	newest, err := cloner.newestShallowCommitTime()
+	require.NoError(t, err)
+	assert.True(t, newest.Before(*cloner.since), "shallow boundary %s should be older than since", newest)
+}
+
+func TestNewestShallowCommitTimeIgnoresMissingCommits(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git is not installed")
+	}
+	origin, commits := buildMergeBoundaryRepo(t)
+	cloner := newTestCloner(t, origin, time.Date(2024, 1, 2, 12, 0, 0, 0, time.UTC))
+	runGit(t, "", "", "clone", "-q", "--bare", "--depth=1", cloner.remoteUrl, cloner.localDir)
+
+	// a depth=1 clone of main leaves only M on the boundary
+	newest, err := cloner.newestShallowCommitTime()
+	require.NoError(t, err)
+	assert.Equal(t, time.Date(2024, 1, 2, 16, 0, 0, 0, time.UTC), newest.UTC())
+
+	// git may list a boundary commit it did not fetch; it must not fail the lookup
+	shallowFile := filepath.Join(cloner.localDir, "shallow")
+	shallow, e := os.ReadFile(shallowFile)
+	require.NoError(t, e)
+	require.NoError(t, os.WriteFile(shallowFile, append(shallow, commits["P"]+"\n"...), 0o644))
+	require.False(t, hasObject(cloner.localDir, commits["P"]))
+	newest, err = cloner.newestShallowCommitTime()
+	require.NoError(t, err)
+	assert.Equal(t, time.Date(2024, 1, 2, 16, 0, 0, 0, time.UTC), newest.UTC())
+
+	// a full clone has no shallow file
+	require.NoError(t, os.Remove(shallowFile))
+	newest, err = cloner.newestShallowCommitTime()
+	require.NoError(t, err)
+	assert.True(t, newest.IsZero())
+}