[GitHub] [arrow] lidavidm commented on a change in pull request #10342: ARROW-12619: [Python] pyarrow sdist should not require git

[GitBox <gi...@apache.org>]

lidavidm commented on a change in pull request #10342:
URL: https://github.com/apache/arrow/pull/10342#discussion_r633744314



##########
File path: ci/scripts/python_sdist_test.sh
##########
@@ -42,10 +42,16 @@ export PYARROW_WITH_DATASET=${ARROW_DATASET:-OFF}
 # unset ARROW_HOME
 # apt purge -y pkg-config
 
+# ARROW-12619
+if command -v git &> /dev/null; then
+  echo "Git exists, remove it from PATH before executing this script."
+  exit 1
+fi
+
 if [ -n "${PYARROW_VERSION:-}" ]; then
   sdist="${arrow_dir}/python/dist/pyarrow-${PYARROW_VERSION}.tar.gz"
 else
-  sdist=$(ls "${arrow_dir}/python/dist/pyarrow-*.tar.gz" | sort -r | head -n1)
+  sdist=$(ls ${arrow_dir}/python/dist/pyarrow-*.tar.gz | sort -r | head -n1)

Review comment:
       very minor nit: you could keep the quoting around just `${arrow_dir}`

##########
File path: ci/scripts/python_sdist_test.sh
##########
@@ -42,10 +42,16 @@ export PYARROW_WITH_DATASET=${ARROW_DATASET:-OFF}
 # unset ARROW_HOME
 # apt purge -y pkg-config
 
+# ARROW-12619
+if command -v git &> /dev/null; then
+  echo "Git exists, remove it from PATH before executing this script."
+  exit 1
+fi
+
 if [ -n "${PYARROW_VERSION:-}" ]; then
   sdist="${arrow_dir}/python/dist/pyarrow-${PYARROW_VERSION}.tar.gz"
 else
-  sdist=$(ls "${arrow_dir}/python/dist/pyarrow-*.tar.gz" | sort -r | head -n1)
+  sdist=$(ls ${arrow_dir}/python/dist/pyarrow-*.tar.gz | sort -r | head -n1)

Review comment:
       That is expected, but `ls "${arrow_dir}"/python/dist/pyarrow-*.tar.gz` doesn't work? e.g.
   
   ```
   $ export arrow_dir=/tmp/foo
   $ touch /tmp/foo/pyarrow-5.0.0.tar.gz
   $ ls "${arrow_dir}"/pyarrow-*.tar.gz
   /tmp/foo/pyarrow-5.0.0.tar.gz
   ```
   
   but either way this is a very minor nit, just if you wanted to be extra-defensive.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org