Upgrade to Electron Packager 11

[malept]

Description of the Change

Hi there, I'm the maintainer of Electron Packager. I just released version 9.0.0, and thought that I could help upgrade a bunch of the projects that depend on it.

Alternate Designs

N/A

Why Should This Be In Core?

This already packages Atom 😄

Benefits

There's so many more features in 11 than 7.3.0, but the most notable ones are that version 8 added armv7l support, 9 added arm64 support, and 11 added mips64el support, so whenever you're ready to start officially supporting those platforms, Packager's ready to help!

Possible Drawbacks

As with any upgrade of a module, there might be some bugs that crept in, but hopefully this whole exercise will help find them.

Applicable Issues

None that I'm aware of.

[50Wliu]

/cc @leroix in case you're still looking at the electron-packager upgrade.

[leroix]

@50Wliu thanks, yea @malept and I spoke about this in the electron channel. Unfortunately, it didn't solve my electron 1.7 upgrade issue.

[mathuin]

I attempted to build Atom on my ARM system (Acer Chromebook 13) using this pull request and it failed at the following point:

Running electron-packager on /home/alarm/git/atom/out/app with app name "atom"
Error: Unsupported arch=arm (string); must be a string matching: ia32, x64, armv7l, arm64
  at unsupportedListOption (/home/alarm/git/atom/script/node_modules/electron-packager/targets.js:49:10)
  at Object.validateListFromOptions (/home/alarm/git/atom/script/node_modules/electron-packager/targets.js:128:16)
  at packagerPromise (/home/alarm/git/atom/script/node_modules/electron-packager/index.js:151:25)
  at packager (/home/alarm/git/atom/script/node_modules/electron-packager/index.js:179:18)
  at runPackager (/home/alarm/git/atom/script/lib/package-application.js:126:10)
  at module.exports (/home/alarm/git/atom/script/lib/package-application.js:17:10)
  at <anonymous>:null:null

uname -a: Linux alarm 3.10.18-24-ARCH #1 SMP Sun Sep 17 21:03:56 CEST 2017 armv7l GNU/Linux

[malept]

@mathuin Hmm, that should map to something. What are the values of process.arch and process.config.variables?

[mathuin]

How do I find out what those values are? I don't know much Javascript but I am willing to learn!

[50Wliu]

@malept I don't believe @mathuin will have access to those variables given that he can't install Atom.

EDIT: Well, I stand corrected :).

[malept]

@mathuin If you can run node in a terminal on your Chromebook, that'll start up the Node REPL. Just type process.arch and hit enter, then process.config.variables and hit enter.

[mathuin]

alarm@alarm:~$ node
> process.arch
'arm'
> process.config.variables
{ arm_float_abi: 'hard',
  arm_fpu: 'vfpv3-d16',
  arm_thumb: 0,
  arm_version: '7',
  asan: 0,
  coverage: false,
  debug_devtools: 'node',
  debug_http2: false,
  debug_nghttp2: false,
  force_dynamic_crt: 0,
  host_arch: 'arm',
  icu_gyp_path: 'tools/icu/icu-system.gyp',
  icu_small: false,
  node_byteorder: 'little',
  node_enable_d8: false,
  node_enable_v8_vtunejit: false,
  node_install_npm: false,
  node_module_version: 59,
  node_no_browser_globals: false,
  node_prefix: '/usr',
  node_release_urlbase: '',
  node_shared: false,
  node_shared_cares: true,
  node_shared_http_parser: true,
  node_shared_libuv: true,
  node_shared_openssl: true,
  node_shared_zlib: true,
  node_tag: '',
  node_use_bundled_v8: true,
  node_use_dtrace: false,
  node_use_etw: false,
  node_use_lttng: false,
  node_use_openssl: true,
  node_use_perfctr: false,
  node_use_v8_platform: true,
  node_without_node_options: false,
  openssl_fips: '',
  openssl_no_asm: 0,
  shlib_suffix: 'so.59',
  target_arch: 'arm',
  uv_parent_path: '/deps/uv/',
  uv_use_dtrace: false,
  v8_enable_gdbjit: 0,
  v8_enable_i18n_support: 1,
  v8_enable_inspector: 1,
  v8_no_strict_aliasing: 1,
  v8_optimized_debug: 0,
  v8_promise_internal_field_count: 1,
  v8_random_seed: 0,
  v8_trace_maps: 0,
  v8_use_snapshot: true,
  want_separate_host_toolset: 0 }
> 

[malept]

Hmmm, that should have triggered automatically setting the arch to armv7l. I'll have to go look to see what's happening there.

[mathuin]

I did some console.log() debugging on the function, and I think I understand the issue.

In targets.js there is a function validateListFromOptions which takes two arguments: opts and name. This code is called twice from packagePromise in index.js: once with name equal to arch, and once with name equal to platform. The opts object is the same in both cases, a collection of key-value pairs with two particularly interesting pairs: arch: 'arm' and platform: 'linux'.

When name is equal to arch, list gets the value arm. Because list has a value, the automatic trigger you mention above in hostArch() is never called. I do not know the language, I do not understand all the ramifications of changing this code, but it may be worth rearranging lines 105-114 in targets.js to move the name === 'arch' check before assigning opts[name] to list. When I made that change, the packaging step succeeded. I will submit a PR to your branch in a few minutes.

For what it's worth, the Atom build failed later on because electron-mksnapshot/bin/mksnapshot is an ELF 32-bit LSB shared object for Intel 80386, but that's a different matter. :-)

[malept]

I figured out why the script here didn't automatically set the arch correctly - it always set the arch itself instead of relying on Electron Packager to set it. As a workaround, I made it use the hostArch() function that Electron Packager defines instead of process.arch.

[mathuin]

I attempted to build Atom from the latest commit on this branch, and got the exact same error that I got before. I can confirm that git grep hostArch brings up the two lines added to script/lib/package-application.js in the most recent commit.

[malept]

@mathuin please run export DEBUG=electron-packager, rerun the build script, and post the output here.

[mathuin]

Running electron-packager on /home/alarm/git/atom/out/app with app name "atom"
  electron-packager Electron Packager 10.1.0 +0ms
  electron-packager Node v6.10.2 +2ms
  electron-packager Host Operating system: linux (arm) +1ms
  electron-packager Packager Options: {"appBundleId":"com.github.atom","appCopyright":"Copyright ?? 2014-2017 GitHub, Inc. All rights reserved.","appVersion":"1.25.0-dev-db4586c78","arch":"arm","asar":{"unpack":"{*.node,ctags-config,ctags-darwin,ctags-linux,ctags-win32.exe,**/node_modules/spellchecker/**,**/node_modules/dugite/git/**,**/node_modules/github/bin/**,**/resources/atom.png}"},"buildVersion":"1.25.0-dev-db4586c78","download":{"cache":"/home/alarm/git/atom/electron"},"dir":"/home/alarm/git/atom/out/app","electronVersion":"1.6.15","extendInfo":"/home/alarm/git/atom/resources/mac/atom-Info.plist","helperBundleId":"com.github.atom.helper","icon":"/home/alarm/git/atom/resources/app-icons/dev/atom","name":"atom","out":"/home/alarm/git/atom/out","overwrite":true,"platform":"linux","prune":false,"win32metadata":{"CompanyName":"GitHub, Inc.","FileDescription":"Atom","ProductName":"Atom"}} +1ms
Error: Unsupported arch=arm (string); must be a string matching: ia32, x64, armv7l, arm64
  at unsupportedListOption (/home/alarm/git/atom/script/node_modules/electron-packager/targets.js:49:10)
  at Object.validateListFromOptions (/home/alarm/git/atom/script/node_modules/electron-packager/targets.js:128:16)
  at packagerPromise (/home/alarm/git/atom/script/node_modules/electron-packager/index.js:151:25)
  at packager (/home/alarm/git/atom/script/node_modules/electron-packager/index.js:179:18)
  at runPackager (/home/alarm/git/atom/script/lib/package-application.js:128:10)
  at module.exports (/home/alarm/git/atom/script/lib/package-application.js:19:10)
  at process._tickCallback (internal/process/next_tick.js:109:7)

(malept-electron-packager-9 %=) alarm@alarm:~/git/atom$ exit

[malept]

I found something interesting about armv7l builds of Node, see electron/packager#783 and the linked Node.js issue. Not sure whether that helps the Chromebook use case though. (I ran into it when I was testing on my Raspberry Pi 3.)

That PR has been released in Electron Packager 10.1.1.

[malept]

The PR has been updated to Electron Packager 11, which has support for mips64el arches in Electron >= 1.8.2.

[50Wliu]

Superseded by #19318.