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

rPackages, R: R 4.3.3 -> 4.4.1 and bump rPackages #315015

Merged
merged 143 commits into from
Jun 20, 2024
Merged

rPackages, R: R 4.3.3 -> 4.4.1 and bump rPackages #315015

merged 143 commits into from
Jun 20, 2024

Conversation

jbedo
Copy link
Contributor

@jbedo jbedo commented May 27, 2024

Update R and rPackages.

Hydra: https://hydra.nixos.org/jobset/nixpkgs/r-updates

nixpkgs-review:

7 packages failed to build:
cantor jasp-desktop labplot python311Packages.rpy2 python311Packages.rpy2.dist sage sageWithDoc

41 packages built:
R R.tex diffoscope diffoscope.dist diffoscope.man jetbrains.dataspell ocamlPackages.ocaml-r postgresql12JitPackages.plr postgresql12Packages.plr postgresql13JitPackages.plr postgresql13Packages.plr postgresql14JitPackages.plr postgresql14Packages.plr postgresql15JitPackages.plr postgresql15Packages.plr postgresql16JitPackages.plr postgresql16Packages.plr python311Packages.cnvkit python311Packages.cnvkit.dist python311Packages.radian python311Packages.radian.dist python311Packages.rchitect python311Packages.rchitect.dist python312Packages.cnvkit python312Packages.cnvkit.dist python312Packages.radian python312Packages.radian.dist python312Packages.rchitect python312Packages.rchitect.dist python312Packages.rpy2 python312Packages.rpy2.dist quarto rWrapper radianWrapper rstudio rstudio-server rstudioServerWrapper rstudioWrapper spark spark_3_4 vscode-extensions.reditorsupport.r

jasp-desktop has warnings being elevated to errors, e.g.,:

jaspObject.cpp:508:25: error: format not a string literal and no format arguments [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-security-Werror=format-security8;;]
  508 |                 Rf_error(("nested key \"" + nestedKeyToString(nestedKey, "$") + "\"does not exist in the options!").c_str());

There doesn't seem to be a newer jasp to bump to, so we could disable format security for the moment. @TomaSajt

Sage is blocked by python3Packages.rpy2, which is failing using 3.11 during testing due to a segfault. This segfault does not happen with python 3.12. Any suggestions @joelmo? Sage builds fine when disabling tests so this is the only problem to resolve for sage.

@TomaSajt
Copy link
Contributor

For jasp-desktop I guess we could just do

env.NIX_CFLAGS_COMPILE = "-Wno-error=format-security";

I think it's fine to include it in this PR.

@b-rodrigues

This comment was marked as outdated.

@collares
Copy link
Member

@jbedo If I understand correctly, the CI runs at rpy2/rpy2#1051 segfault on the same test (test_embedded_r.py) running under Python 3.12, which is consistent with the undefined behavior explanation.

@jbedo
Copy link
Contributor Author

jbedo commented Jun 14, 2024

That's great sluthing, and good to know it isn't resolved with 3.12. I'll spend some time trying to get the ABI mode to work.

@collares
Copy link
Member

collares commented Jun 14, 2024

Actually, I am not sure the ABI suggestion definitely helps. If you run the minimized tastcase under valgrind --trace-children=yes, both the ABI and the API mode report invalid reads when invoking callbacks. But it might be a good enough workaround while we wait for an upstream fix.

In fact, the same valgrind errors show up at 5b706ee, which is the first Nixpkgs commit at which rpy2 builds under Python 3.10 (I can't check Python 3.9 and below, because the valgrind output is a lot messier even for good Python scripts).

Edit: I think using R_tryCatchError with Python callbacks as rpy2 does is just broken, regardless of {Python, cffi, R, rpy2} version. See rpy2/rpy2#1111 (comment) for more details.

@jbedo
Copy link
Contributor Author

jbedo commented Jun 17, 2024

I'm unsure as to the best way forward: this bug really needs to be fixed upstream. Changing ABI/hardening/optimisation really does nothing, we just sidestep the bug during the test case with some luck. Is this any better than disabling tests? rpy2 is also still broken with the old R 4.3.3 version as you've discovered, so there doesn't seem to be much point to holding the R version bump back waiting for a fix and we may as well pick one of these options and then update rpy2 when an upstream fix is available.

@collares
Copy link
Member

collares commented Jun 17, 2024

Sure, I'm happy with something like disabledTests = [ "test_parse_incomplete_error" "test_parse_error" "test_parse_error_when_evaluating" ];, along with a link to the rpy2 issue. Thanks!

Edit: ofBorg is happy.

@collares
Copy link
Member

@ofborg build sageWithDoc

@b-rodrigues
Copy link
Contributor

R 4.4.1 got released last week, what shall we do, merge this one and re-bump, or directly bump to 4.4.1?

@ofborg ofborg bot requested a review from collares June 17, 2024 09:15
@jbedo
Copy link
Contributor Author

jbedo commented Jun 17, 2024

R 4.4.1 got released last week, what shall we do, merge this one and re-bump, or directly bump to 4.4.1?

I'm planning to bump to 4.4.1, just preparing it and checking it. It should be a fairly minor change since we already got 4.4.0 in good shape.

@jbedo
Copy link
Contributor Author

jbedo commented Jun 17, 2024

49 packages built:
R R.tex cantor diffoscope diffoscope.dist diffoscope.man jasp-desktop jetbrains.dataspell labplot ocamlPackages.ocaml-r postgresql12JitPackages.plr postgresql12Packages.plr postgresql13JitPackages.plr postgresql13Packages.plr postgresql14JitPackages.plr postgresql14Packages.plr postgresql15JitPackages.plr postgresql15Packages.plr postgresql16JitPackages.plr postgresql16Packages.plr python311Packages.cnvkit python311Packages.cnvkit.dist python311Packages.radian python311Packages.radian.dist python311Packages.rchitect python311Packages.rchitect.dist python311Packages.rpy2 python311Packages.rpy2.dist python312Packages.cnvkit python312Packages.cnvkit.dist python312Packages.radian python312Packages.radian.dist python312Packages.rchitect python312Packages.rchitect.dist python312Packages.rpy2 python312Packages.rpy2.dist quarto rWrapper radianWrapper resorter rstudio rstudio-server rstudioServerWrapper rstudioWrapper sage sageWithDoc
spark spark_3_4 vscode-extensions.reditorsupport.r

@jbedo jbedo marked this pull request as ready for review June 17, 2024 21:59
@jbedo jbedo changed the title rPackages, R: R 4.3.3 -> 4.4.0 and bump rPackages rPackages, R: R 4.3.3 -> 4.4.1 and bump rPackages Jun 18, 2024
Copy link
Contributor

@b-rodrigues b-rodrigues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any other major packages that must absolutely be fixed, so for me this can be merged!

@jbedo jbedo merged commit e7d6f2f into master Jun 20, 2024
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants