-
Notifications
You must be signed in to change notification settings - Fork 111
Build che-theia plugins from che-theia-dev builder container rather than getting it from released travis built #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need a list of plugins that we add, as in this repository may exist plugins that we do not want do include in Docker image by default
@@ -70,6 +73,13 @@ RUN che:theia production | |||
# change permissions | |||
RUN find production -exec sh -c "chgrp 0 {}; chmod g+rwX {}" \; 2>log.txt | |||
|
|||
# Clone and build che-theia plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make it part of the theia-generator (the clone will be done in init step)
and then the compilation would occurs as another step of theia-generator like 'build-plugins' or something else
Then it will use same flag that @evidolob is adding for dev mode (master branch) vs tagged version, etc
@@ -70,6 +73,13 @@ RUN che:theia production | |||
# change permissions | |||
RUN find production -exec sh -c "chgrp 0 {}; chmod g+rwX {}" \; 2>log.txt | |||
|
|||
# Clone and build che-theia plugins | |||
RUN git clone --branch ${CHE_THEIA_PLUGIN_BRANCH} --single-branch --depth 1 https://github.com/eclipse/che-theia ${HOME}/che-theia-source-code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not add these steps after the changes of permission
besides I'm in favor of adding it to che-theia-generator, I think that the plug-ins should not be compiled in the same image
It should be part of another stage. So if you modify theia but not plug-ins we may benefit of docker cache ?
935be69
to
1369f39
Compare
…leased ones Signed-off-by: Sun Seng David TAN <sutan@redhat.com>
1369f39
to
2278419
Compare
dockerfiles/theia/Dockerfile
Outdated
RUN yarn | ||
|
||
RUN mkdir -p ${HOME}/che-theia-plugins/ && \ | ||
find ${HOME}/che-theia-source-code/plugins/ -name "*.theia" -exec sh -c "chgrp 0 {}; chmod g+rwX {}; cp {} ${HOME}/che-theia-plugins/" \; 2>log.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now it should contain a regexp to only select a subset of plug-ins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure also why we do chgrp/chmod there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now it should contain a regexp to only select a subset of plug-ins
ok which ones do we keep ? @l0rd @evidolob @benoitf
- containers-plugin
- factory-plugin
- ports-plugin
- ssh-plugin
- welcome-plugin
I'm not sure also why we do chgrp/chmod there
not sure I thought we had a good reason as we do it for theia production package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, we do the chgrp/chmod for this folder later in production container so we don't need to do it in the build container
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok i see :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benoitf added the right filter and removed the permission change commands
@@ -96,6 +136,8 @@ ENV USE_LOCAL_GIT=true \ | |||
|
|||
EXPOSE 3100 3130 | |||
|
|||
COPY --from=plugins-builder /home/theia-dev/che-theia-plugins /default-theia-plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--chown=theia:root
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
permissions are updated afterwards for /default-theia-plugins
and group or user doesn't exist yet.
…ting released ones Signed-off-by: Sun Seng David TAN <sutan@redhat.com>
…han getting released ones Signed-off-by: Sun Seng David TAN <sutan@redhat.com>
@@ -82,6 +82,46 @@ RUN che:theia production | |||
# change permissions | |||
RUN find production -exec sh -c "chgrp 0 {}; chmod g+rwX {}" \; 2>log.txt | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello, didn't see it before.
why do we have copy of lines 89--->115 from the previous container ?
couldn't we just add # Clone and build che-theia plugins at the end of previous container ?
here we're duplicating all github token, etc logic in the same file for almost nothing ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe i misunderstood the comments of #43 (comment)
we are duplicating just the github token stuff. I am very fine with cloning and building che-theia plugins at the end of the previous container
fi | ||
|
||
# Clone and build che-theia plugins | ||
RUN git clone --branch ${CHE_THEIA_PLUGIN_BRANCH} --single-branch --depth 1 https://github.com/eclipse/che-theia ${HOME}/che-theia-source-code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks odd to clone CHE_THEIA while we're inside che-theia there, no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, on another hand this is what we do with the generator
It is a proposal, WDYT ?
Signed-off-by: Sun Seng David TAN sutan@redhat.com