From bf3471db54b0255ab5b159005069f37528a151b7 Mon Sep 17 00:00:00 2001 From: Andrew Suffield Date: Thu, 25 Mar 2021 10:21:38 +0000 Subject: [PATCH] add RequireRemoteRefs to PushOptions (#258) The git protocol itself uses a compare-and-swap mechanism, where changes send the old and new values and the change is only applied if the old value matches. This is used to implement the --force-with-lease feature in git push. go-git populates the `old` field with the current value of the ref that is read from the remote. We can implement a convenient (albeit more limited) form of the --force-with-lease feature just by allowing the caller to specify particular values for this ref. Callers can then implement complex multi-step atomic operations by reading the ref themselves at the start of the process, and passing to in RequireRemoteRefs at the end. This is also a suitable building block for implementing --force-with-lease (#101), which is mostly an exercise in computing the correct hash to require. Hence, this appears to be the most reasonable API to expose. --- options.go | 3 +++ remote.go | 34 ++++++++++++++++++++++++++++++++ remote_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/options.go b/options.go index 507fc0714..b5d150376 100644 --- a/options.go +++ b/options.go @@ -210,6 +210,9 @@ type PushOptions struct { InsecureSkipTLS bool // CABundle specify additional ca bundle with system cert pool CABundle []byte + // RequireRemoteRefs only allows a remote ref to be updated if its current + // value is the one specified here. + RequireRemoteRefs []config.RefSpec } // Validate validates the fields and sets the default values. diff --git a/remote.go b/remote.go index 66ba71edc..dde044500 100644 --- a/remote.go +++ b/remote.go @@ -119,6 +119,10 @@ func (r *Remote) PushContext(ctx context.Context, o *PushOptions) (err error) { return err } + if err := r.checkRequireRemoteRefs(o.RequireRemoteRefs, remoteRefs); err != nil { + return err + } + isDelete := false allDelete := true for _, rs := range o.RefSpecs { @@ -1166,3 +1170,33 @@ outer: return r.s.SetShallow(shallows) } + +func (r *Remote) checkRequireRemoteRefs(requires []config.RefSpec, remoteRefs storer.ReferenceStorer) error { + for _, require := range requires { + if require.IsWildcard() { + return fmt.Errorf("wildcards not supported in RequireRemoteRefs, got %s", require.String()) + } + + name := require.Dst("") + remote, err := remoteRefs.Reference(name) + if err != nil { + return fmt.Errorf("remote ref %s required to be %s but is absent", name.String(), require.Src()) + } + + var requireHash string + if require.IsExactSHA1() { + requireHash = require.Src() + } else { + target, err := storer.ResolveReference(remoteRefs, plumbing.ReferenceName(require.Src())) + if err != nil { + return fmt.Errorf("could not resolve ref %s in RequireRemoteRefs", require.Src()) + } + requireHash = target.Hash().String() + } + + if remote.Hash().String() != requireHash { + return fmt.Errorf("remote ref %s required to be %s but is %s", name.String(), requireHash, remote.Hash().String()) + } + } + return nil +} diff --git a/remote_test.go b/remote_test.go index 3446f1a86..bc05b7e55 100644 --- a/remote_test.go +++ b/remote_test.go @@ -971,3 +971,56 @@ func (s *RemoteSuite) TestUseRefDeltas(c *C) { ar.Capabilities.Delete(capability.OFSDelta) c.Assert(r.useRefDeltas(ar), Equals, true) } + +func (s *RemoteSuite) TestPushRequireRemoteRefs(c *C) { + f := fixtures.Basic().One() + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) + + dstFs := f.DotGit() + dstSto := filesystem.NewStorage(dstFs, cache.NewObjectLRUDefault()) + + url := dstFs.Root() + r := NewRemote(sto, &config.RemoteConfig{ + Name: DefaultRemoteName, + URLs: []string{url}, + }) + + oldRef, err := dstSto.Reference(plumbing.ReferenceName("refs/heads/branch")) + c.Assert(err, IsNil) + c.Assert(oldRef, NotNil) + + otherRef, err := dstSto.Reference(plumbing.ReferenceName("refs/heads/master")) + c.Assert(err, IsNil) + c.Assert(otherRef, NotNil) + + err = r.Push(&PushOptions{ + RefSpecs: []config.RefSpec{"refs/heads/master:refs/heads/branch"}, + RequireRemoteRefs: []config.RefSpec{config.RefSpec(otherRef.Hash().String() + ":refs/heads/branch")}, + }) + c.Assert(err, ErrorMatches, "remote ref refs/heads/branch required to be .* but is .*") + + newRef, err := dstSto.Reference(plumbing.ReferenceName("refs/heads/branch")) + c.Assert(err, IsNil) + c.Assert(newRef, DeepEquals, oldRef) + + err = r.Push(&PushOptions{ + RefSpecs: []config.RefSpec{"refs/heads/master:refs/heads/branch"}, + RequireRemoteRefs: []config.RefSpec{config.RefSpec(oldRef.Hash().String() + ":refs/heads/branch")}, + }) + c.Assert(err, ErrorMatches, "non-fast-forward update: .*") + + newRef, err = dstSto.Reference(plumbing.ReferenceName("refs/heads/branch")) + c.Assert(err, IsNil) + c.Assert(newRef, DeepEquals, oldRef) + + err = r.Push(&PushOptions{ + RefSpecs: []config.RefSpec{"refs/heads/master:refs/heads/branch"}, + RequireRemoteRefs: []config.RefSpec{config.RefSpec(oldRef.Hash().String() + ":refs/heads/branch")}, + Force: true, + }) + c.Assert(err, IsNil) + + newRef, err = dstSto.Reference(plumbing.ReferenceName("refs/heads/branch")) + c.Assert(err, IsNil) + c.Assert(newRef, Not(DeepEquals), oldRef) +}