[async-mqtt] Add new port #31902

redboltz · 2023-06-09T03:10:26Z

If this PR adds a new port, please uncomment and fill out this checklist:

END OF NEW PORT CHECKLIST (delete this line)

redboltz · 2023-06-09T03:12:51Z

@microsoft-github-policy-service agree

FrankXie05 · 2023-06-09T06:38:25Z

ports/async-mqtt/portfile.cmake

+
+vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")
+
+file(INSTALL "${SOURCE_PATH}/LICENSE" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright)


Suggested change

file(INSTALL "${SOURCE_PATH}/LICENSE" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright)

Just need function vcpkg_install_copyright.

FrankXie05 · 2023-06-09T06:40:10Z

ports/async-mqtt/portfile.cmake

+vcpkg_from_github(
+    OUT_SOURCE_PATH SOURCE_PATH
+    REPO redboltz/async_mqtt
+    REF 1.0.1


Suggested change

REF 1.0.1

REF "${VERSION}"

We can use the new function.

FrankXie05 · 2023-06-09T06:40:35Z

ports/async-mqtt/portfile.cmake

+
+vcpkg_cmake_install()
+
+vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)


Suggested change

vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)

vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH "lib/cmake/async_mqtt_iface")

FrankXie05 · 2023-06-09T06:45:14Z

ports/async-mqtt/portfile.cmake

+
+vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)
+
+file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug" "${CURRENT_PACKAGES_DIR}/lib")


Is this port a header-only or only dynamic.?

This portis a header-only.

Should these two lines be removed if the port is a header-only library?

No, keep two lines. We don't need empty folders.

"${CURRENT_PACKAGES_DIR}/debug" is no longer needed, with VCPKG_BUILD_TYPE set to release.

FrankXie05 · 2023-06-09T06:46:42Z

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".
That way, I can be aware that you've responded since you can't modify the tags.

FrankXie05 · 2023-06-09T07:34:04Z

ports/async-mqtt/portfile.cmake

