Support protobufs under gopherjs

[adg]

Like App Engine, gopherjs does not support package unsafe.

The work-around on App Engine is to provide a slower, reflection-based version of the protobuf codec that is only built when the appengine build tag is specified.

I propose adding the js build tag (specified when building Go code with gopherjs) to the pointer_reflect.go and pointer_unsafe.go files alongside the appengine tag.

Any objections?

cc @dsymonds @robpike

[bradfitz]

I think js is overly specific. Can't the tag just be safe if its meaning is "don't use unsafe"?

[adg]

@bradfitz that then becomes a new convention. I'm not opposed, but I would like to see App Engine adhere to the convention if we go down that road.

[robpike]

I side with @bradfitz

[adg]

@bradfitz @robpike so are you both suggesting we create a new convention of the safe build tag?

[adg]

And that we add support for it wherever appropriate?

[dsymonds]

The "apppengine" build tag doesn't mean "avoid unsafe". It means code that is suitable for running on App Engine. Sometimes that means "version of code that doesn't use unsafe". Sometimes that means "version of code that doesn't use syscall". Sometimes that means "version of code that uses the urlfetch API". There's a panoply of meaning, but they are all about the App Engine environment specifically.

I don't know anything about GopherJS. Is avoiding "unsafe" the only aspect for which one might want a build tag? Or is there going to be other things?

[adg]

I don't know anything about GopherJS. Is avoiding "unsafe" the only aspect for which one might want a build tag? Or is there going to be other things?

I don't know. But is that relevant here? The distinction between pointer_unsafe.go and pointer_reflect.go is that one uses unsafe and the other does not. That's the concession being made for appengine (why does it have special status?).

I'd like to either make the same concession for GopherJS or establish a new convention that we can use in its place. @robpike and @bradfitz seem keen to establish a new convention.

I don't care either way. I am happy to do the work either way. I just need a decision.

[dsymonds]

If we're talking about a new build tag that's going to be used as a new convention, the semantics matter. App Engine isn't going to follow along with a simple "unsafe" vs. "safe" distinction since (a) that only captures one part of the environmental difference, and (b) isn't necessarily a permanent or fundamental distinction.

I would much rather see "js" or "gopherjs" or whatever than trying to reform the world and make "safe"/"unsafe" happen, plus all the other aspects that would need to be split out.

[bradfitz]

App Engine doesn't have to follow along. We don't have to change the appengine build tag.

I'd like to just add the safe tag for protobuf where it means "don't use unsafe". That doesn't mean it's a global convention. We can test the waters with protobuf. We can use js only when it turns out that gopherjs needs something very specific. I want to stop adding build tags for every environment that can't deal with unsafe.

[adg]

cc @neelance @shurcooL for thoughts on gopherjs setting a safe build tag.

[neelance]

A safe tag makes sense to me, especially since the behavior of unsafe code is not fully specified.

[dmitshur]

I agree that js build tag is specific to GopherJS, just like appengine build tag is specific to Google App Engine. That means adding js build tag to pointer_reflect.go (and its negation to pointer_unsafe.go) would resolve this issue, titled "Support protobufs under gopherjs". But if there's another project or or architecture or environment or operating system in the future that also does not support unsafe, then it'll also need to be added.

Creating a new convention of using safe build tag to opt into code paths that avoid the use of unsafe package sounds okay to me, and it definitely solves the problem at a more general level. Then, GopherJS, GAE and anyone else can simply use safe tag. Otherwise, we're starting to see temptation for non-GAE to use appengine build tag just to opt in to safe versions of code. (Side note, I've also seen temptation to use nacl build tag to opt into "pretend this is a system that doesn't support os/exec.)

In short, I agree with this (and everything else that was said above):

I want to stop adding build tags for every environment that can't deal with unsafe.

Based on @neelance's response above, it sounds like he's happy to have GopherJS use the safe build tag by default, since GopherJS does not support unsafe package, if this build tag is implemented here.

[dmitshur]

I don't know anything about GopherJS. Is avoiding "unsafe" the only aspect for which one might want a build tag? Or is there going to be other things?

@dsymonds To answer your question, from what I know about protobuf, I don't expect there to be anything else needed to achieve working functionality (with about 80-95% certainty).

[dsymonds]

@shurcooL: My question isn't protobuf-specific. It's asking if GopherJS might want to select between files for other reasons, in places other than protobuf. Might some library somewhere want to switch out code when it is being built by GopherJS? Is that going to be only ever due to using "unsafe"?

[dmitshur]

@shurcooL: My question isn't protobuf-specific. It's asking if GopherJS might want to select between files for other reasons, in places other than protobuf. Might some library somewhere want to switch out code when it is being built by GopherJS? Is that going to be only ever due to using "unsafe"?

GopherJS compiles for a pretty specific GOARCH value of js, so if a library is interested in doing something specific for that architecture (differently, from, for example amd64 or arm64 values of GOARCH, or linux/windows/darwin values of GOOS) then it can use that as a build constraint.

To give a concrete example of that being done, take a look at this package's 2 .go files:

https://github.com/goxjs/websocket

It has // +build !js constraint selecting "golang.org/x/net/websocket" package for providing a WebSocket client on desktop OSes, and a // +build js constraint detecting running in browser and implementing the WebSocket client via browser's APIs (via another package).

If you want a more complex example with 3 sets of build constraints (targeting desktop OSes, mobile OSes, and browsers), you can look at https://github.com/goxjs/gl.

[dsymonds]

Oh, so there's already "js" as a build tag for this? Sounds like we should just be following along with that.

[dmitshur]

Oh, so there's already "js" as a build tag for this?

Yes. It's explained here. It's not necessarily specific to GopherJS, but rather to the fact its executable output is JavaScript code (that can run in environments that can only execute JavaScript code).

[bradfitz]

Okay, I'll cave. That's a satisfactory argument for using js.

[bradfitz]

(... but I'd still prefer safe one day)

[awalterschulze]

This might also be useful
https://github.com/gopherjs/gopherjs/blob/master/doc/packages.md

[adg]

So, @dsymonds should I prepare a pull request? Or is there a better way to contribute to this repo? (It has no CONTRIBUTING file.)

[dsymonds]

Make it internally and I'll push it out. This repo isn't really geared for taking contributions.