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

[v14.x] Backport Python 3.10 compatibility patches #40689

Closed
wants to merge 3 commits into from

Conversation

richardlau
Copy link
Member

Backport of #40296.

Required to fix the Windows CI with v14.x-staging. Conflicts were due to v14.x supporting fallback to Python 2 if Python 3 is not available.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v14.x v8 engine Issues and PRs related to the V8 dependency. labels Nov 1, 2021
@nodejs-github-bot

This comment has been minimized.

@richardlau
Copy link
Member Author

Apparently the jinja patches will need work to remain compatible with Python 2.
e.g. https://ci.nodejs.org/job/node-test-commit-linux/nodes=fedora-latest-x64/43511/console

11:51:43 Traceback (most recent call last):
11:51:43   File "../../deps/v8/third_party/inspector_protocol/code_generator.py", line 718, in <module>
11:51:43     main()
11:51:43   File "../../deps/v8/third_party/inspector_protocol/code_generator.py", line 601, in main
11:51:43     jinja_env = initialize_jinja_env(jinja_dir, config.protocol.output, config)
11:51:43   File "../../deps/v8/third_party/inspector_protocol/code_generator.py", line 190, in initialize_jinja_env
11:51:43     import jinja2
11:51:43   File "/home/iojs/build/workspace/node-test-commit-linux/deps/v8/third_party/jinja2/__init__.py", line 33, in <module>
11:51:43     from jinja2.environment import Environment, Template
11:51:43   File "/home/iojs/build/workspace/node-test-commit-linux/deps/v8/third_party/jinja2/environment.py", line 16, in <module>
11:51:43     from jinja2.defaults import BLOCK_START_STRING, \
11:51:43   File "/home/iojs/build/workspace/node-test-commit-linux/deps/v8/third_party/jinja2/defaults.py", line 32, in <module>
11:51:43     from jinja2.tests import TESTS as DEFAULT_TESTS
11:51:43   File "/home/iojs/build/workspace/node-test-commit-linux/deps/v8/third_party/jinja2/tests.py", line 13, in <module>
11:51:43     from collections.abc import Mapping
11:51:43 ImportError: No module named abc
11:51:43 make[2]: *** [tools/v8_gypfiles/v8_base_without_compiler.target.mk:30: 

targos and others added 3 commits November 1, 2021 08:15
PR-URL: nodejs#40296
Fixes: nodejs#40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: nodejs#40296
Fixes: nodejs#40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: nodejs#40296
Fixes: nodejs#40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 1, 2021

@richardlau
Copy link
Member Author

V8 builds are failing with
e.g. https://ci.nodejs.org/job/node-test-commit-v8-linux/4364/nodes=rhel7-s390x,v8test=v8test/console

12:32:18 ERROR at //testing/gmock/BUILD.gn:11:1: Dependency not allowed.
12:32:18 source_set("gmock") {
12:32:18 ^--------------------
12:32:18 The item //testing/gmock:gmock
12:32:18 can not depend on //third_party/googletest:gtest_config
12:32:18 because it is not in //third_party/googletest:gtest_config's visibility list: [
12:32:18   //third_party/googletest:*
12:32:18 ]
12:32:18 
12:32:18 make: *** [Makefile:272: v8] Error 1

I've tried runs on v14.x-staging and v14.x (e.g. without this PR) and the same failure is present 😞.
v14.x-staging: https://ci.nodejs.org/job/node-test-commit-v8-linux/4365/nodes=rhel7-s390x,v8test=v8test/console
v14.x: https://ci.nodejs.org/job/node-test-commit-v8-linux/4366/nodes=rhel7-s390x,v8test=v8test/console

@targos
Copy link
Member

targos commented Nov 1, 2021

Didn't we fix these errors already?

@targos
Copy link
Member

targos commented Nov 1, 2021

Or could it be a problem with the GN version that's in CI?

@targos
Copy link
Member

targos commented Nov 1, 2021

I think the last time V8 CI was executed for v14.x is in #39990: https://ci.nodejs.org/job/node-test-commit-v8-linux/4257/

@richardlau
Copy link
Member Author

Or could it be a problem with the GN version that's in CI?

Yes, most probably by the looks of things. It looks like s390x is the only platform failing this way. When compared to ppc64_le, s390x is on a newer version of gn:

$ ansible -m shell -a "/home/iojs/build-tools/gn --version" test-ibm-rhel7-s390x-?
test-ibm-rhel7-s390x-3 | CHANGED | rc=0 >>
1938 (0153d36)
test-ibm-rhel7-s390x-2 | CHANGED | rc=0 >>
1938 (0153d36)
test-ibm-rhel7-s390x-4 | CHANGED | rc=0 >>
1938 (0153d36)
test-ibm-rhel7-s390x-1 | CHANGED | rc=0 >>
1938 (0153d36)
$ ansible  -m shell -a "/home/iojs/build-tools/gn --version" test-osuosl-centos7-ppc64_le-?
test-osuosl-centos7-ppc64_le-4 | CHANGED | rc=0 >>
1930 (c0a2d23)
test-osuosl-centos7-ppc64_le-3 | CHANGED | rc=0 >>
1930 (c0a2d23)
test-osuosl-centos7-ppc64_le-1 | CHANGED | rc=0 >>
1930 (c0a2d23)
test-osuosl-centos7-ppc64_le-2 | CHANGED | rc=0 >>
1930 (c0a2d23)
$

Looking at the gn commit history

0153d36 Corrects the generation of .stamp file names. by Filip Filmar · 5 weeks ago
de86ec4 Escape spaces in "libs" by Nico Weber · 7 weeks ago
07e2e1b Fix typos in README.md by Ted Pudlik · 7 weeks ago
46b572c Fix configs visibility not always working by Tomasz Śniatowski · 8 weeks ago
69ec4fc C++ modernization improvements. by Brett Wilson · 3 months ago
32aa9d9 Minor style updates. by Brett Wilson · 3 months ago
45aa842 Enable GN to build & run on z/OS by Gaby Baghdadi · 6 months ago
eea3906 Fix a typo in the doc of "generated_file" by Dangyi Liu · 3 months ago
c0a2d23 Update doc/help to say --ninja-executable works for VS Flags by Shezan Baig · 4 months ago

https://gn.googlesource.com/gn/+/46b572ce4ceedfe57f4f84051bd7da624c98bf01 ("Fix configs visibility not always working") sounds like a plausible cause -- that is the newer gn is enforcing visibility rules that were previously not working. I'll try rolling back gn on the s390x machines.

@richardlau
Copy link
Member Author

richardlau commented Nov 1, 2021

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@richardlau
Copy link
Member Author

Looks like Windows CI is timing out on similar tests to the timeouts on master (refs #40684).

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 24, 2021

@richardlau
Copy link
Member Author

Landed in 0682370...233279d.

@richardlau richardlau closed this Nov 24, 2021
richardlau pushed a commit that referenced this pull request Nov 24, 2021
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
richardlau pushed a commit that referenced this pull request Nov 24, 2021
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
richardlau pushed a commit that referenced this pull request Nov 24, 2021
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@richardlau richardlau deleted the v14.x-py3.10 branch November 24, 2021 11:53
richardlau pushed a commit that referenced this pull request Nov 25, 2021
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
richardlau pushed a commit that referenced this pull request Nov 25, 2021
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
richardlau pushed a commit that referenced this pull request Nov 25, 2021
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants