search

joefitzgerald / go-plus

An Enhanced Go Experience For The Atom Editor
https://atom.io/packages/go-plus
Other
1.51k stars 129 forks source link

GOPATH with multiple directories (possible regression) #30

Closed mreynolds closed 10 years ago

mreynolds commented 10 years ago

For some reason, I can't copy the output of the go-plus error window, but it's basically telling me that my GOPATH ("/Users/mreynolds/Desktop/Projects/:/usr/local/opt/go") doesn't exist, which is correct, since it's a PATH, not a single directory.

I'm using 1.0.10 with Brew installed go. I haven't tried manually overriding my env vars in settings.

mreynolds commented 10 years ago

Cancel. I copied what the available package was. I'm running 1.0.1. Sorry!

mreynolds commented 10 years ago

OK, this time, with 1.1.0, I'm still seeing the same issue. Bah. Sorry for the churn.

mreynolds commented 10 years ago

So, I manually set the Gofmt, golint, etc paths and now the JS debugger is popping up complaining about : Window load time: 1045ms /Users/mreynolds/Applications/Atom.app/Contents/Resources/app/src/window-bootstrap.js:18 fmt: error launching command [/usr/local/go/bin/gofmt] – Error: spawn ENOENT – current PATH: [/usr/local/opt/coreutils/libexec/gnubin:/usr/local/opt/gnu-sed/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/opt/go/libexec/bin/] gofmt.coffee:50 vet: error launching vet command [/usr/local/go/bin/go] – Error: spawn ENOENT – current PATH: [/usr/local/opt/coreutils/libexec/gnubin:/usr/local/opt/gnu-sed/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/opt/go/libexec/bin/] govet.coffee:49 lint: error launching command [/usr/local/opt/go/bin/golint] – Error: spawn ENOENT – current PATH: [/usr/local/opt/coreutils/libexec/gnubin:/usr/local/opt/gnu-sed/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/opt/go/libexec/bin/] golint.coffee:49 syntaxcheck: error launching command [/usr/local/go/bin/go] – Error: spawn ENOENT – current PATH: [/usr/local/opt/coreutils/libexec/gnubin:/usr/local/opt/gnu-sed/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/opt/go/libexec/bin/] gobuild.coffee:73 lint: execvp(): No such file or directory golint.coffee:55 fmt: [/usr/local/go/bin/gofmt] exited with code [-2] gofmt.coffee:59 syntaxcheck: [/usr/local/go/bin/go] exited with code [-2] gobuild.coffee:82 lint: [/usr/local/opt/go/bin/golint] exited with code [-2] golint.coffee:58 vet: [/usr/local/go/bin/go] exited with code [-2] govet.coffee:58 2 Uncaught Error: spawn EACCES

mreynolds commented 10 years ago

Found why the debugger popped up, it was an incorrectly set Go Executable Path (I set it to a dir, it wants the Go exec, fixed, but it should probably more gracefully check if it's the right thing?).

mreynolds commented 10 years ago

So, given this ( http://golang.org/doc/code.html#GOPATH ) : Note that this must not be the same path as your Go installation.

... I went ahead an unfucked my previous addition of that to GOPATH. However, now, goplus doesn't find gofmt in the bin dir for the Brew installed path. I'll go try removing it and reinstalling it to see if that works.

joefitzgerald commented 10 years ago

Confirmed regarding the issue you reported with a GOPATH containing multiple elements. We are not splitting the GOPATH correctly for that warning. It should not actually affect your ability to do syntax checking, but needs to be fixed as it could become annoying :)

mreynolds commented 10 years ago

So, gofmt appears to be installed with Brew here : brew --prefix go /usr/local/opt/go

with gofmt installed : ls $(brew --prefix go)/bin -la total 8 drwxr-xr-x 4 mreynolds admin 136 May 11 10:09 . drwxr-xr-x 7 mreynolds admin 238 May 14 15:46 .. lrwxr-xr-x 1 mreynolds admin 17 May 11 10:09 go -> ../libexec/bin/go lrwxr-xr-x 1 mreynolds admin 20 May 11 10:09 gofmt -> ../libexec/bin/gofmt

So, with no overridden settings in GoPlus, it's still looking for gofmt in /usr/local/go/bin/gofmt ( doesn't exist on my machine, not sure where it's getting that path?).

While I could just override, I'm going to figure out how to debug this in Atom and see about setting the variables correctly. I can get my setup to work fine on the CLI, but will try and figure out why Plus doesn't pick up the env vars correctly.

mreynolds commented 10 years ago

Tried setting GOROOT, and despite the reference here ( https://github.com/joefitzgerald/go-plus/blob/master/spec/gofmt-spec.coffee ) it doesn't appear to be honored in the runtime.

I'd love to use the installer, but frankly I'm willing to spend the time to help Plus do this the way Go does, rather than relying on the defaults provided by one install mechanism.

Worst case, I can just configure Plus manually, since that seems to mostly work.

mreynolds commented 10 years ago

So, I think I found the culprit ( https://github.com/joefitzgerald/go-plus/blob/master/lib/gofmt.coffee#L43 ). That call gets the default if the variable isn't set. This doesn't follow the rest of the functionality that buildGoPath does, by honoring the env var if it exists ( https://github.com/joefitzgerald/go-plus/blob/master/lib/dispatch.coffee#L168-L177 ).

Honestly, beyond this, I'm at a loss about how to fix it. Maybe wrap calls to the standard config settings so that they obey the "env var" flag?

mreynolds commented 10 years ago

Is there an easy way for me to fork this and test in Atom? I've never tried that and don't really know where to start, but I'm happy to help since this is important to me.

joefitzgerald commented 10 years ago

Well there's a few things to address in response to your questions:

joefitzgerald commented 10 years ago

Also, to your question about the defaults - they reflect the locations the OS X package installer uses: https://github.com/joefitzgerald/go-plus#defaults.

mreynolds commented 10 years ago

https://github.com/joefitzgerald/go-plus/blob/master/lib/gofmt.coffee#L43 is the path to gofmt - it is not GOPATH, although tokens are replaced in it - for example a $GOPATH variable will be substituted (if it exists) - i.e. $GOPATH/bin/goimports

Debugging via the console, it looked like it wasn't using ${GOPATH}, which I have set. It appears to be using the default, since I haven't specifically overridden the gofmt path in the settings. However, given my lack of knowledge, I need to confirm this.

I have no intention of using GOROOT since I don't need it for normal CLI work ; I was just trying it out to see how it affected the behavior of go-plus. I do run Atom exclusively from my shell, so I'm expecting it to use my env vars, which seems to be the primary issue I'm running into. I'll do some more digging.

mreynolds commented 10 years ago

FYI, I ran apm test with no changes after forking the build and they're failing. Is that expected?

mreynolds commented 10 years ago

OK, it looks like the tests ignore the env vars as well, and expect go to be installed in the installers place ( https://gist.github.com/mreynolds/105f472c99561d242eac ). Honestly, I'm not going to use the standard installer (no one I know does), and I don't want to fight you as the maintainer. Is this something you want me to try and fix?

joefitzgerald commented 10 years ago

No fighting going on. Just trying to explain what's going on. Try running clear ; echo "GOROOT is set to : '$GOROOT', GOPATH is set to '${GOPATH}'" ; GOROOT=/usr/local/opt/go apm test

mreynolds commented 10 years ago

Sorry, strong wording when I didn't mean it! :) Just don't want to cause trouble.

https://gist.github.com/mreynolds/dd4e24a05852f4b65dff

Still getting failures.

mreynolds commented 10 years ago

I think I see part of the problem. Cleaning up path names is part of it, so I think I can change the buildGoPath function to use the File objects and the sync path name function to return a "cleaned" path. Trying that out now.

mreynolds commented 10 years ago

So, I'm trying to use the path module ( http://nodejs.org/docs/v0.4.9/api/path.html ) to cleanup the path using normalize, but something else isn't working. I'm having trouble understanding what the token replacement for GOPATH and other things does ( or, really, why it's needed ). Do you often get gofmt not working correctly because of tokens in the path name? Or is this required from the configuration? Sorry, just having trouble parsing my way through this.

mreynolds commented 10 years ago

OK, I think I get it now. Can I recommend that shell variables that are un-expanded simply not be expanded as part of your toolkit? I can't find an easy way to do this in Node.js so far (I'm guessing you'd have fire up a shell to handle all the permutations, since $HOME and ${HOME} both work, and I would expect other shell operations if you're referencing (BA)SH variables), and it seems like people who'd want to do this should/could simply setup their CLI environment correctly to pass in the right (already expanded) variables.

mreynolds commented 10 years ago

After further review, I'm just going to write up my recommendation and ask you to review the pull request! Sorry for the wall of text.

joefitzgerald commented 10 years ago

Closing in deference to https://github.com/joefitzgerald/go-plus/issues/32.