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

docker builder: Fix build: bump version of wine package, switch to OpenJDK, add babel-preset-es2015 dev dependency #84

Closed
wants to merge 4 commits into from

Conversation

hkjn
Copy link
Contributor

@hkjn hkjn commented Jun 1, 2019

Not sure what changed, but without this patch, the build fails with:

The following packages have unmet dependencies:
 winehq-stable : Depends: wine-stable (= 4.0~stretch)
E: Unable to correct problems, you have held broken packages.

With this patch, the build gets further.

(And then actually still fails for me, with another error, included below, although I still think this patch itself should be an improvement.)

further error installing oracle-java
$ sudo docker build -f scripts/builder.Dockerfile -t spark-builder .
[...]
Get:11 https://dl.winehq.org/wine-builds/debian stretch/main amd64 Packages [430 kB]
Fetched 2076 kB in 2s (865 kB/s)
Reading package lists...
Reading package lists...
Building dependency tree...
Reading state information...
E: Version '8u201-1~webupd8~1' for 'oracle-java8-installer' was not found
The command '/bin/sh -c add-apt-repository "deb http://ppa.launchpad.net/webupd8team/java/ubuntu xenial main" -y   && apt-key adv --keyserver hkp://keyserver.ubuntu.com --recv-keys C2518248EEA14886   && apt-get update   && echo oracle-java8-installer shared/accepted-oracle-license-v1-1 select true | debconf-set-selections   && mkdir -p /usr/share/man/man1   && apt-get install -y --no-install-recommends oracle-java8-installer=8u201-1~webupd8~1' returned a non-zero code: 100

Edit: With b1d685c we also switch to OpenJDK in this PR.

Fixes #79.

@hkjn
Copy link
Contributor Author

hkjn commented Jun 1, 2019

Yeah, Travis CI hits the same error installing oracle-java as I saw locally:

E: Version '8u201-1~webupd8~1' for 'oracle-java8-installer' was not found

(I.e it does get past the wine issues seen without this patch.)

I tried bumping the version to oracle-java8-installer=8u211-1~webupd8~1 which seems via Oracle's website like it should be the latest java8 version, but hit the same error.

I'm not sure about this, but a quick search showed that the PPA might be discontinued:

I couldn't find a reliable way to install Oracle's java8 in the few minutes I had available. In my experience, using OpenJDK is certainly easier unless there's specific reasons why Oracle's Java is required. If we do need to use Oracle's Java, the most maintainable way I've found is fetching the tarball directly from Oracle's servers.

I wonder what changed to cause the wine + Oracle Java breakages?

@shesek
Copy link
Owner

shesek commented Jun 1, 2019

