GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — win: do not use Boxstarter to install tools by joaocgreis · Pull Request #24677 · nodejs/node

Conversation

@joaocgreis

@joaocgreis joaocgreis commented Nov 27, 2018

Copy link
Copy Markdown
Member

The script to install Python and Visual Studio on Windows uses Boxstarter to reboot the machine whenever a reboot is pending and disable some Windows features that can make the installation fail. This has been causing issues for some users, leaving the machine in an unexpected state when the installation is cancelled.

This PR removes Boxstarter, changing the script to use Chocolatey directly, thus not changing machine configuration. This makes it possible for the installation to fail because of interference from updates or previous installations, but I could not make this happen in my tests with the packages installed here. Hence, I believe this is a good trade-off to fix the issues that have been happening.

Fixes: #23838
Refs: #24637

cc @nodejs/platform-windows @gep13

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ferventcoder, aduh95, and CedX reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
@joaocgreis
win: do not use Boxstarter to install tools
37edeea
Use Chocolatey directly in the tools installation script.

Fixes: nodejs#23838
@joaocgreis joaocgreis added windows Issues and PRs related to the Windows platform. install Issues and PRs related to installing Node.js and its installers. tools Issues and PRs related to the tools directory. dont-land-on-v6.x labels Nov 27, 2018
@nodejs-github-bot

nodejs-github-bot commented Nov 27, 2018

Copy link
Copy Markdown
Collaborator

@joaocgreis sadly an error occured when I tried to trigger a build :(

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@refack refack mentioned this pull request Nov 27, 2018
Does not complete installation felixrieseberg/windows-build-tools#150
Open
bzoz
bzoz approved these changes Nov 27, 2018
View reviewed changes
gep13
gep13 suggested changes Nov 28, 2018
View reviewed changes
Comment thread tools/msvs/install_tools/install_tools.bat
echo installation, and will resume automatically.
echo install Windows updates. This will take a while to run.
echo.
echo Please close all open programs for the duration of the installation.

@gep13 gep13 Nov 28, 2018

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is there a reason that this part has been removed? I think this advice is still valid.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@aduh95 aduh95 Nov 28, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Do you really close any program to install a software on Windows? I for one never do, and I haven't seen anyone do that...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@joaocgreis joaocgreis Nov 28, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

While in most case there are no issues, this is always good advice to make sure there is no interference. I'll add it back.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@pauby pauby Dec 4, 2018

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@aduh95 One of the issues we had raised several times on the Boxstarter repo is that users do not know that Boxstarter will be rebooting their machines and they lose their work.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread tools/msvs/install_tools/install_tools.bat Outdated
echo.
echo Please close all open programs for the duration of the installation.
echo If the installation fails, please ensure Windows is fully updated and reboot
echo your computer before trying again.

@gep13 gep13 Nov 28, 2018

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can we clarify exactly what this means? Does this mean trigger the entire msi installation again? Can they get back to the prompt to select the Chocolatey installation? Or is this referring to manually triggering this .bat file?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joaocgreis reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread tools/msvs/install_tools/install_tools.bat Outdated
echo - https://chocolatey.org/packages/chocolatey
echo - https://chocolatey.org/packages/boxstarter
echo - https://chocolatey.org/packages/python2
echo - https://chocolatey.org/packages/visualstudio2017buildtools

@gep13 gep13 Nov 28, 2018

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Based on the Chocolatey packages being installed, this one is no longer being directly installed, so I am assuming that it is coming in as a dependency. Since you are not listing all the dependent packages, I don't think it is right to explicitly call this one out.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joaocgreis reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@gep13

gep13 commented Nov 28, 2018

Copy link
Copy Markdown

@joaocgreis said...
his makes it possible for the installation to fail because of interference from updates or previous installations, but I could not make this happen in my tests with the packages installed here.

Can you elaborate on the tests that you did here?

Based on the installation test that I did here:

https://www.youtube.com/watch?v=gwFpXXIJTvs&t=117s

At least one reboot was required, which if not done, would likely have failed the subsequent package installation.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Nov 28, 2018
Could not install Visual Studio Build Tools felixrieseberg/windows-build-tools#152
Open
Consider directing >=10.13.0 users to built-in script felixrieseberg/windows-build-tools#159
Open
joaocgreis added 2 commits November 28, 2018 19:36
@joaocgreis
fixup: gep13 review
0103c4e
@joaocgreis
fixup set title
1175882
@joaocgreis

joaocgreis commented Nov 28, 2018

Copy link
Copy Markdown
Member Author

@gep13 thanks for your review, updated.

Can you elaborate on the tests that you did here?

I tested this on Windows 2008, 2012, 2016 and 10. I tested this on Windows 10 while system updates were installing. The installation always completed successfully.

It is possible that there is some software or some state a machine can be in that will make this fail in a way that Boxstarter would have prevented, that's why I used it before. What I see here is that the installations always succeed, but leave the machine with a reboot pending. Chocolatey already displays a clear warning in this case when it completes.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joaocgreis

joaocgreis commented Nov 28, 2018 •
edited by Trott
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/view/All/job/node-test-pull-request/19008/ ✔️
Test build: https://nodejs.org/download/test/v12.0.0-test2018112811758825bb/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gep13

gep13 commented Nov 29, 2018

Copy link
Copy Markdown

@joaocgreis said...
I tested this on Windows 2008, 2012, 2016 and 10.

Sounds good!

The potential for problems likely comes machines that don't have the Windows Updates installed. I am going to guess that all the machines that you tested on were reasonably new VM's that had Windows Updates installed recently. As a result, the Windows Updates that are dependent packages didn't actually do anything, as they were not required. This is an area where we might still see some reports, but hopefully fewer.

joaocgreis reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gep13
gep13 approved these changes Nov 29, 2018
View reviewed changes
@Trott

Trott commented Dec 1, 2018

Copy link
Copy Markdown
Member

One more review on this would be good. @nodejs/platform-windows was already pinged, but I guess since this is pretty text-heavy and the text is, in a sense, documentation, /ping @nodejs/documentation

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

richardlau
richardlau approved these changes Dec 1, 2018
View reviewed changes
@joaocgreis joaocgreis mentioned this pull request Dec 4, 2018
Node v11.0 windows installation opting into chocolatey+boxstarter results in infinite reboots #23838
Closed
@joaocgreis

joaocgreis commented Dec 5, 2018

Copy link
Copy Markdown
Member Author

Thanks for the reviews!

Landed in 76afdff.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joaocgreis joaocgreis closed this Dec 5, 2018
joaocgreis added a commit that referenced this pull request Dec 5, 2018
@joaocgreis
win: do not use Boxstarter to install tools
76afdff
Use Chocolatey directly in the tools installation script.

PR-URL: #24677
Fixes: #23838
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@joaocgreis joaocgreis mentioned this pull request Dec 5, 2018
v11.4.0 proposal #24854
Merged
4 tasks done
@Trott Trott added the notable-change PRs with changes that should be highlighted in changelogs. label Dec 6, 2018
BridgeAR pushed a commit that referenced this pull request Dec 6, 2018
@joaocgreis @BridgeAR
win: do not use Boxstarter to install tools
9ff021f
Use Chocolatey directly in the tools installation script.

PR-URL: #24677
Fixes: #23838
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BridgeAR added a commit that referenced this pull request Dec 6, 2018
@BridgeAR
2018-12-06, Version 11.4.0 (Current)
758cb07
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    #23708
  * The inspection `depth` default is now back at 2.
    #24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    #23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    #24739
* readline:
  * The `readline` module now supports async iterators.
    #23916
* repl:
  * The multiline history feature is removed.
    #24804
* tls:
  * Added min/max protocol version options.
    #24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. #24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    #23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    #24677
  * The install-tools scripts or now included in the dist.
    #24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    #24655

PR-URL: #24854
BridgeAR pushed a commit that referenced this pull request Dec 7, 2018
@joaocgreis @BridgeAR
win: do not use Boxstarter to install tools
4d9a265
Use Chocolatey directly in the tools installation script.

PR-URL: #24677
Fixes: #23838
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BridgeAR pushed a commit that referenced this pull request Dec 7, 2018
@joaocgreis @BridgeAR
win: do not use Boxstarter to install tools
85a36b6
Use Chocolatey directly in the tools installation script.

PR-URL: #24677
Fixes: #23838
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BridgeAR added a commit that referenced this pull request Dec 7, 2018
@BridgeAR
2018-12-07, Version 11.4.0 (Current)
4f1297f
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    #23708
  * The inspection `depth` default is now back at 2.
    #24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    #23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    #24739
* readline:
  * The `readline` module now supports async iterators.
    #23916
* repl:
  * The multiline history feature is removed.
    #24804
* tls:
  * Added min/max protocol version options.
    #24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. #24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    #23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    #24677
  * The install-tools scripts or now included in the dist.
    #24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    #24655

PR-URL: #24854
BridgeAR added a commit that referenced this pull request Dec 7, 2018
@BridgeAR
2018-12-07, Version 11.4.0 (Current)
bcef949
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    #23708
  * The inspection `depth` default is now back at 2.
    #24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    #23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    #24739
* readline:
  * The `readline` module now supports async iterators.
    #23916
* repl:
  * The multiline history feature is removed.
    #24804
* tls:
  * Added min/max protocol version options.
    #24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. #24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    #23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    #24677
  * The install-tools scripts or now included in the dist.
    #24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    #24655

PR-URL: #24854
This was referenced Dec 7, 2018
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#261
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#262
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#263
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#264
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#265
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@joaocgreis @refack
win: do not use Boxstarter to install tools
feb2dec
Use Chocolatey directly in the tools installation script.

PR-URL: nodejs#24677
Fixes: nodejs#23838
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@BridgeAR @refack
2018-12-07, Version 11.4.0 (Current)
5912f6d
Notable Changes:

* console,util:
  * `console` functions now handle symbols as defined in the spec.
    nodejs#23708
  * The inspection `depth` default is now back at 2.
    nodejs#24326
* dgram,net:
  * Added ipv6Only option for `net` and `dgram`.
    nodejs#23798
* http:
  * Chosing between the http parser is now possible per runtime flag.
    nodejs#24739
* readline:
  * The `readline` module now supports async iterators.
    nodejs#23916
* repl:
  * The multiline history feature is removed.
    nodejs#24804
* tls:
  * Added min/max protocol version options.
    nodejs#24405
  * The X.509 public key info now includes the RSA bit size and the
    elliptic curve. nodejs#24358
* url:
  * `pathToFileURL()` now supports LF, CR and TAB.
    nodejs#23720
* Windows:
  * Tools are not installed using Boxstarter anymore.
    nodejs#24677
  * The install-tools scripts or now included in the dist.
    nodejs#24233
* Added new collaborator:
  * [antsmartian](https://github.com/antsmartian) - Anto Aravinth.
    nodejs#24655

PR-URL: nodejs#24854
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments
@richardlau richardlau richardlau approved these changes
+3 more reviewers
@pauby pauby pauby left review comments
@gep13 gep13 gep13 approved these changes
@bzoz bzoz bzoz approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

install Issues and PRs related to installing Node.js and its installers. notable-change PRs with changes that should be highlighted in changelogs. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Node v11.0 windows installation opting into chocolatey+boxstarter results in infinite reboots

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@joaocgreis @nodejs-github-bot @gep13 @Trott @richardlau @pauby @aduh95 @bzoz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.