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

mkl: fix install_name on Darwin #57947

Merged
merged 1 commit into from Jun 6, 2019
Merged

mkl: fix install_name on Darwin #57947

merged 1 commit into from Jun 6, 2019

Conversation

smaret
Copy link
Member

@smaret smaret commented Mar 20, 2019

Motivation for this change

Fix MKL install names on Darwin (see #57697).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@smaret smaret force-pushed the fix-mkl branch 2 times, most recently from f9b4a6a to 8f7dae3 Compare March 25, 2019 22:53
@smaret
Copy link
Member Author

smaret commented Mar 25, 2019

There are some @rpath left in a couple of libraries that aren't fixed by fixDarwinDylibNames:

% otool -L libmkl_intel_thread.dylib libmkl_tbb_thread.dylib 
libmkl_intel_thread.dylib:
	/nix/store/gfqn224m0vdj280vynl0g49smka4qdz6-mkl-2019.0.117/lib/libmkl_intel_thread.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libiomp5.dylib (compatibility version 5.0.0, current version 5.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)
libmkl_tbb_thread.dylib:
	/nix/store/gfqn224m0vdj280vynl0g49smka4qdz6-mkl-2019.0.117/lib/libmkl_tbb_thread.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libtbb.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)

This can be fixed with install_name_tool -change. libiomp5.dylib is part of the mkl package, but libtbb.dylib is in a separate one (tbb) and needs to be added as a dependency of mkl.

@smaret smaret force-pushed the fix-mkl branch 2 times, most recently from 036dbe3 to 1a62a1d Compare March 26, 2019 12:18
@smaret smaret changed the title [WIP] mkl: fix install_name on Darwin mkl: fix install_name on Darwin Mar 26, 2019
@smaret
Copy link
Member Author

smaret commented Mar 26, 2019

@bhipple can you please review this PR?

@smaret smaret force-pushed the fix-mkl branch 3 times, most recently from 6266597 to 382d2cf Compare March 31, 2019 11:01
@smaret
Copy link
Member Author

smaret commented Mar 31, 2019

Thanks for the fix @veprbl. I've squashed it and force-pushed it.

@smaret
Copy link
Member Author

smaret commented Apr 2, 2019

After rebasing on master (to update to 2019.3.199), the post-installation phase fails:

post-installation fixup
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_rt.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_blacs_mpich_lp64.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_intel_ilp64.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_vml_avx2.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_vml_avx.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_mc.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_tbb_thread.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_core.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_vml_avx512.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_vml_mc.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_avx.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libiomp5.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libiomp5_db.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_blacs_mpich_ilp64.dylib: fixing dylib
/nix/store/spx5ibqn2wdfdib3ms52mvm72nic8sgx-mkl-2019.3.199/lib/libmkl_cdft_core.dylib: fixing dylib
builder for '/nix/store/lw62ffkis8v9advkdva616jkggmz9c9k-mkl-2019.3.199.drv' failed with exit code 1

I'll look into this.

@smaret
Copy link
Member Author

smaret commented Apr 2, 2019

install_name_tool -id message is more explicit:

error: install_name_tool: changing install names or rpaths can't be redone for: /nix/store/fqvq3kfijmx08p42jy9isfjq33yhd1ig-mkl-2019.3.199/lib/libmkl_cdft_core.dylib (for architecture x86_64) because larger updated load commands do not fit (the program must be relinked, and you may need to use -headerpad or -headerpad_max_install_names)

Obviously we can't relink the library since Intel only provides binary. Is there a way to fix this?

@smaret
Copy link
Member Author

smaret commented Apr 2, 2019

The libmkl_cdft_core.dylib isn't in the derivation for Linux. Is it intended @bhipple? Should we also drop it for Darwin?

@smaret
Copy link
Member Author

smaret commented Apr 2, 2019

In fact the Darwin and Linux derivations do not contain the same libraries:

  • Linux:
% tree result
result
|-- include
|   |-- fftw
|   |   |-- fftw.h
|   |   |-- fftw3-mpi.h
|   |   |-- fftw3-mpi_mkl.h
|   |   |-- fftw3.h
|   |   |-- fftw3_mkl.h
|   |   |-- fftw3_mkl_f77.h
|   |   |-- fftw_mpi.h
|   |   |-- fftw_threads.h
|   |   |-- rfftw.h
|   |   |-- rfftw_mpi.h
|   |   `-- rfftw_threads.h
|   |-- i_malloc.h
|   |-- mkl.h
|   |-- mkl_blacs.h
|   |-- mkl_blas.h
|   |-- mkl_cblas.h
|   |-- mkl_cdft.h
|   |-- mkl_cdft_types.h
|   |-- mkl_cluster_sparse_solver.h
|   |-- mkl_compact.h
|   |-- mkl_df.h
|   |-- mkl_df_defines.h
|   |-- mkl_df_functions.h
|   |-- mkl_df_types.h
|   |-- mkl_dfti.h
|   |-- mkl_direct_blas.h
|   |-- mkl_direct_blas_kernels.h
|   |-- mkl_direct_call.h
|   |-- mkl_direct_lapack.h
|   |-- mkl_direct_types.h
|   |-- mkl_dnn.h
|   |-- mkl_dnn_types.h
|   |-- mkl_dss.h
|   |-- mkl_lapack.h
|   |-- mkl_lapacke.h
|   |-- mkl_pardiso.h
|   |-- mkl_pblas.h
|   |-- mkl_poisson.h
|   |-- mkl_rci.h
|   |-- mkl_scalapack.h
|   |-- mkl_service.h
|   |-- mkl_solvers_ee.h
|   |-- mkl_sparse_handle.h
|   |-- mkl_sparse_qr.h
|   |-- mkl_spblas.h
|   |-- mkl_trans.h
|   |-- mkl_trig_transforms.h
|   |-- mkl_types.h
|   |-- mkl_version.h
|   |-- mkl_vml.h
|   |-- mkl_vml_defines.h
|   |-- mkl_vml_functions.h
|   |-- mkl_vml_types.h
|   |-- mkl_vsl.h
|   |-- mkl_vsl_defines.h
|   |-- mkl_vsl_functions.h
|   `-- mkl_vsl_types.h
`-- lib
    |-- libiomp5.a
    |-- libiomp5.dbg
    |-- libiomp5.so
    |-- libiomp5_db.so
    |-- libiompstubs5.a
    |-- libiompstubs5.so
    |-- libmkl_avx.so
    |-- libmkl_avx2.so
    |-- libmkl_avx512.so
    |-- libmkl_avx512_mic.so
    |-- libmkl_core.so
    |-- libmkl_def.so
    |-- libmkl_intel_ilp64.so
    |-- libmkl_intel_lp64.so
    |-- libmkl_intel_thread.so
    |-- libmkl_mc.so
    |-- libmkl_mc3.so
    |-- libmkl_rt.so
    |-- libmkl_sequential.so
    |-- libmkl_vml_avx.so
    |-- libmkl_vml_avx2.so
    |-- libmkl_vml_avx512.so
    |-- libmkl_vml_avx512_mic.so
    |-- libmkl_vml_cmpt.so
    |-- libmkl_vml_def.so
    |-- libmkl_vml_mc.so
    |-- libmkl_vml_mc2.so
    |-- libmkl_vml_mc3.so
    |-- license.txt
    `-- locale
        |-- en_US
        |   `-- mkl_msg.cat
        `-- ja_JP
            |-- libiomp5.cat
            `-- mkl_msg.cat

6 directories, 89 files
  • Darwin:
result
├── include
│   ├── blas.f90
│   ├── fftw
│   │   ├── fftw.h
│   │   ├── fftw3-mpi.f03
│   │   ├── fftw3-mpi.h
│   │   ├── fftw3-mpi_mkl.h
│   │   ├── fftw3.f
│   │   ├── fftw3.f03
│   │   ├── fftw3.h
│   │   ├── fftw3_mkl.f
│   │   ├── fftw3_mkl.h
│   │   ├── fftw3_mkl_f77.h
│   │   ├── fftw_f77.i
│   │   ├── fftw_mpi.h
│   │   ├── fftw_threads.h
│   │   ├── rfftw.h
│   │   ├── rfftw_mpi.h
│   │   └── rfftw_threads.h
│   ├── i_malloc.h
│   ├── intel64
│   │   ├── ilp64
│   │   │   ├── blas95.mod
│   │   │   ├── f95_precision.mod
│   │   │   ├── lapack95.mod
│   │   │   └── mkl_service.mod
│   │   └── lp64
│   │       ├── blas95.mod
│   │       ├── f95_precision.mod
│   │       ├── lapack95.mod
│   │       └── mkl_service.mod
│   ├── lapack.f90
│   ├── mkl.fi
│   ├── mkl.h
│   ├── mkl_blacs.h
│   ├── mkl_blas.f90
│   ├── mkl_blas.fi
│   ├── mkl_blas.h
│   ├── mkl_cblas.h
│   ├── mkl_cdft.f90
│   ├── mkl_cdft.h
│   ├── mkl_cdft_types.h
│   ├── mkl_cluster_sparse_solver.f90
│   ├── mkl_cluster_sparse_solver.fi
│   ├── mkl_cluster_sparse_solver.h
│   ├── mkl_compact.h
│   ├── mkl_df.f90
│   ├── mkl_df.h
│   ├── mkl_df_defines.h
│   ├── mkl_df_functions.h
│   ├── mkl_df_types.h
│   ├── mkl_dfti.f90
│   ├── mkl_dfti.h
│   ├── mkl_direct_blas.h
│   ├── mkl_direct_blas_kernels.h
│   ├── mkl_direct_call.fi
│   ├── mkl_direct_call.h
│   ├── mkl_direct_lapack.h
│   ├── mkl_direct_types.h
│   ├── mkl_dnn.h
│   ├── mkl_dnn_types.h
│   ├── mkl_dss.f90
│   ├── mkl_dss.fi
│   ├── mkl_dss.h
│   ├── mkl_lapack.fi
│   ├── mkl_lapack.h
│   ├── mkl_lapacke.h
│   ├── mkl_pardiso.f90
│   ├── mkl_pardiso.fi
│   ├── mkl_pardiso.h
│   ├── mkl_pblas.h
│   ├── mkl_poisson.f90
│   ├── mkl_poisson.h
│   ├── mkl_rci.f90
│   ├── mkl_rci.fi
│   ├── mkl_rci.h
│   ├── mkl_scalapack.h
│   ├── mkl_service.f90
│   ├── mkl_service.fi
│   ├── mkl_service.h
│   ├── mkl_solvers_ee.f90
│   ├── mkl_solvers_ee.fi
│   ├── mkl_solvers_ee.h
│   ├── mkl_sparse_handle.f90
│   ├── mkl_sparse_handle.fi
│   ├── mkl_sparse_handle.h
│   ├── mkl_sparse_qr.f90
│   ├── mkl_sparse_qr.h
│   ├── mkl_spblas.f90
│   ├── mkl_spblas.fi
│   ├── mkl_spblas.h
│   ├── mkl_trans.fi
│   ├── mkl_trans.h
│   ├── mkl_trig_transforms.f90
│   ├── mkl_trig_transforms.h
│   ├── mkl_types.h
│   ├── mkl_version.h
│   ├── mkl_vml.f90
│   ├── mkl_vml.fi
│   ├── mkl_vml.h
│   ├── mkl_vml_defines.h
│   ├── mkl_vml_functions.h
│   ├── mkl_vml_types.h
│   ├── mkl_vsl.f90
│   ├── mkl_vsl.fi
│   ├── mkl_vsl.h
│   ├── mkl_vsl_defines.h
│   ├── mkl_vsl_functions.h
│   ├── mkl_vsl_subroutine.fi
│   └── mkl_vsl_types.h
└── lib
    ├── libiomp5.a
    ├── libiomp5.dylib
    ├── libiomp5_db.dylib
    ├── libiompstubs5.a
    ├── libiompstubs5.dylib
    ├── libmkl_avx.dylib
    ├── libmkl_avx2.dylib
    ├── libmkl_avx512.dylib
    ├── libmkl_blacs_mpich_ilp64.a
    ├── libmkl_blacs_mpich_ilp64.dylib
    ├── libmkl_blacs_mpich_lp64.a
    ├── libmkl_blacs_mpich_lp64.dylib
    ├── libmkl_blas95_ilp64.a
    ├── libmkl_blas95_lp64.a
    ├── libmkl_cdft_core.a
    ├── libmkl_cdft_core.dylib
    ├── libmkl_core.a
    ├── libmkl_core.dylib
    ├── libmkl_intel_ilp64.a
    ├── libmkl_intel_ilp64.dylib
    ├── libmkl_intel_lp64.a
    ├── libmkl_intel_lp64.dylib
    ├── libmkl_intel_thread.a
    ├── libmkl_intel_thread.dylib
    ├── libmkl_lapack95_ilp64.a
    ├── libmkl_lapack95_lp64.a
    ├── libmkl_mc.dylib
    ├── libmkl_mc3.dylib
    ├── libmkl_pgi_thread.a
    ├── libmkl_rt.dylib
    ├── libmkl_scalapack_ilp64.a
    ├── libmkl_scalapack_ilp64.dylib
    ├── libmkl_scalapack_lp64.a
    ├── libmkl_scalapack_lp64.dylib
    ├── libmkl_sequential.a
    ├── libmkl_sequential.dylib
    ├── libmkl_tbb_thread.a
    ├── libmkl_tbb_thread.dylib
    ├── libmkl_vml_avx.dylib
    ├── libmkl_vml_avx2.dylib
    ├── libmkl_vml_avx512.dylib
    ├── libmkl_vml_mc.dylib
    ├── libmkl_vml_mc2.dylib
    ├── libmkl_vml_mc3.dylib
    ├── license.txt
    └── locale
        └── en_US
            └── mkl_msg.cat

8 directories, 151 files

Note that the libmkl_blacs_mpich, libmkl_blas95, libmkl_cdft_core, libmkl_scalapack, libmkl_lapack95, libmkl_scalapack and libmkl_tbb_thread are in the Darwin derivation only.

@smaret
Copy link
Member Author

smaret commented Apr 3, 2019

I've just realised that the MKL contains its own libtbb. I've included it in the Darwin derivation and remove tbb from the buildInputs. I guess this will also help to make this a fixed output.

@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/2

@LnL7
Copy link
Member

LnL7 commented May 27, 2019

@GrahamcOfBorg build mkl

@LnL7
Copy link
Member

LnL7 commented May 27, 2019

Fetching the source fails, seems like that's not a stable url.

trying http://registrationcenter-download.intel.com/akdlm/irc_nas/tec/15235/m_mkl_2019.3.199.dmg
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
curl: (22) The requested URL returned error: 404 Not Found

@smaret
Copy link
Member Author

smaret commented May 29, 2019

Fetching the source fails, seems like that's not a stable url.

It's working now. Sounds like a hiccup with Intel's website.

@bhipple
Copy link
Contributor

bhipple commented May 30, 2019

Apologies for taking a while to look at this @smaret!

The libmkl_cdft_core.dylib isn't in the derivation for Linux. Is it intended @bhipple? Should we also drop it for Darwin?

I'm not sure; I don't have access to a Darwin machine to test. But if you don't need it then certainly we should drop it; this package is huge, which is why we have the fixed output derivation to avoid rebuilds whenever possible.

Copy link
Contributor

@bhipple bhipple left a comment

Choose a reason for hiding this comment

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

Is there a way we can leave this as a fixed output derivation?

pkgs/development/libraries/science/math/mkl/default.nix Outdated Show resolved Hide resolved
done
install_name_tool -change @rpath/libiomp5.dylib $out/lib/libiomp5.dylib $out/lib/libmkl_intel_thread.dylib
install_name_tool -change @rpath/libtbb.dylib $out/lib/libtbb.dylib $out/lib/libmkl_tbb_thread.dylib
install_name_tool -change @rpath/libtbbmalloc.dylib $out/lib/libtbbmalloc.dylib $out/lib/libtbbmalloc_proxy.dylib
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure we can modify binaries with the ISSL license agreement?

Copy link
Member Author

@smaret smaret May 30, 2019

Choose a reason for hiding this comment

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

The ISSL allows to "redistribute the software without modification". I doubt that changing the install_name of the libraries can be considered as a modification of the software. Since we're not installing the libraries in a standard location, we can't redistribute them (at least on Darwin) if we don't change the install name accordingly.

Copy link
Member

@Mic92 Mic92 Jun 3, 2019

Choose a reason for hiding this comment

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

If that would be a problem we would be pretty much screwed, because we use patchelf quite heavily in most derivations, including proprietary software.

Copy link
Member

Choose a reason for hiding this comment

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

I am not a lawyer, but I also would consider the ELF header as part of the actual software.

Copy link
Member

Choose a reason for hiding this comment

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

There is a precedent for build recipes that perform modification of mkl: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=intel-parallel-studio-xe#n523

@smaret
Copy link
Member Author

smaret commented Jun 3, 2019

The libmkl_cdft_core.dylib isn't in the derivation for Linux. Is it intended @bhipple? Should we also drop it for Darwin?

I'm not sure; I don't have access to a Darwin machine to test. But if you don't need it then certainly we should drop it; this package is huge, which is why we have the fixed output derivation to avoid rebuilds whenever possible.

You don't need it if you only use the MKL as a BLAS/LAPACK provider. But shouldn't we include all libraries for users that need other MKL functionalities?

@smaret
Copy link
Member Author

smaret commented Jun 3, 2019

Is there a way we can leave this as a fixed output derivation?

@veprbl suggested a way to do this (see his comment above). Personally I'm not convinced it's worth the effort. If I understand correctly, the fixed output derivation avoids rebuilding the package if some of the dependencies (rpmextract or undmg) changes. But my impression is that the MKL itself is updated much more often that its dependencies. Since you need to rebuild the package at each MKL update anyway, I'm not sure that the fixed output makes much of a difference.

@veprbl
Copy link
Member

veprbl commented Jun 3, 2019

If I understand correctly, the fixed output derivation avoids rebuilding the package if some of the dependencies (rpmextract or undmg) changes.

The reverse dependencies that are to change are not just rpmextract and undmg, it will be rebuilt on change of stdenvNoCC. I'm not sure how often it changes compared to stdenv, but that should probably be more often than the mkl updates.

Then if we are talking about a big download (as mentioned in #57947 (comment)) then using fixed output prevents Hydra from caching copies of the same files.
edit: since this package is unfree it is not built by Hydra nor does it go to cache

Closes NixOS#57697

Co-Authored-By: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Copy link
Contributor

@bhipple bhipple left a comment

Choose a reason for hiding this comment

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

Looks good to me!

# fixed-output derivations.
outputHashAlgo = "sha256";
outputHashMode = "recursive";
outputHash = "101krzh2mjbfx8kvxim2zphdvgg7iijhbf9xdz3ad3ncgybxbdvw";
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for keeping this fixed-output on Linux!

@bhipple
Copy link
Contributor

bhipple commented Jun 6, 2019

But my impression is that the MKL itself is updated much more often that its dependencies.

As mentioned by @veprbl, MKL is updated a couple times a year, while stdenv in NixPkg changes very frequently (daily/weekly). It's irrelevant for Hydra, since that isn't caching the package to begin with, but for users who have built and installed this package in their Nix store it's nice for them to not have to "rebuild" it on every NixOS update.

@veprbl
Copy link
Member

veprbl commented Jun 6, 2019

I agree with the summary by @bhipple. Let's merge this PR so that mkl becomes usable on Darwin, so that the extra rebuild issue can be solved later.

@veprbl veprbl merged commit cbaa9a7 into NixOS:master Jun 6, 2019
@smaret smaret deleted the fix-mkl branch June 7, 2019 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants