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

Windows post-install screen additional build tools messaging #4111

Closed
wants to merge 4 commits into from
Closed

Windows post-install screen additional build tools messaging #4111

wants to merge 4 commits into from

Conversation

kaylynb
Copy link

@kaylynb kaylynb commented Dec 2, 2015

This is a concept for #3694.

installmsg

It looks like the standard way to customize the final install dialog in WiX is to fork the built-in dialog from the WiX source and modify it with your custom fields (in this case, the Hyperlink control). This could add some maintenance headaches, so further discussion around messaging placement may be needed.

There are a few related issues as well:

@rvagg
Copy link
Member

rvagg commented Dec 2, 2015

This is fantastic! Great work @kaylynb. The documentation part is interesting, perhaps we should point to a Wiki page maintained at this repo and combine parts of the README with what's already in https://github.com/TooTallNate/node-gyp. The Microsoft page goes too far and our README isn't exactly suitable because it needs to be focused only on add-on compilation and nothing else.

@mscdex mscdex added windows Issues and PRs related to the Windows platform. install Issues and PRs related to the installers. labels Dec 2, 2015
@joaocgreis
Copy link
Member

This is very good indeed! @kaylynb Did you try the instructions under "Customizing the ExitDlg" in http://wixtoolset.org/documentation/manual/v3/wixui/wixui_customizations.html ? I don't know if it can display links, but might be worth trying.

@rvagg is there a way to have a redirect page somewhere that we control? That way we would never need to worry about dead links in already published installers. I think we can have the link point further down in that page, to https://github.com/Microsoft/nodejs-guidelines/blob/master/windows-environment.md#environment-setup-and-configuration . If we end up creating a wiki page, it would still be good to point to the nodejs-guidelines repo.

@kaylynb
Copy link
Author

kaylynb commented Dec 2, 2015

@joaocgreis I did try that, but the link won't display properly if it's not in a hyperlink block:

installfail

I'm not an expert with WiX or anything, so there may be another way.

@rvagg
Copy link
Member

rvagg commented Dec 2, 2015

@joaocgreis absolutely, that's a good idea. We could pin http://nodejs.org/windows-developmen-environment always be pointing to a useful resource.

@jasnell
Copy link
Member

jasnell commented Dec 3, 2015

Not very knowledgeable of Windows installer stuff so not going to LGTM this but +1 on having this!

@mcollina
Copy link
Member

I would put some more emphasis on the fact that user need to install a the build tools. A lot of people do not read those sentences.

To do this, I think we should also add a "UI" version of https://github.com/workshopper/goingnative/tree/master/exercises/am_i_ready to check everything is ok, and have it available also as a separate entry when the node installer starts.

@mcollina
Copy link
Member

To be clear, this is amazing work.
However I imagine myself on a nodeschool event "have you followed the instructions?", with an answer "what instructions?".

@kaylynb
Copy link
Author

kaylynb commented Dec 14, 2015

@mcollina I'm not sure if adding more messaging in the installer will help much. I think lots of people probably just mindlessly click next until it goes away. If people won't read a blurb after installation, another page or more detailed explanation may not help.

Personally, I'm already 50/50 on this modification since it's deviating from the default WiX installer path, and making sure everything works is kind of challenging to test (see all the windows/installer tagged issues).

@mcollina
Copy link
Member

@kaylynb I'm not sure either. This is a user experience problem, and tech should follow a proposed solution to fix it. IMHO, the process start "bad" from the download link on the main website: users download that blob and they expect it to contain everything they need. This is unfortunately not true.

I'm 👍 on the instructions here. At least we are displaying a little warning. Another option is "block" installation if a compiler and python are not available (maybe with a "force" option, but...).

@tracker1
Copy link

If it were easier to include VS2015CE, and Python 2.x installers launched from the wix installer, that could be checkbox items, that may be best (unchecked by default) with a screen saying,

Install Additional Tools
May be required for some binary modules

[ ] Visual Studio 2015 Community Edition
[ ] Python 2...

