search

eclipse-corrosion / corrosion

Eclipse Corrosion - Rust edition in Eclipse IDE
Eclipse Public License 2.0
219 stars 31 forks source link

Download Rust into $XDG_CACHE_HOME #366

Open robertvazan opened 3 years ago

robertvazan commented 3 years ago

$HOME is not writable in flatpak and other app containers, because allowing arbitrary writes under $HOME would allow easy container escape by adding binaries in ~/.local/bin/ for example (which Corrosion happens to be doing with rust-analyzer, BTW). You should instead use variables defined in XDG Base Directory Specification just like the rest of Eclipse does. The most suitable location for Rust downloads would be probably $XDG_CACHE_HOME.

This would also prevent polluting user's home directory with more application-specific data directories as $XDG_CACHE_HOME defaults to $HOME/.cache.

BTW, I did not find a way to point Corrosion to my own installation of Rust. Normal installation includes rustc and cargo binaries and it can include rust-analyzer, but Corrosion for some reason requires rustup (the installer).

mickaelistria commented 3 years ago

Can you please submit a PR for that? The issue if that XDG_CACHE_HOME is not multi-platform.

but Corrosion for some reason requires rustup (the installer).

I think if cargo is found, rustup shouldn't be required. Can you please open a separate issue describing this issue with rustup, with minimal workflow to reproduce?

robertvazan commented 3 years ago

Added #367 for the local installation issue.

I think the rest of Eclipse is using some Eclipse-specific API to find the right directory, isn't it? I have never done Eclipse plugin development before, so I don't know how to implement this and I have no way to test a PR anyway.

$XDG_CACHE_HOME will be empty on other platforms. You can safely fall back to current behavior in that case. It's a simple change that would unblock container use cases.

mickaelistria commented 3 years ago

I think the rest of Eclipse is using some Eclipse-specific API to find the right directory, isn't it?

To be honest, I don't know. I'm not aware of such API. It's most probably that each plugin has it's own logic. Note that the "Install" step does use the rustup script from https://rustup.rs/ . Is this one using the $XDG_CACHE_HOME? If not, please open an issue to rustup first and we'll pick the updated script when it's available.

For rust-analyzer, we're following the install instructions from rust-analyzer itself. I'd rather not deviate from the standard.

robertvazan commented 3 years ago

You can set RUSTUP_HOME:

https://rust-lang.github.io/rustup/environment-variables.html

No idea where does ~/.cargo directory come from. Is that also created by rustup? Corrosion preferences make it look like it's a separate download & install. Why is it even needed when rustup installs cargo too?

Which rust-analyzer install instructions are you referring to? Do you mean the part of the manual linked below?

https://rust-analyzer.github.io/manual.html#rust-analyzer-language-server-binary

It says rust-analyzer just needs to be somewhere in $PATH and that's probably just so that IDE can find it. Since you are installing it yourself, you know where you install it and you don't need to keep it in $PATH.

robertvazan commented 3 years ago

BTW, even outside container, it's probably not such a great idea to install dependencies like this globally. Downloaded dependencies should be somewhere in private Corrosion directory.

robertvazan commented 3 years ago

Turns out this is a long-standing issue in upstream (https://github.com/rust-lang/rfcs/pull/1615). Apparently some people are against it with argument that goes like "why make things easier for 3rd party tools by making our own code more complicated". The 3rd party tool is Corrosion in this case. So that's it. If this is to be done, it will have to be done on Corrosion level with some hope that someday it will be fixed upstream.

mickaelistria commented 3 years ago

Thanks for finding out the root issue upstream. Is this issue a blocker for usage of Corrosion inside a flatpak container? That may have some impact on the priority. Note that, independently of my priorities, I always try to make reviews highest priority. So if you can submit a PR or influence someone in submitting one, I'll gladly review and hopefully merge it.

robertvazan commented 3 years ago

There's no workaround. Flatpak does not allow path remapping. Sharing ~/.local/bin defeats the purpose of containerization. I could install Rust in the right location inside the container, but I cannot configure Corrosion to use it due to #367. So yes, this is a blocker.

I currently don't have time to climb the learning curve of Eclipse plugin development.

mickaelistria commented 3 years ago

So this one is not a blocker but #367 is?

robertvazan commented 3 years ago

Solving either one will unblock container use.

Right now, if I want isolation, I can opt for either virtual machine with Eclipse+Corrosion or lightweight setup with Geany (or other editor) + dockerized Rust.

robertvazan commented 3 years ago

In order to get rustup to install everything in custom location, Corrosion has to:

It also has to download rust-analyzer binary in the custom location. When running rust-analyzer or cargo/rustc, $CARGO-HOME/bin must be added to PATH.

akurtakov commented 2 years ago

Corrosion relies on rustup to install toolchains and I think that should be preserved. If the concern is Flatpak please try installing corrosion in https://github.com/flathub/org.eclipse.Java - it contains a shim which allows executing host commands so corrosion was working fine in flatpak last I tried.

robertvazan commented 2 years ago

Doesn't that create a hole in the container? I have my own Eclipse flatpak build that completely isolates the container from the host.

akurtakov commented 2 years ago

It does but that's not an issue for an IDE and especially when we speak about C/C++ development there is practically no way to have all tools/libraries/headers in the container.

robertvazan commented 2 years ago

@akurtakov I am pretty sure I would be able to cram C/C++ toolchain in the container.

Isolation is a very serious issue for IDEs. IDEs download literally thousands of components that were built on machines of thousands of developers. At least one of them is bound to be compromised. It's either containers or VMs or online IDEs (which themselves run in VMs). I personally prefer containers for their superior desktop integration.

mickaelistria commented 2 years ago

Isolation is a very serious issue for IDEs. IDEs download literally thousands of components that were built on machines of thousands of developers.

Actually, isolation is more often an anti-pattern for IDE. I stands for "Integrated", and on a desktop, it means integrated with other tools the developer may need. Developers who need specific isolation on a desktop IDE need to define it by themselves because there is no 1 common need or common variation that fits everyone. Each developer would basically have to define their own container/sandbox that fits them best. That said, if you're willing to provide a PR to ease Flatpak integration without reducing the quality for the "vanilla" case of the IDE being in a desktop and integration with other local apps efficiently; those would probably be welcome.