-
Notifications
You must be signed in to change notification settings - Fork 11
update.sh: fix to work with all shells and gopherjs 1.10-3. #70
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
Conversation
@@ -8,7 +8,7 @@ cleanup() { | |||
exit | |||
} | |||
|
|||
trap cleanup EXIT SIGHUP SIGINT SIGTERM | |||
trap cleanup EXIT HUP INT TERM |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is good.
@@ -20,18 +20,15 @@ gopherjs build -m | |||
# The GOPATH workspace where the GopherJS project is. | |||
gopherjsgopath=$(go list -f '{{.Root}}' github.com/gopherjs/gopherjs) | |||
|
|||
rm -r pkg/ | |||
rm -rf pkg/ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is good.
@@ -8,7 +8,7 @@ cleanup() { | |||
exit | |||
} | |||
|
|||
trap cleanup EXIT SIGHUP SIGINT SIGTERM | |||
trap cleanup EXIT HUP INT TERM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is good.
@@ -20,18 +20,15 @@ gopherjs build -m | |||
# The GOPATH workspace where the GopherJS project is. | |||
gopherjsgopath=$(go list -f '{{.Root}}' github.com/gopherjs/gopherjs) | |||
|
|||
rm -r pkg/ | |||
rm -rf pkg/ |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Maybe you didn't clear your browser's cache?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 intoplayground.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.
There was a problem hiding this comment.
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?
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. |
Closing because #69 has been resolved. Please let me know if there are any issues. Thanks for the work here. |
Fixes #69