tools, build: macOS installer improvements #15179

jpwesselink · 2017-09-04T12:56:09Z

Replacement for current macOS installer scripts.

This builds on prior work by @rvagg, @lance, @fhemberger & @evanlucas

Uses pkgbuild and productbuild instead of outdated Packagemaker.
Optional installation of npm.
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.

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 thatPackagemaker is poorly documented, and unwieldy to use at best. pkgbuild and productbuild are 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

$ pkgutil --pkg-info org.nodejs.node.pkg
package-id: org.nodejs.node.pkg
version: v9.0.0-nightly2017-09-04c34f2eae80
volume: /
location: /
install-time: 1504526061

$ pkgutil --pkg-info org.nodejs.npm.pkg
package-id: org.nodejs.npm.pkg
version: v5.3.0
volume: /
location: /
install-time: 1504523991

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}.lproj directory to ./tools/macos-installer/productbuild/Resources/. As of writing the supported language is English. Please see Apple's documentation on Localized Resources in Bundles

To 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)

$ DISTTYPE=nightly DATESTRING=2017-09-04 COMMIT=c34f2eae80 make pkg -j 4

cc @nodejs/build, @jasnell, @rvagg, @MylesBorins, @watilde, @ashleygwilliams

Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines
Run on CI
Update build docs at https://github.com/nodejs/build/tree/master/setup/osx

Affected core subsystem(s)

tools, build

watilde · 2017-09-04T12:59:16Z

ping @nodejs/platform-macos

watilde · 2017-09-04T18:09:30Z

ci: https://ci.nodejs.org/job/node-test-pull-request/9940/
~~https://ci.nodejs.org/job/node-test-pull-request/9939/~~

evanlucas

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

Looks good! I noted in the code that the symlink path to npx needs to be changed. Otherwise, it's a great improvement.

lance · 2017-09-05T17:27:46Z

tools/macos-installer/pkgbuild/npm/scripts/postinstall

+
+cd /usr/local/bin || exit 1
+ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm
+ln -sf ../lib/node_modules/npx/bin/npx-cli.js npx


This should be:

ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx

Ok to amend those changes to same commit, or should I add new commits, and squash later?

Everything will be squashed when it lands in master, so you can just add new commits.

fhemberger · 2017-09-06T06:38:43Z

@jpwesselink Awesome, thanks for picking it up! Hope we can finally land a new installer this time. 😀

jpwesselink · 2017-09-06T07:15:54Z

@fhemberger I hope we can :) New users would really benefit from it

jasnell

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 · 2017-09-06T21:06:35Z

@nodejs/platform-macos is probably a better group to ping?

joyeecheung · 2017-09-07T05:34:15Z

tools/macos-installer/pkgbuild/npm/scripts/preinstall

+#!/bin/sh
+
+[[ -d /usr/local/lib/node_modules/npm ]] \
+  && rm -rf /usr/local/lib/node_modules/npm


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?

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

Also a couple suggestions

joyeecheung · 2017-09-07T05:45:33Z

tools/macos-installer/productbuild/Resources/en.lproj/conclusion.html.tmpl

+ </head>
+ <body>
+  <p>
+    Node.js was installed at <code>/usr/local/bin/node</code>


The message looks like this after installation:

Can you wrap each line with <p> or <li>(with <ul> or <ol>) so they would break better? Also it would be nice to include the version numbers in the message.

Like Node.js {nodeversion} was installed ...?

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>

TY, let me grab some screenshots of the result

Here we go.

I think we should wrap the unordered list in a paragraph as well to give the last line some space.

I would suggest to use css for that because technically paragraphs can not contain lists...

joyeecheung · 2017-09-07T05:45:57Z

Makefile

+	$(RM) -r $(MACOSOUTDIR)
+	mkdir -p $(MACOSOUTDIR)/installer/productbuild
+	cat tools/macos-installer/productbuild/distribution.xml.tmpl  \
+	| sed -E "s/\\{nodeversion\\}/$(FULLVERSION)/g" \


Nit: tab indentation (also the similar lines below)

joyeecheung · 2017-09-07T05:51:01Z

tools/macos-installer/productbuild/Resources/en.lproj/welcome.html.tmpl

+ </head>
+ <body>
+  <p>
+    This package will install Node.js {nodeversion} and npm {npmversion} into <code>/usr/local/</code>.


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>

How about this?

Looking good!

jpwesselink · 2017-09-08T09:41:27Z

Also adjusted the vertical alignment of the logo to match the logo alignment of the current installer.

lance · 2017-09-08T16:36:12Z

Should there be a semver-[major|minor] for a change to the installer?

gibfahn · 2017-09-08T21:27:45Z

Should there be a semver-[major|minor] for a change to the installer?

Only if it's a breaking change for end-users. Which bits would be? Maybe this?

Removes previous npm installation before installing npm.

joyeecheung

LGTM, thanks!

joyeecheung · 2017-09-09T01:33:04Z

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

gibfahn · 2018-01-03T16:25:08Z

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 · 2018-01-15T22:26:45Z

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: #15179 Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

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: #15179 Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

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: #15179 Fixes: #15012 Refs: #5656 Refs: #2571 Refs: #7097 Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

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) 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

jpwesselink force-pushed the master branch 2 times, most recently from 5d60f0b to 1c71c0a Compare September 4, 2017 13:30

evanlucas reviewed Sep 5, 2017

View reviewed changes

lance suggested changes Sep 5, 2017

View reviewed changes

jpwesselink force-pushed the master branch from 1c71c0a to 752890d Compare September 5, 2017 18:20

lance approved these changes Sep 5, 2017

View reviewed changes

watilde approved these changes Sep 5, 2017

View reviewed changes

jasnell approved these changes Sep 6, 2017

View reviewed changes

joyeecheung requested changes Sep 7, 2017

View reviewed changes

joyeecheung reviewed Sep 7, 2017

View reviewed changes

jpwesselink force-pushed the master branch from 752890d to 1e3f7c3 Compare September 8, 2017 09:18

richardlau added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 8, 2017

joyeecheung approved these changes Sep 9, 2017

View reviewed changes

joyeecheung mentioned this pull request Oct 17, 2017

Npm installation issue #16253

Closed

gibfahn added lts-watch-v6.x and removed baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v8.x labels Jan 15, 2018

MylesBorins mentioned this pull request Jan 24, 2018

v6.13.0 proposal #18342

Merged

MylesBorins mentioned this pull request May 24, 2018

deps: upgrade npm to 6.1.0 #20190

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools, build: macOS installer improvements #15179

tools, build: macOS installer improvements #15179

jpwesselink commented Sep 4, 2017 •

edited

watilde commented Sep 4, 2017

watilde commented Sep 4, 2017 •

edited

evanlucas left a comment

lance left a comment

lance Sep 5, 2017

jpwesselink Sep 5, 2017 •

edited

lance Sep 5, 2017

fhemberger commented Sep 6, 2017

jpwesselink commented Sep 6, 2017

jasnell left a comment

mscdex commented Sep 6, 2017

joyeecheung Sep 7, 2017

jpwesselink Sep 7, 2017

mgol Feb 14, 2018

joyeecheung left a comment

joyeecheung Sep 7, 2017 •

edited

jpwesselink Sep 7, 2017

joyeecheung Sep 7, 2017

jpwesselink Sep 7, 2017

jpwesselink Sep 7, 2017

joyeecheung Sep 8, 2017

jpwesselink Sep 8, 2017

joyeecheung Sep 7, 2017

joyeecheung Sep 7, 2017

jpwesselink Sep 7, 2017

joyeecheung Sep 8, 2017

jpwesselink commented Sep 8, 2017

lance commented Sep 8, 2017

gibfahn commented Sep 8, 2017

joyeecheung left a comment

joyeecheung commented Sep 9, 2017

gibfahn commented Jan 3, 2018

gibfahn commented Jan 15, 2018

tools, build: macOS installer improvements #15179

tools, build: macOS installer improvements #15179

Conversation

jpwesselink commented Sep 4, 2017 • edited

Checklist

Affected core subsystem(s)

watilde commented Sep 4, 2017

watilde commented Sep 4, 2017 • edited

evanlucas left a comment

Choose a reason for hiding this comment

lance left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpwesselink Sep 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhemberger commented Sep 6, 2017

jpwesselink commented Sep 6, 2017

jasnell left a comment

Choose a reason for hiding this comment

mscdex commented Sep 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joyeecheung left a comment

Choose a reason for hiding this comment

joyeecheung Sep 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpwesselink commented Sep 8, 2017

lance commented Sep 8, 2017

gibfahn commented Sep 8, 2017

joyeecheung left a comment

Choose a reason for hiding this comment

joyeecheung commented Sep 9, 2017

gibfahn commented Jan 3, 2018

gibfahn commented Jan 15, 2018

jpwesselink commented Sep 4, 2017 •

edited

watilde commented Sep 4, 2017 •

edited

jpwesselink Sep 5, 2017 •

edited

joyeecheung Sep 7, 2017 •

edited