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

Fix cmake install on dev branch #3248

Merged
merged 13 commits into from
Oct 15, 2019
Merged

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Oct 15, 2019

Porting #3228 to the dev branch

Description of changes:

  • fix the CMake build system to include libraries in the list of installed files and test it in a dedicated CI job
  • fix the versioning of Cython .so shared objects
  • simplify the CMake tests

junghans and others added 9 commits October 15, 2019 09:32
Installing ESPResSo with `make install DESTDIR="/path/to/es"` will
install files in the wrong folder and set an incorrect runtime path
in Cython shared objects, causing import errors. See full report:
espressomd#3228 (comment)
When installing ESPResSo, the C++ shared objects and Cython shared
objects are stored in the same folder. Since they have the same .so
extension, when importing espressomd.cluster_analysis, the file
cluster_analysis.py is loaded and the file cluster_analysis.so is
automatically considered a Cython shared object, and Python fails
to find the Cython-specific symbol PyInit_cluster_analysis. This
is remediated by giving a different name to the C++ shared object.
Install the files in a different directory and check the python and
tutorials/samples tests can still run with the installed files.
Please note that pypresso will experience a major slowdown in CI
when located outside the build directory, hence the 3h timeout.
This job is meant to be run only during releases and in PRs that
add new core modules.
Takes more than 1 hour to run in CI when pypresso is located
outside the build directory.
@codecov
Copy link

codecov bot commented Oct 15, 2019

Codecov Report

Merging #3248 into python will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #3248   +/-   ##
======================================
- Coverage      85%     85%   -1%     
======================================
  Files         530     528    -2     
  Lines       25795   25790    -5     
======================================
- Hits        22168   22146   -22     
- Misses       3627    3644   +17
Impacted Files Coverage Δ
src/core/integrate.cpp 67% <0%> (-8%) ⬇️
src/core/dpd.cpp 98% <0%> (-2%) ⬇️
src/core/event.cpp 96% <0%> (-1%) ⬇️
src/core/npt.cpp 93% <0%> (-1%) ⬇️
src/core/virtual_sites/VirtualSitesRelative.cpp 85% <0%> (-1%) ⬇️
src/core/electrostatics_magnetostatics/p3m.cpp 86% <0%> (-1%) ⬇️
src/core/rotation.hpp 100% <0%> (ø) ⬆️
src/utils/tests/quaternion_test.cpp
src/utils/include/utils/math/quaternion.hpp
src/core/layered.cpp 78% <0%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94d51f5...1eb21f3. Read the comment docs.

The installed files only exist if the Python bindings are enabled.
Copy link
Member

@KaiSzuttor KaiSzuttor left a comment

Choose a reason for hiding this comment

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

besides comment LGTM

testsuite/cmake/CMakeLists.txt Outdated Show resolved Hide resolved
The clang:6.0 job fails due to undefined ASAN symbols.
The logic to fix that error is in the pypresso wrapper.
@jngrad
Copy link
Member Author

jngrad commented Oct 15, 2019

bors r=KaiSzuttor

bors bot added a commit that referenced this pull request Oct 15, 2019
3248: Fix cmake install on dev branch r=KaiSzuttor a=jngrad

Porting #3228 to the dev branch

Description of changes:
- fix the CMake build system to include libraries in the list of installed files and test it in a dedicated CI job
- fix the versioning of Cython .so shared objects
- simplify the CMake tests


3251: Particle data r=jngrad a=fweik

Fixes #3157.

Description of changes:
 - Split definition of `struct Particle` from rest of particle data.


Co-authored-by: Christoph Junghans <junghans@lanl.gov>
Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
Co-authored-by: Florian Weik <fweik@icp.uni-stuttgart.de>
@bors
Copy link
Contributor

bors bot commented Oct 15, 2019

Build succeeded

@bors bors bot merged commit 1eb21f3 into espressomd:python Oct 15, 2019
@jngrad jngrad deleted the fix-cmake-install-dev branch January 18, 2022 12:09
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.

3 participants