remote: support for non-force, fast-forward-only fetches #665
Conversation
plumbing/storer/reference.go
Outdated
| @@ -16,6 +16,7 @@ var ErrMaxResolveRecursion = errors.New("max. recursion level reached") | |||
| // ReferenceStorer is a generic storage of references. | |||
| type ReferenceStorer interface { | |||
| SetReference(*plumbing.Reference) error | |||
| CheckAndSetReference(new, old *plumbing.Reference) error | |||
There was a problem hiding this comment.
I know that the order methods are updocumented too, I belive because looks like obviuos (even with that we should document the behaviour at some point), but we should document this one now, sinse for me it`s not a obviuos the goal.
| if old == nil { | ||
| return nil | ||
| } | ||
| ref, err := d.readReferenceFrom(f, old.Name().String()) |
There was a problem hiding this comment.
This function is not covered by the test.
There was a problem hiding this comment.
readReferenceFrom is definitely covered by the tests, since it's called by readReferenceFile below. But if you're referring to checkReferenceAndTruncate, sure I'll add a test for it.
storage/memory/storage.go
Outdated
| @@ -202,6 +203,20 @@ func (r ReferenceStorage) SetReference(ref *plumbing.Reference) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (r ReferenceStorage) CheckAndSetReference(ref, old *plumbing.Reference) error { | |||
| if ref != nil { | |||
There was a problem hiding this comment.
Can be change this to:
if ref == nil {
return nil
}
| @@ -202,6 +203,20 @@ func (r ReferenceStorage) SetReference(ref *plumbing.Reference) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (r ReferenceStorage) CheckAndSetReference(ref, old *plumbing.Reference) error { | |||
There was a problem hiding this comment.
This function is not covered by the tests.
There was a problem hiding this comment.
Ah, I didn't notice that the remote tests don't use filesystem storage. I'll add a test.
|
@mcuadros I've added more tests and addressed your comments. Please take another look, thanks! |
| // 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 |
There was a problem hiding this comment.
Maybe it is a good time to add doc comments to the other methods in this interface?
This PR adds support for a missing "+" in a fetch refspec, which should indicate a fast-forward-only fetch. (Alternatively, the caller could put
Force=trueinto theFetchOptionsto indicate that all fetches should be treated as forces.)It relies on file-level locking provided by the storage layer, so that distributed storage layers (being accessed concurrently by multiple independent nodes each running go-git) can be supported.
Code mostly written by @taruti, based on an early version by @lupine in #490.