tools / godep

dependency tool for go
http://godoc.org/github.com/tools/godep
BSD 3-Clause "New" or "Revised" License
5.54k stars 454 forks source link

Import error for Google Appengine packages #103

Closed matthewbelisle-wf closed 9 years ago

matthewbelisle-wf commented 10 years ago

Google Appengine is a common platform for Go apps, but godep can't save appengine packages because the appengine SDK shims its import paths instead of using normal VCS based import paths. So instead of import "github.com/something/appengine/datastore" it does import "appengine/datastore" and then shims the imports somehow to point to the SDK paths for the package.

https://github.com/GoogleCloudPlatform/appengine-guestbook-go/blob/32112ced8de3e8e02bd64801f99f776527c62f23/hello.go#L8

This is a problem because godep throws an error when it tries to import these,

$ godep save .
godep: cannot find package "appengine/datastore" in any of:
        /usr/local/Cellar/go/1.3/libexec/src/pkg/appengine/datastore (from $GOROOT)
        /Users/mgbelisle/go/src/appengine/datastore (from $GOPATH)
godep: error loading dependencies

which makes sense but is unfortunate. What is the proper way to fix that? This change here works but seems a little kludgy.

https://github.com/matthewbelisle-wf/godep/commit/dba190f14fc83759b74df44e4ab4e7a492eedb1d

wlaurance commented 10 years ago

Speaking for myself, I don't really care to save any appengine packages in my Godeps/.

Do you know if there is a way to tell godep to ignore certain packages?

kr commented 10 years ago

My understanding of appengine is that they provide two separate implementations of their packages for the SDK and the production system. Even if godep were to save appengine/..., it wouldn't work.

If this is right, then we need some way to exclude appengine packages.

It could be built in to godep as in https://github.com/matthewbelisle-wf/godep/commit/dba190f14fc83759b74df44e4ab4e7a492eedb1d, or provided by the user as a flag, for example -omit=appengine/....

kr commented 10 years ago

I am leaning toward an "ignore" or "omit" flag plus a default list of patterns to ignore that is just appengine.

wlaurance commented 10 years ago

What about supporting a .godepignore file with a CRLF delimited list of package names to ignore?

luna-duclos commented 9 years ago

Was this ever implemented ? I've stumbled on this problem as well

fredr commented 9 years ago

+1

fredr commented 9 years ago

@kr the -ignore flag, do you want it to be a comma separated list of import paths, or a regex?

arschles commented 9 years ago

+1 on this. I'd be ok with CRLF delimited

vvakame commented 9 years ago

:+1:

sinmetal commented 9 years ago

:+1:

freeformz commented 9 years ago

@neurogeek you may be interested in this one?

neurogeek commented 9 years ago

Sure, I'll take a look later today.

kalbasit commented 9 years ago

I don't believe that this is necessary any longer as the legacy appengine packages were replaced by the ones that run on both appengine and managed appengine. See this and this for more information.

I am currently using google.golang.org/appengine in my project hosted by Google Appengine (not within a Managed VM). I just omitted vm:true from app.yaml.

neurogeek commented 9 years ago

@kalbasit: Interesting. With new qualification over appengine packages, we definitely don't need this. Having said that, I'm inclined to have the omit/ignore flag anyways, just not appengine-centric, since we could run into this in the future. What do you guys think?

freeformz commented 9 years ago

@neurogeek What are the use cases for an omit flag? My intent is not challenging it, I'm asking to clarify your reasoning.

neurogeek commented 9 years ago

@freeformz Sorry it took me a while to answer, rough past few weeks at work. I don't have an answer for you :) I guess I was thinking on ad-hoc workflows that could benefit from having omit.

I'll close this and if needed, people can issue a feature request.

ronoaldo commented 9 years ago

Sorry to "revive" this but the comment regarding appengine pacakges beign "replaced" is misleading, as the new packages are targeted to Managed VMs, and they provide a layer for compatibility that, once built with App Engine SDK (or with the build flag +appengine), will end up importing appengine/* imports as well. Given the fact that App Engine is now out of beta and the classic packages + SDK will follow the deprecation policies, I think this is a real issue.

If you are working with App Engine and run godep save it will not work. I think that a solution like -omit is nice, but should be also recorded in Godep.json, so it would be recorded in the revision control and help other users working on the same repo/codebase to take advantage of that.

przmv commented 9 years ago

:+1: for -omit. I agree with @ronoaldo, using google.golang.org/appengine packages is not a true replacement for appengine packages.

neurogeek commented 9 years ago

Ok.. Let's reopen and I'll take a better look over the weekend.

przmv commented 9 years ago

Thank you @neurogeek! We really appreciate it! :+1:

przmv commented 9 years ago

Hey @neurogeek! Have you had the chance to look at this issue? Thanks

kalbasit commented 9 years ago

@dsymonds I think the community is quite confused about the fate of the Appengine SDK versus the Appengine packages, can you please clarify?

neurogeek commented 9 years ago

Ok, I took a look and also asked around and yes, Go on Appengine needs to be handled differently. The Appending SDK is the new-ish thing, but Appengine packages aren't going away anytime soon. Lets implement the -omit flag. @kr @freeformz on board?

dsymonds commented 9 years ago

google.golang.org/appengine and friends are intended to be a replacement for appengine and friends. If there's a reason why that's not the case, it's a bug (yes, there's a couple of known gaps that we're working on).

They aren't "replacements" completely, though; any existing app using import "appengine" is going to continue to work, though I think it's fine for various tools to only work properly with the new ones.

freeformz commented 9 years ago

@neurogeek I'd rather not add -omit if we can help it.

@dsymonds Does this mean that users should import "google.golang.org/appengine" instead of import "appengine" and apply any updates necessary?

ronoaldo commented 9 years ago

Currently, I'm using the import "appengine" statements on lots of places, including some tools. The way I'm handling vendor on these apps is by vendoring the vendor tool itself: we have a cmd/godep copy that is, basically, a copy from this fork https://github.com/fredr/godep, which works out of the box on both cases. The problem is that this causes confusion, as I have to instruct any team member or new collaborator to checkout a different copy o godep to make it work.

Also, I just noted that if you run godep with a $GOROOT pointing to the App Engine GOROOT (which I was expecting to work, as the appengine package exists there), it does not. The reason is that go list -e -json cannot find any packages at all, since the App Engine SDK $GOROOT does not have the src/pkg folder; all sources are at the src/ folder directly (i.e., src/time instead of src/pkg/time).

So, to sum up my experiences so far:

I'm just wondering if there is a better/recommended way to work with both tools, and avoid the confusion that I am having. If the only supported way is the new imports, then let it be - I would suggest that maybe it worth adding this to the documentation.

The -omit flag may not solve the problem; maybe a special case for appengine/* would help everyone, even if transitivelly until the new packages are more widelly addopted.

dsymonds commented 9 years ago

@freeformz: If the newer packages work for you, sure. They'll be the best supported ones in the long term.

Switching to google.golang.org/appengine should not entail rewriting a lot of code. 95% of the APIs are exactly the same, so it should only be a matter of rewriting the import statement; the google.golang.org/appengine README has a sed command to do that very quickly.

freeformz commented 9 years ago

@dsymonds @ronoaldo For reference that README is here right? https://github.com/golang/appengine

przmv commented 9 years ago

@freeformz @ronoaldo https://github.com/golang/appengine#2-update-import-paths

sed -i '/"appengine/{s,"appengine,"google.golang.org/appengine,;s,appengine_,appengine/,}' \
  $(find . -name '*.go')
ronoaldo commented 9 years ago

@dsymonds @freeformz @pshevtsov thanks. I am happy that the recommendation is then to move to the new packages, which hence are compatible with godep, and that the new App Engine packages are long-term support.

freeformz commented 9 years ago

Awesome. I'm going to close this issue out again then as I think it documents what people should do.

vsiv commented 8 years ago

@freeformz but google.golang.org/appengine itself imports appengine. And hence godep consistently complains "appengine" package is missing. Any help?

Thanks.

vsiv commented 8 years ago

@freeformz just found out the above error is pertaining to https://github.com/tools/godep/issues/348