You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/08/09 08:40:53 UTC
[GitHub] [beam] AnandInguva opened a new pull request, #22635: Add stdlib distutils env variable while building the wheels
AnandInguva opened a new pull request, #22635:
URL: https://github.com/apache/beam/pull/22635
`cibuildwheel==1.11.0` uses `get-pip.py`. When using this with the setuptools >= 60.0, it results in an error and can be found https://github.com/apache/beam/issues/22621 here.
We need to update cibuildwheel to latest version if we want to use setuptools >= 60.0. More details on the bug can be found https://github.com/pypa/setuptools/issues/2993 here.
fixes: https://github.com/apache/beam/issues/22621. All the tests pass with this change can be found at https://github.com/apache/beam/pull/22625/
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941337047
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
So the image cibuildwheel uses has a very old setuptools. For the effort to support python 3.10, I would need setuptools>=60 since the stdlib distutils module is deprecated and distutils provided by setuptools is recommended to use.
Cibuildwheel==1.11.0 uses get-pip.py to install packages. This combined with latest version of setuptools fails because in the setuptools, there is an assertion statement which checks if _distutils is imported or not.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys merged pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
kileys merged PR #22635:
URL: https://github.com/apache/beam/pull/22635
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941332055
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
Added it here just as precaution.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941390925
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
We already set `CIBW_ARCHS_LINUX` like 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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941389744
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
Ok, that's good with me - I guess we do the same thing for all our Jenkins jobs - https://github.com/apache/beam/blob/18ab78c8c42a83816a43a3d5252f6d843a82b36c/.test-infra/jenkins/CommonJobProperties.groovy#L100
Would we be better off just setting this as a job level variable? https://docs.github.com/en/actions/learn-github-actions/environment-variables
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941439809
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
I'd probably vote we move all the CIBW env vars that aren't scoped to a step to job level, but that doesn't need to be a part of this PR. I'm good with trying to take this one forward.
I'll probably let @kileys make the call on merging here and on the cherry pick as release manager 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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941337047
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
So the image cibuildwheel uses has a very old setuptools. For the effort to support python 3.10, I would need setuptools>=60 since the stdlib distutils module is deprecated and distutils provided by setuptools is recommended to use.
Cibuildwheel==1.11.0 uses get-pip.py to install packages. This combined with latest version of setuptools fails because in the setuptools, there is an assertion statement which checks if _distutils is imported or not. There has been some issues on this bug as well.
So to by pass this, i tell the interpreter to use standard lib distutils for building wheels.
As part of this effort, we would need to update the cibuildwheel to latest version and remove this workaround.
It may take a little time to investigate which version of cibuildwheel we need as the latest one builds both manylinux, musllinux, which i suspect is taking longer time to complete the build and resulting in time out.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941429133
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
Hm since this is particular to this block of code and also would be temporary, I think its okay to be here.
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
Hm since this is particular to this block of code and also would be temporary, I thought its okay to be 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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
aaltay commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941611002
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
+1 to a clean up in the master branch (not blocking release). And if any of this is temporary workarounds, please add a link to an issue with a comment.
(In general if a line needs an in depth explanation, or could be removed later (e.g. after python 3.9 deprecation etc.( it would be good to annotate those with a comment.)
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22635:
URL: https://github.com/apache/beam/pull/22635#issuecomment-1209096713
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22635:
URL: https://github.com/apache/beam/pull/22635#issuecomment-1209095075
R: @damccorm @kileys @TheNeuralBit @kileys
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941337047
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
So the image cibuildwheel uses has a very old setuptools. For the effort to support python 3.10, I would need setuptools>=60 since the stdlib distutils module is deprecated and distutils provided by setuptools is recommended to use(it has a local copy of distutils from 60.0)
Cibuildwheel==1.11.0 uses get-pip.py to install packages. This combined with latest version of setuptools fails because in the setuptools, there is an assertion statement which checks if _distutils is imported or not. There has been some issues on this bug as well.
So to by pass this, i tell the interpreter to use standard lib distutils for building wheels.
As part of this effort, we would need to update the cibuildwheel to latest version and remove this workaround.
It may take a little time to investigate which version of cibuildwheel we need as the latest one builds both manylinux, musllinux, which i suspect is taking longer time to complete the build and resulting in time out.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941247881
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
Could you help me understand why we need this (here and below, but especially here since this isn't where its normally failing)?
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22635:
URL: https://github.com/apache/beam/pull/22635#discussion_r941337047
##########
.github/workflows/build_wheels.yml:
##########
@@ -240,6 +240,7 @@ jobs:
working-directory: apache-beam-source
env:
CIBW_BUILD: ${{ matrix.os_python.python }}
+ CIBW_ENVIRONMENT: "SETUPTOOLS_USE_DISTUTILS=stdlib"
Review Comment:
So the image cibuildwheel uses has a very old setuptools. For the effort to support python 3.10, I would need setuptools>=60 since the stdlib distutils module is deprecated and distutils provided by setuptools is recommended to use(it has a local copy of distutils from 60.0)
Cibuildwheel==1.11.0 uses get-pip.py to install packages. This combined with latest version of setuptools fails because in the setuptools, there is an assertion statement which checks if _distutils is imported or not. There has been some issues on this bug as well.
So to by pass this, i tell the interpreter to use standard lib distutils for building wheels, which by passes the assertion statement error.
As part of this effort, we would need to update the cibuildwheel to latest version and remove this workaround.
It may take a little time to investigate which version of cibuildwheel we need as the latest one builds both manylinux, musllinux, which i suspect is taking longer time to complete the build and resulting in time out.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22635:
URL: https://github.com/apache/beam/pull/22635#issuecomment-1209096949
Since this only run during RC, I removed the if conditions which check if its on RC or not and ran it on the fork to verify if the tests work.
Tests pass at : https://github.com/apache/beam/pull/22625/
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #22635: Add stdlib distutils env variable while building the wheels
Posted by GitBox <gi...@apache.org>.
kileys commented on PR #22635:
URL: https://github.com/apache/beam/pull/22635#issuecomment-1209594438
Thanks! Happy to merge and iterate after as 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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org