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

heck installation issue: rhub issue macos (R-devel) #5

Closed
DyfanJones opened this issue Jul 2, 2024 · 14 comments
Closed

heck installation issue: rhub issue macos (R-devel) #5

DyfanJones opened this issue Jul 2, 2024 · 14 comments

Comments

@DyfanJones
Copy link
Owner

* installing *source* package ‘heck’ ...
** using staged installation
** libs
using C compiler: ‘Apple clang version 15.0.0 (clang-1500.0.40.1)’
using SDK: ‘’
rm -Rf heck.so ./rust/target/release/libheck.a entrypoint.o /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/.cargo /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/vendor
clang -arch x86_64 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG   -I/opt/R/x86_64/include    -fPIC  -falign-functions=64 -Wall -g -O2  -c entrypoint.c -o entrypoint.o
if [ -f ./rust/vendor.tar.xz ]; then \
		tar xf rust/vendor.tar.xz && \
		mkdir -p /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/.cargo && \
		cp rust/vendor-config.toml /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/.cargo/config.toml; \
	fi
# In some environments, ~/.cargo/bin might not be included in PATH, so we need
# to set it here to ensure cargo can be invoked. It is appended to PATH and
# therefore is only used if cargo is absent from the user's PATH.
if [ "true" != "true" ]; then \
		export CARGO_HOME=/Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/.cargo; \
	fi && \
		export PATH="/Users/runner/work/heck/heck/check/heck.Rcheck/R_check_bin:/opt/R/x86_64/bin:/opt/gfortran/bin:/usr/local/lib/ruby/gems/3.0.0/bin:/usr/local/opt/ruby@3.0/bin:/usr/local/opt/pipx_bin:/Users/runner/.cargo/bin:/usr/local/opt/curl/bin:/usr/local/bin:/usr/local/sbin:/Users/runner/bin:/Users/runner/.yarn/bin:/Users/runner/Library/Android/sdk/tools:/Users/runner/Library/Android/sdk/platform-tools:/Library/Frameworks/Python.framework/Versions/Current/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/usr/bin:/bin:/usr/sbin:/sbin:/Users/runner/.dotnet/tools:/Users/runner/.cargo/bin" && \
			cargo build -j 2 --offline --lib --release --manifest-path=./rust/Cargo.toml --target-dir ./rust/target && \
			echo `cargo --version` && echo `rustc --version`;
   Compiling proc-macro2 v1.0.79
   Compiling unicode-ident v1.0.12
   Compiling libR-sys v0.7.0
   Compiling paste v1.0.14
   Compiling quote v1.0.36
   Compiling syn v2.0.58
   Compiling extendr-api v0.7.0
error[E0428]: the name `R_altrep_Coerce_method_t` is defined multiple times
   --> /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/./rust/target/release/build/libR-sys-80f46fe92fda2ece/out/bindings.rs:595:1
    |
595 | / pub type R_altrep_Coerce_method_t =
596 | |     ::std::option::Option<unsafe extern "C" fn(arg1: SEXP, arg2: ::std::os::raw::c_int) -> SEXP>;
    | |_________________________________________________________________________________________________^ `R_altrep_Coerce_method_t` redefined here
    |
   ::: /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/vendor/libR-sys/src/lib.rs:80:1
    |
80  | / pub type R_altrep_Coerce_method_t =
81  | |     ::std::option::Option<unsafe extern "C" fn(arg1: SEXP, arg2: SEXPTYPE) -> SEXP>;
    | |____________________________________________________________________________________- previous definition of the type `R_altrep_Coerce_method_t` here
    |
    = note: `R_altrep_Coerce_method_t` must be defined only once in the type namespace of this module

error[E0428]: the name `Rf_isS4` is defined multiple times
    --> /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/./rust/target/release/build/libR-sys-80f46fe92fda2ece/out/bindings.rs:1721:5
     |
