Generate yarn cache with THEIA_VERSION. Generate resolutions to use strict Theia dependencies by AndrienkoAleksandr · Pull Request #10164 · eclipse-che/che

AndrienkoAleksandr · 2018-06-23T10:45:09Z

What does this PR do?

Generate and apply resolutions to the Theia package.json to use strict Theia dependencies versions.

What issues does this PR fix or reference?

Work around for:
eclipse-theia/theia#2137

Signed-off-by: Oleksandr Andriienko oandriie@redhat.com

…dependency with strict version. Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

benoitf · 2018-06-25T05:40:00Z

dockerfiles/theia/Dockerfile

 ADD https://raw.githubusercontent.com/theia-ide/theia/v${THEIA_VERSION}/examples/browser/package.json /home/theia/package.json
 ADD theia-default-package.json /home/default/theia/package.json
-ADD versions.sh /home/default/versions.sh
+ADD src/versions.sh /home/default/versions.sh


maybe not for this PR, but should we store in a directory all files that will go to /home/default so we can use only one ADD instruction, and we don't need to think on adding line each time we require a file ?

benoitf · 2018-06-25T05:41:52Z

dockerfiles/theia/Dockerfile

+    ./versions.sh /home/default/theia/package.json && \
+    node resolutions-provider.js /home/theia/package.json /home/default/resolutions.json && \
+    node resolutions-applier.js /home/default/resolutions.json /home/theia/package.json && \
+    node resolutions-applier.js /home/default/resolutions.json /home/default/theia/package.json && \


maybe we can use pattern

node resolution-applier <resolution-file> <file1> <file2> <file3>

benoitf · 2018-06-25T05:42:25Z

dockerfiles/theia/src/resolutions-applier.js

+const RESOLUTIONS_JSON_PATH = process.argv[2];
+const PACKAGE_JSON_PATH = process.argv[3];
+
+const RESOLITIONS = require(RESOLUTIONS_JSON_PATH);


typo on RESOLUTIONS (I/U)

benoitf · 2018-06-25T05:42:39Z

dockerfiles/theia/src/resolutions-applier.js

+const RESOLITIONS = require(RESOLUTIONS_JSON_PATH);
+const PACKAGE_JSON = require(PACKAGE_JSON_PATH);
+
+PACKAGE_JSON["resolutions"] = RESOLITIONS;


benoitf · 2018-06-25T05:43:45Z

dockerfiles/theia/src/resolutions-provider.js

+
+const PACKAGE_JSON = require(PACKAGE_JSON_PATH);
+const THEIA_VERSION = process.env.THEIA_VERSION;
+const THEID_DEP_PREFIX = "@theia/";


is it a typo / THEID instead of THEIA ?

…ation

skabashnyuk · 2018-07-05T11:38:05Z

@AndrienkoAleksandr @ashumilova Is this work in progress PR?

AndrienkoAleksandr · 2018-07-05T11:46:29Z

@skabashnyuk yes it's in progress.

Don't use yarn cache dir on Theia build, use node_modules, like single storage. Reuse Theia node_modules on default extensions build. Use extensions json to get list default extensions. Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

…ation

benoitf · 2018-07-09T13:15:59Z

dockerfiles/theia/Dockerfile


 ARG GITHUB_TOKEN
-ARG THEIA_VERSION=0.4.0-next.b17727c1
+ARG THEIA_VERSION=3.12


it's not 0.3.12 ?

Ops, I build image manually and apply version with help docker arg, and didn't see this mistake.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

benoitf · 2018-07-09T13:36:11Z

dockerfiles/theia/src/extensions.json

