reorganize cmake files(issue #113) #120

noskill · 2018-07-11T21:54:15Z

It's my attempt at reorganizing cmake files #113

move Find*.cmake files from atomspace to cogutil for installation
generate CogUtilConfig.cmake so that FindCogUtil.cmake is not necessary

linas · 2018-07-11T23:15:19Z

Some fixes are needed; the comments are similar to those of opencog/atomspace#1832

Don't remove the COPYING file, it pertains to the files in this directory.
The valgrind, vagrant, buildbot, conf and spec files have nothing to do with cmake. They belong in some other, appropriately-named directory.

noskill · 2018-07-12T20:06:18Z

updated

ngeiswei · 2018-07-13T05:49:58Z

It seems to make sense (although,with the exception of cmake/CMakeLists.txt, I didn't carefully review it).

I suppose one of us need to test it on a fresh system before merging (I'll test it early next week if none of us has done it by then).

vsbogd · 2018-07-13T13:53:55Z

On my machine make install copied files CogUtilConfig.cmake CogUtilConfigVersion.cmake into root directory. I have passed installation path to cmake manually via CMAKE_INSTALL_PREFIX:

$ cmake -DCMAKE_INSTALL_PREFIX=$OPENCOG/test/build $OPENCOG/test/cogutils
$ make install
$ ls $OPENCOG/test/build
CogUtilConfig.cmake  CogUtilConfigVersion.cmake  include  lib  share

vsbogd

See comment above #120 (comment)
Solved by change #120 (review)

vsbogd · 2018-07-13T14:28:58Z

cmake/CMakeLists.txt

+       ${CMAKE_CURRENT_BINARY_DIR}/CogUtilConfigVersion.cmake
+       ${CMAKE_CURRENT_BINARY_DIR}/CogUtilConfig.cmake
+       DESTINATION
+       ${CMAKE_INSTALL_PREFIX})


Should be ${LIB_INSTALL_DIR}/CogUtil/cmake

vsbogd

Cannot make clean build after CogUtil is installed. Cmake finds CogUtilConfig.cmake file but include paths doesn't work:

 [  2%] Building CXX object opencog/truthvalue/CMakeFiles/truthvalue.dir/AttentionValue.cc.o
/home/vital/projects/opencog/test/atomspace/opencog/truthvalue/AttentionValue.cc:25:10: fatal error: opencog/util/exceptions.h: No such file or directory
 #include <opencog/util/exceptions.h>
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/vital/projects/opencog/test/atomspace/opencog/atoms/proto/FloatValue.cc:23:10: fatal error: opencog/util/exceptions.h: No such file or directory
 #include <opencog/util/exceptions.h>
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~

noskill · 2018-07-13T20:48:47Z

@vsbogd Thanks, i fixed include path, it should work now with custom prefix.

vsbogd · 2018-07-13T21:01:39Z

Checked again, CogUtil buils succesfully. I also can build atomspace but cannot build atomspace unit tests. See linkage error:

[ 17%] Linking CXX executable DefineLinkUTest
CMakeFiles/TVMergeUTest.dir/TVMergeUTest.cpp.o: In function `TVMergeUTest::testMerge()':
/home/vital/projects/opencog/test/atomspace-build/tests/truthvalue/../../../atomspace/tests/truthvalue/TVMergeUTest.cxxtest:96: undefined reference to `opencog::logger()'
...

vsbogd · 2018-07-14T09:13:12Z

CogUtil build successful, unit tests passed. Atomspace build successful, unit tests failed, but this is equal to baseline (opencog/atomspace master branch):

96% tests passed, 4 tests failed out of 113

Total Test time (real) = 233.34 sec

The following tests FAILED:
	 49 - MultiAtomSpaceUTest (Failed)
	 53 - VectorAPIUTest (Failed)
	 99 - NestedPutUTest (Failed)
	110 - BackwardChainerUTest (Failed)

linas · 2018-07-14T21:37:17Z

Um, these unit tests never-ever fail for me; they've been passing just fine for years (well two of them are newer than that, but they always pass) For me, 121 tests total run, and they always pass.

We need a distinct bug report for each, to track down the problems. VectorAPI might be failing for the same reason that Nil has already reported.

amebel · 2018-07-16T08:59:58Z

Master branch unit test states

cogutil master sha = d789e4c                                          
100% tests passed, 0 tests failed out of 12                       
                                                                  
-----------------------                                           
atomspace master sha = ac76f43                                        
100% tests passed, 0 tests failed out of 120                      
                                                                  
-----------------------                                           
opencog master sha = cc9e331                                          
80% tests passed, 4 tests failed out of 20                        
                                                                  
Total Test time (real) = 225.43 sec                               
                                                                  
The following tests FAILED:                                       
          3 - CogServerUTest (SEGFAULT)                           
          6 - AtomOcTreeUTest (SEGFAULT)                          
          7 - TimeSpaceAtomUTest (SEGFAULT)                       
         11 - PLNRulesUTest (Failed)

Pull request's unit test states

cogutil pr 120 sha = 8f7c7e5                                          
100% tests passed, 0 tests failed out of 12                       
                                                                  
-----------------------                                           
atomspace pr 1832 sha= f9809df                                       
atomspace pr 1823 rebased on master sha = 62701cd                                     
100% tests passed, 0 tests failed out of 120                      
                                                                  
-----------------------                                           
opencog master = cc9e331                                          
80% tests passed, 4 tests failed out of 20                        
                                                                  
Total Test time (real) = 225.76 sec                               
                                                                  
The following tests FAILED:                                       
          3 - CogServerUTest (SEGFAULT)                           
          6 - AtomOcTreeUTest (SEGFAULT)                          
          7 - TimeSpaceAtomUTest (SEGFAULT)                       
         11 - PLNRulesUTest (Failed)

vsbogd · 2018-07-17T11:24:29Z

Raised issue opencog/atomspace#1834 on VectorAPIUTest failure. Looks like it fails on clean build because there are no OpenCog node types which are used in test data.

vsbogd · 2018-07-17T12:01:14Z

Other tests fail because I didn't do make install before running. Raised separate issue opencog/atomspace#1835 on this

vsbogd · 2018-07-17T12:48:14Z

cmake/OpenCogFunctions.cmake

+    IF ("${MODULE_NAME}.scm" STREQUAL "${FILE_NAME}")
+        EXECUTE_PROCESS(
+            COMMAND ${CMAKE_COMMAND} -E make_directory ${GUILE_SYMLINK_DIR}/${MODULE_FILE_DIR_PATH}
+            COMMAND ${CMAKE_COMMAND} -E create_symlink "${CMAKE_CURRENT_SOURCE_DIR}/${FILE_NAME}" "${GUILE_SYMLINK_DIR}/${MODULE_FILE_DIR_PATH}/${FILE_NAME}"


It is not this PR issue but rather an issue that exists in opencog/atomspace master branch.

EXECUTE_PROCESS chaining make_directory and create_symlink actions fails sporadically because directory is not created at the moment when symlinks is being created:

failed to create symbolic link '/tmp/test/atomspace-build/opencog/scm//opencog/matrix.scm': No such file or directory

Can be fixed by replacing one EXECUTE_PROCESS with two commands by two EXECUTE_PROCESS lines with one command in each:

IF ("${MODULE_NAME}.scm" STREQUAL "${FILE_NAME}") EXECUTE_PROCESS( COMMAND ${CMAKE_COMMAND} -E make_directory ${GUILE_SYMLINK_DIR}/${MODULE_FILE_DIR_PATH} ) EXECUTE_PROCESS( COMMAND ${CMAKE_COMMAND} -E create_symlink "${CMAKE_CURRENT_SOURCE_DIR}/${FILE_NAME}" "${GUILE_SYMLINK_DIR}/${MODULE_FILE_DIR_PATH}/${FILE_NAME}" ) SET(FILE_INSTALL_PATH "${GUILE_INSTALL_DIR}/${MODULE_FILE_DIR_PATH}" PARENT_SCOPE ) ELSE() EXECUTE_PROCESS( COMMAND ${CMAKE_COMMAND} -E make_directory ${GUILE_SYMLINK_DIR}/${MODULE_DIR_PATH} ) EXECUTE_PROCESS( COMMAND ${CMAKE_COMMAND} -E create_symlink "${CMAKE_CURRENT_SOURCE_DIR}/${FILE_NAME}" "${GUILE_SYMLINK_DIR}/${MODULE_DIR_PATH}/${FILE_NAME}" ) SET(FILE_INSTALL_PATH "${GUILE_INSTALL_DIR}/${MODULE_DIR_PATH}" PARENT_SCOPE ) ENDIF()

Raising it here because it should be fixed either in this PR or after this PR is merged.

ngeiswei · 2018-07-18T08:29:57Z

Testing this is proving more difficult. I could persevere but it seems it has been sufficiently tested by you guy (thanks a lot for your effort!).

I'll wait 24h in case you have something else to say about it and I'll merge.

noskill · 2018-07-19T07:03:21Z

@ngeiswei I moved some opencog cmake files here as well.

vsbogd · 2018-07-19T08:26:49Z

@noskill, I would prefer raising next PR to move .cmake files from opencog/opencog to opencog/cogutil to not test this PR twice.

ngeiswei · 2018-07-19T10:10:07Z

@vsbogd I've sent you an invite so you have merging rights, to share a bit the burden :-) hopefully nobody has something against that and I trust you'll use your rights wisely.

vsbogd · 2018-07-19T10:45:57Z

Thanks, @ngeiswei, will do my best to do the things right.

noskill · 2018-07-19T14:07:06Z

@vsbogd I removed commit with opencog cmake files..

linas · 2018-07-19T17:43:51Z

merging rights

Yeah, OK.

ngeiswei · 2018-07-20T10:55:53Z

Thanks @noskill, and thanks @vsbogd for taking care of it.

linas · 2018-11-29T19:01:26Z

FYI, in issue opencog/atomspace#1923, it has been observed that three files were moved from atomspace to cogutils by mistake: OpenCogAtomTypes.cmake, OpenCogFunctions.cmake and OpenCogMacros.cmake I'm encouraging the required pull reqs to move them back to the atomspace, and remove them from cogutils.

noskill · 2018-11-30T17:24:33Z

@linas OPENCOG_ADD_ATOM_TYPES defined in OpenCogMacros.cmake is used in other projects, not only in atomspace. I think that commonly used cmake files better stay in cogutil.

smigad · 2018-11-30T17:45:57Z

But that is only useful if the atomspace is installed. It makes sense to have it in the atomspace repo don't it.

linas · 2018-11-30T19:18:23Z

@noskill Yes, I'm the one who designed it so that other projects use OPENCOG_ADD_ATOM_TYPES. :-) So @Dagiopia is correct, that macro only makes sense if the atomspace is installed; it references files that exist only in the atomspace, and requires glue code that exists only in the atomspace.

It really did get moved by accident; I just wasn't paying attention when your original pull req went through; and apparently no one else was, either. Accidents happen.

linas · 2018-11-30T19:21:05Z

The point is: @Dagiopia please do go ahead to create a pull req to fix this. It will take a while to get through, as we need a cooling-off period to make sure that the other repos (opencog, as-moses and agi-bio) are not adversely affected. With a bit of luck, this change should be invisible to the other repos.

noskill force-pushed the cmake-br branch from de9930c to dff8b3e Compare July 12, 2018 05:52

noskill added 4 commits July 12, 2018 22:09

move FindCxxtest.cmake from atomspace

bca833d

move Find* files to cmake directory

a2ce43a

copy Find*.cmake files from atomspace

2704512

generate and install cmake files(issue opencog#113)

3aa518a

noskill force-pushed the cmake-br branch from dff8b3e to 3aa518a Compare July 12, 2018 19:42

installing copyright notice

25af65c

linas assigned ngeiswei Jul 12, 2018

linas requested a review from ngeiswei July 12, 2018 23:23

linas unassigned ngeiswei Jul 12, 2018

vsbogd approved these changes Jul 13, 2018

View reviewed changes

vsbogd requested changes Jul 13, 2018

View reviewed changes

vsbogd reviewed Jul 13, 2018

View reviewed changes

correct include path in CogUtilConfig

e8697b0

noskill force-pushed the cmake-br branch from 15463c0 to e8697b0 Compare July 13, 2018 20:44

set COGUTIL_LIBRARY in CogUtilConfig.cmake

8f7c7e5

vsbogd approved these changes Jul 14, 2018

View reviewed changes

vsbogd reviewed Jul 17, 2018

View reviewed changes

noskill force-pushed the cmake-br branch from 55c31b3 to 8f7c7e5 Compare July 19, 2018 13:23

fix making symlinks in OpenCogFunctions.cmake opencog/atomspace#1835

a5e2658

vsbogd merged commit a338378 into opencog:master Jul 20, 2018

vsbogd mentioned this pull request Jul 20, 2018

Organise OpenCog .cmake files #113

Closed

SomeUserName1 mentioned this pull request Jul 27, 2018

Dont delete the SystemActivityTable on the end of the serverloop opencog/opencog#3275

Merged

noskill deleted the cmake-br branch August 3, 2018 11:36

linas mentioned this pull request Nov 29, 2018

Multi-repo entanglement opencog/atomspace#1923

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reorganize cmake files(issue #113) #120

reorganize cmake files(issue #113) #120

noskill commented Jul 11, 2018 •

edited

Loading

linas commented Jul 11, 2018

noskill commented Jul 12, 2018

ngeiswei commented Jul 13, 2018

vsbogd commented Jul 13, 2018 •

edited

Loading

vsbogd left a comment •

edited

Loading

vsbogd Jul 13, 2018 •

edited

Loading

noskill Jul 13, 2018

vsbogd left a comment

noskill commented Jul 13, 2018

vsbogd commented Jul 13, 2018

vsbogd commented Jul 14, 2018

linas commented Jul 14, 2018

amebel commented Jul 16, 2018

vsbogd commented Jul 17, 2018

vsbogd commented Jul 17, 2018

vsbogd Jul 17, 2018

ngeiswei commented Jul 18, 2018

noskill commented Jul 19, 2018

vsbogd commented Jul 19, 2018

ngeiswei commented Jul 19, 2018

vsbogd commented Jul 19, 2018

noskill commented Jul 19, 2018

linas commented Jul 19, 2018

ngeiswei commented Jul 20, 2018

linas commented Nov 29, 2018

noskill commented Nov 30, 2018

smigad commented Nov 30, 2018

linas commented Nov 30, 2018

linas commented Nov 30, 2018

reorganize cmake files(issue #113) #120

reorganize cmake files(issue #113) #120

Conversation

noskill commented Jul 11, 2018 • edited Loading

linas commented Jul 11, 2018

noskill commented Jul 12, 2018

ngeiswei commented Jul 13, 2018

vsbogd commented Jul 13, 2018 • edited Loading

vsbogd left a comment • edited Loading

Choose a reason for hiding this comment

vsbogd Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

noskill Jul 13, 2018

Choose a reason for hiding this comment

vsbogd left a comment

Choose a reason for hiding this comment

noskill commented Jul 13, 2018

vsbogd commented Jul 13, 2018

vsbogd commented Jul 14, 2018

linas commented Jul 14, 2018

amebel commented Jul 16, 2018

Master branch unit test states

Pull request's unit test states

vsbogd commented Jul 17, 2018

vsbogd commented Jul 17, 2018

vsbogd Jul 17, 2018

Choose a reason for hiding this comment

ngeiswei commented Jul 18, 2018

noskill commented Jul 19, 2018

vsbogd commented Jul 19, 2018

ngeiswei commented Jul 19, 2018

vsbogd commented Jul 19, 2018

noskill commented Jul 19, 2018

linas commented Jul 19, 2018

ngeiswei commented Jul 20, 2018

linas commented Nov 29, 2018

noskill commented Nov 30, 2018

smigad commented Nov 30, 2018

linas commented Nov 30, 2018

linas commented Nov 30, 2018

noskill commented Jul 11, 2018 •

edited

Loading

vsbogd commented Jul 13, 2018 •

edited

Loading

vsbogd left a comment •

edited

Loading

vsbogd Jul 13, 2018 •

edited

Loading