Add App Engine implementations

[rakyll]

No description provided.

[rakyll]

@nf, could we provide appengine related implementations in the appengine package? One thing that makes me do so is the fact that appengine is not go-gettable.

[nf]

It should be go-gettable with "goapp get". But I'm not sure what the question is here. Are you asking if we can put the implementation in the google.golang.org/appengine repo? Or just its own package inside the oauth2 repo?

[rakyll]

Are you asking if we can put the implementation in the google.golang.org/appengine repo?

This is what I'm asking.

I don't want to import "appengine" in the oauth2/google/appengine package (the way goauth2 is currently doing). If I did go build ./... will be broken for a GOPATH that doesn't contain appengine on the top level. I don't want specific build rules or configuration. Is it reasonable to add the appengine related implementation to the google.golang.org/appengine repo?

[nf]

If we put the package in "google.golang.org/appengine" then it wouldn't be available to normal App Engine users, which are the majority.

Why don't you want to use specific build rules? If you make all the files in the appengine package use

// +build appengine

then go build ./... won't fail.

But there's another issue: which app engine import paths to use? There's currently "appengine/..." and "google.golang.org/appengine/...". Eventually I think the former will be deprecated in favour of the latter, but right now we need to support both. To do this, there's another build constraint:

// +build appenginevm

So we'd have files that import "appengine/..." with the appengine build tag, and files that import "google.golang.org/appengine/..." with the appenginevm tag.

Kind of ugly, but it's only ugliness that we have to deal with. To the user it should be transparent.

The one final issue is that godoc.org doesn't display docs from packages with build constraints. This we can fix in the godoc.org codebase.

[rakyll]

// +build appenginevm

Do I need it? The file that imports google.golang.org/appengine/... will build anywhere.

The one final issue is that godoc.org doesn't display docs from packages with build constraints. This we can fix in the godoc.org codebase.

Because build tags exist for different implementations of the same interface. I think my generic appengine Config implementation could have been at the google package, and behaviour should have been be delegated to related implementation on a different file on google package init. Too bad that appengine has a god object called context. It's practically not possible to write Go idiomatic code.

I will give the appengine build tags a try. I'm still not happy with it, because I don't want to ignore it during CI, and I still need to provide some build rules to make it happen.

[nf]

Yeah, I feel you. It's not ideal.

FWIW, there's another god object on the horizon: http://godoc.org/code.google.com/p/go.net/context#Context

[rakyll]

I think my generic appengine Config implementation could have been at the google package, and behaviour should have been be delegated to related implementation on a different file on google package init.

This is still a good idea though, all I need is a type assertion.

FWIW, there's another god object on the horizon

Whoa :(

[nf]

This is still a good idea though, all I need is a type assertion.

You could pass in an *http.Request instead of appengine.Context, then you get type safety and the appengine stuff can be transparently hidden behind a build tag. Thoughts?

[rakyll]

It's unfortunately not possible. Look at this signature: https://github.com/golang/appengine/blob/dd45e2517b8cf23d0caeae3868c4f407a461b7ae/identity.go#L78

Only context knows about which project the caller app belongs to :(

On the other hand, I don't want to end up with having a signature like NewAppEngineConfig(context interface{}, ...). So ugly :(

[nf]

Here's an idea: we provide an "oauth2/appengine" package that uses "google.golang.org/appengine" in files with the constraint !appengine and "appengine" in files with the constraint appengine. That way the public interface shows an appengine.Context that anyone can understand, but it behaves correctly when used with the SDK (command goapp) or deployed to app engine. Thoughts?

[rakyll]

Are you saying we should duplicate the implementation for "appengine" and "google.golang.org/appengine"? That's likely, the code is tiny.

[rakyll]

I created a CL for the discussion: https://codereview.appspot.com/104200044/