Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/build: creating vendor/a/b/c implicitly vendors a and a/b too #13832

Closed
alandonovan opened this issue Jan 5, 2016 · 7 comments
Closed

go/build: creating vendor/a/b/c implicitly vendors a and a/b too #13832

alandonovan opened this issue Jan 5, 2016 · 7 comments
Milestone

Comments

@alandonovan
Copy link
Contributor

The AllowVendor logic in (*build.Context).Import checks for a directory called vendor/a/b when importing a package "a/b". If only package "a/b/c" was vendored, the directory a/b must exist but will be empty, causing Import not to find the real (unvendored) package.

Import should check for non-empty directories. This logic differs from the usual (non-vendored) case.

@bradfitz bradfitz added this to the Go1.6 milestone Jan 5, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Jan 5, 2016

I'm going to mark this as Go 1.6, as it's blocking x/tools/cmd/bundle from being able to update the std copy of http2 from x/net.

@rsc
Copy link
Contributor

rsc commented Jan 6, 2016

Hmm. The vendor spec is very clear:

If there is a source directory d/vendor, then, when compiling a source file within the subtree rooted at d, import "p" is interpreted as import "d/vendor/p" if that exists.

But obviously that's not good enough. I'm surprised this is the first we've noticed this. I can't decide if that's good.

The a/b directory in your example is non-empty: it contains c. It's not entirely clear what to do. I guess if there are no .go files at all (not even ones with exclusionary build tags) then we could ignore the directory. I'll try that.

@alandonovan
Copy link
Contributor Author

FWIW, this was an effective fix:

--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -581,7 +581,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa
                                        vendor := ctxt.joinPath(root, sub, "vendor")
                                        if ctxt.isDir(vendor) {
                                                dir := ctxt.joinPath(vendor, path)
-                                               if ctxt.isDir(dir) {
+                                               if entries, _ := ctxt.readDir(dir); len(entries) > 0 {

EDIT: although now that you mention it, I don't know why this worked since the subdirectory counts as an entry too. Will investigate.

@rsc
Copy link
Contributor

rsc commented Jan 6, 2016

@alandonovan, how does that work? Doesn't a/b contain a/b/c? Am I misunderstanding what readDir does? It looks like it calls ioutil.ReadDir. Maybe you overrided the default with a broken ctxt.ReadDir function set that doesn't return subdirectories?

@alandonovan
Copy link
Contributor Author

See my EDIT: I don't yet know why it seems to work.

@alandonovan
Copy link
Contributor Author

I still can't explain it. Consider my patch retracted, and sorry for the noise.

@gopherbot
Copy link

CL https://golang.org/cl/18644 mentions this issue.

@golang golang locked and limited conversation to collaborators Jan 13, 2017
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants