Search for cargo in a more robust way

[malcolmbarrett]

In extendr/libR-sys#54, we ended up discussing whether a find_cargo() helper might be useful, similar to blogdown's find_hugo()

If I understand @yutannihilation's point, it's that Rust, being a whole language, is inherently more complex tooling--including how it was installed--than a single binary like hugo, making this type of solution less obvious for Rust.

My point is that major methods of installation--particularly rustup--install cargo in predictable places, and that it's sensible to check those places if Sys.which("cargo") doesn't work. For instance, in my case, cargo is in ~/.cargo/bin, right where rustup put it.

I was hoping to find the deeper problem in the original thread, but it's still not clear to me why my PATH in R doesn't have cargo but I do otherwise (e.g in the terminal), given that my installation and workflow is similar to others. I seem not to be alone, though. Notably, #99 lets me use rextendr successfully, but it's not clear to me how robust it is.

[yutannihilation]

Thanks for summarizing.

I'm not totally against the idea, but I'm wondering if it's really "robust" to use cargo without having ~/.cargo/bin in PATH. For example, if you want to develop an R package, you'll need cargo in PATH anyway (c.f. an example Makevars). So, I think it's good to encourage users to set up their PATH correctly instead of secretly solving the problems.

[yutannihilation]

it's still not clear to me why my PATH in R doesn't have cargo but I do otherwise (e.g in the terminal), given that my installation and workflow is similar to others

Which do you use zsh or bash? You can check the default shell by echo $SHELL. In the case of bash, this post seems a good write up of how setting files such as ~/.profile and ~/.bashrc work.

https://scriptingosx.com/2017/04/about-bash_profile-and-bashrc-on-macos/

As I wrote on extendr/libR-sys#54, rustup modifies ~/.profile, ~/.bashrc and ~/zshenv. If I understand correctly,

1. ~/.bashrc is sourced when you open a new interactive shell, which is not the case when you launch an App from Dock or Finder.
2. ~/.profile might not be sourced. this SO says ~/.profile might not be called when there's ~/.bash_profile, which might be created on installation of some other tools.
3. ~/.zshenv is not called if you are using bash.

Sys.which("cargo") doesn't work

I really don't understand why this happens when ~/.cargo/bin/ is on PATH. Sys.which() is a simple wrapper of system("which <command>"), so it should succeed when <command> can be found in PATH. I manually tried to reproduce this issue, but couldn't on my Linux laptop... But, if this is real, I feel we should fix Sys.which(), instead of implementing a workaround.

[dcnorris]

I note CRAN package cargo which apparently addresses this issue specifically for CRAN in view of its hands-off rule w.r.t. writing to user filesystem.

[clauswilke]

@dcnorris Interesting! Looks like this was just released.

[andy-thomason]

It would be interesting to see what it does...

[malcolmbarrett]

Sorry for the lack of follow-up @yutannihilation!

you'll need cargo in PATH anyway (c.f. an example Makevars)

That's a pretty good point, and I'm not sure what I'm suggesting would deal with it or if what has happened to my setup will end up being common. I'll play around a bit and see if that is true for my case.

Which do you use zsh or bash?

zsh, but it seems all my profiles are correctly updated by rustup, so I don't know why it works in other terminals but not from R.

I note CRAN package cargo which apparently addresses this issue specifically for CRAN in view of its hands-off rule w.r.t. writing to user filesystem.

Very interesting indeed! It seems like it uses an approach like what I am suggesting here to find cargo

[Robinlovelace]

Looking at the source code of that package it seems like a lot of if statements and system calls: https://github.com/cran/cargo/blob/master/R/cargo.R

Looking at the source code of the salso package for which cargo seems to have been developed I guess it was based on the hellorust template: https://github.com/r-rust/hellorust/tree/master/src

Not sure if that's useful but those are my thoughts on seeing this. I would tend to agree with @yutannihilation that it's better to insist that users have installed cargo properly than to silently fix problems based on the principle of 'fix problems as far upstream as possible' (in this case on installing rust) and after looking at the code in cargo.R.

[dcnorris]

A barrier (thus far) to getting the next version of precautionary (including now very fast Rust implementations of certain models) onto CRAN has been the CRAN policy that

• Packages should not write in the user’s home filespace (including clipboards), nor anywhere else on the file system apart from the R session’s temporary directory (or during installation in the location pointed to by TMPDIR: and such usage should be cleaned up). Installing into the system’s R installation (e.g., scripts to its bin directory) is not allowed.

This policy is violated by writing to ~/.cargo/, which salso apparently avoids with this script in its tools directory.

Having adapted that script for use in precautionary, I'll try another submission soon and report any success.

[yutannihilation]

This policy is violated by writing to ~/.cargo/,

I faced the same problem. For reference, gifski avoid this by setting CARGO_HOME within the package directory (or temporary dir) and removing it after installation. This isn't very convenient for development in that this always starts the building in the very fresh state (even the crate index is not cached), but works.

https://github.com/r-rust/gifski/blob/6b86cc6b60abbc2294db821f27cae37413df70c2/src/Makevars#L10

[dcnorris]

Many thanks Hiroaki. In fact, as noted in this r-pkg-devel thread where I tried to spark some community conversation on this topic, I have indeed adoped your approach.

[yutannihilation]

Oh, glad that you already find the way! Sorry, I think I saw your message on R-pkg-devel mailing list, but I didn't know much at that time. I'm currently struggling to fix CRAN build failures of my package using Rust code, so I hope I can share my experience if I succeeds...

[dcnorris]

So, I've got back onto CRAN at last https://CRAN.R-project.org/package=precautionary, but rather dire-sounding warnings have come in almost immediately (emphasis is mine):

On 09/08/2021 11:24, Prof Brian Ripley wrote:

Dear maintainer,

Please see the problems shown on
https://cran.r-project.org/web/checks/check_results_precautionary.html.

Please correct before 2021-09-02 to safely retain your package on CRAN.

The CRAN Team

"Package authors should make all reasonable efforts to provide
cross-platform portable code."

has not been complied with.

You have added a requirement for cargo but not declared it nor tested
for it. And your Makevars is using GNU make extensions without checking
for them.

Using rust is not portable, as 'Writing R Extensions' makes clear (as it
does about make extensions). It is not something that should be added
to an existing package, and most definitely not in a patch-level update.

Have you had experience with this type of response, Hiroaki? I don't actually see Rust mentioned by name in Writing R Extensions §1.6, but I will certainly look into the GNU extensions issue before reaching out on r-pkg-devel to discuss.

[clauswilke]

@dcnorris Are you on the extendr discord channel? There have been a number of conversations around this exact topic recently.

[dcnorris]

I'm not (yet); how would I join?

[clauswilke]

Try this: https://discord.gg/7hmApuc

[malcolmbarrett]

Just a quick note: I've now had this issue on 4 different MacBooks and have never been able to resolve it more than superficially

[yutannihilation]

Sorry, I forgot to share here. On submitting my package to CRAN, I found this problem exists on CRAN macOS machine. I chose to source ~/.cargo/env, which should be executed if either one of ~/.profile, ~/.bashrc or ~/zshenv is used, to avoid this issue.

https://github.com/yutannihilation/string2path/blob/34c1e5463e9e1490a9138c1b4b586dfcb812fca0/configure#L6-L14

But this way might not very handy when you want to run cargo in an external process (as this requires two separate commands). gifski package adds PATH directly to mimic ~/.cargo/env (because ~/.cargo/env might not exist).

$(STATLIB):
	PATH="$(HOME)/.cargo/bin:$(PATH)" cargo build --release --manifest-path=myrustlib/Cargo.toml

(https://github.com/r-rust/gifski/blob/6b86cc6b60abbc2294db821f27cae37413df70c2/src/Makevars#L12-L13)

So, in short, I think we should add cargo to PATH everytime we use cargo (edit: I meant "if the OS is not Windows").

[Ilia-Kosenkov]

It seems we are about to invent something similar to the code that searches for Rtools installation in {devtools}.
So far I see two things to consider:

1. Look for cargo in key places, which a platform-dependent. If cargo is already in the PATH, store path to it to re-use later.
2. Add cargo to PATH every time we invoke cargo-related operation. This should be safe even if cargo is already on the path.
   I guess this is our top priority now.

[malcolmbarrett]

I think this has adquately been addressed by @yutannihilation in #166