diff options
author | Santiago M. Mola <santi@mola.io> | 2017-07-10 18:25:28 +0200 |
---|---|---|
committer | Santiago M. Mola <santi@mola.io> | 2017-07-11 10:30:51 +0200 |
commit | 09f5f2a35f0f4c42b2def0642540f32e59fdeeb3 (patch) | |
tree | 02bfd08632ac8559e6f35cdbdb218de78a21e1e4 | |
parent | 6b69a1630b30c41f4563fd95aca1d647ba611adf (diff) | |
download | go-git-09f5f2a35f0f4c42b2def0642540f32e59fdeeb3.tar.gz |
improve delete support on push
* server: implement delete-refs and announce it.
* remote: check if server announced delete-refs before trying
to delete and fail fast if it does not.
Note that the client does not need no send 'delete-refs' back
to the server to be able to delete references:
```
delete-refs
-----------
If the server sends back the 'delete-refs' 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
simply informs the client that it can be sent zero-id values
to delete references.
```
So our server implementation does not check if the client sent
delete-refs back, it just accepts deletes if it receives them.
-rw-r--r-- | plumbing/protocol/packp/updreq.go | 1 | ||||
-rw-r--r-- | plumbing/transport/server/receive_pack_test.go | 5 | ||||
-rw-r--r-- | plumbing/transport/server/server.go | 44 | ||||
-rw-r--r-- | plumbing/transport/test/receive_pack.go | 4 | ||||
-rw-r--r-- | remote.go | 22 |
5 files changed, 35 insertions, 41 deletions
diff --git a/plumbing/protocol/packp/updreq.go b/plumbing/protocol/packp/updreq.go index 0624930..b246613 100644 --- a/plumbing/protocol/packp/updreq.go +++ b/plumbing/protocol/packp/updreq.go @@ -42,6 +42,7 @@ func NewReferenceUpdateRequest() *ReferenceUpdateRequest { // - report-status // - ofs-delta // - ref-delta +// - delete-refs // It leaves up to the user to add the following capabilities later: // - atomic // - ofs-delta diff --git a/plumbing/transport/server/receive_pack_test.go b/plumbing/transport/server/receive_pack_test.go index 73ba60b..54c2fba 100644 --- a/plumbing/transport/server/receive_pack_test.go +++ b/plumbing/transport/server/receive_pack_test.go @@ -27,11 +27,6 @@ func (s *ReceivePackSuite) TearDownTest(c *C) { s.Suite.TearDownSuite(c) } -// TODO -func (s *ReceivePackSuite) TestSendPackAddDeleteReference(c *C) { - c.Skip("delete reference not supported yet") -} - // Overwritten, server returns error earlier. func (s *ReceivePackSuite) TestAdvertisedReferencesNotExists(c *C) { r, err := s.Client.NewReceivePackSession(s.NonExistentEndpoint, s.EmptyAuth) diff --git a/plumbing/transport/server/server.go b/plumbing/transport/server/server.go index 89fce5f..dd278c8 100644 --- a/plumbing/transport/server/server.go +++ b/plumbing/transport/server/server.go @@ -225,31 +225,11 @@ func (s *rpSession) ReceivePack(req *packp.ReferenceUpdateRequest) (*packp.Repor return s.reportStatus(), err } - updatedRefs := s.updatedReferences(req) - - if s.caps.Supports(capability.Atomic) && s.firstErr != nil { - //TODO: add support for 'atomic' once we have reference - // transactions, currently we do not announce it. - rs := s.reportStatus() - for _, cs := range rs.CommandStatuses { - if cs.Error() == nil { - cs.Status = "" - } - } - } - - for name, ref := range updatedRefs { - //TODO: add support for 'delete-refs' once we can delete - // references, currently we do not announce it. - err := s.storer.SetReference(ref) - s.setStatus(name, err) - } - + s.updateReferences(req) return s.reportStatus(), s.firstErr } -func (s *rpSession) updatedReferences(req *packp.ReferenceUpdateRequest) map[plumbing.ReferenceName]*plumbing.Reference { - refs := map[plumbing.ReferenceName]*plumbing.Reference{} +func (s *rpSession) updateReferences(req *packp.ReferenceUpdateRequest) { for _, cmd := range req.Commands { exists, err := referenceExists(s.storer, cmd.Name) if err != nil { @@ -265,19 +245,16 @@ func (s *rpSession) updatedReferences(req *packp.ReferenceUpdateRequest) map[plu } ref := plumbing.NewHashReference(cmd.Name, cmd.New) - refs[ref.Name()] = ref + err := s.storer.SetReference(ref) + s.setStatus(cmd.Name, err) case packp.Delete: if !exists { s.setStatus(cmd.Name, ErrUpdateReference) continue } - if !s.caps.Supports(capability.DeleteRefs) { - s.setStatus(cmd.Name, fmt.Errorf("delete not supported")) - continue - } - - refs[cmd.Name] = nil + err := s.storer.RemoveReference(cmd.Name) + s.setStatus(cmd.Name, err) case packp.Update: if !exists { s.setStatus(cmd.Name, ErrUpdateReference) @@ -290,11 +267,10 @@ func (s *rpSession) updatedReferences(req *packp.ReferenceUpdateRequest) map[plu } ref := plumbing.NewHashReference(cmd.Name, cmd.New) - refs[ref.Name()] = ref + err := s.storer.SetReference(ref) + s.setStatus(cmd.Name, err) } } - - return refs } func (s *rpSession) failAtomicUpdate() (*packp.ReportStatus, error) { @@ -368,6 +344,10 @@ func (*rpSession) setSupportedCapabilities(c *capability.List) error { return err } + if err := c.Set(capability.DeleteRefs); err != nil { + return err + } + return c.Set(capability.ReportStatus) } diff --git a/plumbing/transport/test/receive_pack.go b/plumbing/transport/test/receive_pack.go index bb1c58a..15172c8 100644 --- a/plumbing/transport/test/receive_pack.go +++ b/plumbing/transport/test/receive_pack.go @@ -308,6 +308,10 @@ func (s *ReceivePackSuite) testSendPackDeleteReference(c *C) { req.Capabilities.Set(capability.ReportStatus) } + if !ar.Capabilities.Supports(capability.DeleteRefs) { + c.Fatal("capability delete-refs not supported") + } + c.Assert(r.Close(), IsNil) s.receivePack(c, s.Endpoint, req, nil, false) @@ -21,7 +21,10 @@ import ( "gopkg.in/src-d/go-git.v4/utils/ioutil" ) -var NoErrAlreadyUpToDate = errors.New("already up-to-date") +var ( + NoErrAlreadyUpToDate = errors.New("already up-to-date") + ErrDeleteRefNotSupported = errors.New("server does not support delete-refs") +) // Remote represents a connection to a remote repository. type Remote struct { @@ -56,7 +59,6 @@ func (r *Remote) Fetch(o *FetchOptions) error { // Push performs a push to the remote. Returns NoErrAlreadyUpToDate if the // remote was already up-to-date. func (r *Remote) Push(o *PushOptions) (err error) { - // TODO: Support deletes. // TODO: Sideband support if o.RemoteName == "" { @@ -88,6 +90,18 @@ func (r *Remote) Push(o *PushOptions) (err error) { return err } + isDelete := false + for _, rs := range o.RefSpecs { + if rs.IsDelete() { + isDelete = true + break + } + } + + if isDelete && !ar.Capabilities.Supports(capability.DeleteRefs) { + return ErrDeleteRefNotSupported + } + req := packp.NewReferenceUpdateRequestFromCapabilities(ar.Capabilities) if err := r.addReferencesToUpdate(o.RefSpecs, remoteRefs, req); err != nil { return err @@ -284,8 +298,8 @@ func (r *Remote) deleteReferences(rs config.RefSpec, cmd := &packp.Command{ Name: ref.Name(), - Old: ref.Hash(), - New: plumbing.ZeroHash, + Old: ref.Hash(), + New: plumbing.ZeroHash, } req.Commands = append(req.Commands, cmd) return nil |