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,win: clarify WSL support #17008

Closed
wants to merge 2 commits into from
Closed

doc,win: clarify WSL support #17008

wants to merge 2 commits into from

Conversation

joaocgreis
Copy link
Member

Following the discussion in #13471, this adds a note to BUILDING.md to clarify WSL support.

cc @nodejs/platform-windows @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, win

@joaocgreis joaocgreis added doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform. labels Nov 14, 2017
@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 14, 2017
@joaocgreis
Copy link
Member Author

CI: https://ci.nodejs.org/view/All/job/node-test-pull-request/11411/

TimothyGu
TimothyGu approved these changes Nov 14, 2017
View reviewed changes
BUILDING.md Outdated
the GNU/Linux build process and binaries should work. The community will
only address issues that reproduce on native GNU/Linux systems, issues that
only reproduce on WSL should be reported in the
[WSL issue tracker](https://github.com/Microsoft/WSL/issues).
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to mention executing node.exe in WSL is not supported either.

Copy link
Contributor

@refack refack Nov 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Running the Windows binary (`node.exe`) in WSL is not supported, not recommended,
and will not work without adjustment (such as stdio standard stream redirection).

Trott
Trott reviewed Nov 14, 2017
View reviewed changes
BUILDING.md Outdated
@@ -66,6 +66,12 @@ note1 - The gcc4.8-libs package needs to be installed, because node
In "Git bash" if you call the node shell alias (`node` without the `.exe`
extension), `winpty` is used automatically.

*Note*: The Windows Subsystem for Linux (WSL) is not directly supported, but
the GNU/Linux build process and binaries should work. The community will
only address issues that reproduce on native GNU/Linux systems, issues that
Copy link
Member

Choose a reason for hiding this comment

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

Nit: systems, issues that -> systems. Issues that

Trott
Trott reviewed Nov 14, 2017
View reviewed changes
BUILDING.md Outdated
@@ -66,6 +66,12 @@ note1 - The gcc4.8-libs package needs to be installed, because node
In "Git bash" if you call the node shell alias (`node` without the `.exe`
extension), `winpty` is used automatically.

*Note*: The Windows Subsystem for Linux (WSL) is not directly supported, but
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Perhaps remove *Note*: and let the paragraph stand on its own?

@joaocgreis
Copy link
Member Author

Addressed feedback, PTAL.

@TimothyGu @refack Windows binaries should work in WSL, so saying it's not supported is too strong. We don't want to work around bugs in WSL, but if it can provide a valid Windows environment it should work. I've left the "is not recommended", should be clear enough.

joaocgreis added a commit that referenced this pull request Nov 17, 2017
@joaocgreis
doc,win: clarify WSL support
d8debd8 
Fixes: #13471
PR-URL: #17008
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@joaocgreis
Copy link
Member Author

Landed in d8debd8

@joaocgreis joaocgreis closed this Nov 17, 2017
@refack refack added the wsl Issues and PRs related to the Windows Subsystem for Linux. label Nov 17, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@joaocgreis @MylesBorins
doc,win: clarify WSL support
bccdea6 
Fixes: #13471
PR-URL: #17008
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@joaocgreis @gibfahn
doc,win: clarify WSL support
019d1a3 
Fixes: #13471
PR-URL: #17008
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017 •
edited
Loading

@nodejs/build @nodejs/platform-windows I backported this to v8.x-staging and v6.x-staging, as I think the support statement is true for LTS lines as well. If anyone disagrees LMK.

MylesBorins pushed a commit that referenced this pull request Dec 19, 2017
@joaocgreis @MylesBorins
doc,win: clarify WSL support
5cd89c7 
Fixes: #13471
PR-URL: #17008
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@bzoz
Copy link
Contributor

bzoz commented Dec 19, 2017

SGTM

@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@joaocgreis @gibfahn
doc,win: clarify WSL support
97d9be5 
Fixes: #13471
PR-URL: #17008
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
@MylesBorins MylesBorins mentioned this pull request Dec 20, 2017
Merged
@refack refack mentioned this pull request Oct 13, 2018
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack left review comments

@Trott Trott Trott left review comments

@digitalinfinity digitalinfinity digitalinfinity approved these changes

@jasnell jasnell jasnell approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@cjihrig cjihrig cjihrig approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@richardlau richardlau richardlau approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@gibfahn gibfahn gibfahn approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform. wsl Issues and PRs related to the Windows Subsystem for Linux.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.