Remove XEUS_CPP_INCLUDE_DOCS in CMakeLists.txt

[mcbarton]

Description

Please include a summary of changes, motivation and context for this PR.

As the cmake is not currently designed to build the docs despite having XEUS_CPP_INCLUDE_DOCS defined this PR removes the section which mentions this variable. The variable is assumed to be part of an old PR which never got spotted before merge.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

• Bug fix
• New feature
• Added/removed dependencies
• Required documentation updates

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.56%. Comparing base (ca8c42e) to head (8db76e9).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #273      +/-   ##
==========================================
+ Coverage   80.54%   80.56%   +0.02%     
==========================================
  Files          19       20       +1     
  Lines         956      957       +1     
  Branches       88       88              
==========================================
+ Hits          770      771       +1     
  Misses        186      186              

see 2 files with indirect coverage changes

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[anutosh491]

Okay wait, this gets me somewhat confused though :\

If you see the cmake, we have

xeus-cpp/CMakeLists.txt

Lines 556 to 558 in ec63cc3

	if(XEUS_CPP_INCLUDE_DOCS)
	add_subdirectory(docs)
	endif()

But this would want a CMakeLists.txt inside the docs folder correct ? (And we don't have that)

Building with XEUS_CPP_INCLUDE_DOCS=ON shows me this

(xeus-cpp) anutosh491@arm64-apple-darwin20 xeus-cpp % mkdir build
cd build
cmake .. -D CMAKE_PREFIX_PATH=$CONDA_PREFIX -D CMAKE_INSTALL_PREFIX=$CONDA_PREFIX -D CMAKE_INSTALL_LIBDIR=lib -D XEUS_CPP_INCLUDE_DOCS=ON
make install
-- The C compiler identification is Clang 16.0.6
-- The CXX compiler identification is Clang 16.0.6
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Users/anutosh491/micromamba/envs/xeus-cpp/bin/arm64-apple-darwin20.0.0-clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Users/anutosh491/micromamba/envs/xeus-cpp/bin/arm64-apple-darwin20.0.0-clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Building xeus-cpp v0.7.0dev
-- Found nlohmann_json: /Users/anutosh491/micromamba/envs/xeus-cpp/share/cmake/nlohmann_json/nlohmann_jsonConfig.cmake (found suitable exact version "3.11.3")
-- Found compatible xeus version: 5.2.0
-- Found CppInterOp: config=/Users/anutosh491/micromamba/envs/xeus-cpp/lib/cmake/CppInterOp dir=/Users/anutosh491/micromamba/envs/xeus-cpp (found version=1.6.0 compatible with Clang 19.x)
Configure kernels: ...
-- Performing Test HAS_CPP_17_FLAG
-- Performing Test HAS_CPP_17_FLAG - Success
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found OpenSSL: /Users/anutosh491/micromamba/envs/xeus-cpp/lib/libcrypto.dylib (found version "3.4.1")
-- Performing Test HAS_MARCH_NATIVE
-- Performing Test HAS_MARCH_NATIVE - Success
CMake Error at CMakeLists.txt:556 (add_subdirectory):
  The source directory

    /Users/anutosh491/work/xeus-cpp/docs

  does not contain a CMakeLists.txt file.


-- Configuring incomplete, errors occurred!
make: *** No rule to make target 'install'.  Stop.

So I don't even think we are building our docs dependening on this config here !!

Let's just get rid of the following here.

if(XEUS_CPP_INCLUDE_DOCS)
    add_subdirectory(docs)
endif()

[mcbarton]

Okay wait, this gets me very confused though :\

If you see the cmake, we have

xeus-cpp/CMakeLists.txt

Lines 556 to 558 in ec63cc3

	if(XEUS_CPP_INCLUDE_DOCS)
	add_subdirectory(docs)
	endif()

But this would want a CMakeLists.txt inside the docs folder correct ? (And we don't have that)

(xeus-cpp) anutosh491@arm64-apple-darwin20 xeus-cpp % mkdir build
cd build
cmake .. -D CMAKE_PREFIX_PATH=$CONDA_PREFIX -D CMAKE_INSTALL_PREFIX=$CONDA_PREFIX -D CMAKE_INSTALL_LIBDIR=lib -D XEUS_CPP_INCLUDE_DOCS=ON
make install
-- The C compiler identification is Clang 16.0.6
-- The CXX compiler identification is Clang 16.0.6
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Users/anutosh491/micromamba/envs/xeus-cpp/bin/arm64-apple-darwin20.0.0-clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Users/anutosh491/micromamba/envs/xeus-cpp/bin/arm64-apple-darwin20.0.0-clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Building xeus-cpp v0.7.0dev
-- Found nlohmann_json: /Users/anutosh491/micromamba/envs/xeus-cpp/share/cmake/nlohmann_json/nlohmann_jsonConfig.cmake (found suitable exact version "3.11.3")
-- Found compatible xeus version: 5.2.0
-- Found CppInterOp: config=/Users/anutosh491/micromamba/envs/xeus-cpp/lib/cmake/CppInterOp dir=/Users/anutosh491/micromamba/envs/xeus-cpp (found version=1.6.0 compatible with Clang 19.x)
Configure kernels: ...
-- Performing Test HAS_CPP_17_FLAG
-- Performing Test HAS_CPP_17_FLAG - Success
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found OpenSSL: /Users/anutosh491/micromamba/envs/xeus-cpp/lib/libcrypto.dylib (found version "3.4.1")
-- Performing Test HAS_MARCH_NATIVE
-- Performing Test HAS_MARCH_NATIVE - Success
CMake Error at CMakeLists.txt:556 (add_subdirectory):
  The source directory

    /Users/anutosh491/work/xeus-cpp/docs

  does not contain a CMakeLists.txt file.


-- Configuring incomplete, errors occurred!
make: *** No rule to make target 'install'.  Stop.

So I don't even think we are building our docs dependening on this config here !!

Let's just get rid of the following here.

if(XEUS_CPP_INCLUDE_DOCS)
    add_subdirectory(docs)
endif()

Doesn't this suggest a bug in the cmake, and wouldn't it make more sense to add the necessary CMakeLists.txt so we can build the docs from the cmake?

[anutosh491]

Hmmm, let's hear what @JohanMabille has to say here.

I haven't seen docs being built this way on the other xeus kernels I've worked with so a bit confused here. But yeah probably would demand a CMakeLists.txt in the docs if we use this.

[JohanMabille]

Hi, I don't know why XEUS_CPP_INCLUDE_DOCS was initially added to the CMakeLists.txt, maybe it was part of a bigger refactoring / change that has not been fully merged, or a leftover? @mvassilev presumably knows more about it.

The current mechanism for building the docs is independent from cmake, one needs to run make html / make pdf / ... under the docs folder after installing the doc environment. The way it is done is "historical" (i.e. I don't remember the details, although I think it is more or less related to readthedocs constraints ;)) and is consistent with the builds of the docs of all the other xeus repos. Not that I am against changing it, but as @mcbarton pointed it out in his last comment, it would probably require more changes to the CMakeLists.txt to get it work. Another option could be to keep the current docs build mechanism and totally remove the XEUS_CPP_INCLUDE_DOCS from the CMakeLists to avoid confusion in the future.

[anutosh491]

Thanks for the reply.

Another option could be to keep the current docs build mechanism and totally remove the XEUS_CPP_INCLUDE_DOCS from the CMakeLists to avoid confusion in the future.

Yeah that's what was looking reasonable to me and hence I wrote this in my comment above

So I don't even think we are building our docs dependening on this config here !!

Let's just get rid of the following here.

if(XEUS_CPP_INCLUDE_DOCS)
add_subdirectory(docs)
endif()

Maybe let's do this for now ?
And even if we want to take the cmake route, we can do it through a subsequent pr but atleast lets not have any wrong-doing/leftover from previous work for starters ?

[mcbarton]

Thanks for the reply.

Another option could be to keep the current docs build mechanism and totally remove the XEUS_CPP_INCLUDE_DOCS from the CMakeLists to avoid confusion in the future.

Yeah that's what was looking reasonable to me and hence I wrote this in my comment above

So I don't even think we are building our docs dependening on this config here !!

Let's just get rid of the following here.

if(XEUS_CPP_INCLUDE_DOCS)
add_subdirectory(docs)
endif()

Maybe let's do this for now ? And even if we want to take the cmake route, we can do it through a subsequent pr but atleast lets not have any wrong-doing/leftover from previous work for starters ?

@anutosh491 Ive removed the XEUS_CPP_INCLUDE_DOCS section. Can you approve now?

[anutosh491]

Perfect