protocol/packp: UploadPackResponse implementation (#161)

* plumbing/protocol: paktp avoid duplication of haves, wants and shallow

* protocol/pakp: UploadPackResponse implementation

* changes

* changes

* changes

* debug

* changes

17 files changed, 580 insertions, 67 deletions

diff --git a/fixtures/fixtures.go b/fixtures/fixtures.go
index 503f2ea..7cfceae 100644
--- a/fixtures/fixtures.go
+++ b/fixtures/fixtures.go
@@ -3,6 +3,7 @@ package fixtures
 import (
 	"fmt"
 	"go/build"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -195,6 +196,25 @@ func (f *Fixture) DotGit() fs.Filesystem {
 	return osfs.New(path)
 }
 
+func (f *Fixture) Worktree() fs.Filesystem {
+	fn := filepath.Join(RootFolder, DataFolder, fmt.Sprintf("git-%s.tgz", f.DotGitHash))
+	git, err := tgz.Extract(fn)
+	if err != nil {
+		panic(err)
+	}
+
+	worktree, err := ioutil.TempDir("", "worktree")
+	if err != nil {
+		panic(err)
+	}
+
+	if err := os.Rename(git, filepath.Join(worktree, ".git")); err != nil {
+		panic(err)
+	}
+
+	return osfs.New(worktree)
+}
+
 type Fixtures []*Fixture
 
 func (g Fixtures) Test(c *check.C, test func(*Fixture)) {
diff --git a/plumbing/protocol/packp/common.go b/plumbing/protocol/packp/common.go
index c8db931..2328eda 100644
--- a/plumbing/protocol/packp/common.go
+++ b/plumbing/protocol/packp/common.go
@@ -17,12 +17,12 @@ var (
 	eol = []byte("\n")
 	eq  = []byte{'='}
 
-	// advrefs
+	// advertised-refs
 	null       = []byte("\x00")
 	peeled     = []byte("^{}")
 	noHeadMark = []byte(" capabilities^{}\x00")
 
-	// ulreq
+	// upload-request
 	want            = []byte("want ")
 	shallow         = []byte("shallow ")
 	deepen          = []byte("deepen")
@@ -30,6 +30,13 @@ var (
 	deepenSince     = []byte("deepen-since ")
 	deepenReference = []byte("deepen-not ")
 
+	// shallow-update
+	unshallow = []byte("unshallow ")
+
+	// server-response
+	ack = []byte("ACK")
+	nak = []byte("NAK")
+
 	// updreq
 	shallowNoSp = []byte("shallow")
 )
diff --git a/plumbing/protocol/packp/shallowupd.go b/plumbing/protocol/packp/shallowupd.go
new file mode 100644
index 0000000..89063de
--- /dev/null
+++ b/plumbing/protocol/packp/shallowupd.go
@@ -0,0 +1,73 @@
+package packp
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/format/pktline"
+)
+
+const (
+	shallowLineLen   = 48
+	unshallowLineLen = 50
+)
+
+type ShallowUpdate struct {
+	Shallows   []plumbing.Hash
+	Unshallows []plumbing.Hash
+}
+
+func (r *ShallowUpdate) Decode(reader io.Reader) error {
+	s := pktline.NewScanner(reader)
+
+	for s.Scan() {
+		line := s.Bytes()
+
+		var err error
+		switch {
+		case bytes.HasPrefix(line, shallow):
+			err = r.decodeShallowLine(line)
+		case bytes.HasPrefix(line, unshallow):
+			err = r.decodeUnshallowLine(line)
+		case bytes.Compare(line, pktline.Flush) == 0:
+			return nil
+		}
+
+		if err != nil {
+			return err
+		}
+	}
+
+	return s.Err()
+}
+
+func (r *ShallowUpdate) decodeShallowLine(line []byte) error {
+	hash, err := r.decodeLine(line, shallow, shallowLineLen)
+	if err != nil {
+		return err
+	}
+
+	r.Shallows = append(r.Shallows, hash)
+	return nil
+}
+
+func (r *ShallowUpdate) decodeUnshallowLine(line []byte) error {
+	hash, err := r.decodeLine(line, unshallow, unshallowLineLen)
+	if err != nil {
+		return err
+	}
+
+	r.Unshallows = append(r.Unshallows, hash)
+	return nil
+}
+
+func (r *ShallowUpdate) decodeLine(line, prefix []byte, expLen int) (plumbing.Hash, error) {
+	if len(line) != expLen {
+		return plumbing.ZeroHash, fmt.Errorf("malformed %s%q", prefix, line)
+	}
+
+	raw := string(line[expLen-40 : expLen])
+	return plumbing.NewHash(raw), nil
+}
diff --git a/plumbing/protocol/packp/shallowupd_test.go b/plumbing/protocol/packp/shallowupd_test.go
new file mode 100644
index 0000000..d64fb5d
--- /dev/null
+++ b/plumbing/protocol/packp/shallowupd_test.go
@@ -0,0 +1,63 @@
+package packp
+
+import (
+	"bytes"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+
+	. "gopkg.in/check.v1"
+)
+
+type ShallowUpdateSuite struct{}
+
+var _ = Suite(&ShallowUpdateSuite{})
+
+func (s *ShallowUpdateSuite) TestDecode(c *C) {
+	raw := "" +
+		"0034shallow aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +
+		"0034shallow bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +
+		"0000"
+
+	su := &ShallowUpdate{}
+	err := su.Decode(bytes.NewBufferString(raw))
+	c.Assert(err, IsNil)
+
+	plumbing.HashesSort(su.Shallows)
+
+	c.Assert(su.Unshallows, HasLen, 0)
+	c.Assert(su.Shallows, HasLen, 2)
+	c.Assert(su.Shallows, DeepEquals, []plumbing.Hash{
+		plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+		plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"),
+	})
+}
+
+func (s *ShallowUpdateSuite) TestDecodeUnshallow(c *C) {
+	raw := "" +
+		"0036unshallow aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +
+		"0036unshallow bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +
+		"0000"
+
+	su := &ShallowUpdate{}
+	err := su.Decode(bytes.NewBufferString(raw))
+	c.Assert(err, IsNil)
+
+	plumbing.HashesSort(su.Unshallows)
+
+	c.Assert(su.Shallows, HasLen, 0)
+	c.Assert(su.Unshallows, HasLen, 2)
+	c.Assert(su.Unshallows, DeepEquals, []plumbing.Hash{
+		plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+		plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"),
+	})
+}
+
+func (s *ShallowUpdateSuite) TestDecodeMalformed(c *C) {
+	raw := "" +
+		"0035unshallow aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +
+		"0000"
+
+	su := &ShallowUpdate{}
+	err := su.Decode(bytes.NewBufferString(raw))
+	c.Assert(err, NotNil)
+}
diff --git a/plumbing/protocol/packp/srvresp.go b/plumbing/protocol/packp/srvresp.go
new file mode 100644
index 0000000..3284fa2
--- /dev/null
+++ b/plumbing/protocol/packp/srvresp.go
@@ -0,0 +1,70 @@
+package packp
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/format/pktline"
+)
+
+const ackLineLen = 44
+
+// ServerResponse object acknowledgement from upload-pack service
+// TODO: implement support for multi_ack or multi_ack_detailed responses
+type ServerResponse struct {
+	ACKs []plumbing.Hash
+}
+
+// Decode decodes the response into the struct, isMultiACK should be true, if
+// the request was done with multi_ack or multi_ack_detailed capabilities
+func (r *ServerResponse) Decode(reader io.Reader, isMultiACK bool) error {
+	if isMultiACK {
+		return errors.New("multi_ack and multi_ack_detailed are not supported")
+	}
+
+	s := pktline.NewScanner(reader)
+
+	for s.Scan() {
+		line := s.Bytes()
+
+		if err := r.decodeLine(line); err != nil {
+			return err
+		}
+
+		if !isMultiACK {
+			break
+		}
+	}
+
+	return s.Err()
+}
+
+func (r *ServerResponse) decodeLine(line []byte) error {
+	if len(line) == 0 {
+		return fmt.Errorf("unexpected flush")
+	}
+
+	if bytes.Compare(line[0:3], ack) == 0 {
+		return r.decodeACKLine(line)
+	}
+
+	if bytes.Compare(line[0:3], nak) == 0 {
+		return nil
+	}
+
+	return fmt.Errorf("unexpected content %q", string(line))
+}
+
+func (r *ServerResponse) decodeACKLine(line []byte) error {
+	if len(line) < ackLineLen {
+		return fmt.Errorf("malformed ACK %q", line)
+	}
+
+	sp := bytes.Index(line, []byte(" "))
+	h := plumbing.NewHash(string(line[sp+1 : sp+41]))
+	r.ACKs = append(r.ACKs, h)
+	return nil
+}
diff --git a/plumbing/protocol/packp/srvresp_test.go b/plumbing/protocol/packp/srvresp_test.go
new file mode 100644
index 0000000..6078855
--- /dev/null
+++ b/plumbing/protocol/packp/srvresp_test.go
@@ -0,0 +1,48 @@
+package packp
+
+import (
+	"bytes"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+
+	. "gopkg.in/check.v1"
+)
+
+type ServerResponseSuite struct{}
+
+var _ = Suite(&ServerResponseSuite{})
+
+func (s *ServerResponseSuite) TestDecodeNAK(c *C) {
+	raw := "0008NAK\n"
+
+	sr := &ServerResponse{}
+	err := sr.Decode(bytes.NewBufferString(raw), false)
+	c.Assert(err, IsNil)
+
+	c.Assert(sr.ACKs, HasLen, 0)
+}
+
+func (s *ServerResponseSuite) TestDecodeACK(c *C) {
+	raw := "0031ACK 6ecf0ef2c2dffb796033e5a02219af86ec6584e5\n"
+
+	sr := &ServerResponse{}
+	err := sr.Decode(bytes.NewBufferString(raw), false)
+	c.Assert(err, IsNil)
+
+	c.Assert(sr.ACKs, HasLen, 1)
+	c.Assert(sr.ACKs[0], Equals, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
+}
+
+func (s *ServerResponseSuite) TestDecodeMalformed(c *C) {
+	raw := "0029ACK 6ecf0ef2c2dffb796033e5a02219af86ec6584e\n"
+
+	sr := &ServerResponse{}
+	err := sr.Decode(bytes.NewBufferString(raw), false)
+	c.Assert(err, NotNil)
+}
+
+func (s *ServerResponseSuite) TestDecodeMultiACK(c *C) {
+	sr := &ServerResponse{}
+	err := sr.Decode(bytes.NewBuffer(nil), true)
+	c.Assert(err, NotNil)
+}
diff --git a/plumbing/protocol/packp/ulreq.go b/plumbing/protocol/packp/ulreq.go
index 254e85e..d57c3fc 100644
--- a/plumbing/protocol/packp/ulreq.go
+++ b/plumbing/protocol/packp/ulreq.go
@@ -25,6 +25,7 @@ type UploadRequest struct {
 // DepthCommit, DepthSince and DepthReference.
 type Depth interface {
 	isDepth()
+	IsZero() bool
 }
 
 // DepthCommits values stores the maximum number of requested commits in
@@ -34,16 +35,28 @@ type DepthCommits int
 
 func (d DepthCommits) isDepth() {}
 
+func (d DepthCommits) IsZero() bool {
+	return d == 0
+}
+
 // DepthSince values requests only commits newer than the specified time.
 type DepthSince time.Time
 
 func (d DepthSince) isDepth() {}
 
+func (d DepthSince) IsZero() bool {
+	return time.Time(d).IsZero()
+}
+
 // DepthReference requests only commits not to found in the specified reference.
 type DepthReference string
 
 func (d DepthReference) isDepth() {}
 
+func (d DepthReference) IsZero() bool {
+	return string(d) == ""
+}
+
 // NewUploadRequest returns a pointer to a new UploadRequest value, ready to be
 // used. It has no capabilities, wants or shallows and an infinite depth. Please
 // note that to encode an upload-request it has to have at least one wanted hash.
diff --git a/plumbing/protocol/packp/ulreq_encode_test.go b/plumbing/protocol/packp/ulreq_encode_test.go
index b414a37..0890678 100644
--- a/plumbing/protocol/packp/ulreq_encode_test.go
+++ b/plumbing/protocol/packp/ulreq_encode_test.go
@@ -6,9 +6,9 @@ import (
 
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/format/pktline"
+	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability"
 
 	. "gopkg.in/check.v1"
-	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability"
 )
 
 type UlReqEncodeSuite struct{}
diff --git a/plumbing/protocol/packp/uppackreq.go b/plumbing/protocol/packp/uppackreq.go
index 887d27a..84e2b4e 100644
--- a/plumbing/protocol/packp/uppackreq.go
+++ b/plumbing/protocol/packp/uppackreq.go
@@ -13,15 +13,16 @@ import (
 // UploadPackRequest represents a upload-pack request.
 // Zero-value is not safe, use NewUploadPackRequest instead.
 type UploadPackRequest struct {
-	*UploadRequest
-	*UploadHaves
+	UploadRequest
+	UploadHaves
 }
 
 // NewUploadPackRequest creates a new UploadPackRequest and returns a pointer.
 func NewUploadPackRequest() *UploadPackRequest {
+	ur := NewUploadRequest()
 	return &UploadPackRequest{
-		UploadHaves:   &UploadHaves{},
-		UploadRequest: NewUploadRequest(),
+		UploadHaves:   UploadHaves{},
+		UploadRequest: *ur,
 	}
 }
 
@@ -30,9 +31,10 @@ func NewUploadPackRequest() *UploadPackRequest {
 // ones, based on the adv value (advertaised capabilities), the UploadPackRequest
 // it has no wants, haves or shallows and an infinite depth
 func NewUploadPackRequestFromCapabilities(adv *capability.List) *UploadPackRequest {
+	ur := NewUploadRequestFromCapabilities(adv)
 	return &UploadPackRequest{
-		UploadHaves:   &UploadHaves{},
-		UploadRequest: NewUploadRequestFromCapabilities(adv),
+		UploadHaves:   UploadHaves{},
+		UploadRequest: *ur,
 	}
 }
 
diff --git a/plumbing/protocol/packp/uppackreq_test.go b/plumbing/protocol/packp/uppackreq_test.go
index e551f45..273f916 100644
--- a/plumbing/protocol/packp/uppackreq_test.go
+++ b/plumbing/protocol/packp/uppackreq_test.go
@@ -1,11 +1,11 @@
 package packp
 
 import (
+	"bytes"
+
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability"
 
-	"bytes"
-
 	. "gopkg.in/check.v1"
 )
 
diff --git a/plumbing/protocol/packp/uppackresp.go b/plumbing/protocol/packp/uppackresp.go
new file mode 100644
index 0000000..a117956
--- /dev/null
+++ b/plumbing/protocol/packp/uppackresp.go
@@ -0,0 +1,77 @@
+package packp
+
+import (
+	"errors"
+	"io"
+
+	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability"
+)
+
+// ErrUploadPackResponseNotDecoded is returned if Read is called without
+// decoding first
+var ErrUploadPackResponseNotDecoded = errors.New("upload-pack-response should be decoded")
+
+// UploadPackResponse contains all the information responded by the upload-pack
+// service, the response implements io.ReadCloser that allows to read the
+// packfile directly from it.
+type UploadPackResponse struct {
+	ShallowUpdate
+	ServerResponse
+	r io.ReadCloser
+
+	isShallow  bool
+	isMultiACK bool
+	isOk       bool
+}
+
+// NewUploadPackResponse create a new UploadPackResponse instance, the request
+// being responded by the response is required.
+func NewUploadPackResponse(req *UploadPackRequest) *UploadPackResponse {
+	isShallow := !req.Depth.IsZero()
+	isMultiACK := req.Capabilities.Supports(capability.MultiACK) ||
+		req.Capabilities.Supports(capability.MultiACKDetailed)
+
+	return &UploadPackResponse{
+		isShallow:  isShallow,
+		isMultiACK: isMultiACK,
+	}
+}
+
+// Decode decodes all the responses sent by upload-pack service into the struct
+// and prepares it to read the packfile using the Read method
+func (r *UploadPackResponse) Decode(reader io.ReadCloser) error {
+	if r.isShallow {
+		if err := r.ShallowUpdate.Decode(reader); err != nil {
+			return err
+		}
+	}
+
+	if err := r.ServerResponse.Decode(reader, r.isMultiACK); err != nil {
+		return err
+	}
+
+	// now the reader is ready to read the packfile content
+	r.r = reader
+
+	return nil
+}
+
+// Read reads the packfile data, if the request was done with any Sideband
+// capability the content read should be demultiplexed. If the methods wasn't
+// called before the ErrUploadPackResponseNotDecoded will be return
+func (r *UploadPackResponse) Read(p []byte) (int, error) {
+	if r.r == nil {
+		return 0, ErrUploadPackResponseNotDecoded
+	}
+
+	return r.r.Read(p)
+}
+
+// Close the underlying reader, if any
+func (r *UploadPackResponse) Close() error {
+	if r.r == nil {
+		return nil
+	}
+
+	return r.r.Close()
+}
diff --git a/plumbing/protocol/packp/uppackresp_test.go b/plumbing/protocol/packp/uppackresp_test.go
new file mode 100644
index 0000000..c81bb76
--- /dev/null
+++ b/plumbing/protocol/packp/uppackresp_test.go
@@ -0,0 +1,82 @@
+package packp
+
+import (
+	"bytes"
+	"io/ioutil"
+
+	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability"
+
+	. "gopkg.in/check.v1"
+)
+
+type UploadPackResponseSuite struct{}
+
+var _ = Suite(&UploadPackResponseSuite{})
+
+func (s *UploadPackResponseSuite) TestDecodeNAK(c *C) {
+	raw := "0008NAK\n[PACK]"
+
+	req := NewUploadPackRequest()
+	res := NewUploadPackResponse(req)
+	defer res.Close()
+
+	err := res.Decode(ioutil.NopCloser(bytes.NewBufferString(raw)))
+	c.Assert(err, IsNil)
+
+	pack, err := ioutil.ReadAll(res)
+	c.Assert(err, IsNil)
+	c.Assert(pack, DeepEquals, []byte("[PACK]"))
+}
+
+func (s *UploadPackResponseSuite) TestDecodeDepth(c *C) {
+	raw := "00000008NAK\n[PACK]"
+
+	req := NewUploadPackRequest()
+	req.Depth = DepthCommits(1)
+
+	res := NewUploadPackResponse(req)
+	defer res.Close()
+
+	err := res.Decode(ioutil.NopCloser(bytes.NewBufferString(raw)))
+	c.Assert(err, IsNil)
+
+	pack, err := ioutil.ReadAll(res)
+	c.Assert(err, IsNil)
+	c.Assert(pack, DeepEquals, []byte("[PACK]"))
+}
+
+func (s *UploadPackResponseSuite) TestDecodeMalformed(c *C) {
+	raw := "00000008ACK\n[PACK]"
+
+	req := NewUploadPackRequest()
+	req.Depth = DepthCommits(1)
+
+	res := NewUploadPackResponse(req)
+	defer res.Close()
+
+	err := res.Decode(ioutil.NopCloser(bytes.NewBufferString(raw)))
+	c.Assert(err, NotNil)
+}
+
+func (s *UploadPackResponseSuite) TestDecodeMultiACK(c *C) {
+	req := NewUploadPackRequest()
+	req.Capabilities.Set(capability.MultiACK)
+
+	res := NewUploadPackResponse(req)
+	defer res.Close()
+
+	err := res.Decode(ioutil.NopCloser(bytes.NewBuffer(nil)))
+	c.Assert(err, NotNil)
+}
+
+func (s *UploadPackResponseSuite) TestReadNoDecode(c *C) {
+	req := NewUploadPackRequest()
+	req.Capabilities.Set(capability.MultiACK)
+
+	res := NewUploadPackResponse(req)
+	defer res.Close()
+
+	n, err := res.Read(nil)
+	c.Assert(err, Equals, ErrUploadPackResponseNotDecoded)
+	c.Assert(n, Equals, 0)
+}
diff --git a/plumbing/transport/common.go b/plumbing/transport/common.go
index bfc999f..2379422 100644
--- a/plumbing/transport/common.go
+++ b/plumbing/transport/common.go
@@ -69,10 +69,10 @@ type FetchPackSession interface {
 	AdvertisedReferences() (*packp.AdvRefs, error)
 	// FetchPack takes a request and returns a reader for the packfile
 	// received from the server.
-	FetchPack(req *packp.UploadPackRequest) (io.ReadCloser, error)
+	FetchPack(*packp.UploadPackRequest) (*packp.UploadPackResponse, error)
 }
 
-// FetchPackSession represents a git-send-pack session.
+// SendPackSession represents a git-send-pack session.
 // A git-send-pack session has two steps: reference discovery
 // (`AdvertisedReferences` function) and sending pack (`SendPack` function).
 // In that order.
diff --git a/plumbing/transport/http/fetch_pack.go b/plumbing/transport/http/fetch_pack.go
index f250667..20cdb55 100644
--- a/plumbing/transport/http/fetch_pack.go
+++ b/plumbing/transport/http/fetch_pack.go
@@ -11,6 +11,7 @@ import (
 	"gopkg.in/src-d/go-git.v4/plumbing/format/pktline"
 	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
+	"gopkg.in/src-d/go-git.v4/plumbing/transport/internal/common"
 	"gopkg.in/src-d/go-git.v4/utils/ioutil"
 )
 
@@ -74,12 +75,12 @@ func (s *fetchPackSession) AdvertisedReferences() (*packp.AdvRefs, error) {
 	return ar, nil
 }
 
-func (s *fetchPackSession) FetchPack(r *packp.UploadPackRequest) (io.ReadCloser, error) {
-	if r.IsEmpty() {
+func (s *fetchPackSession) FetchPack(req *packp.UploadPackRequest) (*packp.UploadPackResponse, error) {
+	if req.IsEmpty() {
 		return nil, transport.ErrEmptyUploadPackRequest
 	}
 
-	if err := r.Validate(); err != nil {
+	if err := req.Validate(); err != nil {
 		return nil, err
 	}
 
@@ -88,31 +89,27 @@ func (s *fetchPackSession) FetchPack(r *packp.UploadPackRequest) (io.ReadCloser,
 		s.endpoint.String(), transport.UploadPackServiceName,
 	)
 
-	content, err := uploadPackRequestToReader(r)
+	content, err := uploadPackRequestToReader(req)
 	if err != nil {
 		return nil, err
 	}
 
-	res, err := s.doRequest("POST", url, content)
+	res, err := s.doRequest(http.MethodPost, url, content)
 	if err != nil {
 		return nil, err
 	}
 
-	reader, err := ioutil.NonEmptyReader(res.Body)
-	if err == ioutil.ErrEmptyReader || err == io.ErrUnexpectedEOF {
-		return nil, transport.ErrEmptyUploadPackRequest
-	}
-
+	r, err := ioutil.NonEmptyReader(res.Body)
 	if err != nil {
-		return nil, err
-	}
+		if err == ioutil.ErrEmptyReader || err == io.ErrUnexpectedEOF {
+			return nil, transport.ErrEmptyUploadPackRequest
+		}
 
-	rc := ioutil.NewReadCloser(reader, res.Body)
-	if err := discardResponseInfo(rc); err != nil {
 		return nil, err
 	}
 
-	return rc, nil
+	rc := ioutil.NewReadCloser(r, res.Body)
+	return common.DecodeUploadPackResponse(rc, req)
 }
 
 // Close does nothing.
@@ -120,17 +117,6 @@ func (s *fetchPackSession) Close() error {
 	return nil
 }
 
-func discardResponseInfo(r io.Reader) error {
-	s := pktline.NewScanner(r)
-	for s.Scan() {
-		if bytes.Equal(s.Bytes(), []byte{'N', 'A', 'K', '\n'}) {
-			break
-		}
-	}
-
-	return s.Err()
-}
-
 func (s *fetchPackSession) doRequest(method, url string, content *strings.Reader) (*http.Response, error) {
 	var body io.Reader
 	if content != nil {
diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go
index 8b2f9f3..f6aa204 100644
--- a/plumbing/transport/internal/common/common.go
+++ b/plumbing/transport/internal/common/common.go
@@ -7,7 +7,6 @@ package common
 
 import (
 	"bufio"
-	"bytes"
 	"errors"
 	"fmt"
 	"io"
@@ -193,7 +192,7 @@ func (s *session) AdvertisedReferences() (*packp.AdvRefs, error) {
 
 // FetchPack performs a request to the server to fetch a packfile. A reader is
 // returned with the packfile content. The reader must be closed after reading.
-func (s *session) FetchPack(req *packp.UploadPackRequest) (io.ReadCloser, error) {
+func (s *session) FetchPack(req *packp.UploadPackRequest) (*packp.UploadPackResponse, error) {
 	if req.IsEmpty() {
 		return nil, transport.ErrEmptyUploadPackRequest
 	}
@@ -230,7 +229,7 @@ func (s *session) FetchPack(req *packp.UploadPackRequest) (io.ReadCloser, error)
 	wc := &waitCloser{s.Command}
 	rc := ioutil.NewReadCloser(r, wc)
 
-	return rc, nil
+	return DecodeUploadPackResponse(rc, req)
 }
 
 func (s *session) finish() error {
@@ -314,9 +313,7 @@ var (
 // TODO support multi_ack_detailed mode
 // TODO support acks for common objects
 // TODO build a proper state machine for all these processing options
-func fetchPack(w io.WriteCloser, r io.Reader,
-	req *packp.UploadPackRequest) error {
-
+func fetchPack(w io.WriteCloser, r io.Reader, req *packp.UploadPackRequest) error {
 	if err := req.UploadRequest.Encode(w); err != nil {
 		return fmt.Errorf("sending upload-req message: %s", err)
 	}
@@ -333,10 +330,6 @@ func fetchPack(w io.WriteCloser, r io.Reader,
 		return fmt.Errorf("closing input: %s", err)
 	}
 
-	if err := readNAK(r); err != nil {
-		return fmt.Errorf("reading NAK: %s", err)
-	}
-
 	return nil
 }
 
@@ -346,19 +339,16 @@ func sendDone(w io.Writer) error {
 	return e.Encodef("done\n")
 }
 
-func readNAK(r io.Reader) error {
-	s := pktline.NewScanner(r)
-	if !s.Scan() {
-		return s.Err()
+// DecodeUploadPackResponse decodes r into a new packp.UploadPackResponse
+func DecodeUploadPackResponse(r io.ReadCloser, req *packp.UploadPackRequest) (
+	*packp.UploadPackResponse, error,
+) {
+	res := packp.NewUploadPackResponse(req)
+	if err := res.Decode(r); err != nil {
+		return nil, fmt.Errorf("error decoding upload-pack response: %s", err)
 	}
 
-	b := s.Bytes()
-	b = bytes.TrimSuffix(b, eol)
-	if !bytes.Equal(b, nak) {
-		return fmt.Errorf("expecting NAK, found %q instead", string(b))
-	}
-
-	return nil
+	return res, nil
 }
 
 type waitCloser struct {
diff --git a/remote_test.go b/remote_test.go
index 7c714e9..30f68ad 100644
--- a/remote_test.go
+++ b/remote_test.go
@@ -98,6 +98,32 @@ func (s *RemoteSuite) TestFetch(c *C) {
 	}
 }
 
+func (s *RemoteSuite) TestFetchDepth(c *C) {
+	url := s.GetBasicLocalRepositoryURL()
+	sto := memory.NewStorage()
+	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: url})
+	c.Assert(r.Connect(), IsNil)
+
+	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
+	err := r.Fetch(&FetchOptions{
+		RefSpecs: []config.RefSpec{refspec},
+		Depth:    1,
+	})
+
+	c.Assert(err, IsNil)
+	c.Assert(sto.Objects, HasLen, 18)
+
+	expectedRefs := []*plumbing.Reference{
+		plumbing.NewReferenceFromStrings("refs/remotes/origin/master", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5"),
+		plumbing.NewReferenceFromStrings("refs/remotes/origin/branch", "e8d3ffab552895c19b9fcf7aa264d277cde33881"),
+	}
+
+	for _, exp := range expectedRefs {
+		r, _ := sto.Reference(exp.Name())
+		c.Assert(exp.String(), Equals, r.String())
+	}
+}
+
 type mockPackfileWriter struct {
 	Storer
 	PackfileWriterCalled bool
diff --git a/repository_test.go b/repository_test.go
index 11fa5c5..316cdb1 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -1,11 +1,20 @@
 package git
 
 import (
+	"fmt"
+	"os/exec"
+	"path/filepath"
+	"strings"
+
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/fixtures"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
 
+	"os"
+
+	"bytes"
+
 	. "gopkg.in/check.v1"
 )
 
@@ -221,15 +230,14 @@ func (s *RepositorySuite) TestPullSingleBranch(c *C) {
 	c.Assert(storage.Objects, HasLen, 28)
 }
 
-func (s *RepositorySuite) TestPull(c *C) {
-	r := NewMemoryRepository()
+func (s *RepositorySuite) TestPullA(c *C) {
+	path := fixtures.Basic().One().Worktree().Base()
 
-	r.CreateRemote(&config.RemoteConfig{
-		Name: "foo",
-		URL:  s.GetBasicLocalRepositoryURL(),
+	r := NewMemoryRepository()
+	err := r.Clone(&CloneOptions{
+		URL: fmt.Sprintf("file://%s", filepath.Join(path, ".git")),
 	})
 
-	err := r.Pull(&PullOptions{RemoteName: "foo"})
 	c.Assert(err, IsNil)
 
 	storage := r.s.(*memory.Storage)
@@ -239,7 +247,30 @@ func (s *RepositorySuite) TestPull(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(branch.Hash().String(), Equals, "6ecf0ef2c2dffb796033e5a02219af86ec6584e5")
 
-	branch, err = r.Ref("refs/remotes/foo/branch", false)
+	branch, err = r.Ref("refs/remotes/origin/branch", false)
+	c.Assert(err, IsNil)
+	c.Assert(branch.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
+
+	ExecuteOnPath(c, path,
+		"touch foo",
+		"git add foo",
+		"git config --global user.email you@foo.com",
+		"git config --global user.name foo",
+		"git commit -m foo foo",
+	)
+
+	err = r.Pull(&PullOptions{RemoteName: "origin"})
+	c.Assert(err, IsNil)
+
+	// the commit command has introduced a new commit, tree and blob
+	c.Assert(storage.Objects, HasLen, 34)
+
+	branch, err = r.Ref("refs/heads/master", false)
+	c.Assert(err, IsNil)
+	c.Assert(branch.Hash().String(), Not(Equals), "6ecf0ef2c2dffb796033e5a02219af86ec6584e5")
+
+	// the commit command, was in the local branch, so the remote should be read ok
+	branch, err = r.Ref("refs/remotes/origin/branch", false)
 	c.Assert(err, IsNil)
 	c.Assert(branch.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
 }
@@ -412,3 +443,28 @@ func (s *RepositorySuite) TestObjectNotFound(c *C) {
 	c.Assert(err, DeepEquals, plumbing.ErrObjectNotFound)
 	c.Assert(tag, IsNil)
 }
+
+func ExecuteOnPath(c *C, path string, cmds ...string) error {
+	for _, cmd := range cmds {
+		err := executeOnPath(path, cmd)
+		c.Assert(err, IsNil)
+	}
+
+	return nil
+}
+
+func executeOnPath(path, cmd string) error {
+	args := strings.Split(cmd, " ")
+
+	c := exec.Command(args[0], args[1:]...)
+	c.Dir = path
+	c.Env = os.Environ()
+
+	buf := bytes.NewBuffer(nil)
+	c.Stderr = buf
+	c.Stdout = buf
+
+	//defer func() { fmt.Println(buf.String()) }()
+
+	return c.Run()
+}