Skip to content

Conversation

@amoeba
Copy link
Contributor

@amoeba amoeba commented Jul 18, 2025

Summary

Changes to recipe: arrow/21.0.0

Motivation

Updates recipe to use arrow 21.0.0, released July 17th, 2025. See https://github.com/apache/arrow/releases/tag/apache-arrow-21.0.0.

Details

Changes are scripted so this review process may be similar to previous ones.


Copy link
Contributor

@franramirez688 franramirez688 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @amoeba 🙏

A couple of points before moving forward:

@amoeba
Copy link
Contributor Author

amoeba commented Jul 18, 2025

Thanks @franramirez688. Waiting a few days sounds fine and I'll integrate that patch later today.

@amoeba
Copy link
Contributor Author

amoeba commented Jul 21, 2025

Sorry for the delay. I added the patch in 1c71513. The CI was complaining about RapidJSON which wasn't an expected failure.

@amoeba
Copy link
Contributor Author

amoeba commented Jul 21, 2025

arrow 21.0.0 creates a new .so (built by default) so I'm expecting we'll also want a change like,

diff --git a/recipes/arrow/all/conanfile.py b/recipes/arrow/all/conanfile.py
index dfcc5707e..5d27b888a 100644
--- a/recipes/arrow/all/conanfile.py
+++ b/recipes/arrow/all/conanfile.py
@@ -446,6 +446,15 @@ class ArrowConan(ConanFile):
             self.cpp_info.components["libacero"].names["pkg_config"] = "acero"
             self.cpp_info.components["libacero"].requires = ["libarrow"]
 
+        if self.options.compute:
+            self.cpp_info.components["libcompute"].set_property("pkg_config_name", "compute")
+            self.cpp_info.components["libcompute"].set_property("cmake_target_name", f"Compute::arrow_compute_{cmake_suffix}")
+            self.cpp_info.components["libcompute"].libs = [f"arrow_compute{suffix}"]
+            self.cpp_info.components["libcompute"].names["cmake_find_package"] = "compute"
+            self.cpp_info.components["libcompute"].names["cmake_find_package_multi"] = "compute"
+            self.cpp_info.components["libcompute"].names["pkg_config"] = "compute"
+            self.cpp_info.components["libcompute"].requires = ["libarrow"]
+
         if self.options.gandiva:
             self.cpp_info.components["libgandiva"].set_property("pkg_config_name", "gandiva")
             self.cpp_info.components["libgandiva"].set_property("cmake_target_name", f"Gandiva::gandiva_{cmake_suffix}")

Copy link
Contributor

@franramirez688 franramirez688 left a comment

Choose a reason for hiding this comment

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

Hi @amoeba

I have just updated the patch with the correct lines (it was failing when applied) and the options, as parquet now requires RapidJSON.

Regarding the compute one, you're right. It's creating a new target now. I think that the changes to add should be something like:

        if self.options.compute:
            self.cpp_info.components["libarrow_compute"].set_property("pkg_config_name", "arrow_compute")
            self.cpp_info.components["libarrow_compute"].set_property("cmake_target_name", f"ArrowCompute::arrow_compute_{cmake_suffix}")
            self.cpp_info.components["libarrow_compute"].libs = [f"arrow_compute{suffix}"]
            self.cpp_info.components["libarrow_compute"].requires = ["libarrow"]

For the record (and not to be included in this PR), reviewing the rest of the components, I saw the existing one for acero, and I think it should be:

            self.cpp_info.components["libacero"].set_property("cmake_target_name", f"ArrowAcero::arrow_acero_{cmake_suffix}")

@amoeba
Copy link
Contributor Author

amoeba commented Jul 22, 2025

Thanks for the catch on the acero thing, I think you're fix is right.

I pushed up a change to add libarrow_compute like you suggested.

Additionally, I think we need to specify that dataset now depends on compute, see https://github.com/apache/arrow/pull/46261/files#diff-1855e31fd38e7fa63c95344ad9992c27d2f609f015a06d45ca3dd3ecc708243d. I also think there may be a latent bug in that area of the conanfile because I think that libs should be additive and I think they're not currently. Here's my patch with both changes:

diff --git a/recipes/arrow/all/conanfile.py b/recipes/arrow/all/conanfile.py
index 222e71e21..af24dc7ef 100644
--- a/recipes/arrow/all/conanfile.py
+++ b/recipes/arrow/all/conanfile.py
@@ -477,11 +477,16 @@ class ArrowConan(ConanFile):
             self.cpp_info.components["libarrow_flight_sql"].requires = ["libarrow", "libarrow_flight"]
 
         if self.options.dataset_modules:
