From 13bc00d12979be26ae299e88f021fa517ecd7bc3 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Tue, 28 Mar 2017 23:59:06 -0400 Subject: [PATCH] Convert maybeSources to use new patterns --- maybe_source.go | 132 +++++++++++++++++++++++++++++++++--------------- source.go | 77 ++++++++++++++++++++-------- source_test.go | 44 +++++++++++----- 3 files changed, 179 insertions(+), 74 deletions(-) diff --git a/maybe_source.go b/maybe_source.go index e42fc62..765a055 100644 --- a/maybe_source.go +++ b/maybe_source.go @@ -6,6 +6,7 @@ import ( "fmt" "net/url" "path/filepath" + "strings" "github.com/Masterminds/vcs" ) @@ -20,24 +21,37 @@ import ( type maybeSource interface { // TODO(sdboyer) remove ProjectAnalyzer from here after refactor to bring it in on // GetManifestAndLock() calls as a param - try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) + //try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) + try(ctx context.Context, cachedir string, c singleSourceCache) (source, sourceState, error) + getURL() string } type maybeSources []maybeSource -func (mbs maybeSources) try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) { +func (mbs maybeSources) try(ctx context.Context, cachedir string, c singleSourceCache) (source, sourceState, error) { var e sourceFailures for _, mb := range mbs { - src, ident, err := mb.try(ctx, cachedir, c) + src, state, err := mb.try(ctx, cachedir, c) if err == nil { - return src, ident, nil + return src, state, nil } e = append(e, sourceSetupFailure{ - ident: ident, + ident: mb.getURL(), err: err, }) } - return nil, "", e + return nil, 0, e +} + +// This really isn't generally intended to be used - the interface is for +// maybeSources to be able to interrogate its members, not other things to +// interrogate a maybeSources. +func (mbs maybeSources) getURL() string { + strslice := make([]string, 0, len(mbs)) + for _, mb := range mbs { + strslice = append(strslice, mb.getURL()) + } + return strings.Join(strslice, "\n") } type sourceSetupFailure struct { @@ -65,12 +79,13 @@ type maybeGitSource struct { url *url.URL } -func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) { +func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, sourceState, error) { ustr := m.url.String() path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) + r, err := vcs.NewGitRepo(ustr, path) if err != nil { - return nil, ustr, unwrapVcsErr(err) + return nil, 0, unwrapVcsErr(err) } src := &gitSource{ @@ -82,16 +97,27 @@ func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSource }, }, } - src.baseVCSSource.lvfunc = src.listVersions - if !r.CheckLocal() { - _, err = src.listVersions() - if err != nil { - return nil, ustr, unwrapVcsErr(err) - } + + // Pinging invokes the same action as calling listVersions, so just do that. + _, err = src.listVersions() + if err != nil { + return nil, 0, fmt.Errorf("remote repository at %s does not exist, or is inaccessible", ustr) } - return src, ustr, nil + //c.storeVersionMap(vl, true) + //state := sourceIsSetUp | sourceExistsUpstream | sourceHasLatestVersionList + + state := sourceIsSetUp | sourceExistsUpstream + if r.CheckLocal() { + state |= sourceExistsLocally + } + + return src, state, nil +} + +func (m maybeGitSource) getURL() string { + return m.url.String() } type maybeGopkginSource struct { @@ -106,15 +132,16 @@ type maybeGopkginSource struct { major uint64 } -func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) { +func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, sourceState, error) { // We don't actually need a fully consistent transform into the on-disk path // - just something that's unique to the particular gopkg.in domain context. // So, it's OK to just dumb-join the scheme with the path. path := filepath.Join(cachedir, "sources", sanitizer.Replace(m.url.Scheme+"/"+m.opath)) ustr := m.url.String() + r, err := vcs.NewGitRepo(ustr, path) if err != nil { - return nil, ustr, unwrapVcsErr(err) + return nil, 0, unwrapVcsErr(err) } src := &gopkginSource{ @@ -129,40 +156,54 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo }, major: m.major, } - src.baseVCSSource.lvfunc = src.listVersions - if !r.CheckLocal() { - _, err = src.listVersions() - if err != nil { - return nil, ustr, unwrapVcsErr(err) - } + + // Pinging invokes the same action as calling listVersions, so just do that. + _, err = src.listVersions() + if err != nil { + return nil, 0, fmt.Errorf("remote repository at %s does not exist, or is inaccessible", ustr) } - return src, ustr, nil + //c.storeVersionMap(vl, true) + //state := sourceIsSetUp | sourceExistsUpstream | sourceHasLatestVersionList + + state := sourceIsSetUp | sourceExistsUpstream + if r.CheckLocal() { + state |= sourceExistsLocally + } + + return src, state, nil +} + +func (m maybeGopkginSource) getURL() string { + return m.opath } type maybeBzrSource struct { url *url.URL } -func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) { +func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, sourceState, error) { ustr := m.url.String() path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) + r, err := vcs.NewBzrRepo(ustr, path) if err != nil { - return nil, ustr, unwrapVcsErr(err) + return nil, 0, unwrapVcsErr(err) } + if !r.Ping() { - return nil, ustr, fmt.Errorf("remote repository at %s does not exist, or is inaccessible", ustr) + return nil, 0, fmt.Errorf("remote repository at %s does not exist, or is inaccessible", ustr) + } + + state := sourceIsSetUp | sourceExistsUpstream + if r.CheckLocal() { + state |= sourceExistsLocally } src := &bzrSource{ baseVCSSource: baseVCSSource{ dc: c, - ex: existence{ - s: existsUpstream, - f: existsUpstream, - }, crepo: &repo{ r: &bzrRepo{r}, rpath: path, @@ -171,31 +212,38 @@ func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSource } src.baseVCSSource.lvfunc = src.listVersions - return src, ustr, nil + return src, state, nil +} + +func (m maybeBzrSource) getURL() string { + return m.url.String() } type maybeHgSource struct { url *url.URL } -func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, string, error) { +func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceCache) (source, sourceState, error) { ustr := m.url.String() path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) + r, err := vcs.NewHgRepo(ustr, path) if err != nil { - return nil, ustr, unwrapVcsErr(err) + return nil, 0, unwrapVcsErr(err) } + if !r.Ping() { - return nil, ustr, fmt.Errorf("remote repository at %s does not exist, or is inaccessible", ustr) + return nil, 0, fmt.Errorf("remote repository at %s does not exist, or is inaccessible", ustr) + } + + state := sourceIsSetUp | sourceExistsUpstream + if r.CheckLocal() { + state |= sourceExistsLocally } src := &hgSource{ baseVCSSource: baseVCSSource{ dc: c, - ex: existence{ - s: existsUpstream, - f: existsUpstream, - }, crepo: &repo{ r: &hgRepo{r}, rpath: path, @@ -204,5 +252,9 @@ func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceC } src.baseVCSSource.lvfunc = src.listVersions - return src, ustr, nil + return src, state, nil +} + +func (m maybeHgSource) getURL() string { + return m.url.String() } diff --git a/source.go b/source.go index 875af29..281c94a 100644 --- a/source.go +++ b/source.go @@ -47,7 +47,7 @@ const ( existsUpstream ) -type sourceState uint32 +type sourceState int32 const ( sourceIsSetUp sourceState = 1 << iota @@ -101,10 +101,12 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project sc.srcmut.RLock() if url, has := sc.nameToURL[normalizedName]; has { - if srcGate, has := sc.srcs[url]; has { - sc.srcmut.RUnlock() + srcGate, has := sc.srcs[url] + sc.srcmut.RUnlock() + if has { return srcGate, nil } + panic(fmt.Sprintf("%q was URL for %q in nameToURL, but no corresponding srcGate in srcs map", url, normalizedName)) } sc.srcmut.RUnlock() @@ -130,18 +132,18 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project sc.protoSrcs[normalizedName] = []srcReturnChans{rc} sc.psrcmut.Unlock() - doReturn := func(sa *sourceGateway, err error) { + doReturn := func(sg *sourceGateway, err error) { sc.psrcmut.Lock() - if sa != nil { + if sg != nil { for _, rc := range sc.protoSrcs[normalizedName] { - rc.ret <- sa + rc.ret <- sg } } else if err != nil { for _, rc := range sc.protoSrcs[normalizedName] { rc.err <- err } } else { - panic("sa and err both nil") + panic("sg and err both nil") } delete(sc.protoSrcs, normalizedName) @@ -169,8 +171,7 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project doReturn(srcGate, nil) return } - // This should panic, right? - panic("") + panic(fmt.Sprintf("%q was URL for %q in nameToURL, but no corresponding srcGate in srcs map", url, normalizedName)) } sc.srcmut.RUnlock() @@ -220,7 +221,6 @@ type sourceGateway struct { srcState sourceState src source cache singleSourceCache - url string // TODO no nono nononononononooo get it from a call mu sync.Mutex // global lock, serializes all behaviors callMgr *callManager } @@ -421,7 +421,7 @@ func (sg *sourceGateway) sourceURL(ctx context.Context) (string, error) { return "", err } - return sg.url, nil + return sg.src.upstreamURL(), nil } // createSingleSourceCache creates a singleSourceCache instance for use by @@ -439,22 +439,29 @@ func (sg *sourceGateway) require(ctx context.Context, wanted sourceState) (errSt flag = 1 << i if todo&flag != 0 { - // Assign the currently visited bit to the errState so that we - // can return easily later. + // Assign the currently visited bit to errState so that we can + // return easily later. + // + // Also set up addlState so that individual ops can easily attach + // more states that were incidentally satisfied by the op. errState = flag + var addlState sourceState switch flag { case sourceIsSetUp: - sg.src, sg.url, err = sg.maybe.try(ctx, sg.cachedir, sg.cache) + sg.src, addlState, err = sg.maybe.try(ctx, sg.cachedir, sg.cache) case sourceExistsUpstream: - // TODO(sdboyer) doing it this way kinda muddles responsibility - if !sg.src.checkExistence(existsUpstream) { - err = fmt.Errorf("%s does not exist upstream", sg.url) + if !sg.src.existsUpstream(ctx) { + err = fmt.Errorf("%s does not exist upstream", sg.src.upstreamURL()) } case sourceExistsLocally: - // TODO(sdboyer) doing it this way kinda muddles responsibility - if !sg.src.checkExistence(existsInCache) { - err = fmt.Errorf("%s does not exist in the local cache", sg.url) + if !sg.src.existsLocally(ctx) { + err = sg.src.syncLocal() + if err == nil { + addlState |= sourceHasLatestLocally + } else { + err = fmt.Errorf("%s does not exist in the local cache and fetching failed: %s", sg.src.upstreamURL(), err) + } } case sourceHasLatestVersionList: _, err = sg.src.listVersions() @@ -466,8 +473,9 @@ func (sg *sourceGateway) require(ctx context.Context, wanted sourceState) (errSt return } - sg.srcState |= flag - todo -= flag + checked := flag | addlState + sg.srcState |= checked + todo &= ^checked } } @@ -475,6 +483,9 @@ func (sg *sourceGateway) require(ctx context.Context, wanted sourceState) (errSt } type source interface { + existsLocally(context.Context) bool + existsUpstream(context.Context) bool + upstreamURL() string syncLocal() error checkExistence(sourceExistence) bool exportVersionTo(Version, string) error @@ -484,6 +495,16 @@ type source interface { revisionPresentIn(Revision) (bool, error) } +//type source interface { +//syncLocal(context.Context) error +//checkExistence(sourceExistence) bool +//exportRevisionTo(Revision, string) error +//getManifestAndLock(ProjectRoot, Revision, ProjectAnalyzer) (Manifest, Lock, error) +//listPackages(ProjectRoot, Revision) (PackageTree, error) +//listVersions(context.Context) ([]Version, error) +//revisionPresentIn(Revision) (bool, error) +//} + // projectInfo holds manifest and lock type projectInfo struct { Manifest @@ -531,6 +552,18 @@ type baseVCSSource struct { cvsync bool } +func (bs *baseVCSSource) existsLocally(ctx context.Context) bool { + return bs.crepo.r.CheckLocal() +} + +func (bs *baseVCSSource) existsUpstream(ctx context.Context) bool { + return bs.crepo.r.Ping() +} + +func (bs *baseVCSSource) upstreamURL() string { + return bs.crepo.r.Remote() +} + func (bs *baseVCSSource) getManifestAndLock(r ProjectRoot, v Version, an ProjectAnalyzer) (Manifest, Lock, error) { if err := bs.ensureCacheExistence(); err != nil { return nil, nil, err diff --git a/source_test.go b/source_test.go index 31f31a4..bb03805 100644 --- a/source_test.go +++ b/source_test.go @@ -40,7 +40,7 @@ func TestGitSourceInteractions(t *testing.T) { url: u, } - isrc, ident, err := mb.try(context.Background(), cpath, newMemoryCache()) + isrc, state, err := mb.try(context.Background(), cpath, newMemoryCache()) if err != nil { t.Errorf("Unexpected error while setting up gitSource for test repo: %s", err) rf() @@ -52,8 +52,13 @@ func TestGitSourceInteractions(t *testing.T) { rf() t.FailNow() } - if ident != un { - t.Errorf("Expected %s as source ident, got %s", un, ident) + + wantstate := sourceIsSetUp | sourceExistsUpstream + if state != wantstate { + t.Errorf("Expected return state to be %v, got %v", wantstate, state) + } + if un != src.upstreamURL() { + t.Errorf("Expected %s as source URL, got %s", un, src.upstreamURL()) } vlist, err := src.listVersions() @@ -142,7 +147,7 @@ func TestGopkginSourceInteractions(t *testing.T) { major: major, } - isrc, ident, err := mb.try(context.Background(), cpath, newMemoryCache()) + isrc, state, err := mb.try(context.Background(), cpath, newMemoryCache()) if err != nil { t.Errorf("Unexpected error while setting up gopkginSource for test repo: %s", err) return @@ -152,8 +157,13 @@ func TestGopkginSourceInteractions(t *testing.T) { t.Errorf("Expected a gopkginSource, got a %T", isrc) return } - if ident != un { - t.Errorf("Expected %s as source ident, got %s", un, ident) + + wantstate := sourceIsSetUp | sourceExistsUpstream + if state != wantstate { + t.Errorf("Expected return state to be %v, got %v", wantstate, state) + } + if un != src.upstreamURL() { + t.Errorf("Expected %s as source URL, got %s", un, src.upstreamURL()) } if src.major != major { t.Errorf("Expected %v as major version filter on gopkginSource, got %v", major, src.major) @@ -281,7 +291,7 @@ func TestBzrSourceInteractions(t *testing.T) { url: u, } - isrc, ident, err := mb.try(context.Background(), cpath, newMemoryCache()) + isrc, state, err := mb.try(context.Background(), cpath, newMemoryCache()) if err != nil { t.Errorf("Unexpected error while setting up bzrSource for test repo: %s", err) rf() @@ -293,8 +303,13 @@ func TestBzrSourceInteractions(t *testing.T) { rf() t.FailNow() } - if ident != un { - t.Errorf("Expected %s as source ident, got %s", un, ident) + + wantstate := sourceIsSetUp | sourceExistsUpstream + if state != wantstate { + t.Errorf("Expected return state to be %v, got %v", wantstate, state) + } + if un != src.upstreamURL() { + t.Errorf("Expected %s as source URL, got %s", un, src.upstreamURL()) } evl := []Version{ NewVersion("1.0.0").Is(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), @@ -390,7 +405,7 @@ func TestHgSourceInteractions(t *testing.T) { url: u, } - isrc, ident, err := mb.try(context.Background(), cpath, newMemoryCache()) + isrc, state, err := mb.try(context.Background(), cpath, newMemoryCache()) if err != nil { t.Errorf("Unexpected error while setting up hgSource for test repo: %s", err) return @@ -400,8 +415,13 @@ func TestHgSourceInteractions(t *testing.T) { t.Errorf("Expected a hgSource, got a %T", isrc) return } - if ident != un { - t.Errorf("Expected %s as source ident, got %s", un, ident) + + wantstate := sourceIsSetUp | sourceExistsUpstream + if state != wantstate { + t.Errorf("Expected return state to be %v, got %v", wantstate, state) + } + if un != src.upstreamURL() { + t.Errorf("Expected %s as source URL, got %s", un, src.upstreamURL()) } // check that an expected rev is present