Vendoring gopherjs code causes generated js to fail #415

benechols-wf · 2016-03-01T17:01:22Z

I have a piece of go that imports gopherjs

package main

import (
    "github.com/gopherjs/gopherjs/js"
)

func main() {
    js.Global.Set("mypackage", map[string]interface{}{
        "DoSomething":         DoSomething,
    })

    println("gopher bindings loaded")
}

func DoSomething() {
  print("done!")
}

I then vendor gopherjs and try to do a gopherjs build. the command succeeds but the generated code fails as soon as it tries to use 'js.Global.Set' because of nil pointer.

If I then delete the vendor/ directory and rebuild everything works fine.

Is there something else I should be vendoring in order for this to work? Or what is the expected workflow?

dmitshur · 2016-03-01T20:12:09Z

Thanks for reporting.

I'm guessing it happens because the github.com/gopherjs/gopherjs/js import path is special (just like the "C" import path that cgo uses), and we're probably not taking into account that it can change when gopherjs is vendored to be example.com/user/repo/vendor/github.com/gopherjs/gopherjs/js.

benechols-wf · 2016-03-01T23:59:54Z

Quick question related to this -- if I vendor the stdlib overrides(gopherjs/compiler/natives/...) into vendor/github.com/gopherjs/gopherjs will those be used instead of looking for them in $GOPATH ?

neelance · 2016-03-02T00:23:26Z

It will always use those form the GOPATH. Vendoring in this context not possible on a theoretical level, unless we come up with some special vendoring rule. E.g. when GopherJS wants to compile net/http, then this resolves to the normal package in GOROOT. There is no vendor directory in this context.

And yes, the github.com/gopherjs/gopherjs/js import path is special and if the package is vendored then it is not recognized any more. I am not sure if we should fix this, because it is mandatory that the versions of the js package and the used GopherJS compiler are the same. Vendoring the js package could make it go out of sync, which is dangerous.

benechols-wf · 2016-03-11T00:13:35Z

I want to vendor all of gopherjs and my build process will build gopherjs from the vendored version. Then there will never be mismatched versions :)

benechols-wf · 2016-03-14T18:33:01Z

I guess our build process could take the vendored version and copy it back into the GOPATH during the build but that seems bad to me.

mpl · 2016-05-06T20:31:01Z

@neelance As @benechols-wf notes, one hits that bug even when one wants to simply vendor the whole of github.com/gopherjs/gopherjs into a project (which should be a pretty common use case), which would not make github.com/gopherjs/gopherjs/js out of sync wrt to github.com/gopherjs/gopherjs.

So it should definitely be fixed imho.

Because it makes full integration with gopherjs impossible (without polluting the user's GOPATH), as long as gopherjs/gopherjs#415 is not fixed. Also it is kind of antithetical with the point of make.go anyway. We still rely on CAMLI_MAKE_USEGOPATH for the integration tests that run make.go to know that they shouldn't recursively create another temp GOPATH (when they're already in such a temp dir, because they're started through devcam test). Change-Id: Icc6af46ec5976fdf08e9b8bf4249e307a15499cf

This should allow vendoring. (#415)

neelance · 2016-05-14T22:42:22Z

This will probably still run into #462. Working on it.

mpl · 2016-05-20T18:28:41Z

@neelance jsyk, I was hoping f627518 and 2e16c68 had fixed this , so I've just retried, and it still does not seem to work for me.

Layout is like this:

$HOME/project/build-env/
    src/
        app/ui/gopherjs/main.go
        app/vendor/github.com/gopherjs/gopherjs
        app/vendor/github.com/gopherjs/gopherjs/js
    pkg/
    bin/

GOPATH=$HOME/project/build-env/

using gopherjs as:

$HOME/project/build-env/bin/gopherjs build -m -o $HOME/project/build-env/app/ui/gopherjs.js app/ui/gopherjs

and getting this error:

exit status 1, cannot find package "github.com/gopherjs/gopherjs/js" in any of:
    /home/mpl/go1/src/vendor/github.com/gopherjs/gopherjs/js (vendor tree)
    /home/mpl/go1/src/github.com/gopherjs/gopherjs/js (from $GOROOT)
    /home/mpl/project/build-env/src/github.com/gopherjs/gopherjs/js (from $GOPATH)
exit status 1

So it seems like it's still not looking for github.com/gopherjs/gopherjs/js in the vendor dir of my project.

…ted. GopherJS itself supports vendoring, but vendoring _it_ into your project is currently not supported. Issue #415 tracks the progress on fixing that. Helps #415. Updates #537.

* Determines whether there is an import path to a vendored version of gopherjs * If so, updates all import paths to gopherjs in generated code to use the vendored version * Updates runtime/runtime.go to use a string constant for its static reference to the gopherjs import path * Adds logic to update the string constant to be the vendored path during generation Fixes gopherjs#415

nmiyake · 2017-08-17T17:22:21Z

I ran into this as well and was interested in fixing this so made a fix locally. It can still use some iteration, but wanted to verify that the general approach is acceptable.

The main issue seems to be resolving the github.com/gopherjs/gopherjs import path to the desired vendored import path when performing imports from a source path that is outside the project (as is done for the standard library generation). My proposed fix takes the following form:

Determine if a project wants to use a vendored version of gopherjs -- if so, determine the import path to the vendored version
In the importWithSrcDir function in build/build.go, if the requested import path starts with github.com/gopherjs/gopherjs, update that path with the vendored version determined above

This approach solves all of the import resolution. The last issue is that the natives version of runtime/runtime.go has a string constant that is explicitly set to github.com/gopherjs/gopherjs/js. However, build.go already has code that adds in the natives files and reads through the AST, so as long as we're okay with some tight coupling, we can update that code to have special logic that, when processing runtime/runtime.go, updates the value of the import path there to be the vendored one as well (it requires minor refactoring of runtime/runtime.go to extract this value to a constant, but that is trivial).

I tried this approach locally on one of my own projects that use gopherjs (made these changes to gopherjs, vendored it entirely, then generated code by running against the vendored version) and everything seemed to work -- the import paths were correct and the Javascript code worked.

The part of the solution that needs the most work right now is determining when gopherjs is vendored and setting the vendor path at the correct level -- for my prototype I just used a package-global variable for storage and set the value in the code that currently determines whether or not a package is vendored, but a real solution would need to do this more elegantly (and not quite sure where that logic should go).

PR for this is at #678 -- feedback very much appreciated.

* Determines whether there is an import path to a vendored version of gopherjs * If so, updates all import paths to gopherjs in generated code to use the vendored version * Updates runtime/runtime.go to use a string constant for its static reference to the gopherjs import path * Adds logic to update the string constant to be the vendored path during generation Fixes gopherjs#415

dmitshur · 2017-08-18T13:32:26Z

Thanks a lot for looking at this @nmiyake and sending a PR. It's very valuable for us to know the cause of the issue and what it takes to resolve it, so thanks for doing the investigation. I'll be able to review the PR shortly.

hanleym · 2017-12-18T18:39:40Z

@shurcooL Any update on this?

dmitshur · 2017-12-18T23:03:29Z

There are no updates from my side. My last comment on the PR said:

Unfortunately, I don't have an opportunity to review it soon.

That continues to be true, unfortunately.

Also ignore github.com/gopherjs/gopherjs/js -- it cannot be vendored See: gopherjs/gopherjs#415 Fixes https://gitlab.com/agamigo/material/issues/16 CI: Don't need to go get dependencies anymore

nathanhack · 2018-03-02T19:28:00Z

@shurcooL I'm sure you're still busy, but I was curious if there was any update for this issue?

dmitshur · 2018-03-02T20:34:20Z

Now that GopherJS 1.10-1 is out, I'll be prioritizing resolving this issue in the coming weeks. I think it's becoming more important to be able to vendor GopherJS, and be able to use fixed versions of it (rather than always counting on the latest).

The PR just needs to be reviewed and tested.

bradfitz · 2018-03-02T20:46:47Z

Thanks!

nathanhack · 2018-03-02T20:52:58Z

@shurcooL That would be fantastic! Thank you.

nmiyake · 2018-03-02T21:11:13Z

Great! I should be able to iterate on or answer any questions about the PR once it gets a chance to be reviewed (I see that there are some merge conflicts now -- I'll take a look at fixing those after receiving initial feedback).

bradfitz · 2018-04-04T17:11:20Z

@shurcooL, this is holding up Perkeep getting rid of its hacky make.go wrapper that builds a temp dir parallel GOPATH to work around pre-Go 1.10 cmd/go bugs. We want to just use Go 1.10 without GOPATH tempdir hackery.

Can we incentivize you to fix this? We have some budget (https://opencollective.com/perkeep) and could pay for your time.

dmitshur · 2018-04-04T21:46:57Z

Can we incentivize you to fix this?

You already have! I will make this issue my priority for the coming weekend.

this is holding up Perkeep getting rid of its hacky make.go wrapper that builds a temp dir parallel GOPATH to work around pre-Go 1.10 cmd/go bugs. We want to just use Go 1.10 without GOPATH tempdir hackery.

You should be able to use Go 1.10 even before this is resolved. I'll want to chime in on that issue when I'm working on this.

We have some budget (https://opencollective.com/perkeep) and could pay for your time.

Thank you very much, but I'd be worried that I only did a part of all the work required to resolve this issue (I've never been paid to work on an open source issue before, so I'm not sure how to deal with it). Maybe you'd want to buy me a coffee instead. But this weekend is still the soonest I can get to it, because I'll need a large continuous time block.

The Go stdlib overrides (i.e., compiler/natives package) are already embedded into the GopherJS build system. This makes it possible for the gopherjs binary to access them even if the gopherjs repository is not present in GOPATH (e.g., because it was deleted, or because it's vendored). Doing that for natives but not the js, nosync packages makes little sense, since the gopherjs repository still needs to exist in GOPATH. This change fixes that. By embedding the core GopherJS packages analogously to natives, the gopherjs binary becomes more standalone. Now, it only requires GOROOT/src to contain the matching version of Go source code. Non-core GopherJS packages are not embedded (e.g., the GopherJS compiler itself). That means it needs to exist in GOPATH to be buildable (otherwise, the user gets a "cannot find package" error). This is expected just like with any other normal Go package. Fixes #462. Helps #415.

…nosync. These core GopherJS packages are already embedded into the GopherJS build system via the gopherjspkg virtual filesystem. The gopherjspkg package can be safely vendored. Therefore, there's no need to try to use vendor directory to resolve those packages. This change makes it possible to use a vendored copy of GopherJS in a project, and use it to build any Go code. Fixes #415.

The Go stdlib overrides (i.e., compiler/natives package) are already embedded into the GopherJS build system. This makes it possible for the gopherjs binary to access them even if the gopherjs repository is not present in GOPATH (e.g., because it was deleted, or because it's vendored). Doing that for natives but not the js, nosync packages makes little sense, since the gopherjs repository still needs to exist in GOPATH. This change fixes that. By embedding the core GopherJS packages analogously to natives, the gopherjs binary becomes more standalone. Now, it only requires GOROOT/src to contain the matching version of Go source code. Non-core GopherJS packages are not embedded (e.g., the GopherJS compiler itself). That means it needs to exist in GOPATH to be buildable (otherwise, the user gets a "cannot find package" error). This is expected just like with any other normal Go package. Fixes #462. Helps #415.

…nosync. These core GopherJS packages are already embedded into the GopherJS build system via the gopherjspkg virtual filesystem. The gopherjspkg package can be safely vendored. Therefore, there's no need to try to use vendor directory to resolve those packages. This change makes it possible to use a vendored copy of GopherJS in a project, and use it to build any Go code. Fixes #415.

The Go stdlib overrides (i.e., compiler/natives package) are already embedded into the GopherJS build system. This makes it possible for the gopherjs binary to access them even if the gopherjs repository is not present in GOPATH (e.g., because it was deleted, or because it's vendored). Doing that for natives but not the js, nosync packages makes little sense, since the gopherjs repository still needs to exist in GOPATH. This change fixes that. By embedding the core GopherJS packages analogously to natives, the gopherjs binary becomes more standalone. Now, it only requires GOROOT/src to contain the matching version of Go source code. Non-core GopherJS packages are not embedded (e.g., the GopherJS compiler itself). That means it needs to exist in GOPATH to be buildable (otherwise, the user gets a "cannot find package" error). This is expected just like with any other normal Go package. Fixes #462. Helps #415.

…nosync. These core GopherJS packages are already embedded into the GopherJS build system via the gopherjspkg virtual filesystem. The gopherjspkg package can be safely vendored. Therefore, there's no need to try to use vendor directory to resolve those packages. This change makes it possible to use a vendored copy of GopherJS in a project, and use it to build any Go code. Fixes #415.

dmitshur · 2018-04-08T01:29:33Z

I've created PR #787 that should resolve this issue.

In my testing, I was able to vendor the entire GopherJS project in another Go project, and then used it to successfully build Go code (including Go code that imports github.com/gopherjs/gopherjs/js package). This was done without the (normal, non-vendored) GopherJS repository in GOPATH.

I think everything should work, but it's possible I'm not aware of some other ways people are vendoring GopherJS. Please feel free to test the PR and let me know if vendoring doesn't work for you.

The Go stdlib overrides (i.e., compiler/natives package) are already embedded into the GopherJS build system. This makes it possible for the gopherjs binary to access them even if the gopherjs repository is not present in GOPATH (e.g., because it was deleted, or because it's vendored). Doing that for natives but not the js, nosync packages makes little sense, since the gopherjs repository still needs to exist in GOPATH. This change fixes that. By embedding the core GopherJS packages analogously to natives, the gopherjs binary becomes more standalone. Now, it only requires GOROOT/src to contain the matching version of Go source code. Non-core GopherJS packages are not embedded (e.g., the GopherJS compiler itself). That means it needs to exist in GOPATH to be buildable (otherwise, the user gets a "cannot find package" error). This is expected just like with any other normal Go package. Fixes #462. Helps #415.

…nosync. These core GopherJS packages are already embedded into the GopherJS build system via the gopherjspkg virtual filesystem. The gopherjspkg package can be safely vendored. Therefore, there's no need to try to use vendor directory to resolve those packages. This change makes it possible to use a vendored copy of GopherJS in a project, and use it to build any Go code. Fixes #415.

This way, we get more information when/if TestGopherJSCanBeVendored fails. Before: $ go test -v -run=TestGopherJSCanBeVendored === RUN TestGopherJSCanBeVendored --- FAIL: TestGopherJSCanBeVendored (5.20s) gorepo_test.go:38: exit status 1 FAIL exit status 1 FAIL github.com/gopherjs/gopherjs/tests 5.203s After: $ go test -v -run=TestGopherJSCanBeVendored === RUN TestGopherJSCanBeVendored vendoring github.com/gopherjs/gopherjs/js package is not supported, see #415 --- FAIL: TestGopherJSCanBeVendored (5.19s) gorepo_test.go:40: exit status 1 FAIL exit status 1 FAIL github.com/gopherjs/gopherjs/tests 5.190s

The Go stdlib overrides (i.e., compiler/natives package) are already embedded into the GopherJS build system. This makes it possible for the gopherjs binary to access them even if the gopherjs repository is not present in GOPATH (e.g., because it was deleted, or because it's vendored). Doing that for natives but not the js, nosync packages makes little sense, since the gopherjs repository still needs to exist in GOPATH. This change fixes that. By embedding the core GopherJS packages analogously to natives, the gopherjs binary becomes more standalone. Now, it only requires GOROOT/src to contain the matching version of Go source code. Non-core GopherJS packages are not embedded (e.g., the GopherJS compiler itself). That means it needs to exist in GOPATH to be buildable (otherwise, the user gets a "cannot find package" error). This is expected just like with any other normal Go package. Fixes #462. Helps #415.

…nosync. These core GopherJS packages are already embedded into the GopherJS build system via the gopherjspkg virtual filesystem. The gopherjspkg package can be safely vendored. Therefore, there's no need to try to use vendor directory to resolve those packages. This change makes it possible to use a vendored copy of GopherJS in a project, and use it to build any Go code. Fixes #415.

This was referenced Mar 19, 2016

godep is asking to install 'vendored' #423

Closed

Vendoring and sourcemap package #427

Closed

neelance added a commit that referenced this issue May 14, 2016

detect js package by /gopherjs/js suffix

f627518

This should allow vendoring. (#415)

This was referenced Oct 14, 2016

Uncaught RangeError: Maximum call stack size exceeded dominikh/go-js-dom#34

Closed

[WIP] Use Travis for CI myitcv/react#1

Merged

myitcv mentioned this issue Oct 17, 2016

RangeError when using EventSource with vendored dependencies #537

Closed

dmitshur mentioned this issue Oct 18, 2016

README: Document that using vendored GopherJS is currently not supported. #538

Merged

dmitshur mentioned this issue Jan 12, 2017

Endless unwinding call stack for seachJsObject #569

Closed

flimzy mentioned this issue May 10, 2017

Proposal: explicit *js.Object-special struct types #633

Open

dmitshur added NeedsFix bug labels May 11, 2017

nmiyake mentioned this issue Aug 17, 2017

Add support for vendoring gopherjs #678

Closed

mpl mentioned this issue Jan 3, 2018

Depend on Go 1.10, stop using tmp/ directory source code mirror for make.go? perkeep/perkeep#1004

Closed

ralimi mentioned this issue Jan 14, 2018

Vendor gopherjs google/chrome-ssh-agent#1

Closed

bzub added a commit to agamigo/material that referenced this issue Feb 9, 2018

dep init

c8c486f

Also ignore github.com/gopherjs/gopherjs/js -- it cannot be vendored See: gopherjs/gopherjs#415 Fixes https://gitlab.com/agamigo/material/issues/16 CI: Don't need to go get dependencies anymore

dmitshur mentioned this issue Apr 8, 2018

Embed core GopherJS packages into build system; enable vendoring of GopherJS. #787

Merged

dmitshur closed this as completed in #787 Apr 21, 2018

Vendoring gopherjs code causes generated js to fail #415

Vendoring gopherjs code causes generated js to fail #415

Comments

benechols-wf commented Mar 1, 2016

dmitshur commented Mar 1, 2016

Uh oh!

benechols-wf commented Mar 1, 2016

Uh oh!

neelance commented Mar 2, 2016

Uh oh!

benechols-wf commented Mar 11, 2016

Uh oh!

benechols-wf commented Mar 14, 2016

Uh oh!

mpl commented May 6, 2016

Uh oh!

neelance commented May 14, 2016

Uh oh!

mpl commented May 20, 2016

Uh oh!

nmiyake commented Aug 17, 2017

Uh oh!

dmitshur commented Aug 18, 2017

Uh oh!

hanleym commented Dec 18, 2017

Uh oh!

dmitshur commented Dec 18, 2017

Uh oh!

nathanhack commented Mar 2, 2018

Uh oh!

dmitshur commented Mar 2, 2018

Uh oh!

bradfitz commented Mar 2, 2018

Uh oh!

nathanhack commented Mar 2, 2018

Uh oh!

nmiyake commented Mar 2, 2018

Uh oh!

bradfitz commented Apr 4, 2018

Uh oh!

dmitshur commented Apr 4, 2018

Uh oh!

dmitshur commented Apr 8, 2018

Uh oh!