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: make configure.py compatible with python 3 #25580

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@thefourtheye
Copy link
Contributor

thefourtheye commented Jan 19, 2019

This patch replaces the following

  1. Usage of filter with None to remove falsy items.
  2. Usage of map to create lists. (Replaced with List comprehensions).
  3. Dictionary's iteritems which is removed in Python 3.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs/python @cclauss

build: make configure.py compatible with python 3
This patch replaces the following

1. Usage of `filter` with `None` to remove falsy items.
2. Usage of `map` to create lists. (Replaced with List comprehensions).
3. Dictionary's `iteritems` which is removed in Python 3.

@refack refack added the python label Jan 19, 2019

@refack

This comment has been minimized.

Copy link
Member

refack commented Jan 19, 2019

Code LGTM, but... for ./configure ... to actually work, we (read I) need to vendor in the python3 compatible GYP patches, and fix https://github.com/nodejs/node/blob/master/configure

@refack

refack approved these changes Jan 19, 2019

@thefourtheye

This comment has been minimized.

Copy link
Contributor Author

thefourtheye commented Jan 20, 2019

@refack Addressing GYP Python 3 compatibility would be the next big thing. Please let me know where we can collaborate.

@refack

This comment has been minimized.

Copy link
Member

refack commented Jan 20, 2019

Please let me know where we can collaborate.

@thefourtheye, the chromium team, @cclauss, and I have been working on that in recent months. The Google GYP repo (https://github.com/chromium/gyp) and the version in https://github.com/nodejs/node-gyp should be ready, but those don't get CI testing with python3. https://github.com/refack/GYP
is a work in progress on a GYP fork that does CI testing using both python2 & python3.

Cross-ref: nodejs/TSC#642

@addaleax

This comment has been minimized.

@danbev

This comment has been minimized.

Copy link
Member

danbev commented Jan 28, 2019

Re-run of failing node-test-commit-linux ✔️
Re-run of failing node-test-commit-linux-containered ✔️

@danbev

This comment has been minimized.

Copy link
Member

danbev commented Jan 28, 2019

Landed in 26f80dc.

@danbev danbev closed this Jan 28, 2019

danbev added a commit that referenced this pull request Jan 28, 2019

build: make configure.py compatible with python 3
This patch replaces the following

1. Usage of `filter` with `None` to remove falsy items.
2. Usage of `map` to create lists. (Replaced with List comprehensions).
3. Dictionary's `iteritems` which is removed in Python 3.

PR-URL: #25580
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@thefourtheye thefourtheye deleted the thefourtheye:make-configure-python3-compatible branch Jan 28, 2019

addaleax added a commit that referenced this pull request Jan 28, 2019

build: make configure.py compatible with python 3
This patch replaces the following

1. Usage of `filter` with `None` to remove falsy items.
2. Usage of `map` to create lists. (Replaced with List comprehensions).
3. Dictionary's `iteritems` which is removed in Python 3.

PR-URL: #25580
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@targos targos referenced this pull request Jan 29, 2019

Merged

v11.9.0 proposal #25802

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment