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

Release on R-universe #25

Closed
eitsupi opened this issue Jan 14, 2023 · 48 comments
Closed

Release on R-universe #25

eitsupi opened this issue Jan 14, 2023 · 48 comments

Comments

@eitsupi
Copy link
Collaborator

eitsupi commented Jan 14, 2023

How about releasing on R-universe?
It should be possible to build on R-universe with a few modifications to the Makefile.
Like r-universe-org/help#219 (comment)

@sorhawell
Copy link
Collaborator

That would be very awesome. I read the discussion. I don't mind if we run msvc or gnu on windows if it works.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jan 14, 2023

If polars build requires nightly toolchains, we may need to do the following on the Makefile anyway.

rustup:
	rustup toolchain install nightly || true

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jan 31, 2023

I looked into it and it seems that the dependency arrow2 cannot be used without installing the nightly tool chain, but the Linux builder in R-universe does not have rustup installed, so the nightly tool chain cannot be used......
This means that binaries cannot be distributed on R-universe, since mac and Windows jobs will not run without a successful build in the Linux builder.

Perhaps it is necessary to use the pre-build binaries and install method as string2path has done?
https://github.com/yutannihilation/string2path/blob/v0.1.0/tools/configure.R

@sorhawell
Copy link
Collaborator

Nice thanks for noticing.

Does R universe reject any build that fails R CMD check or can that be disabled?
I guess there could be a long term warning for macOS as described in #35. The package works just fine still.

@sorhawell
Copy link
Collaborator

sorhawell commented Jan 31, 2023

oh btw I swapped to Swatinem/rust-cache@v2 in check-workflow, which main polars is also using. It brings down compile times a bit more and does not need much customizing. I flushes the cache once daily it seems, which is nice too.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jan 31, 2023

Does R universe reject any build that fails R CMD check or can that be disabled?

If the first time Linux build failed, I believe it was not registered in R-universe.

If the Linux build is successful, the source will be released.
After that, if the mac build and Windows build are successful, a mac binary release and a Windows binary release will be made, respectively.
If the Linux build fails, all subsequent processes are skipped and nothing is released.

@sorhawell
Copy link
Collaborator

The rpolars R-universe homepage

Some failed packages might appear within some hours

@sorhawell
Copy link
Collaborator

Now r-polars here has a branch "hello_r_universe" and in rpolars/universe packages.json points to that branch.

@eitsupi if it works R-universe should be building from that branch and you can freely push anything to that branch as you like

@sorhawell
Copy link
Collaborator

build status

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 2, 2023

Thank you very much. Just not sure how much I can do......

There are several ways to do this, I think.

  1. Download binaries from elsewhere (same as old string2path).
  2. Use old polars that can be built with old Rust.
  3. Install the toolchain when the installation of rpolars.

1 maybe not worth doing since it is almost the same considering that it can currently be installed from GitHub Releases.
2 is clean but I think we need to use polars that are about 1 year old. (I'll look into it next time.)
3 is not good because it installs the rustup without the user's permission and is not allowed in CRAN, but may be feasible in R-universe...

@jeroen Sorry for tagging you, but do you have any advice? Thanks.

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

There are two typos (trailing commas) in your packages.json and therefore it is not valid json and fails to parse, as you can also see from the ❌ in that repo.

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 2, 2023

like this? ... seems to work now thx

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

Yes it now builds from that branch...
Build still fails with:

...
  cd ./rust/..
  cargo build --lib --release --manifest-path="./rust/Cargo.toml"
      Updating git repository `[https://github.com/extendr/extendr`](https://github.com/extendr/extendr%60)
      Updating git repository `[https://github.com/pola-rs/polars.git`](https://github.com/pola-rs/polars.git%60)
      Updating crates.io index
  error: no matching package named `polars-core` found
  location searched: https://github.com/pola-rs/polars.git?rev=6e64c8a626cd7a9e143e58939bf524153963237b#6e64c8a6
  required by package `rpolars v0.1.0 (/tmp/RtmpAAqQi1/R.INSTALL8163014e937/rpolars/src/rust)`

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 2, 2023

polars-core inherits versions, etc. from the workspace, and this feature is not available in Rust installed in the R-universe builder, so polars-core download from GitHub fails.

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 2, 2023

1 - As a prologue I try to resolve #4 in branch "long_arms64" based on @yutannihilation string2path recipe the following week. That is quite similar to option 1, and perhaps the makevars script can be adapted to do both. I don't mind having 1 as a backup option. It is not unlikely we have to support GPU's and/or quantum computers one day 😂
2 -
@ritchie46 suggested it would be possible to disable simd feature and compile on stable.

Build still fails with: error: no matching package named polars-core found

oh ok, r-polars and nodejs-polars subscribe to some non public features of the rust-polars crate and to bleeding edge. I guess it would be possible to make a PR to rust-polars exposing all non-public features, and only depend on the rust-polars crate. However that could slow down how fast r-polars can keep up with py-polars features.

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

this feature is not available in Rust installed in the R-universe builder, so polars-core download from GitHub fails.

Which version of rustc is required? We have 1.63 now, is that too old?

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 2, 2023

Which version of rustc is required? We have 1.63 now, is that too old?

Not entirely sure, I run 1.68 locally now and I think I had to update rustc locally once or twice since March'22 to compile.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 2, 2023

Which version of rustc is required? We have 1.63 now, is that too old?

It seems to be a feature implemented in 1.64.

Packages can now inherit settings from the workspace so that the settings can be centralized in one place. See workspace.package and workspace.dependencies for more details on how to define these common settings.

https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1640-2022-09-22

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 2, 2023

Fast search on r-polars use of polars_core.

8 results - 5 files

src/rust/src/rdatatype.rs:
  4  use polars::prelude::{self as pl};
  5: use polars_core::prelude::QuantileInterpolOptions;
  6  //expose polars DateType in R

src/rust/src/rlib.rs:
   9  use polars::prelude as pl;
  10: use polars_core::functions as pl_functions;
  11  #[extendr]

  14  
  15:     use polars_core::error::PolarsResult;
  16:     use polars_core::utils::rayon::prelude::*;
  17  

  23  
  24:     let result = polars_core::POOL
  25          .install(|| {

  45  fn diag_concat_df(dfs: &VecDataFrame) -> List {
  46:     let df = pl_functions::diag_concat_df(&dfs.0[..]).map(|ok| DataFrame(ok));
  47      r_result_list(df)

  51  pub fn hor_concat_df(dfs: &VecDataFrame) -> List {
  52:     let df = pl_functions::hor_concat_df(&dfs.0[..]).map(|ok| DataFrame(ok));
  53      r_result_list(df)


src/rust/src/rdataframe/mod.rs:
  23  use polars::prelude::ArrowField;
  24: use polars_core::utils::arrow;
  25  

jeroen added a commit to r-universe-org/base-image that referenced this issue Feb 2, 2023
@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

I'm upgrading it to rust 1.65... should be done in 20min.

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

OK that worked but you now have a new build failure (in the arrow dependency I think): https://github.com/r-universe/rpolars/actions/runs/4068667337/jobs/7021313109

@sorhawell
Copy link
Collaborator

I'll try to flip the simd switch

@sorhawell
Copy link
Collaborator

The downloaded source packages are in
‘/tmp/Rtmp6SM86v/downloaded_packages’
Using github PAT from envvar GITHUB_PAT
Error : Failed to install 'nanoarrow' from GitHub:
HTTP error 404.
Not Found
Did you spell the repo owner (github::apache) and repo name (arrow-nanoarrow) correctly?
- If spelling is correct, check that you have the required permissions to access the repo.

Is this because R package nanoarrow is not in the root dir of the nanoarrow-repository. Not sure, I had some issues installing the package.

error[E0554]: #![feature] may not be used on the stable release channel
--> /github/home/.cargo/git/checkouts/arrow2-945af624853845da/685cf49/src/lib.rs:14:39
|
14 | #![cfg_attr(feature = "simd", feature(portable_simd))]
| ^^^^^^^^^^^^^

hmm more switches needs to be flipped, I will just check up with the polars forum, I'm sure some wanted to do this also recently

@sorhawell
Copy link
Collaborator

image

i uploaded to "hello_r_universe " branch and pressed refresh button but that actually rerun a yesterday commit on main with simd switched on. I tried to increment r-polars version to v0.4.4 and now Runiverse says 0.4.3 is old I guess. I guess the version maybe will show up in a little time @jeroen ?

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 2, 2023

@sorhawell R-universe builds are executed once an hour, so wait a while 👍

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

Yes the new version is in the process of being built: https://github.com/r-universe/rpolars/actions/runs/4075440344

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 2, 2023

Perhaps the Rust code build succeeded and only the manual generation failed? I will take a look at this tomorrow.
Thanks!

image

@sorhawell
Copy link
Collaborator

It could be some exoteric char like a tilde in a subsection that causes the error.
https://latex.org/forum/viewtopic.php?t=25596

I guess the nuclear option if we can't find the error is to drop most R source files an incrementally add them back in until the error appears.

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

On Windows it fail with this message:

 echo "RPOLARS_RUST_SOURCE is set to ./rust"
  RPOLARS_RUST_SOURCE is set to ./rust
  export CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER="x86_64-w64-mingw32.static.posix-gcc.exe" && \
  	export LIBRARY_PATH="${LIBRARY_PATH};/d/a/rpolars/rpolars/RPOLAR~1.RCH/00_PKG~1/rpolars/src/./rust/target/libgcc_mock" && \
  	cargo +nightly-gnu build --target=x86_64-pc-windows-gnu --lib --release --manifest-path="./rust/Cargo.toml"
  error: toolchain 'nightly-x86_64-pc-windows-gnu' is not installed

@jeroen
Copy link
Contributor

jeroen commented Feb 2, 2023

Latex gives an error:

LaTeX errors:
! LaTeX Error: Unicode character � (U+FFFD)
               not set up for use with LaTeX.

If we search the rpolars html manual for this character, it appears in the arguments under lazy_csv_reader

encoding | either "utf8" or "utf8-lossy". Lossy means that invalid utf8 values are replaced with � characters.

I have no idea why latex can't handle this but perhaps it's worth trying to remove that character from the text?

@yutannihilation
Copy link
Contributor

yutannihilation commented Feb 3, 2023

Just curious, polars says the MSRV is 1.58, but is it that this turned out wrong? If so, I think we should report this to the upstream.

https://github.com/pola-rs/polars#rust

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 3, 2023

Just curious, polars says the MSRV is 1.58, but is it that this turned out wrong?

It could still be correct for someone using only public features of the rust-polars crate. Not sure, I will try to test this.

@sorhawell
Copy link
Collaborator

I'll try to fix the hello_r_universe branch for the windows error

@sorhawell
Copy link
Collaborator

I see more Tex errors I will try do debug and fix them locally

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 3, 2023

Just curious, polars says the MSRV is 1.58, but is it that this turned out wrong?

It could still be correct for someone using only public features of the rust-polars crate. Not sure, I will try to test this.

very minimal rust-polars does not work before 1.62 it seems
minimal rextendr-rust-polars-public-API does not seem to before somewhere later than 1.63

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 3, 2023

FYI, when checking MSRV with CI, it seems that cargo-msrv can be configured as follows.
https://github.com/eitsupi/prqlr/blob/a4779c5ec296bf89650259305ec7601b351cedf6/.github/workflows/check-rust.yml#L19-L28

Since Rust on Debian testing on CRAN is 3 to 4 versions behind, perhaps the R package would benefit from setting up such a CI.
I was going to take this to the extendr repository.

@sorhawell
Copy link
Collaborator

uhh that is very elegant 👍

@sorhawell
Copy link
Collaborator

@jeroen I tried to remove toolchain specification from makevars to be able to build to target stable-x86_64-pc-windows-gnu as suggested in r-universe issue#219 and I think that worked, however I see the same link error as @yutannihilation did

https://github.com/r-universe/rpolars/actions/runs/4083526250/jobs/7039673240

= note: link: missing operand after '\377\376"'
Try 'link --help' for more information.

note: link.exe returned an unexpected error

note: you may need to install Visual Studio build tools with the "C++ build tools" workload

do you have a wise suggestion here?

@sorhawell
Copy link
Collaborator

all big lateX are likely fixed in upcomming build, I used \link instead of \url at one place

@eitsupi
Copy link
Collaborator Author

eitsupi commented Feb 3, 2023

@jeroen I tried to remove toolchain specification from makevars to be able to build to target stable-x86_64-pc-windows-gnu as suggested in r-universe issue#219 and I think that worked, however I see the same link error as @yutannihilation did

Oh, sorry that problem continues to occur in prqlr but I forgot to create a new issue.......
I created this r-universe-org/help#231

@sorhawell
Copy link
Collaborator

sorhawell commented Feb 4, 2023

wow so many green builds now :) !

I think there is a last missing target i686-pc-windows-gnu for windows old release r-universe help issue 219 .

https://github.com/r-universe/rpolars/actions/runs/4086679575/jobs/7056707957

error[E0463]: can't find crate for core
|
= note: the i686-pc-windows-gnu target may not be installed
= help: consider downloading the target with rustup target add i686-pc-windows-gnu

error[E0463]: can't find crate for compiler_builtins

@yutannihilation
Copy link
Contributor

I'd recommend you to just abandon the version. R 4.3 is coming and the "old release" will be R 4.2 soon.

@jeroen
Copy link
Contributor

jeroen commented Feb 5, 2023 via email

@eitsupi
Copy link
Collaborator Author

eitsupi commented Mar 6, 2023

By the way, does a search on R-universe not seem to hit rpolars?
https://r-universe.dev/search/?q=rpolars
https://r-universe.dev/search/?q=r-polars

@jeroen
Copy link
Contributor

jeroen commented Mar 11, 2023

A package can be included in many different universes. To prevent duplicates, the search by default only includes packages from it's own universe, i.e. you would need to include it in the universe pola-rs.r-universe.dev. Right now you have only added it in the rpolars universe, that is why it does not show up in search.

I am thinking how to improve this. Maybe using a field in the DESCRIPTION.

jeroen added a commit to r-universe-org/cranlike-server that referenced this issue Mar 11, 2023
@jeroen
Copy link
Contributor

jeroen commented Mar 11, 2023

I have added a new feature that if your package DESCRIPTION file contains the url to your universe in the URL field, then it will also show up in search. I'll send a PR to test this.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Mar 11, 2023

It works! Thanks!

image

@eitsupi
Copy link
Collaborator Author

eitsupi commented Mar 18, 2023

The R-universe release works well now (except for the functionality needs the nightly tool chain), so I'm closing this for now.

See also #80.

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

4 participants