From d0b334b24409ddc190a7010be0072d87df6b6bfe Mon Sep 17 00:00:00 2001 From: Jesse Ezell Date: Thu, 20 Mar 2014 21:56:41 -0700 Subject: [PATCH] cleanup and refactor diff / patch --- diff.go | 258 +++++++++++++++++++++++++++++++------------------- diff_test.go | 41 ++++++++ git.go | 13 +-- git_test.go | 32 ++++++- patch.go | 15 ++- repository.go | 17 ---- 6 files changed, 252 insertions(+), 124 deletions(-) create mode 100644 diff_test.go diff --git a/diff.go b/diff.go index bf08c8b..31000bc 100644 --- a/diff.go +++ b/diff.go @@ -12,72 +12,75 @@ import ( ) type DiffFlag int + const ( - DiffFlagBinary = DiffFlag(C.GIT_DIFF_FLAG_BINARY) - DiffFlagNotBinary = C.GIT_DIFF_FLAG_NOT_BINARY - DiffFlagValidOid = C.GIT_DIFF_FLAG_VALID_OID + DiffFlagBinary DiffFlag = C.GIT_DIFF_FLAG_BINARY + DiffFlagNotBinary = C.GIT_DIFF_FLAG_NOT_BINARY + DiffFlagValidOid = C.GIT_DIFF_FLAG_VALID_OID ) type Delta int + const ( - DeltaUnmodified = Delta(C.GIT_DELTA_UNMODIFIED) - DeltaAdded = C.GIT_DELTA_ADDED - DeltaDeleted = C.GIT_DELTA_DELETED - DeltaModified = C.GIT_DELTA_MODIFIED - DeltaRenamed = C.GIT_DELTA_RENAMED - DeltaCopied = C.GIT_DELTA_COPIED - DeltaIgnored = C.GIT_DELTA_IGNORED - DeltaUntracked = C.GIT_DELTA_UNTRACKED - DeltaTypeChange = C.GIT_DELTA_TYPECHANGE + DeltaUnmodified Delta = C.GIT_DELTA_UNMODIFIED + DeltaAdded = C.GIT_DELTA_ADDED + DeltaDeleted = C.GIT_DELTA_DELETED + DeltaModified = C.GIT_DELTA_MODIFIED + DeltaRenamed = C.GIT_DELTA_RENAMED + DeltaCopied = C.GIT_DELTA_COPIED + DeltaIgnored = C.GIT_DELTA_IGNORED + DeltaUntracked = C.GIT_DELTA_UNTRACKED + DeltaTypeChange = C.GIT_DELTA_TYPECHANGE ) type DiffLineType int + const ( - DiffLineContext = DiffLineType(C.GIT_DIFF_LINE_CONTEXT) - DiffLineAddition = C.GIT_DIFF_LINE_ADDITION - DiffLineDeletion = C.GIT_DIFF_LINE_DELETION - DiffLineContextEOFNL = C.GIT_DIFF_LINE_CONTEXT_EOFNL - DiffLineAddEOFNL = C.GIT_DIFF_LINE_ADD_EOFNL - DiffLineDelEOFNL = C.GIT_DIFF_LINE_DEL_EOFNL + DiffLineContext DiffLineType = C.GIT_DIFF_LINE_CONTEXT + DiffLineAddition = C.GIT_DIFF_LINE_ADDITION + DiffLineDeletion = C.GIT_DIFF_LINE_DELETION + DiffLineContextEOFNL = C.GIT_DIFF_LINE_CONTEXT_EOFNL + DiffLineAddEOFNL = C.GIT_DIFF_LINE_ADD_EOFNL + DiffLineDelEOFNL = C.GIT_DIFF_LINE_DEL_EOFNL DiffLineFileHdr = C.GIT_DIFF_LINE_FILE_HDR DiffLineHunkHdr = C.GIT_DIFF_LINE_HUNK_HDR - DiffLineBinary = C.GIT_DIFF_LINE_BINARY + DiffLineBinary = C.GIT_DIFF_LINE_BINARY ) type DiffFile struct { - Path string - Oid *Oid - Size int + Path string + Oid *Oid + Size int Flags DiffFlag - Mode uint16 + Mode uint16 } -func newDiffFile(file *C.git_diff_file) *DiffFile { +func newDiffFileFromC(file *C.git_diff_file) *DiffFile { return &DiffFile{ - Path: C.GoString(file.path), - Oid: newOidFromC(&file.oid), - Size: int(file.size), + Path: C.GoString(file.path), + Oid: newOidFromC(&file.oid), + Size: int(file.size), Flags: DiffFlag(file.flags), - Mode: uint16(file.mode), + Mode: uint16(file.mode), } } type DiffDelta struct { - Status Delta - Flags DiffFlag + Status Delta + Flags DiffFlag Similarity uint16 - OldFile *DiffFile - NewFile *DiffFile + OldFile *DiffFile + NewFile *DiffFile } -func newDiffDelta(delta *C.git_diff_delta) *DiffDelta { +func newDiffDeltaFromC(delta *C.git_diff_delta) *DiffDelta { return &DiffDelta{ - Status: Delta(delta.status), - Flags: DiffFlag(delta.flags), + Status: Delta(delta.status), + Flags: DiffFlag(delta.flags), Similarity: uint16(delta.similarity), - OldFile: newDiffFile(&delta.old_file), - NewFile: newDiffFile(&delta.new_file), + OldFile: newDiffFileFromC(&delta.old_file), + NewFile: newDiffFileFromC(&delta.new_file), } } @@ -86,38 +89,38 @@ type DiffHunk struct { OldLines int NewStart int NewLines int - Header string + Header string DiffDelta } -func newDiffHunk(delta *C.git_diff_delta, hunk *C.git_diff_hunk) *DiffHunk { +func newDiffHunkFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk) *DiffHunk { return &DiffHunk{ - OldStart: int(hunk.old_start), - OldLines: int(hunk.old_lines), - NewStart: int(hunk.new_start), - NewLines: int(hunk.new_lines), - Header: C.GoStringN(&hunk.header[0], C.int(hunk.header_len)), - DiffDelta: *newDiffDelta(delta), + OldStart: int(hunk.old_start), + OldLines: int(hunk.old_lines), + NewStart: int(hunk.new_start), + NewLines: int(hunk.new_lines), + Header: C.GoStringN(&hunk.header[0], C.int(hunk.header_len)), + DiffDelta: *newDiffDeltaFromC(delta), } } type DiffLine struct { - Origin DiffLineType + Origin DiffLineType OldLineno int NewLineno int - NumLines int - Content string + NumLines int + Content string DiffHunk } -func newDiffLine(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line) *DiffLine { +func newDiffLineFromC(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line) *DiffLine { return &DiffLine{ - Origin: DiffLineType(line.origin), + Origin: DiffLineType(line.origin), OldLineno: int(line.old_lineno), NewLineno: int(line.new_lineno), - NumLines: int(line.num_lines), - Content: C.GoStringN(line.content, C.int(line.content_len)), - DiffHunk: *newDiffHunk(delta, hunk), + NumLines: int(line.num_lines), + Content: C.GoStringN(line.content, C.int(line.content_len)), + DiffHunk: *newDiffHunkFromC(delta, hunk), } } @@ -125,7 +128,7 @@ type Diff struct { ptr *C.git_diff } -func newDiff(ptr *C.git_diff) *Diff { +func newDiffFromC(ptr *C.git_diff) *Diff { if ptr == nil { return nil } @@ -138,100 +141,165 @@ func newDiff(ptr *C.git_diff) *Diff { return diff } -func (diff *Diff) Free() { +func (diff *Diff) Free() error { + if diff.ptr != nil { + return ErrInvalid + } runtime.SetFinalizer(diff, nil) C.git_diff_free(diff.ptr) + return nil } -func (diff *Diff) forEachFileWrap(ch chan *DiffDelta) { - C._go_git_diff_foreach(diff.ptr, 1, 0, 0, unsafe.Pointer(&ch)) - close(ch) +type diffForEachFileData struct { + Callback DiffForEachFileCallback + Error error } -func (diff *Diff) ForEachFile() chan *DiffDelta { - ch := make(chan *DiffDelta, 0) - go diff.forEachFileWrap(ch) - return ch +func (diff *Diff) ForEachFile(cb DiffForEachFileCallback) error { + if diff.ptr != nil { + return ErrInvalid + } + + data := &diffForEachFileData{ + Callback: cb, + } + ecode := C._go_git_diff_foreach(diff.ptr, 1, 0, 0, unsafe.Pointer(&data)) + if ecode < 0 { + return data.Error + } + return nil } //export diffForEachFileCb func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe.Pointer) int { - ch := *(*chan *DiffDelta)(payload) + data := *diffForEachFileData(payload) - select { - case ch <-newDiffDelta(delta): - case <-ch: + err := data.Callback(newDiffDeltaFromC(delta)) + if err != nil { + data.Error = err return -1 } return 0 } -func (diff *Diff) forEachHunkWrap(ch chan *DiffHunk) { - C._go_git_diff_foreach(diff.ptr, 0, 1, 0, unsafe.Pointer(&ch)) - close(ch) +type diffForEachHunkData struct { + Callback DiffForEachHunkCallback + Error error } -func (diff *Diff) ForEachHunk() chan *DiffHunk { - ch := make(chan *DiffHunk, 0) - go diff.forEachHunkWrap(ch) - return ch +type DiffForEachHunkCallback func(*DiffHunk) error + +func (diff *Diff) ForEachHunk(cb DiffForEachHunkCallback) error { + if diff.ptr != nil { + return ErrInvalid + } + data := &diffForEachHunkData{ + Callback: cb, + } + ecode := C._go_git_diff_foreach(diff.ptr, 0, 1, 0, unsafe.Pointer(data)) + if ecode < 0 { + return data.Error + } + return nil } //export diffForEachHunkCb func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload unsafe.Pointer) int { - ch := *(*chan *DiffHunk)(payload) + data := *diffForEachHunkData(payload) - select { - case ch <-newDiffHunk(delta, hunk): - case <-ch: + err := data.Callback(newDiffHunkFromC(delta, hunk)) + if err < 0 { + data.Error = err return -1 } return 0 } -func (diff *Diff) forEachLineWrap(ch chan *DiffLine) { - C._go_git_diff_foreach(diff.ptr, 0, 0, 1, unsafe.Pointer(&ch)) - close(ch) +type diffForEachLineData struct { + Callback DiffForEachLineCallback + Error error } -func (diff *Diff) ForEachLine() chan *DiffLine { - ch := make(chan *DiffLine, 0) - go diff.forEachLineWrap(ch) - return ch +type DiffForEachLineCallback func(*DiffLine) error + +func (diff *Diff) ForEachLine(cb DiffForEachLineCallback) error { + if diff.ptr != nil { + return ErrInvalid + } + + data := &diffForEachLineData{ + Callback: cb, + } + + ecode := C._go_git_diff_foreach(diff.ptr, 0, 0, 1, unsafe.Pointer(data)) + if ecode < 0 { + return data.Error + } + return nil } //export diffForEachLineCb func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, payload unsafe.Pointer) int { - ch := *(*chan *DiffLine)(payload) - select { - case ch <-newDiffLine(delta, hunk, line): - case <-ch: + data := *diffForEachLineData(payload) + + err := data.Callback(newDiffLineFromC(delta, hunk, line)) + if err != nil { + data.Error = err return -1 } return 0 } -func (diff *Diff) NumDeltas() int { - return int(C.git_diff_num_deltas(diff.ptr)) +func (diff *Diff) NumDeltas() (int, error) { + if diff.ptr != nil { + return -1, ErrInvalid + } + return int(C.git_diff_num_deltas(diff.ptr)), nil } -func (diff *Diff) GetDelta(index int) *DiffDelta { +func (diff *Diff) GetDelta(index int) (*DiffDelta, error) { + if diff.ptr != nil { + return nil, ErrInvalid + } ptr := C.git_diff_get_delta(diff.ptr, C.size_t(index)) if ptr == nil { return nil } - return newDiffDelta(ptr) + return newDiffDeltaFromC(ptr), nil } -func (diff *Diff) Patch(deltaIndex int) *Patch { +func (diff *Diff) Patch(deltaIndex int) (*Patch, error) { + if diff.ptr != nil { + return nil, ErrInvalid + } var patchPtr *C.git_patch - C.git_patch_from_diff(&patchPtr, diff.ptr, C.size_t(deltaIndex)) + ecode := C.git_patch_from_diff(&patchPtr, diff.ptr, C.size_t(deltaIndex)) + if ecode < 0 { + return nil, MakeGitError(ecode) + } - return newPatch(patchPtr) + return newPatchFromC(patchPtr), nil +} + +func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) *Diff { + var diffPtr *C.git_diff + var oldPtr, newPtr *C.git_tree + + if oldTree != nil { + oldPtr = oldTree.gitObject.ptr + } + + if newTree != nil { + newPtr = newTree.gitObject.ptr + } + + C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, nil) + + return newDiff(diffPtr) } diff --git a/diff_test.go b/diff_test.go new file mode 100644 index 0000000..15aabc5 --- /dev/null +++ b/diff_test.go @@ -0,0 +1,41 @@ +package git + +import ( + "testing" +) + +func TestDiffTreeToTree(t *testing.T) { + repo := createTestRepo(t) + defer repo.Free() + defer os.RemoveAll(repo.Workdir()) + + _, originalTreeId := seedTestRepo(t, repo) + originalTree, err := repo.LookupTree(originalTreeId) + + checkFatal(t, err) + updateReadme(t, repo, "file changed\n") + + _, newTreeId := seedTestRepo(t, repo) + newTree, err := repo.LookupTree(newTreeId) + checkFatal(t, err) + + diff, err := repo.DiffTreeToTree(originalTreeId, newTreeId) + checkFatal(t, err) + + files := make([]string, 0) + + err := diff.ForEachFile(func(file *DiffFile) error { + files = append(files, file.Path) + return nil + }) + + checkFatal(t, err) + + if len(files) != 0 { + t.Fatal("Incorrect number of files in diff") + } + + if files[0] != "README" { + t.Fatal("File in diff was expected to be README") + } +} diff --git a/git.go b/git.go index 28196c8..4c798b3 100644 --- a/git.go +++ b/git.go @@ -10,8 +10,8 @@ import ( "bytes" "errors" "runtime" - "unsafe" "strings" + "unsafe" ) const ( @@ -22,6 +22,7 @@ const ( var ( ErrIterOver = errors.New("Iteration is over") + ErrInvalid = errors.New("Invalid state for operation") ) func init() { @@ -93,7 +94,7 @@ func (oid *Oid) Equal(oid2 *Oid) bool { } func (oid *Oid) IsZero() bool { - for _, a := range(oid.bytes) { + for _, a := range oid.bytes { if a != '0' { return false } @@ -131,10 +132,10 @@ func ShortenOids(ids []*Oid, minlen int) (int, error) { type GitError struct { Message string - Code int + Code int } -func (e GitError) Error() string{ +func (e GitError) Error() string { return e.Message } @@ -147,14 +148,14 @@ func LastError() error { } func cbool(b bool) C.int { - if (b) { + if b { return C.int(1) } return C.int(0) } func ucbool(b bool) C.uint { - if (b) { + if b { return C.uint(1) } return C.uint(0) diff --git a/git_test.go b/git_test.go index 52aea1d..2f586b5 100644 --- a/git_test.go +++ b/git_test.go @@ -1,8 +1,8 @@ package git import ( - "testing" "io/ioutil" + "testing" "time" ) @@ -14,7 +14,7 @@ func createTestRepo(t *testing.T) *Repository { checkFatal(t, err) tmpfile := "README" - err = ioutil.WriteFile(path + "/" + tmpfile, []byte("foo\n"), 0644) + err = ioutil.WriteFile(path+"/"+tmpfile, []byte("foo\n"), 0644) checkFatal(t, err) return repo @@ -45,3 +45,31 @@ func seedTestRepo(t *testing.T, repo *Repository) (*Oid, *Oid) { return commitId, treeId } +func updateReadme(t *testing.T, repo *Repository, content string) (*Oid, *Oid) { + loc, err := time.LoadLocation("Europe/Berlin") + checkFatal(t, err) + sig := &Signature{ + Name: "Rand Om Hacker", + Email: "random@hacker.com", + When: time.Date(2013, 03, 06, 14, 30, 0, 0, loc), + } + + tmpfile := "README" + err = ioutil.WriteFile(repo.Path()+"/"+tmpfile, []byte(content), 0644) + checkFatal(t, err) + + idx, err := repo.Index() + checkFatal(t, err) + err = idx.AddByPath("README") + checkFatal(t, err) + treeId, err := idx.WriteTree() + checkFatal(t, err) + + message := "This is a commit\n" + tree, err := repo.LookupTree(treeId) + checkFatal(t, err) + commitId, err := repo.CreateCommit("HEAD", sig, sig, message, tree) + checkFatal(t, err) + + return commitId, treeId +} diff --git a/patch.go b/patch.go index 561786e..f2016c4 100644 --- a/patch.go +++ b/patch.go @@ -12,7 +12,7 @@ type Patch struct { ptr *C.git_patch } -func newPatch(ptr *C.git_patch) *Patch { +func newPatchFromC(ptr *C.git_patch) *Patch { if ptr == nil { return nil } @@ -25,13 +25,20 @@ func newPatch(ptr *C.git_patch) *Patch { return patch } -func (patch *Patch) Free() { +func (patch *Patch) Free() error { + if patch.ptr == nil { + return ErrInvalid + } runtime.SetFinalizer(patch, nil) C.git_patch_free(patch.ptr) + return nil } -func (patch *Patch) String() string { +func (patch *Patch) String() (string, error) { + if diff.ptr != nil { + return "", ErrInvalid + } var cptr *C.char C.git_patch_to_str(&cptr, patch.ptr) - return C.GoString(cptr) + return C.GoString(cptr), nil } diff --git a/repository.go b/repository.go index 58d3487..8bb061b 100644 --- a/repository.go +++ b/repository.go @@ -255,23 +255,6 @@ func (v *Repository) CreateCommit( return oid, nil } -func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree) *Diff { - var diffPtr *C.git_diff - var oldPtr, newPtr *C.git_tree - - if oldTree != nil { - oldPtr = oldTree.gitObject.ptr - } - - if newTree != nil { - newPtr = newTree.gitObject.ptr - } - - C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, nil) - - return newDiff(diffPtr) -} - func (v *Odb) Free() { runtime.SetFinalizer(v, nil) C.git_odb_free(v.ptr)