WillAyd commented on code in PR #45854: URL: https://github.com/apache/arrow/pull/45854#discussion_r2014841169
########## python/meson.build: ########## @@ -0,0 +1,101 @@ +# 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( + 'python3', + '-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'], +) + +arrow_dep = dependency( + 'arrow', + 'Arrow', + modules: ['Arrow::arrow_shared'], + required: false, # optional so sdist will work without finding Review Comment: The Arrow dependency is required to build the project, but I don't think the sdist requires it. Unless there's a trick in meson-python I am unaware of, I don't know that there is a way to allow the sdist to forgo the `required` requirement @eli-schwartz ########## 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") + FileUtils.mv("dummy.git", "../.git") + sh("python3", "-m", "pip", "install", "build") Review Comment: Without being overly familiar with this process, I am not sure how to really solve this. Meson's `dist` command requires you to run it from a VCS. From what I can tell, this release script is downloading the source files from an archive, unpacking them, and then trying to create a source distribution out of that. Not sure what the best compromise on this would be @kou @eli-schwartz ########## python/meson.build: ########## @@ -0,0 +1,101 @@ +# 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( + 'python3', + '-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'], +) + +arrow_dep = dependency( + 'arrow', + 'Arrow', + modules: ['Arrow::arrow_shared'], + required: false, # optional so sdist will work without finding +) + +# TODO: uncomment and fix below implementation to allow the Python subproject Review Comment: For the initial implementation it probably makes the most sense to remove this commented block of code and only focus on working with a system-installed Arrow; getting it to build as a subproject can always be done in a later PR ########## ci/scripts/python_build.sh: ########## @@ -55,24 +55,71 @@ if [ ! -z "${CONDA_PREFIX}" ]; then conda list fi -export PYARROW_CMAKE_GENERATOR=${CMAKE_GENERATOR:-Ninja} -export PYARROW_BUILD_TYPE=${CMAKE_BUILD_TYPE:-debug} - -export PYARROW_WITH_ACERO=${ARROW_ACERO:-OFF} -export PYARROW_WITH_AZURE=${ARROW_AZURE:-OFF} -export PYARROW_WITH_CUDA=${ARROW_CUDA:-OFF} -export PYARROW_WITH_DATASET=${ARROW_DATASET:-ON} -export PYARROW_WITH_FLIGHT=${ARROW_FLIGHT:-OFF} -export PYARROW_WITH_GANDIVA=${ARROW_GANDIVA:-OFF} -export PYARROW_WITH_GCS=${ARROW_GCS:-OFF} -export PYARROW_WITH_HDFS=${ARROW_HDFS:-ON} -export PYARROW_WITH_ORC=${ARROW_ORC:-OFF} -export PYARROW_WITH_PARQUET=${ARROW_PARQUET:-OFF} -export PYARROW_WITH_PARQUET_ENCRYPTION=${PARQUET_REQUIRE_ENCRYPTION:-ON} -export PYARROW_WITH_S3=${ARROW_S3:-OFF} -export PYARROW_WITH_SUBSTRAIT=${ARROW_SUBSTRAIT:-OFF} - -export PYARROW_PARALLEL=${n_jobs} +PYARROW_WITH_ACERO=$(case "$ARROW_ACERO" in Review Comment: I've kept the process here of using environment variables but ported them over to the Meson syntax. However, it is worth noting that Meson also has a much easier way of handling this, where you can set an option of `-Dauto_features=on` to enable optional features and then opt out of the ones you don't want. -- 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