diff options
-rw-r--r-- | options.go | 6 | ||||
-rw-r--r-- | plumbing/storer/reference.go | 5 | ||||
-rw-r--r-- | remote.go | 30 | ||||
-rw-r--r-- | remote_test.go | 59 | ||||
-rw-r--r-- | repository.go | 13 | ||||
-rw-r--r-- | storage/filesystem/internal/dotgit/dotgit.go | 66 | ||||
-rw-r--r-- | storage/filesystem/internal/dotgit/dotgit_test.go | 25 | ||||
-rw-r--r-- | storage/filesystem/reference.go | 6 | ||||
-rw-r--r-- | storage/memory/storage.go | 16 | ||||
-rw-r--r-- | worktree.go | 1 |
10 files changed, 205 insertions, 22 deletions
@@ -95,6 +95,9 @@ type PullOptions struct { // stored, if nil nothing is stored and the capability (if supported) // no-progress, is sent to the server to avoid send this information. Progress sideband.Progress + // Force allows the pull to update a local branch even when the remote + // branch does not descend from it. + Force bool } // Validate validates the fields and sets the default values. @@ -142,6 +145,9 @@ type FetchOptions struct { // Tags describe how the tags will be fetched from the remote repository, // by default is TagFollowing. Tags TagMode + // Force allows the fetch to update a local branch even when the remote + // branch does not descend from it. + Force bool } // Validate validates the fields and sets the default values. diff --git a/plumbing/storer/reference.go b/plumbing/storer/reference.go index 5c21f23..ae80a39 100644 --- a/plumbing/storer/reference.go +++ b/plumbing/storer/reference.go @@ -16,6 +16,11 @@ var ErrMaxResolveRecursion = errors.New("max. recursion level reached") // ReferenceStorer is a generic storage of references. type ReferenceStorer interface { SetReference(*plumbing.Reference) error + // CheckAndSetReference sets the reference `new`, but if `old` is + // not `nil`, it first checks that the current stored value for + // `old.Name()` matches the given reference value in `old`. If + // not, it returns an error and doesn't update `new`. + CheckAndSetReference(new, old *plumbing.Reference) error Reference(plumbing.ReferenceName) (*plumbing.Reference, error) IterReferences() (ReferenceIter, error) RemoveReference(plumbing.ReferenceName) error @@ -25,6 +25,7 @@ import ( var ( NoErrAlreadyUpToDate = errors.New("already up-to-date") ErrDeleteRefNotSupported = errors.New("server does not support delete-refs") + ErrForceNeeded = errors.New("some refs were not updated") ) const ( @@ -302,7 +303,7 @@ func (r *Remote) fetch(ctx context.Context, o *FetchOptions) (storer.ReferenceSt } } - updated, err := r.updateLocalReferenceStorage(o.RefSpecs, refs, remoteRefs, o.Tags) + updated, err := r.updateLocalReferenceStorage(o.RefSpecs, refs, remoteRefs, o.Tags, o.Force) if err != nil { return nil, err } @@ -773,8 +774,11 @@ func (r *Remote) updateLocalReferenceStorage( specs []config.RefSpec, fetchedRefs, remoteRefs memory.ReferenceStorage, tagMode TagMode, + force bool, ) (updated bool, err error) { isWildcard := true + forceNeeded := false + for _, spec := range specs { if !spec.IsWildcard() { isWildcard = false @@ -789,9 +793,25 @@ func (r *Remote) updateLocalReferenceStorage( continue } - new := plumbing.NewHashReference(spec.Dst(ref.Name()), ref.Hash()) + localName := spec.Dst(ref.Name()) + old, _ := storer.ResolveReference(r.s, localName) + new := plumbing.NewHashReference(localName, ref.Hash()) + + // If the ref exists locally as a branch and force is not specified, + // only update if the new ref is an ancestor of the old + if old != nil && old.Name().IsBranch() && !force && !spec.IsForceUpdate() { + ff, err := isFastForward(r.s, old.Hash(), new.Hash()) + if err != nil { + return updated, err + } + + if !ff { + forceNeeded = true + continue + } + } - refUpdated, err := updateReferenceStorerIfNeeded(r.s, new) + refUpdated, err := checkAndUpdateReferenceStorerIfNeeded(r.s, new, old) if err != nil { return updated, err } @@ -819,6 +839,10 @@ func (r *Remote) updateLocalReferenceStorage( updated = true } + if err == nil && forceNeeded { + err = ErrForceNeeded + } + return } diff --git a/remote_test.go b/remote_test.go index 3452ddf..e586e7a 100644 --- a/remote_test.go +++ b/remote_test.go @@ -308,6 +308,65 @@ func (s *RemoteSuite) doTestFetchNoErrAlreadyUpToDate(c *C, url string) { c.Assert(err, Equals, NoErrAlreadyUpToDate) } +func (s *RemoteSuite) testFetchFastForward(c *C, sto storage.Storer) { + r := newRemote(sto, &config.RemoteConfig{ + URLs: []string{s.GetBasicLocalRepositoryURL()}, + }) + + s.testFetch(c, r, &FetchOptions{ + RefSpecs: []config.RefSpec{ + config.RefSpec("+refs/heads/master:refs/heads/master"), + }, + }, []*plumbing.Reference{ + plumbing.NewReferenceFromStrings("refs/heads/master", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5"), + }) + + // First make sure that we error correctly when a force is required. + err := r.Fetch(&FetchOptions{ + RefSpecs: []config.RefSpec{ + config.RefSpec("refs/heads/branch:refs/heads/master"), + }, + }) + c.Assert(err, Equals, ErrForceNeeded) + + // And that forcing it fixes the problem. + err = r.Fetch(&FetchOptions{ + RefSpecs: []config.RefSpec{ + config.RefSpec("+refs/heads/branch:refs/heads/master"), + }, + }) + c.Assert(err, IsNil) + + // Now test that a fast-forward, non-force fetch works. + r.s.SetReference(plumbing.NewReferenceFromStrings( + "refs/heads/master", "918c48b83bd081e863dbe1b80f8998f058cd8294", + )) + s.testFetch(c, r, &FetchOptions{ + RefSpecs: []config.RefSpec{ + config.RefSpec("refs/heads/master:refs/heads/master"), + }, + }, []*plumbing.Reference{ + plumbing.NewReferenceFromStrings("refs/heads/master", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5"), + }) +} + +func (s *RemoteSuite) TestFetchFastForwardMem(c *C) { + s.testFetchFastForward(c, memory.NewStorage()) +} + +func (s *RemoteSuite) TestFetchFastForwardFS(c *C) { + dir, err := ioutil.TempDir("", "fetch") + c.Assert(err, IsNil) + + defer os.RemoveAll(dir) // clean up + + fss, err := filesystem.NewStorage(osfs.New(dir)) + c.Assert(err, IsNil) + + // This exercises `storage.filesystem.Storage.CheckAndSetReference()`. + s.testFetchFastForward(c, fss) +} + func (s *RemoteSuite) TestString(c *C) { r := newRemote(nil, &config.RemoteConfig{ Name: "foo", diff --git a/repository.go b/repository.go index 230b3e5..fd6f6d1 100644 --- a/repository.go +++ b/repository.go @@ -625,9 +625,9 @@ func (r *Repository) calculateRemoteHeadReference(spec []config.RefSpec, return refs } -func updateReferenceStorerIfNeeded( - s storer.ReferenceStorer, r *plumbing.Reference) (updated bool, err error) { - +func checkAndUpdateReferenceStorerIfNeeded( + s storer.ReferenceStorer, r, old *plumbing.Reference) ( + updated bool, err error) { p, err := s.Reference(r.Name()) if err != nil && err != plumbing.ErrReferenceNotFound { return false, err @@ -635,7 +635,7 @@ func updateReferenceStorerIfNeeded( // we use the string method to compare references, is the easiest way if err == plumbing.ErrReferenceNotFound || r.String() != p.String() { - if err := s.SetReference(r); err != nil { + if err := s.CheckAndSetReference(r, old); err != nil { return false, err } @@ -645,6 +645,11 @@ func updateReferenceStorerIfNeeded( return false, nil } +func updateReferenceStorerIfNeeded( + s storer.ReferenceStorer, r *plumbing.Reference) (updated bool, err error) { + return checkAndUpdateReferenceStorerIfNeeded(s, r, nil) +} + // Fetch fetches references along with the objects necessary to complete // their histories, from the remote named as FetchOptions.RemoteName. // diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go index 8e5381c..78aa9a2 100644 --- a/storage/filesystem/internal/dotgit/dotgit.go +++ b/storage/filesystem/internal/dotgit/dotgit.go @@ -5,6 +5,7 @@ import ( "bufio" "errors" "fmt" + "io" stdioutil "io/ioutil" "os" "strings" @@ -239,7 +240,39 @@ func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { return d.fs.Open(file) } -func (d *DotGit) SetRef(r *plumbing.Reference) error { +func (d *DotGit) readReferenceFrom(rd io.Reader, name string) (ref *plumbing.Reference, err error) { + b, err := stdioutil.ReadAll(rd) + if err != nil { + return nil, err + } + + line := strings.TrimSpace(string(b)) + return plumbing.NewReferenceFromStrings(name, line), nil +} + +func (d *DotGit) checkReferenceAndTruncate(f billy.File, old *plumbing.Reference) error { + if old == nil { + return nil + } + ref, err := d.readReferenceFrom(f, old.Name().String()) + if err != nil { + return err + } + if ref.Hash() != old.Hash() { + return fmt.Errorf("reference has changed concurrently") + } + _, err = f.Seek(0, io.SeekStart) + if err != nil { + return err + } + err = f.Truncate(0) + if err != nil { + return err + } + return nil +} + +func (d *DotGit) SetRef(r, old *plumbing.Reference) error { var content string switch r.Type() { case plumbing.SymbolicReference: @@ -248,13 +281,34 @@ func (d *DotGit) SetRef(r *plumbing.Reference) error { content = fmt.Sprintln(r.Hash().String()) } - f, err := d.fs.Create(r.Name().String()) + // If we are not checking an old ref, just truncate the file. + mode := os.O_RDWR | os.O_CREATE + if old == nil { + mode |= os.O_TRUNC + } + + f, err := d.fs.OpenFile(r.Name().String(), mode, 0666) if err != nil { return err } defer ioutil.CheckClose(f, &err) + // Lock is unlocked by the deferred Close above. This is because Unlock + // does not imply a fsync and thus there would be a race between + // Unlock+Close and other concurrent writers. Adding Sync to go-billy + // could work, but this is better (and avoids superfluous syncs). + err = f.Lock() + if err != nil { + return err + } + + // this is a no-op to call even when old is nil. + err = d.checkReferenceAndTruncate(f, old) + if err != nil { + return err + } + _, err = f.Write([]byte(content)) return err } @@ -523,13 +577,7 @@ func (d *DotGit) readReferenceFile(path, name string) (ref *plumbing.Reference, } defer ioutil.CheckClose(f, &err) - b, err := stdioutil.ReadAll(f) - if err != nil { - return nil, err - } - - line := strings.TrimSpace(string(b)) - return plumbing.NewReferenceFromStrings(name, line), nil + return d.readReferenceFrom(f, name) } // Module return a billy.Filesystem poiting to the module folder diff --git a/storage/filesystem/internal/dotgit/dotgit_test.go b/storage/filesystem/internal/dotgit/dotgit_test.go index a775536..446a204 100644 --- a/storage/filesystem/internal/dotgit/dotgit_test.go +++ b/storage/filesystem/internal/dotgit/dotgit_test.go @@ -55,24 +55,25 @@ func (s *SuiteDotGit) TestSetRefs(c *C) { fs := osfs.New(tmp) dir := New(fs) - err = dir.SetRef(plumbing.NewReferenceFromStrings( + firstFoo := plumbing.NewReferenceFromStrings( "refs/heads/foo", "e8d3ffab552895c19b9fcf7aa264d277cde33881", - )) + ) + err = dir.SetRef(firstFoo, nil) c.Assert(err, IsNil) err = dir.SetRef(plumbing.NewReferenceFromStrings( "refs/heads/symbolic", "ref: refs/heads/foo", - )) + ), nil) c.Assert(err, IsNil) err = dir.SetRef(plumbing.NewReferenceFromStrings( "bar", "e8d3ffab552895c19b9fcf7aa264d277cde33881", - )) + ), nil) c.Assert(err, IsNil) refs, err := dir.Refs() @@ -105,6 +106,20 @@ func (s *SuiteDotGit) TestSetRefs(c *C) { c.Assert(ref, NotNil) c.Assert(ref.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881") + // Check that SetRef with a non-nil `old` works. + err = dir.SetRef(plumbing.NewReferenceFromStrings( + "refs/heads/foo", + "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", + ), firstFoo) + c.Assert(err, IsNil) + + // `firstFoo` is no longer the right `old` reference, so this + // should fail. + err = dir.SetRef(plumbing.NewReferenceFromStrings( + "refs/heads/foo", + "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", + ), firstFoo) + c.Assert(err, NotNil) } func (s *SuiteDotGit) TestRefsFromPackedRefs(c *C) { @@ -192,7 +207,7 @@ func (s *SuiteDotGit) TestRemoveRefFromReferenceFileAndPackedRefs(c *C) { err := dir.SetRef(plumbing.NewReferenceFromStrings( "refs/remotes/origin/branch", "e8d3ffab552895c19b9fcf7aa264d277cde33881", - )) + ), nil) // Make sure it only appears once in the refs list. refs, err := dir.Refs() diff --git a/storage/filesystem/reference.go b/storage/filesystem/reference.go index 49627d3..54cdf56 100644 --- a/storage/filesystem/reference.go +++ b/storage/filesystem/reference.go @@ -11,7 +11,11 @@ type ReferenceStorage struct { } func (r *ReferenceStorage) SetReference(ref *plumbing.Reference) error { - return r.dir.SetRef(ref) + return r.dir.SetRef(ref, nil) +} + +func (r *ReferenceStorage) CheckAndSetReference(ref, old *plumbing.Reference) error { + return r.dir.SetRef(ref, old) } func (r *ReferenceStorage) Reference(n plumbing.ReferenceName) (*plumbing.Reference, error) { diff --git a/storage/memory/storage.go b/storage/memory/storage.go index 2380fed..c9306ee 100644 --- a/storage/memory/storage.go +++ b/storage/memory/storage.go @@ -12,6 +12,7 @@ import ( ) var ErrUnsupportedObjectType = fmt.Errorf("unsupported object type") +var ErrRefHasChanged = fmt.Errorf("reference has changed concurrently") // Storage is an implementation of git.Storer that stores data on memory, being // ephemeral. The use of this storage should be done in controlled envoriments, @@ -202,6 +203,21 @@ func (r ReferenceStorage) SetReference(ref *plumbing.Reference) error { return nil } +func (r ReferenceStorage) CheckAndSetReference(ref, old *plumbing.Reference) error { + if ref == nil { + return nil + } + + if old != nil { + tmp := r[ref.Name()] + if tmp != nil && tmp.Hash() != old.Hash() { + return ErrRefHasChanged + } + } + r[ref.Name()] = ref + return nil +} + func (r ReferenceStorage) Reference(n plumbing.ReferenceName) (*plumbing.Reference, error) { ref, ok := r[n] if !ok { diff --git a/worktree.go b/worktree.go index 8ababfa..67d7f08 100644 --- a/worktree.go +++ b/worktree.go @@ -69,6 +69,7 @@ func (w *Worktree) PullContext(ctx context.Context, o *PullOptions) error { Depth: o.Depth, Auth: o.Auth, Progress: o.Progress, + Force: o.Force, }) updated := true |