Skip to content

Conversation

@myitcv
Copy link
Member

Fixes#69

}

trap cleanup EXIT SIGHUP SIGINT SIGTERM
trap cleanup EXIT HUP INT TERM
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Not all shells understand the SIG prefix.

Copy link
Member

Choose a reason for hiding this comment

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

This change is good.

gopherjsgopath=$(go list -f '{{.Root}}' github.com/gopherjs/gopherjs)

rm -r pkg/
rm -rf pkg/
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

If the previous go generate failed before pkg was recreated this step fails otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

This change is good.

}

trap cleanup EXIT SIGHUP SIGINT SIGTERM
trap cleanup EXIT HUP INT TERM
Copy link
Member

Choose a reason for hiding this comment

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

This change is good.

gopherjsgopath=$(go list -f '{{.Root}}' github.com/gopherjs/gopherjs)

rm -r pkg/
rm -rf pkg/
Copy link
Member

Choose a reason for hiding this comment

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

This change is good.

mkdir -p pkg/github.com/gopherjs/gopherjs
cp "$GOPATH"/pkg/*_js_min/github.com/gopherjs/gopherjs/js.a pkg/github.com/gopherjs/gopherjs/js.a
cp "$GOPATH"/pkg/*_js_min/github.com/gopherjs/gopherjs/nosync.a pkg/github.com/gopherjs/gopherjs/nosync.a
mkdir pkg
Copy link
Member

@dmitshurdmitshurApr 24, 2018

Choose a reason for hiding this comment

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

I believe this change would break the playground. Have you tested that it works?

I tested a similar change when I was working on cbdec89, and it resulted in failed to load package "github.com/gopherjs/gopherjs/js" errors for me. Is it not the case for you?

See my comment in #69 for more info.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Yes, worked fine locally for me. But I suspect my answer has something to do with that?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, worked fine locally for me.

I see. Can you elaborate on how you tested it locally?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Subject to the SIG change in update.sh, I ran go generate then gopherjs serve (per the README) and everything worked just fine in my browser.

Copy link
Member

Choose a reason for hiding this comment

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

I can't reproduce your results.

If I remove the js.a.js file from pkg and use gopherjs serve, the playground fails to run the initial program.

playground $ rm ./pkg/github.com/gopherjs/gopherjs/js.a.js playground $ gopherjs serve serving at http://localhost:8080 and on port 8080 of any available addresses 

image

Maybe you didn't clear your browser's cache?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I did the following (recreating the root-owned $GOROOT for fair comparison):

cd $GOROOT sudo chown -R root:root . cd `mktemp -d` export GOPATH=$PWD export PATH=$GOPATH/bin:$PATH go get github.com/gopherjs/gopherjs go get github.com/gopherjs/gopherjs.github.io/playground cd $(go list -f "{{.Dir}}" github.com/gopherjs/gopherjs.github.io/playground) git fetch origin pull/70/head git checkout -q FETCH_HEAD which gopherjs go generate gopherjs serve 

Then went to http://localhost:8080/github.com/gopherjs/gopherjs.github.io/playground/ and did an "Empty Cache and Hard Reload" in Chrome and that page, which imports js, works fine for me: I see the alert as well as the console log and the in-page "console" log.

Am I following the right steps?

For me github.com/gopherjs/gopherjs/js is compiled into playground.js:

$ grep '^\$packages\["github.com/gopherjs/gopherjs/js"\]' playground.js | cut -c 1-80 $packages["github.com/gopherjs/gopherjs/js"]=(function(){var $pkg={},$init,A,B,I 

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for providing more information.

Can you check, after you do go generate, are there 2 files in ./pkg/github.com/gopherjs/gopherjs directory?

Based on the steps you're following... Maybe what's happening is that you already have a built js.a and nosync.a in your GOROOT/pkg from before (when your GOROOT was user-writeable), and it gets copied from there?

Copy link
Member

@dmitshurdmitshurApr 24, 2018

Choose a reason for hiding this comment

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

For me github.com/gopherjs/gopherjs/js is compiled into playground.js:

That's expected, it's there for the playground itself. github.com/gopherjs/gopherjs/compiler will also be there, etc. But it doesn't mean it would be used from there during compilation of playground programs.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

That's expected, it's there for the playground itself. github.com/gopherjs/gopherjs/compiler will also be there, etc. But it doesn't mean it would be used from there during compilation of playground programs.

Good point.

Can you check, after you do go generate, are there 2 files in ./pkg/github.com/gopherjs/gopherjs directory?

Yes there are.

I've also just run the steps above but with a writable GOROOT; success again.

But I don't think this is surprising because after the gopherjs install in the fake GOROOT we do:

cp -a "$GOROOT"/pkg/*_js_min/* pkg/ cp -a "$GOROOT"/pkg/*_amd64_js_min/* pkg/ 

and because some of the natives depend on github.com/gopherjs/gopherjs/js it also gets installed (because gopherjs installs all dependencies, unlike Go 1.10).

I think this adds up now?

Incidentally, do the steps in #70 (comment) not work for you?

@dmitshur
Copy link
Member

I'm going to cherry pick the "fix to work with all shells" changes. We'll leave the rest for later. Fixing #69 has to be done in a way that doesn't break playground if GOROOT is non-writeable.

@dmitshur
Copy link
Member

Closing because #69 has been resolved. Please let me know if there are any issues. Thanks for the work here.

@dmitshurdmitshur closed this Dec 2, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

playground: go generate fails when GOROOT is user-writeable.

2 participants

@myitcv@dmitshur