[v14.x] Backport Python 3.10 compatibility patches by richardlau · Pull Request #40689 · nodejs/node · GitHub
Skip to content

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

Closed
richardlau wants to merge 3 commits intonodejs:v14.x-stagingfrom
richardlau:v14.x-py3.10
Closed

[v14.x] Backport Python 3.10 compatibility patches#40689
richardlau wants to merge 3 commits intonodejs:v14.x-stagingfrom
richardlau:v14.x-py3.10

Conversation

@richardlau
Copy link
Copy Markdown
Member

@richardlau richardlau commented Nov 1, 2021

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.

Sign in to view
@richardlau
Copy link
Copy Markdown
Member Author

richardlau commented Nov 1, 2021

targos and others added 3 commits November 1, 2021 08:15
@targos @richardlau
deps: V8: patch jinja2 for Python 3.10 compat
80b7c4a 
PR-URL: nodejs#40296
Fixes: nodejs#40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@targos @richardlau
tools: patch jinja2 for Python 3.10 compat
1007a6e 
PR-URL: nodejs#40296
Fixes: nodejs#40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@iam-frankqiu @richardlau
build: support Python 3.10.0
31c42ae 
PR-URL: nodejs#40296
Fixes: nodejs#40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Nov 1, 2021 •
edited by richardlau
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/40615/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/4364/

@richardlau
Copy link
Copy Markdown
Member Author

richardlau commented Nov 1, 2021

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
Copy Markdown
Member

targos commented Nov 1, 2021

Didn't we fix these errors already?

@targos
Copy link
Copy Markdown
Member

targos commented Nov 1, 2021

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

@targos
Copy link
Copy Markdown
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
Copy Markdown
Member Author

richardlau commented Nov 1, 2021

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
Copy Markdown
Member Author

richardlau commented Nov 1, 2021 •
edited
Loading

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/4367/ (✔️)

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@richardlau
Copy link
Copy Markdown
Member Author

richardlau commented Nov 1, 2021

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.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
This was referenced Nov 2, 2021
Open
Open
Open
This was referenced Nov 5, 2021
Open
Open
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Nov 23, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/41058/

@github-actions github-actions Bot mentioned this pull request Nov 24, 2021
Open
45 tasks
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Nov 24, 2021 •
edited by richardlau
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/41064/ (✔️)

@richardlau
Copy link
Copy Markdown
Member Author

richardlau commented Nov 24, 2021

@richardlau richardlau closed this Nov 24, 2021
richardlau pushed a commit that referenced this pull request Nov 24, 2021
@targos @richardlau
deps: V8: patch jinja2 for Python 3.10 compat
fc517cb 
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this pull request Nov 24, 2021
@targos @richardlau
tools: patch jinja2 for Python 3.10 compat
f16d255 
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this pull request Nov 24, 2021
@iam-frankqiu @richardlau
build: support Python 3.10.0
233279d 
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@richardlau richardlau deleted the v14.x-py3.10 branch November 24, 2021 11:53
@github-actions github-actions Bot mentioned this pull request Nov 25, 2021
Open
38 tasks
richardlau pushed a commit that referenced this pull request Nov 25, 2021
@targos @richardlau
deps: V8: patch jinja2 for Python 3.10 compat
cd7c340 
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this pull request Nov 25, 2021
@targos @richardlau
tools: patch jinja2 for Python 3.10 compat
6b7b2bb 
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this pull request Nov 25, 2021
@iam-frankqiu @richardlau
build: support Python 3.10.0
da35612 
PR-URL: #40296
Backport-PR-URL: #40689
Fixes: #40294
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This was referenced Nov 26, 2021
Open
Open
@richardlau richardlau mentioned this pull request Oct 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targos targos targos approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@richardlau @nodejs-github-bot @targos @iam-frankqiu