Skip to content

Commit

Permalink
gps: Convert to use IgnoredRuleset
Browse files Browse the repository at this point in the history
  • Loading branch information
sdboyer committed Oct 13, 2017
1 parent 4eb59de commit b0face7
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 155 deletions.
25 changes: 4 additions & 21 deletions internal/gps/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,30 +81,13 @@ func (s *solver) writeHashingInputs(w io.Writer) {
// those will have already been implicitly incorporated by the import
// lister.
writeString(hhIgnores)
ig := make([]string, 0, len(s.rd.ig))
for pkg := range s.rd.ig {
// Skip wildcard ignore. They are handled separately below.
if strings.HasSuffix(pkg, wcIgnoreSuffix) {
continue
}

if !strings.HasPrefix(pkg, s.rd.rpt.ImportRoot) || !isPathPrefixOrEqual(s.rd.rpt.ImportRoot, pkg) {
ig = append(ig, pkg)
}
}

// Add wildcard ignores to ignore list.
if s.rd.igpfx != nil {
s.rd.igpfx.Walk(func(s string, v interface{}) bool {
ig = append(ig, s+"*")
return false
})
}

ig := s.rd.ir.ToSlice()
sort.Strings(ig)

for _, igp := range ig {
writeString(igp)
if !strings.HasPrefix(igp, s.rd.rpt.ImportRoot) || !isPathPrefixOrEqual(s.rd.rpt.ImportRoot, igp) {
writeString(igp)
}
}

// Overrides *also* need their own special entry distinct from basic
Expand Down
19 changes: 9 additions & 10 deletions internal/gps/hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"strings"
"testing"
"text/tabwriter"

"github.com/golang/dep/internal/gps/pkgtree"
)

func TestHashInputs(t *testing.T) {
Expand Down Expand Up @@ -64,10 +66,7 @@ func TestHashInputsReqsIgs(t *testing.T) {
fix := basicFixtures["shared dependency with overlapping constraints"]

rm := fix.rootmanifest().(simpleRootManifest).dup()
rm.ig = map[string]bool{
"foo": true,
"bar": true,
}
rm.ig = pkgtree.NewIgnoredRuleset([]string{"foo", "bar"})

params := SolveParameters{
RootDir: string(fix.ds[0].n),
Expand Down Expand Up @@ -611,7 +610,7 @@ func TestHashInputsIneffectualWildcardIgs(t *testing.T) {

cases := []struct {
name string
ignoreMap map[string]bool
ignoreMap []string
elems []string
}{
{
Expand All @@ -634,10 +633,10 @@ func TestHashInputsIneffectualWildcardIgs(t *testing.T) {
},
{
name: "different wildcard ignores",
ignoreMap: map[string]bool{
"foobar*": true,
"foobarbaz*": true,
"foozapbar*": true,
ignoreMap: []string{
"foobar*",
"foobarbaz*",
"foozapbar*",
},
elems: []string{
hhConstraints,
Expand All @@ -662,7 +661,7 @@ func TestHashInputsIneffectualWildcardIgs(t *testing.T) {
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {

rm.ig = c.ignoreMap
rm.ig = pkgtree.NewIgnoredRuleset(c.ignoreMap)

params.Manifest = rm

Expand Down
27 changes: 15 additions & 12 deletions internal/gps/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package gps

import "github.com/golang/dep/internal/gps/pkgtree"

// Manifest represents manifest-type data for a project at a particular version.
// The constraints expressed in a manifest determine the set of versions that
// are acceptable to try for a given project.
Expand Down Expand Up @@ -36,14 +38,15 @@ type RootManifest interface {
// them can harm the ecosystem as a whole.
Overrides() ProjectConstraints

// IngoredPackages returns a set of import paths to ignore. These import
// paths can be within the root project, or part of other projects. Ignoring
// a package means that both it and its (unique) imports will be disregarded
// by all relevant solver operations.
// IngoredPackages returns a pkgtree.IgnoredRuleset, which comprises a set
// of import paths, or import path patterns, that are to be ignored during
// solving. These ignored import paths can be within the root project, or
// part of other projects. Ignoring a package means that both it and its
// (unique) imports will be disregarded by all relevant solver operations.
//
// It is an error to include a package in both the ignored and required
// sets.
IgnoredPackages() map[string]bool
IgnoredPackages() *pkgtree.IgnoredRuleset

// RequiredPackages returns a set of import paths to require. These packages
// are required to be present in any solution. The list can include main
Expand Down Expand Up @@ -76,8 +79,9 @@ func (m SimpleManifest) DependencyConstraints() ProjectConstraints {
// simpleRootManifest exists so that we have a safe value to swap into solver
// params when a nil Manifest is provided.
type simpleRootManifest struct {
c, ovr ProjectConstraints
ig, req map[string]bool
c, ovr ProjectConstraints
ig *pkgtree.IgnoredRuleset
req map[string]bool
}

func (m simpleRootManifest) DependencyConstraints() ProjectConstraints {
Expand All @@ -86,7 +90,7 @@ func (m simpleRootManifest) DependencyConstraints() ProjectConstraints {
func (m simpleRootManifest) Overrides() ProjectConstraints {
return m.ovr
}
func (m simpleRootManifest) IgnoredPackages() map[string]bool {
func (m simpleRootManifest) IgnoredPackages() *pkgtree.IgnoredRuleset {
return m.ig
}
func (m simpleRootManifest) RequiredPackages() map[string]bool {
Expand All @@ -96,7 +100,6 @@ func (m simpleRootManifest) dup() simpleRootManifest {
m2 := simpleRootManifest{
c: make(ProjectConstraints, len(m.c)),
ovr: make(ProjectConstraints, len(m.ovr)),
ig: make(map[string]bool, len(m.ig)),
req: make(map[string]bool, len(m.req)),
}

Expand All @@ -106,13 +109,13 @@ func (m simpleRootManifest) dup() simpleRootManifest {
for k, v := range m.ovr {
m2.ovr[k] = v
}
for k, v := range m.ig {
m2.ig[k] = v
}
for k, v := range m.req {
m2.req[k] = v
}

// IgnoredRulesets are immutable, and safe to reuse.
m2.ig = m.ig

return m2
}

Expand Down
24 changes: 5 additions & 19 deletions internal/gps/rootdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ type rootdata struct {
// Path to the root of the project on which gps is operating.
dir string

// Ruleset for ignored import paths.
ir *pkgtree.IgnoredRuleset

// Map of packages to ignore.
ig map[string]bool

Expand Down Expand Up @@ -57,7 +60,7 @@ type rootdata struct {
// Ignores and requires are taken into consideration, stdlib is excluded, and
// errors within the local set of package are not backpropagated.
func (rd rootdata) externalImportList(stdLibFn func(string) bool) []string {
rm, _ := rd.rpt.ToReachMap(true, true, false, rd.ig)
rm, _ := rd.rpt.ToReachMap(true, true, false, rd.ir)
reach := rm.FlattenFn(stdLibFn)

// If there are any requires, slide them into the reach list, as well.
Expand Down Expand Up @@ -194,7 +197,7 @@ func (rd rootdata) rootAtom() atomWithPackages {

list := make([]string, 0, len(rd.rpt.Packages))
for path, pkg := range rd.rpt.Packages {
if pkg.Err != nil && !rd.ig[path] {
if pkg.Err != nil && !rd.ir.IsIgnored(path) {
list = append(list, path)
}
}
Expand All @@ -205,20 +208,3 @@ func (rd rootdata) rootAtom() atomWithPackages {
pl: list,
}
}

// isIgnored checks if a given path is ignored, comparing with the list of
// ignore packages and ignore prefixes.
func (rd rootdata) isIgnored(path string) bool {
// Check if the path is present in ignore packages.
if rd.ig[path] {
return true
}

if rd.igpfx != nil {
// Check if the path matches any of the ignore prefixes.
_, _, ok := rd.igpfx.LongestPrefix(path)
return ok
}

return false
}
59 changes: 2 additions & 57 deletions internal/gps/rootdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestRootdataExternalImports(t *testing.T) {
// Add an ignore, but not on the required path (Prepare makes that
// combination impossible)

rd.ig["b"] = true
rd.ir = pkgtree.NewIgnoredRuleset([]string{"b"})
want = []string{"a", "c"}
got = rd.externalImportList(params.stdLibFn)
if !reflect.DeepEqual(want, got) {
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestGetApplicableConstraints(t *testing.T) {
// violate the principle of least surprise?
name: "ignore imported and overridden pkg",
mut: func() {
rd.ig["d"] = true
rd.ir = pkgtree.NewIgnoredRuleset([]string{"d"})
},
result: []workingConstraint{
{
Expand Down Expand Up @@ -224,58 +224,3 @@ func TestGetApplicableConstraints(t *testing.T) {
})
}
}

func TestIsIgnored(t *testing.T) {
cases := []struct {
name string
ignorePkgs map[string]bool
wantIgnored []string
wantNotIgnored []string
}{
{
name: "no ignore",
},
{
name: "ignores without wildcard",
ignorePkgs: map[string]bool{
"a/b/c": true,
"m/n": true,
"gophers": true,
},
wantIgnored: []string{"a/b/c", "m/n", "gophers"},
wantNotIgnored: []string{"somerandomstring"},
},
{
name: "ignores with wildcard",
ignorePkgs: map[string]bool{
"a/b/c*": true,
"m/n*/o": true,
"*x/y/z": true,
"A/B*/C/D*": true,
},
wantIgnored: []string{"a/b/c", "a/b/c/d", "a/b/c-d", "m/n*/o", "*x/y/z", "A/B*/C/D", "A/B*/C/D/E"},
wantNotIgnored: []string{"m/n/o", "m/n*", "x/y/z", "*x/y/z/a", "*x", "A/B", "A/B*/C"},
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
rd := rootdata{
ig: c.ignorePkgs,
igpfx: pkgtree.CreateIgnorePrefixTree(c.ignorePkgs),
}

for _, p := range c.wantIgnored {
if !rd.isIgnored(p) {
t.Fatalf("expected %q to be ignored", p)
}
}

for _, p := range c.wantNotIgnored {
if rd.isIgnored(p) {
t.Fatalf("expected %q to be not ignored", p)
}
}
})
}
}
5 changes: 1 addition & 4 deletions internal/gps/solve_bimodal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1345,12 +1345,9 @@ func (f bimodalFixture) rootmanifest() RootManifest {
m := simpleRootManifest{
c: pcSliceToMap(f.ds[0].deps),
ovr: f.ovr,
ig: make(map[string]bool),
ig: pkgtree.NewIgnoredRuleset(f.ignore),
req: make(map[string]bool),
}
for _, ig := range f.ignore {
m.ig[ig] = true
}
for _, req := range f.require {
m.req[req] = true
}
Expand Down
15 changes: 6 additions & 9 deletions internal/gps/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (params SolveParameters) toRootdata() (rootdata, error) {
}

rd := rootdata{
ig: params.Manifest.IgnoredPackages(),
ir: params.Manifest.IgnoredPackages(),
req: params.Manifest.RequiredPackages(),
ovr: params.Manifest.Overrides(),
rpt: params.RootPackageTree.Copy(),
Expand All @@ -206,13 +206,10 @@ func (params SolveParameters) toRootdata() (rootdata, error) {
rd.ovr = make(ProjectConstraints)
}

// Create ignore prefix tree using the provided ignore packages
rd.igpfx = pkgtree.CreateIgnorePrefixTree(rd.ig)

if len(rd.ig) != 0 {
if rd.ir.Len() > 0 {
var both []string
for pkg := range params.Manifest.RequiredPackages() {
if rd.isIgnored(pkg) {
if rd.ir.IsIgnored(pkg) {
both = append(both, pkg)
}
}
Expand Down Expand Up @@ -633,7 +630,7 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]string, []com
return nil, nil, err
}

rm, em := ptree.ToReachMap(true, false, true, s.rd.ig)
rm, em := ptree.ToReachMap(true, false, true, s.rd.ir)
// Use maps to dedupe the unique internal and external packages.
exmap, inmap := make(map[string]struct{}), make(map[string]struct{})

Expand Down Expand Up @@ -661,14 +658,14 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]string, []com
// explicitly listed in the atom
for _, pkg := range a.pl {
// Skip ignored packages
if s.rd.isIgnored(pkg) {
if s.rd.ir.IsIgnored(pkg) {
continue
}

ie, exists := rm[pkg]
if !exists {
// Missing package here *should* only happen if the target pkg was
// poisoned. Check the errors map
// poisoned; check the errors map.
if importErr, eexists := em[pkg]; eexists {
return nil, nil, importErr
}
Expand Down
6 changes: 3 additions & 3 deletions internal/gps/solver_inputs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestBadSolveOpts(t *testing.T) {
}

params.Manifest = simpleRootManifest{
ig: map[string]bool{"foo": true},
ig: pkgtree.NewIgnoredRuleset([]string{"foo"}),
req: map[string]bool{"foo": true},
}
_, err = Prepare(params, sm)
Expand All @@ -105,7 +105,7 @@ func TestBadSolveOpts(t *testing.T) {
}

params.Manifest = simpleRootManifest{
ig: map[string]bool{"foo": true, "bar": true},
ig: pkgtree.NewIgnoredRuleset([]string{"foo", "bar"}),
req: map[string]bool{"foo": true, "bar": true},
}
_, err = Prepare(params, sm)
Expand All @@ -116,7 +116,7 @@ func TestBadSolveOpts(t *testing.T) {
}

params.Manifest = simpleRootManifest{
ig: map[string]bool{"foo*": true},
ig: pkgtree.NewIgnoredRuleset([]string{"foo*"}),
req: map[string]bool{"foo/bar": true},
}
_, err = Prepare(params, sm)
Expand Down
Loading

0 comments on commit b0face7

Please sign in to comment.