From e8da5affe7ada724b71c6603c81d534752d4be6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 14 Jan 2015 13:52:52 +0100 Subject: [PATCH 01/36] Link dynamically to libgit2 With libgit2 v0.22 released, we can expect its API and ABI to remain stable when installed on the system. Linking dynamically allows us to use the go tool alone to build and install the package. --- git.go | 1 + 1 file changed, 1 insertion(+) diff --git a/git.go b/git.go index 8e78710..9496d2d 100644 --- a/git.go +++ b/git.go @@ -3,6 +3,7 @@ package git /* #include #include +#cgo pkg-config: libgit2 */ import "C" import ( From 43f6a750660c3f6a344a54a3ee0772cc57d7c162 Mon Sep 17 00:00:00 2001 From: Matthew Donoughe Date: Fri, 13 Feb 2015 21:40:02 -0500 Subject: [PATCH 02/36] remove static git support files --- .gitmodules | 3 --- Makefile | 11 ++++------- script/build-libgit2-static.sh | 19 ------------------- script/with-static.sh | 12 ------------ vendor/libgit2 | 1 - 5 files changed, 4 insertions(+), 42 deletions(-) delete mode 100644 .gitmodules delete mode 100755 script/build-libgit2-static.sh delete mode 100755 script/with-static.sh delete mode 160000 vendor/libgit2 diff --git a/.gitmodules b/.gitmodules deleted file mode 100644 index 8eb5872..0000000 --- a/.gitmodules +++ /dev/null @@ -1,3 +0,0 @@ -[submodule "vendor/libgit2"] - path = vendor/libgit2 - url = https://github.com/libgit2/libgit2 diff --git a/Makefile b/Makefile index 3040857..9c42283 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,8 @@ default: test -build-libgit2: - ./script/build-libgit2-static.sh - -test: build-libgit2 +test: go run script/check-MakeGitError-thread-lock.go - ./script/with-static.sh go test ./... + go test ./... -install: build-libgit2 - ./script/with-static.sh go install ./... +install: + go install ./... diff --git a/script/build-libgit2-static.sh b/script/build-libgit2-static.sh deleted file mode 100755 index 5723721..0000000 --- a/script/build-libgit2-static.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/sh - -set -ex - -VENDORED_PATH=vendor/libgit2 - -cd $VENDORED_PATH && -mkdir -p install/lib && -mkdir -p build && -cd build && -cmake -DTHREADSAFE=ON \ - -DBUILD_CLAR=OFF \ - -DBUILD_SHARED_LIBS=OFF \ - -DCMAKE_C_FLAGS=-fPIC \ - -DCMAKE_BUILD_TYPE="RelWithDebInfo" \ - -DCMAKE_INSTALL_PREFIX=../install \ - .. && - -cmake --build . diff --git a/script/with-static.sh b/script/with-static.sh deleted file mode 100755 index 3f60e31..0000000 --- a/script/with-static.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/sh - -set -ex - -export BUILD="$PWD/vendor/libgit2/build" -export PCFILE="$BUILD/libgit2.pc" - -FLAGS=$(pkg-config --static --libs $PCFILE) || exit 1 -export CGO_LDFLAGS="$BUILD/libgit2.a -L$BUILD ${FLAGS}" -export CGO_CFLAGS="-I$PWD/vendor/libgit2/include" - -$@ diff --git a/vendor/libgit2 b/vendor/libgit2 deleted file mode 160000 index 04bdd97..0000000 --- a/vendor/libgit2 +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 04bdd97f2b63793a8720fd19007911e946ba3c55 From 5be2387aeb62d70e8baba793d0571142c5744989 Mon Sep 17 00:00:00 2001 From: Matthew Donoughe Date: Fri, 13 Feb 2015 21:40:50 -0500 Subject: [PATCH 03/36] install libgit2 on travis ci --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index f84d07e..efa101f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,10 @@ language: go +install: + - wget -O libgit2-0.22.1.tar.gz https://github.com/libgit2/libgit2/archive/v0.22.1.tar.gz + - tar -xzvf libgit2-0.22.1.tar.gz + - cd libgit2-0.22.1 && mkdir build && cd build && cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr .. && make && make install + go: - 1.1 - 1.2 From 0d3cc8be8ac9d9ce6c63f78239e335a6bf40fa28 Mon Sep 17 00:00:00 2001 From: Matthew Donoughe Date: Fri, 13 Feb 2015 22:00:41 -0500 Subject: [PATCH 04/36] sudo make install? --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index efa101f..3d164f9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ language: go install: - wget -O libgit2-0.22.1.tar.gz https://github.com/libgit2/libgit2/archive/v0.22.1.tar.gz - tar -xzvf libgit2-0.22.1.tar.gz - - cd libgit2-0.22.1 && mkdir build && cd build && cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr .. && make && make install + - cd libgit2-0.22.1 && mkdir build && cd build && cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr .. && make && sudo make install go: - 1.1 From 8bb707b8275163c88e005e4fc7de8d33e69bce31 Mon Sep 17 00:00:00 2001 From: Matthew Donoughe Date: Fri, 13 Feb 2015 22:07:54 -0500 Subject: [PATCH 05/36] return to correct directory for main build --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3d164f9..6d657ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,12 @@ language: go install: + - cd "${HOME}" - wget -O libgit2-0.22.1.tar.gz https://github.com/libgit2/libgit2/archive/v0.22.1.tar.gz - tar -xzvf libgit2-0.22.1.tar.gz - - cd libgit2-0.22.1 && mkdir build && cd build && cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr .. && make && sudo make install + - cd libgit2-0.22.1 && mkdir build && cd build + - cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr .. && make && sudo make install + - cd "${TRAVIS_BUILD_DIR}" go: - 1.1 From 675b9b0df93ecd96aa9e5245676a4fac6df93a00 Mon Sep 17 00:00:00 2001 From: Matthew Donoughe Date: Fri, 13 Feb 2015 22:13:57 -0500 Subject: [PATCH 06/36] use v22 travis results --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9995707..7ebf481 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ git2go ====== -[![GoDoc](https://godoc.org/github.com/libgit2/git2go?status.svg)](http://godoc.org/github.com/libgit2/git2go) [![Build Status](https://travis-ci.org/libgit2/git2go.svg?branch=master)](https://travis-ci.org/libgit2/git2go) +[![GoDoc](https://godoc.org/github.com/libgit2/git2go?status.svg)](http://godoc.org/github.com/libgit2/git2go) [![Build Status](https://travis-ci.org/libgit2/git2go.svg?branch=v22)](https://travis-ci.org/libgit2/git2go) Go bindings for [libgit2](http://libgit2.github.com/). The master branch follows the latest libgit2 release. From 76d600f7b3633f78e5f1433c16eba4eddfdad3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 15 Mar 2015 00:45:30 +0100 Subject: [PATCH 07/36] Correct README on what master tracks The second mention still said that master tracks master. Add a mention of next which will become the branch to track upstream tip. --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index faeb5f5..51543a8 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ git2go [![GoDoc](https://godoc.org/github.com/libgit2/git2go?status.svg)](http://godoc.org/github.com/libgit2/git2go) [![Build Status](https://travis-ci.org/libgit2/git2go.svg?branch=master)](https://travis-ci.org/libgit2/git2go) -Go bindings for [libgit2](http://libgit2.github.com/). The master branch follows the latest libgit2 release. The versioned branches indicate which libgit2 version they work against. +Go bindings for [libgit2](http://libgit2.github.com/). The `master` branch follows the latest libgit2 release. The versioned branches indicate which libgit2 version they work against. Installing ---------- @@ -20,10 +20,11 @@ to use a version of git2go which will work against libgit2 v0.22 and dynamically ### From master -The master branch follows libgit2's master branch, which means there is no stable API or ABI to link against. git2go can statically link against a vendored version of libgit2. +The `next` branch follows libgit2's master branch, which means there is no stable API or ABI to link against. git2go can statically link against a vendored version of libgit2. Run `go get -d github.com/libgit2/git2go` to download the code and go to your `$GOPATH/src/github.com/libgit2/git2go` dir. From there, we need to build the C code and put it into the resulting go binary. + git checkout next git submodule update --init # get libgit2 make install @@ -37,7 +38,7 @@ libgit2 uses OpenSSL and LibSSH2 for performing encrypted network connections. F Running the tests ----------------- -For the stable version, `go test` will work as usual. For the master branch, similarly to installing, running the tests requires linking against the local libgit2 library, so the Makefile provides a wrapper +For the stable version, `go test` will work as usual. For the `next` branch, similarly to installing, running the tests requires linking against the local libgit2 library, so the Makefile provides a wrapper make test From 81d5cc0157d40e40987f570e362499ec0575cf95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 15 Mar 2015 00:53:02 +0100 Subject: [PATCH 08/36] Make travis script install to /usr/local This is the correct place for software not installed by the system's package manager. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6d657ff..f6dc304 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ install: - wget -O libgit2-0.22.1.tar.gz https://github.com/libgit2/libgit2/archive/v0.22.1.tar.gz - tar -xzvf libgit2-0.22.1.tar.gz - cd libgit2-0.22.1 && mkdir build && cd build - - cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr .. && make && sudo make install + - cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr/local .. && make && sudo make install - cd "${TRAVIS_BUILD_DIR}" go: From 063bed33a90e7d5b1ece1b6bd1aba04a69a78a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 15 Mar 2015 01:03:06 +0100 Subject: [PATCH 09/36] Add a call to ldconfig in the travis script This should help it find the library we just installed. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index f6dc304..e833a32 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,7 @@ install: - tar -xzvf libgit2-0.22.1.tar.gz - cd libgit2-0.22.1 && mkdir build && cd build - cmake -DTHREADSAFE=ON -DBUILD_CLAR=OFF -DCMAKE_C_FLAGS=-fPIC -DCMAKE_BUILD_TYPE="RelWithDebInfo" -DCMAKE_INSTALL_PREFIX=/usr/local .. && make && sudo make install + - sudo ldconfig - cd "${TRAVIS_BUILD_DIR}" go: From 1b44c0a2343cc87cb32513ef4625c590c8ecbe8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 15 Mar 2015 01:21:21 +0100 Subject: [PATCH 10/36] Add a bit more on next vs master --- README.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 51543a8..386ff83 100644 --- a/README.md +++ b/README.md @@ -8,17 +8,23 @@ Go bindings for [libgit2](http://libgit2.github.com/). The `master` branch follo Installing ---------- -This project needs libgit2, which is written in C so we need to build that as well. In order to build libgit2, you need `cmake`, `pkg-config` and a C compiler. You will also need the development packages for OpenSSL and LibSSH2 installed if you want libgit2 to support HTTPS and SSH respectively. +This project wraps the functionality provided by libgit2. If you're using a stable version, install it to your system via your system's package manger and then install git2go as usual. + +Otherwise (`next` which tracks an unstable version), we need to build libgit2 as well. In order to build it, you need `cmake`, `pkg-config` and a C compiler. You will also need the development packages for OpenSSL and LibSSH2 installed if you want libgit2 to support HTTPS and SSH respectively. ### Stable version -git2go has versioned branches which indicate which version of libgit2 they work against. Install the development package it on your system via your favourite package manager or from source and you can use a service like gopkg.in to use the appropriate version. For the libgit2 v0.22 case, you can use +git2go has `master` which tracks the latest release of libgit2, and versioned branches which indicate which version of libgit2 they work against. Install the development package it on your system via your favourite package manager or from source and you can use a service like gopkg.in to use the appropriate version. For the libgit2 v0.22 case, you can use import "gopkg.in/libgit2/git2go.v22" -to use a version of git2go which will work against libgit2 v0.22 and dynamically link to the library. +to use a version of git2go which will work against libgit2 v0.22 and dynamically link to the library. You can use -### From master + import "github.com/libgit2/git2go" + +to use the version which works against the latest release. + +### From `next` The `next` branch follows libgit2's master branch, which means there is no stable API or ABI to link against. git2go can statically link against a vendored version of libgit2. From c4b8861b34ead7b06986ea5711c2d9528628215f Mon Sep 17 00:00:00 2001 From: Geoffrey Ragot Date: Sat, 14 Mar 2015 15:27:47 +0100 Subject: [PATCH 11/36] Add possibiliy of checkout on specific path --- checkout.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/checkout.go b/checkout.go index 06d010c..c507172 100644 --- a/checkout.go +++ b/checkout.go @@ -38,6 +38,7 @@ type CheckoutOpts struct { FileMode os.FileMode // Default is 0644 or 0755 as dictated by blob FileOpenFlags int // Default is O_CREAT | O_TRUNC | O_WRONLY TargetDirectory string // Alternative checkout path to workdir + Paths []string } func (opts *CheckoutOpts) toC() *C.git_checkout_options { @@ -65,6 +66,11 @@ func populateCheckoutOpts(ptr *C.git_checkout_options, opts *CheckoutOpts) *C.gi if opts.TargetDirectory != "" { ptr.target_directory = C.CString(opts.TargetDirectory) } + if len(opts.Paths) > 0 { + ptr.paths.strings = makeCStringsFromStrings(opts.Paths) + ptr.paths.count = C.size_t(len(opts.Paths)) + } + return ptr } @@ -73,6 +79,9 @@ func freeCheckoutOpts(ptr *C.git_checkout_options) { return } C.free(unsafe.Pointer(ptr.target_directory)) + if ptr.paths.count > 0 { + freeStrarray(&ptr.paths) + } } // Updates files in the index and the working tree to match the content of @@ -128,4 +137,4 @@ func (v *Repository) CheckoutTree(tree *Tree, opts *CheckoutOpts) error { } return nil -} +} \ No newline at end of file From 43102043fb5311939e9ec50f7210ffb51183684a Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 11 Mar 2015 16:10:32 -0700 Subject: [PATCH 12/36] Add Commit.Amend --- commit.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/commit.go b/commit.go index 57e1a77..52f7c01 100644 --- a/commit.go +++ b/commit.go @@ -9,6 +9,7 @@ import "C" import ( "runtime" + "unsafe" ) // Commit @@ -70,3 +71,40 @@ func (c *Commit) ParentId(n uint) *Oid { func (c *Commit) ParentCount() uint { return uint(C.git_commit_parentcount(c.cast_ptr)) } + +func (c *Commit) Amend(refname string, author, committer *Signature, message string, tree *Tree) (*Oid, error) { + var cref *C.char + if refname == "" { + cref = nil + } else { + cref = C.CString(refname) + defer C.free(unsafe.Pointer(cref)) + } + + cmsg := C.CString(message) + defer C.free(unsafe.Pointer(cmsg)) + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + authorSig, err := author.toC() + if err != nil { + return nil, err + } + defer C.git_signature_free(authorSig) + + committerSig, err := committer.toC() + if err != nil { + return nil, err + } + defer C.git_signature_free(committerSig) + + oid := new(Oid) + + cerr := C.git_commit_amend(oid.toC(), c.cast_ptr, cref, authorSig, committerSig, nil, cmsg, tree.cast_ptr) + if cerr < 0 { + return nil, MakeGitError(cerr) + } + + return oid, nil +} From 8622831b1128fee96da8a1829b948a49adc3dd6e Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 11 Mar 2015 16:12:22 -0700 Subject: [PATCH 13/36] Add DiffTreeToWorkdirWithIndex --- diff.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/diff.go b/diff.go index 63fa867..54e4a92 100644 --- a/diff.go +++ b/diff.go @@ -595,3 +595,28 @@ func (v *Repository) DiffTreeToWorkdir(oldTree *Tree, opts *DiffOptions) (*Diff, } return newDiffFromC(diffPtr), nil } + +func (v *Repository) DiffTreeToWorkdirWithIndex(oldTree *Tree, opts *DiffOptions) (*Diff, error) { + var diffPtr *C.git_diff + var oldPtr *C.git_tree + + if oldTree != nil { + oldPtr = oldTree.cast_ptr + } + + copts, notifyData := diffOptionsToC(opts) + defer freeDiffOptions(copts) + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + ecode := C.git_diff_tree_to_workdir_with_index(&diffPtr, v.ptr, oldPtr, copts) + if ecode < 0 { + return nil, MakeGitError(ecode) + } + + if notifyData != nil && notifyData.Diff != nil { + return notifyData.Diff, nil + } + return newDiffFromC(diffPtr), nil +} From 524cc7967be53176108e5c703818ccdd18882100 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 11 Mar 2015 16:12:39 -0700 Subject: [PATCH 14/36] Add DiffIndexToWorkdir --- diff.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/diff.go b/diff.go index 54e4a92..23469f2 100644 --- a/diff.go +++ b/diff.go @@ -620,3 +620,28 @@ func (v *Repository) DiffTreeToWorkdirWithIndex(oldTree *Tree, opts *DiffOptions } return newDiffFromC(diffPtr), nil } + +func (v *Repository) DiffIndexToWorkdir(index *Index, opts *DiffOptions) (*Diff, error) { + var diffPtr *C.git_diff + var indexPtr *C.git_index + + if index != nil { + indexPtr = index.ptr + } + + copts, notifyData := diffOptionsToC(opts) + defer freeDiffOptions(copts) + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + ecode := C.git_diff_index_to_workdir(&diffPtr, v.ptr, indexPtr, copts) + if ecode < 0 { + return nil, MakeGitError(ecode) + } + + if notifyData != nil && notifyData.Diff != nil { + return notifyData.Diff, nil + } + return newDiffFromC(diffPtr), nil +} From b3e7304abf6f0c4ef50e973acb220c40a5ddac86 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Mon, 23 Mar 2015 12:02:08 -0700 Subject: [PATCH 15/36] Add a FIXME. --- index.go | 1 + 1 file changed, 1 insertion(+) diff --git a/index.go b/index.go index 6b90758..9f37f33 100644 --- a/index.go +++ b/index.go @@ -287,6 +287,7 @@ func (v *Index) HasConflicts() bool { return C.git_index_has_conflicts(v.ptr) != 0 } +// FIXME: this might return an error func (v *Index) CleanupConflicts() { C.git_index_conflict_cleanup(v.ptr) } From 6454808f693435983456bf98a40478641d211c7f Mon Sep 17 00:00:00 2001 From: Artiom Di Date: Fri, 3 Apr 2015 14:53:15 +0300 Subject: [PATCH 16/36] Test on travis using Go1.4 too --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index e833a32..f8b7e93 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ go: - 1.1 - 1.2 - 1.3 + - 1.4 - tip matrix: From e300945a3d456af1b619447347cd19a779b7a8f5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 12:59:29 +0200 Subject: [PATCH 17/36] tests: always clean up temporary repository dirs Some test repositories are not correctly removed after the tests did run. Fix by introducing a function that is to be used for cleaning up temporary test repositories. --- blame_test.go | 4 +--- blob_test.go | 3 +-- branch_test.go | 8 +++++++- cherrypick_test.go | 2 ++ clone_test.go | 7 +++---- diff_test.go | 7 ++----- git_test.go | 13 +++++++++++++ index_test.go | 13 ++++++------- merge_test.go | 11 +++++------ note_test.go | 10 +++++----- object_test.go | 7 ++++--- odb_test.go | 10 ++++++---- patch_test.go | 3 +-- push_test.go | 6 +++--- reference_test.go | 10 +++++----- remote_test.go | 28 +++++++++------------------- revparse_test.go | 7 +++---- status_test.go | 8 +++----- submodule_test.go | 2 ++ tag_test.go | 4 ++-- 20 files changed, 83 insertions(+), 80 deletions(-) diff --git a/blame_test.go b/blame_test.go index 1785042..a2a4d38 100644 --- a/blame_test.go +++ b/blame_test.go @@ -1,15 +1,13 @@ package git import ( - "os" "reflect" "testing" ) func TestBlame(t *testing.T) { repo := createTestRepo(t) - defer repo.Free() - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId1, _ := seedTestRepo(t, repo) commitId2, _ := updateReadme(t, repo, "foo\nbar\nbaz\n") diff --git a/blob_test.go b/blob_test.go index e075192..2b5ec4f 100644 --- a/blob_test.go +++ b/blob_test.go @@ -1,13 +1,12 @@ package git import ( - "os" "testing" ) func TestCreateBlobFromBuffer(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) id, err := repo.CreateBlobFromBuffer(make([]byte, 0)) checkFatal(t, err) diff --git a/branch_test.go b/branch_test.go index 09ebeba..a0834a8 100644 --- a/branch_test.go +++ b/branch_test.go @@ -1,9 +1,13 @@ package git -import "testing" +import ( + "testing" +) func TestBranchIterator(t *testing.T) { repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + seedTestRepo(t, repo) i, err := repo.NewBranchIterator(BranchLocal) @@ -24,6 +28,8 @@ func TestBranchIterator(t *testing.T) { func TestBranchIteratorEach(t *testing.T) { repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + seedTestRepo(t, repo) i, err := repo.NewBranchIterator(BranchLocal) diff --git a/cherrypick_test.go b/cherrypick_test.go index f06dbdd..09bc524 100644 --- a/cherrypick_test.go +++ b/cherrypick_test.go @@ -34,6 +34,8 @@ func readReadme(t *testing.T, repo *Repository) string { func TestCherrypick(t *testing.T) { repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + c1, _ := seedTestRepo(t, repo) c2, _ := updateReadme(t, repo, content) diff --git a/clone_test.go b/clone_test.go index 97366bf..fd83fec 100644 --- a/clone_test.go +++ b/clone_test.go @@ -2,22 +2,21 @@ package git import ( "io/ioutil" - "os" "testing" ) func TestClone(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) seedTestRepo(t, repo) path, err := ioutil.TempDir("", "git2go") checkFatal(t, err) - _, err = Clone(repo.Path(), path, &CloneOptions{Bare: true}) - defer os.RemoveAll(path) + repo2, err := Clone(repo.Path(), path, &CloneOptions{Bare: true}) + defer cleanupTestRepo(t, repo2) checkFatal(t, err) } diff --git a/diff_test.go b/diff_test.go index fc6fed9..464fae6 100644 --- a/diff_test.go +++ b/diff_test.go @@ -2,15 +2,13 @@ package git import ( "errors" - "os" "strings" "testing" ) func TestFindSimilar(t *testing.T) { repo := createTestRepo(t) - defer repo.Free() - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) originalTree, newTree := createTestTrees(t, repo) @@ -65,8 +63,7 @@ func TestFindSimilar(t *testing.T) { func TestDiffTreeToTree(t *testing.T) { repo := createTestRepo(t) - defer repo.Free() - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) originalTree, newTree := createTestTrees(t, repo) diff --git a/git_test.go b/git_test.go index b9cf0a9..58caf71 100644 --- a/git_test.go +++ b/git_test.go @@ -2,11 +2,24 @@ package git import ( "io/ioutil" + "os" "path" "testing" "time" ) +func cleanupTestRepo(t *testing.T, r *Repository) { + var err error + if r.IsBare() { + err = os.RemoveAll(r.Path()) + } else { + err = os.RemoveAll(r.Workdir()) + } + checkFatal(t, err) + + r.Free() +} + func createTestRepo(t *testing.T) *Repository { // figure out where we can create the test repo path, err := ioutil.TempDir("", "git2go") diff --git a/index_test.go b/index_test.go index 98d9a31..647a0b8 100644 --- a/index_test.go +++ b/index_test.go @@ -2,14 +2,13 @@ package git import ( "io/ioutil" - "os" "runtime" "testing" ) func TestCreateRepoAndStage(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) idx, err := repo.Index() checkFatal(t, err) @@ -25,10 +24,10 @@ func TestCreateRepoAndStage(t *testing.T) { func TestIndexWriteTreeTo(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) repo2 := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo2) idx, err := repo.Index() checkFatal(t, err) @@ -44,7 +43,7 @@ func TestIndexWriteTreeTo(t *testing.T) { func TestIndexAddAndWriteTreeTo(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) odb, err := repo.Odb() checkFatal(t, err) @@ -74,7 +73,7 @@ func TestIndexAddAndWriteTreeTo(t *testing.T) { func TestIndexAddAllNoCallback(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) err := ioutil.WriteFile(repo.Workdir()+"/README", []byte("foo\n"), 0644) checkFatal(t, err) @@ -95,7 +94,7 @@ func TestIndexAddAllNoCallback(t *testing.T) { func TestIndexAddAllCallback(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) err := ioutil.WriteFile(repo.Workdir()+"/README", []byte("foo\n"), 0644) checkFatal(t, err) diff --git a/merge_test.go b/merge_test.go index 1eba806..0b1faca 100644 --- a/merge_test.go +++ b/merge_test.go @@ -1,13 +1,13 @@ package git import ( - "os" "testing" ) func TestMergeWithSelf(t *testing.T) { - repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + seedTestRepo(t, repo) master, err := repo.LookupReference("refs/heads/master") @@ -23,8 +23,9 @@ func TestMergeWithSelf(t *testing.T) { } func TestMergeAnalysisWithSelf(t *testing.T) { - repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + seedTestRepo(t, repo) master, err := repo.LookupReference("refs/heads/master") @@ -44,7 +45,6 @@ func TestMergeAnalysisWithSelf(t *testing.T) { } func TestMergeSameFile(t *testing.T) { - file := MergeFileInput{ Path: "test", Mode: 33188, @@ -68,8 +68,7 @@ func TestMergeSameFile(t *testing.T) { } func TestMergeTreesWithoutAncestor(t *testing.T) { repo := createTestRepo(t) - defer repo.Free() - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) _, originalTreeId := seedTestRepo(t, repo) originalTree, err := repo.LookupTree(originalTreeId) diff --git a/note_test.go b/note_test.go index f5e9c01..e6c378d 100644 --- a/note_test.go +++ b/note_test.go @@ -2,7 +2,6 @@ package git import ( "fmt" - "os" "reflect" "testing" "time" @@ -10,7 +9,7 @@ import ( func TestCreateNote(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId, _ := seedTestRepo(t, repo) @@ -29,7 +28,8 @@ func TestCreateNote(t *testing.T) { func TestNoteIterator(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + seedTestRepo(t, repo) notes := make([]*Note, 5) @@ -64,7 +64,7 @@ func TestNoteIterator(t *testing.T) { func TestRemoveNote(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId, _ := seedTestRepo(t, repo) @@ -87,7 +87,7 @@ func TestRemoveNote(t *testing.T) { func TestDefaultNoteRef(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) ref, err := repo.DefaultNoteRef() checkFatal(t, err) diff --git a/object_test.go b/object_test.go index f525351..aa295e5 100644 --- a/object_test.go +++ b/object_test.go @@ -1,13 +1,13 @@ package git import ( - "os" "testing" ) func TestObjectPoymorphism(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + commitId, treeId := seedTestRepo(t, repo) var obj Object @@ -89,7 +89,8 @@ func checkOwner(t *testing.T, repo *Repository, obj Object) { func TestObjectOwner(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + commitId, treeId := seedTestRepo(t, repo) commit, err := repo.LookupCommit(commitId) diff --git a/odb_test.go b/odb_test.go index 5e6b7ff..55ed297 100644 --- a/odb_test.go +++ b/odb_test.go @@ -3,13 +3,13 @@ package git import ( "errors" "io" - "os" "testing" ) func TestOdbStream(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + _, _ = seedTestRepo(t, repo) odb, error := repo.Odb() @@ -38,7 +38,8 @@ func TestOdbStream(t *testing.T) { func TestOdbHash(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + _, _ = seedTestRepo(t, repo) odb, error := repo.Odb() @@ -64,7 +65,8 @@ Initial commit.` func TestOdbForeach(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + _, _ = seedTestRepo(t, repo) odb, err := repo.Odb() diff --git a/patch_test.go b/patch_test.go index a061142..2d52fb4 100644 --- a/patch_test.go +++ b/patch_test.go @@ -7,8 +7,7 @@ import ( func TestPatch(t *testing.T) { repo := createTestRepo(t) - defer repo.Free() - //defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) _, originalTreeId := seedTestRepo(t, repo) originalTree, err := repo.LookupTree(originalTreeId) diff --git a/push_test.go b/push_test.go index cd708c6..e36e407 100644 --- a/push_test.go +++ b/push_test.go @@ -1,15 +1,15 @@ package git import ( - "os" "testing" ) func TestRemotePush(t *testing.T) { repo := createBareTestRepo(t) - defer os.RemoveAll(repo.Path()) + defer cleanupTestRepo(t, repo) + localRepo := createTestRepo(t) - defer os.RemoveAll(localRepo.Workdir()) + defer cleanupTestRepo(t, localRepo) remote, err := localRepo.CreateRemote("test_push", repo.Path()) checkFatal(t, err) diff --git a/reference_test.go b/reference_test.go index c7d52fb..d6b5f22 100644 --- a/reference_test.go +++ b/reference_test.go @@ -1,7 +1,6 @@ package git import ( - "os" "runtime" "sort" "testing" @@ -10,7 +9,7 @@ import ( func TestRefModification(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId, treeId := seedTestRepo(t, repo) @@ -62,7 +61,7 @@ func TestRefModification(t *testing.T) { func TestReferenceIterator(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) loc, err := time.LoadLocation("Europe/Berlin") checkFatal(t, err) @@ -140,7 +139,8 @@ func TestReferenceIterator(t *testing.T) { func TestReferenceOwner(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + commitId, _ := seedTestRepo(t, repo) ref, err := repo.CreateReference("refs/heads/foo", commitId, true, nil, "") @@ -158,7 +158,7 @@ func TestReferenceOwner(t *testing.T) { func TestUtil(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId, _ := seedTestRepo(t, repo) diff --git a/remote_test.go b/remote_test.go index 54a66ed..25ee13d 100644 --- a/remote_test.go +++ b/remote_test.go @@ -2,15 +2,13 @@ package git import ( "fmt" - "os" "testing" "time" ) func TestRefspecs(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) remote, err := repo.CreateAnonymousRemote("git://foo/bar", "refs/heads/*:refs/heads/*") checkFatal(t, err) @@ -31,8 +29,7 @@ func TestRefspecs(t *testing.T) { func TestListRemotes(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) _, err := repo.CreateRemote("test", "git://foo/bar") @@ -59,8 +56,7 @@ func assertHostname(cert *Certificate, valid bool, hostname string, t *testing.T func TestCertificateCheck(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) remote, err := repo.CreateRemote("origin", "https://github.com/libgit2/TestGitRepository") checkFatal(t, err) @@ -79,8 +75,7 @@ func TestCertificateCheck(t *testing.T) { func TestRemoteConnect(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) remote, err := repo.CreateRemote("origin", "https://github.com/libgit2/TestGitRepository") checkFatal(t, err) @@ -91,8 +86,7 @@ func TestRemoteConnect(t *testing.T) { func TestRemoteLs(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) remote, err := repo.CreateRemote("origin", "https://github.com/libgit2/TestGitRepository") checkFatal(t, err) @@ -110,8 +104,7 @@ func TestRemoteLs(t *testing.T) { func TestRemoteLsFiltering(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) remote, err := repo.CreateRemote("origin", "https://github.com/libgit2/TestGitRepository") checkFatal(t, err) @@ -137,8 +130,7 @@ func TestRemoteLsFiltering(t *testing.T) { func TestRemotePruneRefs(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) config, err := repo.Config() checkFatal(t, err) @@ -160,8 +152,7 @@ func TestRemotePruneRefs(t *testing.T) { func TestRemotePrune(t *testing.T) { remoteRepo := createTestRepo(t) - defer os.RemoveAll(remoteRepo.Workdir()) - defer remoteRepo.Free() + defer cleanupTestRepo(t, remoteRepo) head, _ := seedTestRepo(t, remoteRepo) commit, err := remoteRepo.LookupCommit(head) @@ -178,8 +169,7 @@ func TestRemotePrune(t *testing.T) { checkFatal(t, err) repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) - defer repo.Free() + defer cleanupTestRepo(t, repo) config, err := repo.Config() checkFatal(t, err) diff --git a/revparse_test.go b/revparse_test.go index c046a20..2ccdca2 100644 --- a/revparse_test.go +++ b/revparse_test.go @@ -1,13 +1,12 @@ package git import ( - "os" "testing" ) func TestRevparse(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId, _ := seedTestRepo(t, repo) @@ -19,7 +18,7 @@ func TestRevparse(t *testing.T) { func TestRevparseSingle(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) commitId, _ := seedTestRepo(t, repo) @@ -31,7 +30,7 @@ func TestRevparseSingle(t *testing.T) { func TestRevparseExt(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) _, treeId := seedTestRepo(t, repo) diff --git a/status_test.go b/status_test.go index d18fca1..5b97b00 100644 --- a/status_test.go +++ b/status_test.go @@ -2,15 +2,13 @@ package git import ( "io/ioutil" - "os" "path" "testing" ) func TestStatusFile(t *testing.T) { repo := createTestRepo(t) - defer repo.Free() - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) state := repo.State() if state != RepositoryStateNone { @@ -30,10 +28,10 @@ func TestStatusFile(t *testing.T) { func TestStatusList(t *testing.T) { repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + // This commits the test repo README, so it doesn't show up in the status list and there's a head to compare to seedTestRepo(t, repo) - defer repo.Free() - defer os.RemoveAll(repo.Workdir()) err := ioutil.WriteFile(path.Join(path.Dir(repo.Workdir()), "hello.txt"), []byte("Hello, World"), 0644) checkFatal(t, err) diff --git a/submodule_test.go b/submodule_test.go index 1c8f471..ee75d53 100644 --- a/submodule_test.go +++ b/submodule_test.go @@ -6,6 +6,8 @@ import ( func TestSubmoduleForeach(t *testing.T) { repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + seedTestRepo(t, repo) _, err := repo.AddSubmodule("http://example.org/submodule", "submodule", true) diff --git a/tag_test.go b/tag_test.go index 126cf6e..74f9fec 100644 --- a/tag_test.go +++ b/tag_test.go @@ -1,14 +1,14 @@ package git import ( - "os" "testing" "time" ) func TestCreateTag(t *testing.T) { repo := createTestRepo(t) - defer os.RemoveAll(repo.Workdir()) + defer cleanupTestRepo(t, repo) + commitId, _ := seedTestRepo(t, repo) commit, err := repo.LookupCommit(commitId) From a8ad0d204052d8bd9c4d0093cce62c54afa67188 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 19 May 2015 14:33:30 +0200 Subject: [PATCH 18/36] Index: Add ReadTree() --- index.go | 14 ++++++++++++++ index_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/index.go b/index.go index 9f37f33..009aeb6 100644 --- a/index.go +++ b/index.go @@ -240,6 +240,20 @@ func (v *Index) WriteTreeTo(repo *Repository) (*Oid, error) { return oid, nil } +// ReadTree replaces the contents of the index with those of the given +// tree +func (v *Index) ReadTree(tree *Tree) error { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + ret := C.git_index_read_tree(v.ptr, tree.cast_ptr); + if ret < 0 { + return MakeGitError(ret) + } + + return nil +} + func (v *Index) WriteTree() (*Oid, error) { oid := new(Oid) diff --git a/index_test.go b/index_test.go index 647a0b8..a1f0c9c 100644 --- a/index_test.go +++ b/index_test.go @@ -22,6 +22,34 @@ func TestCreateRepoAndStage(t *testing.T) { } } +func TestIndexReadTree(t *testing.T) { + repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + + _, _ = seedTestRepo(t, repo) + + ref, err := repo.Head() + checkFatal(t, err) + + obj, err := ref.Peel(ObjectTree); + checkFatal(t, err) + + tree := obj.(*Tree) + + idx, err := NewIndex() + checkFatal(t, err) + + err = idx.ReadTree(tree) + checkFatal(t, err) + + id, err := idx.WriteTreeTo(repo) + checkFatal(t, err) + + if tree.Id().Cmp(id) != 0 { + t.Fatalf("Read and written trees are not the same") + } +} + func TestIndexWriteTreeTo(t *testing.T) { repo := createTestRepo(t) defer cleanupTestRepo(t, repo) From d7a0495000e35d06993605a9a31a5f9823292f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 19 May 2015 14:56:01 +0200 Subject: [PATCH 19/36] Index: Add OpenIndex This lets you persist an index at an arbitrary location. --- index.go | 18 ++++++++++++++++++ index_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/index.go b/index.go index 009aeb6..2082ebd 100644 --- a/index.go +++ b/index.go @@ -96,6 +96,24 @@ func NewIndex() (*Index, error) { return &Index{ptr: ptr}, nil } +// OpenIndex creates a new index at the given path. If the file does +// not exist it will be created when Write() is called. +func OpenIndex(path string) (*Index, error) { + var ptr *C.git_index + + var cpath = C.CString(path) + defer C.free(unsafe.Pointer(cpath)) + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + if err := C.git_index_open(&ptr, cpath); err < 0 { + return nil, MakeGitError(err) + } + + return &Index{ptr: ptr}, nil +} + // Add adds or replaces the given entry to the index, making a copy of // the data func (v *Index) Add(entry *IndexEntry) error { diff --git a/index_test.go b/index_test.go index a1f0c9c..9b54945 100644 --- a/index_test.go +++ b/index_test.go @@ -2,6 +2,7 @@ package git import ( "io/ioutil" + "os" "runtime" "testing" ) @@ -148,6 +149,29 @@ func TestIndexAddAllCallback(t *testing.T) { } } +func TestIndexOpen(t *testing.T) { + repo := createTestRepo(t) + defer cleanupTestRepo(t, repo) + + path := repo.Workdir() + "/heyindex" + + _, err := os.Stat(path) + if !os.IsNotExist(err) { + t.Fatal("new index file already exists") + } + + idx, err := OpenIndex(path) + checkFatal(t, err) + + err = idx.Write() + checkFatal(t, err) + + _, err = os.Stat(path) + if os.IsNotExist(err) { + t.Fatal("new index file did not get written") + } +} + func checkFatal(t *testing.T, err error) { if err == nil { return From 72c19f73c9170720780839cd1561486e075d35a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 19 May 2015 15:05:00 +0200 Subject: [PATCH 20/36] Index: Add Path() accessor --- index.go | 6 ++++++ index_test.go | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/index.go b/index.go index 2082ebd..1a899f1 100644 --- a/index.go +++ b/index.go @@ -114,6 +114,12 @@ func OpenIndex(path string) (*Index, error) { return &Index{ptr: ptr}, nil } +// Path returns the index' path on disk or an empty string if it +// exists only in memory. +func (v *Index) Path() string { + return C.GoString(C.git_index_path(v.ptr)) +} + // Add adds or replaces the given entry to the index, making a copy of // the data func (v *Index) Add(entry *IndexEntry) error { diff --git a/index_test.go b/index_test.go index 9b54945..9283b83 100644 --- a/index_test.go +++ b/index_test.go @@ -83,6 +83,10 @@ func TestIndexAddAndWriteTreeTo(t *testing.T) { idx, err := NewIndex() checkFatal(t, err) + if idx.Path() != "" { + t.Fatal("in-memory repo has a path") + } + entry := IndexEntry{ Path: "README", Id: blobID, @@ -163,6 +167,10 @@ func TestIndexOpen(t *testing.T) { idx, err := OpenIndex(path) checkFatal(t, err) + if path != idx.Path() { + t.Fatalf("mismatched index paths, expected %v, got %v", path, idx.Path()) + } + err = idx.Write() checkFatal(t, err) From 7750e85fd1ff1006a28a5e292c9bc7ce3e12b586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 21 Apr 2015 13:14:22 +0200 Subject: [PATCH 21/36] Introduce an indirection layer for pointers As the Go runtime can move stacks at any point and the C code runs concurrently with the rest of the system, we cannot assume that the payloads we give to the C code will stay valid for any particular duration. We must therefore give the C code handles which we can then look up in our own list when the callbacks get called. --- git.go | 4 +++ handles.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 handles.go diff --git a/git.go b/git.go index 9496d2d..4f1a65e 100644 --- a/git.go +++ b/git.go @@ -93,7 +93,11 @@ var ( ErrInvalid = errors.New("Invalid state for operation") ) +var pointerHandles *HandleList + func init() { + pointerHandles = NewHandleList() + C.git_libgit2_init() // This is not something we should be doing, as we may be diff --git a/handles.go b/handles.go new file mode 100644 index 0000000..c0d1889 --- /dev/null +++ b/handles.go @@ -0,0 +1,82 @@ +package git + +import ( + "sync" + "unsafe" +) + +type HandleList struct { + sync.RWMutex + // stores the Go pointers + handles []interface{} + // indicates which indices are in use + set map[uintptr]bool +} + +func NewHandleList() *HandleList { + return &HandleList{ + handles: make([]interface{}, 5), + } +} + +// findUnusedSlot finds the smallest-index empty space in our +// list. You must only run this function while holding a write lock. +func (v *HandleList) findUnusedSlot() uintptr { + for i := 0; i < len(v.handles); i++ { + isUsed := v.set[uintptr(i)] + if !isUsed { + return uintptr(i) + } + } + + // reaching here means we've run out of entries so append and + // return the new index, which is equal to the old length. + slot := len(v.handles) + v.handles = append(v.handles, nil) + + return uintptr(slot) +} + +// Track adds the given pointer to the list of pointers to track and +// returns a pointer value which can be passed to C as an opaque +// pointer. +func (v *HandleList) Track(pointer interface{}) unsafe.Pointer { + v.Lock() + + slot := v.findUnusedSlot() + v.handles[slot] = pointer + v.set[slot] = true + + v.Unlock() + + return unsafe.Pointer(slot) +} + +// Untrack stops tracking the pointer given by the handle +func (v *HandleList) Untrack(handle unsafe.Pointer) { + slot := uintptr(handle) + + v.Lock() + + v.handles[slot] = nil + delete(v.set, slot) + + v.Unlock() +} + +// Get retrieves the pointer from the given handle +func (v *HandleList) Get(handle unsafe.Pointer) interface{} { + slot := uintptr(handle) + + v.RLock() + + if _, ok := v.set[slot]; !ok { + panic("invalid pointer handle") + } + + ptr := v.handles[slot] + + v.RUnlock() + + return ptr +} From bde012f3d478752787bab0978e0bfaf5deefa42b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:02:57 +0200 Subject: [PATCH 22/36] handles: correctly initialize all members --- handles.go | 1 + 1 file changed, 1 insertion(+) diff --git a/handles.go b/handles.go index c0d1889..a862746 100644 --- a/handles.go +++ b/handles.go @@ -16,6 +16,7 @@ type HandleList struct { func NewHandleList() *HandleList { return &HandleList{ handles: make([]interface{}, 5), + set: make(map[uintptr]bool), } } From be3a626f2ed67edd266bdf74e68adafcef1b0005 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:03:49 +0200 Subject: [PATCH 23/36] tree: use HandleList for C function callbacks. --- tree.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tree.go b/tree.go index c18d02a..70a3a3f 100644 --- a/tree.go +++ b/tree.go @@ -93,19 +93,25 @@ type TreeWalkCallback func(string, *TreeEntry) int func CallbackGitTreeWalk(_root unsafe.Pointer, _entry unsafe.Pointer, ptr unsafe.Pointer) C.int { root := C.GoString((*C.char)(_root)) entry := (*C.git_tree_entry)(_entry) - callback := *(*TreeWalkCallback)(ptr) - return C.int(callback(root, newTreeEntry(entry))) + if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok { + return C.int(callback(root, newTreeEntry(entry))) + } else { + return C.int(-1) + } } func (t Tree) Walk(callback TreeWalkCallback) error { runtime.LockOSThread() defer runtime.UnlockOSThread() + ptr := pointerHandles.Track(callback) + defer pointerHandles.Untrack(ptr) + err := C._go_git_treewalk( t.cast_ptr, C.GIT_TREEWALK_PRE, - unsafe.Pointer(&callback), + ptr, ) if err < 0 { From de45a4b8ed991cd46e64a1836e25dd9b182c821f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:33:00 +0200 Subject: [PATCH 24/36] submodule: use HandleList for C function callbacks --- submodule.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/submodule.go b/submodule.go index 7c6c922..0588ec5 100644 --- a/submodule.go +++ b/submodule.go @@ -97,17 +97,24 @@ func (repo *Repository) LookupSubmodule(name string) (*Submodule, error) { type SubmoduleCbk func(sub *Submodule, name string) int //export SubmoduleVisitor -func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, cfct unsafe.Pointer) C.int { +func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer) C.int { sub := &Submodule{(*C.git_submodule)(csub)} - fct := *(*SubmoduleCbk)(cfct) - return (C.int)(fct(sub, C.GoString(name))) + + if callback, ok := pointerHandles.Get(handle).(SubmoduleCbk); ok { + return (C.int)(callback(sub, C.GoString(name))) + } else { + return -1 + } } func (repo *Repository) ForeachSubmodule(cbk SubmoduleCbk) error { runtime.LockOSThread() defer runtime.UnlockOSThread() - ret := C._go_git_visit_submodule(repo.ptr, unsafe.Pointer(&cbk)) + handle := pointerHandles.Track(cbk) + defer pointerHandles.Untrack(handle) + + ret := C._go_git_visit_submodule(repo.ptr, handle) if ret < 0 { return MakeGitError(ret) } From 0a336e4abdd7c949c9dd38037165e6a16c7d7ebf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:35:58 +0200 Subject: [PATCH 25/36] handles: start slot indices with 1 Using 0 as the first slot indice leads to not being able to differentiate between a handle to the first element or a NULL-handle. As current code may check whether the pointer is NULL, change the first indice to be 1 instead. --- handles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handles.go b/handles.go index a862746..d173785 100644 --- a/handles.go +++ b/handles.go @@ -23,7 +23,7 @@ func NewHandleList() *HandleList { // findUnusedSlot finds the smallest-index empty space in our // list. You must only run this function while holding a write lock. func (v *HandleList) findUnusedSlot() uintptr { - for i := 0; i < len(v.handles); i++ { + for i := 1; i < len(v.handles); i++ { isUsed := v.set[uintptr(i)] if !isUsed { return uintptr(i) From 9bbec34885aff0287802134acbfdb5a20409fd9e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:40:49 +0200 Subject: [PATCH 26/36] index: use HandleList for C function callbacks. --- index.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/index.go b/index.go index 1a899f1..c344bf8 100644 --- a/index.go +++ b/index.go @@ -162,16 +162,17 @@ func (v *Index) AddAll(pathspecs []string, flags IndexAddOpts, callback IndexMat runtime.LockOSThread() defer runtime.UnlockOSThread() - var cb *IndexMatchedPathCallback + var handle unsafe.Pointer if callback != nil { - cb = &callback + handle = pointerHandles.Track(callback) + defer pointerHandles.Untrack(handle) } ret := C._go_git_index_add_all( v.ptr, &cpathspecs, C.uint(flags), - unsafe.Pointer(cb), + handle, ) if ret < 0 { return MakeGitError(ret) @@ -188,15 +189,16 @@ func (v *Index) UpdateAll(pathspecs []string, callback IndexMatchedPathCallback) runtime.LockOSThread() defer runtime.UnlockOSThread() - var cb *IndexMatchedPathCallback + var handle unsafe.Pointer if callback != nil { - cb = &callback + handle = pointerHandles.Track(callback) + defer pointerHandles.Untrack(handle) } ret := C._go_git_index_update_all( v.ptr, &cpathspecs, - unsafe.Pointer(cb), + handle, ) if ret < 0 { return MakeGitError(ret) @@ -213,15 +215,16 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback) runtime.LockOSThread() defer runtime.UnlockOSThread() - var cb *IndexMatchedPathCallback + var handle unsafe.Pointer if callback != nil { - cb = &callback + handle = pointerHandles.Track(callback) + defer pointerHandles.Untrack(handle) } ret := C._go_git_index_remove_all( v.ptr, &cpathspecs, - unsafe.Pointer(cb), + handle, ) if ret < 0 { return MakeGitError(ret) @@ -231,8 +234,13 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback) //export indexMatchedPathCallback func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) int { - callback := (*IndexMatchedPathCallback)(payload) - return (*callback)(C.GoString(cPath), C.GoString(cMatchedPathspec)) + if payload == nil { + return 0 + } else if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok { + return callback(C.GoString(cPath), C.GoString(cMatchedPathspec)) + } else { + return -1 + } } func (v *Index) RemoveByPath(path string) error { From e91965375551ff2ed68c0cea0c61a9ee4081ceb7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:55:06 +0200 Subject: [PATCH 27/36] odb: use HandleList for C function callbacks. --- odb.go | 13 ++++++++++--- odb_test.go | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/odb.go b/odb.go index ba03860..6b21329 100644 --- a/odb.go +++ b/odb.go @@ -98,8 +98,12 @@ type foreachData struct { } //export odbForEachCb -func odbForEachCb(id *C.git_oid, payload unsafe.Pointer) int { - data := (*foreachData)(payload) +func odbForEachCb(id *C.git_oid, handle unsafe.Pointer) int { + data, ok := pointerHandles.Get(handle).(*foreachData) + + if !ok { + panic("could not retrieve handle") + } err := data.callback(newOidFromC(id)) if err != nil { @@ -119,7 +123,10 @@ func (v *Odb) ForEach(callback OdbForEachCallback) error { runtime.LockOSThread() defer runtime.UnlockOSThread() - ret := C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&data)) + handle := pointerHandles.Track(&data) + defer pointerHandles.Untrack(handle) + + ret := C._go_git_odb_foreach(v.ptr, handle) if ret == C.GIT_EUSER { return data.err } else if ret < 0 { diff --git a/odb_test.go b/odb_test.go index 55ed297..2fb6840 100644 --- a/odb_test.go +++ b/odb_test.go @@ -81,7 +81,7 @@ func TestOdbForeach(t *testing.T) { checkFatal(t, err) if count != expect { - t.Fatalf("Expected %v objects, got %v") + t.Fatalf("Expected %v objects, got %v", expect, count) } expect = 1 From 7ee534d0c50f6b7b54a6e8c83e0953a95a4cac22 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:08:32 +0200 Subject: [PATCH 28/36] handles: print pointer handle on panic. --- handles.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/handles.go b/handles.go index d173785..1cbf6eb 100644 --- a/handles.go +++ b/handles.go @@ -1,6 +1,7 @@ package git import ( + "fmt" "sync" "unsafe" ) @@ -72,7 +73,7 @@ func (v *HandleList) Get(handle unsafe.Pointer) interface{} { v.RLock() if _, ok := v.set[slot]; !ok { - panic("invalid pointer handle") + panic(fmt.Sprintf("invalid pointer handle: %p", handle)) } ptr := v.handles[slot] From fe902f56a8545df76b51458b21aa84916ab51f6c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:10:09 +0200 Subject: [PATCH 29/36] diff: use HandleList for C function callbacks. --- diff.go | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/diff.go b/diff.go index 23469f2..8aa79aa 100644 --- a/diff.go +++ b/diff.go @@ -265,7 +265,11 @@ func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, detail DiffDetail) err data := &diffForEachData{ FileCallback: cbFile, } - ecode := C._go_git_diff_foreach(diff.ptr, 1, intHunks, intLines, unsafe.Pointer(data)) + + handle := pointerHandles.Track(data) + defer pointerHandles.Untrack(handle) + + ecode := C._go_git_diff_foreach(diff.ptr, 1, intHunks, intLines, handle) if ecode < 0 { return data.Error } @@ -273,8 +277,12 @@ func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, detail DiffDetail) err } //export diffForEachFileCb -func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe.Pointer) int { - data := (*diffForEachData)(payload) +func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffForEachData) + if !ok { + panic("could not retrieve data for handle") + } data.HunkCallback = nil if data.FileCallback != nil { @@ -292,8 +300,12 @@ func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe type DiffForEachHunkCallback func(DiffHunk) (DiffForEachLineCallback, error) //export diffForEachHunkCb -func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload unsafe.Pointer) int { - data := (*diffForEachData)(payload) +func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffForEachData) + if !ok { + panic("could not retrieve data for handle") + } data.LineCallback = nil if data.HunkCallback != nil { @@ -311,9 +323,12 @@ func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload u type DiffForEachLineCallback func(DiffLine) error //export diffForEachLineCb -func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, payload unsafe.Pointer) int { - - data := (*diffForEachData)(payload) +func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffForEachData) + if !ok { + panic("could not retrieve data for handle") + } err := data.LineCallback(diffLineFromC(delta, hunk, line)) if err != nil { @@ -479,9 +494,15 @@ type diffNotifyData struct { } //export diffNotifyCb -func diffNotifyCb(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, payload unsafe.Pointer) int { +func diffNotifyCb(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, handle unsafe.Pointer) int { diff_so_far := (*C.git_diff)(_diff_so_far) - data := (*diffNotifyData)(payload) + + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffNotifyData) + if !ok { + panic("could not retrieve data for handle") + } + if data != nil { if data.Diff == nil { data.Diff = newDiffFromC(diff_so_far) @@ -507,6 +528,7 @@ func diffOptionsToC(opts *DiffOptions) (copts *C.git_diff_options, notifyData *d notifyData = &diffNotifyData{ Callback: opts.NotifyCallback, } + if opts.Pathspec != nil { cpathspec.count = C.size_t(len(opts.Pathspec)) cpathspec.strings = makeCStringsFromStrings(opts.Pathspec) @@ -527,7 +549,7 @@ func diffOptionsToC(opts *DiffOptions) (copts *C.git_diff_options, notifyData *d if opts.NotifyCallback != nil { C._go_git_setup_diff_notify_callbacks(copts) - copts.notify_payload = unsafe.Pointer(notifyData) + copts.notify_payload = pointerHandles.Track(notifyData) } } return @@ -539,6 +561,7 @@ func freeDiffOptions(copts *C.git_diff_options) { freeStrarray(&cpathspec) C.free(unsafe.Pointer(copts.old_prefix)) C.free(unsafe.Pointer(copts.new_prefix)) + pointerHandles.Untrack(copts.notify_payload) } } From 83f9e6a7053cae61cf835221b3a61bdeb2e528f4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:13:52 +0200 Subject: [PATCH 30/36] blob: use HandleList for C function callbacks. --- blob.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/blob.go b/blob.go index 5a33bd8..b1fc78a 100644 --- a/blob.go +++ b/blob.go @@ -60,8 +60,13 @@ type BlobCallbackData struct { } //export blobChunkCb -func blobChunkCb(buffer *C.char, maxLen C.size_t, payload unsafe.Pointer) int { - data := (*BlobCallbackData)(payload) +func blobChunkCb(buffer *C.char, maxLen C.size_t, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*BlobCallbackData) + if !ok { + panic("could not retrieve blob callback data") + } + goBuf, err := data.Callback(int(maxLen)) if err == io.EOF { return 0 @@ -83,8 +88,12 @@ func (repo *Repository) CreateBlobFromChunks(hintPath string, callback BlobChunk defer C.free(unsafe.Pointer(chintPath)) } oid := C.git_oid{} + payload := &BlobCallbackData{Callback: callback} - ecode := C._go_git_blob_create_fromchunks(&oid, repo.ptr, chintPath, unsafe.Pointer(payload)) + handle := pointerHandles.Track(payload) + defer pointerHandles.Untrack(handle) + + ecode := C._go_git_blob_create_fromchunks(&oid, repo.ptr, chintPath, handle) if payload.Error != nil { return nil, payload.Error } From a843b7247f24a2125c30c865f9d8ea706ce3d768 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:16:12 +0200 Subject: [PATCH 31/36] packbuilder: use HandleList for C function callbacks. --- packbuilder.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packbuilder.go b/packbuilder.go index 54a8390..4dc352c 100644 --- a/packbuilder.go +++ b/packbuilder.go @@ -110,8 +110,13 @@ type packbuilderCbData struct { } //export packbuilderForEachCb -func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, payload unsafe.Pointer) int { - data := (*packbuilderCbData)(payload) +func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*packbuilderCbData) + if !ok { + panic("could not get packbuilder CB data") + } + slice := C.GoBytes(buf, C.int(size)) err := data.callback(slice) @@ -130,11 +135,13 @@ func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error { callback: callback, err: nil, } + handle := pointerHandles.Track(&data) + defer pointerHandles.Untrack(handle) runtime.LockOSThread() defer runtime.UnlockOSThread() - err := C._go_git_packbuilder_foreach(pb.ptr, unsafe.Pointer(&data)) + err := C._go_git_packbuilder_foreach(pb.ptr, handle) if err == C.GIT_EUSER { return data.err } From d95932c84a2b9c926490def345d71d45bb19f344 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 08:58:21 +0200 Subject: [PATCH 32/36] handles: panic when we cannot retrieve handle data --- index.go | 6 ++---- submodule.go | 2 +- tree.go | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/index.go b/index.go index c344bf8..c1bfb74 100644 --- a/index.go +++ b/index.go @@ -234,12 +234,10 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback) //export indexMatchedPathCallback func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) int { - if payload == nil { - return 0 - } else if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok { + if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok { return callback(C.GoString(cPath), C.GoString(cMatchedPathspec)) } else { - return -1 + panic("invalid matched path callback") } } diff --git a/submodule.go b/submodule.go index 0588ec5..3882462 100644 --- a/submodule.go +++ b/submodule.go @@ -103,7 +103,7 @@ func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer) if callback, ok := pointerHandles.Get(handle).(SubmoduleCbk); ok { return (C.int)(callback(sub, C.GoString(name))) } else { - return -1 + panic("invalid submodule visitor callback") } } diff --git a/tree.go b/tree.go index 70a3a3f..cbba08b 100644 --- a/tree.go +++ b/tree.go @@ -97,7 +97,7 @@ func CallbackGitTreeWalk(_root unsafe.Pointer, _entry unsafe.Pointer, ptr unsafe if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok { return C.int(callback(root, newTreeEntry(entry))) } else { - return C.int(-1) + panic("invalid treewalk callback") } } From 1bd338af5e7a329c8ec5bd85500350795d0793d2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 09:50:16 +0200 Subject: [PATCH 33/36] handles: do not store handles by uintptr If we store values by uintptrs the GC may try to inspect their values when it kicks in. As the pointers are most likely invalid, this will result in an invalid pointer dereference, causing the program to panic. We can fix this by storing values by an int index value instead, returning pointers to those indices as handles instead. --- handles.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/handles.go b/handles.go index 1cbf6eb..ec62a48 100644 --- a/handles.go +++ b/handles.go @@ -11,23 +11,23 @@ type HandleList struct { // stores the Go pointers handles []interface{} // indicates which indices are in use - set map[uintptr]bool + set map[int]bool } func NewHandleList() *HandleList { return &HandleList{ handles: make([]interface{}, 5), - set: make(map[uintptr]bool), + set: make(map[int]bool), } } // findUnusedSlot finds the smallest-index empty space in our // list. You must only run this function while holding a write lock. -func (v *HandleList) findUnusedSlot() uintptr { +func (v *HandleList) findUnusedSlot() int { for i := 1; i < len(v.handles); i++ { - isUsed := v.set[uintptr(i)] + isUsed := v.set[i] if !isUsed { - return uintptr(i) + return i } } @@ -36,7 +36,7 @@ func (v *HandleList) findUnusedSlot() uintptr { slot := len(v.handles) v.handles = append(v.handles, nil) - return uintptr(slot) + return slot } // Track adds the given pointer to the list of pointers to track and @@ -51,12 +51,12 @@ func (v *HandleList) Track(pointer interface{}) unsafe.Pointer { v.Unlock() - return unsafe.Pointer(slot) + return unsafe.Pointer(&slot) } // Untrack stops tracking the pointer given by the handle func (v *HandleList) Untrack(handle unsafe.Pointer) { - slot := uintptr(handle) + slot := *(*int)(handle) v.Lock() @@ -68,7 +68,7 @@ func (v *HandleList) Untrack(handle unsafe.Pointer) { // Get retrieves the pointer from the given handle func (v *HandleList) Get(handle unsafe.Pointer) interface{} { - slot := uintptr(handle) + slot := *(*int)(handle) v.RLock() From c43afaf9c4f5abb7ded44d88c8e9e290f61362fd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 09:56:21 +0200 Subject: [PATCH 34/36] tree: use correct C callback signature --- tree.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree.go b/tree.go index cbba08b..aad2c8d 100644 --- a/tree.go +++ b/tree.go @@ -90,8 +90,8 @@ func (t Tree) EntryCount() uint64 { type TreeWalkCallback func(string, *TreeEntry) int //export CallbackGitTreeWalk -func CallbackGitTreeWalk(_root unsafe.Pointer, _entry unsafe.Pointer, ptr unsafe.Pointer) C.int { - root := C.GoString((*C.char)(_root)) +func CallbackGitTreeWalk(_root *C.char, _entry unsafe.Pointer, ptr unsafe.Pointer) C.int { + root := C.GoString(_root) entry := (*C.git_tree_entry)(_entry) if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok { From e8531dd5c31fc87044e9061b18f37df9b05bd0ac Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 10:01:50 +0200 Subject: [PATCH 35/36] diff: only untrack notify payload when it is set --- diff.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/diff.go b/diff.go index 8aa79aa..5e03175 100644 --- a/diff.go +++ b/diff.go @@ -561,7 +561,9 @@ func freeDiffOptions(copts *C.git_diff_options) { freeStrarray(&cpathspec) C.free(unsafe.Pointer(copts.old_prefix)) C.free(unsafe.Pointer(copts.new_prefix)) - pointerHandles.Untrack(copts.notify_payload) + if copts.notify_payload != nil { + pointerHandles.Untrack(copts.notify_payload) + } } } From 53c158fbd7e5f4dac787f5c3a7107fcb4116f676 Mon Sep 17 00:00:00 2001 From: taylorchu Date: Sat, 30 May 2015 22:27:08 +0200 Subject: [PATCH 36/36] Fix test error messages --- index_test.go | 3 +-- reference_test.go | 3 +-- submodule_test.go | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/index_test.go b/index_test.go index 9283b83..7c65f4f 100644 --- a/index_test.go +++ b/index_test.go @@ -188,8 +188,7 @@ func checkFatal(t *testing.T, err error) { // The failure happens at wherever we were called, not here _, file, line, ok := runtime.Caller(1) if !ok { - t.Fatal() + t.Fatalf("Unable to get caller") } - t.Fatalf("Fail at %v:%v; %v", file, line, err) } diff --git a/reference_test.go b/reference_test.go index d6b5f22..5720a66 100644 --- a/reference_test.go +++ b/reference_test.go @@ -199,8 +199,7 @@ func checkRefType(t *testing.T, ref *Reference, kind ReferenceType) { // The failure happens at wherever we were called, not here _, file, line, ok := runtime.Caller(1) if !ok { - t.Fatal() + t.Fatalf("Unable to get caller") } - t.Fatalf("Wrong ref type at %v:%v; have %v, expected %v", file, line, ref.Type(), kind) } diff --git a/submodule_test.go b/submodule_test.go index ee75d53..27bc193 100644 --- a/submodule_test.go +++ b/submodule_test.go @@ -21,6 +21,6 @@ func TestSubmoduleForeach(t *testing.T) { checkFatal(t, err) if i != 1 { - t.Fatalf("expected one submodule found but got %i", i) + t.Fatalf("expected one submodule found but got %d", i) } }