diff options
-rw-r--r-- | plumbing/reference.go | 36 | ||||
-rw-r--r-- | plumbing/reference_test.go | 5 | ||||
-rw-r--r-- | plumbing/transport/file/client.go | 18 | ||||
-rw-r--r-- | plumbing/transport/internal/common/common.go | 34 | ||||
-rw-r--r-- | remote.go | 60 | ||||
-rw-r--r-- | remote_test.go | 47 |
6 files changed, 149 insertions, 51 deletions
diff --git a/plumbing/reference.go b/plumbing/reference.go index 8fa103e..5d477b9 100644 --- a/plumbing/reference.go +++ b/plumbing/reference.go @@ -15,15 +15,16 @@ const ( symrefPrefix = "ref: " ) -var ( - refPrefixes = []string{ - refHeadPrefix, - refTagPrefix, - refRemotePrefix, - refNotePrefix, - refPrefix, - } -) +// refRevParseRules are a set of rules to parse references into short names. +// These are the same rules as used by git in shorten_unambiguous_ref. +// See: https://github.com/git/git/blob/e0aaa1b6532cfce93d87af9bc813fb2e7a7ce9d7/refs.c#L417 +var refRevParseRules = []string{ + "refs/%s", + "refs/tags/%s", + "refs/heads/%s", + "refs/remotes/%s", + "refs/remotes/%s/HEAD", +} var ( ErrReferenceNotFound = errors.New("reference not found") @@ -60,17 +61,16 @@ func (r ReferenceName) String() string { // Short returns the short name of a ReferenceName func (r ReferenceName) Short() string { - return r.removeRefPrefix() -} - -// Instead of hardcoding a number of components, we should remove the prefixes -// refHeadPrefix, refTagPrefix, refRemotePrefix, refNotePrefix and refPrefix -func (r ReferenceName) removeRefPrefix() string { s := string(r) - for _, prefix := range refPrefixes { - s = strings.TrimPrefix(s, prefix) + res := s + for _, format := range refRevParseRules { + _, err := fmt.Sscanf(s, format, &res) + if err == nil { + continue + } } - return s + + return res } const ( diff --git a/plumbing/reference_test.go b/plumbing/reference_test.go index 6a695f4..97c8772 100644 --- a/plumbing/reference_test.go +++ b/plumbing/reference_test.go @@ -23,6 +23,11 @@ func (s *ReferenceSuite) TestReferenceNameWithSlash(c *C) { c.Assert(r.Short(), Equals, "origin/feature/AllowSlashes") } +func (s *ReferenceSuite) TestReferenceNameNote(c *C) { + r := ReferenceName("refs/notes/foo") + c.Assert(r.Short(), Equals, "notes/foo") +} + func (s *ReferenceSuite) TestNewReferenceFromStrings(c *C) { r := NewReferenceFromStrings("refs/heads/v4", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5") c.Assert(r.Type(), Equals, HashReference) diff --git a/plumbing/transport/file/client.go b/plumbing/transport/file/client.go index a199b01..b6d60c1 100644 --- a/plumbing/transport/file/client.go +++ b/plumbing/transport/file/client.go @@ -46,8 +46,9 @@ func (r *runner) Command(cmd string, ep transport.Endpoint, auth transport.AuthM } type command struct { - cmd *exec.Cmd - closed bool + cmd *exec.Cmd + stderrCloser io.Closer + closed bool } func (c *command) Start() error { @@ -55,7 +56,12 @@ func (c *command) Start() error { } func (c *command) StderrPipe() (io.Reader, error) { - return c.cmd.StderrPipe() + // Pipe returned by Command.StderrPipe has a race with Read + Command.Wait. + // We use an io.Pipe and close it after the command finishes. + r, w := io.Pipe() + c.cmd.Stderr = w + c.stderrCloser = r + return r, nil } func (c *command) StdinPipe() (io.WriteCloser, error) { @@ -72,7 +78,11 @@ func (c *command) Close() error { return nil } - defer func() { c.closed = true }() + defer func() { + c.closed = true + _ = c.stderrCloser.Close() + }() + err := c.cmd.Wait() if _, ok := err.(*os.PathError); ok { return nil diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go index c1e1518..04db770 100644 --- a/plumbing/transport/internal/common/common.go +++ b/plumbing/transport/internal/common/common.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + stdioutil "io/ioutil" "strings" "time" @@ -22,7 +23,6 @@ import ( const ( readErrorSecondsTimeout = 10 - errLinesBuffer = 1000 ) var ( @@ -96,7 +96,7 @@ type session struct { advRefs *packp.AdvRefs packRun bool finished bool - errLines chan string + firstErrLine chan string } func (c *client) newSession(s string, ep transport.Endpoint, auth transport.AuthMethod) (*session, error) { @@ -128,26 +128,29 @@ func (c *client) newSession(s string, ep transport.Endpoint, auth transport.Auth Stdin: stdin, Stdout: stdout, Command: cmd, - errLines: c.listenErrors(stderr), + firstErrLine: c.listenFirstError(stderr), isReceivePack: s == transport.ReceivePackServiceName, }, nil } -func (c *client) listenErrors(r io.Reader) chan string { +func (c *client) listenFirstError(r io.Reader) chan string { if r == nil { return nil } - errLines := make(chan string, errLinesBuffer) + errLine := make(chan string, 1) go func() { s := bufio.NewScanner(r) - for s.Scan() { - line := string(s.Bytes()) - errLines <- line + if s.Scan() { + errLine <- s.Text() + } else { + close(errLine) } + + _, _ = io.Copy(stdioutil.Discard, r) }() - return errLines + return errLine } // AdvertisedReferences retrieves the advertised references from the server. @@ -296,13 +299,10 @@ func (s *session) finish() error { return nil } -func (s *session) Close() error { - if err := s.finish(); err != nil { - _ = s.Command.Close() - return err - } - - return s.Command.Close() +func (s *session) Close() (err error) { + defer ioutil.CheckClose(s.Command, &err) + err = s.finish() + return } func (s *session) checkNotFoundError() error { @@ -312,7 +312,7 @@ func (s *session) checkNotFoundError() error { select { case <-t.C: return ErrTimeoutExceeded - case line, ok := <-s.errLines: + case line, ok := <-s.firstErrLine: if !ok { return nil } @@ -238,24 +238,60 @@ func (r *Remote) addReferencesToUpdate(refspecs []config.RefSpec, req *packp.ReferenceUpdateRequest) error { for _, rs := range refspecs { - iter, err := r.s.IterReferences() - if err != nil { - return err - } - - err = iter.ForEach(func(ref *plumbing.Reference) error { - return r.addReferenceIfRefSpecMatches( - rs, remoteRefs, ref, req, - ) - }) - if err != nil { - return err + if rs.IsDelete() { + if err := r.deleteReferences(rs, remoteRefs, req); err != nil { + return err + } + } else { + if err := r.addOrUpdateReferences(rs, remoteRefs, req); err != nil { + return err + } } } return nil } +func (r *Remote) addOrUpdateReferences(rs config.RefSpec, + remoteRefs storer.ReferenceStorer, req *packp.ReferenceUpdateRequest) error { + iter, err := r.s.IterReferences() + if err != nil { + return err + } + + return iter.ForEach(func(ref *plumbing.Reference) error { + return r.addReferenceIfRefSpecMatches( + rs, remoteRefs, ref, req, + ) + }) +} + +func (r *Remote) deleteReferences(rs config.RefSpec, + remoteRefs storer.ReferenceStorer, req *packp.ReferenceUpdateRequest) error { + iter, err := remoteRefs.IterReferences() + if err != nil { + return err + } + + return iter.ForEach(func(ref *plumbing.Reference) error { + if ref.Type() != plumbing.HashReference { + return nil + } + + if rs.Dst("") != ref.Name() { + return nil + } + + cmd := &packp.Command{ + Name: ref.Name(), + Old: ref.Hash(), + New: plumbing.ZeroHash, + } + req.Commands = append(req.Commands, cmd) + return nil + }) +} + func (r *Remote) addReferenceIfRefSpecMatches(rs config.RefSpec, remoteRefs storer.ReferenceStorer, localRef *plumbing.Reference, req *packp.ReferenceUpdateRequest) error { diff --git a/remote_test.go b/remote_test.go index 4297b81..c12d5d5 100644 --- a/remote_test.go +++ b/remote_test.go @@ -5,6 +5,8 @@ import ( "io" "io/ioutil" "os" + "path/filepath" + "strings" "github.com/src-d/go-git-fixtures" "gopkg.in/src-d/go-git.v4/config" @@ -334,6 +336,32 @@ func (s *RemoteSuite) TestPushNoErrAlreadyUpToDate(c *C) { c.Assert(err, Equals, NoErrAlreadyUpToDate) } +func (s *RemoteSuite) TestPushDeleteReference(c *C) { + f := fixtures.Basic().One() + sto, err := filesystem.NewStorage(f.DotGit()) + c.Assert(err, IsNil) + + dstFs := f.DotGit() + dstSto, err := filesystem.NewStorage(dstFs) + c.Assert(err, IsNil) + prepareRepo(c, dstFs.Root()) + + url := dstFs.Root() + r := newRemote(sto, &config.RemoteConfig{ + Name: DefaultRemoteName, + URL: url, + }) + + rs := config.RefSpec(":refs/heads/branch") + err = r.Push(&PushOptions{ + RefSpecs: []config.RefSpec{rs}, + }) + c.Assert(err, IsNil) + + _, err = dstSto.Reference(plumbing.ReferenceName("refs/heads/branch")) + c.Assert(err, Equals, plumbing.ErrReferenceNotFound) +} + func (s *RemoteSuite) TestPushRejectNonFastForward(c *C) { f := fixtures.Basic().One() sto, err := filesystem.NewStorage(f.DotGit()) @@ -470,3 +498,22 @@ func (s *RemoteSuite) TestPushWrongRemoteName(c *C) { }) c.Assert(err, ErrorMatches, ".*remote names don't match.*") } + +const bareConfig = `[core] +repositoryformatversion = 0 +filemode = true +bare = true` + +func prepareRepo(c *C, path string) { + // git-receive-pack refuses to update refs/heads/master on non-bare repo + // so we ensure bare repo config. + config := filepath.Join(path, "config") + if _, err := os.Stat(config); err == nil { + f, err := os.OpenFile(config, os.O_TRUNC|os.O_WRONLY, 0) + c.Assert(err, IsNil) + content := strings.NewReader(bareConfig) + _, err = io.Copy(f, content) + c.Assert(err, IsNil) + c.Assert(f.Close(), IsNil) + } +} |