-
Notifications
You must be signed in to change notification settings - Fork 131
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
[build][quality] more efficient CI, builds, improved linter config #272
Conversation
52f96ae
to
a4fa02b
Compare
7ad5a79
to
df67351
Compare
a924593
to
f4ff9a8
Compare
45c64c0
to
8be67ed
Compare
7f51255
to
2fffaaf
Compare
@vince-fugnitto could you have another look? |
e1c6f37
to
6f65b08
Compare
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 did not yet have the time for a detailed look, but I did some initial testing.
First, I think the production mode is not fully working. Are we missing a rebuild here?
Reproducer:
git clean -xfd
yarn && yarn build
yarn electron start
leads to
Error: node-loader:
Error: Module did not self-register: '.../applications/electron/lib/backend/native/drivelist.node'.
Also the Browser Docker Image does not seem to be working yet:
Failed to start the backend application:
Error: Cannot find module 'blueprint-product-ext/lib/node/theia-blueprint-backend-module'
at webpackMissingModule (/home/theia/applications/browser/lib/backend/main.js:328:117)
at /home/theia/applications/browser/lib/backend/main.js:328:253
at async Object.start (/home/theia/applications/browser/lib/backend/vendors-node_modules_drivelist_build_Release_drivelist_node-node_modules_stroncium_procfs_lib-63b120.js:4987:20) {
code: 'MODULE_NOT_FOUND'
}
/home/theia/applications/browser/lib/backend/vendors-node_modules_drivelist_build_Release_drivelist_node-node_modules_stroncium_procfs_lib-63b120.js:4984
throw reason;
^
Error: Cannot find module 'blueprint-product-ext/lib/node/theia-blueprint-backend-module'
at webpackMissingModule (/home/theia/applications/browser/lib/backend/main.js:328:117)
at /home/theia/applications/browser/lib/backend/main.js:328:253
at async Object.start (/home/theia/applications/browser/lib/backend/vendors-node_modules_drivelist_build_Release_drivelist_node-node_modules_stroncium_procfs_lib-63b120.js:4987:20) {
code: 'MODULE_NOT_FOUND'
}
…onfig This is a significant refactoring of the repo and CI. 1) Jenkins CI: more efficient, quicker builds I propose that we adopt a "run often, run quickly" approach to our Jenkins CI. ATM we mostly run Jenkins when it's time to release new Blueprint installers (when a PR is merged on master). The other case is if "jenkins" is detected in the PR branch or PR title, then almost a full Jenkins build is done, just stopping short of publishing new Blueprint installers (full signing and notarization is done, which can take a couple of hours). I made it so we run a very light version of the Jenkins CI by default, that builds the "dev" version of the app and pre-packages it using electron builder, generating a directory that contains the app but no installers. We really ought to run tests then, on the unpacked pre-packaged app, but it will need to be done separately. Until then, we do not require the built-in extensions (plugins), so I gave openvsx.org a rest and skip fetching them, by default. When it's detected that the Jenkins CI is processing the result of a merge on master or if the new "release dry-run" mode is enabled [1], a production version of the Blueprint apps will be produced, including built-in plugins, which will then be packaged for "real", producing genuine OS-specific installers, signed and notarized as required. This will take a lot longer to run, of course, but may be required only a minority of the time. Conclusion: with this PR, running Jenkins CI now takes about 12 minutes, and since this is relatively quick, we can afford to run this when any PR branch is created or updated. Running the same CI in "release dry-run mode" takes about an hour. A release will take longer, a bit over an hour, since additional steps are executed, like copying the various installers to a release folder. [1] About the "release dry-run" mode, it can be enabled in JenkinsFile, "pipeline" definition near the top, in "environment": BLUEPRINT_JENKINS_RELEASE_DRYRUN = 'true' I wish I found a better mechanism to enable this "release dry-run" mode. But this will serve for now. The "release dry-run" mode should be disabled before merging a PR to master, but can in the meantime be used to test or troubleshoot the whole pipeline, in particular signing and notarizing. 2) tsconfig/eslint configs: Added configs, to improve linter coverage. This made it possible for some source files, not previously covered, to get ts/linter feedback, both while editing and when running `yarn lint`. This will help keep code in-line with our standards. The config is not perfect and I would welcome further improvements. But for now I think it's a nice improvement. 3) Build "scripts" (package.json) Refactored the build commands ("scripts" section in package.json) - previously, merely running 'yarn" in the repo's root would rebuild every application from scratch. This prevents running a quick "yarn install", e.g. just to re-install build dependencies - the new version permits a granular build, with simple defaults - inspired from a similar change not so long ago in the main repo - see updated README for some examples Other misc items: - renamed extensions / applications - made the browser application a first class citizen, equal to the Electron application - all applications now share a common 'plugins' folder rather than each having their own. Moved the plugin-related entries to root package.json - to gain flexibility about which `yarn workspaces` are invoked for a given `lerna` command, using the `--scope=` CLI option.I renamed the repo's extensions and applications. This permits easily composing commands that target only the extensions or only the applications. e.g.: ``` json "build:extensions": "lerna run --scope=\"blueprint*ext\" build", "build:applications": "lerna run --scope=\"blueprint*app\" build --concurrency 1"," ``` - renamed the extensions folders, made them more straightforward - For systems with limited RAM, like on a Raspberry Pi 4B board with 4GB of RAM, it's now possible to successfully build Blueprint. e.g. use the following cmd: `yarn && yarn build:dev`, optionally followed by a packing command like: `yarn electron package` - (build:dev will build the Blueprint app in dev mode, which can be achieved in 4GB RAM) - [windows][jenkins] stash only dist folder: Currently, and only for Windows, we stash the whole git repo, which is very big and takes long to stash and un-stash. For the other OS, we only stash the dist folder, that contains the platform-specific installer, that we built. Let's try that for Windows too, and see if it works. - [jenkins] Decrease timeout from 5h to 3h" Looking at the build history, all recent builds that succeeded, did do under 2h30. OTOH, when a build hangs, it needs to wait for the timeout to expire, wasting time at the current value of 5h. Let's compromise at 3 hours and see how it goes? - [jenkins][installer build] exclude browser app for now: We do not yet publish the browser app, so let's skip building it to save time/resources. We may revisit when we use the new browser app bundling, recently made available upstream in the Theia framework. Signed-off-by: Marc Dumais <[email protected]>
6f65b08
to
dc3a4bf
Compare
#272 (review) Signed-off-by: Marc Dumais <[email protected]>
@jfaltermeier thanks for the initial look! i have rebased the PR on the latest master and added the couple suggested |
Oops - I had forgotten to build the Theia extensions, which caused a problem at runtime. Signed-off-by: Marc Dumais <[email protected]>
3a3c388
to
f462d26
Compare
I believe that I have fixed the issue with |
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.
Thanks, looks good to me!
Thanks for the review @jfaltermeier! @JonasHelming @thegecko FYI, this PR has some changes in it that could potentially be breaking e.g. if someone has a product based on Blueprint, but I do not think it should be anything too difficult to fix. I wonder how e.g. cdt.cloud consumes / extends / depends on Blueprint? |
CDT Cloud Blueprint looks like a fork of this repo, and I see signs it's kept up-to date. So it will be a bigger rebase than usual. |
@planger FYI |
@planger if it's welcomed, I can follow-up with a |
@marcdumais-work You are certainly more than welcomed to create a follow-up PR for cdt-cloud-blueprint! Thank you very much! :-) However, if you are very busy, feel free to just let us know and we'll take care of upgrading cdt-cloud-blueprint. |
#272 (review) Signed-off-by: Marc Dumais <[email protected]>
@planger unfortunately I do not think I will have time to do the follow-up, in the next little while. Please tell the person that eventually does the uplift that they can tag me on the PR if there are questions. |
@marcdumais-work Ok thanks for letting me know anyways! We'll get back to you if there are questions. Thanks! |
What it does
This is a significant refactoring of the repo and CI.
1) JenKins CI: more efficient, quicker builds
I propose that we adopt a "run often, run quickly" approach to our Jenkins CI.
ATM we mostly run Jenkins when it's time to release new Blueprint installers
(when a PR is merged on master). The other case is if "jenkins" is detected
in the PR branch or PR title, then almost a full Jenkins build is done, just
stopping short of publishing new Blueprint installers (full signing and
notarization is done, which can take a couple of hours).
I made it so we run a very light version of the Jenkins CI by default, that
builds the "dev" version of the app and pre-packages it using electron builder,
generating a directory that contains the app but no installers.
We really ought to run tests then, on the unpacked pre-packaged app, but
it will need to be done separately. Until then, we do not require the built-in
extensions (plugins), so I gave openvsx.org a rest and skip fetching them, by
default.
When it's detected that the Jenkins CI is processing the result of a merge on
master or if the new "release dry-run" mode is enabled [1], a production version
of the Blueprint apps will be produced, including built-in plugins, which will
then be packaged for "real", producing genuine OS-specific installers, signed
and notarized as required. This will take a lot longer to run, of course, but
may be required only a minority of the time.
Conclusion: with this PR, running Jenkins CI now takes about 12 minutes, and
since this is relatively quick, we can afford to run this when any PR branch
is created or updated.
Running the same CI in "release dry-run mode" takes about an hour. A release
will take longer, a bit over an hour, since additional steps are executed,
like copying the various installers to a release folder.
[1] About the "release dry-run" mode, it can be enabled in JenkinsFile,
"pipeline" definition near the top, in "environment":
BLUEPRINT_JENKINS_RELEASE_DRYRUN = 'true
I wish I found a better mechanism to enable this "release dry-run" mode. But
this will serve for now. The "release dry-run" mode should be disabled before
merging a PR to master, but can in the meantime be used to test or troubleshoot
the whole pipeline, in particular signing and notarizing.
2) tsconfig/eslint configs:
Added configs, to improve linter coverage. This made it possible for some
source files, not previously covered, to get ts/linter feedback, both while
editing and when running
yarn lint
. This will help keep code in-line withour standards. The config is not perfect and I would welcome further
improvements. But for now I think it's a nice improvement.
3) Build "scripts" (package.json)
Refactored the build commands ("scripts" section in package.json)
every application from scratch. This prevents running a quick
"yarn install", e.g. just to re-install build dependencies
Other misc items:
renamed extensions / applications
made the browser application a first class citizen, equal to the
Electron application
all applications now share a common 'plugins' folder rather than
each having their own. Moved the plugin-related entries to root
package.json
to gain flexibility about which
yarn workspaces
are invoked for agiven
lerna
command, using the--scope=
CLI option.I renamed therepo's extensions and applications. This permits easily composing
commands that target only the extensions or only the applications.
e.g.:
renamed the extensions folders, made them more straightforward
For systems with limited RAM, like on a Raspberry Pi 4B board with 4GB of RAM,
it's now possible to successfully build Blueprint. e.g. use the following cmd:
yarn && yarn build:dev
, optionally followed by a packing command like:yarn electron package
in 4GB RAM)
[windows][jenkins] stash only dist folder: Currently, and only for Windows,
we stash the whole git repo, which is very big and takes long to stash and
un-stash. For the other OS, we only stash the dist folder, that contains the
platform-specific installer, that we built. Let's try that for Windows too,
and see if it works.
[jenkins] Decrease timeout from 5h to 3h" Looking at the build history, all
recent builds that succeeded, did do under 2h30. OTOH, when a build hangs,
it needs to wait for the timeout to expire, wasting time at the current value
of 5h. Let's compromise at 3 hours and see how it goes?
[jenkins][installer build] exclude browser app for now: We do not yet publish
the browser app, so let's skip building it to save time/resources. We may
revisit when we use the new browser app bundling, recently made available
upstream in the Theia framework.
How to test
Run the tests locally (currently only covers Electron):
A few other things to test. For each make sure that the built-ins are present
yarn electron package # test the generated package. e.g. on Linux: `applications/electron/dist/TheiaBlueprint.AppImage`
Review checklist
Reminder for reviewers