Adds CRAN compliant vendoring helper functions

[JosiahParry]

This PR adds functionality to vendor rust dependencies within an R package. It incorporates much of the draft PR #311.

It adds 2 functions

• use_cargo_vendor(): creates the package scaffolding for using cargo vendor in a CRAN compliant way. It does so by:
  • modifying Makevars and Makevars.win to compile with only 2 cores, always compile in offline mode, and print the versions of rustc and cargo
  • adds a template vendor-config.toml file
  • adds the appropriate files to both .Rbuildignore and .gitignore
• vendor_pkgs() is derived from PR #311. It
  • runs cargo vendor to create the vendor directory
  • checks if the suggested config.toml differs from vendor-config.toml and if so, it updates it accordingly
  • compresses the vendor directory into a vendor.tar.xz
  • invisibly returns a data frame with two columns containing the crate name and the version of the crate

The end user experience is this:

• rextendr::use_cargo_vendor() to instantiate vendoring
• rextendr::vendor_pkgs() to build their compressed zip file or update it

Follow up tasks:

• submit a package to CRAN using this workflow. I plan on doing this for rsgeo given that the r-universe build worked without any problems using these functions
• upon successful CRAN submission, document this workflow in an rextendr vignette

Question for reviewers

There is one question, though, based on #312. Right now the template Makevars add -J 2 --offline to the cargo build statement. Should the templates use the CARGO_BUILD_JOBS environment variable instead?

Design notes

I opted to not modify the existing Makevars and Makevars.win file to use offline and -J 2 options in an if statement. This is because i think it would lead to an even more complex Makevar file which would build differently based on the presence of CRAN or not.

Additionally, I think the vendoring experience should be opt in and done knowingly.

[Ilia-Kosenkov]

This is beyond awesome! To begin with, can you walk me through the process of creating a fresh dummy package and use this functionality? I want to test it locally. Is it enough to just run these two methods one by one?

Another important thing would be to test this workflow on CI. We have a separate CI that simulates creating new package using {rextendr}. We need to have the same but testing vendor capability.

[Ilia-Kosenkov]

Also you can run styler::style_pkg() to please {lintr}

[JosiahParry]

@Ilia-Kosenkov

The process would go like this:

usethis::create_package()
rextendr::use_extendr()
rextendr::use_cargo_vendor()

do some stuff write some functions do some documenting etc. Then when youre ready to compress the vendored packages you write:

rextendr::vendor_pkgs()

[Ilia-Kosenkov]

Ah and I think for starters you can write a snapshot test to see what files are generated when you run this. Check out existing snapshot tests on rextendr::use_extendr(). You can track your test coverage here https://app.codecov.io/github/extendr/rextendr/pull/313

[JosiahParry]

Awesome! I'll get to that in a few hours or first thing in the morning tomorrow.

[Ilia-Kosenkov]

You have a typo somewhere

 Warning: Missing or unexported object: ‘cli::ci_abort’

[Ilia-Kosenkov]

Ok, vendoring works fine locally. What do I do next? Can I just install this package from my directory or what? I figured it out. Works nicely, however there is a UX issue -- if you use rextendr::use_extendr() and then rextendr::use_cargo_vendor(), Makevars are not updated since overwrite = FALSE by default. Perhaps we should do something like {usethis} does when it asks whether we want to do something with some weird answers to choose from, like Yes, Absolutely not!, No way?

[JosiahParry]

@Ilia-Kosenkov correct! Once vendoring works you can devtools::install() successfully. Publishing to r-universe should also work without a hitch.

I think this should be part of an rextendr specific "release" process

[JosiahParry]

Ok, vendoring works fine locally. What do I do next? Can I just install this package from my directory or what? I figured it out. Works nicely, however there is a UX issue -- if you use rextendr::use_extendr() and then rextendr::use_cargo_vendor(), Makevars are not updated since overwrite = FALSE by default. Perhaps we should do something like {usethis} does when it asks whether we want to do something with some weird answers to choose from, like Yes, Absolutely not!, No way?

Good call!

[Ilia-Kosenkov]

Also this is worth adding to Readme.Rmd I believe. Just a small section explaining what it is and why we need that (if you have links to CRAN mailing lists with bundling demands, insert them there)

[Ilia-Kosenkov]

Also pinging @yutannihilation -- this PR is important for all CRAN-related activities, I believe you might be interested :)

[eitsupi]

What is the reason for prioritizing this over my some PRs?

[JosiahParry]

@eitsupi I apologize if it feels like im usurping you! I didn't notice the draft PR. I tossed out what I had for vendoring in favor of the the proposal that was included in the draft #311.

[JosiahParry]

I think i've ticked all the boxes! @Ilia-Kosenkov

[CGMossa]

What is the reason for prioritizing this over my some PRs?

I don't think there is any reason. We have massive guilt due to neglecting this project.. Then a new PR drop and we think.. Let's just get this thing out.

I have many, many neglected PRs over at extendr project... 🤷‍♂️

[Ilia-Kosenkov]

Hm, can we also remove vendored crates when calling rextendr::clean()? I did not check it myself, but I believe it should remove this as it is an artifact .

[Ilia-Kosenkov]

Hey @eitsupi , sorry for the PR neglect, it was an error on our side (and my oversight personally since I was looking only at GH notifications instead of browsing all PRs). Since your PR is in a draft phase, I'd suggest abandoning it in favor of this one, with you reviewing & QA this PR since you already know what should be working.

[JosiahParry]

Hm, can we also remove vendored crates when calling rextendr::clean()? I did not check it myself, but I believe it should remove this as it is an artifact .

I do not believe cargo vendor is capable of removing the entire vendor directory. You can use cargo clean -p to remove a package as you would without vendoring just fine. rextendr::clean() does not remove the vendored libraries.

Please let me know what additional changes are requested.

[eitsupi]

I think #311 and this PR cover very different areas.

311 only adds a function and does not make any changes to the package development workflow.

The reason is as described in #311.

I tossed out what I had for vendoring in favor of the the proposal that was included in the draft #311.

I have looked at this a bit and don't think it follows the tar command options.

Since your PR is in a draft phase, I'd suggest abandoning it in favor of this one, with you reviewing & QA this PR since you already know what should be working.

My PR is a draft because I was looking for opinions. I probably failed because no one saw my PR and I was not able to gather opinions.

[eitsupi]

@Ilia-Kosenkov Could you add @JosiahParry and I as external collaborators for this repository? That way we can edit the PR directly.

[JosiahParry]

Rather than edit the PR directly, can we please make inline comments using the review feature to discuss it? All tar-ing is taken directly from both PR #311 and hellorust https://github.com/r-rust/hellorust/blob/8e5e619dff723708b8e7e99f5f679a6c71a85c43/src/myrustlib/vendor-update.sh#L4.

[eitsupi]

Rather than edit the PR directly, can we please make inline comments using the review feature to discuss it? All tar-ing is taken directly from both PR #311 and hellorust r-rust/hellorust@8e5e619/src/myrustlib/vendor-update.sh#L4.

As far as I have tried, the command in hellorust changes the hash of the tarball each time it is run.

311's one is based on the command used by string2path and produces the same tarball each time (See https://github.com/eitsupi/prqlr/pull/152#issuecomment-1652759510), with the highest compression ratio.

[eitsupi]

I opted to not modify the existing Makevars and Makevars.win file to use offline and -J 2 options in an if statement. This is because i think it would lead to an even more complex Makevar file which would build differently based on the presence of CRAN or not.

I am against the use of multiple Makevars templates. In general, committing a huge file like a tarball to a Git repository reduces performance, i.e., it is a common decision not to commit a tarball like the gifski package. https://github.com/r-rust/gifski/blob/2b7f3bb558e5bfaeb52f9c989ecbff30d8310a05/.gitignore#L12

This special Makevars file is not useful in such a use case if it assumes the existence of the tarball and offline.

Again, is there any reason not to merge #312?

[JosiahParry]

My goal is to make R + Rust be a viable future. As such I made this PR to be a thorough attempt at addressing the most recent CRAN guidance. This PR is tailored strictly towards appeasing CRAN no more no less.

I think a more appropriate place to discuss PRs #311 and #312 would be in those threads repsectively.

It is worth noting that this PR is open to review and comments for changes. Changes could include using CARGO_BUILD_THREADS such as that suggested in PR #312. I would like to keep the scope of the PR to this PR. Further, if this PR is merged, there is always the possibility of a follow up PR to patch and fix or enhance. I do not expect 1 PR to be the end all be all.

[eitsupi]

Sorry for the noisy comments. I will probably fork rextendr and create another package for my own use.

[yutannihilation]

I think it's a reasonable choice. I personally don't use rextendr's templates because I need more opinionated ones for various reasons. Honestly, I don't think it's a good time to define "the standard" templates because the requirements change constantly and rapidly.

[JosiahParry]

I agree! My goal here is to have a minimal template that can get folks less experienced than you both (such as myself) up and running without much thought. It is based on the prior art developed by @eitsupi, @yutannihilation, and Joeren. Your feedback and comments are greatly appreciated!

[yutannihilation]

I appreciate your effort, but, in my opinion, "a minimal template" should not target CRAN.

[JosiahParry]

I appreciate your effort, but, in my opinion, "a minimal template" should not target CRAN.

To clarify, these functions are opt-in and not a replacement of the default templates.