Skip to content

[dbus-cxx] add new port#47617

Merged
JavierMatosD merged 64 commits intomicrosoft:masterfrom
luadebug:dbus
Oct 6, 2025
Merged

[dbus-cxx] add new port#47617
JavierMatosD merged 64 commits intomicrosoft:masterfrom
luadebug:dbus

Conversation

@luadebug
Copy link
Copy Markdown
Contributor

@luadebug luadebug commented Oct 2, 2025

  • Changes comply with the maintainer guide.
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

aui-framework/aui#635
dbus-cxx/dbus-cxx#191

@luadebug luadebug marked this pull request as draft October 2, 2025 20:08
@luadebug luadebug marked this pull request as ready for review October 3, 2025 19:45
@luadebug luadebug requested a review from dg0yt October 3, 2025 19:47
luadebug added a commit to luadebug/dbus-cxx that referenced this pull request Oct 3, 2025
luadebug added a commit to luadebug/dbus-cxx that referenced this pull request Oct 3, 2025
@luadebug luadebug marked this pull request as draft October 4, 2025 01:23
@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 4, 2025

Hm, now I see Qt6 changes... If you want to add a new port, I would strongly recommend to start with minimal changes to upstream. We should review the porting effort here, not innovations which belong to upstream.

@luadebug
Copy link
Copy Markdown
Contributor Author

luadebug commented Oct 4, 2025

Hm, now I see Qt6 changes... If you want to add a new port, I would strongly recommend to start with minimal changes to upstream. We should review the porting effort here, not innovations which belong to upstream.

Should we just use Qt6Core instead of Qt5Core since you have mentioned that Qt5 is EOL, besides that probably should require probably just one line change, rather testing upstream new code. Using Qt6Core instead of Qt5Core is not much innovative. dbus-cxx/dbus-cxx#191

Does merged PR in upstream would increase chance that PR would be merged?

@luadebug luadebug marked this pull request as ready for review October 4, 2025 12:18
@luadebug
Copy link
Copy Markdown
Contributor Author

luadebug commented Oct 6, 2025

Just reminder that dbus-cxx/dbus-cxx#191 is merged and PR is ready to review.

@JavierMatosD JavierMatosD merged commit 4b55d8c into microsoft:master Oct 6, 2025
18 checks passed
Comment on lines +19 to +23
"default-features": [
"glib",
"libuv",
"qt6"
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@JavierMatosD Why is this set of default-features acceptable here and rejected in other proposal? An application wouldn't use three different dispatchers at the same time.

"use Qt, GLib, or libuv as your main event loop without requiring a new thread"

https://dbus-cxx.github.io/ > Features

And all three options are off by default upstream.
https://github.com/dbus-cxx/dbus-cxx/blob/master/CMakeLists.txt#L46-L57

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

They are not acceptable. This was an oversight on my part and should not have been merged in its current form.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reverting this PR @luadebug, please reopen this PR and note we generally try to avoid using "default-features" as it was intended to only provide the minimum required for the port to be usable but default features can be difficult to disable downstream. I recommend taking a look at our docs on default features: https://learn.microsoft.com/en-us/vcpkg/concepts/default-features#role-of-default-features and its constraints https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#default-features-should-enable-behaviors-not-apis

JavierMatosD pushed a commit to JavierMatosD/vcpkg that referenced this pull request Oct 6, 2025
BillyONeal pushed a commit that referenced this pull request Oct 6, 2025
Co-authored-by: Javier Matos <javiermatos@Javiers-Laptop.local>
@luadebug luadebug mentioned this pull request Oct 6, 2025
11 tasks
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.

3 participants