Allow unsupported multi_ack capability

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 to support that
repository users have to work from a clean clone until those
capabilities are fully supported. Commits and pushes back into the
repository have also been tested and work fine.

This change adds an example for cloning Azure DevOps repositories.

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
(cherry picked from commit 9c7d2df)

_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) {