Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@

All notable changes to this project will be documented in this file.

## Unreleased
### Added
* `runExec` supports a `workingDir` option.
* `NEXTCLOUD_E2E_COMPOSER_VERSION` pins the Composer version used to complete cloned shipped apps,
the latest stable release is used if unset.

### Fixed
* fix(docker): install the Composer dependencies of cloned shipped apps \([\#1059](https://github.com/nextcloud-libraries/nextcloud-e2e-test-server/issues/1059)\)
* fix(docker): create `apps-writable` as root so bind mounted apps can be used
* fix(docker): list the apps only after `apps-writable` is a known apps path

## v0.5.0 - 2026-07-02
### Breaking changes
`runExec` and `runOcc` now return an object instead of the plain output string.
Expand Down
147 changes: 140 additions & 7 deletions lib/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ import { User } from './User.ts'

const SERVER_IMAGE = 'ghcr.io/nextcloud/continuous-integration-shallow-server'

// The server image ships PHP but no Composer, so it is downloaded on demand.
// Set `NEXTCLOUD_E2E_COMPOSER_VERSION` to pin a version instead of using the latest one.
const COMPOSER_VERSION = process.env.NEXTCLOUD_E2E_COMPOSER_VERSION || 'latest-stable'
const COMPOSER_PHAR = '/tmp/composer.phar'
/** `COMPOSER_HOME` used inside the server container, must be writable by `www-data` */
const COMPOSER_HOME = '/tmp/composer-home'

export const docker = new Docker()

// Store the container name, different names are used to prevent conflicts when testing multiple apps locally
Expand Down Expand Up @@ -262,13 +269,11 @@ export async function configureNextcloud(apps = ['viewer'], vendoredBranch?: str
}
console.log('│ └─ OK !')

// Build app list
const { stdout: json } = await runOcc(['app:list', '--output', 'json'], { container })
const applist = JSON.parse(json)

console.log('├─ Using "apps-writable" folder for mounted apps')
await runExec(['mkdir', '-p', '/var/www/html/apps-writable'], { container })
await runExec(['chown', 'www-data:www-data', '/var/www/html/apps-writable'], { container })
// Bind mounting an app makes Docker pre-create the directory owned by root,
// so both commands have to run as root to be able to hand it over to `www-data`
await runExec(['mkdir', '-p', '/var/www/html/apps-writable'], { container, user: 'root' })
await runExec(['chown', 'www-data:www-data', '/var/www/html/apps-writable'], { container, user: 'root' })
const appsConfig = `<?php
$CONFIG = [
'apps_paths' => [
Expand All @@ -289,6 +294,10 @@ export async function configureNextcloud(apps = ['viewer'], vendoredBranch?: str
stream.finalize()
await container.putArchive(stream, { path: '/var/www/html/config' })

// Build app list, only now that "apps-writable" is a known apps path so that mounted apps show up
const { stdout: json } = await runOcc(['app:list', '--output', 'json'], { container })
const applist = JSON.parse(json)

// Enable apps and give status
for (const app of apps) {
if (app in applist.enabled) {
Expand All @@ -305,6 +314,7 @@ export async function configureNextcloud(apps = ['viewer'], vendoredBranch?: str
['git', 'clone', '--depth=1', ...branchOption, `https://github.com/nextcloud/${encodeURIComponent(app)}.git`, `apps-writable/${app}`],
{ container, verbose: true },
)
await completeClonedApp(app, container)
await runOcc(['app:enable', '--force', app], { container, verbose: true })
} else {
// try appstore
Expand All @@ -315,6 +325,123 @@ export async function configureNextcloud(apps = ['viewer'], vendoredBranch?: str
console.log('└─ Nextcloud is now ready to use 🎉')
}

/**
* Check whether a path exists inside the container
*
* @param path Absolute path to check
* @param container The server container to use
*/
async function pathExists(path: string, container: Container): Promise<boolean> {
const { exitCode } = await runExec(['test', '-e', path], { container, failOnError: false })
return exitCode === 0
}

/**
* Reduce command output to its last lines so that it fits on a single log line
*
* @param output The output to shorten
*/
function lastLines(output: string): string {
return output.trim().split('\n').slice(-3).join(' | ')
}

/**
* Check whether the Composer autoloader of a cloned app can be loaded
*
* Apps that do not commit their `vendor` directory have `composer/autoload.php` require the git
* ignored `vendor/autoload.php`. Loading the file is what `OC_App::registerAutoloading()` does, so
* this detects the apps that would make every request and every `occ` call fatal.
*
* @param appPath Absolute path of the app inside the container
* @param container The server container to use
*/
async function isComposerAutoloaderBroken(appPath: string, container: Container): Promise<boolean> {
if (!await pathExists(`${appPath}/composer/autoload.php`, container)) {
return false
}
const { exitCode } = await runExec(['php', '-f', `${appPath}/composer/autoload.php`], { container, failOnError: false })
return exitCode !== 0
}

/**
* Make a shipped app that was installed by cloning it loadable
*
* If the Composer dependencies cannot be installed, the `composer` directory is dropped so that the
* server registers PSR-4 for the app's `lib` directory itself. The app then loads but is incomplete.
*
* @param app The app id, cloned to `apps-writable/<app>`
* @param container The server container to use
*/
async function completeClonedApp(app: string, container: Container) {
const appPath = `/var/www/html/apps-writable/${app}`
if (!await isComposerAutoloaderBroken(appPath, container)) {
return
}

console.log(`│ ├─ ${app} was cloned without the Composer dependencies its autoloader needs`)
if (await installComposerDependencies(app, appPath, container)) {
return
}

await runExec(['rm', '-rf', `${appPath}/composer`], { container })
console.log(`│ ├─ Removed '${app}/composer' to make the app loadable without its Composer autoloader`)
console.log(`│ └─ ⚠️ ${app} is incomplete, code using its Composer dependencies (e.g. 'lib/Vendor') will fail`)
}
Comment on lines +375 to +389

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not simply:
IF composer.json exists
THEN composer install --no-dev

?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That was my first attempt, but it breaks on the two different layouts these apps use.

viewer and text have a second composer project in composer/composer.json with "vendor-dir": ".", so composer/ is their vendor dir and it's committed (composer/composer/autoload_real.php is right there in the repo). Their root composer.json is just dev tooling: viewer has an empty require, text only requires php.

notifications is the newer layout. Root composer.json is the runtime project, the vendor dir is the default vendor and git ignored, and composer/autoload.php is a hand written shim that requires it.

So "root composer.json exists, run install" would install the dev tooling project for viewer and text, while their actual runtime autoloader in composer/ is already fine. Nothing gets fixed, it just costs time.

The bigger problem is the combination with the rm -rf composer fallback. If that pointless install fails for any reason, we'd delete a committed, working vendor dir and break an app that was healthy. Running the autoloader only touches apps that would really make the server fatal, and it costs two docker exec round trips per cloned app.

Happy to go your way if you still prefer it. I'd just keep the fallback behind the autoloader check so a working composer/ can never get removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So "root composer.json exists, run install" would install the dev tooling project for viewer and text, while their actual runtime autoloader in composer/ is already fine. Nothing gets fixed, it just costs time.

Why? I checked both cases and they do not have runtime dependencies.
So yes would waste time but a small amount only because with --no-dev nothing is installed.

The bigger problem is the combination with the rm -rf composer fallback.

Why would that be needed? I never had problem cloning any shipped app inside a Nextcloud checkout.
git clone + composer i --no-dev always works for all shipped apps I tried to clone (including text, notifications, activity, viewer, files_downloadlimit etc).

we'd delete a committed, working vendor dir

Which apps checks in vendor? This sound not really a good thing to do 👀


/**
* Install the Composer dependencies of an app inside the container
*
* Scripts are run on purpose, apps such as `notifications` only assemble the prefixed copies of
* their dependencies (`lib/Vendor`) in `post-install-cmd`.
*
* @param app The app id
* @param appPath Absolute path of the app inside the container
* @param container The server container to use
* @return Whether the app's Composer autoloader is usable afterwards
*/
async function installComposerDependencies(app: string, appPath: string, container: Container): Promise<boolean> {
if (!await ensureComposer(container)) {
return false
}

console.log(`│ ├─ Running 'composer install' for ${app}…`)
const { exitCode, stderr } = await runExec(
['php', COMPOSER_PHAR, 'install', '--no-dev', '--no-interaction', '--no-progress', '--no-ansi'],
{ container, workingDir: appPath, env: [`COMPOSER_HOME=${COMPOSER_HOME}`], failOnError: false },
)
if (exitCode !== 0 || await isComposerAutoloaderBroken(appPath, container)) {
console.log(`│ ├─ 🛑 'composer install' failed for ${app}: ${lastLines(stderr)}`)
return false
}

console.log(`│ └─ Composer dependencies of ${app} installed`)
return true
}

/**
* Download the Composer binary into the container, unless it is already there
*
* @param container The server container to use
* @return Whether the binary is available
*/
async function ensureComposer(container: Container): Promise<boolean> {
if (await pathExists(COMPOSER_PHAR, container)) {
return true
}

console.log(`│ ├─ Downloading Composer ${COMPOSER_VERSION} into the container…`)
const url = `https://getcomposer.org/download/${COMPOSER_VERSION}/composer.phar`
const { exitCode, stderr } = await runExec(
['curl', '--silent', '--show-error', '--location', '--fail', '--output', COMPOSER_PHAR, url],
{ container, failOnError: false },
)
if (exitCode !== 0) {
console.log(`│ ├─ 🛑 Could not download Composer: ${lastLines(stderr)}`)
return false
}
return true
}

/**
* Setup test users
*
Expand Down Expand Up @@ -440,6 +567,10 @@ interface RunExecOptions {
* If true, the command's output will be printed to the console. Defaults to false.
*/
verbose: boolean
/**
* Working directory to run the command in. Defaults to the Nextcloud root.
*/
workingDir: string
}

type RunExecResult = {
Expand All @@ -458,10 +589,11 @@ type RunExecResult = {
* @param options.verbose - If true, the command's output will be printed to the console. Defaults to false.
* @param options.env - Environment variables to set for the command. Defaults to an empty array.
* @param options.failOnError - The command will throw an error if it exits with a non-zero exit code. Defaults to true.
* @param options.workingDir - Working directory to run the command in. Defaults to the Nextcloud root.
*/
export async function runExec(
command: string | string[],
{ container, user = 'www-data', verbose = false, env = [], failOnError = true }: Partial<RunExecOptions> = {},
{ container, user = 'www-data', verbose = false, env = [], failOnError = true, workingDir }: Partial<RunExecOptions> = {},
): Promise<RunExecResult> {
container = container || getContainer()
const exec = await container.exec({
Expand All @@ -470,6 +602,7 @@ export async function runExec(
AttachStderr: true,
User: user,
Env: env,
WorkingDir: workingDir,
})

return new Promise<RunExecResult>((resolve, reject) => {
Expand Down
10 changes: 9 additions & 1 deletion tests/docker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('Docker: Pre-installation of apps', async () => {
before(async () => {
const ip = await startNextcloud('master', false, { forceRecreate: true })
await waitOnNextcloud(ip)
await configureNextcloud(['viewer', 'text', 'forms'])
await configureNextcloud(['viewer', 'text', 'forms', 'notifications'])
})

after(async () => await stopNextcloud())
Expand All @@ -37,6 +37,14 @@ describe('Docker: Pre-installation of apps', async () => {
const { enabled } = await getAppsList()
expect.equal('forms' in enabled, true, 'Forms app should be enabled')
})

await test('Additional apps: apps that do not commit their dependencies get them installed', async () => {
const container = getContainer()
// this must not throw
await runExec(['test', '-f', 'apps-writable/notifications/vendor/autoload.php'], { container })
const { enabled } = await getAppsList()
expect.equal('notifications' in enabled, true, 'Notifications app should be enabled')
})
})

async function getAppsList(): Promise<{ enabled: Record<string, string>, disabled: Record<string, string> }> {
Expand Down