And only show if suitable tools aren't detected (per @mcollina 's comment/url above), and download/launch the online installers if checked.

@mcollina
Copy link
Member

That would be the best course of action, if it is feasible.
Il giorno mar 15 dic 2015 alle 11:55 Michael J. Ryan <
notifications@github.com> ha scritto:

If it were easier to include VS2015CE, and Python 2.x installers launched
from the wix installer, that could be checkbox items, that may be best
(unchecked by default) with a screen saying,

Install Additional Tools
May be required for some binary modules

[ ] Visual Studio 2015 Community Edition
[ ] Python 2...

And only show if suitable tools aren't detected (per @mcollina
https://github.com/mcollina 's comment/url above), and download/launch
the online installers if checked.


Reply to this email directly or view it on GitHub
#4111 (comment).

@rvagg
Copy link
Member

rvagg commented Dec 16, 2015

we probably should be careful over-burdening on requirements here, perhaps work on the simplest thing first and move on from there?

joaocgreis added a commit to JaneaSystems/build that referenced this pull request Dec 23, 2015
Redirect pointing to the wiki page, to be used by the Windows installer.

Ref: nodejs/node#4111
joaocgreis added a commit to JaneaSystems/build that referenced this pull request Dec 24, 2015
Redirect pointing to the wiki page, to be used by the Windows installer.

Ref: nodejs/node#4111
joaocgreis added a commit to JaneaSystems/build that referenced this pull request Jan 6, 2016
Redirect pointing to the wiki page, to be used by the Windows installer.

Ref: nodejs/node#4111

PR-URL: nodejs#286
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
@joaocgreis
Copy link
Member

@kaylynb can you update the link to point to https://nodejs.org/windows-environment ?

@fhemberger can you give us the German translation? It would be good to include it in the same commit.

@mcollina
Copy link
Member

@joaocgreis that page returns a 404 currently, I would love to review the content.

@joaocgreis
Copy link
Member

@mcollina sorry for not being clear. The server should be updated soon (ref nodejs/build#286), and the page will redirect to https://github.com/nodejs/node/wiki/Windows-Environment . I filled it with a simple initial version, straight to the point, and a link for the Node.js Guidelines repo. The idea is enabling people to make that page better, so feel free to review it!

@rvagg
Copy link
Member

rvagg commented Jan 12, 2016

sorry for the hold-up, https://nodejs.org/windows-environment is active now!

@@ -35,4 +35,6 @@

<!-- References like [ProductName] are not resolved for Property tags -->
<String Id="WIXUI_EXITDIALOGOPTIONALTEXT">Node.js has been successfully installed.</String>

<String Id="InstalledNativeCompileInfo">Some native modules may require additional build tools to be installed. See the <![CDATA[<a href="https://github.com/Microsoft/nodejs-guidelines/blob/master/windows-environment.md#configuring-your-windows-development-environment">Windows Native Development Guide</a>]]> for more information.</String>
Copy link
Contributor

Choose a reason for hiding this comment

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

German translation:

<String Id="InstalledNativeCompileInfo">Einige native Module benötigen eventuell zusätzliche Build-Tools um installiert werden zu können. Weitere Informationen finden Sie im <![CDATA[<a href="https://github.com/Microsoft/nodejs-guidelines/blob/master/windows-environment.md#configuring-your-windows-development-environment">Windows Native Development Guide</a>]]>.</String>

/cc @nodejs/nodejs-de

@kaylynb
Copy link
Author

kaylynb commented Jan 13, 2016

Added the new link destination to strings. Also the German translation.

Since the new page doesn't have the same name as the Microsoft guide, I did reword the english a bit. Don't know if that's desired or if the German needs updating too.

I won't be able to build the installer and try it out for a few days as I just setup my new workstation and don't have KVM setup yet.

@kaylynb
Copy link
Author

kaylynb commented Jan 16, 2016

New text:

english

Does anybody know how to get German to show up in the installer? I tried setting my system and user locales to de-DE, but the installer still used the English strings. I couldn't even get the official 5.4.1 msi to show localized strings. WiX seems to indicate that a seperate installer may be needed?

@fhemberger
Copy link
Contributor

@kaylynb Yes, just uncomment this section: https://github.com/kaylynb/node/blob/AddMSINativeModuleDescription/tools/msvs/msi/nodemsi.wixproj#L79

The German installer has been prepared but hasn't been enabled for builds so far, because at that time we had a release waiting to be pushed.

@mousetraps
Copy link

Hey, just noticed this thread - awesome to see this work! Regarding the wiki page content, I've just submitted a PR to the node-gyp README (nodejs/node-gyp#867) that includes more complete install instructions (for both the VC++ Build Tools & VS2015 since they can't be installed side-by-side) that we may want to update the wiki page with. Thoughts?

