Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2069210359 ## python/MANIFEST.in: ## @@ -1,15 +0,0 @@ -include README.md -include ../LICENSE.txt Review Comment: I patched meson (not meson-python) so that building binary artifacts and installing them with meson's SBOM generator (automatically produced and installed when passing `--licensedir=/usr/share/licenses/PyArrow/`) should correctly install the license. In principle, that's also how dynamically defined pyproject licenses get inserted into a wheel. Building a standalone `meson dist` tarball release (similar to Autotools `make distcheck`) will not try to resolve the license, at least not currently. My thoughts were that it is *obviously incorrect* to mishandle a clear directive: > license-file: ../XXX.md > install-licenses: yes and we can make this "obviously correct" so that monorepos will install correctly. But it's not immediately apparent whether or not most monorepos utilizing symlinks between different directories, expect that a single directory should also have standalone source code releases (they may only support installing from the entire monorepo workspace, or they may generate per-component binary packages from the monorepo and then tell people to install from binaries). Building source code tarballs is a totally optional thing that many people don't do (both in the C/C++ world and in the python world. When was the last time you saw a Torch sdist on PyPI? When was the last time you tried to build python-tensorflow using any kind of PyPA standards at all? If some organizational design makes it inconvenient or troublesome to do an sdist, that should never be a valid reason to unnecessarily restrict wheel creation. I'm definitely interested to hear more about solutions which monorepo users would like to see, as monorepos are a completely valid use case. But it's possible that producing an sdist from a monorepo requires designing a feature, not just fixing a bug. (Or not. I don't usually work with monorepos so I'm not an expert on designing workflows for them.) And those take longer to formulate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2069210359 ## python/MANIFEST.in: ## @@ -1,15 +0,0 @@ -include README.md -include ../LICENSE.txt Review Comment: I patched meson (not meson-python) so that building binary artifacts and installing them with meson's SBOM generator (automatically produced and installed when passing `--licensedir=/usr/share/licenses/PyArrow/`) should correctly install the license. Building a standalone `meson dist` tarball release (similar to Autotools `make distcheck`) will not try to resolve the license, at least not currently. My thoughts were that it is *obviously incorrect* to mishandle a clear directive: > license-file: ../XXX.md > install-licenses: yes and we can make this "obviously correct" so that monorepos will install correctly. But it's not immediately apparent whether or not most monorepos utilizing symlinks between different directories, expect that a single directory should also have standalone source code releases (they may only support installing from the entire monorepo workspace, or they may generate per-component binary packages from the monorepo and then tell people to install from binaries). Building source code tarballs is a totally optional thing that many people don't do (both in the C/C++ world and in the python world. When was the last time you saw a Torch sdist on PyPI? When was the last time you tried to build python-tensorflow using any kind of PyPA standards at all? If some organizational design makes it inconvenient or troublesome to do an sdist, that should never be a valid reason to unnecessarily restrict wheel creation. I'm definitely interested to hear more about solutions which monorepo users would like to see, as monorepos are a completely valid use case. But it's possible that producing an sdist from a monorepo requires designing a feature, not just fixing a bug. (Or not. I don't usually work with monorepos so I'm not an expert on designing workflows for them.) And those take longer to formulate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2841992929 @WillAyd You should see it above: https://github.com/apache/arrow/pull/45854#pullrequestreview-2751010946 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2068686252 ## python/MANIFEST.in: ## @@ -1,15 +0,0 @@ -include README.md -include ../LICENSE.txt Review Comment: Ah OK; I can get these added into the meson configuration then. I recall @eli-schwartz patched something to allow for this -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2841986439 @pitrou I saw a notification for a comment from you on this, but I don't see one. I am assuming it was deleted, but let me know if I am overlooking something -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2068652909 ## python/MANIFEST.in: ## @@ -1,15 +0,0 @@ -include README.md -include ../LICENSE.txt Review Comment: Yeah, it's a bug in PyArrow, because we misunderstood how MANIFEST works. See https://github.com/pypa/setuptools/issues/4892 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2048874659
##
ci/scripts/python_sdist_build.sh:
##
@@ -23,5 +23,5 @@ source_dir=${1}/python
pushd ${source_dir}
export SETUPTOOLS_SCM_PRETEND_VERSION=${PYARROW_VERSION:-}
-${PYTHON:-python} setup.py sdist
+${PYTHON:-python} -m build --sdist .
Review Comment:
The failure was happening during the release verification script, which was
testing sdist creation outside of the git VCS
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2813378287 Revision: 71d36133dcb2000dc7b01b5d687dd7bb1f395e72 Submitted crossbow builds: [ursacomputing/crossbow @ actions-9556f9e16c](https://github.com/ursacomputing/crossbow/branches/all?query=actions-9556f9e16c) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14519477743/job/40736612383)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478370/job/40736617197)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478333/job/40736617351)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14519477462/job/40736610738)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478058/job/40736614638)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478343/job/40736617764)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14519477806/job/40736612950)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14519477854/job/40736613006)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14519477875/job/40736614305)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14519477459/job/40736611007)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478200/job/40736614729)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478335/job/40736617162)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478400/job/40736618050)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478082/job/40736614736)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14519478439/job/40736618118)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2813371134 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
assignUser commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2048809749
##
ci/scripts/python_sdist_build.sh:
##
@@ -23,5 +23,5 @@ source_dir=${1}/python
pushd ${source_dir}
export SETUPTOOLS_SCM_PRETEND_VERSION=${PYARROW_VERSION:-}
-${PYTHON:-python} setup.py sdist
+${PYTHON:-python} -m build --sdist .
Review Comment:
Isn't this being run in a checkout anyway? (in ci etc.), I assume you saw
failures somewhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2811070577 The minimal job failures I think are from a regression introduced by https://github.com/apache/arrow/pull/46057#issuecomment-2811068182 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2810562790 Revision: 3b907a666f9f8e40bdb44f798bf0ce1a3b03581c Submitted crossbow builds: [ursacomputing/crossbow @ actions-2b1639a26c](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2b1639a26c) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14500960940/job/40680250684)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961025/job/40680252099)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14500960712/job/40680249356)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961398/job/40680253178)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961173/job/40680253365)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14500960886/job/40680250245)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961066/job/40680252063)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14500960718/job/40680248922)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961340/job/40680254256)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14500960823/job/40680249395)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961536/job/40680254220)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14500960668/job/40680248902)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961323/job/40680253626)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961062/job/40680251966)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14500961231/job/40680253292)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2810557307 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
mgorny commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044645547 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", Review Comment: Why do you need to add a dependency on CMake? Such a dependency is particularly problematic for downstreams, given that regular CMake installations don't provide a `cmake` Python distribution, and following the `build-system.requires` here implies having a non-distribution prebuilt CMake version installed and overriding the correct CMake executable. If such a dependency is really required, then it should be added via `get_requires_for_build_wheel()` hook if system CMake executable is not provided, much like `scikit-build-core` does. ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", "cython >= 3", # Starting with NumPy 1.25, NumPy is (by default) as far back compatible # as oldest-support-numpy was (customizable with a NPY_TARGET_VERSION # define). For older Python versions (where NumPy 1.25 is not yet available) # continue using oldest-support-numpy. "oldest-supported-numpy>=0.14; python_version<'3.9'", "numpy>=1.25; python_version>='3.9'", +"meson>=1.3.0", Review Comment: Is this necessary to enforce a minimum meson version? Again, this is a problematic dependency for downstreams. To the best of my knowledge, `meson-python` calls `meson` as an external executable, and we do strongly prefer that package builds used our downstream version of `meson` rather than installing another one for the purpose of building pyarrow. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044767032 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", "cython >= 3", # Starting with NumPy 1.25, NumPy is (by default) as far back compatible # as oldest-support-numpy was (customizable with a NPY_TARGET_VERSION # define). For older Python versions (where NumPy 1.25 is not yet available) # continue using oldest-support-numpy. "oldest-supported-numpy>=0.14; python_version<'3.9'", "numpy>=1.25; python_version>='3.9'", +"meson>=1.3.0", Review Comment: Yes Meson wants you to specify a minimum version. In more recent releases, it will throw deprecation warnings if you do not do this: https://mesonbuild.com/Release-notes-for-1-6-0.html#default-to-printing-deprecations-when-no-minimum-version-is-specified As far as 1.3.0 is concerned, I just started with that as baseline from other projects like arrow-nanoarrow, numpy, etc...If there is a need for older versions, we can definitely analyze how far back we want to go and reassess what features may need to be tweaked With that said, what is preventing you downstream from just using build isolation when installing or building the package? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044740300 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", Review Comment: Ah nice catch - I think this is a relic from the original design which also let you build Arrow C++ from source and use it as a subproject. We scrapped that in favor of requiring Arrow C++ to be installed for PyArrow to build, so this can likely be removed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
mgorny commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2805643046 > > I wonder if this change could make resolving #45576 easier > > I started down this path and ended up removing it in [9f5df6e](https://github.com/apache/arrow/commit/9f5df6e94a8eddcba82115f631935d40e15bd57e) after some feedback and to keep the scope of this more refined. However, I don't think it would be that difficult to achieve with meson-python; generally the way meson forces you to handle dependencies as subprojects helps to streamline your build configuration Yeah, I can imagine people not wanting to have to maintain translation from Meson to CMake (much like my earlier PR #45580 was rejected). However, I think this will become simpler if it's just Meson-to-Meson calling, given that C++ part seems to support Meson now. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044904070 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", "cython >= 3", # Starting with NumPy 1.25, NumPy is (by default) as far back compatible # as oldest-support-numpy was (customizable with a NPY_TARGET_VERSION # define). For older Python versions (where NumPy 1.25 is not yet available) # continue using oldest-support-numpy. "oldest-supported-numpy>=0.14; python_version<'3.9'", "numpy>=1.25; python_version>='3.9'", +"meson>=1.3.0", Review Comment: > Is this necessary to enforce a minimum meson version? Again, this is a problematic dependency for downstreams. To the best of my knowledge, `meson-python` calls `meson` as an external executable, and we do strongly prefer that package builds used our downstream version of `meson` rather than installing another one for the purpose of building pyarrow. I mused about this at https://github.com/mesonbuild/meson-python/pull/280#issuecomment-1411507093 It would be possible for meson-python to read the version requirements from meson.build and update `get_requires_for_*` if not detected as preinstalled at runtime. This would allow maintaining the minimum required version in *one* place, in meson.build, and have it enforced everywhere. (It appears my comment there got misinterpreted, which fair enough, I didn't provide as much detail as I should have.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
mgorny commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044848777 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", "cython >= 3", # Starting with NumPy 1.25, NumPy is (by default) as far back compatible # as oldest-support-numpy was (customizable with a NPY_TARGET_VERSION # define). For older Python versions (where NumPy 1.25 is not yet available) # continue using oldest-support-numpy. "oldest-supported-numpy>=0.14; python_version<'3.9'", "numpy>=1.25; python_version>='3.9'", +"meson>=1.3.0", Review Comment: It would. It would also prevent me from using a new enough meson if it happens to be installed for a different Python version than the one the package is being built for, because then the .dist-info wouldn't be detected. As I've said regarding CMake, the usual way of dealing with this is to enforce the dependency via `get_requires_for_build_wheel`. To be honest, I'm somewhat surprised `meson-python` doesn't do that already — they seem to have correct logic for `ninja` and `patchelf`, but for some reason insist on listing `meson` as a Python dependency rather than an external dependency. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2805774399 > given that C++ part seems to support Meson now. I have been working on this but it is highly experimental and not yet a first class supported build system for Arrow C++. So in any case the CMake -> Meson transition for the Python build I _think_ will be around for a while -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044831701 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", "cython >= 3", # Starting with NumPy 1.25, NumPy is (by default) as far back compatible # as oldest-support-numpy was (customizable with a NPY_TARGET_VERSION # define). For older Python versions (where NumPy 1.25 is not yet available) # continue using oldest-support-numpy. "oldest-supported-numpy>=0.14; python_version<'3.9'", "numpy>=1.25; python_version>='3.9'", +"meson>=1.3.0", Review Comment: Ah OK thanks for clarifying. But wouldn't having this in the pyproject.toml then prevent you from a situation where you have a version in your build environment that doesn't satisfy the requirement in meson.build? Not something I want to get hung up on, but it is also worth noting that meson and meson-python are not attached to the same release schedule, so you can't simply control the version of the latter to always get the functionality from meson that you may require. See also the meson-python docs: https://mesonbuild.com/meson-python/reference/meson-compatibility.html#meson-compatibility -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
mgorny commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2044804389 ## python/pyproject.toml: ## @@ -17,19 +17,21 @@ [build-system] requires = [ +"cmake < 4", "cython >= 3", # Starting with NumPy 1.25, NumPy is (by default) as far back compatible # as oldest-support-numpy was (customizable with a NPY_TARGET_VERSION # define). For older Python versions (where NumPy 1.25 is not yet available) # continue using oldest-support-numpy. "oldest-supported-numpy>=0.14; python_version<'3.9'", "numpy>=1.25; python_version>='3.9'", +"meson>=1.3.0", Review Comment: But that document talks about setting a minimum version in `meson.build` and not `pyproject.toml`. We don't use build isolation because our purpose is to use system packages that 1) have been verified with regards to supply chain security, 2) have been tested and marked stable (rather than using whichever newest version PyPI offers), 3) may include downstream patches if necessary at the point. Just to be clear, I'm not against requiring `>=1.3.0`. I'm against enforcing an artificial Python-level dependency on a package that is used as an external program. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2805534342 > I wonder if this change could make resolving #45576 easier I started down this path and ended up removing it in https://github.com/apache/arrow/pull/45854/commits/9f5df6e94a8eddcba82115f631935d40e15bd57e after some feedback and to keep the scope of this more refined. However, I don't think it would be that difficult to achieve with meson-python; generally the way meson forces you to handle dependencies as subprojects helps to streamline your build configuration -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2033303256
##
dev/release/02-source-test.rb:
##
@@ -84,7 +84,12 @@ def test_csharp_git_commit_information
def test_python_version
source
Dir.chdir("#{@tag_name_no_rc}/python") do
- sh("python3", "setup.py", "sdist")
+ # Meson dist must be run from a VCS, so initiate a dummy repo
+ sh("git", "init", ".")
+ sh("git", "add", "--all", ".")
+ sh("git", "commit", "-m", "dummy commit for meson dist")
+ sh("python3", "-m", "pip", "install", "build")
+ sh("python3", "-m", "build", "--sdist", ".")
Review Comment:
I added this to that script. I'm not sure if you also wanted me to remove it
from this ruby script so I've left it for now, but let me know if that is
mistaken
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2033794212 ## python/MANIFEST.in: ## @@ -1,15 +0,0 @@ -include README.md -include ../LICENSE.txt Review Comment: FWIW it seems like the current setuptools implementation is trying to include LICENSE.txt and NOTICE.txt within this manifest, but the source distribution contained on pypi contains neither. Not sure if that is a bug or not; for now I've just had the meson-python implementation here only match what is currently provided today (i.e. no LICENSE.txt or NOTICE.txt) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2033316983 ## ci/appveyor-cpp-build.bat: ## @@ -158,6 +192,6 @@ set PYARROW_TZDATA_PATH=%USERPROFILE%\Downloads\test\tzdata set AWS_EC2_METADATA_DISABLED=true set PYTHONDEVMODE=1 -python -m pytest -r sxX --durations=15 pyarrow/tests || exit /B - popd + +python -m pytest -r sxX --durations=15 --pyargs pyarrow || exit /B Review Comment: I've moved this to a pre-cursor in https://github.com/apache/arrow/pull/46059, since I don't think there is any downside to doing it now and its not relevant strictly to meson-python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021270630
##
ci/scripts/python_sdist_build.sh:
##
@@ -23,5 +23,5 @@ source_dir=${1}/python
pushd ${source_dir}
export SETUPTOOLS_SCM_PRETEND_VERSION=${PYARROW_VERSION:-}
-${PYTHON:-python} setup.py sdist
+${PYTHON:-python} -m build --sdist .
Review Comment:
This is the documented way to create a source distribution via meson-python.
See also https://mesonbuild.com/meson-python/how-to-guides/sdist.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022986892
##
python/meson.build:
##
@@ -0,0 +1,43 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+project(
+'pyarrow',
+'cython',
+'cpp',
+version: run_command(
+'python',
+'-m',
+'setuptools_scm',
Review Comment:
It is a standalone tool that can be configured and run via setuptools as
well. At least that's where it has been moving for quite some time now.
A bunch of build backends use it as a dependency for their "${backend}-vcs"
type plugins. Hatch, flit_scm, etc.
It could be used by non-python projects too, it just adds a dependency on a
python interpreter. :p
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021252825
##
ci/scripts/python_sdist_build.sh:
##
@@ -23,5 +23,5 @@ source_dir=${1}/python
pushd ${source_dir}
export SETUPTOOLS_SCM_PRETEND_VERSION=${PYARROW_VERSION:-}
-${PYTHON:-python} setup.py sdist
+${PYTHON:-python} -m build --sdist .
Review Comment:
We're not using Meson here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2769855936 Revision: 62f8a290d85985ef017db118d6384cc641d64d05 Submitted crossbow builds: [ursacomputing/crossbow @ actions-90eb21a915](https://github.com/ursacomputing/crossbow/branches/all?query=actions-90eb21a915) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14200339977/job/39785504068)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14200339680/job/39785501966)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340486/job/39785505320)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340560/job/39785506191)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14200341071/job/39785509335)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14200339661/job/39785501946)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340868/job/39785509311)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340473/job/39785505324)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340850/job/39785509345)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340082/job/39785504133)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340805/job/39785508857)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340845/job/39785508930)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14200339739/job/39785501964)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14200340153/job/39785504135)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14200339895/job/39785502959)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770567272 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2019090418
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,398 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
+if not numpy_dep.found()
+incdir_numpy = run_command(
+py,
+[
+'-c',
+'''
+import os
+import numpy as np
+try:
+# Check if include directory is inside the pyarrow dir
+# e.g. a venv created inside the pyarrow dir
+# If so, convert it to a relative path
+incdir = os.path.relpath(np.get_include())
+except Exception:
+incdir = np.get_include()
+print(incdir)
+''',
+],
+check: true,
+).stdout().strip()
+
+numpy_dep = declare_dependency(include_directories: incdir_numpy)
+endif
+
+cython_args = ['--include-dir', meson.current_source_dir()]
+if get_option('buildtype') in ['debug', 'debugoptimized']
+cython_args += ['--gdb']
+endif
+
+pyarrow_srcs = files(
+'src/arrow/python/arrow_to_pandas.cc',
+'src/arrow/python/benchmark.cc',
+'src/arrow/python/common.cc',
+'src/arrow/python/datetime.cc',
+'src/arrow/python/decimal.cc',
+'src/arrow/python/extension_type.cc',
+'src/arrow/python/gdb.cc',
+'src/arrow/python/helpers.cc',
+'src/arrow/python/inference.cc',
+'src/arrow/python/io.cc',
+'src/arrow/python/ipc.cc',
+'src/arrow/python/numpy_convert.cc',
+'src/arrow/python/numpy_init.cc',
+'src/arrow/python/numpy_to_arrow.cc',
+'src/arrow/python/pyarrow.cc',
+'src/arrow/python/python_test.cc',
+'src/arrow/python/python_to_arrow.cc',
+'src/arrow/python/udf.cc',
+)
+
+# TODO: these are optional components so should detect if needed
+# if needs_csv
+pyarrow_srcs += files('src/arrow/python/csv.cc')
+#endif
+# if needs_filesystem
+pyarrow_srcs += files('src/arrow/python/filesystem.cc')
+#endif
+
+subdir('src/arrow/python')
+
+arrow_python_lib = shared_library(
+'arrow_python',
+sources: pyarrow_srcs,
+include_directories: ['src'],
+dependencies: [arrow_dep, numpy_dep, cython_generated_dep,
py.dependency()],
+cpp_args: '-DARROW_PYTHON_EXPORTING',
+override_options: ['b_lundef=false'],
+install: true,
+install_dir: py.get_install_dir() / 'pyarrow',
+)
+
+cython_modules = {
+'lib': {},
+'_compute': {},
+'_csv': {},
+'_feather': {},
+'_fs': {},
+'_json': {},
+'_pyarrow_cpp_tests': {},
+}
+
+if get_option('azure').enabled()
+cython_modules += {'_azurefs': {}}
+endif
+
+if get_option('gcs').enabled()
+cython_modules += {'_gcsfs': {}}
+endif
+
+if get_option('s3').enabled()
+cython_modules += {'_s3fs': {}}
+endif
+
+if get_option('hdfs').enabled()
+cython_modules += {'_hdfs': {}}
+endif
+
+if get_option('cuda').enabled()
+cuda_dep = dependency(
+'arrow-cuda',
+'ArrowCUDA',
+modules: ['ArrowCUDA::arrow_cuda_shared'],
+)
+cython_modules += {'_cuda': {'dependencies': cuda_dep}}
+endif
+
+if get_option('acero').enabled()
+acero_dep = dependency(
+'arrow-acero',
+'ArrowAcero',
+modules: ['ArrowAcero::arrow_acero_shared'],
+)
+
+cython_modules += {'_acero': {'dependencies': acero_dep}}
+endif
+
+if get_option('dataset').enabled()
+dataset_dep = dependency(
+'arrow-dataset',
+'ArrowDataset',
+modules: ['ArrowDataset::arrow_dataset_shared'],
+)
+
+cython_modules += {'_dataset': {'dependencies': dataset_dep}}
+endif
+
+if get_option('parquet').enabled()
+parquet_dep = dependency(
+'parquet',
+'Parquet',
+modules: ['Parquet::parquet_shared'],
+)
+
+cython_modules += {'_parquet': {'dependencies': parquet_dep}}
+
+if get_option('parquet_encryption').enabled()
+arrow_encryption_lib = shared_library(
+'arrow_python_parquet_encryption',
+sources: ['src/arrow/python/parquet_encryption.cc'],
+include_directories: ['src'],
+link_with: [arrow_python_lib],
+
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2766537617 Revision: 14c4c2e053dd570ad370371ac2c3f6a1bf4a48ed Submitted crossbow builds: [ursacomputing/crossbow @ actions-2057340b5a](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2057340b5a) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825868/job/39707067868)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826389/job/39707071256)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825877/job/39707068178)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826933/job/39707073774)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826405/job/39707071520)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826191/job/39707070307)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825735/job/39707067664)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825999/job/39707069107)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825776/job/39707067715)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825375/job/39707066148)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826487/job/39707072060)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826144/job/39707070019)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14174826444/job/39707071569)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825685/job/39707067018)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14174825938/job/39707068621)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022918292
##
ci/docker/conda-python-pandas.dockerfile:
##
@@ -34,3 +34,5 @@ RUN mamba install -q -y --file arrow/ci/conda_env_sphinx.txt
&& \
COPY ci/scripts/install_pandas.sh /arrow/ci/scripts/
RUN mamba uninstall -q -y numpy && \
/arrow/ci/scripts/install_pandas.sh ${pandas} ${numpy}
+
+RUN apt-get update && apt-get install -y patchelf
Review Comment:
Thanks @raulcd . Just so I understand, is your concern that if we start
producing different pyarrow packages like pyarrow-core and pyarrow-all, and
they are both installed on a system with different versions, that an import may
load symbols from the wrong libraries at runtime?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
kou commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2771107586 > * C Glib & Ruby - appears to be an environment setup issue with CMake; likely unrelated Yes. It's unrelated: https://github.com/apache/arrow/issues/45994 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2738713615 @kou I have made some offline progress on this, but one of the things I am getting stuck on is how the pyarrow C++ modules are being compiled. From what I understand, the current build process will compile Cython modules first (at least lib.pyx) and from that auto-generate `lib.h` and `lib_api.h` headers that the pyarrow modules can then reference (?) Assuming that understanding is correct, where in the process are lib.h and lib_api.h being generated? I found the CMake command that copies them from the source to the build folder, but I can't figure out where they come from in the first place. Any guidance would be appreciated. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
raulcd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022856380
##
python/examples/minimal_build/Dockerfile.fedora:
##
@@ -27,6 +27,7 @@ RUN dnf update -y && \
make \
cmake \
ninja-build \
+pkg-config \
Review Comment:
why is this necessary?
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,426 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
Review Comment:
Currently numpy is a required build dependency but an optional runtime
dependency. This seems to point we can build without numpy, is that the case?
##
ci/appveyor-cpp-build.bat:
##
@@ -118,25 +118,60 @@ pushd python
@rem Build and install pyarrow
@rem
-set PYARROW_CMAKE_GENERATOR=%GENERATOR%
-set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS%
-set PYARROW_PARALLEL=2
-set PYARROW_WITH_ACERO=ON
-set PYARROW_WITH_DATASET=ON
-set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT%
-set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA%
-set PYARROW_WITH_GCS=%ARROW_GCS%
-set PYARROW_WITH_ORC=%ARROW_ORC%
-set PYARROW_WITH_PARQUET=ON
-set PYARROW_WITH_PARQUET_ENCRYPTION=ON
-set PYARROW_WITH_S3=%ARROW_S3%
-set PYARROW_WITH_SUBSTRAIT=ON
+set PYARROW_WITH_ACERO=enabled
+set PYARROW_WITH_DATASET=enabled
+
+if /i "%ARROW_BUILD_FLIGHT%" == "ON" (
+set PYARROW_WITH_FLIGHT=enabled
+) else (
+set PYARROW_WITH_FLIGHT=auto
+)
+
+if /i "%ARROW_BUILD_GANDIVA%" == "ON" (
+set PYARROW_WITH_GANDIVA=enabled
+) else (
+set PYARROW_WITH_GANDIVA=auto
+)
+
+if /i "%ARROW_BUILD_GCS%" == "ON" (
+set PYARROW_WITH_GCS=enabled
+) else (
+set PYARROW_WITH_GCS=auto
+)
+
+if /i "%ARROW_BUILD_ORC%" == "ON" (
+set PYARROW_WITH_ORC=enabled
+) else (
+set PYARROW_WITH_ORC=auto
+)
+
+set PYARROW_WITH_PARQUET=enabled
+set PYARROW_WITH_PARQUET_ENCRYPTION=enabled
+
+if /i "%ARROW_BUILD_S3%" == "ON" (
+set PYARROW_WITH_S3=enabled
+) else (
+set PYARROW_WITH_S3=auto
Review Comment:
shouldn't this be `disabled` instead of `auto`? Same for the others
##
ci/docker/conda-python-pandas.dockerfile:
##
@@ -34,3 +34,5 @@ RUN mamba install -q -y --file arrow/ci/conda_env_sphinx.txt
&& \
COPY ci/scripts/install_pandas.sh /arrow/ci/scripts/
RUN mamba uninstall -q -y numpy && \
/arrow/ci/scripts/install_pandas.sh ${pandas} ${numpy}
+
+RUN apt-get update && apt-get install -y patchelf
Review Comment:
would be good to understand this in detail, how are the symbols updated for
example, if what we want to do in the future is split the wheels into different
components, similar to what we did for conda
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2738732561 Ah nevermind I think I have figured it out. So it looks like Cython generates files in the build directory when compiling lib.pyx, so the idea is to copy those header files to a directory structure in the build directory that the sources can resolve to. I'll have to think about the best way to accomplish that via Meson. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2022893121 ## ci/appveyor-cpp-build.bat: ## @@ -118,25 +118,60 @@ pushd python @rem Build and install pyarrow @rem -set PYARROW_CMAKE_GENERATOR=%GENERATOR% -set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS% -set PYARROW_PARALLEL=2 -set PYARROW_WITH_ACERO=ON -set PYARROW_WITH_DATASET=ON -set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT% -set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA% -set PYARROW_WITH_GCS=%ARROW_GCS% -set PYARROW_WITH_ORC=%ARROW_ORC% -set PYARROW_WITH_PARQUET=ON -set PYARROW_WITH_PARQUET_ENCRYPTION=ON -set PYARROW_WITH_S3=%ARROW_S3% -set PYARROW_WITH_SUBSTRAIT=ON +set PYARROW_WITH_ACERO=enabled +set PYARROW_WITH_DATASET=enabled + +if /i "%ARROW_BUILD_FLIGHT%" == "ON" ( +set PYARROW_WITH_FLIGHT=enabled +) else ( +set PYARROW_WITH_FLIGHT=auto +) + +if /i "%ARROW_BUILD_GANDIVA%" == "ON" ( +set PYARROW_WITH_GANDIVA=enabled +) else ( +set PYARROW_WITH_GANDIVA=auto +) + +if /i "%ARROW_BUILD_GCS%" == "ON" ( +set PYARROW_WITH_GCS=enabled +) else ( +set PYARROW_WITH_GCS=auto +) + +if /i "%ARROW_BUILD_ORC%" == "ON" ( +set PYARROW_WITH_ORC=enabled +) else ( +set PYARROW_WITH_ORC=auto +) + +set PYARROW_WITH_PARQUET=enabled +set PYARROW_WITH_PARQUET_ENCRYPTION=enabled + +if /i "%ARROW_BUILD_S3%" == "ON" ( +set PYARROW_WITH_S3=enabled +) else ( +set PYARROW_WITH_S3=auto Review Comment: `auto` is disabled unless you set the `-Dauto_features=enabled` option with Meson; generally you get more control setting things to auto, but its not a huge deal either way here -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
kou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2024001410
##
dev/release/02-source-test.rb:
##
@@ -84,7 +84,12 @@ def test_csharp_git_commit_information
def test_python_version
source
Dir.chdir("#{@tag_name_no_rc}/python") do
- sh("python3", "setup.py", "sdist")
+ # Meson dist must be run from a VCS, so initiate a dummy repo
+ sh("git", "init", ".")
+ sh("git", "add", "--all", ".")
+ sh("git", "commit", "-m", "dummy commit for meson dist")
+ sh("python3", "-m", "pip", "install", "build")
+ sh("python3", "-m", "build", "--sdist", ".")
Review Comment:
Can we update
https://github.com/apache/arrow/blob/main/ci/scripts/python_sdist_build.sh and
use it here?
Our release process uses it:
https://github.com/apache/arrow/blob/main/dev/tasks/python-sdist/github.yml
Hmm... It doesn't use the source archive... We may not need this test...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2023065270
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,426 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
Review Comment:
See also https://github.com/mesonbuild/meson/pull/12891 (ping @rgommers)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770413824 Revision: 4129195903bba424017c0d17a78ad19ff782976c Submitted crossbow builds: [ursacomputing/crossbow @ actions-0371f4be52](https://github.com/ursacomputing/crossbow/branches/all?query=actions-0371f4be52) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779283/job/39796592934)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779360/job/39796593026)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779544/job/39796594864)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779571/job/39796595025)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779915/job/39796597269)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779652/job/39796596554)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779709/job/39796595806)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779583/job/39796595460)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779885/job/39796597236)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779564/job/39796595199)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779789/job/39796596504)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779593/job/39796595479)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779311/job/39796593112)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779721/job/39796595820)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14203779363/job/39796593330)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770572870 Revision: 391c563960177b5286fc482383d1aaf02db15e13 Submitted crossbow builds: [ursacomputing/crossbow @ actions-ef200897b1](https://github.com/ursacomputing/crossbow/branches/all?query=actions-ef200897b1) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061351/job/39800774312)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061627/job/39800777103)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061626/job/39800777066)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061405/job/39800774910)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14205060839/job/39800772165)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061995/job/39800778552)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061687/job/39800777464)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061548/job/39800775986)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061017/job/39800772824)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061560/job/39800775963)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061258/job/39800773825)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061648/job/39800777410)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061174/job/39800773701)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061473/job/39800775616)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14205061106/job/39800773471)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770499411 OK I think all of the major CI / crossbow issues are done. Right now there are 3 remaining CI jobs failing: - C Glib & Ruby - appears to be an environment setup issue with CMake; likely unrelated - test-conda-python-emcscripten - appears to be an environment setup issue with a chrome driver; likely unrelated - test-debian-12-python-3-i386: 2 test failures with floating point precision on 32 bit platform; possibly related I think any fixes to those would be very minor in nature. Seems like there is a more general question surrounding the usage of Meson while CMake is already in use by the C++ code base. I'll comment back on the original issue with findings from this -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770406383 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770341510 Revision: 604512a349907ab9684c7f3d27e58a7405ef8314 Submitted crossbow builds: [ursacomputing/crossbow @ actions-44a2fae50a](https://github.com/ursacomputing/crossbow/branches/all?query=actions-44a2fae50a) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214033/job/39794780547)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213859/job/39794779653)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214182/job/39794781534)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213632/job/39794778400)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214200/job/39794781940)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214243/job/39794781963)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214722/job/39794784026)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214088/job/39794780493)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213697/job/39794778997)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214593/job/39794784070)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213473/job/39794778054)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14203214052/job/39794780569)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213834/job/39794779667)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213203/job/39794777243)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14203213947/job/39794780497)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770335513 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770255219 Revision: 9151b55c84d7b154565ae71a75f757e445e36d06 Submitted crossbow builds: [ursacomputing/crossbow @ actions-ede5ef3990](https://github.com/ursacomputing/crossbow/branches/all?query=actions-ede5ef3990) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573699/job/39792716560)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14202574093/job/39792719789)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573845/job/39792718031)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14202574125/job/39792719849)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14202574087/job/39792719765)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14202574081/job/39792719438)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573748/job/39792716929)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573725/job/39792716697)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573489/job/39792715580)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14202574346/job/39792721363)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573744/job/39792717057)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573734/job/39792716777)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573969/job/39792719459)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14202574257/job/39792721299)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14202573869/job/39792718041)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2770248180 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2023206011 ## ci/appveyor-cpp-build.bat: ## @@ -158,6 +192,6 @@ set PYARROW_TZDATA_PATH=%USERPROFILE%\Downloads\test\tzdata set AWS_EC2_METADATA_DISABLED=true set PYTHONDEVMODE=1 -python -m pytest -r sxX --durations=15 pyarrow/tests || exit /B - popd + +python -m pytest -r sxX --durations=15 --pyargs pyarrow || exit /B Review Comment: The pytest invocation had to change to get out of the python directory and add `--pyargs pyarrow` in this batch script. I guess this worked with setuptools before because it built inplace, but with Meson building out of place staying in that directory and invoking pytest will cause import errors. FWIW all of the other scripts are already using this pattern; my assumption is that it was an oversight to have this invocation like this on main -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2023208490 ## ci/appveyor-cpp-build.bat: ## @@ -158,6 +192,6 @@ set PYARROW_TZDATA_PATH=%USERPROFILE%\Downloads\test\tzdata set AWS_EC2_METADATA_DISABLED=true set PYTHONDEVMODE=1 -python -m pytest -r sxX --durations=15 pyarrow/tests || exit /B - popd + +python -m pytest -r sxX --durations=15 --pyargs pyarrow || exit /B Review Comment: Yes, it's ok to do this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021353522
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
Ah OK thanks. I was not aware Arrow set its own flags. That is certainly
possible to replicate here - the `warning_level` is just a default but not a
hard requirement to use
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2769848127 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2023055525
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
Meson supports using buildtypes to define default optimization/debug flags,
yes.
It's pretty standard for flags specified in $CFLAGS to be considered "user
flags which should take priority over project default flags", so, exporting
those environment variables and having them contain flags you don't actually
want to be added to your build seems generally risky.
It does depend on whether project default flags are handled via the
buildtype (user flags override it) or via "mandatory project options" such as
`add_project_arguments('-DHAVE_CONFIG_H', language: 'cpp')`.
It's possible to set buildtype options multiple times by lumping it in with
mandatory options, but it's not very clean as it is a layering purity violation
and will probably annoy people that do things like build a couple hundred
packages with `export CFLAGS="-O0 -ggdb"` to debug an environment and don't
want to chase down individual per-project options for all hundred packages.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2023038341 ## python/meson.build: ## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +project( +'pyarrow', +'cython', +'cpp', +version: run_command( +'python', +'-m', +'setuptools_scm', +'--force-write-version-files', +check: true, +).stdout().strip(), +license: 'Apache-2.0', +meson_version: '>=1.4.0', +default_options: ['buildtype=release', 'warning_level=0', 'cpp_std=c++17'], Review Comment: You can of course add your own warning flags in meson, but meson provides a quick and easy way to set up some common values such as -Wall -Wextra -Wpedantic (and also clang's -Weverything, which is manually implemented in meson by encoding EVERY single warning option gcc knows about, and passing all of them. Because gcc devs did not want to add `-Weverything`, so meson users that wanted to do it in custom tests needed to manually implement it, and ended up adding it to meson's core.) Setting "0" here won't cause warnings to be suppressed. It simply means meson won't add its own -Wall. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2023030975
##
dev/release/02-source-test.rb:
##
@@ -84,7 +84,12 @@ def test_csharp_git_commit_information
def test_python_version
source
Dir.chdir("#{@tag_name_no_rc}/python") do
- sh("python3", "setup.py", "sdist")
+ # Meson dist must be run from a VCS, so initiate a dummy repo
Review Comment:
Downstream packagers, at least, will never build sdists. We (in my case,
Gentoo) only build wheels. We even wrote our own lightweight build frontend
since `build` has too many dependencies, and our build backend doesn't support
running PEP 517 `build_sdist` hooks at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2023023349
##
ci/docker/conda-python-pandas.dockerfile:
##
@@ -34,3 +34,5 @@ RUN mamba install -q -y --file arrow/ci/conda_env_sphinx.txt
&& \
COPY ci/scripts/install_pandas.sh /arrow/ci/scripts/
RUN mamba uninstall -q -y numpy && \
/arrow/ci/scripts/install_pandas.sh ${pandas} ${numpy}
+
+RUN apt-get update && apt-get install -y patchelf
Review Comment:
For pyprojects that have meson.build files which compile and install BOTH a
shared library, and a python extension that links to the shared library,
meson-python can internally package it in a wheel using the same tricks that
auditwheel uses to bundle up completely third-party shared libraries.
Namely, meson-python installs the shared library into a private directory in
site-packages (.xxx.mesonpy.libs/) and uses patchelf to modify the extension
module to search there for its shared library dependencies.
You shouldn't need patchelf if that relocation isn't being done. Users of
auditwheel are probably accustomed to installing patchelf anyway, though. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2023011646
##
ci/appveyor-cpp-build.bat:
##
@@ -118,25 +118,59 @@ pushd python
@rem Build and install pyarrow
@rem
-set PYARROW_CMAKE_GENERATOR=%GENERATOR%
-set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS%
-set PYARROW_PARALLEL=2
-set PYARROW_WITH_ACERO=ON
-set PYARROW_WITH_DATASET=ON
-set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT%
-set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA%
-set PYARROW_WITH_GCS=%ARROW_GCS%
-set PYARROW_WITH_ORC=%ARROW_ORC%
-set PYARROW_WITH_PARQUET=ON
-set PYARROW_WITH_PARQUET_ENCRYPTION=ON
-set PYARROW_WITH_S3=%ARROW_S3%
-set PYARROW_WITH_SUBSTRAIT=ON
+set PYARROW_WITH_ACERO=enabled
+set PYARROW_WITH_DATASET=enabled
+
+if /i "%ARROW_BUILD_FLIGHT%" == "ON" (
+set PYARROW_WITH_FLIGHT=enabled
+) else (
+set PYARROW_WITH_FLIGHT=auto
+)
+
+if /i "%ARROW_BUILD_GANDIVA%" == "ON" (
+set PYARROW_WITH_GANDIVA=enabled
+) else (
+set PYARROW_WITH_GANDIVA=auto
+)
+
+if /i "%ARROW_BUILD_GCS%" == "ON" (
+set PYARROW_WITH_GCS=enabled
+) else (
+set PYARROW_WITH_GCS=auto
+)
+
+if /i "%ARROW_BUILD_ORC%" == "ON" (
+set PYARROW_WITH_ORC=enabled
+) else (
+set PYARROW_WITH_ORC=auto
+)
+
+set PYARROW_WITH_PARQUET=enabled
+set PYARROW_WITH_PARQUET_ENCRYPTION=enabled
+
+if /i "%ARROW_BUILD_S3%" == "ON" (
+set PYARROW_WITH_S3=enabled
+) else (
+set PYARROW_WITH_S3=auto
+)
+
+set PYARROW_WITH_SUBSTRAIT=enabled
set ARROW_HOME=%CONDA_PREFIX%\Library
@rem ARROW-3075; pkgconfig is broken for Parquet for now
set PARQUET_HOME=%CONDA_PREFIX%\Library
-pip install --no-deps --no-build-isolation -vv --editable .
+pip install --no-deps --no-build-isolation -vv . ^
+-Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^
Review Comment:
You can stick all arguments in a toolchain file and then load the toolchain
file by calling `-Csetup-args=--native-file=$TOOLCHAIN_FILE`.
In general I don't think it's a great idea to allow environment variables to
override every possible build option as it becomes difficult to juggle and
pushes complexity into the build files to do option parsing and evaluate
precedence etc. what happens if you have:
```
export PYARROW_USE_FOO=true
meson setup builddir/ -D use_foo=false
```
Which value wins?
Meson does support environment variables, particularly, standard stuff like
CC, CFLAGS, LLVM_CONFIG, etc. Those will populate the environment object in
meson's interpreter core, since meson knows whether an option was specifically
defined across various data sources. But there's no
`get_option('use_foo').was_set_on_cli()` so I don't really see how you could
correctly implement an environment variable.
More interesting than a general "read environment variables" facility would
be a proposal to have meson support `$MESON_SETUP_ARGS` and have meson parse
them as a preface for `sys.argv` (so that says.argv overrides it when the two
different).
Alternatively perhaps load a toolchain file via an environment variable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022925677
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,426 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
Review Comment:
Ah I see. The comment directly preceding this in the code touches on this
but maybe needs clarification. When NumPy 2.0 becomes the minimum yes you can
make that check required and it will work as you expect, as that support is
built into Meson. However, since we still need to support NumPy < 2, this
injects some custom code to fall back to calling `np.get_include()` whenever
the system cannot detect NumPy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022925677
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,426 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
Review Comment:
Ah I see. The comment above touches on this but maybe needs clarification.
When NumPy 2.0 becomes the minimum yes you can make that check required and it
will work as you expect, as that support is built into Meson. However, since we
still need to support NumPy < 2, this injects some custom code to fall back to
calling `np.get_include()` whenever the system cannot detect NumPy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
raulcd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022916430
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,426 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
Review Comment:
yes, what I meant is that, shouldn't required be true?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2022905526
##
python/pyarrow/meson.build:
##
@@ -0,0 +1,426 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+py = import('python').find_installation(pure: false)
+
+# When NumPy 2.0 becomes the minimum we can remove the
+# custom location check
+numpy_dep = dependency('numpy', required: false)
Review Comment:
I haven't explored in detail, but a quick grep of the python code base shows
the following includes of NumPy:
```sh
./python/pyarrow/src/arrow/python/helpers.h:29:#include
./python/pyarrow/src/arrow/python/type_traits.h:29:#include
./python/pyarrow/src/arrow/python/numpy_interop.h:22:#include
// IWYU pragma: export
./python/pyarrow/src/arrow/python/numpy_interop.h:41:#include
// IWYU pragma: export
./python/pyarrow/src/arrow/python/numpy_interop.h:42:#include
// IWYU pragma: export
./python/pyarrow/src/arrow/python/numpy_interop.h:43:#include
// IWYU pragma: export
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2022900981 ## python/examples/minimal_build/Dockerfile.fedora: ## @@ -27,6 +27,7 @@ RUN dnf update -y && \ make \ cmake \ ninja-build \ +pkg-config \ Review Comment: This is currently needed to install headers from the Arrow project into the PyArrow package. In theory, if CMake exposed a variable for the `ARROW_INCLUDE_DIR` then Meson could wrap that, but AFAIU in the current state that variable is not exposed by the CMake configuration, and is instead just pulled from the `TARGET_INCLUDE_DIRECTORIES` property of the arrow target. See also the comment in pyarrow/meson.build:L405 . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021274909
##
dev/release/02-source-test.rb:
##
@@ -84,7 +84,12 @@ def test_csharp_git_commit_information
def test_python_version
source
Dir.chdir("#{@tag_name_no_rc}/python") do
- sh("python3", "setup.py", "sdist")
+ # Meson dist must be run from a VCS, so initiate a dummy repo
Review Comment:
Yes this looks like a requirement. See also our discussion in
https://github.com/apache/arrow/pull/45854/files#r2014837402
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021269060 ## python/meson.build: ## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +project( +'pyarrow', +'cython', +'cpp', +version: run_command( +'python', +'-m', +'setuptools_scm', +'--force-write-version-files', +check: true, +).stdout().strip(), +license: 'Apache-2.0', +meson_version: '>=1.4.0', +default_options: ['buildtype=release', 'warning_level=0', 'cpp_std=c++17'], Review Comment: `warning_level=0` just turns off warnings, which I did for now to reduce the CI verbosity. Long term this probably should be set to `warning_level=2`, which is akin to `-Wextra -Wall` with gcc/clang, and Meson will provide an equivalent set of flags for MSVC. See also https://mesonbuild.com/Builtin-options.html#details-for-warning_level -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021419042 ## ci/appveyor-cpp-build.bat: ## @@ -118,25 +118,59 @@ pushd python @rem Build and install pyarrow @rem -set PYARROW_CMAKE_GENERATOR=%GENERATOR% -set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS% -set PYARROW_PARALLEL=2 -set PYARROW_WITH_ACERO=ON -set PYARROW_WITH_DATASET=ON -set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT% -set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA% -set PYARROW_WITH_GCS=%ARROW_GCS% -set PYARROW_WITH_ORC=%ARROW_ORC% -set PYARROW_WITH_PARQUET=ON -set PYARROW_WITH_PARQUET_ENCRYPTION=ON -set PYARROW_WITH_S3=%ARROW_S3% -set PYARROW_WITH_SUBSTRAIT=ON +set PYARROW_WITH_ACERO=enabled +set PYARROW_WITH_DATASET=enabled + +if /i "%ARROW_BUILD_FLIGHT%" == "ON" ( +set PYARROW_WITH_FLIGHT=enabled +) else ( +set PYARROW_WITH_FLIGHT=auto +) + +if /i "%ARROW_BUILD_GANDIVA%" == "ON" ( +set PYARROW_WITH_GANDIVA=enabled +) else ( +set PYARROW_WITH_GANDIVA=auto +) + +if /i "%ARROW_BUILD_GCS%" == "ON" ( +set PYARROW_WITH_GCS=enabled +) else ( +set PYARROW_WITH_GCS=auto +) + +if /i "%ARROW_BUILD_ORC%" == "ON" ( +set PYARROW_WITH_ORC=enabled +) else ( +set PYARROW_WITH_ORC=auto +) + +set PYARROW_WITH_PARQUET=enabled +set PYARROW_WITH_PARQUET_ENCRYPTION=enabled + +if /i "%ARROW_BUILD_S3%" == "ON" ( +set PYARROW_WITH_S3=enabled +) else ( +set PYARROW_WITH_S3=auto +) + +set PYARROW_WITH_SUBSTRAIT=enabled set ARROW_HOME=%CONDA_PREFIX%\Library @rem ARROW-3075; pkgconfig is broken for Parquet for now set PARQUET_HOME=%CONDA_PREFIX%\Library -pip install --no-deps --no-build-isolation -vv --editable . +pip install --no-deps --no-build-isolation -vv . ^ +-Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^ Review Comment: > I wanted to avoid the larger diff for this initial PR. Well, if we ever want to switch to Meson, we have to do it completely, so this will have to be investigated. And ideally we should understand the relative strengths of Meson vs., say, scikit-build-dore. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021385763
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
As an FYI here is the upstream discussion around the conda compilers
activation changing C{XX}FLAGS and the subsequent issues it may cause
https://github.com/conda-forge/conda-forge.github.io/issues/2002
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021363161 ## ci/appveyor-cpp-build.bat: ## @@ -118,25 +118,59 @@ pushd python @rem Build and install pyarrow @rem -set PYARROW_CMAKE_GENERATOR=%GENERATOR% -set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS% -set PYARROW_PARALLEL=2 -set PYARROW_WITH_ACERO=ON -set PYARROW_WITH_DATASET=ON -set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT% -set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA% -set PYARROW_WITH_GCS=%ARROW_GCS% -set PYARROW_WITH_ORC=%ARROW_ORC% -set PYARROW_WITH_PARQUET=ON -set PYARROW_WITH_PARQUET_ENCRYPTION=ON -set PYARROW_WITH_S3=%ARROW_S3% -set PYARROW_WITH_SUBSTRAIT=ON +set PYARROW_WITH_ACERO=enabled +set PYARROW_WITH_DATASET=enabled + +if /i "%ARROW_BUILD_FLIGHT%" == "ON" ( +set PYARROW_WITH_FLIGHT=enabled +) else ( +set PYARROW_WITH_FLIGHT=auto +) + +if /i "%ARROW_BUILD_GANDIVA%" == "ON" ( +set PYARROW_WITH_GANDIVA=enabled +) else ( +set PYARROW_WITH_GANDIVA=auto +) + +if /i "%ARROW_BUILD_GCS%" == "ON" ( +set PYARROW_WITH_GCS=enabled +) else ( +set PYARROW_WITH_GCS=auto +) + +if /i "%ARROW_BUILD_ORC%" == "ON" ( +set PYARROW_WITH_ORC=enabled +) else ( +set PYARROW_WITH_ORC=auto +) + +set PYARROW_WITH_PARQUET=enabled +set PYARROW_WITH_PARQUET_ENCRYPTION=enabled + +if /i "%ARROW_BUILD_S3%" == "ON" ( +set PYARROW_WITH_S3=enabled +) else ( +set PYARROW_WITH_S3=auto +) + +set PYARROW_WITH_SUBSTRAIT=enabled set ARROW_HOME=%CONDA_PREFIX%\Library @rem ARROW-3075; pkgconfig is broken for Parquet for now set PARQUET_HOME=%CONDA_PREFIX%\Library -pip install --no-deps --no-build-isolation -vv --editable . +pip install --no-deps --no-build-isolation -vv . ^ +-Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^ Review Comment: I think Meson can handle this better through use of the `-Dauto_features=...` setting. I've essentially tried in this PR to follow the CMake pattern of specifying ON/OFF for each option, with minor tweaks to the syntax. However, with Meson, you can simplify that by opting in/out, depending on what is simpler. For example, if we had 5 options for acero, compute, csv, flight, and substrait, AFAIK with CMake you'd have to provide: ``` -DARROW_ACERO=ON -DARROW_COMPUTE=ON -DARROW_CSV=ON -DARROW_FLIGHT=ON -DARROW_SUBSTRAIT=ON ``` If you wanted to enable all of them. With Meson, you could more simply just write: ``` -Dauto_features=enabled ``` to opt into all of these. If you wanted all but flight, you would then do: ``` -Dauto_features=enabled -Dflight=disabled ``` So generally I think that would be a bit easier, but I wanted to avoid the larger diff for this initial PR. Happy to look further into refactoring towards that approach though if you agree it is easier and think it is a blocker -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021260358
##
ci/docker/conda-python-pandas.dockerfile:
##
@@ -34,3 +34,5 @@ RUN mamba install -q -y --file arrow/ci/conda_env_sphinx.txt
&& \
COPY ci/scripts/install_pandas.sh /arrow/ci/scripts/
RUN mamba uninstall -q -y numpy && \
/arrow/ci/scripts/install_pandas.sh ${pandas} ${numpy}
+
+RUN apt-get update && apt-get install -y patchelf
Review Comment:
When either meson or meson-python creates a wheel file the "install" process
will bundle the required libraries and change the linkage to using rpath
resolution. I believe similar to auditwheel, but it is handled entirely by
meson/meson-python.
@eli-schwartz can speak better to the inner workings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021343545 ## python/meson.build: ## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +project( +'pyarrow', +'cython', +'cpp', +version: run_command( +'python', +'-m', +'setuptools_scm', +'--force-write-version-files', +check: true, +).stdout().strip(), +license: 'Apache-2.0', +meson_version: '>=1.4.0', +default_options: ['buildtype=release', 'warning_level=0', 'cpp_std=c++17'], Review Comment: Ok, but as I said in another comment, we tweak our own warning options. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021340400
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
Well, using CMake two things would happen:
1. the CMake build type (Debug/Release/RelWithDebInfo) automatically enables
appropriate optimization and debug flags
2. additionally, we have [our
own](https://github.com/apache/arrow/blob/main/cpp/cmake_modules/SetupCxxFlags.cmake)
flags definitions for warnings, optimizations and whatnot
If the Meson setup doesn't reproduce this, then it's a problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021340400
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
Well, using CMake two things would happen:
1. the build type (Debug/Release/RelWithDebInfo) selects appropriates
optimization and debug info
2. additionally, we have [our
own](https://github.com/apache/arrow/blob/main/cpp/cmake_modules/SetupCxxFlags.cmake)
flags definitions for warnings, optimizations and whatnot
If the Meson setup doesn't reproduce this, then it's a problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021336703 ## ci/appveyor-cpp-build.bat: ## @@ -118,25 +118,59 @@ pushd python @rem Build and install pyarrow @rem -set PYARROW_CMAKE_GENERATOR=%GENERATOR% -set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS% -set PYARROW_PARALLEL=2 -set PYARROW_WITH_ACERO=ON -set PYARROW_WITH_DATASET=ON -set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT% -set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA% -set PYARROW_WITH_GCS=%ARROW_GCS% -set PYARROW_WITH_ORC=%ARROW_ORC% -set PYARROW_WITH_PARQUET=ON -set PYARROW_WITH_PARQUET_ENCRYPTION=ON -set PYARROW_WITH_S3=%ARROW_S3% -set PYARROW_WITH_SUBSTRAIT=ON +set PYARROW_WITH_ACERO=enabled +set PYARROW_WITH_DATASET=enabled + +if /i "%ARROW_BUILD_FLIGHT%" == "ON" ( +set PYARROW_WITH_FLIGHT=enabled +) else ( +set PYARROW_WITH_FLIGHT=auto +) + +if /i "%ARROW_BUILD_GANDIVA%" == "ON" ( +set PYARROW_WITH_GANDIVA=enabled +) else ( +set PYARROW_WITH_GANDIVA=auto +) + +if /i "%ARROW_BUILD_GCS%" == "ON" ( +set PYARROW_WITH_GCS=enabled +) else ( +set PYARROW_WITH_GCS=auto +) + +if /i "%ARROW_BUILD_ORC%" == "ON" ( +set PYARROW_WITH_ORC=enabled +) else ( +set PYARROW_WITH_ORC=auto +) + +set PYARROW_WITH_PARQUET=enabled +set PYARROW_WITH_PARQUET_ENCRYPTION=enabled + +if /i "%ARROW_BUILD_S3%" == "ON" ( +set PYARROW_WITH_S3=enabled +) else ( +set PYARROW_WITH_S3=auto +) + +set PYARROW_WITH_SUBSTRAIT=enabled set ARROW_HOME=%CONDA_PREFIX%\Library @rem ARROW-3075; pkgconfig is broken for Parquet for now set PARQUET_HOME=%CONDA_PREFIX%\Library -pip install --no-deps --no-build-isolation -vv --editable . +pip install --no-deps --no-build-isolation -vv . ^ +-Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^ Review Comment: Well, Meson sounds annoyingly opinionated. I think we would want two things: 1) if at all possible, support the environment variables, at least temporarily, even if we end up deprecating them 2) provide an alternative idiom that feels better than the awkward `-Csetup_args` hackery -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021252158
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
Hmm, I'm not sure what this means. We should be using the conda compilers
for building (if available), so the flags shouldn't interfere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2766472275 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021254355 ## python/meson.build: ## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +project( +'pyarrow', +'cython', +'cpp', +version: run_command( +'python', +'-m', +'setuptools_scm', Review Comment: Surprisingly no - looks like setuptools_scm does not require setuptools -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021250143
##
ci/docker/conda-python-pandas.dockerfile:
##
@@ -34,3 +34,5 @@ RUN mamba install -q -y --file arrow/ci/conda_env_sphinx.txt
&& \
COPY ci/scripts/install_pandas.sh /arrow/ci/scripts/
RUN mamba uninstall -q -y numpy && \
/arrow/ci/scripts/install_pandas.sh ${pandas} ${numpy}
+
+RUN apt-get update && apt-get install -y patchelf
Review Comment:
I'm curious, why is this needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021264915
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
I am actually surprised that this doesn't affect the current CMake
configuration, but it may just be a matter of pure luck. The conda compilers
package will add flags like "-O2" to the CFLAGS / CXXFLAGS environment
variables, so if the build system doesn't follow that up with a "-O0" then it
is possible for some of the debugging symbols to be optimized out.
In the case of Meson, it appears that the CFLAGS environment variable was
sequenced after any internal settings, so even if you set the mode to debug in
Meson you'd end up with a g++ command like `g++ -O0 -O2 ...`, which causes
failures in the test_gdb.py suite
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021257067 ## ci/appveyor-cpp-build.bat: ## @@ -118,25 +118,59 @@ pushd python @rem Build and install pyarrow @rem -set PYARROW_CMAKE_GENERATOR=%GENERATOR% -set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS% -set PYARROW_PARALLEL=2 -set PYARROW_WITH_ACERO=ON -set PYARROW_WITH_DATASET=ON -set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT% -set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA% -set PYARROW_WITH_GCS=%ARROW_GCS% -set PYARROW_WITH_ORC=%ARROW_ORC% -set PYARROW_WITH_PARQUET=ON -set PYARROW_WITH_PARQUET_ENCRYPTION=ON -set PYARROW_WITH_S3=%ARROW_S3% -set PYARROW_WITH_SUBSTRAIT=ON +set PYARROW_WITH_ACERO=enabled +set PYARROW_WITH_DATASET=enabled + +if /i "%ARROW_BUILD_FLIGHT%" == "ON" ( +set PYARROW_WITH_FLIGHT=enabled +) else ( +set PYARROW_WITH_FLIGHT=auto +) + +if /i "%ARROW_BUILD_GANDIVA%" == "ON" ( +set PYARROW_WITH_GANDIVA=enabled +) else ( +set PYARROW_WITH_GANDIVA=auto +) + +if /i "%ARROW_BUILD_GCS%" == "ON" ( +set PYARROW_WITH_GCS=enabled +) else ( +set PYARROW_WITH_GCS=auto +) + +if /i "%ARROW_BUILD_ORC%" == "ON" ( +set PYARROW_WITH_ORC=enabled +) else ( +set PYARROW_WITH_ORC=auto +) + +set PYARROW_WITH_PARQUET=enabled +set PYARROW_WITH_PARQUET_ENCRYPTION=enabled + +if /i "%ARROW_BUILD_S3%" == "ON" ( +set PYARROW_WITH_S3=enabled +) else ( +set PYARROW_WITH_S3=auto +) + +set PYARROW_WITH_SUBSTRAIT=enabled set ARROW_HOME=%CONDA_PREFIX%\Library @rem ARROW-3075; pkgconfig is broken for Parquet for now set PARQUET_HOME=%CONDA_PREFIX%\Library -pip install --no-deps --no-build-isolation -vv --editable . +pip install --no-deps --no-build-isolation -vv . ^ +-Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^ Review Comment: Meson strongly discourages using environment variables to change the configuration. See also: https://github.com/mesonbuild/meson/discussions/13811 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021257791 ## python/meson.build: ## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +project( +'pyarrow', +'cython', +'cpp', +version: run_command( +'python', +'-m', +'setuptools_scm', +'--force-write-version-files', +check: true, +).stdout().strip(), +license: 'Apache-2.0', +meson_version: '>=1.4.0', +default_options: ['buildtype=release', 'warning_level=0', 'cpp_std=c++17'], Review Comment: I'm not sure what `warning_level=0` means but ideally we would reuse the warnings settings from Arrow C++ (which is what our CMake setup currently does, IIRC). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021253688
##
dev/release/02-source-test.rb:
##
@@ -84,7 +84,12 @@ def test_csharp_git_commit_information
def test_python_version
source
Dir.chdir("#{@tag_name_no_rc}/python") do
- sh("python3", "setup.py", "sdist")
+ # Meson dist must be run from a VCS, so initiate a dummy repo
Review Comment:
Er, really? This seems a bit clumsy (and may annoy downstream packagers).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021264915
##
ci/scripts/python_build.sh:
##
@@ -89,7 +136,37 @@ pushd ${python_build_dir}
# on Debian/Ubuntu (ARROW-15243).
# - Cannot use build isolation as we want to use specific dependency versions
# (e.g. Numpy, Pandas) on some CI jobs.
-${PYTHON:-python} -m pip install --no-deps --no-build-isolation -vv .
+
+# The conda compilers package may mess with C{XX}_FLAGS in a way that
interferes
+# with the compiler
Review Comment:
I am actually surprised that this doesn't affect the current CMake
configuration, but it may just be a matter of pure luck. The conda compilers
package will add flags like "-O2" to the CFLAGS / CXXFLAGS environment
variables, so if the build system doesn't follow that up with a "-O0" then it
is possible for some of the debugging symbols to be optimized out.
In the case of Meson, it appears that the CFLAGS environment variable was
sequenced after any internal settings, so even if you set the mode to debug in
Meson you'd end up with a g++ command like `g++ -O0 -O2`, which would optimize
out some variables and cause failures in the test_gdb.py suite
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021260358
##
ci/docker/conda-python-pandas.dockerfile:
##
@@ -34,3 +34,5 @@ RUN mamba install -q -y --file arrow/ci/conda_env_sphinx.txt
&& \
COPY ci/scripts/install_pandas.sh /arrow/ci/scripts/
RUN mamba uninstall -q -y numpy && \
/arrow/ci/scripts/install_pandas.sh ${pandas} ${numpy}
+
+RUN apt-get update && apt-get install -y patchelf
Review Comment:
When either meson or meson-python creates a wheel file the "installation"
process will bundle the required libraries and change the linkage to using
rpath resolution. I believe similar to auditwheel, but it is handled entirely
by meson/meson-python.
@eli-schwartz can speak better to the inner workings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021253688
##
dev/release/02-source-test.rb:
##
@@ -84,7 +84,12 @@ def test_csharp_git_commit_information
def test_python_version
source
Dir.chdir("#{@tag_name_no_rc}/python") do
- sh("python3", "setup.py", "sdist")
+ # Meson dist must be run from a VCS, so initiate a dummy repo
Review Comment:
Er, really? This seems a bit clumsy (and might annoy downstream packagers).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021248403 ## python/meson.build: ## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +project( +'pyarrow', +'cython', +'cpp', +version: run_command( +'python', +'-m', +'setuptools_scm', Review Comment: It seems unexpected to depend on `setuptools_scm` for this. Does it required a `setup.py`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
pitrou commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2021249211 ## ci/appveyor-cpp-build.bat: ## @@ -118,25 +118,59 @@ pushd python @rem Build and install pyarrow @rem -set PYARROW_CMAKE_GENERATOR=%GENERATOR% -set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS% -set PYARROW_PARALLEL=2 -set PYARROW_WITH_ACERO=ON -set PYARROW_WITH_DATASET=ON -set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT% -set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA% -set PYARROW_WITH_GCS=%ARROW_GCS% -set PYARROW_WITH_ORC=%ARROW_ORC% -set PYARROW_WITH_PARQUET=ON -set PYARROW_WITH_PARQUET_ENCRYPTION=ON -set PYARROW_WITH_S3=%ARROW_S3% -set PYARROW_WITH_SUBSTRAIT=ON +set PYARROW_WITH_ACERO=enabled +set PYARROW_WITH_DATASET=enabled + +if /i "%ARROW_BUILD_FLIGHT%" == "ON" ( +set PYARROW_WITH_FLIGHT=enabled +) else ( +set PYARROW_WITH_FLIGHT=auto +) + +if /i "%ARROW_BUILD_GANDIVA%" == "ON" ( +set PYARROW_WITH_GANDIVA=enabled +) else ( +set PYARROW_WITH_GANDIVA=auto +) + +if /i "%ARROW_BUILD_GCS%" == "ON" ( +set PYARROW_WITH_GCS=enabled +) else ( +set PYARROW_WITH_GCS=auto +) + +if /i "%ARROW_BUILD_ORC%" == "ON" ( +set PYARROW_WITH_ORC=enabled +) else ( +set PYARROW_WITH_ORC=auto +) + +set PYARROW_WITH_PARQUET=enabled +set PYARROW_WITH_PARQUET_ENCRYPTION=enabled + +if /i "%ARROW_BUILD_S3%" == "ON" ( +set PYARROW_WITH_S3=enabled +) else ( +set PYARROW_WITH_S3=auto +) + +set PYARROW_WITH_SUBSTRAIT=enabled set ARROW_HOME=%CONDA_PREFIX%\Library @rem ARROW-3075; pkgconfig is broken for Parquet for now set PARQUET_HOME=%CONDA_PREFIX%\Library -pip install --no-deps --no-build-isolation -vv --editable . +pip install --no-deps --no-build-isolation -vv . ^ +-Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^ Review Comment: This is cumbersome. Can't we read environment variables from a Meson config file? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2766528842 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2766480274 Revision: 8372b0e91d2d8f7eb6c4d449d369d64fe2eccd30 Submitted crossbow builds: [ursacomputing/crossbow @ actions-2e5da2b119](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2e5da2b119) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415687/job/39705694965)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415444/job/39705692400)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415774/job/39705695422)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14174414932/job/39705690584)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415900/job/39705696150)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415129/job/39705690775)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415166/job/39705691243)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415389/job/39705692556)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14174416080/job/39705696729)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415142/job/39705691066)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415852/job/39705695573)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415642/job/39705694284)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415947/job/39705696784)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415463/job/39705693358)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14174415666/job/39705694514)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2762363061 Revision: 46b10603d03d6de2d3b9aa62b59183bdacd1710a Submitted crossbow builds: [ursacomputing/crossbow @ actions-fe293149f6](https://github.com/ursacomputing/crossbow/branches/all?query=actions-fe293149f6) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173557/job/39611575278)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174268/job/39611579752)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173927/job/39611577066)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174184/job/39611578334)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174174/job/39611578342)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173634/job/39611575272)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174379/job/39611579617)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174188/job/39611579345)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173877/job/39611577003)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174290/job/39611578871)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173925/job/39611576564)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173822/job/39611576597)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174035/job/39611576705)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14137174087/job/39611577478)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14137173686/job/39611575584)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2762358067 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
github-actions[bot] commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2761790656 Revision: cb925ac437a89c1857eb6b06ca498d22d8b6aa8b Submitted crossbow builds: [ursacomputing/crossbow @ actions-e5a82684b6](https://github.com/ursacomputing/crossbow/branches/all?query=actions-e5a82684b6) |Task|Status| ||--| |example-python-minimal-build-fedora-conda|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066387/job/39598051561)| |example-python-minimal-build-ubuntu-venv|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067242/job/39598057725)| |test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066999/job/39598054755)| |test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067749/job/39598060999)| |test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066534/job/39598051637)| |test-conda-python-3.10-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066699/job/39598052598)| |test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066932/job/39598053864)| |test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067388/job/39598058196)| |test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067416/job/39598058194)| |test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066703/job/39598052672)| |test-conda-python-3.11-pandas-latest-numpy-1.26|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067060/job/3959808)| |test-conda-python-3.11-pandas-latest-numpy-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067542/job/39598058296)| |test-conda-python-3.11-pandas-nightly-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14133066905/job/39598053729)| |test-conda-python-3.11-pandas-upstream_devel-numpy-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067140/job/39598056746)| |test-conda-python-3.11-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/14133067061/job/39598055522)| |test-cond
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2761781630 @github-actions crossbow submit -g python -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2018833185 ## .github/workflows/dev.yml: ## @@ -124,7 +124,7 @@ jobs: shell: bash run: | gem install test-unit - pip install "cython>=3" setuptools pytest requests setuptools-scm + pip install "cython>=3" meson-python "cmake<4" pytest requests meson-python setuptools-scm Review Comment: It looks like cmake=4.0.0 was just released today and is causing some issues with mimalloc and boost resolution. Hope that this upper bound won't have to stay long ## python/requirements-test.txt: ## @@ -4,3 +4,4 @@ pandas pytest pytz pyuwsgi; sys.platform != 'win32' and python_version < '3.13' +setuptools>=64 Review Comment: While setuptools isn't required to build anymore, there are a few tests that create inline setup.py files and use them to test the building of third party libraries. It might be nice to port those off of setuptools, but I don't think that is critical for the initial PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2761521425 Question above can be ignored - I think solved by fetching the include directory variable from the Arrow dependency and installing that -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on PR #45854: URL: https://github.com/apache/arrow/pull/45854#issuecomment-2759126610 Hopefully the last major hurdle here is dealing with the installation of the Arrow dependency. My interpretation of the status quo is that PyArrow will always bundle a copy of the Arrow headers, and potentially bundle the Arrow libraries if `PYARROW_BUNDLE_ARROW_CPP` is set. I'm assuming if the latter is not set, then it will be left to a third party tool like auditwheel to repack the wheel with the required libraries (?) I'm not sure how to solve the former issue. Is there a way that Meson can install the headers from a dependency into a Python package? Is it really even needed by PyArrow if none of the libraries are bundled? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2017082834 ## python/pyarrow/src/arrow/python/meson.build: ## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cython_generated_headers = custom_target( +'lib-pyx-headers', +input: ['../../../lib.pyx'], +output: ['lib.h', 'lib_api.h'], +command: [ +'cython', +'--cplus', +'@INPUT@', +'-o', +meson.current_build_dir() / 'lib.cc', Review Comment: I suppose an issue in including the `lib.cc` file is that we also need to install these headers to a particular location; leaving it out of the output makes that simpler I think -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2016996338 ## python/pyarrow/src/arrow/python/meson.build: ## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cython_generated_headers = custom_target( +'lib-pyx-headers', +input: ['../../../lib.pyx'], +output: ['lib.h', 'lib_api.h'], +command: [ +'cython', +'--cplus', +'@INPUT@', +'-o', +meson.current_build_dir() / 'lib.cc', Review Comment: Just the headers -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2016994667 ## python/pyarrow/src/arrow/python/meson.build: ## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cython_generated_headers = custom_target( +'lib-pyx-headers', +input: ['../../../lib.pyx'], +output: ['lib.h', 'lib_api.h'], +command: [ +'cython', +'--cplus', +'@INPUT@', +'-o', +meson.current_build_dir() / 'lib.cc', Review Comment: What I mean is, do you not need lib.cc to be one of the source that get compiled? Do you *just* need the headers? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2016981481 ## python/pyarrow/src/arrow/python/meson.build: ## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cython_generated_headers = custom_target( +'lib-pyx-headers', +input: ['../../../lib.pyx'], +output: ['lib.h', 'lib_api.h'], +command: [ +'cython', +'--cplus', +'@INPUT@', +'-o', +meson.current_build_dir() / 'lib.cc', Review Comment: Cool thanks. Its a bit strange currently, but the `lib.cc` module isn't used by any other target for linkage (I assume Python just dlopen's it at runtime as needed). The dependency target is used by the `arrow_python_lib` target currently, which needs the headers to be generated before it can compile -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] GH-36411: [C++][Python] Use meson-python for PyArrow build system [arrow]
eli-schwartz commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2016974556 ## python/pyarrow/src/arrow/python/meson.build: ## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cython_generated_headers = custom_target( +'lib-pyx-headers', +input: ['../../../lib.pyx'], +output: ['lib.h', 'lib_api.h'], +command: [ +'cython', +'--cplus', +'@INPUT@', +'-o', +meson.current_build_dir() / 'lib.cc', Review Comment: ```suggestion '-o', '@OUTPUT0@', ``` and `output: ['lib.cc', 'lib.h', 'lib_api.h']` You can then pass e.g. `[cython_generated_headers[1], cython_generated_headers[2]]` as the generated dep (where is `cython_generated_headers[0]`, i.e. lib.cc, used?) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
