[man-group-sparrow] New port #43989

toge · 2025-02-23T17:11:53Z

There are several sparrow pacakges. ref
sparrow 0.3.0 provides shared library only.

ports/man-group-sparrow/vcpkg.json

MonicaLiu0311 · 2025-02-24T02:37:35Z

Please get failure logs for x64-windows here.

[15/27] C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1442~1.344\bin\Hostx64\x64\cl.exe   /TP -DSPARROW_EXPORTS -Dsparrow_EXPORTS -ID:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\include /nologo /DWIN32 /D_WINDOWS /utf-8 /GR /EHsc /MP  /MDd /Z7 /Ob0 /Od /RTC1  -std:c++20 -MDd /showIncludes @CMakeFiles\sparrow.dir\src\types\data_type.cpp.obj.modmap /FoCMakeFiles\sparrow.dir\src\types\data_type.cpp.obj /FdCMakeFiles\sparrow.dir\ /FS -c D:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\src\types\data_type.cpp
FAILED: CMakeFiles/sparrow.dir/src/types/data_type.cpp.obj 
C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1442~1.344\bin\Hostx64\x64\cl.exe   /TP -DSPARROW_EXPORTS -Dsparrow_EXPORTS -ID:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\include /nologo /DWIN32 /D_WINDOWS /utf-8 /GR /EHsc /MP  /MDd /Z7 /Ob0 /Od /RTC1  -std:c++20 -MDd /showIncludes @CMakeFiles\sparrow.dir\src\types\data_type.cpp.obj.modmap /FoCMakeFiles\sparrow.dir\src\types\data_type.cpp.obj /FdCMakeFiles\sparrow.dir\ /FS -c D:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\src\types\data_type.cpp
D:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\include\sparrow/details/3rdparty/large_integers/int128_t.hpp(92): error C2065: 'int128_t': undeclared identifier
D:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\include\sparrow/details/3rdparty/large_integers/int128_t.hpp(92): error C2923: 'primesum::prt::numeric_limits': 'int128_t' is not a valid template type argument for parameter 'T'
D:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\include\sparrow/details/3rdparty/large_integers/int128_t.hpp(92): note: see declaration of 'int128_t'
D:\b\man-group-sparrow\src\0.3.0-779a92d04f.clean\include\sparrow/details/3rdparty/large_integers/int128_t.hpp(92): error C2913: explicit specialization; 'primesum::prt::numeric_limits' is not a specialization of a class template

MonicaLiu0311 · 2025-02-25T08:54:24Z

When testing usage, the following error occurs:

MSBuild version 17.11.2+c078802d4 for .NET Framework

  1>Checking Build System
  Building Custom Rule E:/test_usage/test/CMakeLists.txt
  test.cpp
E:\man-group-sparrow\installed\x64-windows\include\sparrow\details\3rdparty\large_integers\int128_t.hpp(92,23): error C2065: 'int128_t': undeclared identifier [E:\test_usage\test\b
uild\main.vcxproj]
  (compiling source file '../test.cpp')

E:\man-group-sparrow\installed\x64-windows\include\sparrow\details\3rdparty\large_integers\int128_t.hpp(92,8): error C2923: 'primesum::prt::numeric_limits': 'int128_t' is not a valid tem
plate type argument for parameter 'T' [E:\test_usage\test\build\main.vcxproj]
  (compiling source file '../test.cpp')
      E:\man-group-sparrow\installed\x64-windows\include\sparrow\details\3rdparty\large_integers\int128_t.hpp(92,23):
      see declaration of 'int128_t'

test.cpp

#include <iostream>
#include "sparrow/sparrow.hpp"
using namespace std;
int main()

{

cout << "Hello CMake." << endl;

return 0;

}

CMakeLists.txt

cmake_minimum_required (VERSION 3.8)
set(CMAKE_CXX_STANDARD 20)

set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(CMAKE_TOOLCHAIN_FILE "E:/man-group-sparrow/scripts/buildsystems/vcpkg.cmake")
project ("test")
add_library (main "test.cpp")
find_package(sparrow CONFIG REQUIRED)

target_link_libraries(main PRIVATE sparrow)

SylvainCorlay · 2025-02-25T13:53:10Z

@toge would you be able to name that package "sparrow" rather than "man-group-sparrow"?

It is named sparrow on Fedora and conda-forge. Vcpkg using a different name for the package would cause a lot of confusion in my opinion.

Alex-PLACET · 2025-02-25T14:11:45Z

ports/man-group-sparrow/find-date.patch

+if("@USE_DATE_POLYFILL@")
+    find_dependency(date)
+endif()
+


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

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

Alex-PLACET · 2025-02-25T14:18:59Z

ports/man-group-sparrow/portfile.cmake

+    OPTIONS
+        ${FEATURE_OPTIONS}
+        -DBUILD_TESTS=OFF
+        -DBUILD_EXAMPLES=OFF


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

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

Alex-PLACET · 2025-02-25T14:19:59Z

ports/man-group-sparrow/portfile.cmake

+vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
+    FEATURES
+        date                  USE_DATE_POLYFILL
+        large-int-placeholder USE_LARGE_INT_PLACEHOLDERS


USE_LARGE_INT_PLACEHOLDERS is mandatory on Windows

This is the fix for: #43989 (comment)

@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.

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

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.

Alex-PLACET · 2025-02-25T14:28:42Z

ports/man-group-sparrow/portfile.cmake

+if(VCPKG_TARGET_IS_LINUX)
+    message("Warning: `sparrow` requires Clang18+ or GCC 12+ on Linux")
+endif()


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

@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.

toge · 2025-02-25T17:34:08Z

@SylvainCorlay
Thank you for your comment.
I am naming the packages this way because I have no choice but to follow the maintainers guide for package names.

https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md#check-names-against-other-repositories

@MonicaLiu0311
Are there any maintener guid exception conditions in the package naming?
If there are conditions, could they apply to this case?

SylvainCorlay · 2025-02-25T18:13:52Z

@toge would you be able to name that package "sparrow" rather than "man-group-sparrow"?

It is named sparrow on Fedora and conda-forge. Vcpkg using a different name for the package would cause a lot of confusion in my opinion.

I am naming the packages this way because I have no choice but to follow the maintainers guide for package names.

https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md#check-names-against-other-repositories

The policy says "Projects with short names or named after common words may require disambiguation, specially when there are no projects with a strong association to the given word. "

I guess that we fall in the category of "common word", although the policy uses the "may" word, which is not a strong statement. In terms of strong association, sparrow is clearly not zlib or openssl, but it is available under that name on several software distributions, including a linux distribution.

dg0yt · 2025-02-25T19:10:24Z

There was a name discussion last year, with that outcome: #37221 (comment)

SylvainCorlay · 2025-02-25T20:48:31Z

There was a name discussion last year, with that outcome: #37221 (comment)

At that time, sparrow was not available on Fedora. We were at at a 0.0.x release level. So I believe the situation is a bit different now.

MonicaLiu0311 · 2025-02-27T10:02:22Z

There was a name discussion last year, with that outcome: #37221 (comment)

At that time, sparrow was not available on Fedora. We were at at a 0.0.x release level. So I belive the situation is a bit different now.

@BillyONeal Please help us check whether we should maintain the original judgment on the port name. Information about sparrow on other platforms:

https://packages.fedoraproject.org/pkgs/sparrow/sparrow/
https://anaconda.org/conda-forge/sparrow

… due to gcc 11

[man-group-sparrow] add port

9218f80

JonLiu1993 assigned MonicaLiu0311 Feb 24, 2025

MonicaLiu0311 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Feb 24, 2025

MonicaLiu0311 reviewed Feb 24, 2025

View reviewed changes

ports/man-group-sparrow/vcpkg.json Outdated Show resolved Hide resolved

MonicaLiu0311 marked this pull request as draft February 24, 2025 02:37

MonicaLiu0311 changed the title ~~[man-group-sparrow] add port~~ [man-group-sparrow] New port Feb 24, 2025

toge added 4 commits February 24, 2025 13:17

fix licenses, make date default-features, disable ci on x64-linux

0463467

add large-int-placeholder feature

a7a84c3

unsupport static

b09fe5f

revert x64-linux build

f0bc949

toge marked this pull request as ready for review February 25, 2025 00:11

MonicaLiu0311 marked this pull request as draft February 25, 2025 08:55

Alex-PLACET reviewed Feb 25, 2025

View reviewed changes

update 0.4.0

39a4f9e

make large-int-placeholder default-features on windows only

8c3fb6d

force large-int-placefolder on arm-*-android, disable ci on x64-linux…

88d4d63

… due to gcc 11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[man-group-sparrow] New port #43989

[man-group-sparrow] New port #43989

toge commented Feb 23, 2025

MonicaLiu0311 commented Feb 24, 2025

MonicaLiu0311 commented Feb 25, 2025

SylvainCorlay commented Feb 25, 2025 •

edited

Loading

Alex-PLACET Feb 25, 2025

toge Feb 25, 2025

Alex-PLACET Feb 25, 2025

toge Feb 25, 2025

Alex-PLACET Feb 25, 2025

Alex-PLACET Feb 25, 2025

toge Feb 25, 2025

dg0yt Feb 25, 2025

dg0yt Feb 26, 2025

Alex-PLACET Feb 25, 2025

toge Feb 25, 2025

toge commented Feb 25, 2025

SylvainCorlay commented Feb 25, 2025

dg0yt commented Feb 25, 2025

SylvainCorlay commented Feb 25, 2025 •

edited

Loading

MonicaLiu0311 commented Feb 27, 2025 •

edited

Loading

[man-group-sparrow] New port #43989

Are you sure you want to change the base?

[man-group-sparrow] New port #43989

Conversation

toge commented Feb 23, 2025

MonicaLiu0311 commented Feb 24, 2025

MonicaLiu0311 commented Feb 25, 2025

SylvainCorlay commented Feb 25, 2025 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toge commented Feb 25, 2025

SylvainCorlay commented Feb 25, 2025

dg0yt commented Feb 25, 2025

SylvainCorlay commented Feb 25, 2025 • edited Loading

MonicaLiu0311 commented Feb 27, 2025 • edited Loading

SylvainCorlay commented Feb 25, 2025 •

edited

Loading

SylvainCorlay commented Feb 25, 2025 •

edited

Loading

MonicaLiu0311 commented Feb 27, 2025 •

edited

Loading