The problem here is that github.com/gonum/blas imports github.com/gonum/internal/asm/f32, which is outside the blas module. We will need to decide what the rules are for cross-module internal like that.

The internal packages design document states the rule as:

An import of a path containing the element “internal” is disallowed if the importing code is outside the tree rooted at the parent of the “internal” directory.

A compatible rule, thinking just in terms of import paths is:

An import of a path containing the element “internal” is disallowed if the importing code does not have the same path prefix as the “internal” path.

The example in this issue has github.com/gonum/blas importing github.com/gonum/internal/asm/f32. The path prefix for the internal import is "github.com/gonum". Since github.com/gonum/blas also has that prefix, the import would be allowed.

This modified rule would, however, allow a situation that is not allowed by the current implementation (go1.10.1): cross-GOPATH internal imports:

Given GOPATH=$HOME/a:$HOME/b and two files:

$HOME/a/src/prefix/importer/importer.go:

package importer

import "prefix/internal"

func G() {
	internal.F()
}

and $HOME/b/src/prefix/internal/internal.go:

package internal

func F() {
	println("internal")
}

The build fails with:

$ go build prefix/importer
a/src/prefix/importer/importer.go:3:8: use of internal package not allowed

The modified rule would not disallow this as it only considers the import path and not where the package is on disk.

When both files are in the same GOPATH root (e.g., GOPATH=$HOME/combined with files named combined/src/prefix/importer/importer.go and combined/src/prefix/internal/internal.go) the build succeeds.

This is the only case I know of where the modified rule differs from the current implementation.

The modified rule is, I believe, a valid interpretation of the current rule; thinking of the (de facto) import path hierarchy instead of the disk hierarchy.

@nathankerr, right, your counter-example is exactly why we can't redefine the general internal import rule as you described. That would allow, for example, every package in the world to import "internal/testenv", which is meant to be internal to the standard library. The use of directory, not import path, was intentional.

That said, it does seem like focusing on import path for the narrower case of "one module referring to packages in another module" does make sense, because those could have been in a single GOPATH before anyway.

I'll mark this NeedsFix but we need to be careful with the fix not to open things too far.

@rsc I hadn't considered GOROOT. Good catch.

Viewing all modules as being in the same tree (separate from GOROOT) as if they were all in a single GOPATH keeps the original rule intact.

github.com/cznic/lldb imports github.com/cznic/internal/buffer which seems to be fordbidden, now.
What is the solution?

What is the solution?

@tgulacsi Regardless of the solution, if you meanwhile need any of those internal packages made generally importable, please fill an issue at cznic/internal and I will let it happen. The original intent was to let practice show/prove if those API are acceptable/working well. IIRC, they were not changed for a long time, if at all. So I guess it's safe to move them out from 'internal'.

Thanks, @cznic - my main concern is though I'd happily use vgo as it tries to solve some of my problems, I stumble walls everywhere - github.com/cznic/internal (for github.com/cznic/kv for camlistore.org/pkg/sorted/kvfile), github.com/russross/blackfriday (for I don't know what).

I'd file issues to those upstream packages, but I feel vgo is still in its infancy, and don't want to bother those nice guys for such a volatile target.

@tgulacsi I agree with you: we should be fixing vgo, not forcing other projects to work around its bugs.

another possible way to look at this: now that we have a go.mod file, we could perhaps add another (optional) section that lists modules that are allowed to import that module ?

that's what the internal special name was for in the end...

admittedly, this does complexify the logic.

@sbinet I don't think we have a use-case that requires an explicit list. Let's see how far we get with just the path-based approach for now.

I think this issue would also have an interesting interaction with the go mod -vendor and go build -getmode=vendor commands. Assuming go mod -vendor successfully copies the dependencies to the vendor folder that would restore the pre-module directory structure and presumably go build -getmode=vendor would work.

Change https://golang.org/cl/125676 mentions this issue: cmd/go: allow internal imports based on module paths

So, here's an interesting question: how should internal imports interact with replace directives?

Should the effective path for the replace directive be the import path seen by the program, or the module path as declared in the replacement's module definition? (The former is easier to implement, but the latter seems more correct.)

Change https://golang.org/cl/125836 mentions this issue: cmd/go: use replacement module path to check internal imports

What was the outcome of this problem as hitting similar issues with github.com/mongodb/mongo-go-driver? And lets say trying to use github.com/mongodb/mongo-go-driver/core/connstring.

internal imports should work based on the import path. (The importer of an internal package must share the import-path prefix leading up to the /internal component, but can be in a separate module.)