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

Automatic source discovery with rust's new rust-src package #595

Closed
Diggsey opened this issue Aug 26, 2016 · 19 comments
Closed

Automatic source discovery with rust's new rust-src package #595

Diggsey opened this issue Aug 26, 2016 · 19 comments

Comments

@Diggsey
Copy link
Contributor

Diggsey commented Aug 26, 2016

Rust now has an "official" location to install sources to (<toolchain root>/lib/rustlib/src/rust). Although there is no separate installer for this new component, it can be installed via rustup.

edit: scratch everything that was previously here, I'm an idiot 😞

It's as simple as racer calling rustc --print sysroot and locating the source that way. This will then seamlessly work with rustup.

@jwilm
Copy link
Collaborator

jwilm commented Aug 26, 2016

Cool! Presumably this is aware of the active toolchain in the current directory given that it uses rustc from user's PATH?

@Diggsey
Copy link
Contributor Author

Diggsey commented Aug 26, 2016

@jwilm rustup proxies the rustc command through to the "real" rustc for the active toolchain, so rustc --print sysroot will print the toolchain root correctly.

@jplatte
Copy link

jplatte commented Aug 27, 2016

So right now as an arch linux user I unfortunately have to places to search for rust sources, $(rustc --print sysroot)/src/rust/src, and $(rustc --print sysroot)/lib/rustlib/src/rust/src. Which one has the sources depends on how I installed rust-src (via rustup vs from AUR). Before we hardcode the former in racer, could we wait to see if either the rustup component or AUR package install path can be changed so they are both the same? I've already asked on AUR, let's see what the package maintainer thinks: https://aur.archlinux.org/packages/rust-src/#news

@Diggsey
Copy link
Contributor Author

Diggsey commented Aug 27, 2016

@jplatte I would expect that if racer cannot find the source via this method it would fall-back to the old behaviour, so it shouldn't cause any problems.

@jplatte
Copy link

jplatte commented Aug 27, 2016

Well yes, but I'd really like to get rid of the need to set RUST_SRC_PATH no matter if I install rust-src via rustup or AUR. And I'm pretty sure it won't be very hard to find a consensus about where the standard install location for rust-src should be.

I think we should make racer as much of an out-of-the-box experience as possible for as many users as possible (without adding unnecessary complexity though, which is why I proposed to wait for a standard install location to surface instead of trying multiple locations).

@sickHamm
Copy link

sickHamm commented Aug 28, 2016

I don't think there will be a standard install location for linux distribution's rust source package. (even, windows, mac), but the <toolchain root>/lib/rustlib/src/rust is rust standard now rust-lang-deprecated/rust-buildbot#102 , racer should support.

and explicit setting RUST_SRC_PATH should be higher priority.

@Diggsey
Copy link
Contributor Author

Diggsey commented Aug 28, 2016

I've put in a PR

@brson
Copy link

brson commented Aug 29, 2016

cc @jonathandturner

@jwilm
Copy link
Collaborator

jwilm commented Sep 6, 2016

@Diggsey @brson would it be possible to factor a small library out of rustup.rs which provides an API for getting current toolchain info? Would be nice to have such a thing versus calling the rustc wrapper.

@brson
Copy link

brson commented Sep 6, 2016

@jwilm Yes, it would. rustup is factored as a series of libraries exactly for your use case, but we haven't put much effort into making it usable as a library. It would be relatively easy to clean it up and publish a usable build, though that would come with some heavy deps that you might not want, particularly for network access. Creating a library that only queries the local configuration is somewhat more work.

@Diggsey
Copy link
Contributor Author

Diggsey commented Sep 6, 2016

The main problem I see with this (and why I never pursued this idea further in the past) is that it makes any change to rustup's metadata a breaking change. At the moment, we're free to do pretty much anything as long as we have a migration path in place, but that will not be possible for a library since we have to continue to support arbitrarily old versions of the library and they may be older or newer than the installed rustup.

@jwilm
Copy link
Collaborator

jwilm commented Sep 6, 2016

@brson we've avoided using stuff like cargo metadata so far to 1) keep racer usable offline and 2) keep racer fast. You mention that it would be more work to create a local-only library, but would that be much more than reading a few files? Surely that code must already exist in rustup.rs.

@Diggsey You're right about that. 😞 If only we could dynamically link to a rustup library and then it would just be an API that must be stable versus the metadata itself.

Does rustc --print sysroot hit the network?

@Diggsey
Copy link
Contributor Author

Diggsey commented Sep 6, 2016

rustc --print sysroot shouldn't hit the network, although I haven't checked.

@jwilm
Copy link
Collaborator

jwilm commented Sep 6, 2016

D'oh, should have just traced it in the first place. Answer: no it doesn't 😄

@Cxarli
Copy link

Cxarli commented Aug 29, 2017

Bump?

@jwilm
Copy link
Collaborator

jwilm commented Aug 29, 2017

@C-Bouthoorn bump what? Racer should use rustc --print sysroot today if you don't set an environment variable.

@Cxarli
Copy link

Cxarli commented Sep 3, 2017

Then I guess the problem is in atom-racer. Excuse me for bumping this.

@freedaun
Copy link

Hello.. I'd like to alert that setting a PATH is not a user responsibility. The Rust ecosystem (cargo) knows everything about its install, the user definitely does not.

https://github.com/racer-rust/racer#configuration has lots of instructions a computer is very able of following. Heck, why should there be additional commands after running cargo install racer? Makes no sense.

Paths are considered evil, the equivalent of dangling pointers in C. That's pain right there. Put humans in the loop to deal with them and you have a recipy for torture.

@TedDriggs
Copy link
Contributor

@freedaun we'd welcome a PR that interacted with cargo to get that information automatically. For compat reasons, we'd need to honor user-specified values when they exist, but your PR could reduce setup complexity for newcomers.

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

8 participants