@@ -0,0 +1,16 @@
+{


what is doing this file ? (in comparison with npmjs where some extensions are deployed ? )

It's file with list default extensions

my question is: what's the difference with https://github.com/eclipse/che/blob/db0cb63926b0110e05d4ba6d78912f1f4d856f95/dockerfiles/theia/src/package.json#L14 ?

I didn't saw this changes, I will check it. add-extension.js use extensions.js script to get default extensions from github or volume, build these extensions and rebuild theia with them. Do you propose doesn't support this stuff?

I was suggesting as higlighted by @sunix that if extension is stored on npmjs then we can use npmjs way of grabbing extension.

@benoitf I checked factory extension works with this pr too. I think, if we have released extension we can apply it to the package.json, like it was done by sunix. But in case if we have github link or volume to the source code we can use add-extensions.js script to development purpose. add-extensions.js contains optimization to reuse Theia node_modules. @benoitf What do you think? Like variant I can apply empty extensions.json, like sample. And we will apply machine-extension and che-theia-hosted-plugin-manager-extension later after release on the npm.

@AndrienkoAleksandr yes it's nice to keep extensibility with this file but if possible use package.json for released artifacts on npmjs

AndrienkoAleksandr · 2018-07-09T13:47:58Z

Applied changes:

use extension.json with list default extension;
don't use yarn cache, include all dependencies to the node_modules;
default extension reuse Theia node_modules;
After optimization docker image size decreased from more than 3 Gb to less than 900 Mb.

Tested on the openshift.io.

benoitf · 2018-07-12T05:59:56Z

dockerfiles/theia/Dockerfile

+    # Apply resolution section to the Theia package.json to use strict versions for Theia dependencies
+    node ${HOME}/resolutions-provider.js ${HOME}/package.json && \
+    # generate node_modules, should be reused with default extensions
+    yarn --global-folder ${HOME}/node_modules/ --cache-folder ${HOME}/node_modules/ && \


how does it behave inside the container when running a new yarn command in for example /tmp/my-project

does yarn will download again all dependencies as cache will be different location ?
(for example if user create a new Theia plugin and want to compile it)

should we use yarn config set or .yarnrc file

@benoitf I think the easiest way to increase docker image size it's a using yarn cache clean command. What do you think?

benoitf · 2018-07-12T06:00:31Z

dockerfiles/theia/Dockerfile

-    yarn && \
    yarn theia build && \
    # Install Theia plugin generator
    #==TODO== add after generator-theia-plugin release


TODO can be removed
https://www.npmjs.com/package/@theia/generator-plugin

Fix bug with using not "Sync exists" function. Reinstall wget, download files by https links doensn't work without this action after update alpine image.

…ation

mmorhun

Nice work!

mmorhun · 2018-07-16T06:51:47Z

dockerfiles/theia/Dockerfile

+    node ${HOME}/resolutions-provider.js ${HOME}/package.json && \
+    # generate node_modules, should be reused with default extensions
+    yarn --global-folder ${HOME}/node_modules/ --cache-folder ${HOME}/node_modules/ && \
+    rm -rf ${HOME}/extensions && \


Do we really need this step?

mmorhun · 2018-07-16T06:59:31Z

dockerfiles/theia/Dockerfile

+    yarn --global-folder ${HOME}/node_modules/ --cache-folder ${HOME}/node_modules/ && \
+    rm -rf ${HOME}/extensions && \
+    # Add default Theia extensions
+    node ${HOME}/add-extensions.js && \


We can remove this script after invoking.

mmorhun · 2018-07-16T07:01:49Z

dockerfiles/theia/README.md

+
 with native docker:
+
+```


I think given version in the example won't work because plugin model is released in 0.3.12. Please add note and correct example.

mmorhun · 2018-07-16T07:16:35Z

dockerfiles/theia/src/add-extensions.js

+function checkoutRepo(path, checkoutTarget) {
+    try {
+         if (!checkoutTarget) {
+            checkoutTarget = 'master';


I think we shouldn't checkout at all if checkoutTarget is not set.

mmorhun · 2018-07-16T07:21:06Z

dockerfiles/theia/src/extensions.json

+        {
+            "name": "theia-machines-extension",
+            "source": "https://github.com/eclipse/che-theia-machines-plugin.git",
+            "version": "0.3.12",


Please rename to version to branch ot whatever because it is actually checkout argument.

…ation

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

…ation

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

AndrienkoAleksandr · 2018-07-20T07:42:57Z

Hmm... Sometimes docker build fail:

warning "@theia/metrics > @theia/application-manager > font-awesome-webpack@0.0.5-beta.2" has unmet peer dependency "font-awesome@>=4.3.0".
[4/4] Building fresh packages...
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
error /home/theia/node_modules/electron: Command failed.
Exit code: 1
Command: node install.js
Arguments:
Directory: /home/theia/node_modules/electron
Output:
Downloading SHASUMS256.txt
Error: socket hang up
/home/theia/node_modules/electron/install.js:47
  throw err
  ^

Error: socket hang up
    at createHangUpError (_http_client.js:331:15)
    at TLSSocket.socketOnEnd (_http_client.js:423:23)
    at emitNone (events.js:111:20)
    at TLSSocket.emit (events.js:208:7)
    at endReadableNT (_stream_readable.js:1064:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)

I need take a look...

benoitf · 2018-07-20T07:48:06Z

dockerfiles/theia/Dockerfile

    fi && \
    # install remaining packages
-    apk add --no-cache make gcc g++ python git bash supervisor && \
+    apk add --no-cache make gcc g++ python git bash supervisor curl && \


FYI curl is installed few lines before

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

AndrienkoAleksandr · 2018-07-23T08:00:31Z

Checked failing build:

Error: socket hang up
    at createHangUpError (_http_client.js:331:15)
    at TLSSocket.socketOnEnd (_http_client.js:423:23)
    at emitNone (events.js:111:20)
    at TLSSocket.emit (events.js:208:7)
    at endReadableNT (_stream_readable.js:1064:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)

Today I build 5 times and everything is OK. In Friday 20 was unstable github, so I think it was core reason of failing:
https://status.github.com/messages#2018-07-20

benoitf

I'm travelling today so approving to not block process ;-)

Generate cache with THEIA_VERSION, Generate resolutions to use theia …

09b2661

…dependency with strict version. Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

AndrienkoAleksandr requested review from a user, benoitf and mmorhun June 23, 2018 10:45

AndrienkoAleksandr requested a review from riuvshin as a code owner June 23, 2018 10:45

AndrienkoAleksandr mentioned this pull request Jun 23, 2018

Apply yarn.lock file to stabilize Theia image rebuild after new Theia releases #10113

Closed

AndrienkoAleksandr changed the title ~~Generate cache with THEIA_VERSION. Generate resolutions for Theia dependencies~~ Generate yarn cache with THEIA_VERSION. Generate resolutions for Theia dependencies Jun 23, 2018

AndrienkoAleksandr changed the title ~~Generate yarn cache with THEIA_VERSION. Generate resolutions for Theia dependencies~~ Generate yarn cache with THEIA_VERSION. Generate resolutions to use strict Theia dependencies Jun 23, 2018

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jun 23, 2018

benoitf reviewed Jun 25, 2018

View reviewed changes

Merge branch 'master' of github.com:eclipse/che into fixTheiaImageCre…

8fa0785

…ation

AndrienkoAleksandr added 2 commits July 9, 2018 15:53

Optimize docker images size.

82ccb98

Don't use yarn cache dir on Theia build, use node_modules, like single storage. Reuse Theia node_modules on default extensions build. Use extensions json to get list default extensions. Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Merge branch 'master' of github.com:eclipse/che into fixTheiaImageCre…

8bf8337

…ation

benoitf reviewed Jul 9, 2018

View reviewed changes

AndrienkoAleksandr added 2 commits July 9, 2018 16:25

Code clean up.

e46c270

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Fix default Theia version and extensions links.

db0cb63

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

benoitf reviewed Jul 9, 2018

View reviewed changes

This was referenced Jul 9, 2018

Update to Theia 0.3.12 eclipse-archived/che-theia-machines-plugin#3

Open

Use Theia version 0.3.12 eclipse-archived/che-theia-hosted-plugin-manager-extension#2

Merged

ashumilova mentioned this pull request Jul 11, 2018

IDE2-2018-07-31 (Sprint: 152) #10348

Closed

20 tasks

benoitf reviewed Jul 12, 2018

View reviewed changes

AndrienkoAleksandr added 2 commits July 13, 2018 09:02

Install yeoman generator for plugins.

852b923

Fix bug with using not "Sync exists" function. Reinstall wget, download files by https links doensn't work without this action after update alpine image.

Merge branch 'master' of github.com:eclipse/che into fixTheiaImageCre…

2621d56

…ation

mmorhun reviewed Jul 16, 2018

View reviewed changes

mmorhun mentioned this pull request Jul 17, 2018

Use Theia 0.3.12 core #10441

Closed

Merge branch 'master' of github.com:eclipse/che into fixTheiaImageCre…

03a03b1

…ation

AndrienkoAleksandr added 5 commits July 19, 2018 14:13

Merge branch 'master' of github.com:eclipse/che into fixTheiaImageCre…

6ed18ec

…ation

Next

a4df065

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Fix up

9e1cd84

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

Merge branch 'master' of github.com:eclipse/che into fixTheiaImageCre…

462c5f8

…ation

Clean up.

2abf55b

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

benoitf reviewed Jul 20, 2018

View reviewed changes

Remove duplicated installation of the curl.

55c0070

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

benoitf approved these changes Jul 23, 2018

View reviewed changes

AndrienkoAleksandr merged commit 88add6e into master Jul 23, 2018

AndrienkoAleksandr deleted the fixTheiaImageCreation branch July 23, 2018 08:31

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jul 23, 2018

benoitf added this to the 6.9.0 milestone Jul 23, 2018

Conversation

AndrienkoAleksandr commented Jun 23, 2018

What does this PR do?

What issues does this PR fix or reference?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skabashnyuk commented Jul 5, 2018

Uh oh!

AndrienkoAleksandr commented Jul 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrienkoAleksandr commented Jul 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmorhun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrienkoAleksandr commented Jul 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrienkoAleksandr commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benoitf left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AndrienkoAleksandr commented Jul 23, 2018 •

edited

Loading