Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[man-group-sparrow] New port #43989

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+108 −0
Open

[man-group-sparrow] New port #43989

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9218f80
[man-group-sparrow] add port
toge Feb 23, 2025
0463467
fix licenses, make `date` default-features, disable ci on x64-linux
toge Feb 24, 2025
a7a84c3
add large-int-placeholder feature
toge Feb 24, 2025
b09fe5f
unsupport static
toge Feb 24, 2025
f0bc949
revert x64-linux build
toge Feb 24, 2025
39a4f9e
update 0.4.0
toge Feb 25, 2025
8c3fb6d
make large-int-placeholder default-features on windows only
toge Feb 26, 2025
88d4d63
force large-int-placefolder on arm-*-android, disable ci on x64-linux…
toge Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions ports/man-group-sparrow/find-date.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
diff --git a/sparrowConfig.cmake.in b/sparrowConfig.cmake.in
index 07a688f..cadb864 100644
--- a/sparrowConfig.cmake.in
+++ b/sparrowConfig.cmake.in
@@ -18,6 +18,10 @@

include(CMakeFindDependencyMacro)

+if("@USE_DATE_POLYFILL@")
+ find_dependency(date)
+endif()
+
Comment on lines +9 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't hesitate to open a PR on https://github.com/man-group/sparrow/
We can review and merge it asap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alex-PLACET
Thank you for your advise!
I am going to create PR for it.

if(NOT TARGET @PROJECT_NAME@)
include("${CMAKE_CURRENT_LIST_DIR}/@[email protected]")
get_target_property(@PROJECT_NAME@_INCLUDE_DIRS @PROJECT_NAME@ INTERFACE_INCLUDE_DIRECTORIES)
35 changes: 35 additions & 0 deletions ports/man-group-sparrow/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
if(VCPKG_TARGET_IS_LINUX)
message("Warning: `sparrow` requires Clang18+ or GCC 12+ on Linux")
endif()
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

It also MSVC 19.41+ on Windows and Apple Clang 16+ on MacOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alex-PLACET
I agree. However, this seems to be a vcpkg convention.

Because only Linux outputs a warning message about the compiler version when checking other ports.


vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO man-group/sparrow
REF "${VERSION}"
SHA512 f60cf7386955793b34720a2c24ecf29ad2568855c7344a6ab3e72c8bde2899cc9ea0ed880cc84b95feeb32413941e5c895d740c04cd2efd3fecd9b109adac317
HEAD_REF main
PATCHES
find-date.patch
)

vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
FEATURES
date USE_DATE_POLYFILL
large-int-placeholder USE_LARGE_INT_PLACEHOLDERS
Copy link
Contributor

Choose a reason for hiding this comment

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

USE_LARGE_INT_PLACEHOLDERS is mandatory on Windows

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the fix for: #43989 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alex-PLACET
I think this seems a little different.
This PR has large-int-placeholder enabled by default for compatibility.
This means that USE_LARGE_INT_PLACEHOLDERS is enabled and the library can be compiled successfully.

However, when compiling a test program that uses sparrow, MSVC mets compile errors due to the absence of USE_LARGE_INT_PLACEHOLDERS.

To avoid this, it would be better to use config.h which defines USE_LARGE_INT_PLACEHOLDERS using CMake's configure.

I am going to create a PR on upstream.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could make the port depend on its own feature large-int-placeholder for "platform": "windows".

Copy link
Contributor

Choose a reason for hiding this comment

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

I really mean listing in dependencies, not in default features:

  {
    "name": "man-group-sparrow",
    "default-features": false,
    "platform": "windows",
    "features": [
      "large-int-placeholder"
    ]
  },

plus formatting/sorting as needed.

)

vcpkg_cmake_configure(
SOURCE_PATH "${SOURCE_PATH}"
OPTIONS
${FEATURE_OPTIONS}
-DBUILD_TESTS=OFF
-DBUILD_EXAMPLES=OFF
Copy link
Contributor

Choose a reason for hiding this comment

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

In the 0.4 version, you can compile shared library using -DSPARROW_BUILD_SHARED, otherwise it compile in static

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alex-PLACET
Thanks!
I will upgrade sparrow to 0.4.0.

)

vcpkg_cmake_install()
vcpkg_cmake_config_fixup(PACKAGE_NAME sparrow CONFIG_PATH share/cmake/sparrow)

file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/share")

vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")
33 changes: 33 additions & 0 deletions ports/man-group-sparrow/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"name": "man-group-sparrow",
"version": "0.3.0",
"description": "C++20 idiomatic APIs for the Apache Arrow Columnar Format",
"homepage": "https://github.com/man-group/sparrow",
"license": "Apache-2.0",
"supports": "!uwp & !static",
"dependencies": [
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}
],
"default-features": [
"date",
"large-int-placeholder"
],
"features": {
"date": {
"description": "Use date polyfill implementation",
"dependencies": [
"date"
]
},
"large-int-placeholder": {
"description": "use types without api for big integers"
}
}
}
4 changes: 4 additions & 0 deletions versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5732,6 +5732,10 @@
"baseline": "1.0.3",
"port-version": 0
},
"man-group-sparrow": {
"baseline": "0.3.0",
"port-version": 0
},
"manif": {
"baseline": "0.0.5",
"port-version": 0
Expand Down
9 changes: 9 additions & 0 deletions versions/m-/man-group-sparrow.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"versions": [
{
"git-tree": "523558574344a27dd460f6b298e4c3b2043f1027",
"version": "0.3.0",
"port-version": 0
}
]
}