+            dataset_requires = []
             self.cpp_info.components["dataset"].libs = ["arrow_dataset"]
             if self.options.parquet:
-                self.cpp_info.components["dataset"].requires = ["libparquet"]
+                dataset_requires.append("libparquet")
             if self.options.acero and Version(self.version) >= "19.0.0":
-                self.cpp_info.components["dataset"].requires = ["libacero"]
+                dataset_requires.append("libacero")
+            if self.options.compute and Version(self.version) >= "21.0.0":
+                dataset_requires.append("libcompute")
+            self.cpp_info.components["dataset"].requires = dataset_requires
 
         if self.options.cli and (self.options.with_cuda or self.options.with_flight_rpc or self.options.parquet):
             binpath = os.path.join(self.package_folder, "bin")

cc @raulcd

@raulcd
Copy link
Contributor

raulcd commented Jul 23, 2025

Acero depends on compute, and Dataset depends on Acero (and by transitive dependency on Compute), so I would add it as a requirement to Acero not to Compute.
Taking a look on the recipe there's also a couple of things that seems wrong, shouldn't the following be:

diff --git a/recipes/arrow/all/conanfile.py b/recipes/arrow/all/conanfile.py
index dfcc5707e..4dffde6a1 100644
--- a/recipes/arrow/all/conanfile.py
+++ b/recipes/arrow/all/conanfile.py
@@ -430,7 +430,7 @@ class ArrowConan(ConanFile):
 
         if self.options.get_safe("substrait"):
             self.cpp_info.components["libarrow_substrait"].set_property("pkg_config_name", "arrow_substrait")
-            self.cpp_info.components["libarrow_substrait"].set_property("cmake_target_name", f"Arrow::arrow_substrait_{cmake_suffix}")
+            self.cpp_info.components["libarrow_substrait"].set_property("cmake_target_name", f"ArrowSubstrait::arrow_substrait_{cmake_suffix}")
             self.cpp_info.components["libarrow_substrait"].libs = [f"arrow_substrait{suffix}"]
             self.cpp_info.components["libarrow_substrait"].requires = ["libparquet", "dataset"]
 
@@ -439,7 +439,7 @@ class ArrowConan(ConanFile):
 
         if self.options.acero:
             self.cpp_info.components["libacero"].set_property("pkg_config_name", "acero")
-            self.cpp_info.components["libacero"].set_property("cmake_target_name", f"Acero::arrow_acero_{cmake_suffix}")
+            self.cpp_info.components["libacero"].set_property("cmake_target_name", f"ArrowAcero::arrow_acero_{cmake_suffix}")
             self.cpp_info.components["libacero"].libs = [f"arrow_acero{suffix}"]
             self.cpp_info.components["libacero"].names["cmake_find_package"] = "acero"
             self.cpp_info.components["libacero"].names["cmake_find_package_multi"] = "acero"

Also, seems weird that dataset is treated differently as we can also expose the cmake_find_package, cmake_target_name as we do with others but we can do that on a separate PR. Link to documentation:
https://arrow.apache.org/docs/dev/cpp/build_system.html#other-available-packages

@franramirez688
Copy link
Contributor

Thanks for the new feedback @amoeba @raulcd 👏

Then, let's address in this PR that requirement acero -> compute, could you @amoeba?
I'll open another PR later to fix all the mentioned stuff 😁

@amoeba
Copy link
Contributor Author

amoeba commented Jul 23, 2025

Thanks @raulcd. Changed in e011d66.

Copy link
Contributor

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

These changes LGTM, I understand we want to do other changes on a follow up PR.

Copy link
Contributor

@franramirez688 franramirez688 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @amoeba @raulcd for the PR and the comments 👏

@jcar87 jcar87 merged commit af9371e into conan-io:master Jul 24, 2025
12 checks passed
@amoeba
Copy link
Contributor Author

amoeba commented Jul 24, 2025

Thanks @franramirez688 for the merge.

jeremydumais pushed a commit to jeremydumais/conan-center-index that referenced this pull request Oct 11, 2025
* arrow: add version 21.0.0

* Add patch

* parquet requires rapidjson and fixed patch

* Add libarrow_compute

* Add libarrow_compute as dep on acero

* Update recipes/arrow/all/conanfile.py

---------

Co-authored-by: Francisco Ramirez de Anton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants