Subject: [bug#71897] [PATCH v8 6/8] gnu: Add cpp-ada-url-parser.

From: Maxim Cournoyer <[email-masked]>
Date: Sun, 22 Sep 2024 12:13:50 +0900

Hi Dan,

dan <[email-masked]> writes:

> *
> gnu/packages/patches/cpp-ada-url-parser-find-system-testing-dependencies.patch:
> New file.
> * gnu/local.mk (dist_patch_DATA): Register it.
> * gnu/packages/cpp.scm (cpp-ada-url-parser): New variable.
>
> Change-Id: I9924bba53ed58bbf306bf073c9724cd7bd6f570a
> ---
>  gnu/local.mk                                  |   1 +
>  gnu/packages/cpp.scm                          |  26 ++++
>  ...ser-find-system-testing-dependencies.patch | 130 ++++++++++++++++++
>  3 files changed, 157 insertions(+)
>  create mode 100644 gnu/packages/patches/cpp-ada-url-parser-find-system-testing-dependencies.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 802f4be4fe..829c5e166e 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1103,6 +1103,7 @@ dist_patch_DATA =						\
>    %D%/packages/patches/cool-retro-term-wctype.patch		\
>    %D%/packages/patches/coq-autosubst-1.8-remove-deprecated-files.patch		\
>    %D%/packages/patches/coreutils-gnulib-tests.patch		\
> +  %D%/packages/patches/cpp-ada-url-parser-find-system-testing-dependencies.patch\
>    %D%/packages/patches/cppcheck-disable-char-signedness-test.patch	\
>    %D%/packages/patches/cppdap-add-CPPDAP_USE_EXTERNAL_GTEST_PACKAGE.patch\
>    %D%/packages/patches/cpulimit-with-glib-2.32.patch		\
> diff --git a/gnu/packages/cpp.scm b/gnu/packages/cpp.scm
> index c805dae825..92be49967f 100644
> --- a/gnu/packages/cpp.scm
> +++ b/gnu/packages/cpp.scm
> @@ -3284,3 +3284,29 @@ (define-public tl-optional
>  the std::optional for C++11/14/17, with support for monadic operations added in
>  C++23.")
>      (license license:cc0)))
> +
> +(define-public cpp-ada-url-parser
> +  (package
> +    (name "cpp-ada-url-parser")

I wasn't sure about the naming, since it doesn't match with the upstream
repository name convention, but after looking into it, it seems
reasonable given their own naming is not explicit enough and there are
bindings for various languages.

> +    (version "2.9.2")
> +    (source
> +      (origin
> +        (method git-fetch)
> +        (uri (git-reference
> +              (url "https://github.com/ada-url/ada.git")

I believe 'guix lint' would complain about a URL redirect on the
above. '.git' suffices haven't been used in years for GitHub.

> +              (commit (string-append "v" version))))
> +        (file-name (git-file-name name version))
> +        (sha256 (base32 "0xvvjlia627ajl966gdxzy2b1j0jiimx7zx8ypmffwx0k6x72qam"))
> +        (patches (search-patches "cpp-ada-url-parser-find-system-testing-dependencies.patch"))))

Too long line; please break it so it fits under 80 chars (that's our
convention), although 'guix lint' won't warn until 100 chars.

> +    (build-system cmake-build-system)
> +    (native-inputs
> +     (list cxxopts
> +           fmt
> +           googletest
> +           python
> +           simdjson))
> +    (home-page "https://github.com/ada-url/ada")
> +    (synopsis "URL parser")
> +    (description "Ada is a fast and spec-compliant URL parser written in C++.
> +Specification for URL parser can be found from the WHATWG website.")
> +    (license license:gpl3+)))
> diff --git a/gnu/packages/patches/cpp-ada-url-parser-find-system-testing-dependencies.patch b/gnu/packages/patches/cpp-ada-url-parser-find-system-testing-dependencies.patch
> new file mode 100644
> index 0000000000..b32d162530
> --- /dev/null
> +++ b/gnu/packages/patches/cpp-ada-url-parser-find-system-testing-dependencies.patch
> @@ -0,0 +1,130 @@
> +From 74cac57a3cebe0cfbbc44f46270d5e51986f4881 Mon Sep 17 00:00:00 2001
> +From: dan <[email-masked]>
> +Date: Sat, 21 Sep 2024 18:49:10 +0800
> +Subject: [PATCH] find system testing dependencies.
> +
> +---
> + CMakeLists.txt             | 50 +++++++++-----------------------------
> + singleheader/amalgamate.py |  2 +-
> + tools/cli/CMakeLists.txt   |  8 ++----
> + 3 files changed, 14 insertions(+), 46 deletions(-)
> +
> +diff --git a/CMakeLists.txt b/CMakeLists.txt
> +index a7ce3796..0903cc31 100644
> +--- a/CMakeLists.txt
> ++++ b/CMakeLists.txt
> +@@ -28,43 +28,23 @@ option(ADA_TESTING "Build tests" ${BUILD_TESTING})
> + # errors due to CPM, so this is here to support disabling all the testing
> + # and tooling for ada if one only wishes to use the ada library.
> + if(ADA_TESTING OR ADA_BENCHMARKS OR ADA_TOOLS)
> +-  include(cmake/CPM.cmake)
> +-  # CPM requires git as an implicit dependency
> +-  find_package(Git QUIET)
> +   # We use googletest in the tests
> +-  if(Git_FOUND AND ADA_TESTING)
> +-    CPMAddPackage(
> +-      NAME GTest
> +-      GITHUB_REPOSITORY google/googletest
> +-      VERSION 1.14.0
> +-      OPTIONS  "BUILD_GMOCK OFF" "INSTALL_GTEST OFF"
> +-    )
> ++  if(ADA_TESTING)
> ++    find_package(GTest)
> +   endif()
> +   # We use simdjson in both the benchmarks and tests
> +-  if(Git_FOUND AND (ADA_TESTING OR ADA_BENCHMARKS))
> +-    CPMAddPackage("gh:simdjson/[email-masked]")
> ++  if(ADA_TESTING OR ADA_BENCHMARKS)
> ++    find_package(simdjson)
> +   endif()
> +   # We use Google Benchmark, but it does not build under several 32-bit systems.
> +-  if(Git_FOUND AND ADA_BENCHMARKS AND (CMAKE_SIZEOF_VOID_P EQUAL 8))
> +-    CPMAddPackage(
> +-      NAME benchmark
> +-      GITHUB_REPOSITORY google/benchmark
> +-      GIT_TAG f91b6b4
> +-      OPTIONS "BENCHMARK_ENABLE_TESTING OFF"
> +-              "BENCHMARK_ENABLE_INSTALL OFF"
> +-              "BENCHMARK_ENABLE_WERROR OFF"
> +-
> +-    )
> ++  if(ADA_BENCHMARKS AND (CMAKE_SIZEOF_VOID_P EQUAL 8))
> ++    find_package(benchmark)
> +   endif()
> + 
> +   if (ADA_TESTING AND NOT EMSCRIPTEN)
> +-    if(Git_FOUND)
> +-      set(CTEST_TEST_TIMEOUT 5)
> +-      message(STATUS "The tests are enabled.")
> +-      add_subdirectory(tests)
> +-    else()
> +-      message(STATUS "The tests are disabled because git was not found.")
> +-    endif()
> ++    set(CTEST_TEST_TIMEOUT 5)
> ++    message(STATUS "The tests are enabled.")
> ++    add_subdirectory(tests)
> +   else()
> +     if(is_top_project)
> +       message(STATUS "The tests are disabled.")
> +@@ -72,12 +52,8 @@ if(ADA_TESTING OR ADA_BENCHMARKS OR ADA_TOOLS)
> +   endif(ADA_TESTING AND NOT EMSCRIPTEN)
> + 
> +   If(ADA_BENCHMARKS AND NOT EMSCRIPTEN)
> +-    if(Git_FOUND)
> +-      message(STATUS "Ada benchmarks enabled.")
> +-      add_subdirectory(benchmarks)
> +-    else()
> +-      message(STATUS "The benchmarks are disabled because git was not found.")
> +-    endif()
> ++    message(STATUS "Ada benchmarks enabled.")
> ++    add_subdirectory(benchmarks)
> +   else(ADA_BENCHMARKS AND NOT EMSCRIPTEN)
> +     if(is_top_project)
> +       message(STATUS "Ada benchmarks disabled. Set ADA_BENCHMARKS=ON to enable them.")
> +@@ -107,11 +83,7 @@ if(NOT ADA_COVERAGE AND NOT EMSCRIPTEN)
> + endif()
> + 
> + if(ADA_TOOLS)
> +-  if(Git_FOUND)
> +     add_subdirectory(tools)
> +-  else()
> +-    message(STATUS "The tools are disabled because git was not found.")
> +-  endif()
> + endif()
> + 
> + install(
> +diff --git a/singleheader/amalgamate.py b/singleheader/amalgamate.py
> +index 52b154b6..94e4e638 100755
> +--- a/singleheader/amalgamate.py
> ++++ b/singleheader/amalgamate.py
> +@@ -138,7 +138,7 @@ if SCRIPTPATH != AMALGAMATE_OUTPUT_PATH:
> + 
> + shutil.copy2(os.path.join(AMALGAMATE_INCLUDE_PATH, 'ada_c.h'), AMALGAMATE_OUTPUT_PATH)
> + 
> +-zf = zipfile.ZipFile(os.path.join(AMALGAMATE_OUTPUT_PATH, 'singleheader.zip'), 'w', zipfile.ZIP_DEFLATED)
> ++zf = zipfile.ZipFile(os.path.join(AMALGAMATE_OUTPUT_PATH, 'singleheader.zip'), 'w', zipfile.ZIP_DEFLATED, strict_timestamps=False)
> + zf.write(os.path.join(AMALGAMATE_OUTPUT_PATH, 'ada.cpp'), 'ada.cpp')
> + zf.write(os.path.join(AMALGAMATE_OUTPUT_PATH, 'ada.h'), 'ada.h')
> + zf.write(os.path.join(AMALGAMATE_INCLUDE_PATH, 'ada_c.h'), 'ada_c.h')
> +diff --git a/tools/cli/CMakeLists.txt b/tools/cli/CMakeLists.txt
> +index 9f0da167..d0f7e0c9 100644
> +--- a/tools/cli/CMakeLists.txt
> ++++ b/tools/cli/CMakeLists.txt
> +@@ -8,12 +8,8 @@ if(MSVC AND BUILD_SHARED_LIBS)
> +         "$<TARGET_FILE:ada>"      # <--this is in-file
> +         "$<TARGET_FILE_DIR:adaparse>")                 # <--this is out-file path
> + endif()
> +-CPMAddPackage("gh:fmtlib/fmt#10.2.1")
> +-CPMAddPackage(
> +-  GITHUB_REPOSITORY jarro2783/cxxopts
> +-  VERSION 3.2.0
> +-  OPTIONS "CXXOPTS_BUILD_EXAMPLES NO" "CXXOPTS_BUILD_TESTS NO" "CXXOPTS_ENABLE_INSTALL YES"
> +-)
> ++find_package(fmt)
> ++find_package(cxxopts)
> + target_link_libraries(adaparse PRIVATE cxxopts::cxxopts fmt::fmt)
> + 
> + if(MSVC OR MINGW)
> +-- 
> +2.45.2
> +

That's not upstreamable work, so should ideally be marked as such as a
patch at the top of the patch file (Upstream-status: N/A or similar).
It's a bit sad that this will need ongoing maintenance (careful rebasing
when they touch that file).  I'd like to see the problem with CPM
commented in a bit more details -- wondering if it could be made to work
without patching it out, e.g. perhaps some CMake variables can be set to
have CPM work offline?

-- 
Thanks,
Maxim