search

gopherjs / gopherjs.github.io

GopherJS Playground
https://gopherjs.github.io/playground/
23 stars 12 forks source link

playground: Explicitly set GOARCH=js during go generate. #66

Closed dmitshur closed 7 years ago

dmitshur commented 7 years ago

This is needed after gopherjs/gopherjs#594 was resolved (/cc @bep), because in Go 1.8, go generate sets GOARCH to source platform it's running on, but we want it to be either unset or GOARCH=js.

This is very similar to issue gopherjs/gopherjs#601 (/cc @DrJosh9000).

Also use tabs rather than spaces for indentation (since it no longer aligns anyway).

This was used locally to generate 37d98840c93dad01418d9aaf1dfb6c5df86f9d32.

dmitshur commented 7 years ago

@neelance Did you read the PR description or commit message, and it's still unclear? I can elaborate further, just want to make sure you didn't miss this:

This is needed after gopherjs/gopherjs#594 was resolved, because in Go 1.8, go generate sets GOARCH to source platform it's running on, but we want it to be either unset or GOARCH=js.

This is very similar to issue gopherjs/gopherjs#601.

Without this change, you'd get the following error:

gopherjs: unsupported GOOS/GOARCH pair darwin/amd64
neelance commented 7 years ago

Yeah, I read it, but I didn't understand. Now that I looked again it is suddenly clear to me. ;-) But I have to say: Are we really sure that failing on a bad GOARCH is a good idea? To me this seems a lot like ease of use should win by just always ignoring GOARCH. Imho there is no proper benefit in the current solution. I liked it at first to be consistent with gc, but now I see the issues that it creates.

dmitshur commented 7 years ago

I'm not sure. I thought that users would only ever run into the issue if they explicitly set wrong GOARCH themselves. But that was before I found out that go generate always sets GOOS, GOARCH, etc.

Given that GopherJS only supports one value for target architecture, and never will support other targets (presumably), ignoring GOARCH env var is a valid option too.

But what do we do about GOOS?

neelance commented 7 years ago

But that was before I found out that go generate always sets GOOS, GOARCH, etc.

Exactly. Same here.

But what do we do about GOOS?

There is no issue with that, right? So keep it as is.

dmitshur commented 7 years ago

This PR is no longer needed after gopherjs/gopherjs#605. Closing.