Skip to content

fix(docker): list the apps after apps-writable is a known apps path - #1063

Merged
susnux merged 1 commit into
nextcloud-libraries:mainfrom
luflow:fix/noid/bind-mounted-apps
Aug 3, 2026
Merged

fix(docker): list the apps after apps-writable is a known apps path#1063
susnux merged 1 commit into
nextcloud-libraries:mainfrom
luflow:fix/noid/bind-mounted-apps

Conversation

@luflow

@luflow luflow commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Rebased on the current main and reduced to what is left. #1061 already fixed the chown part independently, so only the app list ordering remains.

Problem

configureNextcloud() fails as soon as an app is bind mounted into apps-writable, which startNextcloud(branch, true) does by default:

[cause]: { stdout: 'attendance already installed\n', stderr: '', exitCode: 1 }

app:list is queried before apps.config.php registers apps-writable as an apps path, so the mounted app is missing from the list. It is then not found in shipped.json either and ends up in the app store branch, where app:install --force exits non-zero with "already installed".

The repo's own test suite passes mountApp: false, so this is not covered by CI.

Fix

Build the app list only after apps.config.php has been written. Four lines moved, no behaviour change for the non mounted case.

Testing

  • npm run lint, npm run build, npx tsc --noEmit clean
  • npm run test:node 4/4 pass (unchanged behaviour, mountApp: false)
  • Against a bind mounted app (nextcloud/attendance, stable34, macOS + Docker Desktop): configureNextcloud now completes and reports attendance 1.42.0 enabled instead of aborting. The same run also confirmed that with the named volume from fix(podman): Ensure compatibility with Podman #1061 the mkdir no longer needs to run as root, so that part of my original change is gone.

Comment thread CHANGELOG.md Outdated
### Fixed
* 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

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.

this is generated on release

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad, fixed it.

@susnux susnux left a comment

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.

one comment - otherwise looks good

`app:list` was queried before `apps.config.php` registers `apps-writable` as an
apps path, so a bind mounted app is missing from the list. It is then not found
in `shipped.json` either and ends up in the app store branch, where
`app:install --force` exits non-zero with "<app> already installed".

Signed-off-by: Florian Ludwig <florian@krautnerds.de>
@luflow
luflow force-pushed the fix/noid/bind-mounted-apps branch from 3c205ee to 137ccb7 Compare August 2, 2026 20:40
@luflow luflow changed the title fix(docker): make configureNextcloud work with bind mounted apps fix(docker): list the apps after apps-writable is a known apps path Aug 2, 2026
@luflow

luflow commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@susnux heads up, I rebased this on the current main and updated the title and description, so the PR is a lot smaller now.

#1061 landed in the meantime and already contains the chown ... user: 'root' fix I had here. It also replaced the Volumes block with a proper named volume mount, which means mkdir -p no longer needs to run as root either. I verified that against a bind mounted app on stable34.

What is left is only the app list ordering: app:list runs before apps.config.php registers apps-writable, so a mounted app is missing from the list and ends up in the app store branch where app:install --force exits non-zero with "already installed". Four lines moved.

The CHANGELOG change is gone as well, thanks for the pointer.

@susnux
susnux merged commit 137ccb7 into nextcloud-libraries:main Aug 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants