Skip to content
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

appengine build tag is not set in new go111 runtime #334

Closed
mtraver opened this issue Oct 17, 2018 · 6 comments
Closed

appengine build tag is not set in new go111 runtime #334

mtraver opened this issue Oct 17, 2018 · 6 comments

Comments

@mtraver
Copy link
Contributor

mtraver commented Oct 17, 2018

App Engine just launched a new Go 1.11 runtime. The go111 runtime uses the stock Go toolchain and does not set the appengine build tag.

I encountered this when using AppEngineTokenSource, which panicked because appengine_hook.go wasn't built. Does this code need to be behind a build tag? If someone tries to use App Engine specific features when not on App Engine failure should be expected.

cc @sbuss, lead on go111 runtime

@bradfitz
Copy link
Contributor

I suspect users won't care about about the App Engine implementation details (build tags, gvisor, etc) and will just think, "I'm using App Engine, so I should use the AppEngineTokenSource". So we should make it work automatically somehow.

I await guidance from @sbuss.

@sbuss
Copy link
Contributor

sbuss commented Oct 18, 2018

I suspect users won't care about about the App Engine implementation details (build tags, gvisor, etc) and will just think, "I'm using App Engine, so I should use the AppEngineTokenSource".

Agreed; the easiest and most unsurprising way forward is to not hide AppEngineTokenSource behind the appengine build tag. If a user tries to use it when they're not on App Engine, a failure should be expected.

@bradfitz
Copy link
Contributor

bradfitz commented Oct 18, 2018

Agreed; the easiest and most unsurprising way forward is to not hide AppEngineTokenSource behind the appengine build tag. If a user tries to use it when they're not on App Engine, a failure should be expected.

That's already what's happens. This issue as I understand it is about Go 1.11 on App Engine not using build tags, so they're on App Engine and trying to use AppEngineTokenSource but getting a panic/error that AppEngineTokenSource is only valid on App Engine. That seemed non-intuitive for users not knowing about Go 1.11 vs Go 1.earlier using different build systems & sandboxes.

Or do I misunderstand? Does Go 1.11 on App Engine define the appengine build tag? Or does it set different build tags like the old(?) appenginevm thing?

@sbuss
Copy link
Contributor

sbuss commented Oct 18, 2018

Sorry, that was my misunderstanding. @mtraver clarified to me that the actual issue is that appengineTokenFunc is behind the appengine build tag, which go1.11 on GAE no longer sets. So this init hook never gets run in the go1.11 environment.

Does Go 1.11 on App Engine define the appengine build tag?

no

Or does it set different build tags like the old(?) appenginevm thing?

no, it sets no build tags other than what go build sets by default (eg the language version tags)

@bradfitz
Copy link
Contributor

Is there a reasonable way to make AppEngineTokenSource work for Go 1.11?

Or are the APIs it used no longer available in the gvisor sandbox?

@sbuss
Copy link
Contributor

sbuss commented Oct 18, 2018

AppEngineTokenSource does work with Go 1.11, but the code in https://github.com/golang/oauth2/blob/ef147856a6ddbb60760db74283d2424e98c87bff/google/appengine_hook.go doesn't run because it's behind the appengine build tag. Removing the build tag makes everything work as expected; confirmed by @mtraver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants