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.hdf5r: fix compile error #291004

Closed
wants to merge 9 commits into from
Closed

rPackages.hdf5r: fix compile error #291004

wants to merge 9 commits into from

Conversation

Kupac
Copy link
Contributor

@Kupac Kupac commented Feb 23, 2024

Description of changes

Due to HDFGroup/hdf5#1814
, the package doesn't compile, because it needs h5cc -show and h5cc -showconfig outputs in the config script. Switching back to hdf5_1_10 solved the problem, albeit it's an older version of the hdf5 lib (but seems to work).

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Kupac Kupac requested a review from jbedo as a code owner February 23, 2024 22:31
@philipp-baumann
Copy link
Contributor

confirm nix-build --pure -A rPackages.rhdf5 compiles successfully on this PR for darwin-aarch64

# ...
/nix/store/k687y4hiraslz65qz8pli1wb5wmcabvn-clang-wrapper-16.0.6/bin/c++ -std=gnu++17 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -L/nix/store/1hfs8ylpsyfrrakhyvskr4dgycd0l16c-R-4.3.2/lib/R/lib -L/nix/store/hi4fialjfb9wd2f5al4vpzlyh8dz38fz-libcxx-16.0.6/lib -o rhdf5.so H5.o H5A.o H5D.o H5E.o H5F.o H5G.o H5I.o H5L.o H5O.o H5P.o H5R.o H5S.o H5S_extras.o H5T.o H5T_extras.o H5Z.o H5constants.o HandleList.o HandleListWrap.o bit64conversion.o external_filters.o h5dump.o h5ls.o h5testLock.o h5writeDataFrame.o printdatatype.o utils.o wrap.o -lhdf5 -lhdf5 -L/nix/store/1hfs8ylpsyfrrakhyvskr4dgycd0l16c-R-4.3.2/lib/R/lib -lR -lintl -Wl,-framework -Wl,CoreFoundation
installing to /nix/store/5q2cqvk4ixvpbj9wkni9z1w6l5vk0h9d-r-rhdf5-2.46.1/library/00LOCK-rhdf5/00new/rhdf5/libs
** R
** inst
** byte-compile and prepare package for lazy loading
During startup - Warning messages:
1: Setting LC_CTYPE failed, using "C" 
2: Setting LC_TIME failed, using "C" 
3: Setting LC_MESSAGES failed, using "C" 
4: Setting LC_MONETARY failed, using "C" 
** help
*** installing help indices
*** copying figures
** building package indices
During startup - Warning messages:
1: Setting LC_CTYPE failed, using "C" 
2: Setting LC_TIME failed, using "C" 
3: Setting LC_MESSAGES failed, using "C" 
4: Setting LC_MONETARY failed, using "C" 
** installing vignettes
** testing if installed package can be loaded from temporary location
During startup - Warning messages:
1: Setting LC_CTYPE failed, using "C" 
2: Setting LC_TIME failed, using "C" 
3: Setting LC_MESSAGES failed, using "C" 
4: Setting LC_MONETARY failed, using "C" 
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
During startup - Warning messages:
1: Setting LC_CTYPE failed, using "C" 
2: Setting LC_TIME failed, using "C" 
3: Setting LC_MESSAGES failed, using "C" 
4: Setting LC_MONETARY failed, using "C" 
** testing if installed package keeps a record of temporary installation path
* DONE (rhdf5)
Running phase: fixupPhase
checking for references to /private/tmp/nix-build-r-rhdf5-2.46.1.drv-0/ in /nix/store/5q2cqvk4ixvpbj9wkni9z1w6l5vk0h9d-r-rhdf5-2.46.1...
patching script interpreter paths in /nix/store/5q2cqvk4ixvpbj9wkni9z1w6l5vk0h9d-r-rhdf5-2.46.1
rewriting symlink /nix/store/5q2cqvk4ixvpbj9wkni9z1w6l5vk0h9d-r-rhdf5-2.46.1/nix-support/propagated-user-env-packages to be relative to /nix/store/5q2cqvk4ixvpbj9wkni9z1w6l5vk0h9d-r-rhdf5-2.46.1
/nix/store/5q2cqvk4ixvpbj9wkni9z1w6l5vk0h9d-r-rhdf5-2.46.1

@b-rodrigues
Copy link
Contributor

Built and tested successfully on linux-aarch64 (rpi5):

> hdf5r::h5version()
hdf5r version 1.3.8 with C-library HDF5 Version  1.10.11 
[1] "1.10.11"

@b-rodrigues
Copy link
Contributor

For curiosity's sake, if we wished to be on the latest version, would we need to overlay this definition?

version = "1.10.11";

@Kupac
Copy link
Contributor Author

Kupac commented Feb 24, 2024

For curiosity's sake, if we wished to be on the latest version, would we need to overlay this definition?

For curiosity's sake, I'm overriding the version and src of hdf5_1_10 with those of hdf5. if that works, we'd have the latest version compiled without cmake - best of both worlds. And we wouldn't have to update hash at each version bump ;) Let's see how that works !

@Kupac
Copy link
Contributor Author

Kupac commented Feb 24, 2024

> h5version()
hdf5r version 1.3.8 with C-library HDF5 Version  1.14.3 
[1] "1.14.3"

@Kupac
Copy link
Contributor Author

Kupac commented Feb 24, 2024

I'm gonna push it, and see what @jbedo thinks. I kept it in a separate commit for now, in case we have to roll it back

@b-rodrigues
Copy link
Contributor

Everything seems to be working (tested on linux-aarch64):

> hdf5r::h5version()
hdf5r version 1.3.8 with C-library HDF5 Version  1.14.3 
[1] "1.14.3"

@@ -472,7 +472,10 @@ let
VariantAnnotation = with pkgs; [ zlib.dev curl.dev ];
snpStats = [ pkgs.zlib.dev ];
vcfppR = [ pkgs.curl.dev pkgs.bzip2 pkgs.zlib.dev pkgs.xz];
hdf5r = [ pkgs.hdf5.dev ];
hdf5r = [ (pkgs.hdf5_1_10.dev.overrideAttrs (prev: new: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we keep this, this should likely go all the way down starting at line 944 (where packages with overrides are defined)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm not sure where it belongs. Just keep in mind that it's not an rPackage itself that's overridden, but one of it's nativeBuildInputs.

@jbedo
Copy link
Contributor

jbedo commented Mar 29, 2024

Fixed in #298837 by leveraging Rhdf5lib's library. If we need a newer version we can work on updating Rhdf5lib.

@Kupac Kupac closed this Mar 29, 2024
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.

5 participants