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

[build][quality] more efficient CI, builds, improved linter config #272

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Jun 14, 2023

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 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.:

      "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.

How to test

Run the tests locally (currently only covers Electron):

cd theia-blueprint
yarn && yarn build && yarn download:plugins && yarn package:preview
yarn test

A few other things to test. For each make sure that the built-ins are present

  • the browser app starts
yarn browser start
  • the electron app starts
yarn electron start
  • packaging and starting the electron app works
yarn electron package
# test the generated package. e.g. on Linux: `applications/electron/dist/TheiaBlueprint.AppImage`

Review checklist

Reminder for reviewers

@marcdumais-work marcdumais-work force-pushed the sprint-cleaning branch 4 times, most recently from 52f96ae to a4fa02b Compare June 14, 2023 15:47
@marcdumais-work
Copy link
Contributor Author

Progress: using this PR branch I can successfully build and run Blueprint browser on my Pi 4:
Screenshot from 2023-06-14 13-01-55

@marcdumais-work marcdumais-work force-pushed the sprint-cleaning branch 12 times, most recently from 7ad5a79 to df67351 Compare June 19, 2023 16:39
@marcdumais-work marcdumais-work force-pushed the sprint-cleaning branch 4 times, most recently from a924593 to f4ff9a8 Compare June 19, 2023 19:23
@marcdumais-work marcdumais-work changed the title [WIP] Refactor build scripts, one plugins folder [WIP] Refactor build scripts, one plugins folder - Jenkins Jun 20, 2023
@marcdumais-work marcdumais-work changed the title [WIP] Refactor build scripts, one plugins folder - Jenkins [WIP] Refactor build scripts, one plugins folder Jun 20, 2023
@marcdumais-work marcdumais-work force-pushed the sprint-cleaning branch 2 times, most recently from 45c64c0 to 8be67ed Compare June 20, 2023 15:41
@marcdumais-work marcdumais-work changed the title [WIP] Refactor build scripts, one plugins folder [WIP] Refactor build scripts, one plugins folder. jenkins Jun 20, 2023
@marcdumais-work marcdumais-work force-pushed the sprint-cleaning branch 3 times, most recently from 7f51255 to 2fffaaf Compare June 20, 2023 18:00
@marcdumais-work
Copy link
Contributor Author

@vince-fugnitto could you have another look?

@marcdumais-work marcdumais-work added enhancement new features or requests quality issues related to code quality ci issues related to ci - build, linting labels Jul 14, 2023
@marcdumais-work marcdumais-work changed the title [build][Juekins] more efficient Jenkins CI, builds, improved linter config [build][quality] more efficient CI, builds, improved linter config Jul 14, 2023
@marcdumais-work marcdumais-work force-pushed the sprint-cleaning branch 2 times, most recently from e1c6f37 to 6f65b08 Compare July 14, 2023 19:09
Copy link
Contributor

@jfaltermeier jfaltermeier left a 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'
}

applications/electron/package.json Outdated Show resolved Hide resolved
…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]>
@marcdumais-work
Copy link
Contributor Author

@jfaltermeier thanks for the initial look! i have rebased the PR on the latest master and added the couple suggested yarn rebuild. I will try to fix the docker problem and update the PR.

Oops - I had forgotten to build the Theia extensions, which caused a
problem at runtime.

Signed-off-by: Marc Dumais <[email protected]>
@marcdumais-work
Copy link
Contributor Author

Also the Browser Docker Image does not seem to be working yet:

I believe that I have fixed the issue with browser.Dockerfile

Copy link
Contributor

@jfaltermeier jfaltermeier left a 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!

@marcdumais-work
Copy link
Contributor Author

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?

@marcdumais-work
Copy link
Contributor Author

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.

@JonasHelming
Copy link
Contributor

@planger FYI

@marcdumais-work
Copy link
Contributor Author

@planger if it's welcomed, I can follow-up with a cdt-cloud-blueprint PR, that would update it , so it "fits" with the new Blueprint, after the merge button is pressed. I may require a bit of feedback, since I do not know the ins-and-outs of that product and how it's used. WDYT?

@planger
Copy link
Contributor

planger commented Aug 11, 2023

@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.

@marcdumais-work marcdumais-work merged commit 9bdd610 into master Aug 11, 2023
3 checks passed
marcdumais-work added a commit that referenced this pull request Aug 11, 2023
@marcdumais-work marcdumais-work deleted the sprint-cleaning branch August 11, 2023 15:01
@marcdumais-work
Copy link
Contributor Author

@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.

@planger
Copy link
Contributor

planger commented Sep 6, 2023

@marcdumais-work Ok thanks for letting me know anyways! We'll get back to you if there are questions. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to ci - build, linting enhancement new features or requests quality issues related to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants