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

Allow dot-prefixed packages as valid import paths #572

Merged
merged 5 commits into from
May 16, 2017
Merged

Allow dot-prefixed packages as valid import paths #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6af821f
Allow dot-prefixed packages as valid import paths
ascandella May 12, 2017
9b9b6c1
Fix test case description
ascandella May 12, 2017
b3102f1
Add licence header to test file
ascandella May 12, 2017
fb6bacb
Move .m1p tests to dotgodir
ascandella May 12, 2017
36b6d46
Rename vcsRoots
ascandella May 15, 2017
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
7 changes: 7 additions & 0 deletions internal/gps/_testdata/src/dotgodir/.go/dot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package dot

// nothing to see here
2 changes: 1 addition & 1 deletion ...rnal/gps/_testdata/src/disallow/.m1p/a.go → ...rnal/gps/_testdata/src/dotgodir/.m1p/a.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package m1p
import (
"sort"

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

var (
Expand Down
0 ...rnal/gps/_testdata/src/disallow/.m1p/b.go → ...rnal/gps/_testdata/src/dotgodir/.m1p/b.go
View file
Open in desktop
File renamed without changes.
20 changes: 16 additions & 4 deletions internal/gps/pkgtree/pkgtree.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ type Package struct {
TestImports []string // Imports from all go test files (in go/build parlance: both TestImports and XTestImports)
}

// vcsRoots is a set of directories we should not descend into in ListPackages when
// searching for Go packages
var vcsRoots = map[string]struct{}{
".git": struct{}{},
".bzr": struct{}{},
".svn": struct{}{},
".hg": struct{}{},
}

// ListPackages reports Go package information about all directories in the tree
// at or below the provided fileRoot.
//
Expand Down Expand Up @@ -78,10 +87,13 @@ func ListPackages(fileRoot, importRoot string) (PackageTree, error) {
case "vendor", "Godeps":
return filepath.SkipDir
}
// We do skip dot-dirs, though, because it's such a ubiquitous standard
// that they not be visited by normal commands, and because things get
// really weird if we don't.
if strings.HasPrefix(fi.Name(), ".") {

// Skip dirs that are known to be VCS roots.
//
// Note that there are some pathological edge cases this doesn't cover,
// such as a user using Git for version control, but having a package
// named "svn" in a directory named ".svn".
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want to try and address this -- should we only skip these dirs if we're at the root of the project? Will that cause problems for submodules?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're OK ignoring it for now, though I appreciate you pointing it out. It's a very, very edge case, and trying to deal with it here would do some level-busting: a fix would equate "root of repo" with "root of project". That's something that's true 99% of the time, but isn't actually entailed by the design of the system - at least, not this far down.

if _, ok := vcsRoots[fi.Name()]; ok {
return filepath.SkipDir
}

Expand Down
35 changes: 20 additions & 15 deletions internal/gps/pkgtree/pkgtree_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,20 +1073,6 @@ func TestListPackages(t *testing.T) {
},
},
},
// disallow/.m1p is ignored by listPackages...for now. Kept
// here commented because this might change again...
//"disallow/.m1p": {
//P: Package{
//ImportPath: "disallow/.m1p",
//CommentPath: "",
//Name: "m1p",
//Imports: []string{
//"github.com/golang/dep/internal/gps",
//"os",
//"sort",
//},
//},
//},
"disallow/testdata": {
P: Package{
ImportPath: "disallow/testdata",
Expand Down Expand Up @@ -1280,7 +1266,7 @@ func TestListPackages(t *testing.T) {
},
},
},
"skip directories starting with '.'": {
"does not skip directories starting with '.'": {
fileRoot: j("dotgodir"),
importRoot: "dotgodir",
out: PackageTree{
Expand All @@ -1292,13 +1278,32 @@ func TestListPackages(t *testing.T) {
Imports: []string{},
},
},
"dotgodir/.go": {
P: Package{
ImportPath: "dotgodir/.go",
Name: "dot",
Imports: []string{},
},
},
"dotgodir/foo.go": {
P: Package{
ImportPath: "dotgodir/foo.go",
Name: "foo",
Imports: []string{"sort"},
},
},
"dotgodir/.m1p": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad you caught these to move them, I entirely forgot they were there 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP :)

P: Package{
ImportPath: "dotgodir/.m1p",
CommentPath: "",
Name: "m1p",
Imports: []string{
"github.com/golang/dep/internal/gps",
"os",
"sort",
},
},
},
},
},
},
Expand Down