#2676 Issue `closed`: False ERROR ".../outputfs not empty, cannot remove" when OUTPUT_URL is unset¶

Labels: cleanup, fixed / solved / done

jsmeix opened issue at 2021-09-07 15:33:¶

Cf. https://github.com/rear/rear/issues/2667#issuecomment-914358940

I use this one-line etc/rear/local.conf

OUTPUT=ISO

With current GitHub master code

# usr/sbin/rear -v mkrescue
...
Wrote ISO image: /root/rear.github.master/var/lib/rear/output/rear-linux-h9wr.iso (77M)
Exiting rear mkrescue (PID 26137) and its descendant processes ...
Running exit tasks
Could not remove build area /var/tmp/rear.3wJORZAXiSOtf9D (something still exists therein)
To manually remove the build area use (with caution): rm -Rf --one-file-system /var/tmp/rear.3wJORZAXiSOtf9D

With the changes in https://github.com/rear/rear/pull/2675

# usr/sbin/rear -v mkrescue
...
Wrote ISO image: /root/rear.github.master/var/lib/rear/output/rear-linux-h9wr.iso (77M)
Exiting rear mkrescue (PID 15145) and its descendant processes ...
Running exit tasks
ERROR: 
====================
BUG in /root/rear.github.master/usr/share/rear/lib/_input-output-functions.sh line 321:
'Directory /var/tmp/rear.UlCOcpGubZ4QEax/outputfs not empty, cannot remove'
--------------------
Please report this issue at https://github.com/rear/rear/issues
and include at least all related parts from /root/rear.github.master/var/log/rear/rear-linux-h9wr.log
preferably the whole debug information via 'rear -D mkrescue'
====================
Some latest log messages since the last called script 980_umount_output_dir.sh:
  2021-09-07 17:26:45.076243475 Finished running mkrescue workflow
  2021-09-07 17:26:45.087342962 Exiting rear mkrescue (PID 15145) and its descendant processes ...
  2021-09-07 17:26:48.128244631 rear,15145 usr/sbin/rear -v mkrescue
                                  `-rear,3576 usr/sbin/rear -v mkrescue
                                      `-pstree,3577 -Aplau 15145
  2021-09-07 17:26:48.153788841 Running exit tasks
  2021-09-07 17:26:48.160697183 Finished rear mkrescue in 58 seconds
  2021-09-07 17:26:48.164837257 Removing build area /var/tmp/rear.UlCOcpGubZ4QEax
Use debug mode '-d' for some debug messages or debugscript mode '-D' for full debug messages with 'set -x' output
Aborting due to an error, check /root/rear.github.master/var/log/rear/rear-linux-h9wr.log for details
Terminated

It works with that etc/rear/local.conf

OUTPUT=ISO
OUTPUT_URL=null

cf. https://github.com/rear/rear/issues/2667#issuecomment-914377933

Same result with current GitHub master code
and with the current changes as of today
in https://github.com/rear/rear/pull/2675

# usr/sbin/rear -v mkrescue
....
Wrote ISO image: /root/rear.github.master/var/lib/rear/output/rear-linux-h9wr.iso (77M)
Exiting rear mkrescue (PID 3679) and its descendant processes ...
Running exit tasks

pcahyna commented at 2021-09-07 16:24:¶

@jsmeix you can assign the issue to me if you want. Some analysis here: https://github.com/rear/rear/issues/2667#issuecomment-914447326, there are probably many places that are not prepared to handle empty OUTPUT_URL, especially an empty output from url_scheme.

pcahyna commented at 2021-09-07 16:38:¶

Continuing the discussion from #2667 :

If so, one might simply set OUTPUT_URL=null, if otherwise unset, in default.conf or in some script.

This would need to happen after usr/share/rear/prep/default/020_translate_url.sh and usr/share/rear/prep/default/030_translate_tape.sh as these two scripts set OUTPUT_URL if otherwise unset.
This also means that the semantics of empty OUTPUT_URL is somewhat special: it means, take the value from USB_DEVICE if set, else set it to BACKUP_URL, and if still empty, set it according to TAPE_DEVICE. (Why is TAPE_DEVICE handled differently from USB_DEVICE?)

pcahyna commented at 2021-09-08 09:54:¶

Hi @jsmeix, it seems that the simplest fix would be to treat empty scheme as null.

diff --git a/usr/share/rear/lib/global-functions.sh b/usr/share/rear/lib/global-functions.sh
index 4614af47..de76f291 100644
--- a/usr/share/rear/lib/global-functions.sh
+++ b/usr/share/rear/lib/global-functions.sh
@@ -258,6 +258,10 @@ function url_scheme() {
     local url=$1
     # the scheme is the leading part up to '://'
     local scheme=${url%%://*}
+    if [ -z "$scheme" ]; then
+        scheme=null
+    fi
+
     # rsync scheme does not have to start with rsync:// it can also be scp style
     # see the comments in usr/share/rear/prep/RSYNC/default/100_check_rsync.sh
     echo $scheme | grep -q ":" && echo rsync || echo $scheme

WDYT?

jsmeix commented at 2021-09-08 10:56:¶

@pcahyna

In output/default/250_create_lock.sh

scheme_supports_filesystem "$scheme" || return 0

does not help because scheme_supports_filesystem "" returns success
so scheme_supports_filesystem() needs to be made more fail-safe
(in general it is better to improve the function instead of all its callers)
so this change fixes this issue here for me

--- a/usr/share/rear/lib/global-functions.sh
+++ b/usr/share/rear/lib/global-functions.sh
@@ -369,6 +369,10 @@ function scheme_accepts_files() {
 ### only that it can be mounted (use mount_url() first)
 function scheme_supports_filesystem() {
     local scheme=$1
+    # Return false if scheme is empty or blank (e.g. when OUTPUT_URL is unset or empty or blank)
+    # cf. https://github.com/rear/rear/issues/2676
+    # and https://github.com/rear/rear/issues/2667#issuecomment-914447326
+    test $scheme || return 1
     case $scheme in
         (null|tape|obdr|rsync|fish|ftp|ftps|hftp|http|https|sftp)
             return 1

By the way:
We also have backup/NETFS/default/250_create_lock.sh
and its symlink backup/BLOCKCLONE/default/250_create_lock.sh
but those don't call scheme_supports_filesystem.

jsmeix commented at 2021-09-08 10:59:¶

Currently I cannot foresee what unexpected obscure side effects might appear
in this or that special cases if we set an empty scheme as "null".
I would have to do a thorough analysis of how all that code actually works
but I would prefer to not having to do that right now ;-)

What worries me is code like

scheme=$( url_scheme $BACKUP_URL )

Because the scheme is the leading part up to '://' in a URL
it means if we set an empty / non-existent scheme as "null"
such calls would result that scheme="null" but perhaps
the caller's code expects scheme="" when there is no
leading part up to '://' in a URL?

jsmeix commented at 2021-09-08 11:01:¶

@pcahyna
if my change of scheme_supports_filesystem() in
https://github.com/rear/rear/issues/2676#issuecomment-915133176
looks OK to you (i.e. if you would approve such a change)
I would add that change to https://github.com/rear/rear/pull/2675
and merge it to get both https://github.com/rear/rear/issues/2667
and this one https://github.com/rear/rear/issues/2676 fixed.

pcahyna commented at 2021-09-08 12:49:¶

In output/default/250_create_lock.sh
scheme_supports_filesystem "$scheme" || return 0
does not help because scheme_supports_filesystem "" returns success

Sure. Adding quotes around $scheme are necessary to support empty scheme properly, but not sufficient, other changes are needed.

jsmeix commented at 2021-09-09 11:05:¶

With https://github.com/rear/rear/pull/2675 merged
this issue should be solved.

[Export of Github issue for rear/rear.]

#2676 Issue closed: False ERROR ".../outputfs not empty, cannot remove" when OUTPUT_URL is unset¶