Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
tools, build: macOS installer improvements#15179
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
jpwesselink commented Sep 4, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
watilde commented Sep 4, 2017
ping @nodejs/platform-macos |
5d60f0b to 1c71c0aComparewatilde commented Sep 4, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
evanlucas left a comment
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.
Thanks for picking this up!! I'll try to run it this week and confirm that code signing still works and such as well.
lance left a comment
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.
Looks good! I noted in the code that the symlink path to npx needs to be changed. Otherwise, it's a great improvement.
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.
This should be:
ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx
jpwesselinkSep 5, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
Ok to amend those changes to same commit, or should I add new commits, and squash later?
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.
Everything will be squashed when it lands in master, so you can just add new commits.
fhemberger commented Sep 6, 2017
@jpwesselink Awesome, thanks for picking it up! Hope we can finally land a new installer this time. 😀 |
jpwesselink commented Sep 6, 2017
@fhemberger I hope we can :) New users would really benefit from it |
jasnell left a comment
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.
Gave it a try and it appears to work. Rubber stamp LGTM but I'm not an expert with macos installers so would appreciate others taking a look @nodejs/tsc
mscdex commented Sep 6, 2017
@nodejs/platform-macos is probably a better group to ping? |
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.
This script needs to return an exit code 0 to work (see docosx). Otherwise it will abort (showing "The installation failed" if there isn't a /usr/local/lib/node_modules/npm ). Can you add
exit 0 to the end of this file?
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.
Sure can
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.
Doesn't rm -f return 0 even when there was no file to remove? And when it failed to remove npm then we have an unexpected failure anyway.
joyeecheung left a comment
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.
Also a couple suggestions
joyeecheungSep 7, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
Like Node.js{nodeversion} was installed ...?
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.
Maybe something like
<div><p>This package has installed:</p><ul><li>Node.js{nodeversion} to <code>/usr/local/bin/node</code></li><li>npm{npmversion} to <code>/usr/local/bin/npm</code></li></ul><p>Make sure that <code>/usr/local/bin</code> is in your <code>$PATH</code>.</p></div>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.
TY, let me grab some screenshots of the result
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.
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.
I would suggest to use css for that because technically paragraphs can not contain lists...
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.
!
Makefile Outdated
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.
Nit: tab indentation (also the similar lines below)
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.
It would be nice if this message is formatted similar to how the conclusion message is formatted, maybe like this:
<div><p>This package will install</p><ul><li>Node.js{nodeversion} to <code>/usr/local/bin/node</code></li><li>npm{npmversion} to <code>/usr/local/bin/npm</code></li></ul></div>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.
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.
Looking good!
jpwesselink commented Sep 8, 2017
Also adjusted the vertical alignment of the logo to match the logo alignment of the current installer. |
lance commented Sep 8, 2017
Should there be a |
gibfahn commented Sep 8, 2017
Only if it's a breaking change for end-users. Which bits would be? Maybe this?
|
joyeecheung left a comment
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.
LGTM, thanks!
joyeecheung commented Sep 9, 2017
gibfahn commented Jan 3, 2018
Looks like this causes two lines to be printed on top of each other (#17965). @jpwesselink any ideas as to why this is happening? |
gibfahn commented Jan 15, 2018
Release team were +1 on backporting to v6.x. |
Creates macOS pkg installer by using `pkgbuild` and `productbuild`. Removes previous npm installation before installing npm. Packages carry correct version attributes. Support for intl installer features, defaults to `en`. Fancy formatted license. Renamed `osx` references to `macOS`. Optional installation of npm. PR-URL: #15179Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Creates macOS pkg installer by using `pkgbuild` and `productbuild`. Removes previous npm installation before installing npm. Packages carry correct version attributes. Support for intl installer features, defaults to `en`. Fancy formatted license. Renamed `osx` references to `macOS`. Optional installation of npm. PR-URL: #15179Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Creates macOS pkg installer by using `pkgbuild` and `productbuild`. Removes previous npm installation before installing npm. Packages carry correct version attributes. Support for intl installer features, defaults to `en`. Fancy formatted license. Renamed `osx` references to `macOS`. Optional installation of npm. PR-URL: #15179Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
This LTS release comes with 109 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 29 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) #12678 * crypto: - expose ECDH class (Bryan English) #8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) #10209 - warn on invalid authentication tag length (Tobias Nießen) #17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) #16835 * dgram: - added socket.setMulticastInterface() (Will Young) #7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) #13005 * lib: - return this from net.Socket.end() (Sam Roberts) #13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) #16386 * net: - return this from getConnections() (Sam Roberts) #13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) #13784 * repl: - improve require() autocompletion (Alexey Orlenko) #14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) #16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) #12087 - add process.ppid (cjihrig) #16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) #12839 * tools, build: - a new macOS installer! (JP Wesselink) #15179 * url: - WHATWG URL api support (James M Snell) #7448 * util: - add %i and %f formatting specifiers (Roman Reiss) #10308 PR-URL: #18342
Creates macOS pkg installer by using `pkgbuild` and `productbuild`. Removes previous npm installation before installing npm. Packages carry correct version attributes. Support for intl installer features, defaults to `en`. Fancy formatted license. Renamed `osx` references to `macOS`. Optional installation of npm. PR-URL: #15179Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
This LTS release comes with 109 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 29 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) #12678 * crypto: - expose ECDH class (Bryan English) #8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) #10209 - warn on invalid authentication tag length (Tobias Nießen) #17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) #16835 * dgram: - added socket.setMulticastInterface() (Will Young) #7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) #13005 * lib: - return this from net.Socket.end() (Sam Roberts) #13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) #16386 * net: - return this from getConnections() (Sam Roberts) #13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) #13784 * repl: - improve require() autocompletion (Alexey Orlenko) #14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) #16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) #12087 - add process.ppid (cjihrig) #16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) #12839 * tools, build: - a new macOS installer! (JP Wesselink) #15179 * url: - WHATWG URL api support (James M Snell) #7448 * util: - add %i and %f formatting specifiers (Roman Reiss) #10308 PR-URL: #18342
Creates macOS pkg installer by using `pkgbuild` and `productbuild`. Removes previous npm installation before installing npm. Packages carry correct version attributes. Support for intl installer features, defaults to `en`. Fancy formatted license. Renamed `osx` references to `macOS`. Optional installation of npm. PR-URL: #15179Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
This LTS release comes with 109 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 29 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) #12678 * crypto: - expose ECDH class (Bryan English) #8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) #10209 - warn on invalid authentication tag length (Tobias Nießen) #17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) #16835 * dgram: - added socket.setMulticastInterface() (Will Young) #7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) #13005 * lib: - return this from net.Socket.end() (Sam Roberts) #13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) #16386 * net: - return this from getConnections() (Sam Roberts) #13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) #13784 * repl: - improve require() autocompletion (Alexey Orlenko) #14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) #16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) #12087 - add process.ppid (cjihrig) #16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) #12839 * tools, build: - a new macOS installer! (JP Wesselink) #15179 * url: - WHATWG URL api support (James M Snell) #7448 * util: - add %i and %f formatting specifiers (Roman Reiss) #10308 PR-URL: #18342
This LTS release comes with 112 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 30 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) #12678 * crypto: - expose ECDH class (Bryan English) #8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) #10209 - warn on invalid authentication tag length (Tobias Nießen) #17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) #16835 * dgram: - added socket.setMulticastInterface() (Will Young) #7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) #13005 * lib: - return this from net.Socket.end() (Sam Roberts) #13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) #16386 * net: - return this from getConnections() (Sam Roberts) #13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) #13784 * repl: - improve require() autocompletion (Alexey Orlenko) #14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) #16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) #12087 - add process.ppid (cjihrig) #16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) #12839 * tools, build: - a new macOS installer! (JP Wesselink) #15179 * url: - WHATWG URL api support (James M Snell) #7448 * util: - add %i and %f formatting specifiers (Roman Reiss) #10308 PR-URL: #18342
This LTS release comes with 112 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 30 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) #12678 * crypto: - expose ECDH class (Bryan English) #8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) #10209 - warn on invalid authentication tag length (Tobias Nießen) #17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) #16835 * dgram: - added socket.setMulticastInterface() (Will Young) #7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) #13005 * lib: - return this from net.Socket.end() (Sam Roberts) #13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) #16386 * net: - return this from getConnections() (Sam Roberts) #13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) #13784 * repl: - improve require() autocompletion (Alexey Orlenko) #14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) #16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) #12087 - add process.ppid (cjihrig) #16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) #12839 * tools, build: - a new macOS installer! (JP Wesselink) #15179 * url: - WHATWG URL api support (James M Snell) #7448 * util: - add %i and %f formatting specifiers (Roman Reiss) #10308 PR-URL: #18342
This LTS release comes with 112 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 30 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) nodejs#12678 * crypto: - expose ECDH class (Bryan English) nodejs#8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) nodejs#10209 - warn on invalid authentication tag length (Tobias Nießen) nodejs#17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) nodejs#16835 * dgram: - added socket.setMulticastInterface() (Will Young) nodejs#7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) nodejs#13005 * lib: - return this from net.Socket.end() (Sam Roberts) nodejs#13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) nodejs#16386 * net: - return this from getConnections() (Sam Roberts) nodejs#13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) nodejs#13784 * repl: - improve require() autocompletion (Alexey Orlenko) nodejs#14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) nodejs#16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) nodejs#12087 - add process.ppid (cjihrig) nodejs#16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) nodejs#12839 * tools, build: - a new macOS installer! (JP Wesselink) nodejs#15179 * url: - WHATWG URL api support (James M Snell) nodejs#7448 * util: - add %i and %f formatting specifiers (Roman Reiss) nodejs#10308 PR-URL: nodejs#18342



Replacement for current macOS installer scripts.
This builds on prior work by @rvagg, @lance, @fhemberger & @evanlucas
pkgbuildandproductbuildinstead of outdatedPackagemaker.en.osxreferences tomacOS.Packagemaker, which is used for building the macOS packages is outdated. The last release was in 2012, as part of the Auxiliary Tools for Xcode. Next to this, it seems that
Packagemakeris poorly documented, and unwieldy to use at best.pkgbuildandproductbuildare easier to use, still supported by Apple, and (correct me if I'm wrong), the de facto standard for creating macOS installation files.The optional installation of npm also removes any previous installed npm distribution.
Package versions also propagate to the package artefacts, which will fix#15012
License is formatted using
./tools/license2rtf.js, but since the font size turns out fairly small, it might need some tweaking.Localized resources can be provided by adding them to
{lang}.lprojdirectory to./tools/macos-installer/productbuild/Resources/. As of writing the supported language is English. Please see Apple's documentation on Localized Resources in BundlesTo test packaging without code or package signing, I found that the method below worked best for me. (if there is a better way to do this, I'd love to know)
cc @nodejs/build, @jasnell, @rvagg, @MylesBorins, @watilde, @ashleygwilliams
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
tools, build