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

Don't default to hardcoded absolute paths #59

Open
mtorromeo opened this issue Oct 16, 2015 · 6 comments
Open

Don't default to hardcoded absolute paths #59

mtorromeo opened this issue Oct 16, 2015 · 6 comments

Comments

@mtorromeo
Copy link

Since the dawn of time Unix supported searching for binary in $PATH. Please just use racer as the default value for the utility instead of /usr/local/bin/racer which is still worse than the standard /usr/bin/racer.

Also maybe also use /usr/src/rust/src? Any official package, when provided by a Linux distro, would use this path anyway.

Thanks!

@alkama
Copy link
Contributor

alkama commented Oct 16, 2015

Hello,
Just a few notes.

Of course, that's what we planned first!
In fact, initially, we wanted to rely on not having any settings and just silently use racer (from the PATH) and RUST_SRC_PATH environment variable (that the users would have setup on their system).

Sadly, it turned not that simple, at least for OSX, and would have required a lot of users fiddling.
Last time I checked, the environment variables that Atom sees is not the users one, but the systems one (on OSX). I think it comes from the fact that app launching is done by launchd (but I may be wrong). For exemple, last time I checked, setting a RUST_SRC_PATH in your bashrc or profile wont make it available for Atom to use and see.
To have it happen, the user would have to use a line like launchctl setenv RUST_SRC_PATH [the path] and they also would have to open a terminal after each reboot before launching Atom for that line to execute.

In the end, we turned to settings, providing explicit pathes that anyone can change.
We used default values that made sense back when rust and racer were evolving quickly and were not available as distro packages and were also pretty common on OSX.

About /usr/src and /usr/bin, on OSX El Capitan, this path is even protected by "SIP" and even root cannot alter it. So racer and rust source have no chance to ever install there. And I dont even mention windows.

And if I focus on linux only, I highly doubt that there's a common sensible place that all distro use to install things either. Not even talking about when compiling things by hand.

Lastly, from experience, we had a LOT of issues where users put directories instead of the full path in the settings field for racer. Even when showing explicit requirements, users get confused. I'm a bit scared of what might happen if they dont even SEE a real path there.

So hardcoded default values feel just as sensible (providing explicit requirements) as system guessed paths that wont ever work for a lot of users.

Anyway, if you're confident it's a better option, then you're welcome to change it and provide a pull request.

@mtorromeo
Copy link
Author

I completely undestand. I still think there's a better solution though.

For the racer tool you could use a 2 steps process where you first run which racer and if successful use this path or fallback to /usr/local/bin/racer. (BTW, OSX doesn't have /usr/local/bin in the PATH?)

Same for RUST_SRC_PATH. If the env variable is set use it. If not, use /usr/local/src/rust/src.
Even better you could detect the OS at runtime and default to /usr/local/src/rust/src on OSX and /usr/src/rust/src as fallback.

@edubkendo
Copy link
Owner

We would happily consider a robust pull request.

@Luthaf
Copy link

Luthaf commented Oct 22, 2016

This is a grip point for me because I am synchronizing my atom setting between an OS X machine and a Linux machine, and hard coded path do not play well here ...

I could give it a try, but I am not sure I understood all the constraints here.

What is the problem with having racer as the default path, and letting the users override it to ~/.cargo/bin/racer or /usr/local/bin/racer as they want? This package would work with no change for previous users (the setting are conserved I believe). For new users, this would work out of the bow for people launching atom in command line on OS X (I do this) and on Linux, and others would need to set the path they need.

I think this is better than relying on /usr/local/bin/racer which is the brew installation path, because even on OS X people could use cargo install to get racer.

@bronson
Copy link

bronson commented Nov 1, 2016

last time I checked, setting a RUST_SRC_PATH in your bashrc or profile wont make it available for Atom to use and see.

Happily, this changed a year ago: http://blog.atom.io/2016/03/17/atom-1-6-and-1-7-beta.html#environment-patching-on-os-x

It's now common for Atom packages to read from the environment.

I'll try to find time for a PR. Very much hoping someone can beat me to it. :)

@Luthaf I have the same problem: my Atom config is shared between Mac and Linux. Everything works except for this package.

@bronson
Copy link

bronson commented Nov 1, 2016

Looks like the PATH fix is already written: #60

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

No branches or pull requests

5 participants