feat: Prefix temp snapshot copies with backintime_
#1940
Merged
Conversation
This file contains 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
Back in Time will sometimes create a temp dir, to copy (parts of) a snapshot. These have a generic name, in the form of `tmpXXXXXXXX_SNAPSHOTID`, where `XXXXXXXX` are random characters and `SNAPSHOTID` is the snapshot id (e.g. `20241121-112648-601`). Only the latter gives a hint that these dirs are from Back in Time but it's hard to tell as they're just some numbers. This makes it tricky for the user to know if it's safe to delete these dirs if they're left around (Back in Time will remove them if it exits cleanly but that's not always possible, e.g. might hang if disk space gets full, like in bit-team#1902 or when comparing too large snapshots). Added an explicit `backintime_` prefix to the temp dir to make their origin more clear.
sevens
force-pushed
the
add_prefix_to_temp_dirs
branch
from
24a7812
to
d0c0ffe
Compare
|
I also tested changing the suffix code to: sid = f'_profile_{sid.profileID}_snapshot_{sid.sid}'but not sure if that's too verbose. Left it out of this PR for now, let me know if it should be added back... Example dir with this change: |
buhtz
added
PR: Waiting for review
|
Ready to merge. |
buhtz
added
PR: Merge after creative-break
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.
I checked if it was easily possible to add a more specific prefix (e.g.
backintime-snapshot_compare_for when comparing snapshots) but the function gets used in 3 places (2 through calls toapp.MainWindow.openPath()), all quite different use cases. Couldn't find a nice way to pass more specific info totmpCopy()from them (don't know the code well enough to decide on a clean and good way to do this from all places, if it's even desirable at all).Don't think this warrants a
CHANGESentry?! Only case I can think of is if there are people who have e.g. some/tmp(and/var/tmp) cleanup scripts that assume the current format, but AFAICT it's nigh impossible to reliably determine if an old-format created dir is from BiT or not so that seems unlikely to exist to me.Tested:
/tmp/backintime_yvvem486_20241119-165907-278(last 3 parts being snapshot id)btnOpenCurrentItemClicked():Alt+Down(seems to be the only way to trigger this function)/tmp/backintime_esh5heni_20241121-112648-601/filesViewItemActivated():/tmp/backintime_ge543330_20241121-112648-601/There don't seem to be any relevant unittests (same as for #1937).
See also: #1902 (comment) and #1902 (comment)