Yes, I am aware of the issue with Java (#79), the PPA is indeed discontinued. I also couldn't find a quick alternative solution when I looked into it. But I'll have to figure this out soon, this is preventing me from making new android releases. :-\

Not sure what's the issue with Wine, I'll look into that. Sometimes the version number has to be bumped, because they're making older versions unavailable.

@hkjn hkjn changed the title docker builder: Add more wine packages to attempt to fix build docker builder: Add more wine packages and switch to OpenJDK to attempt to fix build Jun 3, 2019
@hkjn
Copy link
Contributor Author

hkjn commented Jun 3, 2019

Yes, I am aware of the issue with Java (#79), the PPA is indeed discontinued. I also couldn't find a quick alternative solution when I looked into it. But I'll have to figure this out soon, this is preventing me from making new android releases. :-\

Okay, thanks for confirming. I couldn't find any easy way (and license-respecting) way of continuing to use Oracle's Java, but switching to OpenJDK was trivial, so I did that in b1d685c.

Note that I haven't really tested b1d685c, but it does build succcessfully (with sudo docker build -f scripts/builder.Dockerfile -t spark-builder ..) Let me know how I can verify that I didn't end up breaking anything, but since the build works with these patches maybe it still makes sense to merge after some light testing and then fix forward in case there's any issues due to the OpenJDK switch.

Not sure what's the issue with Wine, I'll look into that. Sometimes the version number has to be bumped, because they're making older versions unavailable.

Okay, let me know if you'd prefer a separate way of addressing that issue. I just added the packages that apt claimed were necessary in ef9ee07 until the installation completed successfully.

@hkjn
Copy link
Contributor Author

hkjn commented Jun 3, 2019

I noticed that the Travis build was failing with:

The following packages have unmet dependencies:
 openjdk-8-jdk-headless : Depends: openjdk-8-jre-headless (= 8u212-b01-1~deb9u1) but 8u212-b03-2~deb9u1 is to be installed

Re-running the build locally (bypassing the Docker cache) reproduces the issue, so it seems package versions changed between the time I built locally and when Travis did its build. I've switched to
openjdk-8-jdk-headless=8u212-b03-2~deb9u1 (earlier 8u212-b01-1~deb9u1) at this time.

@hkjn
Copy link
Contributor Author

hkjn commented Jun 3, 2019

The next failure was:

+ uglifyjs --compress --mangle
Error: Couldn't find preset "es2015" relative to directory "/opt/spark/node_modules/estraverse" while parsing file: /opt/spark/node_modules/estraverse/estraverse.js
    at /opt/spark/node_modules/babel-core/lib/transformation/file/options/option-manager.js:293:19
    at Array.map (<anonymous>)
    at OptionManager.resolvePresets (/opt/spark/node_modules/babel-core/lib/transformation/file/options/option-manager.js:275:20)
    at OptionManager.mergePresets (/opt/spark/node_modules/babel-core/lib/transformation/file/options/option-manager.js:264:10)
    at OptionManager.mergeOptions (/opt/spark/node_modules/babel-core/lib/transformation/file/options/option-manager.js:249:14)
    at OptionManager.init (/opt/spark/node_modules/babel-core/lib/transformation/file/options/option-manager.js:368:12)
    at File.initOptions (/opt/spark/node_modules/babel-core/lib/transformation/file/index.js:212:65)
    at new File (/opt/spark/node_modules/babel-core/lib/transformation/file/index.js:135:24)
    at Pipeline.transform (/opt/spark/node_modules/babel-core/lib/transformation/pipeline.js:46:16)
    at Babelify._flush (/opt/spark/electron/node_modules/babelify/index.js:26:24)

The issue persisted after force-pushing a change to this branch.

I found a stackoverflow.com post with similar symptoms, and tried the suggested steps in that answer:

$ npm install babel-cli babel-preset-es2015

That actually did lead to a passing build, based on this branch with another commit hkjn@09bb7f9, which adds the changes produced by the npm install command above:

-----BEGIN SHA256SUM-----
32d8269b451f9691bf3518fcc408c3cb3a496e92682451b32396d976764c20f8  spark-wallet-0.2.5-npm.tgz
a53b7c233f58a31928e0b4c072f8928c9f0620002ab63de7a3791f21b3806031  spark-wallet-0.2.5-linux-x86_64.AppImage
d7640f3ed5b4f07ae768a2d1659fec531171a18a99ec3226a42ed7e105088462  spark-wallet-0.2.5-linux-amd64.deb
7b2d82c4701d2ae2442ade1884bbe8b57ef9179d2245134fc54ed0fec3a0eb80  spark-wallet-0.2.5-linux-amd64.snap
83eabf846fda82d4b74b7e78f1e9d9e1a21eb5c020bbe151b769b695aafc4246  spark-wallet-0.2.5-linux-x64.tar.gz
1e0df0bfc03520f3369940e424d95844c815d8d1cb15785bab93ee71c1689ab7  spark-wallet-0.2.5-win-portable.exe
c9117909df25d83fecf48a35cee48808cb2b02eefc6d68fadda7ac3c11213196  spark-wallet-0.2.5-win-setup.exe
47af0a44abc0bacc9ddf33ceae9d6735d62d07e57e2889f59d6658df87fb472d  spark-wallet-0.2.5-mac.zip
5576101159c599c2591e02cd0d5b131714ddab150fe8cbecf5e053e24aad78a4  spark-wallet-0.2.5-android-debug.apk
The command "docker run --cap-add SYS_ADMIN --device /dev/fuse spark-builder" exited with 0.

I'm not sure if I want to add that commit to this PR, since it also results in a large diff to npm-shrinkwrap.json. I'm not particularly familiar with modern JS tools and can't claim to understand what broke or how that commit fixes things.

@shesek: Thoughts / suggestions?

@hkjn
Copy link
Contributor Author

hkjn commented Jun 4, 2019

Okay, I can't say I still really understand what I'm doing, but it seems that most of the diffs to npm-shrinkwrap.js were the result of various 3rd party dependencies (or their transitive dependencies) changing underneath us, and not the new babel-preset-es2015 package. In 8cde08a, I ran npm update repeatedly until there was no further diffs produced. (I have no idea why the tool would produce diffs when run more than once, but it seems to have done just this.)

In e24622c, I added the specific babel-preset-es2015 package which seems to have thrown the earlier error by running npm install babel-preset-es2015 --save-dev.

@shesek: The build seems to pass with these patches. Let me know if you'd like me to test anything, or have suggestions for how to approach this a different way (feel free to make changes to the branch).

@hkjn hkjn changed the title docker builder: Add more wine packages and switch to OpenJDK to attempt to fix build docker builder: Fix build: add wine packages, switch to OpenJDK, add babel-preset-es2015 dev dependency Jun 4, 2019
@shesek
Copy link
Owner

shesek commented Jun 6, 2019

Thanks for the help!

switching to OpenJDK was trivial, so I did that in b1d685c.

I'm trying to build locally and test on my android, will report back.

but it does build succcessfully (with sudo docker build -f scripts/builder.Dockerfile -t spark-builder ..)

Note that this doesn't actually build the distribution files, it just prepares the docker environment where they could be built.

I just added the packages that apt claimed were necessary in ef9ee07 until the installation completed successfully

It looks like bumping winehq-stable's version to 4.0.1~stretch fixes this without having to manually specify the other packages.

The next failure was: .. Couldn't find preset "es2015" relative to directory

npm install babel-cli babel-preset-es2015

This should not be necessary, babel-preset-env supersedes babel-preset-es2015 and is the recommended way to use babel (See here: "babel-preset-es2015 -> babel-preset-env" ... "instead of continuing yearly presets, the team recommends using babel-preset-env").

Will look into this once I have the docker builder ready.

In 8cde08a, I ran npm update repeatedly until there was no further diffs produced.

I'll need to go over the changelogs and run some tests to make sure these dependency updates aren't breaking anything before upgrading, so better keep that separate from this PR.

hkjn added 4 commits June 6, 2019 16:21
Not sure what changed, but without this patch, the build
fails with:

```
The following packages have unmet dependencies:
 winehq-stable : Depends: wine-stable (= 4.0~stretch)
E: Unable to correct problems, you have held broken packages.
```
Produced by running `npm update` repeatedly, until there were no
more changes produced.
Produced by running `npm install babel-preset-es2015 --save-dev`,
via https://stackoverflow.com/a/36363346.
@hkjn
Copy link
Contributor Author

hkjn commented Jun 6, 2019

It looks like bumping winehq-stable's version to 4.0.1~stretch fixes this without having to manually specify the other packages.

Good call, that works. I've force pushed a new commit that just bumps the version of winehq-stable instead of adding more wine packages.

This should not be necessary, babel-preset-env supersedes babel-preset-es2015 and is the recommended way to use babel (See here: "babel-preset-es2015 -> babel-preset-env" ... "instead of continuing yearly presets, the team recommends using babel-preset-env").

Will look into this once I have the docker builder ready.

Makes sense. Thanks, any help would be appreciated; I did notice that message, but as mentioned earlier, I was more-or-less running commands suggested by random stackoverflow.com answers blindly until something led to a successful image build. :)

I'll need to go over the changelogs and run some tests to make sure these dependency updates aren't breaking anything before upgrading, so better keep that separate from this PR.

Sounds good and thanks here as well, I don't know how to even start doing that; surprised at how quickly NPM packages seem to get updated / how large the dependency graphs often are..

Let me know here as the updated packages are merged to master and I'll rebase to bring in those changes to this branch. (Or feel free to cherry-pick commits / take inspiration from this branch and close the current PR; whatever is easiest for you.)

@hkjn hkjn changed the title docker builder: Fix build: add wine packages, switch to OpenJDK, add babel-preset-es2015 dev dependency docker builder: Fix build: bump version of wine package, switch to OpenJDK, add babel-preset-es2015 dev dependency Jun 6, 2019
shesek added a commit that referenced this pull request Jun 6, 2019
shesek added a commit that referenced this pull request Jun 6, 2019
shesek added a commit that referenced this pull request Jun 6, 2019
This should not be needed since Spark uses the "env" preset,
but it seems like one of the dependencies expects this module
to be installed. Refs #84
@shesek
Copy link
Owner

shesek commented Jun 6, 2019

I tested on my android, OpenJDK works great. This was easier than I expected. :)

The es2015 preset should not be necessary, but appears to be used by a sub-dependency of spark (maybe something that was changed in a recent update?), so I ended up including it like you did.

I committed these changes separately (1f1b738, 82763ed, 9d59930), thank you so much for looking into this and the help figuring this out!

I can finally make a proper release, will hopefully get one out in the next day or two. Cheers :)

@shesek shesek closed this Jun 6, 2019
@hkjn hkjn deleted the 20190601-fix-build branch June 6, 2019 16:16
@hkjn
Copy link
Contributor Author

hkjn commented Jun 6, 2019

Glad to hear it worked out.

No problem, and many thanks for building such an excellent wallet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ppa:webupd8team/java is discontinued
2 participants