upgrade(package): Bump electron-builder 19.9.1 -> 19.40.0 #1905

jhermsmeier · 2017-12-05T17:08:42Z

Change-Type: patch
Connects To: #1914

jhermsmeier · 2017-12-05T17:17:51Z

This currently fails for deb packages due to

{:timestamp=>"2017-12-05T17:06:10.361199+0000", :message=>"Process failed: tar failed (exit code 2). Full command was:[\"tar\", \"-C\", \"/tmp/package-dir-staging-8abc4590f5fcbb8ef993da8fef97ba2aa9169d2c54a24f906c4da425a75a\", \"-I'/etcher/node_modules/7zip-bin/7x.sh'\", \"--numeric-owner\", \"--owner\", \"0\", \"--numeric-owner\", \"--group\", \"0\", \"-cf\", \"/tmp/package-deb-build-f194c570b6596c408446b22df37210ba51e03b9d6a4b2d491ca71786153b/data.tar.xz\", \".\"]", :level=>:error}

lurch · 2017-12-05T17:50:32Z

This introduces a massive chunk of changes to npm-shrinkwrap.json - perhaps I'm being overly conservative, but does that mean we ought to do a bit of extra testing before release, just to make sure this doesn't accidentally break anything?

jhermsmeier · 2017-12-05T17:55:05Z

This isn't actually even working for deb packages atm; and I haven't gotten it to work so far, let alone figured out what exactly is causing fpm to explode

lurch · 2017-12-06T13:51:17Z

scripts/build/docker/Dockerfile.template

@@ -12,6 +12,10 @@ RUN echo "deb http://archive.ubuntu.com/ubuntu precise-backports main restricted
 RUN echo "deb http://ftp.debian.org/debian jessie-backports main" >> /etc/apt/sources.list
 <% } %>

+# Workaround: Install a newer version of `tar` to make fpm in electron-builder work again
+RUN echo "deb http://ftp.de.debian.org/debian wheezy main contrib non-free" >> /etc/apt/sources.list


Should we just use http://ftp.debian.org/ as in the previous lines, rather than selecting the de-specific subdomain?

lurch · 2017-12-06T13:56:08Z

scripts/build/docker/Dockerfile.template

@@ -43,6 +47,10 @@ RUN apt-get update \
    zip \
    rpm

+# Workaround: Install a newer version of `tar` to make fpm in electron-builder work again


IMHO these two 'Workaround' blocks should only be run in the architecture == 'i686' and architecture == 'x86_64' Dockerfiles - they're not needed in the architecture == 'armv7hf' Dockerfile as that uses a newer base-image and so the workaroud would actually downgrade the version of tar.

Something like

<% if (architecture == 'i686' || architecture == 'x86_64') { %> ... <% } %>

should do.

jviotti · 2017-12-06T14:02:25Z

scripts/build/docker/Dockerfile.template

@@ -14,7 +14,7 @@ RUN echo "deb http://ftp.debian.org/debian jessie-backports main" >> /etc/apt/so

 # Install dependencies
 RUN apt-get update \
-  && apt-get install -y \
+  && apt-get install --force-yes -y \


Is this really needed at this point?

Nope not anymore, removed

jviotti · 2017-12-06T14:03:30Z

scripts/build/docker/Dockerfile.template

@@ -43,6 +43,12 @@ RUN apt-get update \
    zip \
    rpm

+# Workaround: Install a newer version of `tar` to make fpm in electron-builder work again
+RUN echo "deb http://ftp.de.debian.org/debian wheezy main contrib non-free" >> /etc/apt/sources.list


I guess we don't need the non-free. We can also try to remove contrib

Yeah, libacl1 is on main

lurch · 2017-12-06T14:08:33Z

Is the new assets/icons/256x256.png just a duplicate of the existing assets/icon.png? If that's the case, perhaps a neater solution would be to remove this new file, and instead add:

assets/icons/256x256.png: assets/icon.png
    cp $< $@

to the Makefile, and then list assets/icons/256x256.png as a dependency of those electron-builder rules that actually need it? (and that file should also then be deleted by the clean rule, and be added to .gitignore - and in fact these last two steps should probably be added for assets/osx/installer.tiff too)

EDIT: Hah, I guess the latest commit nullifies this comment ;-)

lurch · 2017-12-06T14:19:08Z

scripts/build/docker/Dockerfile.template

+# Workaround: Install a newer version of `tar` to make fpm in electron-builder work again
+RUN echo "deb http://ftp.debian.org/debian wheezy main" >> /etc/apt/sources.list
+RUN echo "deb http://ftp.debian.org/debian wheezy-backports main" >> /etc/apt/sources.list
+RUN apt-get install --force-yes -y -t wheezy libacl1 && \


Ah, you need an apt-get update between modifying sources.list and running apt-get install

...and perhaps this should be done in a single-step RUN command (just like the compiler-install step below), rather than as separate RUN commands?

I was thinking more:

RUN echo -e "deb http://ftp.debian.org/debian wheezy main\ndeb http://ftp.debian.org/debian wheezy-backports main" >> /etc/apt/sources.list && \ apt-get update && apt-get install --force-yes -y -t wheezy libacl1 && \ apt-get install --force-yes -y -t wheezy-backports tar

But it probably makes little difference here.

lurch · 2017-12-06T15:00:29Z

Is it worth adding a note to the commit message about why we've put so much effort into upgrading electron-builder? (IIRC it's so that the hdiutil still works on the latest versions of OSX?)

jviotti · 2017-12-15T21:49:23Z

@jhermsmeier Even though DMGs build fine, I get weird ABI issues when running this particular electron-builder version (and Concourse gets them as well). I started bisecting it a bit, and found that 19.40.0 works like a charm. Can we upgrade to that instead?

lurch · 2017-12-16T01:49:19Z

@jviotti out of curiosity, did you also try 19.49.0 ?

EDIT: Now that we've "solved" npm-shrinkwrapping, I wonder if electron-builder is going to become our new nemesis? ;-)

jviotti · 2017-12-18T15:58:05Z

I'm getting the same weird ABI error on 19.49.0.

…

On Fri, Dec 15, 2017 at 05:49:21PM -0800, Andrew Scheller wrote: @jviotti out of curiosity, did you also try `19.49.0` ? -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #1905 (comment)

-- Juan Cruz Viotti Software Engineer

lurch · 2017-12-18T16:19:06Z

Have you filed an upstream bug? ;)

jhermsmeier · 2017-12-19T14:32:34Z

@jviotti can you post the relevant log of that ABI error?

jviotti · 2017-12-19T15:27:31Z

@jhermsmeier The CI services are showing the exact error I get: https://travis-ci.org/resin-io/etcher/jobs/312464338#L2132

jviotti · 2017-12-19T15:34:35Z

@lurch See electron-userland/electron-builder#2260. Apparently the solution is:

To avoid such errors in the future, we will get rid of NodeJS and this logic will reimplemented using Go.

jhermsmeier · 2017-12-19T15:43:05Z

Oh, I see – that's because the module's been compiled against Electron's ABI, while being invoked by Node during the build step. One way to get around this would be to use the Node version corresponding to Electron's Node (v7.9.0 in Electron v1.7.9), but that's not something we want to do I think (our build process should ideally be fairly independent of Node version). I'll downgrade electron-builder to 19.40.0 for now then.

Change-Type: patch

lurch · 2017-12-19T16:35:52Z

See electron-userland/electron-builder#2260

...which says "The rabin dependency was added as part of commit 1dc2e49, so 19.36.0 onward exhibit this issue. (19.35.1 works)" ??

jviotti · 2017-12-19T17:29:10Z

