Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhanced AUTOEXCLUDE_PATH /media /run /mnt and /tmp #2261

Merged

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Oct 24, 2019

This pull request supersedes #2244

I used a test system with a separated ext2 filesystem on /tmp
cf. #2239 (comment)

With AUTOEXCLUDE_PATH=( /media /run /mnt /tmp )
I still got in disklayout.conf the ext2 filesystem on /tmp
which is the intended behaviour.

  • Brief description of the changes in this pull request:

Enhanced AUTOEXCLUDE_PATH in default.conf
from only AUTOEXCLUDE_PATH=( /media )
to AUTOEXCLUDE_PATH=( /media /run /mnt /tmp )
plus explanatory comment in default.conf
how AUTOEXCLUDE_PATH works.

@jsmeix jsmeix added the enhancement Adaptions and new features label Oct 24, 2019
@jsmeix jsmeix added this to the ReaR v2.6 milestone Oct 24, 2019
@jsmeix jsmeix self-assigned this Oct 24, 2019
@jsmeix jsmeix merged commit a8edfa6 into rear:master Oct 24, 2019
@jsmeix jsmeix deleted the enhanced_default_AUTOEXCLUDE_PATH_issue2239 branch October 24, 2019 13:06
# For example a separated filesystem with mountpoint /tmp is not excluded
# but the files in the /tmp directory are excluded from the backup
# when an internal backup method is used by BACKUP_PROG_EXCLUDE=( '/tmp/*' ... )
# so that this filesystem will be recreated as an empty filesystem.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jsmeix I don't get it, why to mention this in the description of AUTOEXCLUDE_PATH? The comment above is not really related to AUTOEXCLUDE_PATH, it describes the behavior of BACKUP_PROG_EXCLUDE.
(I am asking because it is confusing, without a very careful reading one is left with the expectation that /tmp in AUTOEXCLUDE_PATH leads to adding '/tmp/*' to BACKUP_PROG_EXCLUDE.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I describe the default behaviour.
In default.conf there is (excerpts)

BACKUP_PROG_EXCLUDE=( '/tmp/*' ... )
...
AUTOEXCLUDE_PATH=( ... /tmp )

and both default settings together result that
for a separated filesystem with mountpoint /tmp
the files in the /tmp directory are excluded from the backup
(provided an internal backup method is used)
and this filesystem will be recreated as an empty filesystem
(with mountpoint /tmp).

In general regarding the include/exclude stuff in ReaR
cf. my comments like
#2229 (comment)
and
#2772 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Via
81af521
I made the description in default.conf (hopefully) more clear
how the defaults for AUTOEXCLUDE_PATH and BACKUP_PROG_EXCLUDE
interact when /tmp is a separated filesystem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsmeix I understand that you are describing the default behavior, and the description is correct.

My problem is that the description occurs in a phrase starting "For example ...", which thus has the point of illustrating the behavior of AUTOEXCLUDE_PATH via an example. But the treatment of BACKUP_PROG_EXCLUDE is not relevant to the explanation of AUTOEXCLUDE_PATH - if one adds something else to AUTOEXCLUDE_PATH, it will not get added into BACKUP_PROG_EXCLUDE, unless the user adds it explicitly there. Actually, the /tmp entry in AUTOEXCLUDE_PATH is unrelated to the /tmp/* entry in BACKUP_PROG_EXCLUDE (except that both are somehow related to the /tmp directory), because the former excludes filesystems mounted under /tmp, while the latter excludes the content of the /tmp filesystem itself.
I believe that we need to decide whether this phrase is

  • intended to explain the behavior of AUTOEXCLUDE_PATH, and then we should choose another example than /tmp - it is a poor example, because it gets treated specially by the defaults in a way that no other directory does - and a special case is never a good illustration of a general rule, or,
  • intended as a complete description of the treatment of /tmp via the various defaults, but then the "For example" should be deleted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can split the phrase in two and do both of course. I can send a PR once we agree on the approach.

Copy link
Member Author

@jsmeix jsmeix Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used "for example" because it is
an example that /tmp is a separated filesystem
(normally /tmp is a directory of the root filesystem).
I used '/tmp as separated filesystem' as an example
because that was what I had explicitly tested in
#2261
so I knew how the include/exclude stuff in ReaR
actually behaves in this case - at least for me.
I won't dare to make generic statements how
that "hairy" include/exclude stuff in ReaR
behaves in general ;-)

@pcahyna
of course feel free to make a PR with a better text.
I would much appreciate it.
In particular when you could also test how that
include/exclude stuff in ReaR behaves for you.
Perhaps you may find interesting/unexpected behaviour
with your tests where we may have to fix something
or at least better desribe how it actually works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsmeix added a commit that referenced this pull request Jan 8, 2024
In default.conf make it more clear how the defaults for
AUTOEXCLUDE_PATH and BACKUP_PROG_EXCLUDE interact
when /tmp is a separated filesystem, see
#2261 (comment)
pcahyna added a commit to pcahyna/rear that referenced this pull request Jan 8, 2024
Since PR rear#2261, /media is included in the default AUTOEXCLUDE_PATH. This
means that a filesystem mounted on a directory like /media/backup will
not be included in the layout. This makes the example invalid, as it
describes how a filesystem mounted under /media/backup will be included
in the layout and what manual steps are needed to exclude it.

To preserve the validity of the example, change all paths in the example
from /media/backup to /backup.

Take this opportunity to also describe the AUTOEXCLUDE_PATH variable
among other autoexclusions.
pcahyna added a commit to pcahyna/rear that referenced this pull request Jan 9, 2024
Since PR rear#2261, /media is included in the default AUTOEXCLUDE_PATH. This
means that a filesystem mounted on a directory like /media/backup will
not be included in the layout. This makes the example invalid, as it
describes how a filesystem mounted under /media/backup will be included
in the layout and what manual steps are needed to exclude it.

To preserve the validity of the example, change all paths in the example
from /media/backup to /backup.

Take this opportunity to also describe the AUTOEXCLUDE_PATH variable
among other autoexclusions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants