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

r-modules: Updated definitions of CRAN and Bioc packages. #108268

Merged
merged 2 commits into from Jan 11, 2021

Conversation

TikhonJelvis
Copy link
Contributor

Updated the R package definitions.

This involved two manual code changes:

  1. Removing a few deleted packages from default.nix
  2. Renaming the "assert" package to "r_assert" in generate-r-packages.R because "assert" is a keyword in Nix

I followed the steps in the R manual section, testing the result with nix-build test-evaluation.nix --dry-run.

Motivation for this change

R package set was last updated >4 months ago (2020-08-19).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

The last snapshot was 4 months ago (2020-08-19). I also found that I needed newer definitions when I was trying to fix the R arrow package.

This update required a couple of manual changes:

  1. Removing a few deleted packages from default.nix
  2. Renaming the "assert" package to "r_assert" in generate-r-packages.R because "assert" is a keyword in Nix
@SuperSandro2000
Copy link
Member

darwin patch

diff --git a/pkgs/development/r-modules/default.nix b/pkgs/development/r-modules/default.nix
index 387a5d0bb63..7cdb5f72658 100644
--- a/pkgs/development/r-modules/default.nix
+++ b/pkgs/development/r-modules/default.nix
@@ -379,6 +379,7 @@ let
     rmutil = lib.optionals stdenv.isDarwin [ pkgs.libiconv ];
     robustbase = lib.optionals stdenv.isDarwin [ pkgs.libiconv ];
     SparseM = lib.optionals stdenv.isDarwin [ pkgs.libiconv ];
+    hexbin = lib.optionals stdenv.isDarwin [ pkgs.libiconv ];
     svKomodo = [ pkgs.which ];
     nat = [ pkgs.which ];
     nat_templatebrains = [ pkgs.which ];

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108268 run on x86_64-linux 1

1 package failed to build and are new build failure:
  • rstudioWrapper: log was empty
5 packages built:
  • python37Packages.rpy2
  • python38Packages.rpy2
  • rWrapper
  • sage
  • sageWithDoc

`data.table` had a `postInstall` step to rename `data.table.so` to `datatable.so`, but after the package bump the file was already called `datatable.so` and `mv` command would fail.
@TikhonJelvis
Copy link
Contributor Author

I added @SuperSandro2000's Darwin patch and also fixed a bug with the data_table override in default.nix. The override had a postInstall step

postInstall = "mv $out/library/data.table/libs/{data.table,datatable}.so";

but after the package bump, data.table.so was already called datatable.so and the mv command would fail, so I removed that line.

I only caught the problem with data_table because it caused nix-shell generate-shell.nix to fail. Is there some kind of test I should run to check other R packages don't have similar problems?

Also, for context, I'm doing this bump now because I'm trying to fix the R Arrow package (#81761). I got a fix for that running locally, but found that I needed to bump the R packages so that the R Arrow package has the same version as the C++ Arrow library in Nixpkgs. But it also seems good to have more up to date package definitions anyway.

@SuperSandro2000
Copy link
Member

Is there some kind of test I should run to check other R packages don't have similar problems?

I don't know anything like this. If ofborg evals everything it is usually fine.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 108268 run on x86_64-darwin 1

2 packages built:
  • python37Packages.rpy2
  • python38Packages.rpy2

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108268 run on x86_64-linux 1

4 packages built:
  • python37Packages.rpy2
  • python38Packages.rpy2
  • rWrapper
  • rstudioWrapper

Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

Test builds are running at https://hydra.nixos.org/jobset/nixpkgs/r-updates ...

@peti
Copy link
Member

peti commented Jan 11, 2021

Thank you very much for your efforts, everyone!

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

3 participants