diff --git a/backend/plugins/gitextractor/parser/clone_gitcli.go b/backend/plugins/gitextractor/parser/clone_gitcli.go index 3b5c9450b1d..f61dbc10d14 100644 --- a/backend/plugins/gitextractor/parser/clone_gitcli.go +++ b/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,6 +395,11 @@ 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) @@ -325,7 +407,26 @@ func (g *GitcliCloner) git(env []string, dir string, gitcmd string, args ...stri 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 { diff --git a/backend/plugins/gitextractor/parser/clone_gitcli_test.go b/backend/plugins/gitextractor/parser/clone_gitcli_test.go new file mode 100644 index 00000000000..49b6ea32d45 --- /dev/null +++ b/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()) +}