Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion options.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var (
type CloneOptions struct {
// The (possibly remote) repository URL to clone from
URL string
// Auth credentials, if required, to uses with the remote repository
// Auth credentials, if required, to use with the remote repository
Auth transport.AuthMethod
// Name of the remote to be added, by default `origin`
RemoteName string
Expand Down Expand Up @@ -61,6 +61,8 @@ type PullOptions struct {
SingleBranch bool
// Limit fetching to the specified number of commits.
Depth int
// Auth credentials, if required, to use with the remote repository
Auth transport.AuthMethod
}

// Validate validate the fields and set the default values.
Expand All @@ -84,6 +86,8 @@ type FetchOptions struct {
// Depth limit fetching to the specified number of commits from the tip of
// each remote branch history.
Depth int
// Auth credentials, if required, to use with the remote repository
Auth transport.AuthMethod
}

// Validate validate the fields and set the default values
Expand Down
1 change: 1 addition & 0 deletions plumbing/transport/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
ErrAuthorizationRequired = errors.New("authorization required")
ErrEmptyUploadPackRequest = errors.New("empty git-upload-pack given")
ErrInvalidAuthMethod = errors.New("invalid auth method")
ErrAlreadyConnected = errors.New("session already established")
)

const (
Expand Down
10 changes: 10 additions & 0 deletions plumbing/transport/file/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ func (r *runner) Command(cmd string, ep transport.Endpoint) (common.Command, err
case transport.ReceivePackServiceName:
cmd = r.ReceivePackBin
}

if _, err := exec.LookPath(cmd); err != nil {
return nil, err
}

return &command{cmd: exec.Command(cmd, ep.Path)}, nil
}

Expand All @@ -51,6 +56,11 @@ func (c *command) SetAuth(auth transport.AuthMethod) error {
return nil
}

func (c *command) Connect() error {
// the file transport requires no network connectivity
return nil
}

func (c *command) Start() error {
return c.cmd.Start()
}
Expand Down
16 changes: 3 additions & 13 deletions plumbing/transport/git/common.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package git

import (
"errors"
"fmt"
"io"
"net"
Expand All @@ -13,23 +12,14 @@ import (
"gopkg.in/src-d/go-git.v4/utils/ioutil"
)

var (
errAlreadyConnected = errors.New("tcp connection already connected")
)

// DefaultClient is the default git client.
var DefaultClient = common.NewClient(&runner{})

type runner struct{}

// Command returns a new Command for the given cmd in the given Endpoint
func (r *runner) Command(cmd string, ep transport.Endpoint) (common.Command, error) {
c := &command{command: cmd, endpoint: ep}
if err := c.connect(); err != nil {
return nil, err
}

return c, nil
return &command{command: cmd, endpoint: ep}, nil
}

type command struct {
Expand All @@ -52,9 +42,9 @@ func (c *command) Start() error {
return e.Encode([]byte(cmd))
}

func (c *command) connect() error {
func (c *command) Connect() error {
if c.connected {
return errAlreadyConnected
return transport.ErrAlreadyConnected
}

var err error
Expand Down
83 changes: 60 additions & 23 deletions plumbing/transport/internal/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ type Commander interface {
type Command interface {
// SetAuth sets the authentication method.
SetAuth(transport.AuthMethod) error
// Perform the actual network connection.
Connect() error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetAuth is actually a leftover from a previous implementation that should have been gone. Auth can only be set before connection and we don't support reusing commands or connections at all.

So, instead of adding a new Connect function, I think it would be better to just remove SetAuth from all interfaces (Command, Session) and add transport.AuthMethod as a second parameter to NewUploadPackSession and NewReceivePackSession.

That would keep these interfaces simpler and avoid invalid sequences of function calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I had wondered about that. I've moved AuthMethod to the PackSession functions now.

While doing that, I noticed that server also uses these functions. I've left a TODO for now to design / implement AuthMethod for servers. I think this could be used to verify a client connection to the go-git server in the future.

// StderrPipe returns a pipe that will be connected to the command's
// standard error when the command starts. It should not be called after
// Start.
Expand Down Expand Up @@ -100,6 +102,7 @@ type session struct {
Stdout io.Reader
Command Command

connected bool
isReceivePack bool
advRefs *packp.AdvRefs
packRun bool
Expand All @@ -113,35 +116,13 @@ func (c *client) newSession(s string, ep transport.Endpoint) (*session, error) {
return nil, err
}

stdin, err := cmd.StdinPipe()
if err != nil {
return nil, err
}

stdout, err := cmd.StdoutPipe()
if err != nil {
return nil, err
}

stderr, err := cmd.StderrPipe()
if err != nil {
return nil, err
}

if err := cmd.Start(); err != nil {
return nil, err
}

return &session{
Stdin: stdin,
Stdout: stdout,
Command: cmd,
errLines: c.listenErrors(stderr),
isReceivePack: s == transport.ReceivePackServiceName,
}, nil
}

func (c *client) listenErrors(r io.Reader) chan string {
func (s *session) listenErrors(r io.Reader) chan string {
if r == nil {
return nil
}
Expand All @@ -158,13 +139,55 @@ func (c *client) listenErrors(r io.Reader) chan string {
return errLines
}

func (s *session) connect() error {
if s.connected {
return nil
}

err := s.Command.Connect()
if err != nil {
return err
}

stdin, err := s.Command.StdinPipe()
if err != nil {
return err
}

stdout, err := s.Command.StdoutPipe()
if err != nil {
return err
}

stderr, err := s.Command.StderrPipe()
if err != nil {
return err
}

if err := s.Command.Start(); err != nil {
return err
}

s.Stdin = stdin
s.Stdout = stdout
s.errLines = s.listenErrors(stderr)
s.connected = true

return nil
}

// SetAuth delegates to the command's SetAuth.
func (s *session) SetAuth(auth transport.AuthMethod) error {
return s.Command.SetAuth(auth)
}

// AdvertisedReferences retrieves the advertised references from the server.
func (s *session) AdvertisedReferences() (*packp.AdvRefs, error) {
// connect to the remote, allow ErrAlreadyConnected as we might re-use connections
if err := s.connect(); err != nil {
return nil, err
}

if s.advRefs != nil {
return s.advRefs, nil
}
Expand Down Expand Up @@ -222,6 +245,11 @@ func (s *session) handleAdvRefDecodeError(err error) error {
// UploadPack 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) UploadPack(req *packp.UploadPackRequest) (*packp.UploadPackResponse, error) {
// connect to the remote, allow ErrAlreadyConnected as we might re-use connections
if err := s.connect(); err != nil {
return nil, err
}

if req.IsEmpty() {
return nil, transport.ErrEmptyUploadPackRequest
}
Expand Down Expand Up @@ -260,6 +288,11 @@ func (s *session) UploadPack(req *packp.UploadPackRequest) (*packp.UploadPackRes
}

func (s *session) ReceivePack(req *packp.ReferenceUpdateRequest) (*packp.ReportStatus, error) {
// connect to the remote, allow ErrAlreadyConnected as we might re-use connections
if err := s.connect(); err != nil {
return nil, err
}

if _, err := s.AdvertisedReferences(); err != nil {
return nil, err
}
Expand Down Expand Up @@ -307,6 +340,10 @@ func (s *session) finish() error {
}

func (s *session) Close() error {
if !s.connected {
return nil
}

if err := s.finish(); err != nil {
_ = s.Command.Close()
return nil
Expand Down
22 changes: 7 additions & 15 deletions plumbing/transport/ssh/common.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package ssh

import (
"errors"
"fmt"
"strings"

Expand All @@ -11,22 +10,13 @@ import (
"golang.org/x/crypto/ssh"
)

var (
errAlreadyConnected = errors.New("ssh session already created")
)

// DefaultClient is the default SSH client.
var DefaultClient = common.NewClient(&runner{})

type runner struct{}

func (r *runner) Command(cmd string, ep transport.Endpoint) (common.Command, error) {
c := &command{command: cmd, endpoint: ep}
if err := c.connect(); err != nil {
return nil, err
}

return c, nil
return &command{command: cmd, endpoint: ep}, nil
}

type command struct {
Expand Down Expand Up @@ -71,13 +61,15 @@ func (c *command) Close() error {
// SetAuth method, by default uses an auth method based on PublicKeysCallback,
// it connects to a SSH agent, using the address stored in the SSH_AUTH_SOCK
// environment var.
func (c *command) connect() error {
func (c *command) Connect() error {
if c.connected {
return errAlreadyConnected
return transport.ErrAlreadyConnected
}

if err := c.setAuthFromEndpoint(); err != nil {
return err
if c.auth == nil {
if err := c.setAuthFromEndpoint(); err != nil {
return err
}
}

var err error
Expand Down
4 changes: 4 additions & 0 deletions remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ func (r *Remote) fetch(o *FetchOptions) (refs storer.ReferenceStorer, err error)
return nil, err
}

if o.Auth != nil {
s.SetAuth(o.Auth)
}

defer ioutil.CheckClose(s, &err)

ar, err := s.AdvertisedReferences()
Expand Down
2 changes: 2 additions & 0 deletions repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ func (r *Repository) Clone(o *CloneOptions) error {
remoteRefs, err := remote.fetch(&FetchOptions{
RefSpecs: r.cloneRefSpec(o, c),
Depth: o.Depth,
Auth: o.Auth,
})
if err != nil {
return err
Expand Down Expand Up @@ -321,6 +322,7 @@ func (r *Repository) Pull(o *PullOptions) error {

remoteRefs, err := remote.fetch(&FetchOptions{
Depth: o.Depth,
Auth: o.Auth,
})
if err != nil {
return err
Expand Down