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.

Sign up for GitHub

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

Jump to bottom

doc: updated building docs for windows #49767

Merged
merged 5 commits into from
Oct 1, 2023
Merged

doc: updated building docs for windows #49767

merged 5 commits into from
Oct 1, 2023

Conversation

ovflowd
Copy link
Member

@ovflowd ovflowd commented Sep 22, 2023

This PR updates the Windows Building Instructions and the Boxstarter Automated Installation Script due to the following:

  • The vcbuild.bat will require Python3 to be installed, but on Windows, if Python 2 is installed, it will take precedence over the $PATH; also, Python2 is not needed for Node.js builds anymore.
  • If installed on ARM64, the recommended packages are incompatible. ARM64 compatibility was added with Visual Studio 2022 (17.4.0 or higher)
    • Updated the scripts to use VS2022 Chocolatey Packages
    • Updated the Building docs to use the newest Visual Studio Packages (which are compatible with Windows 10 and Windows 11)
  • Updated Docs to clarify that folks should first clone the repository and head to the cloned directory. A bunch of newcomers on Grace Hopper were confused about this.

@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. tools Issues and PRs related to the tools directory. labels Sep 22, 2023
@ovflowd

This comment was marked as outdated.

Sign in to view
@Trott
Copy link
Member

Trott commented Sep 22, 2023

@nodejs/platform-windows @nodejs/build

JakobJingleheimer
JakobJingleheimer approved these changes Sep 22, 2023
View reviewed changes
BUILDING.md Show resolved Hide resolved
RafaelGSS
RafaelGSS reviewed Sep 24, 2023
View reviewed changes
BUILDING.md Outdated Show resolved Hide resolved
@richardlau
Copy link
Member

This PR updates the Windows Building Instructions and the Boxstarter Automated Installation Script due to the following:
* If installed on ARM64, the recommended packages are incompatible. ARM64 compatibility was added with Visual Studio 2022 (17.4.0 or higher)

FYI @nodejs/platform-windows-arm Do these changes look good to you?

targos
targos reviewed Sep 26, 2023
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

Suggestions based on https://github.com/nodejs/node/pull/49051/files

BUILDING.md Outdated Show resolved Hide resolved
BUILDING.md Outdated Show resolved Hide resolved
BUILDING.md Outdated Show resolved Hide resolved
tools/bootstrap/windows_boxstarter Outdated Show resolved Hide resolved
@ovflowd @targos
Apply suggestions from code review
524159a 
Co-authored-by: Michaël Zasso <targos@protonmail.com>
@ovflowd ovflowd added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Sep 29, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 29, 2023
@nodejs-github-bot

This comment was marked as resolved.

Sign in to view
@ovflowd ovflowd added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 29, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 29, 2023
@nodejs-github-bot

This comment was marked as resolved.

Sign in to view
@ovflowd

This comment was marked as outdated.

Sign in to view
@targos

This comment was marked as resolved.

Sign in to view
@ovflowd

This comment was marked as resolved.

Sign in to view
@ovflowd ovflowd force-pushed the doc-updates-windows-install branch from 775caf0 to 524159a Compare October 1, 2023 09:22
@ovflowd ovflowd added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 1, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 1, 2023
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@ovflowd
Copy link
Member Author

ovflowd commented Oct 1, 2023

Can someone reapprove the PR? Apparently node land is failing due last commit not having an approval

@ovflowd ovflowd removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 1, 2023
@ovflowd ovflowd requested a review from targos October 1, 2023 13:55
@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 1, 2023
@ovflowd ovflowd added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Oct 1, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 1, 2023
@nodejs-github-bot nodejs-github-bot merged commit 47b2883 into nodejs:main Oct 1, 2023
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 47b2883

@ovflowd ovflowd deleted the doc-updates-windows-install branch October 1, 2023 14:46
@RafaelGSS RafaelGSS mentioned this pull request Oct 9, 2023
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@ovflowd @alexfernandez
doc: updated building docs for windows
0f6e4a8 
PR-URL: nodejs#49767
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this pull request Nov 11, 2023
@ovflowd @targos
doc: updated building docs for windows
f63a92b 
PR-URL: #49767
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@targos targos mentioned this pull request Nov 12, 2023
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
Merged
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@ovflowd @debadree25
doc: updated building docs for windows
5415139 
PR-URL: nodejs#49767
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@aymen94 aymen94 aymen94 approved these changes

@MoLow MoLow MoLow approved these changes

@richardlau richardlau richardlau approved these changes

@JakobJingleheimer JakobJingleheimer JakobJingleheimer approved these changes

@lpinca lpinca lpinca approved these changes

@targos targos Awaiting requested review from targos

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@ovflowd @Trott @richardlau @nodejs-github-bot @targos @lpinca @JakobJingleheimer @MoLow @aymen94 @RafaelGSS