#3518 PR merged: Fix duplicate automatic recovery start after merge of PR #3041 and related changes

Labels: bug

pcahyna opened issue at 2025-09-10 12:21:

Pull Request Details:

  • Type: Bug Fix

  • Impact: High

  • Reference to related issue (URL): https://github.com/rear/rear/pull/3041#issuecomment-3210788660

  • How was this pull request tested?

    • Full backup and unattended recovery, the console shows sysinit.service started before ReaR automated recovery (automatic-rear.service) begins starting:
Starting         
sysinit.service   
 - Initialize Rescue System...  
[      
  OK     
] Finished         
systemd-udev-trigger.service   
 - udev Coldplug all Devices.  
[      
  OK     
] Found device         
dev-ttyS0.device   
 - /dev/ttyS0.  
[    4.336554] Error: Driver 'pcspkr' is already registered, aborting...

Verifying md5sums of the files in the Relax-and-Recover rescue system 
md5sums are OK

Configuring Relax-and-Recover rescue system

Running 00-functions.sh... 
(...)
Running 99-makedev.sh...

Relax-and-Recover rescue system is ready

[      
  OK     
] Finished         
sysinit.service   
 - Initialize Rescue System.  
[      
  OK     
] Reached target         
network-online.target   
 - Network is Online.  
[      
  OK     
] Reached target         
sysinit.target   
 - System Initialization.  
[      
  OK     
] Listening on         
dbus.socket   
 - D-Bus System Message Bus Socket.  
[      
  OK     
] Listening on         
syslog.socket   
 - Syslog Socket.  
[      
  OK     
] Reached target         
sockets.target   
 - Sockets.  
[      
  OK     
] Reached target         
basic.target   
 - Basic System.  
         Starting         
automatic-rear.service   
 - …overy automatically if requested...  
[      
  OK     
] Started         
systemd-journald.service   
 - Journal Service.  
[   24.895473] systemd-journald[532]: 1 unknown file descriptors passed, closing. 
Launching 'rear recover' automatically in unattended (non-interactive) mode 
Relax-and-Recover 2.9-git.5699.8831a249.fixautomaticrearmismerge.changed / 2025-08-26 
Running rear recover (PID 548 date 2025-09-09 06:33:57)
  • full backup and unattended recovery with the line mv /usr/share/rear/conf/default.conf /usr/share/rear/conf/default.conf.bak inserted at the beginning of usr/share/rear/skel/default/etc/scripts/system-setup to test behavior when it breaks, automated recovery does not start, console log then shows:
ERROR: ReaR recovery cannot work without /usr/share/rear/conf/default.conf

[   14.292649] systemd[1]: sysinit.service: Failed with result 'exit-code'. 
[   14.293966] systemd[1]: Failed to start sysinit.service - Initialize Rescue System. 
[        
FAILED   
] Failed to start         
sysinit.service   
 - Initialize Rescue System.  
See 'systemctl status sysinit.service' for details.  
[   14.297570] systemd[1]: Dependency failed for automatic-rear.service - Run Relax-and-Recover recovery automatically if requested. 
[ [0;1;38:5:185mDEPEND   
] Dependency failed for         
automatic-re…   
ecovery automatically if requested.
  • Description of the changes in this pull request:
    • Fix botched merge of #3041 : the code tor automatic ReaR startup was left in usr/share/rear/skel/default/etc/scripts/system-setup in addition to the new copy in usr/share/rear/skel/default/etc/scripts/run-automatic-rear, so when the unattended command line is present, rear recover starts in automatic (but not unattended!) mode even before systemd finishes initialization of all services. Moreover, if that recovery fails, ReaR executes it again but now with the correct mode and order using the automatic-rear.service. Found by @lzaoral in https://github.com/rear/rear/pull/3041#issuecomment-3210788660. Fixed by removing the duplicate original code and carefully merging changes done to it since the code was copied into the new copy in order to preserve them.
    • Restore the behavior of automated recovery on errors: before #3041, if the script system-setup terminated with an error, automated recovery was not executed. After #3041 (this is not related to the merge botch), it starts in any case, as it is a separate script and service unit. Fix by introducing a hard dependency between the units.
    • Add back a mention of unattended mode when ReaR recovery starts unattended. Now it prints Launching 'rear recover' automatically in unattended (non-interactive) mode. This was lost in fec92e291024ccfd329e51ed02265f18713d28b0 by @jsmeix.
    • Shorten description of automatic-rear.service. Shorter description will not be truncated when included in system boot
      messages printed on console, resulting in better readability.
      Message before: Starting automatic-rear.service - …overy automatically if requested...
      Message after: Starting automatic-rear.service - Automatic recovery if requested...

pcahyna commented at 2025-09-10 12:23:

@jsmeix commit 8831a249e87fe06a136e12914d3c53119eb95802 revers one change that you did, please have a look if you agree.

pcahyna commented at 2025-09-10 12:28:

Note: Codacy is wrong, one does not need to export variables set in a shell function in order to be seen outside the function (not marking them as local is enough) and they can be also used in called functions / sourced scripts, as the shell uses dynamic and not lexical scoping of variables.

jsmeix commented at 2025-09-10 13:18:

@pcahyna
thank you for your fix of my code!
Your
https://github.com/rear/rear/commit/8831a249e87fe06a136e12914d3c53119eb95802
does exactly the right thing because in my
https://github.com/rear/rear/commit/fec92e291024ccfd329e51ed02265f18713d28b0
I first correctly replaced for automatic_recovery mode

echo -e "\nLaunching 'rear recover' automatically\n"

by

echo "Launching '$RECOVERY_COMMANDS_LABEL' automatically"

but subsequently
I falsely replaced for unattended_recovery mode

echo -e "\nLaunching 'rear recover' automatically in unattended (non-interactive) mode\n"

by the same as above

echo "Launching '$RECOVERY_COMMANDS_LABEL' automatically"

instead of the correct

echo "Launching '$RECOVERY_COMMANDS_LABEL' automatically in unattended (non-interactive) mode"

I am sure this happended because I did copy&paste
of my code for the automatic_recovery case
without proper adaptions to the unattended_recovery case.

pcahyna commented at 2025-09-10 14:22:

@jsmeix you are welcome and the change was actually found by @lzaoral, thanks!

jsmeix commented at 2025-09-10 14:40:

@lzaoral
thank you for your fix of my code!

pcahyna commented at 2025-09-11 13:04:

@jsmeix thanks for checking and the confirmation. I pushed to update the commit message (avoiding the "perhaps it was
intentional" part), no code changes.

pcahyna commented at 2025-09-11 18:24:

I rebased the branch on top of the current master and added another "by the way" change: I shortened the description of automatic-rear.service so that systemd does not truncate it when printing system boot messages on console.

pcahyna commented at 2025-09-12 16:52:

@rear/contributors I would like to merge it next week.

pcahyna commented at 2025-09-17 14:07:

@rear/contributors If there are no objections, I plan to merge it tomorrow before noon.

[Export of Github issue for rear/rear.]