build: Detect vendored js package and return error. #572
Conversation
| @@ -79,6 +79,11 @@ func importWithSrcDir(path string, srcDir string, mode build.ImportMode, install | |||
| return nil, err | |||
| } | |||
|
|
|||
| // TODO: Resolve issue #415 and remove this temporary workaround. | |||
| if strings.HasSuffix(pkg.ImportPath, "/vendor/github.com/gopherjs/gopherjs/js") { | |||
There was a problem hiding this comment.
Does this condition work? path == "github.com/gopherjs/gopherjs/js" && pkg.ImportPath != path
There was a problem hiding this comment.
I know pkg.ImportPath will always be an absolute import path, but I'm not sure about path. It might be relative. E.g., consider:
cd $GOPATH/src/github.com/gopherjs/gopherjs
gopherjs install ./js
And:
pkg, err := buildContext.Import(path, srcDir, mode)Is there some issue with the current approach, or do you dislike it because of a string comparison?
I think this is the canonical way to detect vendored packages, but I could be wrong.
There was a problem hiding this comment.
Yes, path might be relative in the case you mentioned, but I don't think that case is relevant here. Still, on second thought I think your solution is also good. Let's merge.
| @@ -79,6 +79,11 @@ func importWithSrcDir(path string, srcDir string, mode build.ImportMode, install | |||
| return nil, err | |||
| } | |||
|
|
|||
| // TODO: Resolve issue #415 and remove this temporary workaround. | |||
| if strings.HasSuffix(pkg.ImportPath, "/vendor/github.com/gopherjs/gopherjs/js") { | |||
There was a problem hiding this comment.
Yes, path might be relative in the case you mentioned, but I don't think that case is relevant here. Still, on second thought I think your solution is also good. Let's merge.
If you look at 42e6459, this solution is effectively taken from our existing code to detect |
This follows #538.
Vendoring
github.com/gopherjs/gopherjs/jspackage is not supported, and can cause hard to understand runtime errors. Better catch it earlier and print a clear error message.Helps #569.