From 05c414a169a75ca933402e5be19a5c4304aa4f00 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Mon, 26 Mar 2018 23:18:54 +0200 Subject: *: Use CheckClose with named returns Previously some close errors were losts. This is specially problematic in go-git as lots of work is done here like generating indexes and moving packfiles. Signed-off-by: Javi Fontan --- plumbing/format/packfile/common.go | 3 +-- plumbing/format/packfile/scanner.go | 5 +++-- plumbing/object/blob.go | 2 +- plumbing/object/commit.go | 2 +- plumbing/object/file.go | 2 +- plumbing/object/tag.go | 5 +++-- plumbing/object/tree.go | 4 ++-- plumbing/transport/http/common.go | 6 +++--- remote.go | 12 ++++++------ repository.go | 6 ++---- storage/filesystem/config.go | 8 ++++---- storage/filesystem/index.go | 4 ++-- storage/filesystem/internal/dotgit/dotgit.go | 8 ++++---- storage/filesystem/internal/dotgit/dotgit_setref.go | 2 +- storage/filesystem/object.go | 8 ++++---- 15 files changed, 38 insertions(+), 39 deletions(-) diff --git a/plumbing/format/packfile/common.go b/plumbing/format/packfile/common.go index 7dad1f6..beb015d 100644 --- a/plumbing/format/packfile/common.go +++ b/plumbing/format/packfile/common.go @@ -40,8 +40,7 @@ func UpdateObjectStorage(s storer.EncodedObjectStorer, packfile io.Reader) error return err } -func writePackfileToObjectStorage(sw storer.PackfileWriter, packfile io.Reader) error { - var err error +func writePackfileToObjectStorage(sw storer.PackfileWriter, packfile io.Reader) (err error) { w, err := sw.PackfileWriter() if err != nil { return err diff --git a/plumbing/format/packfile/scanner.go b/plumbing/format/packfile/scanner.go index 8c216f1..6fc183b 100644 --- a/plumbing/format/packfile/scanner.go +++ b/plumbing/format/packfile/scanner.go @@ -279,14 +279,15 @@ func (s *Scanner) NextObject(w io.Writer) (written int64, crc32 uint32, err erro // from it zlib stream in an object entry in the packfile. func (s *Scanner) copyObject(w io.Writer) (n int64, err error) { if s.zr == nil { - zr, err := zlib.NewReader(s.r) + var zr io.ReadCloser + zr, err = zlib.NewReader(s.r) if err != nil { return 0, fmt.Errorf("zlib initialization error: %s", err) } s.zr = zr.(readerResetter) } else { - if err := s.zr.Reset(s.r, nil); err != nil { + if err = s.zr.Reset(s.r, nil); err != nil { return 0, fmt.Errorf("zlib reset error: %s", err) } } diff --git a/plumbing/object/blob.go b/plumbing/object/blob.go index 2608477..f376baa 100644 --- a/plumbing/object/blob.go +++ b/plumbing/object/blob.go @@ -67,7 +67,7 @@ func (b *Blob) Decode(o plumbing.EncodedObject) error { } // Encode transforms a Blob into a plumbing.EncodedObject. -func (b *Blob) Encode(o plumbing.EncodedObject) error { +func (b *Blob) Encode(o plumbing.EncodedObject) (err error) { o.SetType(plumbing.BlobObject) w, err := o.Writer() diff --git a/plumbing/object/commit.go b/plumbing/object/commit.go index a317714..c9a4c0e 100644 --- a/plumbing/object/commit.go +++ b/plumbing/object/commit.go @@ -226,7 +226,7 @@ func (b *Commit) Encode(o plumbing.EncodedObject) error { return b.encode(o, true) } -func (b *Commit) encode(o plumbing.EncodedObject, includeSig bool) error { +func (b *Commit) encode(o plumbing.EncodedObject, includeSig bool) (err error) { o.SetType(plumbing.CommitObject) w, err := o.Writer() if err != nil { diff --git a/plumbing/object/file.go b/plumbing/object/file.go index 40b5206..1c5fdbb 100644 --- a/plumbing/object/file.go +++ b/plumbing/object/file.go @@ -44,7 +44,7 @@ func (f *File) Contents() (content string, err error) { } // IsBinary returns if the file is binary or not -func (f *File) IsBinary() (bool, error) { +func (f *File) IsBinary() (bin bool, err error) { reader, err := f.Reader() if err != nil { return false, err diff --git a/plumbing/object/tag.go b/plumbing/object/tag.go index 19e55cf..905206b 100644 --- a/plumbing/object/tag.go +++ b/plumbing/object/tag.go @@ -95,7 +95,8 @@ func (t *Tag) Decode(o plumbing.EncodedObject) (err error) { r := bufio.NewReader(reader) for { - line, err := r.ReadBytes('\n') + var line []byte + line, err = r.ReadBytes('\n') if err != nil && err != io.EOF { return err } @@ -168,7 +169,7 @@ func (t *Tag) Encode(o plumbing.EncodedObject) error { return t.encode(o, true) } -func (t *Tag) encode(o plumbing.EncodedObject, includeSig bool) error { +func (t *Tag) encode(o plumbing.EncodedObject, includeSig bool) (err error) { o.SetType(plumbing.TagObject) w, err := o.Writer() if err != nil { diff --git a/plumbing/object/tree.go b/plumbing/object/tree.go index 2fcd979..c2399f8 100644 --- a/plumbing/object/tree.go +++ b/plumbing/object/tree.go @@ -233,7 +233,7 @@ func (t *Tree) Decode(o plumbing.EncodedObject) (err error) { } // Encode transforms a Tree into a plumbing.EncodedObject. -func (t *Tree) Encode(o plumbing.EncodedObject) error { +func (t *Tree) Encode(o plumbing.EncodedObject) (err error) { o.SetType(plumbing.TreeObject) w, err := o.Writer() if err != nil { @@ -242,7 +242,7 @@ func (t *Tree) Encode(o plumbing.EncodedObject) error { defer ioutil.CheckClose(w, &err) for _, entry := range t.Entries { - if _, err := fmt.Fprintf(w, "%o %s", entry.Mode, entry.Name); err != nil { + if _, err = fmt.Fprintf(w, "%o %s", entry.Mode, entry.Name); err != nil { return err } diff --git a/plumbing/transport/http/common.go b/plumbing/transport/http/common.go index 24e63a4..2c337b7 100644 --- a/plumbing/transport/http/common.go +++ b/plumbing/transport/http/common.go @@ -31,7 +31,7 @@ func applyHeadersToRequest(req *http.Request, content *bytes.Buffer, host string const infoRefsPath = "/info/refs" -func advertisedReferences(s *session, serviceName string) (*packp.AdvRefs, error) { +func advertisedReferences(s *session, serviceName string) (ref *packp.AdvRefs, err error) { url := fmt.Sprintf( "%s%s?service=%s", s.endpoint.String(), infoRefsPath, serviceName, @@ -52,12 +52,12 @@ func advertisedReferences(s *session, serviceName string) (*packp.AdvRefs, error s.ModifyEndpointIfRedirect(res) defer ioutil.CheckClose(res.Body, &err) - if err := NewErr(res); err != nil { + if err = NewErr(res); err != nil { return nil, err } ar := packp.NewAdvRefs() - if err := ar.Decode(res.Body); err != nil { + if err = ar.Decode(res.Body); err != nil { if err == packp.ErrEmptyAdvRefs { err = transport.ErrEmptyRemoteRepository } diff --git a/remote.go b/remote.go index 8db645c..666c9f5 100644 --- a/remote.go +++ b/remote.go @@ -73,7 +73,7 @@ func (r *Remote) Push(o *PushOptions) error { // The provided Context must be non-nil. If the context expires before the // operation is complete, an error is returned. The context only affects to the // transport operations. -func (r *Remote) PushContext(ctx context.Context, o *PushOptions) error { +func (r *Remote) PushContext(ctx context.Context, o *PushOptions) (err error) { if err := o.Validate(); err != nil { return err } @@ -243,12 +243,12 @@ func (r *Remote) Fetch(o *FetchOptions) error { return r.FetchContext(context.Background(), o) } -func (r *Remote) fetch(ctx context.Context, o *FetchOptions) (storer.ReferenceStorer, error) { +func (r *Remote) fetch(ctx context.Context, o *FetchOptions) (sto storer.ReferenceStorer, err error) { if o.RemoteName == "" { o.RemoteName = r.c.Name } - if err := o.Validate(); err != nil { + if err = o.Validate(); err != nil { return nil, err } @@ -295,7 +295,7 @@ func (r *Remote) fetch(ctx context.Context, o *FetchOptions) (storer.ReferenceSt return nil, err } - if err := r.fetchPack(ctx, o, s, req); err != nil { + if err = r.fetchPack(ctx, o, s, req); err != nil { return nil, err } } @@ -354,7 +354,7 @@ func (r *Remote) fetchPack(ctx context.Context, o *FetchOptions, s transport.Upl defer ioutil.CheckClose(reader, &err) - if err := r.updateShallow(o, reader); err != nil { + if err = r.updateShallow(o, reader); err != nil { return err } @@ -872,7 +872,7 @@ func (r *Remote) buildFetchedTags(refs memory.ReferenceStorage) (updated bool, e } // List the references on the remote repository. -func (r *Remote) List(o *ListOptions) ([]*plumbing.Reference, error) { +func (r *Remote) List(o *ListOptions) (rfs []*plumbing.Reference, err error) { s, err := newUploadPackSession(r.c.URLs[0], o.Auth) if err != nil { return nil, err diff --git a/repository.go b/repository.go index 98558d9..4ea51f5 100644 --- a/repository.go +++ b/repository.go @@ -270,9 +270,7 @@ func dotGitToOSFilesystems(path string) (dot, wt billy.Filesystem, err error) { return dot, fs, nil } -func dotGitFileToOSFilesystem(path string, fs billy.Filesystem) (billy.Filesystem, error) { - var err error - +func dotGitFileToOSFilesystem(path string, fs billy.Filesystem) (bfs billy.Filesystem, err error) { f, err := fs.Open(".git") if err != nil { return nil, err @@ -1092,7 +1090,7 @@ func (r *Repository) createNewObjectPack(cfg *RepackConfig) (h plumbing.Hash, er if los, ok := r.Storer.(storer.LooseObjectStorer); ok { err = los.ForEachObjectHash(func(hash plumbing.Hash) error { if ow.isSeen(hash) { - err := los.DeleteLooseObject(hash) + err = los.DeleteLooseObject(hash) if err != nil { return err } diff --git a/storage/filesystem/config.go b/storage/filesystem/config.go index a2cc173..85feaf0 100644 --- a/storage/filesystem/config.go +++ b/storage/filesystem/config.go @@ -13,7 +13,7 @@ type ConfigStorage struct { dir *dotgit.DotGit } -func (c *ConfigStorage) Config() (*config.Config, error) { +func (c *ConfigStorage) Config() (conf *config.Config, err error) { cfg := config.NewConfig() f, err := c.dir.Config() @@ -32,15 +32,15 @@ func (c *ConfigStorage) Config() (*config.Config, error) { return nil, err } - if err := cfg.Unmarshal(b); err != nil { + if err = cfg.Unmarshal(b); err != nil { return nil, err } return cfg, err } -func (c *ConfigStorage) SetConfig(cfg *config.Config) error { - if err := cfg.Validate(); err != nil { +func (c *ConfigStorage) SetConfig(cfg *config.Config) (err error) { + if err = cfg.Validate(); err != nil { return err } diff --git a/storage/filesystem/index.go b/storage/filesystem/index.go index 14ab09a..092edec 100644 --- a/storage/filesystem/index.go +++ b/storage/filesystem/index.go @@ -12,7 +12,7 @@ type IndexStorage struct { dir *dotgit.DotGit } -func (s *IndexStorage) SetIndex(idx *index.Index) error { +func (s *IndexStorage) SetIndex(idx *index.Index) (err error) { f, err := s.dir.IndexWriter() if err != nil { return err @@ -25,7 +25,7 @@ func (s *IndexStorage) SetIndex(idx *index.Index) error { return err } -func (s *IndexStorage) Index() (*index.Index, error) { +func (s *IndexStorage) Index() (i *index.Index, err error) { idx := &index.Index{ Version: 2, } diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go index 027ef83..60d122f 100644 --- a/storage/filesystem/internal/dotgit/dotgit.go +++ b/storage/filesystem/internal/dotgit/dotgit.go @@ -375,7 +375,7 @@ func (d *DotGit) findPackedRefsInFile(f billy.File) ([]*plumbing.Reference, erro return refs, s.Err() } -func (d *DotGit) findPackedRefs() ([]*plumbing.Reference, error) { +func (d *DotGit) findPackedRefs() (r []*plumbing.Reference, error error) { f, err := d.fs.Open(packedRefsPath) if err != nil { if os.IsNotExist(err) { @@ -676,7 +676,7 @@ func (d *DotGit) PackRefs() (err error) { // Gather all refs using addRefsFromRefDir and addRefsFromPackedRefs. var refs []*plumbing.Reference seen := make(map[plumbing.ReferenceName]bool) - if err := d.addRefsFromRefDir(&refs, seen); err != nil { + if err = d.addRefsFromRefDir(&refs, seen); err != nil { return err } if len(refs) == 0 { @@ -684,7 +684,7 @@ func (d *DotGit) PackRefs() (err error) { return nil } numLooseRefs := len(refs) - if err := d.addRefsFromPackedRefsFile(&refs, f, seen); err != nil { + if err = d.addRefsFromPackedRefsFile(&refs, f, seen); err != nil { return err } @@ -701,7 +701,7 @@ func (d *DotGit) PackRefs() (err error) { w := bufio.NewWriter(tmp) for _, ref := range refs { - _, err := w.WriteString(ref.String() + "\n") + _, err = w.WriteString(ref.String() + "\n") if err != nil { return err } diff --git a/storage/filesystem/internal/dotgit/dotgit_setref.go b/storage/filesystem/internal/dotgit/dotgit_setref.go index c732c9f..d27c1a3 100644 --- a/storage/filesystem/internal/dotgit/dotgit_setref.go +++ b/storage/filesystem/internal/dotgit/dotgit_setref.go @@ -9,7 +9,7 @@ import ( "gopkg.in/src-d/go-git.v4/utils/ioutil" ) -func (d *DotGit) setRef(fileName, content string, old *plumbing.Reference) error { +func (d *DotGit) setRef(fileName, content string, old *plumbing.Reference) (err error) { // If we are not checking an old ref, just truncate the file. mode := os.O_RDWR | os.O_CREATE if old == nil { diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 9f1c5ef..26190fd 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -55,7 +55,7 @@ func (s *ObjectStorage) requireIndex() error { return nil } -func (s *ObjectStorage) loadIdxFile(h plumbing.Hash) error { +func (s *ObjectStorage) loadIdxFile(h plumbing.Hash) (err error) { f, err := s.dir.ObjectPackIdx(h) if err != nil { return err @@ -94,7 +94,7 @@ func (s *ObjectStorage) PackfileWriter() (io.WriteCloser, error) { } // SetEncodedObject adds a new object to the storage. -func (s *ObjectStorage) SetEncodedObject(o plumbing.EncodedObject) (plumbing.Hash, error) { +func (s *ObjectStorage) SetEncodedObject(o plumbing.EncodedObject) (h plumbing.Hash, err error) { if o.Type() == plumbing.OFSDeltaObject || o.Type() == plumbing.REFDeltaObject { return plumbing.ZeroHash, plumbing.ErrInvalidType } @@ -113,11 +113,11 @@ func (s *ObjectStorage) SetEncodedObject(o plumbing.EncodedObject) (plumbing.Has defer ioutil.CheckClose(or, &err) - if err := ow.WriteHeader(o.Type(), o.Size()); err != nil { + if err = ow.WriteHeader(o.Type(), o.Size()); err != nil { return plumbing.ZeroHash, err } - if _, err := io.Copy(ow, or); err != nil { + if _, err = io.Copy(ow, or); err != nil { return plumbing.ZeroHash, err } -- cgit From 7a02aee903bebd1f5023793c35171953c1d0a7cf Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Tue, 27 Mar 2018 17:48:58 +0200 Subject: plumbing: transport, make target repo writeable in tests Some tests write to an already existent repository retrieved from fixtures. The permissions of these files are read only and make receive pack fail. This was shadowed before as close errors were lost. Signed-off-by: Javi Fontan --- plumbing/transport/test/receive_pack.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/plumbing/transport/test/receive_pack.go b/plumbing/transport/test/receive_pack.go index a68329e..6179850 100644 --- a/plumbing/transport/test/receive_pack.go +++ b/plumbing/transport/test/receive_pack.go @@ -8,6 +8,8 @@ import ( "context" "io" "io/ioutil" + "os" + "path/filepath" "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" @@ -225,6 +227,25 @@ func (s *ReceivePackSuite) receivePackNoCheck(c *C, ep *transport.Endpoint, ep.String(), url, callAdvertisedReferences, ) + // Set write permissions to endpoint directory files. By default + // fixtures are generated with read only permissions, this casuses + // errors deleting or modifying files. + rootPath := ep.Path + println("STAT", rootPath) + stat, err := os.Stat(ep.Path) + + if rootPath != "" && err == nil && stat.IsDir() { + objectPath := filepath.Join(rootPath, "objects/pack") + files, err := ioutil.ReadDir(objectPath) + c.Assert(err, IsNil) + + for _, file := range files { + path := filepath.Join(objectPath, file.Name()) + err = os.Chmod(path, 0644) + c.Assert(err, IsNil) + } + } + r, err := s.Client.NewReceivePackSession(ep, s.EmptyAuth) c.Assert(err, IsNil, comment) defer func() { c.Assert(r.Close(), IsNil, comment) }() -- cgit From 6e07548d9078505ca2945f09d11729b14abcc907 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Wed, 28 Mar 2018 10:59:33 +0200 Subject: storage: filesystem/dotgit, fix typo in return param Signed-off-by: Javi Fontan --- storage/filesystem/internal/dotgit/dotgit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go index 60d122f..6f0f1a5 100644 --- a/storage/filesystem/internal/dotgit/dotgit.go +++ b/storage/filesystem/internal/dotgit/dotgit.go @@ -375,7 +375,7 @@ func (d *DotGit) findPackedRefsInFile(f billy.File) ([]*plumbing.Reference, erro return refs, s.Err() } -func (d *DotGit) findPackedRefs() (r []*plumbing.Reference, error error) { +func (d *DotGit) findPackedRefs() (r []*plumbing.Reference, err error) { f, err := d.fs.Open(packedRefsPath) if err != nil { if os.IsNotExist(err) { -- cgit