-
Notifications
You must be signed in to change notification settings - Fork 189
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
CMake minor fixes #3258
CMake minor fixes #3258
Conversation
Codecov Report
@@ Coverage Diff @@
## python #3258 +/- ##
======================================
- Coverage 85% 85% -1%
======================================
Files 531 531
Lines 25796 25796
======================================
- Hits 22169 22168 -1
- Misses 3627 3628 +1
Continue to review full report at Codecov.
|
@@ -20,6 +20,7 @@ | |||
|
|||
cmake_minimum_required(VERSION 3.4) | |||
include(FeatureSummary) | |||
include(GNUInstallDirs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the block
if(NOT DEFINED BINDIR)
set(BINDIR "bin")
endif(NOT DEFINED BINDIR)
in line 401 should be removed now. Did you check that the paths provided by GNUInstallDirs
are used for all install targets? I did a quick grep and did find nothing, but I only checked the install
commands, not sure
if this effects rpaths and so on....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I overlooked that one. The install()
commands have to be configured such that submodules get installed in the python espressomd
folder, if not the new test in c43d057 will detect that (it searches for misplaced files in CMAKE_INSTALL_PREFIX
). The GNU variables inherit from CMAKE_INSTALL_PREFIX
, so we should use them as often as possible.
I've a few more improvements. |
The Python guard isn't necessary anymore because submodules are now installed in the Python package espressomd, and the testsuite itself already has a Python guard in the top-level CMakeLists.txt.
The failures in CI are actually a bug in the build system, not a regression. Working on it. |
Remove the non-standard CMAKE_INSTALL_EXEC_PREFIX variable in favor of CMAKE_INSTALL_PREFIX. Install espressomd in the standard python3.X folder instead of the deprecated python3 folder. Remove duplicated code logic. Use explicit keyword arguments in calls to functions from distutils.sysconfig.
@@ -241,7 +242,7 @@ if(WITH_PYTHON) | |||
execute_process( | |||
COMMAND | |||
${PYTHON_EXECUTABLE} -c | |||
"import distutils.sysconfig as cg; print(cg.get_python_lib(1,0,prefix='${CMAKE_INSTALL_EXEC_PREFIX}'))" | |||
"import distutils.sysconfig as cg; print(cg.get_python_lib(prefix='${CMAKE_INSTALL_PREFIX}', plat_specific=True, standard_lib=False).replace('${CMAKE_INSTALL_PREFIX}/', '', 1))" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@junghans this change might affect your packaging workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for asking, but it won't as PYTHON_INSTDIR
is set for packaging, so this call isn't actually used.
bors r=fweik |
3216: Refactor ghosts.cpp r=KaiSzuttor a=hirschsn More refactoring that builds upon #3212 Description of *major* changes: - remove GhostCommunication::mpi_comm as it is not used in ghosts.cpp, - make GhostCommunication::part_lists a std::vector, - remove static variables s_buffer and r_buffer, - factor out memory handling, - change loops to range based for, - use boost::mpi. - Replace the manual poststore and prefetch loops by find_ifs Mainly, ghosts.cpp now defines CommBuf, which is a container for the data to be sent or received as well as two classes (Archiver and BondArchiver), that insert and extract the memory from CommBuf. Some of these changes, like the removal of static variables, is necessary for my implementation of asynchronous ghost communication. PR Checklist ------------ - [ ] Tests? - [ ] Interface - [ ] Core - [ ] Docs? 3239: Added test criteria for the charged_system-2 tutorial r=RudolfWeeber,jngrad a=reinaual 3253: Refactor NpT public interface r=fweik a=jngrad Description of changes: - remove the silent conversion of the incorrect input parameter `dimension=[0,0,0]` to `[1,1,1]` in the core (bypassing sanity checks), now the checks will throw an exception for fixed-volume NpT; the original behavior was counter-intuitive and undocumented until 2 days ago - remove the automatic decay of NpT to NVT upon initialization of NpT with incorrect parameters - remove unused `p_inst_av` variable (average instantaneous pressure) - cleanup integrator documentation 3258: CMake minor fixes r=fweik a=jngrad Description of changes: - change next milestone to 4.2 - load `GNUInstallDirs` to make standard GNU paths accessible from CMake variables - simplify CMake logic and install in `python3.X` folder instead of the deprecated `python3` folder - add extra check to make sure install paths are correctly configured (all python and shared object files must be inside the package `espressomd`) Co-authored-by: Steffen Hirschmann <[email protected]> Co-authored-by: Florian Weik <[email protected]> Co-authored-by: Alexander Reinauer <[email protected]> Co-authored-by: Jean-Noël Grad <[email protected]>
Build succeeded |
Description of changes:
GNUInstallDirs
to make standard GNU paths accessible from CMake variablespython3.X
folder instead of the deprecatedpython3
folderespressomd
)