-
Notifications
You must be signed in to change notification settings - Fork 656
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
Support the new package binary file format (coming in Go 1.7) #305
Comments
Thanks for letting me know. In my opinion they should just gzip the old format, gives similar ratios. |
FYI, golang/go@7538b1d has landed. gocode no longer works with go tip unless |
Will take a look at it this weekend. |
A workaround for editors that uses autobuild (vscode in my case):
|
Or don't use Go master version? What's the rush? Use Go 1.6. P.S. Already working on binary format parser. |
Working on a CL for go1.7, so... but that's great to hear. |
Thank you! |
With go tip and gocode including de6353c should I still be seeing a panic?
|
Tried nuking $GOPATH/pkg as well, didn't help. |
No, it's a bug. Is it happens on Go tip if you run gocode's |
I get a failure from test.0032 and when trying to use gocode in my editor. |
Ok, I'll take a look at latest Go master. Maybe they've changed something. |
This failure is probably also relevant: #350. But sorry guys, super busy this weekend. In general I will make sure it works when 1.7 is closer. Maybe when they start marking release candidates. |
It seems that gocode is better to use codes from cmd/compile/internal/gc/bimport.go . maybe keeping compatibility for older version seems to be too difficult for me. |
Why does gocode need to parse export data itself? Is there functionality missing from go/importer? |
Well, first of all I can parse it directly to the format I use, which is historically a "go/ast" based format (in the early days it was possible to parse .a file by actual Go parser with some preprocessing hacks). Secondly "go/importer" has some weird logic when it comes to getting the package files. It asks you to pass in the package path as seen by a Go source file and in theory it should allow customization of a lookup function. But in practice it doesn't support non-default lookup functions: https://golang.org/src/go/importer/importer.go?s=807:862#L14 So I simply can't use it for gocode, because gocode has |
Sorry, I don't use gb so I don't understand the problem. gb still uses .a files like normal cmd/go builds, right? You can use importer.Default().Import("/path/to/goroot/pkg/linux_amd64/fmt") for example to load /path/to/goroot/pkg/linux_amd64/fmt.a. |
BTW, I have a mostly working prototype that uses go/importer to directly create decls for imported objects, bypassing creating the go/ast intermediate form. I just need to fix two last test failures. |
Well, hm, maybe. But I just like to do things my own way. Although, if you can make it work I'll take a look. |
A temporary workaround until gocode supports the new export format is compiling go with this patch:
https://gist.github.com/OneOfOne/c4bc9eb3206e41ab133ae1d3e70194b0 |
I've ported all importer changes from Go's master to gocode. All gocode's tests pass, but if you have any issues with new binary format and gocode, please report them here. A friendly reminder: make sure before or after updating gocode with |
@nsf I can confirm it's working. |
FWIW, I uploaded my modified version of gocode as github.com/mdempsky/gocode. It's dramatically smaller (~1600, down from ~6500) mostly due to using go/types and go/importer. It also doesn't attempt any caching, but still seems fast enough for interactive use. It currently only supports cmd/go builds, because I haven't yet grok'd how gb works. |
@mdempsky it looks interesting, but I can't really test it out in "production", because we use gb. And as a result, I can't compare it to current gocode version. Test coverage in gocode is pretty bad and you can't rely on it, the only way to find out is to try using it. Hence, I can't just take your patch and apply it. But I will definitely give it a try if you implement gb support. And if you eventually want to step over and maintain gocode for the rest of your life, I totally don't mind. I don't have time for it anymore. In case if someone is worried - don't. I will keep maintaining gocode for the whole Go 1 lifecycle, because it does a pretty good job at what it was made for, at least for me anyways. If there is a better alternative - great, less code for me to maintain. |
Confirmed to work against master. While I'm a happy user of the current version, @mdempsky if you're planning on maintaining your branch and need help testing it, I rely on gocode quite heavily and would be glad to help test. |
@nsf Thanks for great works! BTW, I found two bugs. Maybe it's related new binary format. |
@zchee a new issue would be nice |
@nsf Thanks, I'll create a new issue later. |
@zchee I also probably won't take a look at it until the weekend. Well, depends on complexity. |
@mattetti I do plan on maintaining my version. If you find any issues, feel free to report them under mdempsky/gocode (so we don't spam @nsf :)), and I'll take a look. I'm keeping the interface backwards compatible so it should be easy to switch back and forth between versions of gocode if there are problems (just reinstall and restart). |
Update: I've added gb support to mdempsky/gocode. It hopefully works automatically and without any configuration: if gocode notices that you're editing a file within DIR/src or DIR/vendor/src and DIR is not a member of GOPATH, then it will also check DIR/pkg for .a packages compiled by gb. Note: this is in addition to and after normal GOPATH .a package searching, if any. Experimentally seems to work with gb's example-gsftp sample project. |
A note to others, in addition to the above I also had to run |
|
Well actually, after running |
I have no explanation for that scenario. |
Hi. I'd like to chime in here and offer some observations and point out an opportunity. First, huge, huge thanks to @nsf (and contributors to gocode) for creating gocode and maintaining it all this time. I've been following its development since 2012, using it in my projects, and using it for all my Go writing autocomplete needs as part of GoSublime. Basically every day, thousands of times. So thank you. I use Go and care about it a lot, so I want the best future for gocode. I really hope the rest of my comment is constructive and helpful – at least that's my intention. Please consider the following observations, and feel free to correct me if I get some details wrong. I know that @nsf has said in the past that gocode could really use a rewrite, at least in theory.
Source: #307 As he pointed out, starting with Go 1.6, and continuing with 1.7 and 1.8, there are multiple internal changes done and planned to the binary format for packages. The Go compiler team wants to improve things in that area. In order to be able to do that without breaking Go tooling, they've provided packages such as go/types, go/importer, etc., with stable APIs that enable access to the underlying data. This allows them to break internal format details, updating those packages appropriately to keep them functioning as before, and iterate forward. The current gocode, primarily for historical reasons (source), still tries to parses the binary format itself without going through the stable APIs, and that's what's causing many tip-related breakages. As I understand, @mdempsky was motivated to try to fix gocode for tip since many people (including the Go team, who often use tip) depend on it, and he didn't want it to break as him and his team were introducing internal breaking changes to the binary format (but the stable APIs continue to work).
Source: #305 (comment) I know that @nsf has started the project long ago and used C-style snake case for variable names, etc. It's completely valid to do that since it's his project and it's not a part of a public API. However, given that this project provides value to so many people, who may want to contribute, following the Go idiomatic style would make it easier for other people to do that. I see that @mdempsky has already done the work to change the internal code style to be more idiomatic, see mdempsky@e117c26. @nsf made a good point that:
But I see that @mdempsky has completed gb support as of May 18:
Source: #305 (comment) And I know @nsf has said in the past he wouldn't mind if others helped out with maintaining this project. @mdempsky has also said he plans to maintain his current version:
Source: #305 (comment) So this looks like a great opportunity to me! Since mdempsky/gocode version has gb support now, @nsf should be able to give it a try in production and see if that version works without issues (or report any). If that goes well, perhaps all the improvements from mdempsky/gocode version can be merged into nsf/gocode, so we have the best of all worlds in one. It really looks like it could be win-win for everyone. At least from what I can tell, but of course it's possible I'm overlooking some details or I'm wrong. Please let me know if so. I hope this comment doesn't come out rude; I really just want to point out what looks (from my perspective) like a very positive opportunity to help make this project better in the long run. Thanks everyone! |
The reason why I don't like relying on provided semantic analysis libraries is because gocode works with invalid go code. And what happens if these libs fail to handle some case which is 100% invalid from their point of view, but for gocode it would be nice to handle it. Do I fork the libs? As for me testing the alternative gocode, sure I can do that, but it's unlikely that I'm merging it anyway. I can also put a message on README.md so that users try that other version of gocode, but the problem is - I don't think that many users actually read README.md. You see gocode is not an end user product, there is a ton of editors that rely on gocode being as it is and it puts a lot of pressure on me when it comes to global changes like that. I can't just throw in a new version of gocode silently. Then if it fails what do I do? Revert everything back? I would rather maintain it as it is. But ok, I'm gonna try this new version of gocode when I'll be working on gocode this weekend (I need to rewrite the way package resolving are done due to the bug). And see if it performs well. Will also try some artificial but realistic cases I have in mind. |
[UPDATE] FIX: gocode is panicing for me:
|
What exactly in my style prevents users from contributing? They can't read snake casing? I will not turn down code because it uses camel casing or pascal casing or whatever you want to call it. I see no logic in your statement at all. If I said to somebody that I will not accept your patch because it uses incorrect styling - I'm sorry. But IIRC I never said that to anyone.
Well, they can write the tool for themselves, they are well paid to work on Go. Since 1.0 I always tell gocode users - don't use tip, use a stable Go release version, that's how I maintain gocode. If somebody is willing to submit a patch that backports the features from tip without breaking anything - great (thanks @zchee). But rewriting gocode to achieve what a few dozen people want?
In that rewrite post I talked about main pain points of gocode from a user point of view. None of them are solved by a fork. Sure with "go/types" you can import packages with a custom importer, but there is none of that in the fork. Tools like gocode need to parse the source code tree instead of relying on binary compiler-dependent formats. While this path has some challenges, it's the right thing to do in the long term. We need not only always relevant autocompletion results (possibly before compilation is performed), but also additional info that is available only in the source code - the documentation. Can "go/types" preserve documentation together with semantic info? I don't know, maybe. But I would rather keep doing things my own way. Autocompletion doesn't require that much of a semantic analysis. So, what even to discuss here? |
I didn't mean it prevents people from contributing, I said that it's easier to contribute to a project that follows idiomatic Go style because it's easier to read/understand the code, and to make changes to it without being distracted by extraneous style concerns. My point is, given two equivalent code bases:
IMO the former is easier to contribute to (to some degree, it may not be a large amount).
I am not an expert in this area, but your argument sounds valid to me. @mdempsky, what are your thoughts on that? |
@sethgrid Yeah, I also have this issue, so I send 2735323 patch. @shurcooL This comment is not a criticize to you. and I totally understand you said means, why need idiomatic Go style. When #352, I found a bug that related new binary format. After that, I found causes point. solved the problem #357. If you pointed out the C style naming is not Go style, did it interfere with my understanding? I think no. It might be not Go style, might be easier to read someone. |
@zchee just to be clear, this is not my style at all, I have no preference at the moment and can happily write javascript with camelCasing. Changing gocode to snake casing was a bad idea, but my point is that it doesn't affect anyhting really and changing it back brings zero benefits. |
@nsf Yeah, I know :)
Yes, Maybe it's not you style at now, but still you write gocode use snake_case with any reason(in this case, "it doesn't affect anyhting really and changing it back brings zero benefits."), so I think it's nsf's gocode project coding style. I mean, In C++, Designed by Bjarne Stroustrup. So we should respect his and C++ language rules? yes. but we should respect his coding style? no. Ask "Why you use this style?". ok, maybe project author will answer. I want to say If project author has any coding style per project with any reason(or reason that do not want to change style), we should respect it. |
Probably nothing to do for this issue until Go 1.6 is out but I thought I would file the issue as a reminder anyway.
A new binary export format is coming with Go 1.6 (disabled by default). See the following commit for more details: golang/go@ae2f54a
As I've been experimenting using Go built from tip I've noticed that gocode panics when the new export format is used.
The text was updated successfully, but these errors were encountered: