Skip to content
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

meson include directory #792

Open
jwidauer opened this issue Jul 18, 2023 · 6 comments
Open

meson include directory #792

jwidauer opened this issue Jul 18, 2023 · 6 comments
Labels
build-system/meson Issues regarding the Meson build system type/docs Issues related to improving documentation

Comments

@jwidauer
Copy link

Description

Hi there!
I've been using doctest for quite a while using CMake and Conan and have been loving it!
I was now trying to use it directly as a meson subproject.
While doing so, I noticed that in the current meson.build file, it declares the dependency to directly include the doctest directory.
It's specifically this line:

doctest_dep = declare_dependency(include_directories: include_directories('doctest'))

This means that, to be able to use doctest from a meson project, one would have to include it like this:

#include <doctest.h>

Instead of the way that is shown in the examples and done for all the other build systems:

#include <doctest/doctest.h>

Is this intended?
If not, I would assume the above line in the meson.build file to be:

doctest_dep = declare_dependency(include_directories: include_directories('.'))

Steps to reproduce

Try and use doctest as a subproject in a project using meson as the build system.

@eli-schwartz
Copy link
Contributor

This is basically just #670 but from the other direction. When I opened the other ticket, I based it on the assumption that the directions in the tutorial and throughout the documentation -- which say to use #include "doctest.h" -- were correct.

@mitchgrout
Copy link

I would support #include <doctest.h> / #include "doctest.h" as the way to properly include the library, as <doctest/doctest.h> personally feels redundant. It also leaves #include <doctest/part> as an obvious choice for include parts of the library if there's ever any splitting of the header into multiple smaller components.

@gerion0
Copy link

gerion0 commented Jun 30, 2024

Just to add another argument:

Installing doctest on Gentoo leads to these files:

/usr
/usr/include
/usr/include/doctest
/usr/include/doctest/doctest.h
/usr/include/doctest/extensions
/usr/include/doctest/extensions/doctest_mpi.h
/usr/include/doctest/extensions/doctest_util.h
/usr/include/doctest/extensions/mpi_reporter.h
/usr/include/doctest/extensions/mpi_sub_comm.h
/usr/lib64
/usr/lib64/cmake
/usr/lib64/cmake/doctest
/usr/lib64/cmake/doctest/doctest.cmake
/usr/lib64/cmake/doctest/doctestAddTests.cmake
/usr/lib64/cmake/doctest/doctestConfig.cmake
/usr/lib64/cmake/doctest/doctestConfigVersion.cmake
/usr/lib64/cmake/doctest/doctestTargets.cmake
/usr/share
/usr/share/doc
/usr/share/doc/doctest-2.4.11
/usr/share/doc/doctest-2.4.11/README.md.xz

Using dependency('doctest') in meson.build within another project and try to link against the globally installed doctest leads to an error then:

The Meson build system
Version: 1.4.1
...
Found CMake: /usr/bin/cmake (3.28.5)
Run-time dependency doctest found: YES 2.4.11
...
[1/2] Compiling C++ object tests/unit_tests.p/unit_tests.cpp.o
FAILED: tests/unit_tests.p/unit_tests.cpp.o 
ccache c++ -Itests/unit_tests.p -Itests -I../tests -Iinclude -I../include -I/usr/include -fdiagnostics-color=always -D_GLIBCXX_ASSERTIONS=1 -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Wpedantic -Werror -std=c++17 -O0 -g -MD -MQ tests/unit_tests.p/unit_tests.cpp.o -MF tests/unit_tests.p/unit_tests.cpp.o.d -o tests/unit_tests.p/unit_tests.cpp.o -c ../tests/unit_tests.cpp
../tests/unit_tests.cpp:2:10: fatal error: doctest.h: No such file or directory
    2 | #include "doctest.h"
      |  

(I tried doctest = dependency('doctest', method: 'cmake') and method: 'auto'.)
Changing the include to doctest/doctest.h works since Meson (just?) adds -I/usr/include.

TL;DR: #include "doctest.h" works with doctest as wrap file/subproject currently, #include "doctest/doctest.h" works with a system wide installed doctest.

@cdeln
Copy link

cdeln commented Aug 19, 2024

As discussed in #670 , #include <doctest/doctest.h> is the "correct" way to include the header, because that is how it's installed (as in cmake --install <build-dir> --prefix <install-prefix>). This is how it's included when installed by apt-get on Ubuntu (and Gentoo as pointed out by @gerion0 ). But in the end it's C++, it always depends on how the header is installed relative to the include search path passed to the compiler.

I'm in favor of the solution proposed by @jwidauer , if this only has as a side effect that it changes the include path by downstream projects. Not being a Meson though, so any Meson people please verify the approach.

@mitchgrout
Copy link

Regardless of which include style is preferred, I would say that changing the include_directories counts as a breaking change, as this will break any project which uses the #include <doctest.h> syntax. Even if it's wrong, since the library permitted it, it's now implicitly part of the interface.

@cdeln
Copy link

cdeln commented Aug 20, 2024

I agree with that as well, and that is an important point. Let's think about non-breaking workarounds.

One is to change the statement to doctest_dep = declare_dependency(include_directories: include_directories('.', 'doctest')) and change the docs to use <doctest/doctest.h> consistently to encourage people to transition to that.

The breaking change could be introduced in a 3.xx release.
For those that still haven't transitioned I believe it should be easy to manually add the include directories in your own projects meson file as a workaround? Please advice, I'm no meson expert.

@cdeln cdeln added type/docs Issues related to improving documentation build-system/meson Issues regarding the Meson build system labels Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system/meson Issues regarding the Meson build system type/docs Issues related to improving documentation
Projects
None yet
Development

No branches or pull requests

5 participants