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

Split CMake identities: superproject and 'developer' #392

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

jeremy-murphy
Copy link
Contributor

I prefer to use CMake as a build system, but the existing CMake files in Boost don't really help.

  1. I don't want to treat every library as an active project when I'm only working on one.
  2. I need the header files to be part of the project so that an IDE treats them as the source code of the project.

I'm always maintaining some split-identity CMake like this in order to do work, so I thought I may as well merge it upstream if

  1. It's compatible with super-project plans for CMake, and
  2. other developers would find it useful.

@jeremy-murphy jeremy-murphy marked this pull request as draft October 31, 2024 01:05
@jeremy-murphy jeremy-murphy changed the title Draft: Split CMake identities: superproject and 'developer' Split CMake identities: superproject and 'developer' Oct 31, 2024
@jeremy-murphy
Copy link
Contributor Author

This is still somewhat broken, I haven't got the tests to compile properly yet, I think it's partly missing the required Boost.UTF macros, but also some unexpected static_assert failures.

@jeremy-murphy
Copy link
Contributor Author

I actually don't know who else works on Boost.Graph and prefers CMake, so I'll just tag a few recent contributors to see if they are interested: @jan-grimo @danielyxyang @brunom @sehe @jcdong98 @andrea-cassioli-maersk @vslashg

@jeremy-murphy
Copy link
Contributor Author

Peter, I assigned you because I assume you know about the super-project CMake business.

@andrea-cassioli-maersk
Copy link
Contributor

I do work with CMake usually/

@pdimov
Copy link
Member

pdimov commented Oct 31, 2024

This looks OK at a first glance, but you should change

if (BOOST_SUPERPROJECT_VERSION)

to

if(NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)

which checks whether this is the root project (as opposed to checking whether it's part of the Boost superproject.)

In principle, Boost libraries are also usable without a superproject, as direct subprojects, although in this specific case this probably isn't very practical because of the number of dependencies.

@pdimov
Copy link
Member

pdimov commented Oct 31, 2024

Also, if you're going to make changes to CMakeLists.txt, you should first add CI jobs that verify that current uses aren't broken. Or I can add them for you if you prefer.

@jeremy-murphy
Copy link
Contributor Author

Also, if you're going to make changes to CMakeLists.txt, you should first add CI jobs that verify that current uses aren't broken. Or I can add them for you if you prefer.

If you have some pre-canned test, then yes, please go ahead!

@jeremy-murphy
Copy link
Contributor Author

This looks OK at a first glance, but you should change

if (BOOST_SUPERPROJECT_VERSION)

to

if(NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)

which checks whether this is the root project (as opposed to checking whether it's part of the Boost superproject.)

In principle, Boost libraries are also usable without a superproject, as direct subprojects, although in this specific case this probably isn't very practical because of the number of dependencies.

OK, sure, will do. I see there are some new variables with PROJECT in their name specifically for this purpose, but they require a fairly recent version of CMake, so I'll just go with whatever is supported by our minimum requirement.

@pdimov
Copy link
Member

pdimov commented Nov 7, 2024

CI jobs for testing CMake subdir/install use added. I arbitrarily picked the adj_list_edge_list_set.cpp test as the main.cpp file in both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants