Repository.Progress moved as a field in *Options (#237)

Author: mcuadros

examples/progress/main.go

@@ -15,17 +15,17 @@ func main() {
 	r, err := git.NewFilesystemRepository(directory)
 	CheckIfError(err)
 
-	// as git does, when you make a clone, pull or some other operations, the
-	// server sends information via the sideband, this information can being
-	// collected provinding a io.Writer to the repository
-	r.Progress = os.Stdout
-
 	// Clone the given repository to the given directory
 	Info("git clone %s %s", url, directory)
 
 	err = r.Clone(&git.CloneOptions{
 		URL:   url,
 		Depth: 1,
+
+		// as git does, when you make a clone, pull or some other operations the
+		// server sends information via the sideband, this information can being
+		// collected provinding a io.Writer to the CloneOptions options
+		Progress: os.Stdout,
 	})
 
 	CheckIfError(err)

options.go

@@ -5,6 +5,7 @@ import (
 
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/sideband"
 	"gopkg.in/src-d/go-git.v4/plumbing/transport"
 )
 
@@ -32,6 +33,10 @@ type CloneOptions struct {
 	SingleBranch bool
 	// Limit fetching to the specified number of commits
 	Depth int
+	// Progress is where the human readable information sent by the server is
+	// stored, if nil nothing is stored and the capability (if supported)
+	// no-progress, is sent to the server to avoid send this information
+	Progress sideband.Progress
 }
 
 // Validate validates the fields and sets the default values
@@ -63,6 +68,10 @@ type PullOptions struct {
 	Depth int
 	// Auth credentials, if required, to use with the remote repository
 	Auth transport.AuthMethod
+	// Progress is where the human readable information sent by the server is
+	// stored, if nil nothing is stored and the capability (if supported)
+	// no-progress, is sent to the server to avoid send this information
+	Progress sideband.Progress
 }
 
 // Validate validates the fields and sets the default values.
@@ -88,6 +97,10 @@ type FetchOptions struct {
 	Depth int
 	// Auth credentials, if required, to use with the remote repository
 	Auth transport.AuthMethod
+	// Progress is where the human readable information sent by the server is
+	// stored, if nil nothing is stored and the capability (if supported)
+	// no-progress, is sent to the server to avoid send this information
+	Progress sideband.Progress
 }
 
 // Validate validates the fields and sets the default values

plumbing/protocol/packp/sideband/demux.go

@@ -11,9 +11,8 @@ import (
 // ErrMaxPackedExceeded returned by Read, if the maximum packed size is exceeded
 var ErrMaxPackedExceeded = errors.New("max. packed size exceeded")
 
-// Progress allows to read the progress information
+// Progress where the progress information is stored
 type Progress interface {
-	io.Reader
 	io.Writer
 }
 

plumbing/protocol/packp/sideband/demux_test.go

@@ -84,23 +84,24 @@ func (s *SidebandSuite) TestDecodeFromFailingReader(c *C) {
 func (s *SidebandSuite) TestDecodeWithProgress(c *C) {
 	expected := []byte("abcdefghijklmnopqrstuvwxyz")
 
-	buf := bytes.NewBuffer(nil)
-	e := pktline.NewEncoder(buf)
+	input := bytes.NewBuffer(nil)
+	e := pktline.NewEncoder(input)
 	e.Encode(PackData.WithPayload(expected[0:8]))
 	e.Encode(ProgressMessage.WithPayload([]byte{'F', 'O', 'O', '\n'}))
 	e.Encode(PackData.WithPayload(expected[8:16]))
 	e.Encode(PackData.WithPayload(expected[16:26]))
 
+	output := bytes.NewBuffer(nil)
 	content := make([]byte, 26)
-	d := NewDemuxer(Sideband64k, buf)
-	d.Progress = bytes.NewBuffer(nil)
+	d := NewDemuxer(Sideband64k, input)
+	d.Progress = output
 
 	n, err := io.ReadFull(d, content)
 	c.Assert(err, IsNil)
 	c.Assert(n, Equals, 26)
 	c.Assert(content, DeepEquals, expected)
 
-	progress, err := ioutil.ReadAll(d.Progress)
+	progress, err := ioutil.ReadAll(output)
 	c.Assert(err, IsNil)
 	c.Assert(progress, DeepEquals, []byte{'F', 'O', 'O', '\n'})
 }

remote.go

@@ -26,11 +26,10 @@ var NoErrAlreadyUpToDate = errors.New("already up-to-date")
 type Remote struct {
 	c *config.RemoteConfig
 	s Storer
-	p sideband.Progress
 }
 
-func newRemote(s Storer, p sideband.Progress, c *config.RemoteConfig) *Remote {
-	return &Remote{s: s, p: p, c: c}
+func newRemote(s Storer, c *config.RemoteConfig) *Remote {
+	return &Remote{s: s, c: c}
 }
 
 // Config return the config
@@ -59,6 +58,7 @@ func (r *Remote) Fetch(o *FetchOptions) error {
 // TODO: Support deletes.
 // TODO: Support pushing tags.
 // TODO: Check if force update is given, otherwise reject non-fast forward.
+// TODO: Sideband support
 func (r *Remote) Push(o *PushOptions) (err error) {
 	if o.RemoteName == "" {
 		o.RemoteName = r.c.Name
@@ -222,7 +222,7 @@ func (r *Remote) fetchPack(o *FetchOptions, s transport.UploadPackSession,
 	}
 
 	if err = packfile.UpdateObjectStorage(r.s,
-		buildSidebandIfSupported(req.Capabilities, reader, r.p),
+		buildSidebandIfSupported(req.Capabilities, reader, o.Progress),
 	); err != nil {
 		return err
 	}
@@ -400,7 +400,7 @@ func (r *Remote) newUploadPackRequest(o *FetchOptions,
 		}
 	}
 
-	if r.p == nil && ar.Capabilities.Supports(capability.NoProgress) {
+	if o.Progress == nil && ar.Capabilities.Supports(capability.NoProgress) {
 		if err := req.Capabilities.Set(capability.NoProgress); err != nil {
 			return nil, err
 		}

remote_test.go

@@ -26,25 +26,25 @@ type RemoteSuite struct {
 var _ = Suite(&RemoteSuite{})
 
 func (s *RemoteSuite) TestFetchInvalidEndpoint(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "qux"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux"})
 	err := r.Fetch(&FetchOptions{})
 	c.Assert(err, ErrorMatches, ".*invalid endpoint.*")
 }
 
 func (s *RemoteSuite) TestFetchNonExistentEndpoint(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "ssh://non-existent/foo.git"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "ssh://non-existent/foo.git"})
 	err := r.Fetch(&FetchOptions{})
 	c.Assert(err, NotNil)
 }
 
 func (s *RemoteSuite) TestFetchInvalidSchemaEndpoint(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
 	err := r.Fetch(&FetchOptions{})
 	c.Assert(err, ErrorMatches, ".*unsupported scheme.*")
 }
 
 func (s *RemoteSuite) TestFetchInvalidFetchOptions(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
 	invalid := config.RefSpec("^*$ñ")
 	err := r.Fetch(&FetchOptions{RefSpecs: []config.RefSpec{invalid}})
 	c.Assert(err, Equals, ErrInvalidRefSpec)
@@ -53,7 +53,7 @@ func (s *RemoteSuite) TestFetchInvalidFetchOptions(c *C) {
 func (s *RemoteSuite) TestFetch(c *C) {
 	url := s.GetBasicLocalRepositoryURL()
 	sto := memory.NewStorage()
-	r := newRemote(sto, nil, &config.RemoteConfig{Name: "foo", URL: url})
+	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: url})
 
 	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
 	err := r.Fetch(&FetchOptions{
@@ -77,7 +77,7 @@ func (s *RemoteSuite) TestFetch(c *C) {
 func (s *RemoteSuite) TestFetchDepth(c *C) {
 	url := s.GetBasicLocalRepositoryURL()
 	sto := memory.NewStorage()
-	r := newRemote(sto, nil, &config.RemoteConfig{Name: "foo", URL: url})
+	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: url})
 
 	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
 	err := r.Fetch(&FetchOptions{
@@ -112,11 +112,12 @@ func (s *RemoteSuite) TestFetchWithProgress(c *C) {
 	sto := memory.NewStorage()
 	buf := bytes.NewBuffer(nil)
 
-	r := newRemote(sto, buf, &config.RemoteConfig{Name: "foo", URL: url})
+	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: url})
 
 	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
 	err := r.Fetch(&FetchOptions{
 		RefSpecs: []config.RefSpec{refspec},
+		Progress: buf,
 	})
 
 	c.Assert(err, IsNil)
@@ -147,7 +148,7 @@ func (s *RemoteSuite) TestFetchWithPackfileWriter(c *C) {
 	mock := &mockPackfileWriter{Storer: fss}
 
 	url := s.GetBasicLocalRepositoryURL()
-	r := newRemote(mock, nil, &config.RemoteConfig{Name: "foo", URL: url})
+	r := newRemote(mock, &config.RemoteConfig{Name: "foo", URL: url})
 
 	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
 	err = r.Fetch(&FetchOptions{
@@ -183,7 +184,7 @@ func (s *RemoteSuite) TestFetchNoErrAlreadyUpToDateWithNonCommitObjects(c *C) {
 func (s *RemoteSuite) doTestFetchNoErrAlreadyUpToDate(c *C, url string) {
 
 	sto := memory.NewStorage()
-	r := newRemote(sto, nil, &config.RemoteConfig{Name: "foo", URL: url})
+	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: url})
 
 	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
 	o := &FetchOptions{
@@ -197,7 +198,7 @@ func (s *RemoteSuite) doTestFetchNoErrAlreadyUpToDate(c *C, url string) {
 }
 
 func (s *RemoteSuite) TestString(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{
+	r := newRemote(nil, &config.RemoteConfig{
 		Name: "foo",
 		URL:  "https://github.com/git-fixtures/basic.git",
 	})
@@ -216,7 +217,7 @@ func (s *RemoteSuite) TestPushToEmptyRepository(c *C) {
 	dstFs := fixtures.ByTag("empty").One().DotGit()
 	url := fmt.Sprintf("file://%s", dstFs.Base())
 
-	r := newRemote(sto, nil, &config.RemoteConfig{
+	r := newRemote(sto, &config.RemoteConfig{
 		Name: DefaultRemoteName,
 		URL:  url,
 	})
@@ -253,7 +254,7 @@ func (s *RemoteSuite) TestPushNoErrAlreadyUpToDate(c *C) {
 	sto, err := filesystem.NewStorage(f.DotGit())
 	c.Assert(err, IsNil)
 	url := fmt.Sprintf("file://%s", f.DotGit().Base())
-	r := newRemote(sto, nil, &config.RemoteConfig{
+	r := newRemote(sto, &config.RemoteConfig{
 		Name: DefaultRemoteName,
 		URL:  url,
 	})
@@ -266,32 +267,32 @@ func (s *RemoteSuite) TestPushNoErrAlreadyUpToDate(c *C) {
 }
 
 func (s *RemoteSuite) TestPushInvalidEndpoint(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "qux"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux"})
 	err := r.Push(&PushOptions{})
 	c.Assert(err, ErrorMatches, ".*invalid endpoint.*")
 }
 
 func (s *RemoteSuite) TestPushNonExistentEndpoint(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "ssh://non-existent/foo.git"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "ssh://non-existent/foo.git"})
 	err := r.Push(&PushOptions{})
 	c.Assert(err, NotNil)
 }
 
 func (s *RemoteSuite) TestPushInvalidSchemaEndpoint(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
 	err := r.Push(&PushOptions{})
 	c.Assert(err, ErrorMatches, ".*unsupported scheme.*")
 }
 
 func (s *RemoteSuite) TestPushInvalidFetchOptions(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
+	r := newRemote(nil, &config.RemoteConfig{Name: "foo", URL: "qux://foo"})
 	invalid := config.RefSpec("^*$ñ")
 	err := r.Push(&PushOptions{RefSpecs: []config.RefSpec{invalid}})
 	c.Assert(err, Equals, ErrInvalidRefSpec)
 }
 
 func (s *RemoteSuite) TestPushInvalidRefSpec(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{
+	r := newRemote(nil, &config.RemoteConfig{
 		Name: DefaultRemoteName,
 		URL:  "file:///some-url",
 	})
@@ -304,7 +305,7 @@ func (s *RemoteSuite) TestPushInvalidRefSpec(c *C) {
 }
 
 func (s *RemoteSuite) TestPushWrongRemoteName(c *C) {
-	r := newRemote(nil, nil, &config.RemoteConfig{
+	r := newRemote(nil, &config.RemoteConfig{
 		Name: DefaultRemoteName,
 		URL:  "file:///some-url",
 	})

repository.go

@@ -7,7 +7,6 @@ import (
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/object"
-	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/sideband"
 	"gopkg.in/src-d/go-git.v4/plumbing/storer"
 	"gopkg.in/src-d/go-git.v4/storage/filesystem"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
@@ -27,11 +26,6 @@ var (
 type Repository struct {
 	r map[string]*Remote
 	s Storer
-
-	// Progress is where the human readable information sent by the server is
-	// stored, if nil nothing is stored and the capability (if supported)
-	// no-progress, is sent to the server to avoid send this information
-	Progress sideband.Progress
 }
 
 // NewMemoryRepository creates a new repository, backed by a memory.Storage
@@ -77,7 +71,7 @@ func (r *Repository) Remote(name string) (*Remote, error) {
 		return nil, ErrRemoteNotFound
 	}
 
-	return newRemote(r.s, r.Progress, c), nil
+	return newRemote(r.s, c), nil
 }
 
 // Remotes return all the remotes
@@ -91,7 +85,7 @@ func (r *Repository) Remotes() ([]*Remote, error) {
 
 	var i int
 	for _, c := range cfg.Remotes {
-		remotes[i] = newRemote(r.s, r.Progress, c)
+		remotes[i] = newRemote(r.s, c)
 		i++
 	}
 
@@ -104,7 +98,7 @@ func (r *Repository) CreateRemote(c *config.RemoteConfig) (*Remote, error) {
 		return nil, err
 	}
 
-	remote := newRemote(r.s, r.Progress, c)
+	remote := newRemote(r.s, c)
 
 	cfg, err := r.s.Config()
 	if err != nil {
@@ -169,6 +163,7 @@ func (r *Repository) Clone(o *CloneOptions) error {
 		RefSpecs: r.cloneRefSpec(o, c),
 		Depth:    o.Depth,
 		Auth:     o.Auth,
+		Progress: o.Progress,
 	})
 	if err != nil {
 		return err
@@ -343,8 +338,9 @@ func (r *Repository) Pull(o *PullOptions) error {
 	}
 
 	remoteRefs, err := remote.fetch(&FetchOptions{
-		Depth: o.Depth,
-		Auth:  o.Auth,
+		Depth:    o.Depth,
+		Auth:     o.Auth,
+		Progress: o.Progress,
 	})
 
 	updated := true
@@ -405,7 +401,7 @@ func (r *Repository) Push(o *PushOptions) error {
 	return remote.Push(o)
 }
 
-// object.Commit return the commit with the given hash
+// Commit return the commit with the given hash
 func (r *Repository) Commit(h plumbing.Hash) (*object.Commit, error) {
 	return object.GetCommit(r.s, h)
 }