@mcollina
Copy link
Member

@mousetraps I think those instructions are great and highly needed, thanks :).

@rvagg
Copy link
Member

rvagg commented Jan 18, 2016

@mousetraps thanks for working on that! One of us will review it properly as soon as possible. It might be good to try and unify the instructions somewhat, feel free to treat the wiki as a wiki and edit it where you think it needs it and perhaps update your PR to redirect people to there so we don't repeat ourselves. Great to have people focusing on Windows users btw, not enough of us are comfortable enough in Windows to look after the swathes of people who have trouble with this stuff.

@joaocgreis
Copy link
Member

I did a more careful review (hoping to get this landed soon) but 2 problems popped up:

@MylesBorins
Copy link
Contributor

@joaocgreis what is still blocking this?

@joaocgreis
Copy link
Member

@thealphanerd just the copyright notice. I'm not sure if it can simply be removed, or if we have to add our own, or what needs to be done. I don't know if there's anyone else that can be mentioned for this that's not already here.

The Hyperlink control is not a problem if this lands on version 6.

@estliberitas estliberitas force-pushed the master branch 2 times, most recently from 7da4fd4 to c7066fb Compare April 26, 2016 05:22
@jasnell jasnell added the blocked PRs that are blocked by other issues or PRs. label Apr 30, 2016
@Trott
Copy link
Member

Trott commented Jun 8, 2016

Is there any way to get the copyright question resolved? Or is this possibly in limbo indefinitely?

There is a copyright notice at the top of NodeExitDialog.wxs. I'm guessing this is because this file is an adaptation of https://github.com/wixtoolset/wix3/blob/master/src/ext/UIExtension/wixlib/ExitDialog.wxs . I don't know how much of an issue is this, but at least the LICENCE.TXT file mentioned is a different thing in this repository. @mikeal do you know what should be done with this or who should we ask? Thanks!

@orangemocha
Copy link
Contributor

@mikeal @nodejs/tsc : do we have any resources at the Foundation for answering licensing questions?

@rvagg
Copy link
Member

rvagg commented Jul 18, 2016

Given the inability of the Legal Committee to get back to the TSC on the questions we asked it many months ago I'd say the answer is no. I'm pretty sure that any staff of the NF or LF are not going to be willing to express an official opinion on anything for us without going through the Legal Committee and it's not really in a position to make quick enough progress on anything we need.

@jasnell
Copy link
Member

jasnell commented Jul 20, 2016

@rvagg ... this is definitely a problem that needs to be raised and discussed at the next board meeting. A clear channel of communication with the legal committee on licensing issues is something the foundation must provide.

@jasnell
Copy link
Member

jasnell commented Feb 28, 2017

Recommend closing given the lack of forward progress on this.

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Mar 1, 2017
@tracker1
Copy link

tracker1 commented Mar 1, 2017

In the interim, would it be possible to have a short url that redirects (ex: https://nodejs.org/win) or similar? So it can be typed in... also, if this is already in place, apologies, I haven't installed on windows in well over a year (been using nvm-for-windows).

@fhinkel
Copy link
Member

fhinkel commented Mar 26, 2017

Feel free to reopen this PR if you get back to working on it.

@fhinkel fhinkel closed this Mar 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. install Issues and PRs related to the installers. 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.