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

build: add workaround for V8 builds #38632

Merged
merged 1 commit into from
May 12, 2021
Merged

Conversation

richardlau
Copy link
Member

V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
find_executable from distutils.spawn to allow the configure
script to run in the V8 test job.

Refs: #38600 (comment)

cc @targos

@github-actions github-actions bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels May 11, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 11, 2021

@richardlau
Copy link
Member Author

ugh. V8 CI is failing with

11:02:05 make[2]: execvp: printf: Argument list too long

which I don't think is this PR (since the same failure is seen in https://ci.nodejs.org/job/node-test-commit-v8-linux/3982/ which is before #38600 landed)... my suspicion would be #38576 although the regular CI runs are passing (including today's master https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/2309/).

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member Author

richardlau commented May 11, 2021

11:02:05 make[2]: execvp: printf: Argument list too long

I've enabled a custom child workspace directory in the V8 CI job configuration so that instead of building in a workspace directory of

/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark-ubuntu1604-intel-64/v8test/v8test

the job will now build in

/home/iojs/build/workspace/node-test-commit-v8-linux

which should be safe as all of our Jenkins nodes only have one executor so can only run one job at a time. This will result in shorter command lines, hopefully within the limit 🤞.
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3987/ (v8-updates/test-linux-perf test failure)
https://ci.nodejs.org/job/node-test-commit-v8-linux/3988/ (✔️)

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 11, 2021
@targos targos added the fast-track PRs that do not need to wait for 48 hours to land. label May 11, 2021
@github-actions
Copy link
Contributor

Fast-track has been requested by @targos. Please 👍 to approve.

V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
`find_executable` from `distutils.spawn` to allow the configure
script to run in the V8 test job.

PR-URL: nodejs#38632
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@richardlau
Copy link
Member Author

Landed in d206aa3.

@richardlau richardlau merged commit d206aa3 into nodejs:master May 12, 2021
@richardlau richardlau deleted the workaroundv8 branch May 12, 2021 09:46
targos pushed a commit that referenced this pull request May 17, 2021
V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
`find_executable` from `distutils.spawn` to allow the configure
script to run in the V8 test job.

PR-URL: #38632
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request May 30, 2021
V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
`find_executable` from `distutils.spawn` to allow the configure
script to run in the V8 test job.

PR-URL: #38632
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
`find_executable` from `distutils.spawn` to allow the configure
script to run in the V8 test job.

PR-URL: #38632
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
`find_executable` from `distutils.spawn` to allow the configure
script to run in the V8 test job.

PR-URL: #38632
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
V8's build toolchain is not compatible with Python 3 and the CI job
that tests V8 needs to be run with Python 2. Add a fallback to
`find_executable` from `distutils.spawn` to allow the configure
script to run in the V8 test job.

PR-URL: #38632
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants