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

Fix prune abort when time is unset for pack-to-delete + add test to check #711

Merged
merged 2 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog/new.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Changes in version x.x.x:
Breaking changes:

Bugs fixed:
- prune did abort when no time was set for a pack-do-delete. This case is now handled correctly.

New features:
- New global configuration paths are available, located at /etc/rustic/*.toml or %PROGRAMDATA%/rustic/config/*.toml, depending on your platform.
Expand All @@ -13,3 +14,4 @@ New features:
- fix: wait for password-command to successfully exit, allowing to input something into the command, and read password from stdout.
- repoinfo: Added new options --json, --only-files, --only-index
- Creation of new keys now enforces confirmation of entered key. This helps to prevent mistype of passwords during the initial entry
- Check: Add check if time is set for packs-to-delete
11 changes: 8 additions & 3 deletions crates/rustic_core/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,19 @@ fn check_packs(
IndexType::FullTrees
});

let mut process_pack = |p: IndexPack| {
let mut process_pack = |p: IndexPack, check_time: bool| {
let blob_type = p.blob_type();
let pack_size = p.pack_size();
_ = packs.insert(p.id, pack_size);
if hot_be.is_some() && blob_type == BlobType::Tree {
_ = tree_packs.insert(p.id, pack_size);
}

// Check if time is set _
if check_time && p.time.is_none() {
error!("pack {}: No time is set! Run prune to correct this!", p.id);
}

// check offsests in index
let mut expected_offset: u32 = 0;
let mut blobs = p.blobs;
Expand Down Expand Up @@ -234,10 +239,10 @@ fn check_packs(
let index = index?.1;
index_collector.extend(index.packs.clone());
for p in index.packs {
process_pack(p);
process_pack(p, false);
}
for p in index.packs_to_delete {
process_pack(p);
process_pack(p, true);
}
}

Expand Down
34 changes: 20 additions & 14 deletions crates/rustic_core/src/commands/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/// App-local prelude includes `app_reader()`/`app_writer()`/`app_config()`
/// accessors along with logging macros. Customize as you see fit.
use log::info;
use log::{info, warn};

use std::{
cmp::Ordering,
Expand Down Expand Up @@ -279,6 +279,7 @@ enum PackToDo {
Repack,
MarkDelete,
KeepMarked,
KeepMarkedAndCorrect,
Recover,
Delete,
}
Expand Down Expand Up @@ -370,7 +371,7 @@ impl PrunePack {
stats.packs_to_delete.remove += 1;
stats.size_to_delete.remove += u64::from(self.size);
}
PackToDo::KeepMarked => {
PackToDo::KeepMarked | PackToDo::KeepMarkedAndCorrect => {
stats.packs_to_delete.keep += 1;
stats.size_to_delete.keep += u64::from(self.size);
}
Expand Down Expand Up @@ -571,15 +572,16 @@ impl PrunePlan {
.push((pi, PartlyUsed, index_num, pack_num));
}
}
(true, 0, _) => {
let local_date_time =
pack.time.ok_or(CommandErrorKind::NoTimeInPacksToDelete)?;
if self.time - local_date_time >= keep_delete {
(true, 0, _) => match pack.time {
Some(local_date_time) if self.time - local_date_time >= keep_delete => {
pack.set_todo(PackToDo::Delete, &pi, &mut self.stats);
} else {
pack.set_todo(PackToDo::KeepMarked, &pi, &mut self.stats);
}
}
None => {
warn!("pack to delete {}: no time set, this should not happen! Keeping this pack.", pack.id);
aawsome marked this conversation as resolved.
Show resolved Hide resolved
pack.set_todo(PackToDo::KeepMarkedAndCorrect, &pi, &mut self.stats);
}
Some(_) => pack.set_todo(PackToDo::KeepMarked, &pi, &mut self.stats),
},
(true, 1.., _) => {
// needed blobs; mark this pack for recovery
pack.set_todo(PackToDo::Recover, &pi, &mut self.stats);
Expand Down Expand Up @@ -685,7 +687,10 @@ impl PrunePlan {
PackToDo::Repack => {
check_size()?;
}
PackToDo::MarkDelete | PackToDo::Delete | PackToDo::KeepMarked => {}
PackToDo::MarkDelete
| PackToDo::Delete
| PackToDo::KeepMarked
| PackToDo::KeepMarkedAndCorrect => {}
}
}

Expand Down Expand Up @@ -746,7 +751,6 @@ impl PrunePlan {
opts: &PruneOpts,
) -> RusticResult<()> {
repo.warm_up_wait(self.repack_packs().into_iter())?;

let be = &repo.dbe;
let pb = &repo.pb;

Expand Down Expand Up @@ -891,12 +895,14 @@ impl PrunePlan {
indexer.write().unwrap().add_remove(pack)?;
}
}
PackToDo::KeepMarked => {
PackToDo::KeepMarked | PackToDo::KeepMarkedAndCorrect => {
if opts.instant_delete {
delete_pack(pack);
} else {
// keep pack: add to new index
let pack = pack.into_index_pack();
// keep pack: add to new index; keep the timestamp.
// Note the timestap shouldn't be None here, however if it is not not set, use the current time to heal the entry!
let time = pack.time.unwrap_or(self.time);
let pack = pack.into_index_pack_with_time(time);
indexer.write().unwrap().add_remove(pack)?;
}
}
Expand Down
2 changes: 0 additions & 2 deletions crates/rustic_core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ pub enum CommandErrorKind {
PathIsNoDir(String),
/// used blobs are missing: blob {0} doesn't existing
BlobsMissing(Id),
/// packs_to_delete doesn't contain `time`.
NoTimeInPacksToDelete,
aawsome marked this conversation as resolved.
Show resolved Hide resolved
/// used pack {0}: size does not match! Expected size: {1}, real size: {2}
PackSizeNotMatching(Id, u32, u32),
/// "used pack {0} does not exist!
Expand Down
2 changes: 1 addition & 1 deletion crates/rustic_core/src/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{
use bytes::Bytes;
use log::{debug, error, info};

use dialoguer::Password;
use nom::{
branch::alt,
bytes::complete::{is_not, tag},
Expand All @@ -18,7 +19,6 @@ use nom::{
sequence::delimited,
IResult,
};
use dialoguer::Password;

use serde_with::{serde_as, DisplayFromStr};

Expand Down
9 changes: 5 additions & 4 deletions src/commands/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ pub(crate) fn save_config(
let key = Key::new();

let pass = password.map_or_else(
|| match Password::new().with_prompt("enter password for new key")
.allow_empty_password(true)
.with_confirmation("confirm password", "passwords do not match")
.interact()
|| match Password::new()
.with_prompt("enter password for new key")
.allow_empty_password(true)
.with_confirmation("confirm password", "passwords do not match")
.interact()
{
Ok(it) => it,
Err(err) => {
Expand Down
9 changes: 5 additions & 4 deletions src/commands/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ impl AddCmd {
let key = repo.key;

let pass = self.new_password_file.as_ref().map_or_else(
|| match Password::new().with_prompt("enter password for new key")
.allow_empty_password(true)
.with_confirmation("confirm password", "passwords do not match")
.interact()
|| match Password::new()
.with_prompt("enter password for new key")
.allow_empty_password(true)
.with_confirmation("confirm password", "passwords do not match")
.interact()
{
Ok(it) => it,
Err(err) => {
Expand Down
Loading