Clear up CMake confusion under Majel Directory

[gangliao]

I found there are some confusion in #2118, I will explain all of them in here.

[gangliao]

TO @helinwang

请问为何既然有了

add_dependencies(majel_tests majel)
还需要

target_link_libraries(majel_tests
...
majel
)
感觉不是重复地指定依赖了？
我试了一下删掉add_dependencies，貌似也能编译。

其实是这样，在cmake中，add_library负责指定需要生成的库，add_dependencies 表示在编译当前target之前，应该先编译某几个target， 比如编译majel_tests之前就应该先编译majel库，因为他们是有先后的依赖关系，这样才能保证编译不会出错。

删掉add_dependencies，貌似也能编译。其实不是这样，如果依赖关系更复杂一些，并且你开启并行编译的话，make -j8, 肯定会出错的

target_link_libraries 用于在最终生成二进制时候(add_executable)，对于依赖的库进行链接的操作, 保证需要的symbol符号能够被链接到二进制里面去。

include(external/gtest)

Could you explain where does "external" directory come from (have not found any directory > named "external" after building majel)? This line works, but I don't understand how it works...

因为我们所有的第三方库都是采用了CMake的Add_ExternalProject来生成的，所以当包含include(external/gtest), CMake会自动的下载，编译，安装gtest. external/gtest 其实是在Paddle/cmake/extern/gtest.cmake

include_directories(${Boost_INCLUDE_DIRS})
include_directories(${CMAKE_CURRENT_SOURCE_DIR})
我对cmake不是很熟悉，为何这里要把当前目录给include了？repo/paddle下面貌似没有source code呀。

CMAKE_CURRENT_SOURCE_DIR 表示 The path to the source directory currently being processed.

CMAKE_SOURCE_DIR 表示 The path to the top level of the source tree.

所以这里的CMAKE_CURRENT_SOURCE_DIR 其实就是 Paddle/paddle/ 下面这个目录。
inlude 这个目录是因为，我看到majel里面很多代码是直接#include<majel/xxx.h>，这样能够保证编译的时候，顺利的找到和解析头文件。

我记得design doc里说用轻量级的boost替代品，这里仍然是用的boost吗？

下一个PR就换哈

third_party要不要放到运行cmake的目录（比如./build/）里面，而不是${CMAKE_CURRENT_SOURCE_DIR}里面?这样用户如果删了./build/就肯定是一个clean build。

最早的时候，我也确实是你这样设计的，但是因为是第三方的库文件，其实我们一般是不应该删的，移到专门的third_party目录，即使你rm -rf build了, 重新编译的时候，也不需要再次编译第三方依赖。

[gangliao]

TO @wangkuiyi

+if(GTEST_INCLUDE_DIR AND GTEST_LIBRARIES)
Given that we are building Docker container, we can assume in CMakeLists.txt that we have boost > and gtest already installed and don't need conditions like this, isn't it?

You can ignore this code snippet. Because it's to support the Majel compilation under Paddle/paddle/majel directory, not Paddle directory

if(GTEST_INCLUDE_DIR AND GTEST_LIBRARIES)
...
else()
**Acutally, this part is only needed when you want to build
majel in `Paddle/paddle/majel` directory**.
Because it's an independent  `CMake build project` in
`Paddle/paddle/majel`, It cannot get any global
information from Paddle CMake.
endif()

Why we need this file? I think we can just link -lgtest_main to have the main function?

Yeah, sounds reasonable. I will try this in next PR.

Sorry that I forgot to reorder the include files following >https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes. You can do that > in this PR.

I think the right order is:

#include "majel/place.h"

#include

#include "gtest/gtest.h"

I didn't modify the order of header files. I guess clang-format will do it automatically.

I agree that if a source file doesn't include CUDA code, the suffix shouldn't be .cu. But I personally prefer .cc over .cpp following Google C++ Style Guide.

me too. maybe next PR.

[wangkuiyi]

Great! Thanks!

I created the following issues as the action items:

• Remove package and use -lgtest_main #2124
• In majel package, using Google Style filename conventions #2125