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

panic: Should never need to recheck imports/constraints from root during solve #1123

Closed
F21 opened this issue Sep 5, 2017 · 4 comments · Fixed by #1147
Closed

panic: Should never need to recheck imports/constraints from root during solve #1123

F21 opened this issue Sep 5, 2017 · 4 comments · Fixed by #1147

Comments

@F21
Copy link

F21 commented Sep 5, 2017

What version of dep are you using (dep version)?

1963157

What dep command did you run?

I have a library (github.com/F21/some-lib) in my GOPATH. I then accidentally ran dep inside the project importing itself.

$ dep ensure -v -add github.com/F21/some-lib
Root project is "github.com/F21/some-lib"
 1 transitively valid internal packages
 8 external packages imported from 8 projects
(0)   ✓ select (root)
(1)     ? revisit github.com/F21/some-lib to add 1 pkgs
panic: Should never need to recheck imports/constraints from root during solve

goroutine 1 [running]:
github.com/golang/dep/internal/gps.(*solver).getImportsAndConstraintsOf(0xc042398600, 0xc0421d6060, 0x21, 0x0, 0x0, 0xb13a00, 0xb67000, 0xc04228a200, 0x1, 0x1, ...)
        D:/work/src/github.com/golang/dep/internal/gps/solver.go:617 +0xeb6
github.com/golang/dep/internal/gps.(*solver).check(0xc042398600, 0xc0421d6060, 0x21, 0x0, 0x0, 0xb13a00, 0xb67000, 0xc04228a200, 0x1, 0x1, ...)
        D:/work/src/github.com/golang/dep/internal/gps/satisfy.go:43 +0x1f7
github.com/golang/dep/internal/gps.(*solver).solve(0xc042398600, 0x0, 0x0, 0xc042381ad0)
        D:/work/src/github.com/golang/dep/internal/gps/solver.go:540 +0x33a
github.com/golang/dep/internal/gps.(*solver).Solve(0xc042398600, 0x38, 0xb0f040, 0xb667d0, 0xc04216e330)
        D:/work/src/github.com/golang/dep/internal/gps/solver.go:442 +0xb1
main.(*ensureCommand).runAdd(0xc0420e95e0, 0xc04208ee60, 0xc042072040, 0x1, 0x4, 0xc04203b440, 0xb13f80, 0xc0422d01e0, 0xc04215c040, 0x38, ...)
        D:/work/src/github.com/golang/dep/cmd/dep/ensure.go:597 +0x15f3
main.(*ensureCommand).Run(0xc0420e95e0, 0xc04208ee60, 0xc042072040, 0x1, 0x4, 0x0, 0x0)
        D:/work/src/github.com/golang/dep/cmd/dep/ensure.go:186 +0x56b
main.(*Config).Run(0xc04203ef00, 0xc04203ef00)
        D:/work/src/github.com/golang/dep/cmd/dep/main.go:159 +0x7a9
main.main()
        D:/work/src/github.com/golang/dep/cmd/dep/main.go:45 +0x203

What did you expect to see?

Maybe there should be a nicer error message rather than a panic?

What did you see instead?

There was a panic

@F21 F21 closed this as completed Sep 5, 2017
@F21 F21 reopened this Sep 5, 2017
@sdboyer sdboyer added the bug label Sep 5, 2017
@sdboyer
Copy link
Member

sdboyer commented Sep 5, 2017

OH, hah! yeah, this should be a simple case to guard against. just need to reject arguments to dep ensure -add that come back to the same project as we're currently in. i...guess? hmm. it's actually kinda weird we can get here at all.

well, this'll take a little tracing to see how we reach this path, but the fix should be pretty straightforward.

@sdboyer
Copy link
Member

sdboyer commented Sep 5, 2017

this'd be a really good first PR - if someone's interested, drop a note, and i'll provide more guidance.

@vedhavyas
Copy link
Contributor

@sdboyer I can take it up

@sdboyer
Copy link
Member

sdboyer commented Sep 5, 2017

@vedhavyas woot!

so basically, i think this should just amount to one more check in this loop, comparing the parsed root to the current project's root and failing out if they're the same.

we'll want to add another harness test to capture this specifically, too. there's a README on how to write those in general, and then lots of examples to work from. though this case may be a bit tricky, as i can't recall what the root of the project in the harness tests actually ends up being - we might have it hardcoded to github.com/golang/notexist.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants