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

doc: note the system requirements for V8 tests #38319

Closed

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Apr 20, 2021

The test-v8 Makefile target still requires Python 2, and it requires a full Xcode install on macOS, due to its use of xcodebuild at some points.

Refs: #36691 (comment)

(Note for reviewers, a related PR: The test-v8 Makefile target also requires ninja, but I think it doesn't really need to require ninja. It actually downloads ninja as part of Google's depot_tools, but (apparently by mistake) does not put those on the PATH. See my other PR to put this copy of ninja on the PATH: #38299. I didn't add ninja as a requirement in this documentation, since I hope my PR #38299 will make that requirement go away.)

The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: nodejs#36691
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 20, 2021
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT with some optional nits.

doc/guides/maintaining-V8.md Outdated Show resolved Hide resolved
doc/guides/maintaining-V8.md Outdated Show resolved Hide resolved
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 21, 2021

I would like to document the ninja requirement, if #38299 or similar can't be merged. If someone can take a look at and give an idea if that can be merged, or feedback to make it mergeable, that would be much appreciated, and it would help to make sure this documentation is comprehensive. Thank you.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Apr 26, 2021

This and the related #38299 are good to go, from my (PR author's) point of view. Thanks for the reviews.

@jasnell
Copy link
Member

jasnell commented Apr 27, 2021

Landed in d14d9f9

jasnell pushed a commit that referenced this pull request Apr 27, 2021
The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: #36691

PR-URL: #38319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@jasnell jasnell closed this Apr 27, 2021
targos pushed a commit that referenced this pull request Apr 29, 2021
The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: #36691

PR-URL: #38319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@targos targos mentioned this pull request May 3, 2021
targos pushed a commit that referenced this pull request May 30, 2021
The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: #36691

PR-URL: #38319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: #36691

PR-URL: #38319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: #36691

PR-URL: #38319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
The `test-v8` Makefile target still requires Python 2,
and it requires a full Xcode install on macOS.

Refs: #36691

PR-URL: #38319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants