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

tools: add Boxstarter script #17046

Closed
wants to merge 4 commits into from
Closed

Conversation

bzoz
Copy link
Contributor

@bzoz bzoz commented Nov 15, 2017

Adds a Boxstarter script and documentation for an easy way to install Node.js building prerequisites on Windows. It will install git, python, msbuild tools and optionally VSCode.

Ref: nodejs/code-and-learn#71

The links will not work until this lands. A version with working links can be found here: tools/boxstarter/README.md
Edit: new link - tools/bootstrap/README.md.

Checklist
Affected core subsystem(s)

build, doc, tools

Adds a Boxstarter script for easy Node.js building prerequisites installation
@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 Nov 15, 2017
@bzoz
Copy link
Contributor Author

bzoz commented Nov 15, 2017

@benjamingr
Copy link
Member

benjamingr commented Nov 15, 2017

cc @refack @mwrock


Great work, I'm hugely +1 on anything that'll make onboarding windows users easier. Boxstarter which I was unfamiliar with looks based on the built in package manager and with a permissive license.

Install-BoxstarterPackage https://raw.githubusercontent.com/nodejs/node/master/tools/boxstarter/node_boxstarter -DisableReboots
```

* For Node.js prerequisites with VS Code:
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why we need this?

#

# Git and Unix tools will be added to the PATH
choco install git -params /GitAndUnixToolsOnPath -y
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer it if we didn't install the editor as well - most people being onboarded have their own preference for an editor.

@@ -0,0 +1,54 @@
# Boxstarter setup for Node.js

A [Boxstarter][] script can be used for an easy setup of Windows systems with all
Copy link
Member

Choose a reason for hiding this comment

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

maybe remove an.

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Waiting for vscode

choco install visualstudio2017-workload-vctools -y

# Installs Visual Studio Code
choco install visualstudiocode -y
Copy link
Member

Choose a reason for hiding this comment

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

+1 to not installing an editor, especially if it means duplicating the whole script.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of duplicating the script, we could just check if an environment variable is set indicating whether an editor should be installed? (not advocating one way or the other but just pointing out that we likely don't need to duplicate the whole script)

@refack
Copy link
Contributor

refack commented Nov 15, 2017

Love it.
I would go one step further and add a shortcut through vcbuild boxstarter:

powershell.exe -ExecutionPolicy Unrestricted -noprofile -Command "&{iex ((New-Object System.Net.WebClient).DownloadString('http://boxstarter.org/bootstrapper.ps1')); get-boxstarter -Force; Install-BoxstarterPackage https://raw.githubusercontent.com/nodejs/node/master/tools/boxstarter/node_boxstarter -DisableReboots}"

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Cautiously +1 on this, but it seems odd to just have Windows packages documented. I think I'd prefer something in doc/guides/ that covered all platforms (e.g. brew for macOS, apt/yum/pacman+ for Linux etc.).


# Installs VS 2017 Build Tools
choco install visualstudio2017buildtools -y
choco install visualstudio2017-workload-vctools -y
Copy link
Member

Choose a reason for hiding this comment

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

What is the benefit of using Boxstarter here rather than just using chocolatey directly? Is it the web url?

Looking at their page, I'm not seeing anything obviously beneficial, and it's another thing for new starters to understand.

:: Copied from chocolatey website
@"%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe" -NoProfile -InputFormat None -ExecutionPolicy Bypass -Command "iex ((New-Object System.Net.WebClient).DownloadString('https://chocolatey.org/install.ps1'))" && SET "PATH=%PATH%;%ALLUSERSPROFILE%\chocolatey\bin"

choco install git -params /GitAndUnixToolsOnPath -y
choco install python2 visualstudio2017buildtools visualstudio2017-workload-vctools -y

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use a more generic directory name like tools/bootstrap/ and then iterate over recipes.

Copy link
Member

Choose a reason for hiding this comment

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

This is an improvement either way, but I'd go with boxstarter rather than just chocolatey. For users that want to understand this or already know about chocolatey, the script is very easy to understand and use as a guide for their own custom installation (plus, dependencies are already documented in BUILDING.md).

Looking at the Boxstarter page, it adds a lot of resilience to the process (reboots, disabling updates, etc), moving it much closer to the ideal one-click-no-issues setup.

@refack refack added the windows Issues and PRs related to the Windows platform. label Nov 15, 2017
Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

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

LGTM with or without VSCode.

Also cc @nodejs/platform-windows

## WebLauncher Installation

To install Node.js prerequisites using [Boxstarter WebLauncher][], just open
**one** of the following links with Internet Explorer or Edge browser on the
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does it need to be just those two browsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a "Click Once" app (https://msdn.microsoft.com/en-us/library/t71a733d.aspx). It works in IE and Edge by default, other browsers need a plugin.

choco install visualstudio2017-workload-vctools -y

# Installs Visual Studio Code
choco install visualstudiocode -y
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of duplicating the script, we could just check if an environment variable is set indicating whether an editor should be installed? (not advocating one way or the other but just pointing out that we likely don't need to duplicate the whole script)

@digitalinfinity
Copy link
Contributor

This is awesome, thanks @bzoz! (cc @yodurr @rachelnicole). +1 to @refack's suggestion of adding this support to vcbuild too (although I would still prefer having an explicit developer shell for Node but that can come later 😄).

@bzoz
Copy link
Contributor Author

bzoz commented Nov 16, 2017

Updated the PR, PTAL.

I've renamed it from tools/boxstarter to tools/bootstrap. I've dropped VSCode - I've assumed one of the use cases would be installing dev environment on a completely clean Windows install, having a link that would also install editor sounded reasonable.

@refack vcbuild boxstart would be great, but it needs to be run in a elevated terminal. I guess some more powershell can do the UAC. It could be also used to disable Windows Defender for the Node folder (as mentioned in #16278).

@gibfahn
Copy link
Member

gibfahn commented Nov 16, 2017

I think you can do this to get elevated powershell:

@"%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe" -NoProfile -InputFormat None -ExecutionPolicy Bypass -Command "..."

(copied from the chocolatey getting started.

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

LGTM modulo nit

BUILDING.md Outdated
@@ -202,6 +202,9 @@ Prerequisites:
[Git for Windows](http://git-scm.com/download/win) includes Git Bash
and tools which can be included in the global `PATH`.

*Note:* All prerequisites can be easily installed by following
[this bootstrapping guide](https://github.com/nodejs/node/blob/master/tools/bootstrap/README.md#Windows).
Copy link
Member

Choose a reason for hiding this comment

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

Maybe not move this to the top of the list and drop the #Windows off the end? That way it'll work when we add other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't understand what you mean

Copy link
Member

Choose a reason for hiding this comment

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

Move this line to L83, and change it to:

All prerequisites can be easily installed by following
[this bootstrapping guide](https://github.com/nodejs/node/blob/master/tools/bootstrap/README.md).

@bzoz
Copy link
Contributor Author

bzoz commented Nov 22, 2017

Updated - moved the link, and added instructions for Linux. PTAL

@@ -22,6 +22,20 @@ get-boxstarter -Force
Install-BoxstarterPackage https://raw.githubusercontent.com/nodejs/node/master/tools/bootstrap/windows_boxstarter -DisableReboots
```

## Linux

For building Node.js on Linux, following packets are required (note, that this
Copy link
Member

Choose a reason for hiding this comment

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

Should packets be packages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Yeah this looks great, worth adding macOS too, but otherwise let's land.

To bootstrap Node.js on Linux, run in terminal:
* OpenSUSE: `sudo zypper install git-core python gcc-c++ make`
* Fedora: `sudo dnf install git-core python gcc-c++ make`
* Ubuntu, Debian: `sudo apt-get install git-core python g++ make`
Copy link
Member

Choose a reason for hiding this comment

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

Does git-core work? From the website it seems like it should be git for all three distros.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like on openSUSE git-core is more lightweight than git package. But I think I'll change this anyway.

* OpenSUSE: `sudo zypper install git-core python gcc-c++ make`
* Fedora: `sudo dnf install git-core python gcc-c++ make`
* Ubuntu, Debian: `sudo apt-get install git-core python g++ make`

Copy link
Member

Choose a reason for hiding this comment

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

For macOS I'm pretty sure it's just:

xcode-select --install # Installs git, make, and clang

python2 is already installed.

@digitalinfinity
Copy link
Contributor

I gave this a spin on a new machine which happened to have VS (internal dogfood) and git already installed. There were probably some issues caused by VS already being installed because the boxstarter script ended with a strange warning about chocolatey failing but didn't actually print out what the error was, it just asked me to hit enter and then it exited- but it looks like python27 was installed correctly.

One thing that might be useful to add in the readme is how much free disk space the user should be expected to have before running the script?

@bzoz
Copy link
Contributor Author

bzoz commented Nov 23, 2017

@digitalinfinity I just tried that, a fresh Win2016 install on amazon takes 13.7 GB. After the script it rises to 21.6 GB

@digitalinfinity
Copy link
Contributor

Great, thanks for checking! Perhaps it would be good to document that expectation?

@bzoz
Copy link
Contributor Author

bzoz commented Nov 28, 2017

Updated, PTAL

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 30, 2017
@bzoz
Copy link
Contributor Author

bzoz commented Nov 30, 2017

Landed in 61d46dd

@bzoz bzoz closed this Nov 30, 2017
bzoz added a commit that referenced this pull request Nov 30, 2017
Adds a Boxstarter script for easy Node.js building prerequisites installation

PR-URL: #17046
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 30, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Adds a Boxstarter script for easy Node.js building prerequisites installation

PR-URL: #17046
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Adds a Boxstarter script for easy Node.js building prerequisites installation

PR-URL: #17046
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
Adds a Boxstarter script for easy Node.js building prerequisites installation

PR-URL: #17046
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
Adds a Boxstarter script for easy Node.js building prerequisites installation

PR-URL: #17046
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
Adds a Boxstarter script for easy Node.js building prerequisites installation

PR-URL: #17046
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
@MylesBorins MylesBorins mentioned this pull request Dec 20, 2017
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. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants