[cpprealm] new port #39561
[cpprealm] new port #39561
Conversation
jimwang118
added
the
category:new-port
|
Note: I will be converting your PR to draft status. The above suggested changes are only recommendations. If you are willing to adopt them, please click "ready for review" after making the modifications. If you do not wish to make any changes, please click "ready for review" directly. That way, I can be aware that you've responded since you can't modify the tags. |
ports/cpprealm/portfile.cmake
Outdated
| file(READ "${CURRENT_PACKAGES_DIR}/debug/include/cpprealm/internal/bridge/bridge_types.hpp" DEBUG_TYPE_HEADER_CONTENTS) | ||
| file(READ "${CURRENT_PACKAGES_DIR}/include/cpprealm/internal/bridge/bridge_types.hpp" RELEASE_TYPE_HEADER_CONTENTS) | ||
| file(WRITE "${CURRENT_PACKAGES_DIR}/include/cpprealm/internal/bridge/bridge_types.hpp" " | ||
| #if defined(_DEBUG) || !defined(NDEBUG) |
There was a problem hiding this comment.
IMO using NDEBUG is asking for trouble. The user project might set it independent of build type, anytime, to control assert, not ABI.
There was a problem hiding this comment.
I've removed this completely in favour of using the REALM_DISABLE_ALIGNED_STORAGE cmake flag we expose.
| { | ||
| "name": "curl", | ||
| "platform": "linux", | ||
| "version>=": "8.4.0" | ||
| }, | ||
| { | ||
| "name": "libuv", | ||
| "platform": "!osx, !ios", | ||
| "version>=": "1.43.0" | ||
| }, | ||
| { | ||
| "name": "realm-core", | ||
| "version>=": "14.8.0" | ||
| }, |
There was a problem hiding this comment.
In other cases, version constraints were rejected for the main registry.
There was a problem hiding this comment.
Could you expand more on what you mean by this, and what action we should take?
There was a problem hiding this comment.
@leemaguire can you explain why this version constraint is required?
There was a problem hiding this comment.
In other cases, version constraints were rejected for the main registry.
It looks like we have plenty of those to me:
The situation I know we had a problem with was that we opposed trying to keep 'metapackages' consistent because it made every change invalidate the universe. (as in, changing one thing about boost-build shouldn't force a package-version update on every other boost thing)
There was a problem hiding this comment.
(That said, "just fill in the version that is current that a baseline would do anyway" should not happen)
There was a problem hiding this comment.
@JavierMatosD I have placed the version constraints as these are the minimum versions of the dependancies we test on for this version of cpprealm.
There was a problem hiding this comment.
@BillyONeal @JavierMatosD if there is a more correct way to reference the dependancies in this case please let me know.
There was a problem hiding this comment.
I have placed the version constraints as these are the minimum versions of the dependancies we test on for this version of cpprealm.
In that case, I think we should drop the version constraints.
| { | ||
| "name": "curl", | ||
| "platform": "linux", | ||
| "version>=": "8.4.0" | ||
| }, | ||
| { | ||
| "name": "libuv", | ||
| "platform": "!osx, !ios", | ||
| "version>=": "1.43.0" | ||
| }, | ||
| { | ||
| "name": "realm-core", | ||
| "version>=": "14.8.0" | ||
| }, |
There was a problem hiding this comment.
@leemaguire can you explain why this version constraint is required?
| @@ -0,0 +1,32 @@ | |||
| { | |||
| "name": "cpprealm", | |||
There was a problem hiding this comment.
Should this be renamed to realm-cpp to match realm-core and the upstream repo?
There was a problem hiding this comment.
The target name is cpprealm for historical reasons, and our docs & folder structure already reflect that target name.
I have no issue changing the port folder and name to be realm-cpp as long as the cpprealm target name can be kept. What are you thoughts @JavierMatosD?
There was a problem hiding this comment.
I don't think changing the name is necessary if that's the target name.
|
Converting the PR to draft. Please mark ready for review once you've replied. |
find_packagecalls are REQUIRED, are satisfied byvcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.jsonmatches what upstream says.vcpkg.jsonmatches what upstream says../vcpkg x-add-version --alland committing the result.