chore: Add docker images for building the different platforms #11397
Conversation
User Test ResultsTest specification and instructions User tests are not required |
github-actions
bot
added
linux/
common/
common/resources/
ermshiperete
force-pushed
the
chore/linux/cicontainer
branch
2 times, most recently
from
90fe8aa to
1e10c81
Compare
This updates the minimum required node version to Node 20. The latest version of Node 18 still contains the npm bug (npm/cli#7072) whereas Node 20 got updated to a npm version that contains a fix. Node 20 is known to work with our current code, so this change updates to that as an intermediate step before we investigate if we can update to Node 22 as discussed for Keyman 18.
This change allows to build a docker image that can build Keyman for Linux with test coverage reports, and installs the necessary dependencies for running integration tests.
ermshiperete
force-pushed
the
chore/linux/cicontainer
branch
from
1e10c81 to
8a4cdce
Compare
- use KEYMAN_USE_NVM and KEYMAN_USE_EMSDK - pre-install node (where necessary) to prevent having to do it on each build - adjust to current `master` - fix a few bugs in the Dockerfile - get required node version from package.json (through `shellHelperFunctions.sh`)
| By default this will create 64-bit images for building | ||
| Keyman Core, Keyman for Android, Keyman for Linux and | ||
| Keyman for Web. These images are based on the Ubuntu 24.04 | ||
| with Node 20 and Emscripten 3.1.58 (for the exact versions, |
There was a problem hiding this comment.
Some of the Dockerfiles below use Node 18?
ARG REQUIRED_NODE_VERSION=18
There was a problem hiding this comment.
No, that's just the default value that doesn't get used. The node version get's passed in. I had to put something here, so I choose a somewhat sensible value that is different from what we currently have so that it's easier noticable if passing the value doesn't work.
There was a problem hiding this comment.
Should be node 20 -- I think the req shifted since this PR started
Also add test action to build.sh which builds all images and then tests them by running configure,build,test on each. Also add new dependencies to web image which Playwright requires.
These changes bring us one step further, but the build is still failing.
Setting permissions is now done in the base image.
| $HOME/tmp/gradlew --quiet && \ | ||
| rm -rf $HOME/tmp | ||
|
|
||
| ENTRYPOINT [ "/usr/bin/bashwrapper" ] |
There was a problem hiding this comment.
Does this file need to be fully deleted?
There was a problem hiding this comment.
yes, it's replaced by this PR. Splitting it in multiple Dockerfiles has the advantage that we notice if we have the documented dependencies wrong for a platform. Previously it was possible that e.g. the dependencies for Android were wrong in the documentation, but the build still worked locally because for Linux you already installed the missing dependencies. Now when you tried to set up a build agent that only builds Android things would fail.
Also some cleanup.
This change adds the ability to build several docker images which can be used to build the different platforms (core, linux, web, android) and run tests. This can be used on a developer's machine but also on CI.
This change also updates the minimum node version to Node 20 as a temporary step until we try if we can use Node 22. This is necessary because the latest release of Node 18 still comes with the buggy npm that crashes the network. A better solution might have been to use
nvmin the docker images (which would allow to easily update to the latest npm), but that would have required more changes and I decided to do it this way for now in order to finish this PR.@keymanapp-test-bot skip