Skip to content

Simplify directory structure - #293

Closed
ClausKlein wants to merge 1 commit into
bemanproject:mainfrom
ClausKlein:feature/simplify-directory-structure
Closed

ClausKlein wants to merge 1 commit into
bemanproject:mainfrom
ClausKlein:feature/simplify-directory-structure

Conversation

@ClausKlein

@ClausKlein ClausKlein commented Jan 23, 2026 •

Copy link
Copy Markdown
Contributor
bash-5.3$ tree --gitignore
.
├── CMakeLists.txt
├── CMakePresets.json
├── LICENSE
├── README.md
├── cmake
│   └── beman.exemplar-config.cmake.in
├── examples
│   ├── CMakeLists.txt
│   ├── identity_as_default_projection.cpp
│   └── identity_direct_usage.cpp
├── include
│   └── beman
│       └── exemplar
│           ├── CMakeLists.txt
│           └── identity.hpp
├── lockfile.json
└── tests
    ├── CMakeLists.txt
    └── identity.test.cpp

7 directories, 13 files
bash-5.3$ 

fix #292 too

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 100.0%. remained the same
when pulling ed18a73 on ClausKlein:feature/simplify-directory-structure
into 57fd7e9 on bemanproject:main.

@ednolan

ednolan commented Jan 23, 2026

Copy link
Copy Markdown
Member

This branch contains three separate changes and should be split up into three pull requests.

The first change is to fix the duplicative GTest::gtest GTest::gtest_main dependency in tests/CMakeLists.txt. This is a bugfix that can be merged immediately.

The second change is to move the test files from tests/beman/exemplar to just tests. This change makes sense to me but I'd like to open it up to commentary from the community to see if anyone objects.

The third change is to remove the include/beman/exemplar/CMakeLists.txt file and move the listing of the FILE_SET headers into the top level CMakeLists.txt. This has been previously discussed here. @camio pointed out that "For a repository with multiple libraries, the top-level CMake file would grow quite large and we lose the "one CMakeLists.txt file per library" separation of concerns" and @steve-downey replied that "It shouldn't be necessary to have the list of files in the root, but I have found that creating the library and naming the header set at the root makes it more local." This was the approach taken in bemanproject/optional, which I copied when migrating exemplar to an INTERFACE library. We can discuss changing that approach, but I would be interested to hear the justification.

You should write more descriptive commit titles and commit messages. A commit title named "Simplify directory structure" whose commit message is the output of a tree command does not give casual observers enough information to determine what's going on here.

@ednolan ednolan closed this Jan 23, 2026
@ClausKlein

Copy link
Copy Markdown
Contributor Author

I agree, but you should note, for cxx_modules, we will need to create a library with the module and an header only interface libray too. Then you split the sources in tree different CMakeLists.txt!

What would be the use case to write tow or more libs apart from this?

@ednolan

ednolan commented Jan 23, 2026

Copy link
Copy Markdown
Member

I agree, but you should note, for cxx_modules, we will need to create a library with the module and an header only interface libray too. Then you split the sources in tree different CMakeLists.txt!

I don't think we need to be that strict about "one CMakeLists.txt per library." Both the modules CMake target and the header-only CMake target can be thought of as being part of the same "library" for these purposes. We can keep the modules C++ code in the same directory structure as the headers, and keep the modules CMake code in the same CMakeLists.txt files that define the header only libraries.

What would be the use case to write tow or more libs apart from this?

Some papers are large enough that there may be reasons to split them up in to separate libraries. Alternatively, we might want to split up a project into C++26 and C++29 components; bemanproject/execution sort of does this with its include/beman/execution26 directory, although it doesn't yet define a separate CMake target for that.

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.

ld: warning: ignoring duplicate libraries: 'lib/libgtest.a'

3 participants