Skip to content
This repository has been archived by the owner on Feb 3, 2018. It is now read-only.

Commit

Permalink
Create impls and tests for future-based deduction
Browse files Browse the repository at this point in the history
  • Loading branch information
sdboyer committed Aug 14, 2016
1 parent b663ae8 commit 8aae7f2
Show file tree
Hide file tree
Showing 2 changed files with 318 additions and 5 deletions.
139 changes: 139 additions & 0 deletions manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path"
"runtime"
"sort"
"sync"
"testing"

"github.com/Masterminds/semver"
Expand Down Expand Up @@ -338,3 +339,141 @@ func TestGetInfoListVersionsOrdering(t *testing.T) {
t.Errorf("Expected three results from ListVersions, got %v", len(v))
}
}

func TestDeduceProjectRoot(t *testing.T) {
sm, clean := mkNaiveSM(t)
defer clean()

in := "github.com/sdboyer/gps"
pr, err := sm.DeduceProjectRoot(in)
if err != nil {
t.Errorf("Problem while detecting root of %q %s", in, err)
}
if string(pr) != in {
t.Errorf("Wrong project root was deduced;\n\t(GOT) %s\n\t(WNT) %s", pr, in)
}
if sm.rootxt.Len() != 1 {
t.Errorf("Root path trie should have one element after one deduction, has %v", sm.rootxt.Len())
}

pr, err = sm.DeduceProjectRoot(in)
if err != nil {
t.Errorf("Problem while detecting root of %q %s", in, err)
} else if string(pr) != in {
t.Errorf("Wrong project root was deduced;\n\t(GOT) %s\n\t(WNT) %s", pr, in)
}
if sm.rootxt.Len() != 1 {
t.Errorf("Root path trie should have one element after performing the same deduction twice; has %v", sm.rootxt.Len())
}

// Now do a subpath
sub := path.Join(in, "foo")
pr, err = sm.DeduceProjectRoot(sub)
if err != nil {
t.Errorf("Problem while detecting root of %q %s", sub, err)
} else if string(pr) != in {
t.Errorf("Wrong project root was deduced;\n\t(GOT) %s\n\t(WNT) %s", pr, in)
}
if sm.rootxt.Len() != 2 {
t.Errorf("Root path trie should have two elements, one for root and one for subpath; has %v", sm.rootxt.Len())
}

// Now do a fully different root, but still on github
in2 := "github.com/bagel/lox"

This comment has been minimized.

Copy link
@kamilchm

kamilchm Jan 11, 2017

Contributor

Using nonexistent github repo makes go test prompts for github username and password to check if you have rights for a closed repo. Little confusing when you run it for the first time.

This comment has been minimized.

Copy link
@sdboyer

sdboyer Jan 11, 2017

Author Owner

hmm...this test doesn't really need to hit the net. this operation can be strictly local. if it it's going out over the network, it's b/c the SourceMgr is trying to get a little parallelization benefit and reach out to the remote. but we need better control. i don't remember setting it to operate that way, but that whole part of the codebase is squirrely and coming up for a refactor soon now, anyway.

also - there are env settings in the way git is invoked that are supposed to prevent github from prompting you for a username/pw. these work for me, and for gps' CI afaik, but i guess they don't for your setup?

This comment has been minimized.

Copy link
@kamilchm

kamilchm Jan 11, 2017

Contributor

GIT_TERMINAL_PROMPT=0 didn't work for me, but git config core.askPass "" do.

This comment has been minimized.

Copy link
@sdboyer

sdboyer Jan 11, 2017

Author Owner

Bleh. We can't touch git's config, of course.

I'll have to look into this more. Would you mind opening an issue?

This comment has been minimized.

Copy link
@sdboyer

sdboyer Jan 11, 2017

Author Owner

Oh, what version of git are you using? I guess this is a new-ish feature: https://github.com/blog/1957-git-2-3-has-been-released

This comment has been minimized.

Copy link
@kamilchm

kamilchm Jan 11, 2017

Contributor

Found it, I have x11-ssh-askpass as git askPass set and thats why GIT_TERMINAL_PROMPT=0 doesn't block git from showing X11 prompt.
It behaves as expected when I set GIT_ASKPASS= GIT_TERMINAL_PROMPT=0.
Git version 2.11.
Do you want me to create a PR with GIT_ASKPASS= added to

c.Env = mergeEnvLists([]string{"GIT_TERMINAL_PROMPT=0"}, os.Environ())
?

This comment has been minimized.

Copy link
@sdboyer

sdboyer Jan 11, 2017

Author Owner

Yes, please do.

sub2 := path.Join(in2, "cheese")
pr, err = sm.DeduceProjectRoot(sub2)
if err != nil {
t.Errorf("Problem while detecting root of %q %s", sub2, err)
} else if string(pr) != in2 {
t.Errorf("Wrong project root was deduced;\n\t(GOT) %s\n\t(WNT) %s", pr, in)
}
if sm.rootxt.Len() != 4 {
t.Errorf("Root path trie should have four elements, one for each unique root and subpath; has %v", sm.rootxt.Len())
}

// Ensure that our prefixes are bounded by path separators
in4 := "github.com/bagel/loxx"
pr, err = sm.DeduceProjectRoot(in4)
if err != nil {
t.Errorf("Problem while detecting root of %q %s", in4, err)
} else if string(pr) != in4 {
t.Errorf("Wrong project root was deduced;\n\t(GOT) %s\n\t(WNT) %s", pr, in)
}
if sm.rootxt.Len() != 5 {
t.Errorf("Root path trie should have five elements, one for each unique root and subpath; has %v", sm.rootxt.Len())
}
}

// Test that the future returned from SourceMgr.deducePathAndProcess() is safe
// to call concurrently.
//
// Obviously, this is just a heuristic; passage does not guarantee correctness
// (though failure does guarantee incorrectness)
func TestMultiDeduceThreadsafe(t *testing.T) {
sm, clean := mkNaiveSM(t)
defer clean()

in := "github.com/sdboyer/gps"
rootf, srcf, err := sm.deducePathAndProcess(in)
if err != nil {
t.Errorf("Known-good path %q had unexpected basic deduction error: %s", in, err)
t.FailNow()
}

cnum := 50
wg := &sync.WaitGroup{}

// Set up channel for everything else to block on
c := make(chan struct{}, 1)
f := func(rnum int) {
wg.Add(1)
defer func() {
if e := recover(); e != nil {
t.Errorf("goroutine number %v panicked with err: %s", rnum, e)
}
}()
<-c
_, err := rootf()
if err != nil {
t.Errorf("err was non-nil on root detection in goroutine number %v: %s", rnum, err)
}
wg.Done()
}

for k := range make([]struct{}, cnum) {
go f(k)
runtime.Gosched()
}
close(c)
wg.Wait()
if sm.rootxt.Len() != 1 {
t.Errorf("Root path trie should have just one element; has %v", sm.rootxt.Len())
}

// repeat for srcf
c = make(chan struct{}, 1)
f = func(rnum int) {
wg.Add(1)
defer func() {
if e := recover(); e != nil {
t.Errorf("goroutine number %v panicked with err: %s", rnum, e)
}
}()
<-c
_, _, err := srcf()
if err != nil {
t.Errorf("err was non-nil on root detection in goroutine number %v: %s", rnum, err)
}
wg.Done()
}

for k := range make([]struct{}, cnum) {
go f(k)
runtime.Gosched()
}
close(c)
wg.Wait()
if len(sm.srcs) != 2 {
t.Errorf("Sources map should have just two elements, but has %v", len(sm.srcs))
}
}
184 changes: 179 additions & 5 deletions source_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"strings"
"sync"
"sync/atomic"

"github.com/Masterminds/semver"
"github.com/Masterminds/vcs"
Expand Down Expand Up @@ -77,13 +78,16 @@ type SourceMgr struct {
cachedir string
pms map[string]*pmState
pmut sync.RWMutex
srcs map[string]source
srcmut sync.RWMutex
rr map[string]struct {
rr *remoteRepo
err error
}
rmut sync.RWMutex
an ProjectAnalyzer
dxt deducerTrie
rmut sync.RWMutex
an ProjectAnalyzer
dxt deducerTrie
rootxt prTrie
}

var _ SourceManager = &SourceMgr{}
Expand Down Expand Up @@ -137,12 +141,14 @@ func NewSourceManager(an ProjectAnalyzer, cachedir string, force bool) (*SourceM
return &SourceMgr{
cachedir: cachedir,
pms: make(map[string]*pmState),
srcs: make(map[string]source),
rr: make(map[string]struct {
rr *remoteRepo
err error
}),
an: an,
dxt: pathDeducerTrie(),
an: an,
dxt: pathDeducerTrie(),
rootxt: newProjectRootTrie(),
}, nil
}

Expand Down Expand Up @@ -239,6 +245,174 @@ func (sm *SourceMgr) ExportProject(id ProjectIdentifier, v Version, to string) e
return pms.pm.ExportVersionTo(v, to)
}

// DeduceRootProject takes an import path and deduces the
//
// Note that some import paths may require network activity to correctly
// determine the root of the path, such as, but not limited to, vanity import
// paths. (A special exception is written for gopkg.in to minimize network
// activity, as its behavior is well-structured)
func (sm *SourceMgr) DeduceProjectRoot(ip string) (ProjectRoot, error) {
if prefix, root, has := sm.rootxt.LongestPrefix(ip); has {
// The non-matching tail of the import path could still be malformed.
// Validate just that part, if it exists
if prefix != ip {
if !pathvld.MatchString(strings.TrimPrefix(ip, prefix)) {
return "", fmt.Errorf("%q is not a valid import path", ip)
}
// There was one, and it validated fine - add it so we don't have to
// revalidate it later
sm.rootxt.Insert(ip, root)
}
return root, nil
}

rootf, _, err := sm.deducePathAndProcess(ip)
if err != nil {
return "", err
}

r, err := rootf()
return ProjectRoot(r), err
}

func (sm *SourceMgr) getSourceFor(id ProjectIdentifier) (source, error) {
nn := id.netName()

sm.srcmut.RLock()
src, has := sm.srcs[nn]
sm.srcmut.RUnlock()
if has {
return src, nil
}

_, srcf, err := sm.deducePathAndProcess(nn)
if err != nil {
return nil, err
}

// we don't care about the ident here
src, _, err = srcf()
return src, err
}

func (sm *SourceMgr) deducePathAndProcess(path string) (stringFuture, sourceFuture, error) {
df, err := sm.deduceFromPath(path)
if err != nil {
return nil, nil, err
}

var rstart, sstart int32
rc := make(chan struct{}, 1)
sc := make(chan struct{}, 1)

// Rewrap in a deferred future, so the caller can decide when to trigger it
rootf := func() (pr string, err error) {
// CAS because a bad interleaving here would panic on double-closing rc
if atomic.CompareAndSwapInt32(&rstart, 0, 1) {
go func() {
defer close(rc)
pr, err = df.root()
if err != nil {
// Don't cache errs. This doesn't really hurt the solver, and is
// beneficial for other use cases because it means we don't have to
// expose any kind of controls for clearing caches.
return
}

tpr := ProjectRoot(pr)
sm.rootxt.Insert(pr, tpr)
// It's not harmful if the netname was a URL rather than an
// import path
if pr != path {
// Insert the result into the rootxt twice - once at the
// root itself, so as to catch siblings/relatives, and again
// at the exact provided import path (assuming they were
// different), so that on subsequent calls, exact matches
// can skip the regex above.
sm.rootxt.Insert(path, tpr)
}
}()
}

<-rc
return pr, err
}

// Now, handle the source
fut := df.psf(sm.cachedir, sm.an)

// Rewrap in a deferred future, so the caller can decide when to trigger it
srcf := func() (src source, ident string, err error) {
// CAS because a bad interleaving here would panic on double-closing sc
if atomic.CompareAndSwapInt32(&sstart, 0, 1) {
go func() {
defer close(sc)
src, ident, err = fut()
if err != nil {
// Don't cache errs. This doesn't really hurt the solver, and is
// beneficial for other use cases because it means we don't have
// to expose any kind of controls for clearing caches.
return
}

sm.srcmut.Lock()
defer sm.srcmut.Unlock()

// Check to make sure a source hasn't shown up in the meantime, or that
// there wasn't already one at the ident.
var hasi, hasp bool
var srci, srcp source
if ident != "" {
srci, hasi = sm.srcs[ident]
}
srcp, hasp = sm.srcs[path]

// if neither the ident nor the input path have an entry for this src,
// we're in the simple case - write them both in and we're done
if !hasi && !hasp {
sm.srcs[path] = src
if ident != path && ident != "" {
sm.srcs[ident] = src
}
return
}

// Now, the xors.
//
// If already present for ident but not for path, copy ident's src
// to path. This covers cases like a gopkg.in path referring back
// onto a github repository, where something else already explicitly
// looked up that same gh repo.
if hasi && !hasp {
sm.srcs[path] = srci
src = srci
}
// If already present for path but not for ident, do NOT copy path's
// src to ident, but use the returned one instead. Really, this case
// shouldn't occur at all...? But the crucial thing is that the
// path-based one has already discovered what actual ident of source
// they want to use, and changing that arbitrarily would have
// undefined effects.
if hasp && !hasi && ident != "" {
sm.srcs[ident] = src
}

// If both are present, then assume we're good, and use the path one
if hasp && hasi {
// TODO(sdboyer) compare these (somehow? reflect? pointer?) and if they're not the
// same object, panic
src = srcp
}
}()
}

<-sc
return
}

return rootf, srcf, nil
}

// getProjectManager gets the project manager for the given ProjectIdentifier.
//
// If no such manager yet exists, it attempts to create one.
Expand Down

0 comments on commit 8aae7f2

Please sign in to comment.