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

Protoype: allow constraint.source to be a sub-package of an repo #1000

Closed
wants to merge 1 commit into from
Closed

Protoype: allow constraint.source to be a sub-package of an repo #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions internal/gps/deduce.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"net/url"
"path"
"path/filepath"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -587,7 +588,7 @@ func (dc *deductionCoordinator) deduceRootPath(ctx context.Context, path string)
if has && isPathPrefixOrEqual(prefix, path) {
switch d := data.(type) {
case maybeSource:
return pathDeduction{root: prefix, mb: d}, nil
return pathDeduction{root: prefix, postfix: pathPostfix(path, prefix), mb: d}, nil
case *httpMetadataDeducer:
// Multiple calls have come in for a similar path shape during
// the window in which the HTTP request to retrieve go get
Expand Down Expand Up @@ -643,12 +644,19 @@ func (dc *deductionCoordinator) deduceRootPath(ctx context.Context, path string)
return hmd.deduce(ctx, path)
}

// pathPostfix removes the prefix from s (including the path seperator.
func pathPostfix(s, prefix string) string {
return strings.TrimLeft(strings.TrimPrefix(s, prefix), string([]rune{filepath.Separator}))
}

// pathDeduction represents the results of a successful import path deduction -
// a root path, plus a maybeSource that can be used to attempt to connect to
// the source.
// the source. The source corresponds to the root path. If a sub-path of root has
// been given for deduction, the root+"/"+postfix is the original input path.
type pathDeduction struct {
root string
mb maybeSource
root string
postfix string
mb maybeSource
}

var errNoKnownPathMatch = errors.New("no known path match")
Expand All @@ -671,8 +679,9 @@ func (dc *deductionCoordinator) deduceKnownPaths(path string) (pathDeduction, er
}

return pathDeduction{
root: root,
mb: mb,
root: root,
postfix: pathPostfix(path, root),
mb: mb,
}, nil
}

Expand All @@ -685,8 +694,9 @@ func (dc *deductionCoordinator) deduceKnownPaths(path string) (pathDeduction, er
}

return pathDeduction{
root: root,
mb: mb,
root: root,
postfix: pathPostfix(path, root),
mb: mb,
}, nil
}

Expand Down Expand Up @@ -729,6 +739,7 @@ func (hmd *httpMetadataDeducer) deduce(ctx context.Context, path string) (pathDe
return
}
pd.root = root
pd.postfix = pathPostfix(path, root)

// If we got something back at all, then it supersedes the actual input for
// the real URL to hit
Expand Down
8 changes: 4 additions & 4 deletions internal/gps/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,14 +354,14 @@ func TestGetSources(t *testing.T) {
t.Run(lpi.normalizedSource(), func(t *testing.T) {
t.Parallel()

srcg, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
if err != nil {
t.Errorf("unexpected error setting up source: %s", err)
return
}

// Re-get the same, make sure they are the same
srcg2, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
srcg2, _, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
if err != nil {
t.Errorf("unexpected error re-getting source: %s", err)
} else if srcg != srcg2 {
Expand All @@ -370,7 +370,7 @@ func TestGetSources(t *testing.T) {

// All of them _should_ select https, so this should work
lpi.Source = "https://" + lpi.Source
srcg3, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
srcg3, _, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
if err != nil {
t.Errorf("unexpected error getting explicit https source: %s", err)
} else if srcg != srcg3 {
Expand All @@ -379,7 +379,7 @@ func TestGetSources(t *testing.T) {

// Now put in http, and they should differ
lpi.Source = "http://" + string(lpi.ProjectRoot)
srcg4, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
srcg4, _, err := sm.srcCoord.getSourceGatewayFor(ctx, lpi)
if err != nil {
t.Errorf("unexpected error getting explicit http source: %s", err)
} else if srcg == srcg4 {
Expand Down
32 changes: 18 additions & 14 deletions internal/gps/source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"context"
"errors"
"fmt"
"path/filepath"
"strings"
"sync"

"github.com/golang/dep/internal/gps/pkgtree"
Expand Down Expand Up @@ -62,9 +64,9 @@ func newSourceCoordinator(superv *supervisor, deducer deducer, cachedir string)
}
}

func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id ProjectIdentifier) (*sourceGateway, error) {
func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id ProjectIdentifier) (*sourceGateway, string, error) {
if sc.supervisor.getLifetimeContext().Err() != nil {
return nil, errors.New("sourceCoordinator has been terminated")
return nil, "", errors.New("sourceCoordinator has been terminated")
}

normalizedName := id.normalizedSource()
Expand All @@ -74,7 +76,7 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
srcGate, has := sc.srcs[url]
sc.srcmut.RUnlock()
if has {
return srcGate, nil
return srcGate, "", nil
}
panic(fmt.Sprintf("%q was URL for %q in nameToURL, but no corresponding srcGate in srcs map", url, normalizedName))
}
Expand All @@ -92,7 +94,8 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
}
sc.protoSrcs[normalizedName] = append(chans, rc)
sc.psrcmut.Unlock()
return rc.awaitReturn()
sg, err := rc.awaitReturn()
return sg, "", err
}

sc.protoSrcs[normalizedName] = []srcReturnChans{}
Expand Down Expand Up @@ -121,7 +124,7 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
// As in the deducer, don't cache errors so that externally-driven retry
// strategies can be constructed.
doReturn(nil, err)
return nil, err
return nil, "", err
}

// It'd be quite the feat - but not impossible - for a gateway
Expand All @@ -135,7 +138,7 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
if srcGate, has := sc.srcs[url]; has {
sc.srcmut.RUnlock()
doReturn(srcGate, nil)
return srcGate, nil
return srcGate, "", nil
}
panic(fmt.Sprintf("%q was URL for %q in nameToURL, but no corresponding srcGate in srcs map", url, normalizedName))
}
Expand All @@ -156,25 +159,26 @@ func (sc *sourceCoordinator) getSourceGatewayFor(ctx context.Context, id Project
url, err := srcGate.sourceURL(ctx)
if err != nil {
doReturn(nil, err)
return nil, err
return nil, "", err
}

// We know we have a working srcGateway at this point, and need to
// integrate it back into the main map.
sc.srcmut.Lock()
defer sc.srcmut.Unlock()
// Record the name -> URL mapping, even if it's a self-mapping.
sc.nameToURL[normalizedName] = url
name := strings.TrimRight(strings.TrimSuffix(normalizedName, pd.postfix), string([]rune{filepath.Separator}))
sc.nameToURL[name] = url

if sa, has := sc.srcs[url]; has {
// URL already had an entry in the main map; use that as the result.
doReturn(sa, nil)
return sa, nil
return sa, pd.postfix, nil
}

sc.srcs[url] = srcGate
doReturn(srcGate, nil)
return srcGate, nil
return srcGate, pd.postfix, nil
}

// sourceGateways manage all incoming calls for data from sources, serializing
Expand Down Expand Up @@ -322,7 +326,7 @@ func (sg *sourceGateway) getManifestAndLock(ctx context.Context, pr ProjectRoot,

// FIXME ProjectRoot input either needs to parameterize the cache, or be
// incorporated on the fly on egress...?
func (sg *sourceGateway) listPackages(ctx context.Context, pr ProjectRoot, v Version) (pkgtree.PackageTree, error) {
func (sg *sourceGateway) listPackages(ctx context.Context, sourceRoot string, pr ProjectRoot, v Version) (pkgtree.PackageTree, error) {
sg.mu.Lock()
defer sg.mu.Unlock()

Expand All @@ -343,7 +347,7 @@ func (sg *sourceGateway) listPackages(ctx context.Context, pr ProjectRoot, v Ver

label := fmt.Sprintf("%s:%s", pr, sg.src.upstreamURL())
err = sg.suprvsr.do(ctx, label, ctListPackages, func(ctx context.Context) error {
ptree, err = sg.src.listPackages(ctx, pr, r)
ptree, err = sg.src.listPackages(ctx, sourceRoot, pr, r)
return err
})

Expand All @@ -362,7 +366,7 @@ func (sg *sourceGateway) listPackages(ctx context.Context, pr ProjectRoot, v Ver
}

err = sg.suprvsr.do(ctx, label, ctGetManifestAndLock, func(ctx context.Context) error {
ptree, err = sg.src.listPackages(ctx, pr, r)
ptree, err = sg.src.listPackages(ctx, sourceRoot, pr, r)
return err
})
}
Expand Down Expand Up @@ -548,7 +552,7 @@ type source interface {
updateLocal(context.Context) error
listVersions(context.Context) ([]PairedVersion, error)
getManifestAndLock(context.Context, ProjectRoot, Revision, ProjectAnalyzer) (Manifest, Lock, error)
listPackages(context.Context, ProjectRoot, Revision) (pkgtree.PackageTree, error)
listPackages(context.Context, string, ProjectRoot, Revision) (pkgtree.PackageTree, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this strikes me as unnecessary - it's a fairly straightforward manipulation to select a subtree of a PackageTree. as such, this could just be a method on PackageTree itself; we needn't push it into the SourceManager's API.

GetManifestAndLock() is the harder one that may actually need this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could just be a method on PackageTree itself;

I actually had that and hit some issues due to the cache. Didn't dig any deeper and just hacked listPackages. You have a better overview for a more elegant implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, the cache is not designed to parameterize on a subpath. it'd be quite wasteful to inflate the cache with the additional parameter, and applying it as a post-cache transform wouldn't add much value.

better to have e.g. func (ptree PackageTree) Subtree(subpath string) (PackageTree, error), which produces a new PackageTree containing only the subpackages along that subpath. error could be omitted - OTTOMH the only possible error it could express is if the provided subpath didn't match any packages in the receiver PackageTree.

i think this would fit in quite snugly with the other PackageTree methods.

revisionPresentIn(Revision) (bool, error)
exportRevisionTo(context.Context, Revision, string) error
sourceType() string
Expand Down
17 changes: 9 additions & 8 deletions internal/gps/source_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func (sm *SourceMgr) GetManifestAndLock(id ProjectIdentifier, v Version, an Proj
return nil, nil, smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
return nil, nil, err
}
Expand All @@ -397,12 +397,13 @@ func (sm *SourceMgr) ListPackages(id ProjectIdentifier, v Version) (pkgtree.Pack
return pkgtree.PackageTree{}, smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, postfix, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
return pkgtree.PackageTree{}, err
}

return srcg.listPackages(context.TODO(), id.ProjectRoot, v)
ret, err := srcg.listPackages(context.TODO(), postfix, id.ProjectRoot, v)
return ret, err
}

// ListVersions retrieves a list of the available versions for a given
Expand All @@ -422,7 +423,7 @@ func (sm *SourceMgr) ListVersions(id ProjectIdentifier) ([]PairedVersion, error)
return nil, smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
// TODO(sdboyer) More-er proper-er errors
return nil, err
Expand All @@ -438,7 +439,7 @@ func (sm *SourceMgr) RevisionPresentIn(id ProjectIdentifier, r Revision) (bool,
return false, smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
// TODO(sdboyer) More-er proper-er errors
return false, err
Expand All @@ -454,7 +455,7 @@ func (sm *SourceMgr) SourceExists(id ProjectIdentifier) (bool, error) {
return false, smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
return false, err
}
Expand All @@ -472,7 +473,7 @@ func (sm *SourceMgr) SyncSourceFor(id ProjectIdentifier) error {
return smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
return err
}
Expand All @@ -487,7 +488,7 @@ func (sm *SourceMgr) ExportProject(id ProjectIdentifier, v Version, to string) e
return smIsReleased{}
}

srcg, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
srcg, _, err := sm.srcCoord.getSourceGatewayFor(context.TODO(), id)
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions internal/gps/source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func testSourceGateway(t *testing.T) {
sc := newSourceCoordinator(superv, newDeductionCoordinator(superv), cachedir)

id := mkPI("github.com/sdboyer/deptest")
sg, err := sc.getSourceGatewayFor(ctx, id)
sg, _, err := sc.getSourceGatewayFor(ctx, id)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -123,7 +123,7 @@ func testSourceGateway(t *testing.T) {
t.Fatalf("wanted nonexistent err when passing bad version, got: %s", err)
}

_, err = sg.listPackages(ctx, ProjectRoot("github.com/sdboyer/deptest"), badver)
_, err = sg.listPackages(ctx, "", ProjectRoot("github.com/sdboyer/deptest"), badver)
if err == nil {
t.Fatal("wanted err on nonexistent version")
} else if err.Error() != wanterr.Error() {
Expand All @@ -150,15 +150,15 @@ func testSourceGateway(t *testing.T) {
},
}

ptree, err := sg.listPackages(ctx, ProjectRoot("github.com/sdboyer/deptest"), Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"))
ptree, err := sg.listPackages(ctx, "", ProjectRoot("github.com/sdboyer/deptest"), Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"))
if err != nil {
t.Fatalf("unexpected err when getting package tree with known rev: %s", err)
}
if !reflect.DeepEqual(wantptree, ptree) {
t.Fatalf("got incorrect PackageTree:\n\t(GOT): %#v\n\t(WNT): %#v", ptree, wantptree)
}

ptree, err = sg.listPackages(ctx, ProjectRoot("github.com/sdboyer/deptest"), NewVersion("v1.0.0"))
ptree, err = sg.listPackages(ctx, "", ProjectRoot("github.com/sdboyer/deptest"), NewVersion("v1.0.0"))
if err != nil {
t.Fatalf("unexpected err when getting package tree with unpaired good version: %s", err)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/gps/vcs_source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ func (bs *baseVCSSource) updateLocal(ctx context.Context) error {
return nil
}

func (bs *baseVCSSource) listPackages(ctx context.Context, pr ProjectRoot, r Revision) (ptree pkgtree.PackageTree, err error) {
func (bs *baseVCSSource) listPackages(ctx context.Context, sourceRoot string, pr ProjectRoot, r Revision) (ptree pkgtree.PackageTree, err error) {
err = bs.repo.updateVersion(ctx, r.String())

if err != nil {
err = unwrapVcsErr(err)
} else {
ptree, err = pkgtree.ListPackages(bs.repo.LocalPath(), string(pr))
ptree, err = pkgtree.ListPackages(filepath.Join(bs.repo.LocalPath(), sourceRoot), string(pr))
}

return
Expand Down