From cb663ca7db0901dc65b1026c0bcfda5cc7b1bf9b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 May 2020 15:07:31 +0100 Subject: [PATCH 1/3] Fix issue with DiffIndex on initial commit Unfortunately #11614 introduced a bug whereby the initial commit of a repository could not be seen due to there being no parent commit to create a clear diff from. Here we ensure that the index is empty by referring to a non-existent file and the create a diffstat from the difference between it and the parentless SHA. Fix #11650 Signed-off-by: Andrew Thornton --- modules/git/repo_compare.go | 6 +++--- modules/repofiles/temp_repo.go | 2 +- services/gitdiff/gitdiff.go | 19 +++++++++++++++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 5faadcf3f024d..0d7707b7bed82 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -122,11 +122,11 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string) (int, error) { // GetDiffShortStat counts number of changed files, number of additions and deletions func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAdditions, totalDeletions int, err error) { - return GetDiffShortStat(repo.Path, base+"..."+head) + return GetDiffShortStat(nil, repo.Path, base+"..."+head) } // GetDiffShortStat counts number of changed files, number of additions and deletions -func GetDiffShortStat(repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) { +func GetDiffShortStat(env []string, repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) { // Now if we call: // $ git diff --shortstat 1ebb35b98889ff77299f24d82da426b434b0cca0...788b8b1440462d477f45b0088875 // we get: @@ -136,7 +136,7 @@ func GetDiffShortStat(repoPath string, args ...string) (numFiles, totalAdditions "--shortstat", }, args...) - stdout, err := NewCommand(args...).RunInDir(repoPath) + stdout, err := NewCommand(args...).RunInDirWithEnv(repoPath, env) if err != nil { return 0, 0, 0, err } diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 2b03db8b4a310..4860de453a1a8 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -299,7 +299,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { t.repo.FullName(), err, stderr) } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.basePath, "--cached", "HEAD") + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(nil, t.basePath, "--cached", "HEAD") if err != nil { return nil, err } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index af88ed4d40efe..055eb2de08098 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -17,6 +17,7 @@ import ( "net/url" "os" "os/exec" + "path/filepath" "sort" "strconv" "strings" @@ -664,7 +665,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID ctx, cancel := context.WithCancel(git.DefaultContext) defer cancel() var cmd *exec.Cmd - if len(beforeCommitID) == 0 && commit.ParentCount() == 0 { + if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 { cmd = exec.CommandContext(ctx, git.GitExecutable, "show", afterCommitID) } else { actualBeforeCommitID := beforeCommitID @@ -711,7 +712,21 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID return nil, fmt.Errorf("Wait: %v", err) } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, beforeCommitID+"..."+afterCommitID) + shortstatArgs := []string{beforeCommitID + "..." + afterCommitID} + var env []string + if len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA { + shortstatArgs = []string{"--cached", afterCommitID} + tmpDir, err := ioutil.TempDir("", "gitdiff") + if err != nil { + return nil, err + } + defer func() { + _ = os.RemoveAll(tmpDir) + }() + env = append(os.Environ(), "GIT_INDEX_FILE="+filepath.Join(tmpDir, "non-existent-index")) + + } + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(env, repoPath, shortstatArgs...) if err != nil { return nil, err } From 51caecdb99655925eb2f98265f441dab67133794 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 29 May 2020 16:54:52 +0100 Subject: [PATCH 2/3] Update services/gitdiff/gitdiff.go --- services/gitdiff/gitdiff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 055eb2de08098..1a213f9a4e92f 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -715,7 +715,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID shortstatArgs := []string{beforeCommitID + "..." + afterCommitID} var env []string if len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA { - shortstatArgs = []string{"--cached", afterCommitID} + shortstatArgs = []string{"--cached", "-R", afterCommitID} tmpDir, err := ioutil.TempDir("", "gitdiff") if err != nil { return nil, err From 960d8d6c7a23dd6a06789862754c7c091b85bed9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 May 2020 17:00:34 +0100 Subject: [PATCH 3/3] Following a clever trick shown to me by @L0veSunshine use the SHA of an empty Tree See https://github.com/go-gitea/gitea/pull/11674#issuecomment-635999084 Co-Authored-By: L0veSunshine Signed-off-by: Andrew Thornton --- modules/git/repo_compare.go | 6 +++--- modules/git/sha1.go | 3 +++ modules/repofiles/temp_repo.go | 2 +- services/gitdiff/gitdiff.go | 15 ++------------- 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 0d7707b7bed82..5faadcf3f024d 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -122,11 +122,11 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string) (int, error) { // GetDiffShortStat counts number of changed files, number of additions and deletions func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAdditions, totalDeletions int, err error) { - return GetDiffShortStat(nil, repo.Path, base+"..."+head) + return GetDiffShortStat(repo.Path, base+"..."+head) } // GetDiffShortStat counts number of changed files, number of additions and deletions -func GetDiffShortStat(env []string, repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) { +func GetDiffShortStat(repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) { // Now if we call: // $ git diff --shortstat 1ebb35b98889ff77299f24d82da426b434b0cca0...788b8b1440462d477f45b0088875 // we get: @@ -136,7 +136,7 @@ func GetDiffShortStat(env []string, repoPath string, args ...string) (numFiles, "--shortstat", }, args...) - stdout, err := NewCommand(args...).RunInDirWithEnv(repoPath, env) + stdout, err := NewCommand(args...).RunInDir(repoPath) if err != nil { return 0, 0, 0, err } diff --git a/modules/git/sha1.go b/modules/git/sha1.go index bccc94f10349a..06c8ad14b55c0 100644 --- a/modules/git/sha1.go +++ b/modules/git/sha1.go @@ -17,6 +17,9 @@ import ( // EmptySHA defines empty git SHA const EmptySHA = "0000000000000000000000000000000000000000" +// EmptyTreeSHA is the SHA of an empty tree +const EmptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" + // SHAPattern can be used to determine if a string is an valid sha var SHAPattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 4860de453a1a8..2b03db8b4a310 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -299,7 +299,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { t.repo.FullName(), err, stderr) } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(nil, t.basePath, "--cached", "HEAD") + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.basePath, "--cached", "HEAD") if err != nil { return nil, err } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 1a213f9a4e92f..02aef70882bb0 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -17,7 +17,6 @@ import ( "net/url" "os" "os/exec" - "path/filepath" "sort" "strconv" "strings" @@ -713,20 +712,10 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID } shortstatArgs := []string{beforeCommitID + "..." + afterCommitID} - var env []string if len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA { - shortstatArgs = []string{"--cached", "-R", afterCommitID} - tmpDir, err := ioutil.TempDir("", "gitdiff") - if err != nil { - return nil, err - } - defer func() { - _ = os.RemoveAll(tmpDir) - }() - env = append(os.Environ(), "GIT_INDEX_FILE="+filepath.Join(tmpDir, "non-existent-index")) - + shortstatArgs = []string{git.EmptyTreeSHA, afterCommitID} } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(env, repoPath, shortstatArgs...) + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, shortstatArgs...) if err != nil { return nil, err }