apple / apple/foundationdb

CMake: Review and cleanup target_include_directories()

Open
#1,000 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
16.7k
Forks
1.6k
Avg merge
1d 20h
Merged PRs (30d)
126

Description

In CMake build scripts we are using `target_include_directories()` to make all the header files public. With the current setup, we refer all the header files either with their complete path (relative to the root of the repo) or with the local path.

For example, `fdbclient/versions.h` could be included one of two ways

* `#include "fdbclient/versions.h"` from any file in repo.
* `#include "versions.h"` from the files in `fdbclient/` only

By making header files export PUBLIC in `target_include_directories()` we are adding the third path, which is relative to module root path.

* `#include "versions.h"` from any files in the dependent modules of `fdbclient`

Setting PUBLIC/PRIVATE `target_include_directories()` is the right way to do it, I agree. But, a couple of concerns.

* We are exporting PUBLIC for all. Not sure if we want that.
* These new paths are not consistent with the older way of doing things. This is a bigger concern.

I suggest, for now, go with making all PRIVATE includes to be consistent with the old build. Then, we can slowly move to a new and better way of doing things.

@mpilman @alexmiller-apple

Related to: #999

Contributor guide

Open the contributing guide

Research direction

Search the CMake build scripts for target_include_directories() and compare the current PUBLIC declarations with the include patterns described for fdbclient/versions.h. Review related issue #999, then verify that changing the relevant include visibility to PRIVATE preserves the existing include paths and that the project still configures and builds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.