Skip to content

Prepare cxx_module usage#40

Merged
dietmarkuehl merged 5 commits intomainfrom
feature/sync_with_task
Feb 23, 2026
Merged

Prepare cxx_module usage#40
dietmarkuehl merged 5 commits intomainfrom
feature/sync_with_task

Conversation

@ClausKlein
Copy link
Contributor

@ClausKlein ClausKlein commented Feb 22, 2026

  • TODO: disabled examples for now?
  • TODO: disabled VERIFY_INTERFACE_HEADER_SETS for now

This fix Install for #24

@ClausKlein ClausKlein self-assigned this Feb 22, 2026
@ClausKlein ClausKlein marked this pull request as draft February 22, 2026 21:52
@ClausKlein
Copy link
Contributor Author

ClausKlein commented Feb 23, 2026

@dietmarkuehl No one os the examples compiles after updating?

and VERIFY_INTERFACE_HEADER_SETS fails too!

@coveralls
Copy link

coveralls commented Feb 23, 2026

Coverage Status

coverage: 100.0% (+96.6%) from 3.39%
when pulling 2a02d3e on feature/sync_with_task
into 157512f on main.

ClausKlein and others added 2 commits February 23, 2026 15:23
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@ClausKlein ClausKlein linked an issue Feb 23, 2026 that may be closed by this pull request
@ClausKlein ClausKlein marked this pull request as ready for review February 23, 2026 15:41
Comment on lines 30 to 44
FetchContent_Declare(
execution
# SOURCE_DIR ${CMAKE_SOURCE_DIR}/../execution
# FETCHCONTENT_SOURCE_DIR_EXECUTION ${CMAKE_SOURCE_DIR}/../execution
GIT_REPOSITORY https://github.com/bemanproject/execution
GIT_TAG c587808
GIT_TAG 66295d5
SYSTEM
FIND_PACKAGE_ARGS
0.2.0
EXACT
NAMES
beman.execution
COMPONENTS
execution_headers
)
FetchContent_MakeAvailable(execution)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not list this here, the direct dependency is only task.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting question: when including C++ headers, depending on indirect includes is frowned upon, at least in the world I'm in. net certainly uses both beman.execution and beman.task directly. However, since there are [potentially] version numbers referenced which may deviate, there is an obvious problem when.

In the end, I don't have much of an opinion on that.

# NOTE: this must be done before tests! CK
include(./cmake/beman-install-library.cmake)
beman_install_library(${PROJECT_NAME} TARGETS beman.net_headers
DEPENDENCIES [===[beman.task 0.2.0 EXACT]===]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment above.

# function!**
#
# **Only header files contained in a `PUBLIC FILE_SET TYPE HEADERS` will be
# install with this function!**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# install with this function!**
# installed with this function!**

Comment on lines 30 to 44
FetchContent_Declare(
execution
# SOURCE_DIR ${CMAKE_SOURCE_DIR}/../execution
# FETCHCONTENT_SOURCE_DIR_EXECUTION ${CMAKE_SOURCE_DIR}/../execution
GIT_REPOSITORY https://github.com/bemanproject/execution
GIT_TAG c587808
GIT_TAG 66295d5
SYSTEM
FIND_PACKAGE_ARGS
0.2.0
EXACT
NAMES
beman.execution
COMPONENTS
execution_headers
)
FetchContent_MakeAvailable(execution)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting question: when including C++ headers, depending on indirect includes is frowned upon, at least in the world I'm in. net certainly uses both beman.execution and beman.task directly. However, since there are [potentially] version numbers referenced which may deviate, there is an obvious problem when.

In the end, I don't have much of an opinion on that.

@dietmarkuehl dietmarkuehl merged commit 981e018 into main Feb 23, 2026
36 checks passed
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.

beman.net is missing install cmake flow

3 participants