Properly handle update-requests capabilities and always use context I/O

internal/transport/test/receive_pack.go

@@ -274,11 +274,21 @@ func (s *ReceivePackSuite) receivePackNoCheck(ep *transport.Endpoint,
 		s.Require().NotNil(info, comment)
 	}
 
-	if fixture != nil {
-		s.Require().NotNil(fixture.Packfile())
-		req.Packfile = fixture.Packfile()
-	} else {
-		req.Packfile = s.emptyPackfile()
+	var needPackfile bool
+	for _, cmd := range req.Commands {
+		if cmd.Action() != packp.Delete {
+			needPackfile = true
+			break
+		}
+	}
+
+	if needPackfile {
+		if fixture != nil {
+			s.Require().NotNil(fixture.Packfile())
+			req.Packfile = fixture.Packfile()
+		} else {
+			req.Packfile = s.emptyPackfile()
+		}
 	}
 
 	return conn.Push(ctx, req)
@@ -370,9 +380,6 @@ func (s *ReceivePackSuite) testSendPackAddReference() {
 			{Name: "refs/heads/newbranch", Old: plumbing.ZeroHash, New: plumbing.NewHash(fixture.Head)},
 		},
 	}
-	// if refs.Capabilities.Supports(capability.ReportStatus) {
-	// 	req.Capabilities.Set(capability.ReportStatus)
-	// }
 
 	s.receivePack(s.Endpoint, req, fixture, false)
 	s.checkRemoteReference(s.Endpoint, "refs/heads/newbranch", plumbing.NewHash(fixture.Head))
@@ -400,9 +407,6 @@ func (s *ReceivePackSuite) testSendPackDeleteReference() {
 			{Name: "refs/heads/newbranch", Old: plumbing.NewHash(fixture.Head), New: plumbing.ZeroHash},
 		},
 	}
-	// if refs.Capabilities.Supports(capability.ReportStatus) {
-	// 	req.Capabilities.Set(capability.ReportStatus)
-	// }
 
 	if !caps.Supports(capability.DeleteRefs) {
 		s.Fail("capability delete-refs not supported")

plumbing/protocol/packp/capability/capability.go

@@ -195,6 +195,13 @@ const (
 	// successful, it will send back an error message.  See pack-protocol.txt
 	// for example messages.
 	ReportStatus Capability = "report-status"
+	// ReportStatusV2 extends capability report-status by adding new "option"
+	// directives in order to support reference rewritten by the "proc-receive"
+	// hook. The "proc-receive" hook may handle a command for a
+	// pseudo-reference which may create or update a reference with different
+	// name, new-oid, and old-oid. While the capability report-status cannot
+	// report for such case. See gitprotocol-pack[5] for details.
+	ReportStatusV2 Capability = "report-status-v2"
 	// DeleteRefs If the server sends back this capability, it means that
 	// it is capable of accepting a zero-id value as the target
 	// value of a reference update.  It is not sent back by the client, it

plumbing/transport/push.go

@@ -17,24 +17,23 @@ import (
 func buildUpdateRequests(caps *capability.List, req *PushRequest) *packp.UpdateRequests {
 	upreq := packp.NewUpdateRequests()
 
-	// Prepare the request and capabilities
-	if caps.Supports(capability.Agent) {
-		upreq.Capabilities.Set(capability.Agent, capability.DefaultAgent()) // nolint: errcheck
-	}
-	if caps.Supports(capability.ReportStatus) {
-		upreq.Capabilities.Set(capability.ReportStatus) // nolint: errcheck
-	}
-	if req.Progress != nil {
-		if caps.Supports(capability.Sideband64k) {
-			upreq.Capabilities.Set(capability.Sideband64k) // nolint: errcheck
-		} else if caps.Supports(capability.Sideband) {
-			upreq.Capabilities.Set(capability.Sideband) // nolint: errcheck
+	// The reference discovery phase is done nearly the same way as it is in
+	// the fetching protocol. Each reference obj-id and name on the server is
+	// sent in packet-line format to the client, followed by a flush-pkt. The
+	// only real difference is that the capability listing is different - the
+	// only possible values are report-status, report-status-v2, delete-refs,
+	// ofs-delta, atomic and push-options.
+	for _, cap := range []capability.Capability{
+		capability.ReportStatus,
+		capability.ReportStatusV2,
+		capability.DeleteRefs,
+		capability.OFSDelta,
+		capability.Atomic,
+		// capability.PushOptions, // This is set later if options are present.
+	} {
+		if caps.Supports(cap) {
+			upreq.Capabilities.Set(cap) //nolint:errcheck
 		}
-	} else if caps.Supports(capability.NoProgress) {
-		upreq.Capabilities.Set(capability.NoProgress) // nolint: errcheck
-	}
-	if req.Atomic && caps.Supports(capability.Atomic) {
-		upreq.Capabilities.Set(capability.Atomic) // nolint: errcheck
 	}
 
 	upreq.Commands = req.Commands
@@ -54,6 +53,21 @@ func SendPack(
 	writer = ioutil.NewContextWriteCloser(ctx, writer)
 	reader = ioutil.NewContextReadCloser(ctx, reader)
 
+	var needPackfile bool
+	for _, cmd := range req.Commands {
+		if cmd.Action() != packp.Delete {
+			needPackfile = true
+			break
+		}
+	}
+
+	if !needPackfile && req.Packfile != nil {
+		return fmt.Errorf("packfile is not accepted for push request without new objects")
+	}
+	if needPackfile && req.Packfile == nil {
+		return fmt.Errorf("packfile is required for push request with new objects")
+	}
+
 	caps := conn.Capabilities()
 	upreq := buildUpdateRequests(caps, req)
 	usePushOptions := len(req.Options) > 0 && caps.Supports(capability.PushOptions)

plumbing/transport/receive_pack.go

@@ -38,6 +38,8 @@ func ReceivePack(
 		return fmt.Errorf("nil writer")
 	}
 
+	w = ioutil.NewContextWriteCloser(ctx, w)
+
 	if opts == nil {
 		opts = &ReceivePackOptions{}
 	}
@@ -68,6 +70,8 @@ func ReceivePack(
 		return fmt.Errorf("nil reader")
 	}
 
+	r = ioutil.NewContextReadCloser(ctx, r)
+
 	rd := bufio.NewReader(r)
 	l, _, err := pktline.PeekLine(rd)
 	if err != nil {
@@ -100,7 +104,7 @@ func ReceivePack(
 
 	// Should we expect a packfile?
 	for _, cmd := range updreq.Commands {
-		if cmd.New != plumbing.ZeroHash {
+		if cmd.Action() != packp.Delete {
 			needPackfile = true
 			break
 		}

plumbing/transport/upload_pack.go

@@ -17,6 +17,7 @@ import (
 	"github.com/go-git/go-git/v6/plumbing/protocol/packp/sideband"
 	"github.com/go-git/go-git/v6/plumbing/revlist"
 	"github.com/go-git/go-git/v6/storage"
+	"github.com/go-git/go-git/v6/utils/ioutil"
 )
 
 // UploadPackOptions is a set of options for the UploadPack service.
@@ -38,6 +39,8 @@ func UploadPack(
 		return fmt.Errorf("nil writer")
 	}
 
+	w = ioutil.NewContextWriteCloser(ctx, w)
+
 	if opts == nil {
 		opts = &UploadPackOptions{}
 	}
@@ -68,6 +71,8 @@ func UploadPack(
 		return fmt.Errorf("nil reader")
 	}
 
+	r = ioutil.NewContextReadCloser(ctx, r)
+
 	rd := bufio.NewReader(r)
 	l, _, err := pktline.PeekLine(rd)
 	if err != nil {