-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
build: remove dependency on distutils.spawn
#38600
Conversation
cc @nodejs/python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for doing this.
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: nodejs#38600 Refs: nodejs#30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 16982b6. |
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: nodejs#38600 Refs: nodejs#30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: nodejs#38600 Refs: nodejs#30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This broke V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3984/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/console
|
https://docs.python.org/3/library/shutil.html#shutil.which has been around since Python 3.3 |
Yeah, but V8 build system doesn't support Python 3, so this job still has to use Python 2 |
😞. #38632 implements the fallback to |
This should be landable on earlier releases as long as it is accompanied by #38632. |
Python 2 died 500 daze ago... Did they not get the memo? |
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: #38600 Refs: #30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: #38600 Refs: #30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: #38600 Refs: #30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: #38600 Refs: #30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Debian based packages of Python 3 do not include `distutils.spawn` and require an additional apt package to be installed (`python3-distutils`). Replace use of `distutils.spawn` with `shutil.which`, available in all versions of Python currently allowed by our configure scripts. For the `configure` script only, fall back to `distutils.spawn` to allow friendlier error messages when run on older unsupported versions of Python (e.g. 2.7). `configure.py` also uses `distutils.version` -- this appears to be available in Debian packaged Python 3 without installing `python3-distutils` so has been left as-is. PR-URL: #38600 Refs: #30189 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Debian based packages of Python 3 do not include
distutils.spawn
andrequire an additional apt package to be installed (
python3-distutils
).Replace use of
distutils.spawn
withshutil.which
, available in allversions of Python currently allowed by our configure scripts.
For the
configure
script only, fall back todistutils.spawn
to allowfriendlier error messages when run on older unsupported versions of
Python (e.g. 2.7).
configure.py
also usesdistutils.version
-- this appears to beavailable in Debian packaged Python 3 without installing
python3-distutils
so has been left as-is.Refs: #30189