Closed noskill closed 5 years ago
That's a fair point @AmeBel. Although at the present time as-moses is still fully backward compatible with moses so it shouldn't be too problematic, but it does open the door to confusions.
@pennachin what do you think? Should as-moses supplant moses, or should they be allowed to live side by side?
I suppose the latter is better but it does require to rename the CMake project as-moses
as well as the various include and lib paths and configuration files.
MOSES shouldn't be removed any time soon, it's actually used by MOZI and as-moses is a researchy project that shouldn't be burdened with supporting production use at the moment.
I would say for this PR at least internal definitions can be changed to something like AsMoses
and as-moses MOSESConfig.cmake
will define MOSES variables using new definitions, like the following:
@PACKAGE_INIT@
include("@CMAKE_INSTALL_PREFIX@/lib/cmake/AsMoses/AsMosesTargets.cmake")
set(MOSES_LIBRARY asmoses)
set(MOSES_EXEC_LIBRARY asmoses_exec)
set(COMBOANT_LIBRARY asmoses_comboant)
set(COMBOREDUCT_LIBRARY asmoses_comboreduct)
set(FEATURE_SELECTION_LIBRARY asmoses_feature_selection)
# Copy the results to the output variables.
IF (MOSES_LIBRARY)
SET(MOSES_FOUND 1)
SET(MOSES_LIBRARIES ${MOSES_LIBRARY} ${MOSES_EXEC_LIBRARY}
${COMBOANT_LIBRARY} ${COMBOREDUCT_LIBRARY}
${FEATURE_SELECTION_LIBRARY})
MESSAGE(STATUS "Found MOSES library: ${MOSES_LIBRARIES}")
ELSE (MOSES_LIBRARY)
SET(MOSES_FOUND 0)
SET(MOSES_LIBRARIES)
ENDIF (MOSES_LIBRARY)
MARK_AS_ADVANCED(
MOSES_LIBRARIES
)
set(MOSES_INCLUDE_DIR "@CMAKE_INSTALL_PREFIX@/include/")
set(MOSES_VERSION "@ASMOSES_VERSION@")
set(MOSES_FOUND 1)
@ngeiswei, @AmeBel, @noskill - what do you think?
Then https://github.com/opencog/as-moses/issues/22 means that one should replace MOSESConfig.cmake by ASMOSESConfig.cmake, rename MOSES_xxx to ASMOSES_xxx and use it in opencog.
@vsbogd yes renaming the libraries, executables, and cmake-variables ensures that users can use both as-moses and moses installed on their system, without any conflict, and so it makes sense to me.
@vsbogd That makes sense. I'll do necessary renames..
Looks good for me.
As far as I see Anatoly creates ASMOSESConfig.cmake
file instead of MOSESConfig.cmake
so both libraries can be installed simultaneously. If client uses MOSES
one it should call FIND(MOSES)
in CMakeLists.txt, in case of ASMOSES
FIND(ASMOSES)
should be used. Both configs exports the same set of MOSES_
variables so no other client code need to be modified to switch between libraries.
@AmeBel, @ngeiswei, I think it is better to make a smokecheck for this change. Could you please help with it?
@AmeBel Thanks for testing this. I run the build in clean environment, linking should work now..
@AmeBel I made new PR for updating include path for the test https://github.com/opencog/as-moses/pull/25
@noskill Thanks all builds run and 37 tests pass. There seem to be certain targets (e.g moses, ant_scoring ) that are not renamed, why is that?
@ngeiswei I don't have a sense of the structure of the code, so please review to ensure that the changes make sense, and wouldn't cause conflict with moses
.
@AmeBel I didn't rename ant_scoring since it don't get installed, so it shouldn't be a problem. Moses is renamed with set_target_properties(moses PROPERTIES OUTPUT_NAME "asmoses"). I'll recheck other targets..
Ok, it seems all files that are installed either renamed or put into different directory.
Thanks a lot for the work and reviews guys!
This PR: 1) creates config-file cmake package, so that we can rid off of FindMoses.cmake in projects that use moses. 2) deletes cmake files that are provided by cogutil
see issue https://github.com/opencog/cogutil/issues/113