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 Snapper id too long #36 #37

Merged
merged 2 commits into from
Jan 6, 2018
Merged

fix Snapper id too long #36 #37

merged 2 commits into from
Jan 6, 2018

Conversation

Antynea
Copy link
Owner

@Antynea Antynea commented Jan 6, 2018

fix Snapper id too long #36

fix Snapper id too long #36
@@ -293,7 +293,7 @@ snapshot_list()
for id in "${ids[@]}"; do
for j in "${!snapper_ids[@]}"; do
local snapper_id="${snapper_ids[$j]//[[:space:]]/}"
if [[ "$snapper_id" -eq "$id" ]]; then
if [[ "${snapper_id#0}" -eq "${id#0}" ]]; then
Copy link
Contributor

@maximbaz maximbaz Jan 6, 2018

Choose a reason for hiding this comment

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

This isn't very nice, would string comparison work? if [[ "$snapper_id" == "$id" ]]

UPDATE: just tested, the == works just as good for snapper, so it won't break anything.

Copy link
Owner Author

Choose a reason for hiding this comment

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

it's just a quick fix
it is necessary to improve the recognition of snapshots created by snapper, and those made manually.
I didn't have time to look at your code in detail to do a more mature fix.

the error occurs when a snapshot contains a 0 at the beginning of its name
because you get your id out of a brutal hand.
this part of the code is not robust enough.

I was able to reproduce the error easily by setting up snapper
but still having my snapshots made manually.

for example:

btrfs sub list -sa /

ID 276 gen 266 cgen 265 top level 263 otime 2018-01-06 20:05:51 path <FS_TREE>/@snapshots/@06-01-2018-20:05_test1

id="${snap_path_name//[!0-9]}"

id: 0601201820051

local entry="${snap[@]:10:2} | ${snap_path_name}"

entries: 2018-01-06 20:05:51 | @snapshots/@06-01-2018-20:05_test1

when the comparison is executed,
bash considers the number starting with a 0 as an octal number,
Stupid interpreter...
so to fix it quickly, I removed the "0" at the beginning of the ids.

Copy link
Contributor

@maximbaz maximbaz Jan 6, 2018

Choose a reason for hiding this comment

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

I see, I still think you should just change to == instead of -eq, it was a bad choice on my side to use -eq in the first place as it treats numbers specially - IDs are strings and == should handle them just fine. Then there is no need to remove 0 from the beginning.

Copy link
Owner Author

Choose a reason for hiding this comment

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

good deduction and more elegant solution
after testing, error no longer appears

Found snapshot: 2018-01-06 23:00:37 | @racine/.snapshots/10/snapshot | single | timeline
Found snapshot: 2018-01-06 20:05:51 | @snapshots/@06-01-2018-20:05_test1
Found snapshot: 2018-01-06 20:00:00 | @racine/.snapshots/9/snapshot | single | timeline

fix Snapper id too long #36 #37
@Antynea
Copy link
Owner Author

Antynea commented Jan 6, 2018

if it's ok, i can merge

@maximbaz
Copy link
Contributor

maximbaz commented Jan 6, 2018

Works on my side 👍

@Antynea Antynea merged commit 5cfbf46 into master Jan 6, 2018
@Antynea Antynea deleted the Snapper-id-too-long-#36 branch January 6, 2018 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants