Merge pull request #613 from fluxcd/unsupported

Allow unsupported `multi_ack` capability

_examples/azure_devops/main.go

@@ -0,0 +1,56 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	git "github.com/go-git/go-git/v5"
+	. "github.com/go-git/go-git/v5/_examples"
+	"github.com/go-git/go-git/v5/plumbing/protocol/packp/capability"
+	"github.com/go-git/go-git/v5/plumbing/transport"
+	"github.com/go-git/go-git/v5/plumbing/transport/http"
+)
+
+func main() {
+	CheckArgs("<url>", "<directory>", "<azuredevops_username>", "<azuredevops_password>")
+	url, directory, username, password := os.Args[1], os.Args[2], os.Args[3], os.Args[4]
+
+	// Clone the given repository to the given directory
+	Info("git clone %s %s", url, directory)
+
+	// Azure DevOps requires capabilities multi_ack / multi_ack_detailed,
+	// which are not fully implemented and by default are included in
+	// transport.UnsupportedCapabilities.
+	//
+	// The initial clone operations require a full download of the repository,
+	// and therefore those unsupported capabilities are not as crucial, so
+	// by removing them from that list allows for the first clone to work
+	// successfully.
+	//
+	// Additional fetches will yield issues, therefore work always from a clean
+	// clone until those capabilities are fully supported.
+	//
+	// New commits and pushes against a remote worked without any issues.
+	transport.UnsupportedCapabilities = []capability.Capability{
+		capability.ThinPack,
+	}
+
+	r, err := git.PlainClone(directory, false, &git.CloneOptions{
+		Auth: &http.BasicAuth{
+			Username: username,
+			Password: password,
+		},
+		URL:      url,
+		Progress: os.Stdout,
+	})
+	CheckIfError(err)
+
+	// ... retrieving the branch being pointed by HEAD
+	ref, err := r.Head()
+	CheckIfError(err)
+	// ... retrieving the commit object
+	commit, err := r.CommitObject(ref.Hash())
+	CheckIfError(err)
+
+	fmt.Println(commit)
+}

plumbing/protocol/packp/srvresp.go

@@ -21,11 +21,6 @@ type ServerResponse struct {
 // 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 *bufio.Reader, isMultiACK bool) error {
-	// TODO: implement support for multi_ack or multi_ack_detailed responses
-	if isMultiACK {
-		return errors.New("multi_ack and multi_ack_detailed are not supported")
-	}
-
 	s := pktline.NewScanner(reader)
 
 	for s.Scan() {
@@ -48,7 +43,23 @@ func (r *ServerResponse) Decode(reader *bufio.Reader, isMultiACK bool) error {
 		}
 	}
 
-	return s.Err()
+	// isMultiACK is true when the remote server advertises the related
+	// capabilities when they are not in transport.UnsupportedCapabilities.
+	//
+	// Users may decide to remove multi_ack and multi_ack_detailed from the
+	// unsupported capabilities list, which allows them to do initial clones
+	// from Azure DevOps.
+	//
+	// Follow-up fetches may error, therefore errors are wrapped with additional
+	// information highlighting that this capabilities are not supported by go-git.
+	//
+	// TODO: Implement support for multi_ack or multi_ack_detailed responses.
+	err := s.Err()
+	if err != nil && isMultiACK {
+		return fmt.Errorf("multi_ack and multi_ack_detailed are not supported: %w", err)
+	}
+
+	return err
 }
 
 // stopReading detects when a valid command such as ACK or NAK is found to be
@@ -113,8 +124,9 @@ func (r *ServerResponse) decodeACKLine(line []byte) error {
 }
 
 // Encode encodes the ServerResponse into a writer.
-func (r *ServerResponse) Encode(w io.Writer) error {
-	if len(r.ACKs) > 1 {
+func (r *ServerResponse) Encode(w io.Writer, isMultiACK bool) error {
+	if len(r.ACKs) > 1 && !isMultiACK {
+		// For further information, refer to comments in the Decode func above.
 		return errors.New("multi_ack and multi_ack_detailed are not supported")
 	}
 

plumbing/protocol/packp/srvresp_test.go

@@ -72,8 +72,21 @@ func (s *ServerResponseSuite) TestDecodeMalformed(c *C) {
 	c.Assert(err, NotNil)
 }
 
+// multi_ack isn't fully implemented, this ensures that Decode ignores that fact,
+// as in some circumstances that's OK to assume so.
+//
+// TODO: Review as part of multi_ack implementation.
 func (s *ServerResponseSuite) TestDecodeMultiACK(c *C) {
+	raw := "" +
+		"0031ACK 1111111111111111111111111111111111111111\n" +
+		"0031ACK 6ecf0ef2c2dffb796033e5a02219af86ec6584e5\n" +
+		"00080PACK\n"
+
 	sr := &ServerResponse{}
-	err := sr.Decode(bufio.NewReader(bytes.NewBuffer(nil)), true)
-	c.Assert(err, NotNil)
+	err := sr.Decode(bufio.NewReader(bytes.NewBufferString(raw)), true)
+	c.Assert(err, IsNil)
+
+	c.Assert(sr.ACKs, HasLen, 2)
+	c.Assert(sr.ACKs[0], Equals, plumbing.NewHash("1111111111111111111111111111111111111111"))
+	c.Assert(sr.ACKs[1], Equals, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
 }

plumbing/protocol/packp/uppackresp.go

@@ -78,7 +78,7 @@ func (r *UploadPackResponse) Encode(w io.Writer) (err error) {
 		}
 	}
 
-	if err := r.ServerResponse.Encode(w); err != nil {
+	if err := r.ServerResponse.Encode(w, r.isMultiACK); err != nil {
 		return err
 	}
 

plumbing/protocol/packp/uppackresp_test.go

@@ -59,6 +59,10 @@ func (s *UploadPackResponseSuite) TestDecodeMalformed(c *C) {
 	c.Assert(err, NotNil)
 }
 
+// multi_ack isn't fully implemented, this ensures that Decode ignores that fact,
+// as in some circumstances that's OK to assume so.
+//
+// TODO: Review as part of multi_ack implementation.
 func (s *UploadPackResponseSuite) TestDecodeMultiACK(c *C) {
 	req := NewUploadPackRequest()
 	req.Capabilities.Set(capability.MultiACK)
@@ -67,7 +71,7 @@ func (s *UploadPackResponseSuite) TestDecodeMultiACK(c *C) {
 	defer res.Close()
 
 	err := res.Decode(ioutil.NopCloser(bytes.NewBuffer(nil)))
-	c.Assert(err, NotNil)
+	c.Assert(err, IsNil)
 }
 
 func (s *UploadPackResponseSuite) TestReadNoDecode(c *C) {