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.

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

toge
Copy link
Contributor

@toge toge commented Feb 23, 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.

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

@MonicaLiu0311 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
Copy link
Contributor

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 MonicaLiu0311 marked this pull request as draft February 24, 2025 02:37
@MonicaLiu0311 MonicaLiu0311 changed the title [man-group-sparrow] add port [man-group-sparrow] New port Feb 24, 2025
@toge toge marked this pull request as ready for review February 25, 2025 00:11
@MonicaLiu0311
Copy link
Contributor

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)

@MonicaLiu0311 MonicaLiu0311 marked this pull request as draft February 25, 2025 08:55
@SylvainCorlay
Copy link

SylvainCorlay commented Feb 25, 2025

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

Comment on lines +9 to +12
+if("@USE_DATE_POLYFILL@")
+ find_dependency(date)
+endif()
+
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

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

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)

Comment on lines +1 to +3
if(VCPKG_TARGET_IS_LINUX)
message("Warning: `sparrow` requires Clang18+ or GCC 12+ on Linux")
endif()
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants