Skip to content

Add RFC for supporting Conan as a build/package option for CppMicroSe…#28

Open
achristoforides wants to merge 3 commits intomasterfrom
conan-support
Open

Add RFC for supporting Conan as a build/package option for CppMicroSe…#28
achristoforides wants to merge 3 commits intomasterfrom
conan-support

Conversation

@achristoforides
Copy link
Copy Markdown
Member

…rvices

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new RFC proposing Conan-based packaging and externally managed dependency support for CppMicroServices while preserving the current vendored third_party/ build as the default path.

Changes:

  • Introduces an RFC for US_USE_SYSTEM_* CMake options that switch dependencies between vendored and externally resolved packages.
  • Describes compatibility shims and installed CMake helper behavior needed for Conan consumers.
  • Documents Conan packaging strategy, dependency visibility, constraints, drawbacks, and alternatives.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread text/0017-conan-support.md
Comment thread text/0017-conan-support.md
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Copy link
Copy Markdown
Member

@jeffdiclemente jeffdiclemente left a comment

Choose a reason for hiding this comment

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

inline PR comments are down on github.com, so resorting to putting my feedback here instead....

  • What's the upgrade path when a vendored dep is updated — is there a policy to bump the Conan recipe pin at the same time? There should be some mention of how this should work.
  • Will there be any new CI pipelines in the CppMicroServices github repo for the conan recipe? or will we rely on conan-center-index CI?

one line 16

behavioral mismatches with other copies of the same library in the same process.

This is not the relevant problem. cppms explicitly stays away from linking to other shared libraries and all its vendored dependencies are either directly compiled into the cppms binaries using source or using static libs that don't expose the exported APIs, thus eliminating the "same library in the same process" problem.

A more concrete problem, which the conan CCI folks have voiced, is the lack of visibility for consumers to reason about the versions as it relates to security (i.e. CVEs) for the purpose of SBOMs (software bill of materials).

on line 18
in the current state the versions of the vendored dependencies are not conflicts, since they are built directly into the cppms bianries/exes without exposing the vendored libraries exported APIs. in this case, what is the problem exposed by downstream integration?

on line 77
is there anyway to converge on one nowide C++ namespace and one set of header #include paths. This is a design smell; having to have two divergent paths.

on line 93
The CLI11 forwarding header is a design smell. there should be no need for it as you can just adjust the include path appropriately at the CMakeLists.txt level. There should not be a need for a forwarding header.

on line 208
Explicitly state in the design doc's detailed design section which combinations are supported, currently this is buried in the drawbacks section

Comment thread text/0017-conan-support.md Outdated

- **`framework`** — always present. Maps to the `CppMicroServices` CMake target.
- **`logservice`** — always present (header-only). Maps to `usLogService`.
- **`logservice`** — always present (header-only). Maps to `usLogService`. Separated from the compendium group because it is a pure interface library (abstract classes only, no compiled sources) that is built unconditionally, whereas the other compendium services require both `shared=True` and `with_threading=True`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand the rationale, but I think this is the wrong approach to organizing the bundles. What about usLogService only having a pure virtual interface requires it to be in a separate group? DS and CA also have interfaces, however they are being grouped in the compendium group. I don't understand the rationale for this grouping. Also, where is the logservice impl located within these three component groups?

OSGi v8 has moved the logservice into the "core" specification, alongside what we consider the cppms framework. I wonder whether the component groups should be: core and compendium?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The conceptual grouping is core (framework + logservice, per OSGi v8) and compendium (everything else). Within compendium there are two kinds:

  1. Compile-time dependencies — ServiceComponent, AsyncWorkService, EventAdmin. These have public headers that consumers #include and link against.
  2. Runtime-loaded bundles — DS, CA, LogServiceImpl. These have no public headers for consumers; they're loaded at runtime via installAndStart().

They're kept as individual Conan components rather than a single aggregate because consumers need to link specific compile-time targets (e.g. usServiceComponent) independently of the runtime bundles. Collapsing them into one component would force consumers to express a link dependency on things they only load at runtime.

I've updated the RFC to reflect this grouping and make the distinction explicit.

Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md Outdated
Comment thread text/0017-conan-support.md
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.

6 participants