Merged
Conversation
joshuacurtiss
added
bug
There was a problem hiding this comment.
Pull request overview
This PR consolidates multiple fixes and features aimed at achieving "Version 1.0" of the mdl release. The changes improve developer experience, fix bugs, and make the codebase more resilient to different Docker images and configurations by removing hard-coded Bitnami references.
Key changes:
- Integrated fast database backup/restore functionality into main backup/restore scripts, eliminating separate fastdb scripts
- Fixed plugin installer to correctly parse
version.phpcomponent names, enabling support for all public Moodle plugins - Added environment name validation and custom compose file configuration support
- Replaced
whichcommand withcommand -vfor better cross-platform compatibility - Implemented dynamic path detection via container inspection instead of hard-coded paths
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| libexec/mdl-stop.sh | Added check to skip processing if compose path cannot be determined |
| libexec/mdl-status.sh | Added check to skip processing if compose path cannot be determined |
| libexec/mdl-start.sh | Added check to skip processing if compose path cannot be determined |
| libexec/mdl-restore.sh | Integrated fast database restore with --fastdb flag; uses dynamic paths and parallel compression; removed hard-coded Bitnami references |
| libexec/mdl-logs.sh | Added error handling for missing compose path |
| libexec/mdl-list.sh | Updated label extraction to use awk pattern matching for better parsing |
| libexec/mdl-install-plugin.sh | Fixed to parse component name from version.php instead of directory name for proper plugin type/name detection |
| libexec/mdl-init.sh | Added environment name validation; changed default compose file to default.yml; added COMPOSE_FILE configuration option; uses dynamic path detection; improved version parsing |
| libexec/mdl-info.sh | Added COMPOSE_FILE to displayed configuration; replaced which with command -v |
| libexec/mdl-fast-db-restore.sh | Deleted - functionality merged into mdl-restore.sh |
| libexec/mdl-fast-db-backup.sh | Deleted - functionality merged into mdl-backup.sh |
| libexec/mdl-cli.sh | Uses dynamic path detection instead of hard-coded /bitnami/moodle |
| libexec/mdl-calc-compose-path.sh | Enhanced to support custom COMPOSE_FILE from environment config with fallback logic |
| libexec/mdl-box.sh | Replaced which with command -v |
| libexec/mdl-backup.sh | Integrated fast database backup with --fastdb flag; uses dynamic paths, docker cp, and parallel compression; improved module filtering logic |
| lib/mdl-common.sh | Added calc_compression_tool for parallel compression support; replaced which with command -v; updated update_config to use dynamic path detection |
| installers/uninstall.sh | Replaced which with command -v |
| installers/install.sh | Replaced which with command -v; updated default compose file reference |
| custom-words.txt | Added parallel compression tool names (bsdtar, pbzip, pigz, pixz) |
| compose/default.yml | Changed MariaDB volume mount from /bitnami/mariadb to /bitnami/mariadb/data |
Comments suppressed due to low confidence (1)
compose/default.yml:17
- The volume mount point has changed from
/bitnami/mariadbto/bitnami/mariadb/data. This is a breaking change for existing environments that were using the old path. When users upgrade and use the new compose file, the database container will try to use a different path, potentially losing access to existing database files. Consider documenting this breaking change or providing a migration path for existing installations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@akamal4 There are a lot of good tips in copilot's review ☝🏻 so I'm going to try and apply some of these points before sending the PR to you. |
Originally, it was not desirable to wrap fastdb logic into the official backup/restore scripts, since fastdb is an unsafe backup approach. However, with the change where the DB restore now occurs immediately as part of the restore (instead of just decompressing a SQL dump file for when the container starts), we lose the ability to utilize fastdb to expedite a restore in development. By incorporating fastdb into the backup/restore scripts, we can reestablish the DX feature of being able to rapidly restore an environment. Example usage: Make a normal backup with a fastdb archive, with no compression: mdl backup mymoodle container -l wow --fastdb -c none Restore with fastdb: mdl restore mymoodle wow --fastdb
joshuacurtiss
force-pushed
the
jcurt/misc-fixes
branch
from
f3bc658 to
ae2e66d
Compare
Whereas it is good practice to use your component name as the output directory of a zipped plugin, so plugin authors do not use the exact frankenstyle component name. However, they do use the correct frankenstyle name in version.php because it is required. To make `mdl install-plugin` more compatible, we update it to inspect version.php and get the component name (and thus type and install path) from there.
joshuacurtiss
force-pushed
the
jcurt/misc-fixes
branch
from
ae2e66d to
9d730c9
Compare
We want it to just list versions as x.y (i.e. 4.5), but when we changed versions.txt to use the entire image page, we broke this. By piping `cut -d':' -f2`, we only look at the tag of the image, which is our desired effect. Fixes #15.
joshuacurtiss
force-pushed
the
jcurt/misc-fixes
branch
from
9d730c9 to
f34c1bf
Compare
The rule for Moodle and Docker names: Must consist only of lowercase alphanumeric characters, hyphens, and underscores as well as start with a letter or number. We discovered two issues: - If a name had invalid characters, the project will fail to start up. - If a name has an underscore (which is valid), backup operations are buggy. To fix this, we add name validation in `mdl init`, and we improve spots where labels are calculated for backup handling. Instead of referencing the second or third item when separating the name into a list by underscores, we remove the start and end parts of the name, leaving just the full label. We can also combine some of the logic into a single awk command to make it cleaner. Fixes #9.
Recently, macOS changed its implementation of `which`, causing it to return its error message on stdout instead of stderr when a command is not found. This breaks conventional usage of `which` in scripts to check for command existence. To address this, we replace all instances of `which <command>` with `command -v <command>` which is more reliable and consistent across different Unix-like systems.
Use standard `docker cp` tooling to transfer tar archives of the volumes, instead of running tar within the container. Also, if pbzip2, pigz, or pixz exist on the system, they will be used, instead of their standard compression tool equivalents that are only single-threaded (bzip2, gzip, xz). If bsdtar is available on the system, it will use that to filter the tar stream of the paths that don't need to be included in the backup. However, if bsdtar is not available, we also handle removing these paths during the restore process as well. So, we always ensure the caches and sessions are eliminated during restore. Closes #6.
Future Moodle images may not be based on Bitnami and may have different src/data paths. We try to dynamically calculate these paths by querying the container of its mount points. This approach helped us realize that the DB volume really is pointed at the file system incorrectly for the Bitnami implementation of MariaDB. Should be /bitnami/mariadb/data, not /bitnami/mariadb. This change is more accurate and also will make it compatible with other non-Bitnami images for MySQL/MariaDB databases. Closes #13.
Changing the compose file name from `compose.yml` to `default.yml` paves the way for the provided compose file to be the default configuration but to support customize configs as well. See #14.
joshuacurtiss
force-pushed
the
jcurt/misc-fixes
branch
from
f34c1bf to
1f45b54
Compare
Whereas the default.yml compose file will be used if an environment does not specify a custom configuration with the `COMPOSE_FILE` variable, this feature improves the `mdl calc-compose-path` script to actually do some work: It will check the existence of the compose file path (first relative to the `MDL_COMPOSE_DIR` directory, then as an absolute path), and it will output the path of the compose file, and throw an error if the compose file does not exist. This will enable users to customize their compose files. Closes #14.
MariaDB can safely run without using privileged mode. Moodle itself may need privileged mode is some scenarios, especially when using podman, so for now we leave privileged mode enabled for the Moodle container.
joshuacurtiss
force-pushed
the
jcurt/misc-fixes
branch
from
1f45b54 to
95dbec9
Compare
akamal4
approved these changes
Dec 23, 2025
Contributor
akamal4
left a comment
akamal4
left a comment
There was a problem hiding this comment.
Reviewed, Tested, and works without issue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR address numerous fixes or features that we desire to be in what's considered "Version 1.0" of the mdl release. Notably:
mdl restore, you can now opt to use the fast database archive in lieu of a traditional database dump file when doing the restore.mdl install-pluginto work with public plugins. Whereas the installer was working with our authored plugins, it technically wasn't getting naming convention fromversion.phpas it should, so other public plugins were potentially breaking. This fixesmdl install-pluginto work correctly for all.mdl init#15 Fix bug in the Moodle version list onmdl initwhichcommand isn't behaving accordingly to standard Linux conventions.docker cpto copy volume data..envfile to have aCOMPOSE_FILEconfiguration to point to a custom compose file without needing to customize any existing code. (i.e. We don't want the user to customize our standard compose.yml file in case we update it in the future, plus we want the user to be able to have multiple compose files potentially.)🚨 Warning: Breaking Changes
/var/moodle/compose/compose.ymlfile has changed to/var/moodle/compose/default.yml, so this PR will require an uninstall/reinstall whether running in dev mode or not, so that the compose file is named properly./bitnami/mariadbto/bitnami/mariadb/data, which is where the mount point should be for Bitnami's implementation of MariaDB to be consistent with other platforms, such as MySQL or genuine MariaDB, which do not keep an additional-leveldatadirectory like Bitnami's image does. A simple backup before applying the new code, then apply/install the new code, and then restore from the backup with a traditional dump file (not a "Fast DB" backup), will get the data files restored correctly. Same with a live server situation: Just backup right before, install the new version, and restore the backup with the new version. The database image will be fixed.These breaks demonstrate why this is not formally released as a public 1.0 project yet, as we want to get these breaking changes out first.