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

Add satisfiability check for case variants #1079

Merged
merged 14 commits into from
Sep 18, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Add the wrongCaseFailure, and scads more fixtures
  • Loading branch information
sdboyer committed Sep 10, 2017
commit 3d369c50a97e2eb77c262e77a8a09b8728819461
26 changes: 26 additions & 0 deletions internal/gps/satisfy.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,32 @@ func (s *solver) checkRootCaseConflicts(a atomWithPackages, cdep completeDep) er
s.fail(d.depender.id)
}

// If a project has multiple packages that import each other, we treat that
// as establishing a canonical case variant for the ProjectRoot. It's possible,
// however, that that canonical variant is not the same one that others
// imported it under. If that's the situation, then we'll have arrived here
// when visiting the project, not its dependers, having misclassified its
// internal imports as external. That means the atomWithPackages will
// be the wrong case variant induced by the importers, and the cdep will be
// a link pointing back at the canonical case variant.
//
// If this is the case, use a special failure, wrongCaseFailure, that
// makes a stronger statement as to the correctness of case variants.
//
// TODO(sdboyer) This approach to marking failure is less than great, as
// this will mark the current atom as failed, as well, causing the
// backtracker to work through it. While that could prove fruitful, it's
// quite likely just to be wasted effort. Addressing this - if that's a good
// idea - would entail creating another path back out of checking to enable
// backjumping directly to the incorrect importers.
if current == a.a.id.ProjectRoot {
return &wrongCaseFailure{
correct: pr,
goal: dependency{depender: a.a, dep: cdep},
badcase: deps,
}
}

return &caseMismatchFailure{
goal: dependency{depender: a.a, dep: cdep},
current: current,
Expand Down
3 changes: 3 additions & 0 deletions internal/gps/solve_basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,9 @@ type basicFixture struct {
changeall bool
// individual projects to change
changelist []ProjectRoot
// if the fixture is currently broken/expected to fail, this has a message
// recording why
broken string
}

func (f basicFixture) name() string {
Expand Down
105 changes: 85 additions & 20 deletions internal/gps/solve_bimodal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ var bimodalFixtures = map[string]bimodalFixture{
"a 1.0.0",
),
},
"simple case-only variations": {
"simple case-only differences": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo", "bar")),
Expand All @@ -706,6 +706,23 @@ var bimodalFixtures = map[string]bimodalFixture{
},
},
},
"case variations acceptable with agreement": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0"),
pkg("foo", "Bar", "baz")),
dsp(mkDepspec("baz 1.0.0"),
pkg("baz", "Bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
},
r: mksolution(
"foo 1.0.0",
"Bar 1.0.0",
"baz 1.0.0",
),
},
"case variations within root": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
Expand All @@ -714,8 +731,6 @@ var bimodalFixtures = map[string]bimodalFixture{
pkg("foo")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("Bar 1.0.0"),
pkg("Bar")),
},
fail: &noVersionError{
pn: mkPI("foo"),
Expand All @@ -730,6 +745,7 @@ var bimodalFixtures = map[string]bimodalFixture{
},
},
},
broken: "need to implement checking for import case variations *within* the root",
},
"case variations within single dep": {
ds: []depspec{
Expand All @@ -739,8 +755,6 @@ var bimodalFixtures = map[string]bimodalFixture{
pkg("foo", "bar", "Bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("Bar 1.0.0"),
pkg("Bar")),
},
fail: &noVersionError{
pn: mkPI("foo"),
Expand All @@ -755,8 +769,9 @@ var bimodalFixtures = map[string]bimodalFixture{
},
},
},
broken: "need to implement checking for import case variations *within* a single project",
},
"case variations within multiple deps": {
"case variations across multiple deps": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo", "bar")),
Expand All @@ -766,8 +781,6 @@ var bimodalFixtures = map[string]bimodalFixture{
pkg("baz", "Bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("Bar 1.0.0"),
pkg("Bar")),
},
fail: &noVersionError{
pn: mkPI("baz"),
Expand All @@ -787,37 +800,86 @@ var bimodalFixtures = map[string]bimodalFixture{
},
},
// This isn't actually as crazy as it might seem, as the root is defined by
// the addresser, not the addressee. It would occur if, e.g. something
// imports github.com/Sirupsen/logrus, as the contained subpackage at
// the addresser, not the addressee. It would occur (to provide a
// real-as-of-this-writing example) if something imports
// github.com/Sirupsen/logrus, as the contained subpackage at
// github.com/Sirupsen/logrus/hooks/syslog imports
// github.com/sirupsen/logrus. The only reason that doesn't blow up all the
// time is that most people only import the root package, not the syslog
// subpackage.
"case variations from self": {
"canonical case is established by mutual self-imports": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0"),
pkg("foo", "bar")),
pkg("foo", "Bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar", "Bar")),
dsp(mkDepspec("Bar 1.0.0"),
pkg("Bar")),
pkg("bar", "bar/subpkg"),
pkg("bar/subpkg")),
},
fail: &noVersionError{
pn: mkPI("foo"),
pn: mkPI("Bar"),
fails: []failedVersion{
{
v: NewVersion("1.0.0"),
f: &caseMismatchFailure{
goal: mkDep("foo 1.0.0", "Bar 1.0.0", "Bar"),
current: ProjectRoot("bar"),
failsib: []dependency{mkDep("root", "foo 1.0.0", "foo")},
f: &wrongCaseFailure{
correct: ProjectRoot("bar"),
goal: mkDep("Bar 1.0.0", "bar 1.0.0", "bar"),
badcase: []dependency{mkDep("foo 1.0.0", "Bar 1.0.0", "Bar/subpkg")},
},
},
},
},
},
"canonical case only applies if relevant imports are activated": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0"),
pkg("foo", "Bar/subpkg")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar", "bar/subpkg"),
pkg("bar/subpkg")),
},
r: mksolution(
"foo 1.0.0",
mklp("Bar 1.0.0", "subpkg"),
),
},
"simple case-only variations plus source variance": {
// COPYPASTA BELOW, FIX IT
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo", "Bar")), // TODO align the froms
dsp(mkDepspec("foo 1.0.0", "Bar from quux 1.0.0"),
pkg("foo", "Bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("quux 1.0.0"),
pkg("bar")),
},
r: mksolution(
"foo 1.0.0",
"Bar from quux 1.0.0",
),
},
"case-only variations plus source variance with internal canonicality": {
// COPYPASTA BELOW, FIX IT
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo", "Bar")),
dsp(mkDepspec("foo 1.0.0", "Bar from quux 1.0.0"),
pkg("foo", "Bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("quux 1.0.0"),
pkg("bar")),
},
r: mksolution(
"foo 1.0.0",
"Bar from quux 1.0.0",
),
},
"alternate net address": {
ds: []depspec{
dsp(mkDepspec("root 1.0.0", "foo from bar 2.0.0"),
Expand Down Expand Up @@ -1240,6 +1302,9 @@ type bimodalFixture struct {
ignore []string
// pkgs to require
require []string
// if the fixture is currently broken/expected to fail, this has a message
// recording why
broken string
}

func (f bimodalFixture) name() string {
Expand Down
4 changes: 2 additions & 2 deletions internal/gps/solve_failures.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,13 @@ type wrongCaseFailure struct {

func (e *wrongCaseFailure) Error() string {
if len(e.badcase) == 1 {
str := "Could not introduce %s; its packages import each other using %q, establishing that as correct, but %s tried to import it as %q"
str := "Could not introduce %s; mutual imports by its packages establish %q as the canonical casing for root, but %s tried to import it as %q"
return fmt.Sprintf(str, a2vs(e.goal.depender), e.correct, a2vs(e.badcase[0].depender), e.badcase[0].dep.Ident.ProjectRoot)
}

var buf bytes.Buffer

str := "Could not introduce %s; its packages import each other using %q, establishing that as correct, but the following projects tried to import it as %q"
str := "Could not introduce %s; mutual imports by its packages establish %q as the canonical casing for root, but the following projects tried to import it as %q"
fmt.Fprintf(&buf, str, a2vs(e.goal.depender), e.correct, e.badcase[0].dep.Ident.ProjectRoot)

for _, c := range e.badcase {
Expand Down
6 changes: 6 additions & 0 deletions internal/gps/solve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func TestBasicSolves(t *testing.T) {

func solveBasicsAndCheck(fix basicFixture, t *testing.T) (res Solution, err error) {
sm := newdepspecSM(fix.ds, nil)
if fix.broken != "" {
t.Skip(fix.broken)
}

params := SolveParameters{
RootDir: string(fix.ds[0].n),
Expand Down Expand Up @@ -103,6 +106,9 @@ func TestBimodalSolves(t *testing.T) {

func solveBimodalAndCheck(fix bimodalFixture, t *testing.T) (res Solution, err error) {
sm := newbmSM(fix)
if fix.broken != "" {
t.Skip(fix.broken)
}

params := SolveParameters{
RootDir: string(fix.ds[0].n),
Expand Down