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

petsc: Fix install_name_tool patch #93428

Merged
merged 1 commit into from Jul 18, 2020

Conversation

johnbcoughlin
Copy link
Contributor

The PETSc library's config/install.py script checks whether
/usr/bin/install_name_tool is an existing file:

https://gitlab.com/petsc/petsc/-/blob/master/config/install.py#L368

Therefore, it is not enough to replace it with something that we expect
to be on the PATH, as this will cause the linked if statement to be
silently skipped. The consequence is that applications linked against
this version of petsc will fail at runtime on MacOS with a dynamic
loading error.

Instead, we should replace install_name_tool with the absolute path of a
binary we know will be present at build time.

Note that this should be fixed upstream as well, but this is sufficient
to fix the runtime error.

Motivation for this change
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 PETSc library's config/install.py script checks whether
/usr/bin/install_name_tool is an existing file:

https://gitlab.com/petsc/petsc/-/blob/master/config/install.py#L368

Therefore, it is not enough to replace it with something that we expect
to be on the PATH, as this will cause the linked if statement to be
silently skipped. The consequence is that applications linked against
this version of petsc will fail at runtime on MacOS with a dynamic
loading error.

Instead, we should replace install_name_tool with the absolute path of a
binary we know will be present at build time.

Note that this should be fixed upstream as well, but this is sufficient
to fix the runtime error.
Copy link
Contributor

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

Thank you for your first contribution 👍

Tested on Darwin and confirmed that this corrects the library path.

@danieldk danieldk merged commit cf059b3 into NixOS:master Jul 18, 2020
@johnbcoughlin johnbcoughlin deleted the petsc-install_name_tool branch July 19, 2020 03:27
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