extendr / libR-sys

Low level R bindgen interface
https://extendr.github.io/libR-sys/libR_sys/
MIT License
52 stars 23 forks source link

cargo: command not found #54

Closed malcolmbarrett closed 3 years ago

malcolmbarrett commented 3 years ago

In running tests on rextendr on MacOS, I'm realizing there is something not correctly configured about my setup. When I try to run rust from R (using rextendr), I get:

#> sh: cargo: command not found

When I try running cargo from R manually, e.g. system("cargo --version"), I get the same issue.

Rust works fine on my computer, as does cargo. I also seem to be able to run cargo build successfully on this repo.

Any clue what might be the issue? I thought it would be good to ask here because I might be able to improve the docs to help others.

clauswilke commented 3 years ago

It's probably a path issue. You may have to add your path to cargo to your .Renviron file. Where does your cargo live? Did you install with rustup? For me it lives in ~/.cargo/bin/cargo and that seems to work without any special path setup.

malcolmbarrett commented 3 years ago

hm that seems to be where mine is, as well, and I did install via rustup. How would I specify that path in my .Renviron file?

clauswilke commented 3 years ago

You just add a path statement. For example, I have the following:

> cat ~/.Renviron 
PATH=/usr/local/opt/llvm/bin:${PATH}
malcolmbarrett commented 3 years ago

Bingo. For some reason, I needed PATH="~/.cargo/bin:${PATH}" in my .Renviron file. Thanks! Is it useful to include this in one of the READMEs?

clauswilke commented 3 years ago

I think it would belong here: https://extendr.github.io/rextendr/articles/setting_up_rust.html

yutannihilation commented 3 years ago

FWIW, on Ubuntu (or any other Linux whose default shell is dash), this line is installed in ~/.profile so I don't need to tweak .Renviron.

❯ tail -1 ~/.profile
source "$HOME/.cargo/env"

https://github.com/rust-lang/rustup/blob/84974df1387812269c7b29fa5f3bb1c6480a6500/src/cli/self_update/shell.rs#L113

For other Unix-alikes (e.g. RHEL/CentOS use bash, and macOS use zsh?, iiuc), other files like ~/.bashrc and ~/.zshenv is used. I guess they are not called when R process is launched.

https://github.com/rust-lang/rustup/blob/84974df1387812269c7b29fa5f3bb1c6480a6500/src/cli/self_update/shell.rs#L135 https://github.com/rust-lang/rustup/blob/84974df1387812269c7b29fa5f3bb1c6480a6500/src/cli/self_update/shell.rs#L183

(I don't remember which one is called when, so I might be wrong...)

clauswilke commented 3 years ago

I have this in my .profile on my Mac also. Maybe that's why it works for me.

yutannihilation commented 3 years ago

Ah, sorry, my explanation was wrong; I thought it modifies only one RC file based on the default shell, but it seems it modifies all the existing RC files, so ~/.profile should be modified on all UNIX-alikes.

https://github.com/rust-lang/rustup/blob/84974df1387812269c7b29fa5f3bb1c6480a6500/src/cli/self_update/unix.rs#L84

Anyway I agree it's useful to suggest to set PATH in .Renviron in case ~/.profile (or some equivalent) doesn't work for some reason.

malcolmbarrett commented 3 years ago

Hmm I have that in ~/.profile, too. In experimenting with this, the only thing that seems to help besides adding to .Renviron (which works fine) is adding the path to /etc/paths. Is the cargo path listed there for you?

I'm wondering about solutions beyond putting the path in .Renviron, e.g. making rextendr a bit more robust to where it searches for cargo.

A good example of this is in blogdown, which has hugo_cmd():

https://github.com/rstudio/blogdown/blob/a6c927343aa09222b7197c5d1c0f03f1a843e614/R/hugo.R#L6-L9

and find_hugo():

https://github.com/rstudio/blogdown/blob/69b5c383a77df3ac6d97f466d0bf98607e887cf2/R/install.R#L259-L274

I wonder if rextendr would benefit from corresponding cargo_cmd() and find_cargo() functions that use absolute paths the way blogdown does

malcolmbarrett commented 3 years ago

btw, does processx::run("cargo", "--version") work for you? It doesn't for me even with PATH set, which surprised me and makes me think there's something deeper about my setup (processx::run(normalizePath("~/.cargo/bin/cargo"), "--version") does work for me)

clauswilke commented 3 years ago
processx::run("cargo", "--version")
#> $status
#> [1] 0
#> 
#> $stdout
#> [1] "cargo 1.48.0 (65cbdd2dc 2020-10-14)\n"
#> 
#> $stderr
#> [1] ""
#> 
#> $timeout
#> [1] FALSE

Created on 2021-04-01 by the reprex package (v1.0.0)

yutannihilation commented 3 years ago

I wonder if rextendr would benefit from corresponding cargo_cmd() and find_cargo() functions that use absolute paths the way blogdown does

It might be useful, but I'd note that blogdown's case is a bit different in that hugo is a single binary and can be placed anywhere so the package has install_hugo(). In our case, it would be a bit difficult to provide install_cargo() as it's fairly complex.

malcolmbarrett commented 3 years ago

@clauswilke how are you running your R code? Are you using RStudio? I read in this post: "this is a problem that arises because the PATH environment variable is different for programs launched by Finder (or the Dock) than it is in the Terminal"

I notice that running R in the terminal or using vscode, I do not have these path problems.

@yutannihilation yes, it's definitely a simpler case. blogdown also attempts to shield the user from Hugo a bit, which rextendr is of course not doing with rust. I haven't given much thought to an equivalent of the install function (although wrapping rustup wouldn't be hard, no?). That said, find_hugo() exists precisely because it could be in many places, and it doesn't assume you used install_hugo(). It might even be simpler in Rust since there are some predictable places where cargo is installed.

clauswilke commented 3 years ago

RStudio

yutannihilation commented 3 years ago

Sorry, I think I'm getting lost. What problem are we trying to solve here...? Doesn't .Renviron work for you?

although wrapping rustup wouldn't be hard, no?

I'd say installing a toolset into a system properly is a different thing than just placing a binary on an arbitrary place. The users can have various Rust setups for various purposes. I don't think it's safe to tweak the setup.

It might even be simpler in Rust since there are some predictable places where cargo is installed.

Rust can be installed by other tools than rustup (e.g. brew, apt); it might depend on to what extent we provide support, but I'm afraid it's not very simple.

That said, find_hugo() exists precisely because it could be in many places, and it doesn't assume you used install_hugo().

My understanding is opposite. It's needed just because hugo might be on a different place than PATH. In our case, Sys.which() is suffice (I'm not against creating find_cargo() as an alias to Sys.which("cargo")).

malcolmbarrett commented 3 years ago

Yes, this thread is diffusing a bit. I'll close this, post an issue to update the docs re: .Renviron, and post an issue to discuss whether a more robust search for the location of cargo is useful.

To address the points here, though, .Renviron only sort of works for me. For instance, Sys.which("cargo") does not work but system("cargo --version") does.

Installation support is probably not as useful as in the case of Hugo because of the reasons you say.

My understanding is opposite. It's needed just because hugo might be on a different place than PATH.

That's actually my point, too 😅. Except, as I say, for some reason, Sys.which("cargo") does not actually suffice for me, even setting PATH manually. Yet, my cargo is in a predictable spot that rextendr could check