Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-12619: [Python] pyarrow sdist should not require git #10342

Closed
wants to merge 3 commits into from

Conversation

kszucs
Copy link
Member

@kszucs kszucs commented May 17, 2021

There is a fallback_version configuration option for setuptools_scm which we don't use: https://github.com/pypa/setuptools_scm#configuration-parameters

Although this setting seems to have issues according to pypa/setuptools_scm#549
We already have a workaround in setup.py for the functionality of the fallback_version option, but it is disabled for the case of sdist: https://github.com/apache/arrow/blob/master/python/setup.py#L529

@github-actions
Copy link

@kszucs
Copy link
Member Author

kszucs commented May 17, 2021

@github-actions crossbow submit python-sdist wheel-*

Still need to verify this locally.

@github-actions
Copy link

Revision: 4964100

Submitted crossbow builds: ursacomputing/crossbow @ actions-414

Task Status
python-sdist Github Actions
wheel-manylinux2010-cp36-amd64 Github Actions
wheel-manylinux2010-cp37-amd64 Github Actions
wheel-manylinux2010-cp38-amd64 Github Actions
wheel-manylinux2010-cp39-amd64 Github Actions
wheel-manylinux2014-cp36-amd64 Github Actions
wheel-manylinux2014-cp36-arm64 TravisCI
wheel-manylinux2014-cp37-amd64 Github Actions
wheel-manylinux2014-cp37-arm64 TravisCI
wheel-manylinux2014-cp38-amd64 Github Actions
wheel-manylinux2014-cp38-arm64 TravisCI
wheel-manylinux2014-cp39-amd64 Github Actions
wheel-manylinux2014-cp39-arm64 TravisCI
wheel-osx-high-sierra-cp36 Github Actions
wheel-osx-high-sierra-cp37 Github Actions
wheel-osx-high-sierra-cp38 Github Actions
wheel-osx-high-sierra-cp39 Github Actions
wheel-osx-mavericks-cp36 Github Actions
wheel-osx-mavericks-cp37 Github Actions
wheel-osx-mavericks-cp38 Github Actions
wheel-osx-mavericks-cp39 Github Actions
wheel-windows-cp36 Github Actions
wheel-windows-cp37 Github Actions
wheel-windows-cp38 Github Actions
wheel-windows-cp39 Github Actions

@kszucs kszucs requested a review from pitrou May 17, 2021 12:37
@kszucs
Copy link
Member Author

kszucs commented May 17, 2021

I'm updating the sdist build to check the tarball without git installed.

@kszucs
Copy link
Member Author

kszucs commented May 17, 2021

@github-actions crossbow submit python-sdist

This should test the produced source tarball in a git-less environment.

@github-actions
Copy link

Revision: c423b96

Submitted crossbow builds: ursacomputing/crossbow @ actions-416

Task Status
python-sdist Github Actions

@kszucs kszucs requested review from kou and lidavidm May 17, 2021 17:51
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

LGTM!

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)
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly it didn't pick up the tarball with the quotes, which I wouldn't have expected either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Creating arrow_ubuntu-python-sdist-test_run ... done
root@202aa2bdd8d0:/# ls "/arrow/python/dist/pyarrow-*.tar.gz"
ls: cannot access '/arrow/python/dist/pyarrow-*.tar.gz': No such file or directory
root@202aa2bdd8d0:/# ls /arrow/python/dist/pyarrow-*.tar.gz
/arrow/python/dist/pyarrow-5.0.0.dev866+g49641009e.d20210517.tar.gz
root@202aa2bdd8d0:/#

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

$arrow_dir is going to be /arrow :)

@kszucs
Copy link
Member Author

kszucs commented May 17, 2021

The macOS wheel builds are failing due to network timeout.

@kszucs kszucs closed this in 8688141 May 17, 2021
kszucs added a commit that referenced this pull request May 17, 2021
There is a fallback_version configuration option for setuptools_scm which we don't use: https://github.com/pypa/setuptools_scm#configuration-parameters

Although this setting seems to have issues according to pypa/setuptools_scm#549
We already have a workaround in setup.py for the functionality of the fallback_version option, but it is disabled for the case of sdist: https://github.com/apache/arrow/blob/master/python/setup.py#L529

Closes #10342 from kszucs/ARROW-12619

Authored-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 13, 2021
There is a fallback_version configuration option for setuptools_scm which we don't use: https://github.com/pypa/setuptools_scm#configuration-parameters

Although this setting seems to have issues according to pypa/setuptools_scm#549
We already have a workaround in setup.py for the functionality of the fallback_version option, but it is disabled for the case of sdist: https://github.com/apache/arrow/blob/master/python/setup.py#L529

Closes apache#10342 from kszucs/ARROW-12619

Authored-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants