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

Move use extendr_engine inside test! macro #424

Merged

Conversation

yutannihilation
Copy link
Contributor

Address this concern: #392 (comment)

@Ilia-Kosenkov
Copy link
Member

Is this PR ready or wip?

@yutannihilation
Copy link
Contributor Author

This is ready.

@Ilia-Kosenkov Ilia-Kosenkov merged commit c6929eb into extendr:master Aug 14, 2022
@yutannihilation yutannihilation deleted the fix/remove-non-API-call-again branch August 14, 2022 06:22
@yutannihilation
Copy link
Contributor Author

Thanks!

@eitsupi
Copy link
Contributor

eitsupi commented Dec 18, 2022

Hi, has a version reflecting this change been released yet?
(I'm not sure because there are no tags in this repository.) The following note appears in the package using version 0.3.1 (https://github.com/eitsupi/prqlr/blob/9c8a0330d6e57dd0bb8f4550e1ea159c96b11cca/src/rust/Cargo.toml#L12).

* checking compiled code ... NOTE
File 'prqlr/libs/x64/prqlr.dll':
  Found non-API calls to R: 'R_CStackLimit', 'R_CleanTempDir',
    'R_RunExitFinalizers', 'Rf_initialize_R', 'setup_Rmainloop'

Compiled code should not call non-API entry points in R.

Thanks for working on this!

eitsupi added a commit to PRQL/prqlc-r that referenced this pull request Dec 18, 2022
@yutannihilation
Copy link
Contributor Author

Not released yet. Sorry for inconvenience. At the moment, you have to use the dev version like my package. Note that I use my branch to test another dev version of libR-sys, but this is not necessary.

c.f. https://github.com/yutannihilation/string2path/blob/6b293f07bf4c0136ace45f7c158dd709163f6099/src/rust/Cargo.toml#L10

I personally consider this is a serious problem that is worth another release, but, since few people here care about CRAN much (and I'm too lazy to explain the details), it's not released yet. Btw, I recently found I have permission on crates.io, so it's possible to release if we agree to release.

@Ilia-Kosenkov
Copy link
Member

Ilia-Kosenkov commented Dec 18, 2022

It is possible it has not been released yet. Can you try building your package targeting github versio on of extendr?

Sorry, did not see there already is an answer when typing from phone.

eitsupi added a commit to PRQL/prqlc-r that referenced this pull request Dec 19, 2022
- Fix documents
- Use unreleased version of `extendr-api` (to include extendr/extendr#424)
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

Successfully merging this pull request may close these issues.

None yet

3 participants