cmake-tidy-mircommon #4603
Merged
cmake-tidy-mircommon #4603
+71
−94
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
CI isn't happy, I suspect it's these lines since Lines 69 to 72 in 57f9b45 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restructures the mircommon component by consolidating internal development headers into the main libmircommon-dev package and reorganizing the build system to use modular OBJECT libraries instead of a monolithic source list approach.
Key changes:
- Removed the separate
libmircommon-internal-devpackage, merging it intolibmircommon-dev - Refactored CMake build to use OBJECT libraries (
mircommondispatch,mircommoninput) for better modularity - Moved internal headers (e.g.,
event_private.h,signal_blocker.h) from public includes to source-local includes
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/common/thread/CMakeLists.txt | Removed signal_blocker.cpp from mirsharedthread (moved to dispatch) |
| src/common/mircommon-internal.pc.in | Deleted obsolete pkg-config file for removed internal package |
| src/common/input/xkb_mapper.cpp | Updated include path for event_private.h to relative path |
| src/common/input/input_event.cpp | Updated include path for event_private.h to relative path |
| src/common/input/CMakeLists.txt | Created new mircommoninput OBJECT library |
| src/common/events/event_builders.cpp | Updated include path for event_private.h to relative path |
| src/common/event_printer.cpp | Updated include path for event_private.h to relative path |
| src/common/event.cpp | Updated include path for event_private.h to relative path |
| src/common/dispatch/threaded_dispatcher.cpp | Updated include path for signal_blocker.h to relative path |
| src/common/dispatch/signal_blocker.cpp | Updated include path for signal_blocker.h to relative path |
| src/common/dispatch/CMakeLists.txt | Created mircommondispatch OBJECT library and moved signal_blocker.cpp here |
| src/common/CMakeLists.txt | Refactored to link OBJECT libraries instead of accumulating source lists |
| debian/libmircommon-internal-dev.install | Deleted install manifest for removed package |
| debian/control | Converted libmircommon-internal-dev to transitional package |
Contributor
Author
|
Hmm there seems to be a problem with installing On armhf On arm64 ... I don't think it is related |
jhenstridge
reviewed
Jan 7, 2026
debian/control
Outdated
| libmircommon-internal-dev (<< 2.25) | ||
| Replaces: mircommon-dev (<< 0.6), | ||
| libmircommon-internal-dev (<< 2.25) | ||
| Provides: libmircommon-internal-dev |
Contributor
There was a problem hiding this comment.
Does this provides line make sense? I know the intent is to get rid of the libmircommon-internal-dev package, but libmircommon-dev does not seem to provide any of the functionality of the other package (pkg-config files, headers).
Would the Breaks / Replaces be enough on their own?
Contributor
Author
2 tasks
Contributor
|
@AlanGriffiths I've opened a new pull request, #4606, to work on those changes. Once the pull request is ready, I'll request review from you. |
The Provides field is not appropriate here because libmircommon-dev doesn't actually provide the same pkg-config files (mircommon-internal.pc) or header directory (mircommon-internal/) that libmircommon-internal-dev used to provide. The functionality has been merged/renamed into mircommon, not duplicated. The Breaks and Replaces fields are sufficient for handling the package transition, along with the transitional package that depends on libmircommon-dev. Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>
Related: #4603 ## What's new? Removed `Provides: libmircommon-internal-dev` from the `libmircommon-dev` package definition in `debian/control`. The `Provides` field is semantically incorrect here because `libmircommon-dev` does not provide the same files that `libmircommon-internal-dev` installed (`mircommon-internal.pc`, `mircommon-internal/` headers). The functionality was merged/renamed into `mircommon`, not duplicated. `Breaks` and `Replaces` are sufficient for the package transition, along with the transitional package depending on `libmircommon-dev (>= 2.26)`. ## How to test Debian package metadata change only - no functional testing required. ## Checklist - [x] Tests added and pass - [x] Adequate documentation added <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
RAOF
previously requested changes
Jan 8, 2026
Contributor
RAOF
left a comment
RAOF
left a comment
There was a problem hiding this comment.
We should just remove the libmircommon-internal-dev package; it serves no purpose. (We do need to keep the Breaks/Replaces so that upgrades go cleanly)
Apart from that, looks good.
TICS Quality Gate✔️ Passedmir
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More tidy up of mircommon structure:
libmircommon-internal-devpackage, merging it intolibmircommon-devmircommondispatch,mircommoninput) for better modularityevent_private.h,signal_blocker.h) from public includes to source-local includes