Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Commit ab6f224

Browse files
authored
Merge pull request #292 from ajnavarro/improvement/revlist
plumbing/revlist: input as a slice of hashes instead of commits
2 parents 0e9dea1 + 9a469de commit ab6f224

File tree

5 files changed

+144
-98
lines changed

5 files changed

+144
-98
lines changed

plumbing/revlist/revlist.go

Lines changed: 67 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package revlist
44

55
import (
6+
"fmt"
67
"io"
78

89
"srcd.works/go-git.v4/plumbing"
@@ -11,86 +12,103 @@ import (
1112
)
1213

1314
// Objects applies a complementary set. It gets all the hashes from all
14-
// the reachable objects from the given commits. Ignore param are object hashes
15-
// that we want to ignore on the result. It is a list because is
16-
// easier to interact with other porcelain elements, but internally it is
17-
// converted to a map. All that objects must be accessible from the object
18-
// storer.
15+
// the reachable objects from the given objects. Ignore param are object hashes
16+
// that we want to ignore on the result. All that objects must be accessible
17+
// from the object storer.
1918
func Objects(
2019
s storer.EncodedObjectStorer,
21-
commits []*object.Commit,
20+
objects []plumbing.Hash,
2221
ignore []plumbing.Hash) ([]plumbing.Hash, error) {
2322

2423
seen := hashListToSet(ignore)
2524
result := make(map[plumbing.Hash]bool)
26-
for _, c := range commits {
27-
err := reachableObjects(s, c, seen, func(h plumbing.Hash) error {
28-
if !seen[h] {
29-
result[h] = true
30-
seen[h] = true
31-
}
3225

33-
return nil
34-
})
26+
walkerFunc := func(h plumbing.Hash) {
27+
if !seen[h] {
28+
result[h] = true
29+
seen[h] = true
30+
}
31+
}
3532

36-
if err != nil {
33+
for _, h := range objects {
34+
if err := processObject(s, h, seen, walkerFunc); err != nil {
3735
return nil, err
3836
}
3937
}
4038

4139
return hashSetToList(result), nil
4240
}
4341

42+
// processObject obtains the object using the hash an process it depending of its type
43+
func processObject(
44+
s storer.EncodedObjectStorer,
45+
h plumbing.Hash,
46+
seen map[plumbing.Hash]bool,
47+
walkerFunc func(h plumbing.Hash),
48+
) error {
49+
o, err := s.EncodedObject(plumbing.AnyObject, h)
50+
if err != nil {
51+
return err
52+
}
53+
54+
do, err := object.DecodeObject(s, o)
55+
if err != nil {
56+
return err
57+
}
58+
59+
switch do := do.(type) {
60+
case *object.Commit:
61+
return reachableObjects(do, seen, walkerFunc)
62+
case *object.Tree:
63+
return iterateCommitTrees(seen, do, walkerFunc)
64+
case *object.Tag:
65+
walkerFunc(do.Hash)
66+
return processObject(s, do.Target, seen, walkerFunc)
67+
case *object.Blob:
68+
walkerFunc(do.Hash)
69+
default:
70+
return fmt.Errorf("object type not valid: %s. "+
71+
"Object reference: %s", o.Type(), o.Hash())
72+
}
73+
74+
return nil
75+
}
76+
4477
// reachableObjects returns, using the callback function, all the reachable
4578
// objects from the specified commit. To avoid to iterate over seen commits,
4679
// if a commit hash is into the 'seen' set, we will not iterate all his trees
4780
// and blobs objects.
4881
func reachableObjects(
49-
s storer.EncodedObjectStorer,
5082
commit *object.Commit,
5183
seen map[plumbing.Hash]bool,
52-
cb func(h plumbing.Hash) error) error {
53-
54-
return iterateCommits(commit, func(commit *object.Commit) error {
84+
cb func(h plumbing.Hash)) error {
85+
return object.WalkCommitHistory(commit, func(commit *object.Commit) error {
5586
if seen[commit.Hash] {
5687
return nil
5788
}
5889

59-
if err := cb(commit.Hash); err != nil {
90+
cb(commit.Hash)
91+
92+
tree, err := commit.Tree()
93+
if err != nil {
6094
return err
6195
}
6296

63-
return iterateCommitTrees(s, commit, func(h plumbing.Hash) error {
64-
return cb(h)
65-
})
66-
})
67-
}
68-
69-
// iterateCommits iterate all reachable commits from the given one
70-
func iterateCommits(commit *object.Commit, cb func(c *object.Commit) error) error {
71-
if err := cb(commit); err != nil {
72-
return err
73-
}
74-
75-
return object.WalkCommitHistory(commit, func(c *object.Commit) error {
76-
return cb(c)
97+
return iterateCommitTrees(seen, tree, cb)
7798
})
7899
}
79100

80101
// iterateCommitTrees iterate all reachable trees from the given commit
81102
func iterateCommitTrees(
82-
s storer.EncodedObjectStorer,
83-
commit *object.Commit,
84-
cb func(h plumbing.Hash) error) error {
85-
86-
tree, err := commit.Tree()
87-
if err != nil {
88-
return err
89-
}
90-
if err := cb(tree.Hash); err != nil {
91-
return err
103+
seen map[plumbing.Hash]bool,
104+
tree *object.Tree,
105+
cb func(h plumbing.Hash)) error {
106+
if seen[tree.Hash] {
107+
return nil
92108
}
93109

110+
cb(tree.Hash)
111+
94112
treeWalker := object.NewTreeWalker(tree, true)
95113

96114
for {
@@ -101,9 +119,12 @@ func iterateCommitTrees(
101119
if err != nil {
102120
return err
103121
}
104-
if err := cb(e.Hash); err != nil {
105-
return err
122+
123+
if seen[e.Hash] {
124+
continue
106125
}
126+
127+
cb(e.Hash)
107128
}
108129

109130
return nil

plumbing/revlist/revlist_test.go

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,65 @@ func (s *RevListSuite) TestRevListObjects(c *C) {
7676
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa": true, // CHANGELOG
7777
}
7878

79-
initCommit := s.commit(c, plumbing.NewHash(initialCommit))
80-
secondCommit := s.commit(c, plumbing.NewHash(secondCommit))
79+
localHist, err := Objects(s.Storer,
80+
[]plumbing.Hash{plumbing.NewHash(initialCommit)}, nil)
81+
c.Assert(err, IsNil)
82+
83+
remoteHist, err := Objects(s.Storer,
84+
[]plumbing.Hash{plumbing.NewHash(secondCommit)}, localHist)
85+
c.Assert(err, IsNil)
86+
87+
for _, h := range remoteHist {
88+
c.Assert(revList[h.String()], Equals, true)
89+
}
90+
c.Assert(len(remoteHist), Equals, len(revList))
91+
}
92+
93+
func (s *RevListSuite) TestRevListObjectsTagObject(c *C) {
94+
sto, err := filesystem.NewStorage(
95+
fixtures.ByTag("tags").
96+
ByURL("https://github.com/git-fixtures/tags.git").One().DotGit())
97+
c.Assert(err, IsNil)
98+
99+
expected := map[string]bool{
100+
"70846e9a10ef7b41064b40f07713d5b8b9a8fc73": true,
101+
"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391": true,
102+
"ad7897c0fb8e7d9a9ba41fa66072cf06095a6cfc": true,
103+
"f7b877701fbf855b44c0a9e86f3fdce2c298b07f": true,
104+
}
105+
106+
hist, err := Objects(sto, []plumbing.Hash{plumbing.NewHash("ad7897c0fb8e7d9a9ba41fa66072cf06095a6cfc")}, nil)
107+
c.Assert(err, IsNil)
108+
109+
for _, h := range hist {
110+
c.Assert(expected[h.String()], Equals, true)
111+
}
112+
113+
c.Assert(len(hist), Equals, len(expected))
114+
}
115+
116+
// ---
117+
// | |\
118+
// | | * b8e471f Creating changelog
119+
// | |/
120+
// * | 35e8510 binary file
121+
// |/
122+
// * b029517 Initial commit
123+
func (s *RevListSuite) TestRevListObjectsWithBlobsAndTrees(c *C) {
124+
revList := map[string]bool{
125+
"b8e471f58bcbca63b07bda20e428190409c2db47": true, // second commit
126+
}
81127

82-
localHist, err := Objects(s.Storer, []*object.Commit{initCommit}, nil)
128+
localHist, err := Objects(s.Storer,
129+
[]plumbing.Hash{
130+
plumbing.NewHash(initialCommit),
131+
plumbing.NewHash("c2d30fa8ef288618f65f6eed6e168e0d514886f4"),
132+
plumbing.NewHash("d3ff53e0564a9f87d8e84b6e28e5060e517008aa"),
133+
}, nil)
83134
c.Assert(err, IsNil)
84135

85-
remoteHist, err := Objects(s.Storer, []*object.Commit{secondCommit}, localHist)
136+
remoteHist, err := Objects(s.Storer,
137+
[]plumbing.Hash{plumbing.NewHash(secondCommit)}, localHist)
86138
c.Assert(err, IsNil)
87139

88140
for _, h := range remoteHist {
@@ -92,25 +144,25 @@ func (s *RevListSuite) TestRevListObjects(c *C) {
92144
}
93145

94146
func (s *RevListSuite) TestRevListObjectsReverse(c *C) {
95-
initCommit := s.commit(c, plumbing.NewHash(initialCommit))
96-
secondCommit := s.commit(c, plumbing.NewHash(secondCommit))
97147

98-
localHist, err := Objects(s.Storer, []*object.Commit{secondCommit}, nil)
148+
localHist, err := Objects(s.Storer,
149+
[]plumbing.Hash{plumbing.NewHash(secondCommit)}, nil)
99150
c.Assert(err, IsNil)
100151

101-
remoteHist, err := Objects(s.Storer, []*object.Commit{initCommit}, localHist)
152+
remoteHist, err := Objects(s.Storer,
153+
[]plumbing.Hash{plumbing.NewHash(initialCommit)}, localHist)
102154
c.Assert(err, IsNil)
103155

104156
c.Assert(len(remoteHist), Equals, 0)
105157
}
106158

107159
func (s *RevListSuite) TestRevListObjectsSameCommit(c *C) {
108-
commit := s.commit(c, plumbing.NewHash(secondCommit))
109-
110-
localHist, err := Objects(s.Storer, []*object.Commit{commit}, nil)
160+
localHist, err := Objects(s.Storer,
161+
[]plumbing.Hash{plumbing.NewHash(secondCommit)}, nil)
111162
c.Assert(err, IsNil)
112163

113-
remoteHist, err := Objects(s.Storer, []*object.Commit{commit}, localHist)
164+
remoteHist, err := Objects(s.Storer,
165+
[]plumbing.Hash{plumbing.NewHash(secondCommit)}, localHist)
114166
c.Assert(err, IsNil)
115167

116168
c.Assert(len(remoteHist), Equals, 0)
@@ -122,15 +174,14 @@ func (s *RevListSuite) TestRevListObjectsSameCommit(c *C) {
122174
// * 918c48b some code
123175
// -----
124176
func (s *RevListSuite) TestRevListObjectsNewBranch(c *C) {
125-
someCommit := s.commit(c, plumbing.NewHash(someCommit))
126-
someCommitBranch := s.commit(c, plumbing.NewHash(someCommitBranch))
127-
someCommitOtherBranch := s.commit(c, plumbing.NewHash(someCommitOtherBranch))
128-
129-
localHist, err := Objects(s.Storer, []*object.Commit{someCommit}, nil)
177+
localHist, err := Objects(s.Storer,
178+
[]plumbing.Hash{plumbing.NewHash(someCommit)}, nil)
130179
c.Assert(err, IsNil)
131180

132181
remoteHist, err := Objects(
133-
s.Storer, []*object.Commit{someCommitBranch, someCommitOtherBranch}, localHist)
182+
s.Storer, []plumbing.Hash{
183+
plumbing.NewHash(someCommitBranch),
184+
plumbing.NewHash(someCommitOtherBranch)}, localHist)
134185
c.Assert(err, IsNil)
135186

136187
revList := map[string]bool{

plumbing/transport/server/server.go

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"srcd.works/go-git.v4/plumbing"
1111
"srcd.works/go-git.v4/plumbing/format/packfile"
12-
"srcd.works/go-git.v4/plumbing/object"
1312
"srcd.works/go-git.v4/plumbing/protocol/packp"
1413
"srcd.works/go-git.v4/plumbing/protocol/packp/capability"
1514
"srcd.works/go-git.v4/plumbing/revlist"
@@ -153,26 +152,12 @@ func (s *upSession) UploadPack(req *packp.UploadPackRequest) (*packp.UploadPackR
153152
}
154153

155154
func (s *upSession) objectsToUpload(req *packp.UploadPackRequest) ([]plumbing.Hash, error) {
156-
commits, err := s.commitsToUpload(req.Wants)
155+
haves, err := revlist.Objects(s.storer, req.Haves, nil)
157156
if err != nil {
158157
return nil, err
159158
}
160159

161-
return revlist.Objects(s.storer, commits, req.Haves)
162-
}
163-
164-
func (s *upSession) commitsToUpload(wants []plumbing.Hash) ([]*object.Commit, error) {
165-
var commits []*object.Commit
166-
for _, h := range wants {
167-
c, err := object.GetCommit(s.storer, h)
168-
if err != nil {
169-
return nil, err
170-
}
171-
172-
commits = append(commits, c)
173-
}
174-
175-
return commits, nil
160+
return revlist.Objects(s.storer, req.Wants, haves)
176161
}
177162

178163
func (*upSession) setSupportedCapabilities(c *capability.List) error {

plumbing/transport/server/upload_pack_test.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,3 @@ func (s *UploadPackSuite) TestAdvertisedReferencesNotExists(c *C) {
3838
c.Assert(err, Equals, transport.ErrRepositoryNotFound)
3939
c.Assert(r, IsNil)
4040
}
41-
42-
// TODO revList implementation is returning more objects than expected.
43-
func (s *UploadPackSuite) TestUploadPackPartial(c *C) {
44-
c.Skip("Fix revList implementation")
45-
}

remote.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"srcd.works/go-git.v4/config"
99
"srcd.works/go-git.v4/plumbing"
1010
"srcd.works/go-git.v4/plumbing/format/packfile"
11-
"srcd.works/go-git.v4/plumbing/object"
1211
"srcd.works/go-git.v4/plumbing/protocol/packp"
1312
"srcd.works/go-git.v4/plumbing/protocol/packp/capability"
1413
"srcd.works/go-git.v4/plumbing/protocol/packp/sideband"
@@ -97,7 +96,7 @@ func (r *Remote) Push(o *PushOptions) (err error) {
9796
return NoErrAlreadyUpToDate
9897
}
9998

100-
commits, err := commitsToPush(r.s, req.Commands)
99+
objects, err := objectsToPush(req.Commands)
101100
if err != nil {
102101
return err
103102
}
@@ -107,7 +106,7 @@ func (r *Remote) Push(o *PushOptions) (err error) {
107106
return err
108107
}
109108

110-
hashesToPush, err := revlist.Objects(r.s, commits, haves)
109+
hashesToPush, err := revlist.Objects(r.s, objects, haves)
111110
if err != nil {
112111
return err
113112
}
@@ -476,22 +475,17 @@ func (r *Remote) buildFetchedTags(refs storer.ReferenceStorer) error {
476475
})
477476
}
478477

479-
func commitsToPush(s storer.EncodedObjectStorer, commands []*packp.Command) ([]*object.Commit, error) {
480-
var commits []*object.Commit
478+
func objectsToPush(commands []*packp.Command) ([]plumbing.Hash, error) {
479+
var objects []plumbing.Hash
481480
for _, cmd := range commands {
482481
if cmd.New == plumbing.ZeroHash {
483482
continue
484483
}
485484

486-
c, err := object.GetCommit(s, cmd.New)
487-
if err != nil {
488-
return nil, err
489-
}
490-
491-
commits = append(commits, c)
485+
objects = append(objects, cmd.New)
492486
}
493487

494-
return commits, nil
488+
return objects, nil
495489
}
496490

497491
func referencesToHashes(refs storer.ReferenceStorer) ([]plumbing.Hash, error) {

0 commit comments

Comments
 (0)