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: updating vs cpp build tool installation steps in building #24462

Closed
wants to merge 2 commits into from

Conversation

krishnanandsivaraj
Copy link

@krishnanandsivaraj krishnanandsivaraj commented Nov 18, 2018

Checklist
  • documentation is changed or added
  • commit message follows [commit guidelines]

I have set up the source code in windows. It seems the installation steps for visual c++ build tools have changed a little recently. Previously it might be 'build tools for visual c++' installer directly available in the visual studio website. Now the steps are to download 'Build Tools for visual studio 2017' and choose 'visual c++ build tools'.

Please refer the screenshot for more information
buildtools

vs2017

This is my first commit. Let me know for any changes. Willing to contribute more in the coming days :)

Thanks
Krishnanand Sivaraj

@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. labels Nov 18, 2018
BUILDING.md Outdated Show resolved Hide resolved
@vsemozhetbyt vsemozhetbyt added the windows Issues and PRs related to the Windows platform. label Nov 18, 2018
BUILDING.md Outdated Show resolved Hide resolved
BUILDING.md Show resolved Hide resolved
* The "Desktop development with C++" workload from
[Visual Studio 2017](https://www.visualstudio.com/downloads/) or the
"Visual C++ build tools" workload from the
* The "Build Tools for Visual Studio 2017" workload from the
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll try to suggest an alternative for this whole bullet point:

[Visual Studio 2017](https://www.visualstudio.com/downloads/) or the	
[Build Tools for Visual Studio 2017](https://visualstudio.microsoft.com/downloads/#build-tools-for-visual-studio-2017).
Installed with the "Visual C++ build tools" Workload,
and if available the "Git for Windows" and "Python 2 32-bit (2.7.x)" Individual
Components.

Based on these two screens:
image
and
image

Copy link
Member

@targos targos Nov 18, 2018

Choose a reason for hiding this comment

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

Sounds good to me 👍
BTW do you know what the Node.js component adds?

Copy link
Contributor

@refack refack Nov 18, 2018

Choose a reason for hiding this comment

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

BTW do you know what the Node.js component adds?

AFAIK it installs an LTS version (with our installer and it's defaults). Apropo that, it installs python with it's installer and it's defaults, which unfortunately do not include "Add to Path". So out-of-the-box it only works with the "Developer Command Prompt for VS 2017".

But as I showed before, even if you don't select it the node binary gets in there anyway (FTR I did not start any of those explicitly) 😜
image

Copy link
Contributor

@refack refack Nov 18, 2018

Choose a reason for hiding this comment

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

There are two more
image

ping @joshgav @johnmont @AmandaSilver could you tell us what do the 4 above mentioned Node.js related components install, or please connect us with someone who can help?

P.S. the two in the screenshot and

  • Runtime for components based on Node.js v6.4.0 (x86)
  • Runtime for components based on Node.js v7.4.0 (x86)

BUILDING.md Show resolved Hide resolved
BUILDING.md Show resolved Hide resolved
@refack
Copy link
Contributor

refack commented Nov 18, 2018

Hello @krishnanandsivaraj and welcome. Thank you very much for the contribution 🥇
If you haven't already, it's recommended you take a look at CONTRIBUTING.md guide (especially the part about "discuss and update"). Customarily PRs are kept open for at least 48 hour so that anyone interested gets a chance to comment or review.
Also note that documentation changes, while easy to do are more difficult to validate. With code we can write tests and the CI can validate the change. With documentation the only way to validate is with human eyes, so don't be dismayed if you get a lot of feedback.
Anyway thank you again for the contribution, and good luck 👍.

P.S. If you have any questions you can also feel free to contact me directly.

@refack refack added the meta Issues and PRs related to the general management of the project. label Nov 18, 2018
@gireeshpunathil gireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 20, 2018
@gireeshpunathil
Copy link
Member

@krishnanandsivaraj - if you can, please see to fix the linter errors:
https://travis-ci.com/nodejs/node/jobs/159338431#L1886
https://travis-ci.com/nodejs/node/jobs/159338431#L1887
https://travis-ci.com/nodejs/node/jobs/159338431#L1888

If you want to know the reason for this failure, the contributing.md that @refack pointed out has the guidelines for commit messages which the linter enforces.

@gireeshpunathil
Copy link
Member

gireeshpunathil commented Nov 22, 2018

@krishnanandsivaraj - looks like you acknowledged my request but did not action on it. Let me clarify;

     ✖  0:0      Missing subsystem.                        subsystem
     ⚠  0:50     Title should be <= 50 columns.            title-length
     ✖  0:55     Do not use punctuation at end of title.   title-format

you could fix all of these by:
git rebase -i HEAD~5

that opens up the editor, in which you could:
reword the first commit (replace pick with reword)
fixup all other 4 commits (replace pick with fixup)

and then save and quit. the editor opens again, this time you could amend the commit message to address the three errors.

then save and quit, and git push --force-with-lease origin master

Hope this helps.

@krishnanandsivaraj
Copy link
Author

@krishnanandsivaraj - looks like you acknowledged my request but did not action on it. Let me clarify;

     ✖  0:0      Missing subsystem.                        subsystem
     ⚠  0:50     Title should be <= 50 columns.            title-length
     ✖  0:55     Do not use punctuation at end of title.   title-format

you could fix all of these by:
git rebase -i HEAD~5

that opens up the editor, in which you could:
reword the first commit (replace pick with reword)
fixup all other 4 commits (replace pick with fixup)

and then save and quit. the editor opens again, this time you could amend the commit message to address the three errors.

then save and quit, and git push --force-with-lease origin master

Hope this helps.

I am fixing it.

@krishnanandsivaraj krishnanandsivaraj changed the title docs: updating vs2017 c++ build tool installation steps in building.md doc: updating vs cpp build tool installation steps in building Nov 23, 2018
@krishnanandsivaraj
Copy link
Author

@krishnanandsivaraj - looks like you acknowledged my request but did not action on it. Let me clarify;

     ✖  0:0      Missing subsystem.                        subsystem
     ⚠  0:50     Title should be <= 50 columns.            title-length
     ✖  0:55     Do not use punctuation at end of title.   title-format

you could fix all of these by:
git rebase -i HEAD~5
that opens up the editor, in which you could:
reword the first commit (replace pick with reword)
fixup all other 4 commits (replace pick with fixup)
and then save and quit. the editor opens again, this time you could amend the commit message to address the three errors.
then save and quit, and git push --force-with-lease origin master
Hope this helps.

I am fixing it.

Okay. Fixed.

@gireeshpunathil
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/18939/

@krishnanandsivaraj
Copy link
Author

CI: https://ci.nodejs.org/job/node-test-pull-request/18939/

https://ci.nodejs.org/job/node-test-commit-freebsd/nodes=freebsd11-x64/22269/console

Would like to know more about this build failure. This is not because of my changes I hope.

20:56:32 not ok 693 parallel/test-gc-http-client-onerror 20:56:32 --- 20:56:32 duration_ms: 0.533 20:56:32 severity: fail 20:56:32 exitcode: 1 20:56:32 stack: |- 20:56:32 We should do 500 requests 20:56:32 Done: 0/500 20:56:32 Collected: 0/170 20:56:32 Done: 0/500 20:56:32 Collected: 0/350 20:56:32 /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd11-x64/test/parallel/test-gc-http-client-onerror.js:51 20:56:32 throw err; 20:56:32 ^ 20:56:32 20:56:32 Error: connect ECONNRESET 127.0.0.1:64368 - Local (127.0.0.1:64776) 20:56:32 at internalConnect (net.js:860:16) 20:56:32 at defaultTriggerAsyncIdScope (internal/async_hooks.js:294:19) 20:56:32 at GetAddrInfoReqWrap.emitLookup [as callback] (net.js:1007:9) 20:56:32 at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:63:10)

@refack
Copy link
Contributor

refack commented Nov 24, 2018

Would like to know more about this build failure. This is not because of my changes I hope.

It's a flake (#23089), that is a test that fails but only occasionally (usually because of a delicate timing issue, or a very specific state of the testing machine).
We have a list of those at https://github.com/nodejs/node/projects/8

Resume CI: https://ci.nodejs.org/job/node-test-pull-request/18944/

Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

Need to restore mention of Visual Studio 2017

@refack
Copy link
Contributor

refack commented Nov 24, 2018

FYI @krishnanandsivaraj because our policy state that a PR could land with only 1 approval after 7 days, I've used the ❌, so that this doesn't land by mistake before we finalize the wording.

@krishnanandsivaraj
Copy link
Author

FYI @krishnanandsivaraj because our policy state that a PR could land with only 1 approval after 7 days, I've used the ❌, so that this doesn't land by mistake before we finalize the wording.

Okay. I would also like to know the finalized wordings. Are these finalized?

Visual Studio 2017 or the
Build Tools for Visual Studio 2017.
Installed with the "Visual C++ build tools" Workload,
and if available the "Git for Windows" and "Python 2 32-bit (2.7.x)" Individual
Components.

If yes, I would like to try setup node again with these steps, make a commit and push it here shortly.

@refack
Copy link
Contributor

refack commented Nov 26, 2018

If yes, I would like to try setup node again with these steps, make a commit and push it here shortly.

That would be very responsible (what I should expect, but don't assume ;)
I've become aware of some caveats RE and if available the "Git for Windows" and "Python 2 32-bit (2.7.x)" Individual Components.
It seems like they are not added to the PATH by default, so verifying vcbuild.bat can find them would be very helpful 👍

@Trott
Copy link
Member

Trott commented Nov 28, 2018

@krishnanandsivaraj Do you need any more information or do you have what you need to finish this?

@krishnanandsivaraj
Copy link
Author

krishnanandsivaraj commented Nov 28, 2018

@krishnanandsivaraj Do you need any more information or do you have what you need to finish this?

I will go with @refack 's suggestions. There are two ways to install it 1) through vs2017, 2) using chocolatey. I would like to try that in my system, check if works before i commit. I will be able to progress only by this weekend. I think i have enough information :)

@krishnanandsivaraj
Copy link
Author

@krishnanandsivaraj Do you need any more information or do you have what you need to finish this?

I will go with @refack 's suggestions. There are two ways to install it 1) through vs2017, 2) using chocolatey. I would like to try that in my system, check if works before i commit. I will be able to progress only by this weekend. I think i have enough information :)

few more days. It will be done!!

@Trott Trott added the stalled Issues and PRs that are stalled. label Jul 3, 2019
@Trott
Copy link
Member

Trott commented Jul 3, 2019

I'm going to close this as stalled, but feel absolutely free to comment and/or use the Reopen button if this is still being worked on. (Same for any Collaborators that want to jump in and finish this. @refack)

@Trott Trott closed this Jul 3, 2019
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. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. stalled Issues and PRs that are stalled. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants