Use new gps capabilities during init's dep search #305
Conversation
Also refactors a bit and adds some comments for clarity.
sdboyer
changed the title
cmd/dep/init.go
Outdated
| dependencies[pr] = append(dependencies[pr], ip) | ||
| } | ||
| vlogf("Building dependency graph...") | ||
| for _, ip := range rm.Flatten(false) { |
There was a problem hiding this comment.
Can you give this false a meaningful name?
eg
const foo = false
for _, ip := range rm.Flatten(foo) {
because right now I have no idea what false means here.
There was a problem hiding this comment.
hah, interesting, i'd never thought to do something like that. sure, i can - the false just indicates that stdlib imports should be omitted.
| @@ -350,13 +335,31 @@ func getProjectData(ctx *dep.Ctx, pkgT gps.PackageTree, cpr string, sm *gps.Sour | |||
| // project is not present in the workspace, and so we need to | |||
| // solve to deal with this dep. | |||
| r := filepath.Join(ctx.GOPATH, "src", string(pr)) | |||
| _, err := os.Lstat(r) | |||
| if os.IsNotExist(err) { | |||
| fi, err := os.Stat(r) | |||
There was a problem hiding this comment.
Why the change from Lstat to Stat?
There was a problem hiding this comment.
Because, unless I misunderstand how this all works, we could Lstat a symlink that exists, but its referent does not. In that case, the subsequent check of os.IsNotExist() would not have the desired effect - it would tell us the symlink exists, but what we really care about is the referent existing.
...At least, I think that's what I think we care about here. My latest explorations into symlinks, and what we should do/the toolchain currently does left me dizzy.
|
Looks good. |
Use new gps capabilities during init's dep search
Also refactors a bit and adds some comments for clarity. Most importantly, this fixes #298.
Let's wait to merge this until after #287; I want to add a new test specifically for this case, but not until the new harness is ready.