@@ -0,0 +1,27 @@
+vcpkg_from_github(


Suggested change

vcpkg_from_github(

#header-only library

vcpkg_from_github(

Rather:

set(VCPKG_BUILD_TYPE release) # header-only

and there is no need to remove any debug dirs...

I looked over this comment. I just reflected it.

FrankXie05 · 2023-06-09T07:35:17Z

ports/async-mqtt/portfile.cmake

+
+vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)
+
+file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug" "${CURRENT_PACKAGES_DIR}/lib")


No, keep two lines. We don't need empty folders.

dg0yt · 2023-06-09T08:03:11Z

ports/async-mqtt/portfile.cmake

+    -DASYNC_MQTT_BUILD_TOOLS=OFF
+    -DASYNC_MQTT_BUILD_EXAMPLES=OFF
+    -DASYNC_MQTT_BUILD_UNIT_TESTS=OFF
+    -DASYNC_MQTT_BUILD_SYSTEM_TESTS=OFF
+    -DCMAKE_DISABLE_FIND_PACKAGE_Doxygen=ON


Please indent the actual options:

OPTIONS -DASYNC_MQTT_BUILD_TOOLS=OFF -DASYNC_MQTT_BUILD_EXAMPLES=OFF -DASYNC_MQTT_BUILD_UNIT_TESTS=OFF -DASYNC_MQTT_BUILD_SYSTEM_TESTS=OFF -DCMAKE_DISABLE_FIND_PACKAGE_Doxygen=ON

Add header-only on the top of file. Use ${VERSION} instead of the literal. Fix indent. Add double qoute to path Remove copyright install.

redboltz · 2023-06-09T10:10:19Z

I think that all changes request is reflected.

redboltz · 2023-06-09T10:58:38Z

#31902 (comment) is fixed too.
So now ready to review.

dg0yt

Just from looking at it in GH, only one nit-pick left.

The port is submitted from the owner, and it is not yet a project with many GH stars, so vcpkg maintainers must decide if they want the port under this name or under redboltz-async-mqtt. (There is at least a JS project under the same name. https://github.com/search?q=async-mqtt&type=repositories)

dg0yt · 2023-06-09T12:55:09Z

ports/async-mqtt/portfile.cmake

+
+vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)
+
+file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug" "${CURRENT_PACKAGES_DIR}/lib")


"${CURRENT_PACKAGES_DIR}/debug" is no longer needed, with VCPKG_BUILD_TYPE set to release.

redboltz · 2023-06-10T03:50:55Z

Just from looking at it in GH, only one nit-pick left.

The port is submitted from the owner, and it is not yet a project with many GH stars, so vcpkg maintainers must decide if they want the port under this name or under redboltz-async-mqtt. (There is at least a JS project under the same name. https://github.com/search?q=async-mqtt&type=repositories)

Thank you for the comment. I fixed that removing debug directory.
Let me explain some background. I am the author both
https://github.com/redboltz/async_mqtt (3 starred) and
https://github.com/redboltz/mqtt_cpp (376 starred) .
mqtt_cpp has been registered in vcpkg. Some contributor did it.
async_mqtt is successor project of mqtt_cpp. It coveres all asynchronous functionality of mqtt_cpp, but using more elegant way.

After I released async_mqtt, vcpkg support is requested by user redboltz/async_mqtt#16
So I createed this PR. I accept async-mqtt, redboltz-async-mqtt, or perhaps async-mqtt-cpp.
(The name of https://github.com/redboltz/async_mqtt is not changed.)

FrankXie05 · 2023-06-14T02:50:46Z

@redboltz
It seems that when generating the cmake configuration file, a parenthesis is missing.

async_mqtt_ifaceConfig.cmake:

set(Boost_USE_MULTITHREADED ON)
include(CMakeFindDependencyMacro)
find_dependency((Threads)
find_dependency((Boost 1.81.0 COMPONENTS )
...
...

CMake Error at E:/usage-test/vcpkg/installed/x64-windows/share/async_mqtt_iface/async_mqtt_ifaceConfig.cmake:3:
  Parse error.  Function missing ending ")".  End of file reached.

https://github.com/redboltz/async_mqtt/blob/3d773f4971ea9ff98e61314f29991eda2ecf4926/cmake/Config.cmake.in#L3

dg0yt · 2023-06-14T05:11:30Z

find_dependency((Boost 1.81.0 COMPONENTS )

There is a double (.

FrankXie05 · 2023-06-14T08:42:53Z

find_dependency((Threads)

FrankXie05 · 2023-06-14T08:43:36Z

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".
That way, I can be aware that you've responded since you can't modify the tags.

redboltz · 2023-06-14T09:51:47Z

I fixed async_mqtt cmake config issue and then released it as 1.0.2.
Then I updated this PR.

redboltz · 2023-06-14T11:31:59Z

All checks have passed, now ready for review.

FrankXie05 · 2023-06-15T01:57:40Z

The usage has been tested successfully locally.

async-mqtt provides CMake targets:

    # this is heuristically generated, and may not be correct
    find_package(async_mqtt_iface CONFIG REQUIRED)
    target_link_libraries(main PRIVATE async_mqtt_iface::async_mqtt_iface)

redboltz force-pushed the add_async_mqtt branch 3 times, most recently from 3c8f86f to 02110f9 Compare June 9, 2023 05:50

FrankXie05 self-assigned this Jun 9, 2023

redboltz force-pushed the add_async_mqtt branch from 02110f9 to f10781d Compare June 9, 2023 05:55

FrankXie05 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jun 9, 2023

[async-mqtt] Add async-mqtt port.

23cfeee

redboltz force-pushed the add_async_mqtt branch from f10781d to 23cfeee Compare June 9, 2023 06:11

redboltz marked this pull request as ready for review June 9, 2023 06:16

FrankXie05 requested changes Jun 9, 2023

View reviewed changes

FrankXie05 marked this pull request as draft June 9, 2023 06:46

FrankXie05 changed the title ~~[async-mqtt] Add async-mqtt.~~ [async-mqtt] Add new port Jun 9, 2023

FrankXie05 reviewed Jun 9, 2023

View reviewed changes

dg0yt reviewed Jun 9, 2023

View reviewed changes

Reflect review comments.

e76f5e6

Add header-only on the top of file. Use ${VERSION} instead of the literal. Fix indent. Add double qoute to path Remove copyright install.

redboltz force-pushed the add_async_mqtt branch from 381ac8f to e76f5e6 Compare June 9, 2023 09:55

redboltz marked this pull request as ready for review June 9, 2023 10:03

Set build type to release.

bda6f30

redboltz force-pushed the add_async_mqtt branch from 936f4f2 to bda6f30 Compare June 9, 2023 10:46

redboltz mentioned this pull request Jun 9, 2023

vcpkg version of library redboltz/async_mqtt#16

Closed

redboltz requested review from FrankXie05 and dg0yt June 9, 2023 11:53

dg0yt approved these changes Jun 9, 2023

View reviewed changes

Removed debug directory removal.

e4d38ff

redboltz force-pushed the add_async_mqtt branch from a4b56d5 to e4d38ff Compare June 9, 2023 13:12

FrankXie05 previously approved these changes Jun 13, 2023

View reviewed changes

FrankXie05 marked this pull request as draft June 14, 2023 08:43

redboltz dismissed FrankXie05’s stale review via b24b112 June 14, 2023 09:50

Fix async_mqtt installed cmake config.

6543d9d

redboltz force-pushed the add_async_mqtt branch from b24b112 to 6543d9d Compare June 14, 2023 10:08

redboltz marked this pull request as ready for review June 14, 2023 11:32

FrankXie05 approved these changes Jun 15, 2023

View reviewed changes

FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Jun 15, 2023

vicroms merged commit 019fc6b into microsoft:master Jun 15, 2023

redboltz deleted the add_async_mqtt branch June 15, 2023 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[async-mqtt] Add new port #31902

[async-mqtt] Add new port #31902

redboltz commented Jun 9, 2023

redboltz commented Jun 9, 2023

FrankXie05 Jun 9, 2023

FrankXie05 Jun 9, 2023

FrankXie05 Jun 9, 2023

FrankXie05 Jun 9, 2023

redboltz Jun 9, 2023

redboltz Jun 9, 2023

FrankXie05 Jun 9, 2023

dg0yt Jun 9, 2023

FrankXie05 commented Jun 9, 2023

FrankXie05 Jun 9, 2023

dg0yt Jun 9, 2023

redboltz Jun 9, 2023

FrankXie05 Jun 9, 2023

dg0yt Jun 9, 2023

redboltz commented Jun 9, 2023

redboltz commented Jun 9, 2023

dg0yt left a comment

dg0yt Jun 9, 2023

redboltz commented Jun 10, 2023 •

edited

Loading

FrankXie05 commented Jun 14, 2023 •

edited

Loading

dg0yt commented Jun 14, 2023

FrankXie05 commented Jun 14, 2023

FrankXie05 commented Jun 14, 2023

redboltz commented Jun 14, 2023

redboltz commented Jun 14, 2023

FrankXie05 commented Jun 15, 2023


		vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")

		file(INSTALL "${SOURCE_PATH}/LICENSE" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright)


		vcpkg_cmake_install()

		vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)


		vcpkg_cmake_config_fixup(PACKAGE_NAME async_mqtt_iface CONFIG_PATH lib/cmake/async_mqtt_iface)

		file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug" "${CURRENT_PACKAGES_DIR}/lib")

[async-mqtt] Add new port #31902

[async-mqtt] Add new port #31902

Conversation

redboltz commented Jun 9, 2023

redboltz commented Jun 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrankXie05 commented Jun 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redboltz commented Jun 9, 2023

redboltz commented Jun 9, 2023

dg0yt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redboltz commented Jun 10, 2023 • edited Loading

FrankXie05 commented Jun 14, 2023 • edited Loading

dg0yt commented Jun 14, 2023

FrankXie05 commented Jun 14, 2023

FrankXie05 commented Jun 14, 2023

redboltz commented Jun 14, 2023

redboltz commented Jun 14, 2023

FrankXie05 commented Jun 15, 2023

redboltz commented Jun 10, 2023 •

edited

Loading

FrankXie05 commented Jun 14, 2023 •

edited

Loading