#782 PR merged: Fixes #781 Fix for removing directory in case that they are not empty

Labels: enhancement

phracek opened issue at 2016-02-17 08:55:

Signed-off-by: Petr Hracek phracek@redhat.com

pcahyna commented at 2021-03-25 10:57:

Hello, @phracek @gdha , why this change? It reverts (partially) the change in 8a545d608b7c4baf408ca1033458cd8a0ace5fdd and risks reintroducing bug #465. The point is, when the directory is not empty, you should not remove it, because it can contain valuable data, like previous backups or backups from other machines.
I also don't see how it fixes #781 (support for S/390), since I suspect the nonempty directory is a consequence of not having support for S/390, not the cause.

pcahyna commented at 2021-03-25 11:21:

More general question, the function cleanup_build_area_and_end_program and the AddExitTask in 100_mount_output_path.sh seem to have the same purpose. Why do we have both?

Similar question applies to

[[ -d $BUILD_DIR/outputfs/$NETFS_PREFIX ]] && rm -rf $v $BUILD_DIR/outputfs/$NETFS_PREFIX
[[ -d $BUILD_DIR/outputfs/$RSYNC_PREFIX ]] && rm -rf $v $BUILD_DIR/outputfs/$RSYNC_PREFIX

in usr/share/rear/output/default/980_umount_output_dir.sh although this part has not been touched by this change nor by 8a545d6. (I am asking because there seem to be way too many places where we rm -rf potentially valuable data.)

pcahyna commented at 2021-03-25 11:32:

I also found that rm has a useful --one-file-system option. As ReaR does not support RHEL 5 anymore, we could start using it, unless there are some supported SLES versions that lack it (Debian 8 and Ubuntu 16.04 and RHEL 6 have it: http://manpages.ubuntu.com/manpages/xenial/man1/rm.1.html).

jsmeix commented at 2021-03-25 11:46:

@pcahyna
in general ReaR has grown according to what was needed in practice
by various different contributors over the time.
So we do have lots of duplicated convoluted, obfuscated, outdated,
quick-and-dirty, and whatever else kind of "bad code" places.
I tried to clean up several of them (those places where I was mostly hit)
as good as I could (with my limited overall knowledge of the whole picture).
So when you are hit by whatever kind of "bad code" place
feel free and be brave and merciless and try to clean up the mess.
Usually things won't get worse when you do it with some reasonable care.
Miracles are not expected (i.e. you may make mistakes - that's normal).
But please add meaningful and explanatory comments to your cleaned up code
to enable others to fix and enhance your code properly if needed later, cf.
https://github.com/rear/rear/wiki/Coding-Style

pcahyna commented at 2021-03-25 11:52:

@jsmeix thanks for your encouragement, but I first wanted to learn the intent of the current code (in this case, duplication of output directory removal) before attemtoting any cleanup :-)

[Export of Github issue for rear/rear.]