borgbackup / borg

Deduplicating archiver with compression and authenticated encryption.
https://www.borgbackup.org/
Other
10.73k stars 734 forks source link

changed insufficiently reserved length for log message #8152

Closed galqiwi closed 2 months ago

galqiwi commented 3 months ago

During borg prune I've encountered a minor issue -- output was not properly aligned.

Keeping archive (rule: hourly #3):       2024-02-20-23:13:36                  Tue, 2024-02-20 23:13:38 [53dbb412a3d28b8d788cee0fe189d1ceefd4affd27866d23ffaee7cd45af5971]
Keeping archive (rule: hourly[oldest] #4): 2024-02-20-23:09:22                  Tue, 2024-02-20 23:09:24 [66392999b4cb3a71262f1bdff4ecd05bcb6b74c6dc1ae6a2c9903bcf4a8137b3]

This PR fixes this issue. Full command with output can be viewed here.

galqiwi commented 3 months ago

It's not a proper fix, and I don't think that there is a proper fix. We can't know beforehand what length all these messages will have without adding unreasonable amount of complexity

ThomasWaldmann commented 3 months ago

@galqiwi Thanks for the PR!

Hmm, did you experience the issue with master branch? Or rather with some borg 1.2.x release (that code is in 1.2-maint branch)?

In general, the issue should first get fixed in the branch where it has been encountered and after that, it needs checking whether the other branches are affected also. Active branches are: 1.2-maint, 1.4-maint, master.

About the fix: I guess increasing by 4 instead of by 2 would cover more issues of the same kind.

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.74%. Comparing base (6de9ca8) to head (e3f1349). Report is 29 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #8152 +/- ## ========================================== + Coverage 83.49% 83.74% +0.24% ========================================== Files 67 67 Lines 12046 12061 +15 Branches 2185 2189 +4 ========================================== + Hits 10058 10100 +42 + Misses 1389 1368 -21 + Partials 599 593 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ThomasWaldmann commented 3 months ago

@galqiwi did you see my feedback?

galqiwi commented 2 months ago

@galqiwi did you see my feedback?

Yes, sorry for late response. I've encountered this bug in 1.2.0 version. Will make a PR to the corresponding branch. By looking at the code, I see that 1.3.x and master are affected too.

ThomasWaldmann commented 2 months ago

OK, so now 1.2 and 1.4 branches are fixed, thanks!

You experienced the issue with 1.2.x and 1.4 is quite similar to 1.2.

OTOH, master branch has major changes, so guess one should try how it looks like there.

Also, width in this PR might need increasing (e.g. to 44), but you'll see that when practically trying it out.

galqiwi commented 2 months ago

Also, width in this PR might need increasing (e.g. to 44), but you'll see that when practically trying it out.

Yeah, my bad. Fixed it.

ThomasWaldmann commented 2 months ago

Thanks!