aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorzeripath2020-10-14 12:27:21 +0100
committerGitHub2020-10-14 14:27:21 +0300
commit99fb2564118286a5fc2db5676b44029a96a14087 (patch)
tree958a3a3b47dce08e36822af9ad4f6dda17df99a2
parent09abdb8a65ee1e935a63add6d7f6398d36be7ebe (diff)
Finally fix diff names (#13136) (#13139)
Backport #13136 it is possible to have an ambiguous line here. if they needed to be and if one was quoted then both would be. Both of these were wrong. I have now discovered `--src-prefix` and `--dst-prefix` which means that we can set this in such a way to force the git diff to always be unambiguous. Therefore this PR rollsback most of the changes in #12771 and uses these options to fix this. Signed-off-by: Andrew Thornton <art27@cantab.net>
-rw-r--r--modules/repofiles/temp_repo.go2
-rw-r--r--services/gitdiff/gitdiff.go164
-rw-r--r--services/gitdiff/gitdiff_test.go39
3 files changed, 81 insertions, 124 deletions
diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go
index 89f9b0b20..ca35b51c2 100644
--- a/modules/repofiles/temp_repo.go
+++ b/modules/repofiles/temp_repo.go
@@ -278,7 +278,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
var diff *gitdiff.Diff
var finalErr error
- if err := git.NewCommand("diff-index", "--cached", "-p", "HEAD").
+ if err := git.NewCommand("diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD").
RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) error {
_ = stdoutWriter.Close()
diff, finalErr = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader)
diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index a2d098e57..22f031ab7 100644
--- a/services/gitdiff/gitdiff.go
+++ b/services/gitdiff/gitdiff.go
@@ -448,46 +448,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
}
line := linebuf.String()
- if strings.HasPrefix(line, "--- ") {
- if line[4] == '"' {
- fmt.Sscanf(line[4:], "%q", &curFile.OldName)
- } else {
- curFile.OldName = line[4:]
- if strings.Contains(curFile.OldName, " ") {
- // Git adds a terminal \t if there is a space in the name
- curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
- }
- }
- if curFile.OldName[0:2] == "a/" {
- curFile.OldName = curFile.OldName[2:]
- }
- continue
- } else if strings.HasPrefix(line, "+++ ") {
- if line[4] == '"' {
- fmt.Sscanf(line[4:], "%q", &curFile.Name)
- } else {
- curFile.Name = line[4:]
- if strings.Contains(curFile.Name, " ") {
- // Git adds a terminal \t if there is a space in the name
- curFile.Name = curFile.Name[:len(curFile.Name)-1]
- }
- }
- if curFile.Name[0:2] == "b/" {
- curFile.Name = curFile.Name[2:]
- }
- curFile.IsRenamed = (curFile.Name != curFile.OldName) && !(curFile.IsCreated || curFile.IsDeleted)
- if curFile.IsDeleted {
- curFile.Name = curFile.OldName
- curFile.OldName = ""
- } else if curFile.IsCreated {
- curFile.OldName = ""
- }
- continue
- } else if len(line) == 0 {
- continue
- }
-
- if strings.HasPrefix(line, "+++") || strings.HasPrefix(line, "---") || len(line) == 0 {
+ if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") || len(line) == 0 {
continue
}
@@ -571,10 +532,42 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
break
}
+ // Note: In case file name is surrounded by double quotes (it happens only in git-shell).
+ // e.g. diff --git "a/xxx" "b/xxx"
+ var a string
+ var b string
+
+ rd := strings.NewReader(line[len(cmdDiffHead):])
+ char, _ := rd.ReadByte()
+ _ = rd.UnreadByte()
+ if char == '"' {
+ fmt.Fscanf(rd, "%q ", &a)
+ if a[0] == '\\' {
+ a = a[1:]
+ }
+ } else {
+ fmt.Fscanf(rd, "%s ", &a)
+ }
+ char, _ = rd.ReadByte()
+ _ = rd.UnreadByte()
+ if char == '"' {
+ fmt.Fscanf(rd, "%q", &b)
+ if b[0] == '\\' {
+ b = b[1:]
+ }
+ } else {
+ fmt.Fscanf(rd, "%s", &b)
+ }
+ a = a[2:]
+ b = b[2:]
+
curFile = &DiffFile{
- Index: len(diff.Files) + 1,
- Type: DiffFileChange,
- Sections: make([]*DiffSection, 0, 10),
+ Name: b,
+ OldName: a,
+ Index: len(diff.Files) + 1,
+ Type: DiffFileChange,
+ Sections: make([]*DiffSection, 0, 10),
+ IsRenamed: a != b,
}
diff.Files = append(diff.Files, curFile)
curFileLinesCount = 0
@@ -583,7 +576,6 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
curFileLFSPrefix = false
// Check file diff type and is submodule.
- loop:
for {
line, err := input.ReadString('\n')
if err != nil {
@@ -594,67 +586,29 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
}
}
- if curFile.Type != DiffFileRename {
- switch {
- case strings.HasPrefix(line, "new file"):
- curFile.Type = DiffFileAdd
- curFile.IsCreated = true
- case strings.HasPrefix(line, "deleted"):
- curFile.Type = DiffFileDel
- curFile.IsDeleted = true
- case strings.HasPrefix(line, "index"):
- curFile.Type = DiffFileChange
- case strings.HasPrefix(line, "similarity index 100%"):
- curFile.Type = DiffFileRename
- }
- if curFile.Type > 0 && curFile.Type != DiffFileRename {
- if strings.HasSuffix(line, " 160000\n") {
- curFile.IsSubmodule = true
- }
- break
- }
- } else {
- switch {
- case strings.HasPrefix(line, "rename from "):
- if line[12] == '"' {
- fmt.Sscanf(line[12:], "%q", &curFile.OldName)
- } else {
- curFile.OldName = line[12:]
- curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
- }
- case strings.HasPrefix(line, "rename to "):
- if line[10] == '"' {
- fmt.Sscanf(line[10:], "%q", &curFile.Name)
- } else {
- curFile.Name = line[10:]
- curFile.Name = curFile.Name[:len(curFile.Name)-1]
- }
- curFile.IsRenamed = true
- break loop
- case strings.HasPrefix(line, "copy from "):
- if line[10] == '"' {
- fmt.Sscanf(line[10:], "%q", &curFile.OldName)
- } else {
- curFile.OldName = line[10:]
- curFile.OldName = curFile.OldName[:len(curFile.OldName)-1]
- }
- case strings.HasPrefix(line, "copy to "):
- if line[8] == '"' {
- fmt.Sscanf(line[8:], "%q", &curFile.Name)
- } else {
- curFile.Name = line[8:]
- curFile.Name = curFile.Name[:len(curFile.Name)-1]
- }
- curFile.IsRenamed = true
- curFile.Type = DiffFileCopy
- break loop
- default:
- if strings.HasSuffix(line, " 160000\n") {
- curFile.IsSubmodule = true
- } else {
- break loop
- }
+ switch {
+ case strings.HasPrefix(line, "copy from "):
+ curFile.IsRenamed = true
+ curFile.Type = DiffFileCopy
+ case strings.HasPrefix(line, "copy to "):
+ curFile.IsRenamed = true
+ curFile.Type = DiffFileCopy
+ case strings.HasPrefix(line, "new file"):
+ curFile.Type = DiffFileAdd
+ curFile.IsCreated = true
+ case strings.HasPrefix(line, "deleted"):
+ curFile.Type = DiffFileDel
+ curFile.IsDeleted = true
+ case strings.HasPrefix(line, "index"):
+ curFile.Type = DiffFileChange
+ case strings.HasPrefix(line, "similarity index 100%"):
+ curFile.Type = DiffFileRename
+ }
+ if curFile.Type > 0 {
+ if strings.HasSuffix(line, " 160000\n") {
+ curFile.IsSubmodule = true
}
+ break
}
}
}
@@ -723,7 +677,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
parentCommit, _ := commit.Parent(0)
actualBeforeCommitID = parentCommit.ID.String()
}
- diffArgs := []string{"diff", "-M"}
+ diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"}
if len(whitespaceBehavior) != 0 {
diffArgs = append(diffArgs, whitespaceBehavior)
}
diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go
index 36731ed58..e0008f032 100644
--- a/services/gitdiff/gitdiff_test.go
+++ b/services/gitdiff/gitdiff_test.go
@@ -56,9 +56,9 @@ func TestParsePatch_singlefile(t *testing.T) {
tests := []testcase{
{
name: "readme.md2readme.md",
- gitdiff: `diff --git "a/README.md" "b/README.md"
---- a/README.md
-+++ b/README.md
+ gitdiff: `diff --git "\\a/README.md" "\\b/README.md"
+--- "\\a/README.md"
++++ "\\b/README.md"
@@ -1,3 +1,6 @@
# gitea-github-migrator
+
@@ -68,9 +68,10 @@ func TestParsePatch_singlefile(t *testing.T) {
+ cut off
+ cut off
`,
- addition: 4,
- deletion: 1,
- filename: "README.md",
+ addition: 4,
+ deletion: 1,
+ filename: "README.md",
+ oldFilename: "README.md",
},
{
name: "A \\ B",
@@ -85,16 +86,17 @@ func TestParsePatch_singlefile(t *testing.T) {
Docker Pulls
+ cut off
+ cut off`,
- addition: 4,
- deletion: 1,
- filename: "A \\ B",
+ addition: 4,
+ deletion: 1,
+ filename: "A \\ B",
+ oldFilename: "A \\ B",
},
{
name: "really weird filename",
- gitdiff: `diff --git a/a b/file b/a a/file b/a b/file b/a a/file
+ gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/file b/a a/file"
index d2186f1..f5c8ed2 100644
---- a/a b/file b/a a/file
-+++ b/a b/file b/a a/file
+--- "\\a/a b/file b/a a/file"
++++ "\\b/a b/file b/a a/file"
@@ -1,3 +1,2 @@
Create a weird file.
@@ -107,10 +109,10 @@ index d2186f1..f5c8ed2 100644
},
{
name: "delete file with blanks",
- gitdiff: `diff --git a/file with blanks b/file with blanks
+ gitdiff: `diff --git "\\a/file with blanks" "\\b/file with blanks"
deleted file mode 100644
index 898651a..0000000
---- a/file with blanks
+--- "\\a/file with blanks"
+++ /dev/null
@@ -1,5 +0,0 @@
-a blank file
@@ -119,9 +121,10 @@ index 898651a..0000000
-
-the 5th line is the last
`,
- addition: 0,
- deletion: 5,
- filename: "file with blanks",
+ addition: 0,
+ deletion: 5,
+ filename: "file with blanks",
+ oldFilename: "file with blanks",
},
{
name: "rename a—as",
@@ -137,7 +140,7 @@ rename to "a\342\200\224as"
},
{
name: "rename with spaces",
- gitdiff: `diff --git a/a b/file b/a a/file b/a b/a a/file b/b file
+ gitdiff: `diff --git "\\a/a b/file b/a a/file" "\\b/a b/a a/file b/b file"
similarity index 100%
rename from a b/file b/a a/file
rename to a b/a a/file b/b file