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

[ada-url] update to 3.0.1 #43594

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

[ada-url] update to 3.0.1 #43594

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
001fd5a
[ada-url] update to 3.0.1
toge Feb 2, 2025
e828d1c
revert crlf
toge Feb 2, 2025
4bad00a
use ADA_TESTING
toge Feb 3, 2025
fc2644c
enable C++20, remove default options
toge Feb 3, 2025
5acd6f4
(trial) disable building tool
toge Feb 3, 2025
d173099
remove patches, disable linux-x64 tasks (using gcc11)
toge Feb 9, 2025
91c3393
disable benchmark and test explicitly
toge Feb 11, 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
79 changes: 48 additions & 31 deletions ports/ada-url/no-cpm.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,48 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b206edb6..1db4099d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -30,7 +30,6 @@ 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
diff --git a/tools/cli/CMakeLists.txt b/tools/cli/CMakeLists.txt
index ff57220b..a6d90f29 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 CONFIG REQUIRED)
+find_package(cxxopts CONFIG REQUIRED)
Copy link
Member

Choose a reason for hiding this comment

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

I believe this patch needs to be restored to ensure these come from their associated ports rather than being effectively vendored. @anonrig I believe you are mistaken. In particular, CPM is not given the find_package'd name of these packages, so I don't understand how it could ever resolve them correctly.

Building ada-url:[email protected]...
Trying to download ada-url-ada-v3.0.1.tar.gz using asset cache https://vcpkgassetcachewus.blob.core.windows.net/cache/3a81da352ad47395d83172f1477e8165483c5dc30923ba4387463a03aa01df5d6718dd54d22f568a654300e777b3eafce7f266504a27bfeb1f8de57503670dc4?*** SECRET ***
Download successful! Asset cache hit, did not try authoritative source https://github.com/ada-url/ada/archive/v3.0.1.tar.gz
-- Extracting source D:/downloads/ada-url-ada-v3.0.1.tar.gz
-- Using source at D:/b/ada-url/src/v3.0.1-d5321571b6.clean
-- Found external ninja('1.12.1').
-- Configuring x64-windows
CMake Warning at D:/installed/x64-windows/share/vcpkg-cmake/vcpkg_cmake_configure.cmake:344 (message):
  The following variables are not used in CMakeLists.txt:

      CPM_USE_LOCAL_PACKAGES

  Please recheck them and remove the unnecessary options from the
  `vcpkg_cmake_configure` call.

  If these options should still be passed for whatever reason, please use the
  `MAYBE_UNUSED_VARIABLES` argument.
Call Stack (most recent call first):
  ports/ada-url/portfile.cmake:19 (vcpkg_cmake_configure)
  scripts/ports.cmake:196 (include)


-- Building x64-windows-dbg
-- Building x64-windows-rel
-- Performing post-build validation
Store success

target_link_libraries(adaparse PRIVATE cxxopts::cxxopts fmt::fmt)

if(MSVC OR MINGW)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c70094f..b10b3c7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -22,7 +22,6 @@ set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/scripts/cmake)
# 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
# We use googletest in the tests
if(ADA_TESTING)
@@ -77,6 +76,7 @@ endif()


add_library(ada::ada ALIAS ada)
+target_compile_features(ada PUBLIC cxx_std_20)

Copy link

Choose a reason for hiding this comment

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

Why is this required? It should work as it is.

if(ADA_TESTING)
# IMPORTANT!
@@ -103,7 +103,7 @@ if(NOT ADA_COVERAGE AND NOT EMSCRIPTEN)
endif()

if(ADA_TOOLS)
- add_subdirectory(tools)
+# add_subdirectory(tools)
endif()
Copy link

Choose a reason for hiding this comment

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

This seems to be unnecessary.


install(
diff --git a/tools/cli/CMakeLists.txt b/tools/cli/CMakeLists.txt
index 397b428..87e405e 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#11.0.2")
-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 CONFIG REQUIRED)
+find_package(cxxopts CONFIG REQUIRED)
target_link_libraries(adaparse PRIVATE cxxopts::cxxopts fmt::fmt)
Copy link

Choose a reason for hiding this comment

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

These changes are unnecessary. If you pass the correct build flag, CPM will never install a new package and only use the existing packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for review.
I agree wiyh you.
These modifications are my trials for CI fails.
Now I undestand how to fix CI fails, I will revert these modifications.
Please wait a moment.


if(MSVC OR MINGW)
8 changes: 5 additions & 3 deletions ports/ada-url/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
if(VCPKG_TARGET_IS_LINUX)
message(WARNING "Building ${PORT} requires a C++20 compliant compiler. GCC 12 and Clang 15 are known to work.")
endif()

vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO ada-url/ada
REF "v${VERSION}"
SHA512 bc876db537153d6b0599215ca8be8261bceca6d213fcc63af5fda13c1b32876496cb8d4e98c787f17317cf8ffd1940431551513807f1a18f9ce993fad35f5ec6
SHA512 3a81da352ad47395d83172f1477e8165483c5dc30923ba4387463a03aa01df5d6718dd54d22f568a654300e777b3eafce7f266504a27bfeb1f8de57503670dc4
HEAD_REF main
PATCHES
no-cpm.patch
Expand All @@ -17,8 +21,6 @@ vcpkg_check_features(
vcpkg_cmake_configure(
SOURCE_PATH "${SOURCE_PATH}"
OPTIONS
-DADA_BENCHMARKS=OFF
-DBUILD_TESTING=OFF
FrankXie05 marked this conversation as resolved.
Show resolved Hide resolved
-DCMAKE_DISABLE_FIND_PACKAGE_Python3=ON
${FEATURE_OPTIONS}
OPTIONS_DEBUG
Expand Down
4 changes: 2 additions & 2 deletions ports/ada-url/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"name": "ada-url",
"version": "2.9.2",
"version": "3.0.1",
"description": "WHATWG-compliant and fast URL parser written in modern C++",
"homepage": "https://ada-url.com/",
"license": "MIT",
"license": "MIT OR Apache-2.0",
"dependencies": [
{
"name": "vcpkg-cmake",
Expand Down
5 changes: 5 additions & 0 deletions versions/a-/ada-url.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "c930f5c5ebdb9169116415060e03e3b17a608741",
"version": "3.0.1",
"port-version": 0
},
{
"git-tree": "ca729da9e664e3a59d88115c9e8a8d59b775801c",
"version": "2.9.2",
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"port-version": 17
},
"ada-url": {
"baseline": "2.9.2",
"baseline": "3.0.1",
"port-version": 0
},
"ade": {
Expand Down
Loading