fix(backup): support older rclone on restore and add a backup --list preflight#478
Merged
mrrobot47 merged 1 commit intoJun 12, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves robustness of EasyEngine’s rclone-backed site backup/restore flow by (1) avoiding restore failures on older rclone versions that hard-fail on unknown flags, and (2) ensuring ee site backup --list runs the same friendly rclone/config preflight as backup/restore paths.
Changes:
- Gate newer rclone multi-thread download flags (
--multi-thread-write-buffer-size,--multi-thread-chunk-size) behind detected rclone version. - Add
get_rclone_version()and minimum-version constants to drive the gating behavior (safe default when version cannot be determined). - Extract rclone presence/backend checks into
check_rclone_available()and invoke it on the--listpath and frompre_backup_restore_checks().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two robustness follow-ups to the rclone memory-budgeting work merged in EasyEngine#477: 1) Restore on older rclone (regression). rclone_download() unconditionally passed --multi-thread-write-buffer-size (added in rclone v1.63.0) and --multi-thread-chunk-size (added in v1.64.0). Older rclone hard-fails on unknown flags (Fatal error: unknown flag, exit code 2), so any rclone < 1.63 -- common in distro packages -- broke restore/rollback. Both flags are now gated behind the detected rclone version (get_rclone_version()), with thresholds in RCLONE_MIN_VERSION_MT_* constants. The memory budget already assumes their defaults, so gating them out changes only which knobs reach rclone, not the computed footprint; --transfers/--buffer-size/--multi-thread-streams (all >= v1.48, long-standing) are always passed. When the version cannot be determined, the newer flags are skipped (safe). 2) Friendly preflight on read paths. The rclone-presence and backend-config checks lived inside pre_backup_restore_checks(), which the 'backup --list' branch and the 'restore --id' branch (via verify_backup_id() -> rclone lsf) both reach before, so a missing rclone surfaced as a raw 'sh: 1: rclone: not found' (--list) or a misleading 'Invalid backup ID' (restore --id). Extracted the checks into check_rclone_available() and call it on both read paths (and from pre_backup_restore_checks() for the main backup/restore flows).
51fac0a to
2465122
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/helper/Site_Backup_Restore.php:893
- The rclone presence check runs
rclone --version, which can still emit a raw shell error likesh: 1: rclone: not found(and can also print the version on success). Since the goal here is a clean, friendly preflight, prefer checking availability viacommand -v rclone(silenced) before any rclone invocation.
$command = 'rclone --version';
$return_code = EE::exec( $command );
if ( ! $return_code ) {
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
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.
Summary
Two robustness follow-ups to the rclone memory-budgeting work merged in #477, both addressing issues reported from the field:
ee site backup --listshows a rawsh: 1: rclone: not foundwhen rclone is missing, instead of the friendly install message that already exists for other paths.1 — Gate multi-thread download flags by rclone version
#477's
rclone_download()unconditionally passes--multi-thread-write-buffer-sizeand--multi-thread-chunk-size. These flags are recent, and rclone hard-fails on unknown flags (Fatal error: unknown flag, exit code 2) — it does not ignore them — so any older rclone aborts the copy and restore/rollback fails entirely. Distro-packaged rclone is frequently older than these versions.Confirmed introduction versions (rclone changelog):
--multi-thread-streams,--multi-thread-cutoff--multi-thread-write-buffer-size--multi-thread-chunk-sizeThe two newer flags are now gated behind the detected rclone version (
get_rclone_version()), with thresholds inRCLONE_MIN_VERSION_MT_*constants:< 1.63→ only--multi-thread-streams(+--transfers/--buffer-size), all available since v1.48.>= 1.63→ adds--multi-thread-write-buffer-size.>= 1.64→ adds--multi-thread-chunk-size.The memory budget already assumes these flags' defaults, so gating them out changes only which knobs reach rclone, not the computed footprint — the OOM protection from #477 is unaffected on every rclone version.
2 — Friendly preflight for
ee site backup --listThe rclone-presence and backend-config checks (with their friendly "rclone is not installed… install via https://rclone.org/downloads/" messages) lived inside
pre_backup_restore_checks(). The--listbranch returns before that runs, so a missing rclone surfaced as a rawsh: 1: rclone: not found.Extracted those checks into
check_rclone_available()and call it on the--listpath (and frompre_backup_restore_checks()for the backup/restore paths), so--listnow gets the same clear guidance.Testing
php -l src/helper/Site_Backup_Restore.phppasses.ee site backup --listnow routes throughcheck_rclone_available()before any rclone invocation.Notes