@lurch Yeah, but that version hangs on High Sierra with 100% CPU usage when building the DMG :( 19.40.0 works like a charm from all sides. I say lets upgrade to that, and let electron-builder sort out the ABI problem. We will try again once they have a fix. @jhermsmeier what do you think? Lets try to get this merged soon so I can integrate Concourse without hanging all the jobs

jviotti · 2017-12-19T17:29:26Z

I'll downgrade electron-builder to 19.40.0 for now then.

Ah, I missed that comment :D

jhermsmeier requested review from Shou and jviotti December 5, 2017 17:08

jviotti approved these changes Dec 5, 2017

View reviewed changes

jhermsmeier force-pushed the bump-electron-builder branch from 48623d9 to c94dd10 Compare December 6, 2017 13:25

lurch reviewed Dec 6, 2017

View reviewed changes

jhermsmeier force-pushed the bump-electron-builder branch from c94dd10 to 918f7eb Compare December 6, 2017 14:00

jviotti reviewed Dec 6, 2017

View reviewed changes

jhermsmeier force-pushed the bump-electron-builder branch from 918f7eb to de6849d Compare December 6, 2017 14:09

lurch reviewed Dec 6, 2017

View reviewed changes

jhermsmeier added this to the v1.2.1 milestone Dec 6, 2017

jhermsmeier mentioned this pull request Dec 6, 2017

Release v1.2.1 #1906

Merged

14 tasks

jhermsmeier removed this from the v1.2.1 milestone Dec 6, 2017

jhermsmeier mentioned this pull request Dec 11, 2017

Categories= key missing in Linux desktop file #1914

Closed

jhermsmeier added 3 commits December 19, 2017 16:43

upgrade(package): Bump electron-builder 19.9.1 -> 19.47.1

d769f7e

Change-Type: patch

refactor(electron-builder): Update configuration & icon asset

5c9a222

Change-Type: patch

fix(dockerfile): Add tar/fpm/electron-builder workaround

837054c

Change-Type: patch

jhermsmeier added 5 commits December 19, 2017 16:43

fix(electron-builder): Add full icon set for Linux

4174acc

fix(dockerfile): Add missing apt update

3a61420

fix(dockerfile): Re-order wheezy workaround

2e31028

fix(package): Fix shrinkwrap file

c35a214

fix(package): Downgrade to electron-builder@19.40.0

64a5ab2

Change-Type: patch

jhermsmeier force-pushed the bump-electron-builder branch from ced6036 to 64a5ab2 Compare December 19, 2017 15:56

jviotti changed the title ~~upgrade(package): Bump electron-builder 19.9.1 -> 19.47.1~~ upgrade(package): Bump electron-builder 19.9.1 -> 19.40.0 Dec 19, 2017

jviotti merged commit abfa44a into master Dec 19, 2017

jviotti deleted the bump-electron-builder branch December 19, 2017 17:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade(package): Bump electron-builder 19.9.1 -> 19.40.0 #1905

upgrade(package): Bump electron-builder 19.9.1 -> 19.40.0 #1905

jhermsmeier commented Dec 5, 2017 •

edited

Loading

jhermsmeier commented Dec 5, 2017

lurch commented Dec 5, 2017

jhermsmeier commented Dec 5, 2017 •

edited

Loading

lurch Dec 6, 2017

jhermsmeier Dec 6, 2017

lurch Dec 6, 2017

jviotti Dec 6, 2017

jhermsmeier Dec 6, 2017

jviotti Dec 6, 2017

jhermsmeier Dec 6, 2017

jviotti Dec 6, 2017

jviotti Dec 6, 2017

jhermsmeier Dec 6, 2017

lurch commented Dec 6, 2017 •

edited

Loading

lurch Dec 6, 2017 •

edited

Loading

lurch Dec 6, 2017

jhermsmeier Dec 6, 2017

lurch Dec 6, 2017

lurch commented Dec 6, 2017

jviotti commented Dec 15, 2017 •

edited

Loading

lurch commented Dec 16, 2017 •

edited

Loading

jviotti commented Dec 18, 2017 via email

lurch commented Dec 18, 2017

jhermsmeier commented Dec 19, 2017

jviotti commented Dec 19, 2017

jviotti commented Dec 19, 2017

jhermsmeier commented Dec 19, 2017

lurch commented Dec 19, 2017

jviotti commented Dec 19, 2017

jviotti commented Dec 19, 2017

upgrade(package): Bump electron-builder 19.9.1 -> 19.40.0 #1905

upgrade(package): Bump electron-builder 19.9.1 -> 19.40.0 #1905

Conversation

jhermsmeier commented Dec 5, 2017 • edited Loading

jhermsmeier commented Dec 5, 2017

lurch commented Dec 5, 2017

jhermsmeier commented Dec 5, 2017 • edited Loading

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

lurch commented Dec 6, 2017 • edited Loading

lurch Dec 6, 2017 • edited Loading

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

lurch commented Dec 6, 2017

jviotti commented Dec 15, 2017 • edited Loading

lurch commented Dec 16, 2017 • edited Loading

jviotti commented Dec 18, 2017 via email

lurch commented Dec 18, 2017

jhermsmeier commented Dec 19, 2017

jviotti commented Dec 19, 2017

jviotti commented Dec 19, 2017

jhermsmeier commented Dec 19, 2017

lurch commented Dec 19, 2017

jviotti commented Dec 19, 2017

jviotti commented Dec 19, 2017

jhermsmeier commented Dec 5, 2017 •

edited

Loading

jhermsmeier commented Dec 5, 2017 •

edited

Loading

lurch commented Dec 6, 2017 •

edited

Loading

lurch Dec 6, 2017 •

edited

Loading

jviotti commented Dec 15, 2017 •

edited

Loading

lurch commented Dec 16, 2017 •

edited

Loading