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 apps into the container image #17

Merged
merged 8 commits into from
Jul 11, 2024
Merged

Conversation

thlehmann-ionos
Copy link

@thlehmann-ionos thlehmann-ionos commented Jul 8, 2024

Build apps into the container image:

  • Custom-apps (simplesettings)
  • External-apps (user_oidc)

This requires a build script change too: IONOS-Productivity/nc-config#8

You can inspect the build image by

$ podman pull ghcr.io/ionos-productivity/nc-server:tl-dev-build-apps
$ podman image save ghcr.io/ionos-productivity/nc-server:tl-dev-build-apps > /tmp/foo.tar

$ cd /tmp
$ mkdir image-contents
$ tar xf /tmp/foo.tar -C image-contents
$ cd image-contents/

$ jq '.[0].Layers[1]' manifest.json
"774b8e7173159788a0d2722760c4e271a1c6a52f935288a79661285c7729c7e9.tar"

# inspect contents
$ tar tf 774b8e7173159788a0d2722760c4e271a1c6a52f935288a79661285c7729c7e9.tar | less

@thlehmann-ionos thlehmann-ionos changed the base branch from master to ionos-dev July 8, 2024 14:40
@thlehmann-ionos thlehmann-ionos deleted the tl/dev/build-apps branch July 8, 2024 14:44
@thlehmann-ionos thlehmann-ionos restored the tl/dev/build-apps branch July 8, 2024 14:44
@thlehmann-ionos thlehmann-ionos force-pushed the tl/dev/build-apps branch 3 times, most recently from d94a875 to 6aaf768 Compare July 8, 2024 15:25
@thlehmann-ionos
Copy link
Author

Hm, app build failed with

ERROR in [eslint] Failed to load plugin '@typescript-eslint' declared in '.eslintrc.js » @nextcloud/eslint-config#overrides[0] » @vue/eslint-config-typescript/recommended » /home/runner/work/nc-server/nc-server/apps-custom/simplesettings/node_modules/@vue/eslint-config-typescript/index.js': Cannot find module '@typescript-eslint/eslint-plugin'

Yet, when building the dev image it build nicely:

www-data@nc-dev-container:/var/www/html/apps-custom/simplesettings$ rm -rf node_modules/* ; npm ci ; npm --version
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@nextcloud/[email protected]',
npm warn EBADENGINE   required: { node: '^20.0.0', npm: '^9.0.0' },
npm warn EBADENGINE   current: { node: 'v20.15.0', npm: '10.7.0' }
npm warn EBADENGINE }
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@nextcloud/[email protected]',
npm warn EBADENGINE   required: { node: '^20.0.0', npm: '^9.0.0' },
npm warn EBADENGINE   current: { node: 'v20.15.0', npm: '10.7.0' }
npm warn EBADENGINE }
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@nextcloud/[email protected]',
npm warn EBADENGINE   required: { node: '^20.0.0', npm: '^9.0.0' },
npm warn EBADENGINE   current: { node: 'v20.15.0', npm: '10.7.0' }
npm warn EBADENGINE }
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: '@nextcloud/[email protected]',
npm warn EBADENGINE   required: { node: '^20.0.0', npm: '^9.0.0' },
npm warn EBADENGINE   current: { node: 'v20.15.0', npm: '10.7.0' }
npm warn EBADENGINE }
npm warn deprecated [email protected]: Please upgrade to consolidate v1.0.0+ as it has been modernized with several long-awaited fixes implemented. Maintenance is supported by Forward Email at https://forwardemail.net ; follow/watch https://github.com/ladjs/consolidate for updates and release changelog
npm warn deprecated [email protected]: Vue 2 has reached EOL and is no longer actively maintained. See https://v2.vuejs.org/eol/ for more details.

added 1091 packages, and audited 1092 packages in 4s

296 packages are looking for funding
  run `npm fund` for details

5 vulnerabilities (4 moderate, 1 high)

To address issues that do not require attention, run:
  npm audit fix

Some issues need review, and may require choosing
a different dependency.

Run `npm audit` for details.
10.7.0
www-data@nc-dev-container:/var/www/html/apps-custom/simplesettings$ npm run build ; npm run lint

> [email protected] build
> NODE_ENV=production webpack --config webpack.js --progress

Building simplesettings 1.0.0 

assets by status 3.67 MiB [big]
  asset simplesettings-vendors-node_modules_nextcloud_dialogs_dist_chunks_FilePicker-DUbP4INd_mjs.js?v=4f95a0dc40aa1caf3154 2.44 MiB [emitted] [immutable] [minimized] [big] (id hint: vendors) 2 related assets
  asset main.js 1.23 MiB [compared for emit] [minimized] [big] (name: main) 2 related assets
asset simplesettings-data_image_svg_xml_3c_21--_20-_20SPDX-FileCopyrightText_202020_20Google_20Inc_20-_20SPDX-Lice-cc29b1.js?v=5f14bd6d6668e4b120db 1.67 KiB [emitted] [immutable] [minimized]
asset simplesettings-node_modules_nextcloud_dialogs_dist_chunks_index-CYiQsZoY_mjs.js?v=f58059ec8cbe972fa6ae 565 bytes [emitted] [immutable] [minimized] 1 related asset
orphan modules 3.61 MiB [orphan] 585 modules
runtime modules 6.56 KiB 11 modules
cacheable modules 9.03 MiB
  modules by path ./node_modules/ 8.78 MiB 326 modules
  modules by path ./src/ 222 KiB
    modules by path ./src/components/ 15.9 KiB 6 modules
    + 2 modules
  modules by path ../../node_modules/path/ 32.7 KiB
    modules by path ../../node_modules/path/node_modules/util/ 15.4 KiB 2 modules
    ../../node_modules/path/path.js 16.7 KiB [built] [code generated]
    ../../node_modules/path/node_modules/inherits/inherits_browser.js 672 bytes [built] [code generated]
  asset modules 1.45 KiB
    data:image/svg+xml,%3c%21--%20-%20S.. 400 bytes [built] [code generated]
    data:image/svg+xml,%3c%21--%20-%20S.. 346 bytes [built] [code generated]
    + 2 modules

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  main.js (1.23 MiB)
  simplesettings-vendors-node_modules_nextcloud_dialogs_dist_chunks_FilePicker-DUbP4INd_mjs.js?v=4f95a0dc40aa1caf3154 (2.44 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (1.23 MiB)
      main.js


webpack 5.92.1 compiled with 2 warnings in 12217 ms

> [email protected] lint
> eslint src

@thlehmann-ionos
Copy link
Author

Ah, I see, this was because the dependency was resolvable via Nextcloud node_modules/ directory two levels up.

Now also fails in the dev container after removing the upper node_modules:

www-data@nc-dev-container:/var/www/html/apps-custom/simplesettings$ rm -rf ../../node_modules/*
www-data@nc-dev-container:/var/www/html/apps-custom/simplesettings$ npm run build ; npm run lint

> [email protected] build
> NODE_ENV=production webpack --config webpack.js --progress

Building simplesettings 1.0.0 

assets by status 3.66 MiB [cached] 4 assets
orphan modules 3.61 MiB [orphan] 585 modules
runtime modules 6.56 KiB 11 modules
cacheable modules 9.01 MiB
  modules by path ./node_modules/ 8.79 MiB 327 modules
  modules by path ./src/ 222 KiB
    modules by path ./src/components/ 15.9 KiB
      modules by path ./src/components/security/*.vue 6.16 KiB 4 modules
      + 2 modules
    ./src/main.ts + 72 modules 205 KiB [built] [code generated]
    ./node_modules/css-loader/dist/cjs.js!./node_modules/vue-loader/lib/loaders/stylePostLoader.js!./node_modules/sass-loader/dist/cjs.js!./node_modules/vue-loader/lib/index.js??vue-loader-options!./src/App.vue?vue&type=style&index=0&id=5b81281e&prod&scoped=true&lang=scss 856 bytes [built] [code generated]
  asset modules 1.45 KiB
    data:image/svg+xml,%3c%21--%20-%20S.. 400 bytes [built] [code generated]
    data:image/svg+xml,%3c%21--%20-%20S.. 346 bytes [built] [code generated]
    data:image/svg+xml,%3c%21--%20-%20S.. 343 bytes [built] [code generated]
    data:image/svg+xml,%3c%21--%20-%20S.. 395 bytes [built] [code generated]

ERROR in [eslint] Failed to load plugin '@typescript-eslint' declared in '.eslintrc.js » @nextcloud/eslint-config#overrides[0] » @vue/eslint-config-typescript/recommended » /var/www/html/apps-custom/simplesettings/node_modules/@vue/eslint-config-typescript/index.js': Cannot find module '@typescript-eslint/eslint-plugin'
Require stack:
- /var/www/html/apps-custom/simplesettings/__placeholder__.js
Referenced from: /var/www/html/apps-custom/simplesettings/node_modules/@vue/eslint-config-typescript/index.js

webpack 5.92.1 compiled with 1 error in 10174 ms

> [email protected] lint
> eslint src


Oops! Something went wrong! :(

ESLint: 8.57.0

ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin".

(The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/var/www/html/apps-custom/simplesettings".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

    npm install @typescript-eslint/eslint-plugin@latest --save-dev

The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in ".eslintrc.js » @nextcloud/eslint-config#overrides[0] » @vue/eslint-config-typescript/recommended » /var/www/html/apps-custom/simplesettings/node_modules/@vue/eslint-config-typescript/index.js".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

@thlehmann-ionos
Copy link
Author

@thlehmann-ionos thlehmann-ionos force-pushed the tl/dev/build-apps branch 2 times, most recently from 0cdcb6f to b6a2884 Compare July 9, 2024 14:23
@printminion-co printminion-co self-requested a review July 10, 2024 08:32
@thlehmann-ionos thlehmann-ionos marked this pull request as ready for review July 10, 2024 08:34
Copy link

@printminion-co printminion-co left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review ok if changes are addressed

.github/workflows/easycloud-build.yml Outdated Show resolved Hide resolved
.github/workflows/easycloud-build.yml Outdated Show resolved Hide resolved
.github/workflows/easycloud-build.yml Outdated Show resolved Hide resolved
Latest changes:

* app install / enable refactoring
* enabling apps

Signed-off-by: Thomas Lehmann <[email protected]>
Actually required for the custom and external apps. Since the
workflow starts in the project root, it initially "sees" the
Nextcloud package.json.

Since entering each app directory and setting up different Node/npm
versions is no feasible anyway, let's set it here.

Signed-off-by: Thomas Lehmann <[email protected]>
To track it as submodule in version control

Signed-off-by: Thomas Lehmann <[email protected]>
Exclude node_modules, tests and GitHub/Gut dirs too.

Signed-off-by: Thomas Lehmann <[email protected]>
…pps-external/

Build simplesettings (custom) and user_oidc (external)

Signed-off-by: Thomas Lehmann <[email protected]>
* Enable custom apps too
* user_oidc will no longer be downloaded

Signed-off-by: Thomas Lehmann <[email protected]>
Fix missing package which prevents from-scratch build (updated amended
commit)

Signed-off-by: Thomas Lehmann <[email protected]>
@thlehmann-ionos thlehmann-ionos merged commit 7c5dae4 into ionos-dev Jul 11, 2024
3 of 4 checks passed
@thlehmann-ionos thlehmann-ionos deleted the tl/dev/build-apps branch July 11, 2024 08:13
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants