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

CrystFEL: support aarch64-linux and HDF5 on Darwin #218968

Merged
merged 3 commits into from Apr 26, 2023

Conversation

pmiddend
Copy link
Contributor

@pmiddend pmiddend commented Mar 1, 2023

Description of changes

The CrystFEL software used to not run on aarch64. This is due to:

  1. An old version of the dependent library libccp4, which couldn't find the gfortran dependency. There is a newer version, 8.0.0 of this software, but this didn't work out of the box either (the CMake build process assumes certain unnecessary libraries to be installed), so the author of CrystFEL recommended to me to use a patch that adds the meson build systems instead. This is also used by the official package, see here (in nixpkgs, I avoided the wrap functionality and packaged the dependencies using Nix instead). It does not depend on gfortran anymore, it only builds the C version (for CrystFEL, only that is important).
  2. On aarch64-darwin, the function fmemopen doesn't exist (used to map a region of memory to a FILE* pointer). The corresponding code I patched and sent the patch upstream (see here). On aarch64-linux, we have the memorymappingHook which (still) works.

Also, the HDF5-External-Filter-Plugin didn't work on Darwin because of a linker issue. This has been fixed by an external contributor.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

@pmiddend
Copy link
Contributor Author

pmiddend commented Mar 1, 2023

@ofborg build crystfel-headless

@pmiddend
Copy link
Contributor Author

pmiddend commented Mar 2, 2023

@ofborg build crystfel

@pmiddend pmiddend changed the title CrystFEL: support aarch64-linux CrystFEL: support aarch64-linux and HDF5 on Darwin Mar 3, 2023
@pmiddend pmiddend marked this pull request as ready for review March 3, 2023 07:43
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Mar 3, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1913

@pmiddend
Copy link
Contributor Author

pmiddend commented Mar 8, 2023

@SuperSandro2000 Thanks for the review and for pressing me on the meson patch thingy. The patch is actually in meson's "wrapdb" from where I shamelessly copied it from.

Now it's downloaded from there. I'm not sure about the new postUnpack attribute though. The problem was that wrapdb doesn't give you a patch file, but a zip archive with libccp4-8.0.0/meson.build in it, so I opted for the fetchzip + cp option. Does that seem fine with you?

@pmiddend
Copy link
Contributor Author

@SuperSandro2000 Any further changes?

@pmiddend
Copy link
Contributor Author

@SuperSandro2000 Ping

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/2131

@wegank
Copy link
Member

wegank commented Apr 26, 2023

@ofborg build crystfel crystfel.passthru.tests

@wegank
Copy link
Member

wegank commented Apr 26, 2023

Result of nixpkgs-review pr 218968 run on aarch64-darwin 1

2 packages built:
  • crystfel
  • crystfel-headless

@wegank wegank merged commit 7b57f59 into NixOS:master Apr 26, 2023
7 of 9 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

4 participants