1721 |     pub fn Rf_isS4(arg1: SEXP) -> Rboolean;
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rf_isS4` redefined here
     |
    ::: /Users/runner/work/heck/heck/check/heck.Rcheck/00_pkg_src/heck/src/vendor/libR-sys/src/lib.rs:83:1
     |
83   | pub unsafe fn Rf_isS4(arg1: SEXP) -> Rboolean {
     | --------------------------------------------- previous definition of the value `Rf_isS4` here
     |
     = note: `Rf_isS4` must be defined only once in the value namespace of this module

For more information about this error, try `rustc --explain E0428`.
error: could not compile `libR-sys` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
make: *** [rust/target/release/libheck.a] Error 101
ERROR: compilation failed for package ‘heck’
* removing ‘/Users/runner/work/heck/heck/check/heck.Rcheck/heck’
@DyfanJones
Copy link
Owner Author

@JosiahParry
Copy link
Contributor

This issue is also on CRAN. It can be fixed by bumping the version of libR-sys.

Check: https://www.r-project.org/nosvn/R.check/r-oldrel-macos-x86_64/heck-00check.html

[patch.crates-io]
libR-sys = { git = "https://github.com/extendr/libR-sys", rev = "8649204b2490f3aa3732d56f4d84af7f042e5f05" }

I am referencing a specific commit here in arcpbf that is after this fix went in. @CGMossa, do you think we can publish a patched version of libR-sys so this comes from crates.io?

@CGMossa
Copy link

CGMossa commented Sep 3, 2024

I've made a new release.

@DyfanJones
Copy link
Owner Author

I am getting the following error when I add libR-sys to the cargo.toml

[patch.crates-io]
libR-sys = { git = "https://github.com/extendr/libR-sys", rev = "8649204b2490f3aa3732d56f4d84af7f042e5f05" }
> rextendr::document()
✔ Saving changes in the open files.Generating extendr wrapper functions for package: heck.Re-compiling heck (debug build)
── R CMD INSTALL ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
─  installing *source* packageheck...
   ** using staged installation
   ** libs
   using C compiler:Apple clang version 15.0.0 (clang-1500.3.9.4)’
   using SDK:MacOSX14.4.sdkrm -Rf heck.so ./rust/target/release/libheck.a entrypoint.o /Users/dyfanjones/Documents/Packages/heck/src/.cargo /Users/dyfanjones/Documents/Packages/heck/src/vendor
   clang -arch arm64 -I"/Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/include" -DNDEBUG   -I/opt/R/arm64/include    -fPIC  -falign-functions=64 -Wall -g -O2  -UNDEBUG -Wall -pedantic -g -O0 -fdiagnostics-color=always -c entrypoint.c -o entrypoint.o
   if [ -f ./rust/vendor.tar.xz ]; then \
   		tar xf rust/vendor.tar.xz && \
   		mkdir -p /Users/dyfanjones/Documents/Packages/heck/src/.cargo && \
   		cp rust/vendor-config.toml /Users/dyfanjones/Documents/Packages/heck/src/.cargo/config.toml; \
   	fi
   # In some environments, ~/.cargo/bin might not be included in PATH, so we need
   # to set it here to ensure cargo can be invoked. It is appended to PATH and
   # therefore is only used if cargo is absent from the user's PATH.
   if [ "true" != "true" ]; then \
   		export CARGO_HOME=/Users/dyfanjones/Documents/Packages/heck/src/.cargo; \
   	fi && \
   		export PATH="/Users/dyfanjones/Library/r-miniconda-arm64/condabin:/Users/dyfanjones/Library/r-miniconda-arm64/envs/r-reticulate/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/X11/bin:/usr/local/munki:/Applications/quarto/bin:/Users/dyfanjones/.cargo/bin:/usr/ucb:/Library/TeX/texbin:/usr/texbin:/Applications/RStudio.app/Contents/Resources/app/bin/postback:/Users/dyfanjones/.cargo/bin" && \
   			cargo build -j 2 --offline --lib --release --manifest-path=./rust/Cargo.toml --target-dir ./rust/target && \
   			echo `cargo --version` && echo `rustc --version`;
      Compiling extendr-api v0.7.0
   error[E0425]: cannot find function, tuple struct or tuple variant `Rf_isValidString` in this scope
       --> /Users/dyfanjones/Documents/Packages/heck/src/vendor/extendr-api/src/robj/rinternals.rs:358:18
        |
   358  |         unsafe { Rf_isValidString(self.get()).into() }
        |                  ^^^^^^^^^^^^^^^^ help: a function with a similar name exists: `Rf_isBlankString`
        |
       ::: /Users/dyfanjones/Documents/Packages/heck/src/./rust/target/release/build/libR-sys-54899a8164457f5c/out/bindings.rs:1297:5
        |
   1297 |     pub fn Rf_isBlankString(arg1: *const ::std::os::raw::c_char) -> Rboolean;
        |     ------------------------------------------------------------------------ similarly named function `Rf_isBlankString` defined here
   
   error[E0425]: cannot find function, tuple struct or tuple variant `Rf_isValidStringF` in this scope
       --> /Users/dyfanjones/Documents/Packages/heck/src/vendor/extendr-api/src/robj/rinternals.rs:363:18
        |
   363  |         unsafe { Rf_isValidStringF(self.get()).into() }
        |                  ^^^^^^^^^^^^^^^^^ help: a function with a similar name exists: `Rf_isBlankString`
        |
       ::: /Users/dyfanjones/Documents/Packages/heck/src/./rust/target/release/build/libR-sys-54899a8164457f5c/out/bindings.rs:1297:5
        |
   1297 |     pub fn Rf_isBlankString(arg1: *const ::std::os::raw::c_char) -> Rboolean;
        |     ------------------------------------------------------------------------ similarly named function `Rf_isBlankString` defined here
   
   For more information about this error, try `rustc --explain E0425`.
   error: could not compile `extendr-api` (lib) due to 2 previous errors
   make: *** [rust/target/release/libheck.a] Error 101
   ERROR: compilation failed for packageheck’
─  removing/private/var/folders/sp/scbzkbwx6hbchmylsx0y52k80000gn/T/RtmpYROWtd/devtools_install_7c4deac4773/heckError in `(function (command = NULL, args = character(), error_on_status = TRUE, …`:
! System command 'R' failed
---
Exit status: 1
stdout & stderr: <printed>
---
Type .Last.error to see the more details.

Updated cargo.toml:

[package]
name = 'heck'
version = '0.1.0'
edition = '2021'

[lib]
crate-type = ['staticlib']
name = 'heck'

[dependencies]
extendr-api = '0.7.0'
heck = "0.5.0"

[patch.crates-io]
libR-sys = { git = "https://github.com/extendr/libR-sys", rev = "8649204b2490f3aa3732d56f4d84af7f042e5f05" }

I am guessing I am missing something :)

@JosiahParry
Copy link
Contributor

Did you re-vendor the dependencies? Typically, jsut to be safe, I will delete everything that starts with vendor and then run rextendr::vendor_pkgs()

@DyfanJones
Copy link
Owner Author

Aye indeed. I wonder if it is a mac arm issue 🤔

@JosiahParry
Copy link
Contributor

I can try and check it out later today

@JosiahParry
Copy link
Contributor

I have MacOS with Arm and was able to build. Is this issue on R-hub or on your own machine?

@CGMossa
Copy link

CGMossa commented Sep 3, 2024

I'd like to know the r version available at r_home

@JosiahParry
Copy link
Contributor

I'd like to know the r version available at r_home

Ah yeah, I think we now have a restriction of R >= 4.1, is that right? Should go in rextendr defaults.

@JosiahParry
Copy link
Contributor

JosiahParry commented Sep 3, 2024

@DyfanJones I can reproduce by putting libR-sys = "0.7.1" in the Cargo.toml. A quick summary of the issue:

  • R-core has changed what is considered "public API" for their C code
  • This has changed frequently
  • In order to not have packages removed from CRAN we need to remove use of the non-API symbols
  • Rf_isValidString is now non-API which was fixed in libR-sys 0.7.1
  • Rf_isValidString is not removed from extendr-api (see is_valid_string() is no longer public API extendr/extendr#841)

There is an extendr-api PR to address this: extendr/extendr#842

@DyfanJones
Copy link
Owner Author

Hmmm I am not sure why my mac isn't building it correctly. I might need to do a 2 stage release. First release the cran requirements (so the package doesn't get taken down) and then circle back to try and sort out my mac.

@CGMossa
Copy link

CGMossa commented Sep 5, 2024

Hello @DyfanJones. Do you mind trying "everything" again? We just made a patch release to extendr, so the current version is 0.7.1 for extendr-api, extendr-engine, extendr-macros, and even libR-sys. If you use cargo update within src/rust, and then see if it works now? Even on your Mac?

Also, is your Mac an Intel mac?

@DyfanJones
Copy link
Owner Author

@CGMossa I have a Mac M1 (arm64). It looks all good through my rhub checks :D https://github.com/DyfanJones/heck/actions/runs/10725077185 Thanks @CGMossa @JosiahParry for replying and fixing the issue. I will close this now and release the package shortly :)

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

3 participants