-
Notifications
You must be signed in to change notification settings - Fork 14
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: backup file even if failed listing extended attributes #233
fix: backup file even if failed listing extended attributes #233
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nardoor for this PR!
I think we should only change the loglevel (and remove the comment about the log-discussion)
Hello @aawsome , Thanks for taking the time to review. I'll fix the cross compilation issue and apply the requested change soon. I still personally think a warning level for this log might harm some user's UX. |
ab9f4ac
to
80c99d4
Compare
80c99d4
to
3561d75
Compare
This should fix rustic-rs#232 The idea: Even on a system that supports extended-attributes, we might encounter some part of the file-system that doesn't. For instance a `mount` with the right option (streams_interface=windows for instace) will not support xattr. AFAIK it also seems to be the case with a simple sshfs on a debian/ubuntu.
3561d75
to
4df1ec4
Compare
Push forced the branch after I rebased on upstream/main |
## 🤖 New release * `rustic_backend`: 0.1.1 -> 0.2.0 (✓ API compatible changes) * `rustic_core`: 0.2.0 -> 0.3.0 (✓ API compatible changes) * `rustic_config`: 0.1.0 -> 0.2.0 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> ## `rustic_backend` <blockquote> ## [0.2.0](rustic_backend-v0.1.1...rustic_backend-v0.2.0) - 2024-08-18 ### Added - *(backends)* Add throttle option to opendal backend ([#216](#216)) - *(backend)* [**breaking**] remove s3 and sftp wrapper around opendal ([#200](#200)) - [**breaking**] move clippy lints to cargo manifest and fix upcoming issues all over the workspace ([#176](#176)) - *(opendal)* Add option connections ([#155](#155)) ### Fixed - clippy lints ([#220](#220)) - *(backends)* local: Only create repo dir when creating the repository ([#206](#206)) - *(deps)* update rust crate reqwest to 0.11.26 ([#196](#196)) - *(deps)* update rust crate thiserror to 1.0.58 ([#192](#192)) - *(deps)* update rust crate anyhow to 1.0.81 ([#191](#191)) - *(rclone)* Use semver for version checking ([#188](#188)) - *(deps)* update rust crate clap to 4.5.2 ([#183](#183)) - *(config)* Merge repository options for multiple config sources ([#171](#171)) - *(backend)* Give useful error message when no repository is given. - updated msrv and fix clippy lints ([#160](#160)) ### Other - dependency updates - *(deps)* more version updates ([#237](#237)) - Update MSRV to 1.76.0 - *(deps)* Several version updates ([#234](#234)) - Update MSRV (needed by opendal) - update opendal to 0.46 and refactor accordingly ([#225](#225)) - Update MSRV to 1.73.0 - fix clippy lints - *(deps)* update dependencies ([#180](#180)) - add rustic_testing to workspace crates - reset again after release to workspace dependencies for workspace crates </blockquote> ## `rustic_core` <blockquote> ## [0.3.0](rustic_core-v0.2.0...rustic_core-v0.3.0) - 2024-08-18 ### Added - *(forget)* [**breaking**] Make keep-* Options and add keep-none ([#238](#238)) - add search methods to Repository ([#212](#212)) - [**breaking**] Allow specifying many options in config profile without array ([#211](#211)) - [**breaking**] move clippy lints to cargo manifest and fix upcoming issues all over the workspace ([#176](#176)) - Add extra check before writing data ([#154](#154)) - Allow missing fields in snapshot summary - Hide plain text password from help text ([#170](#170)) - Add Repository::to_indexed_checked and ::to_index_ids_checked() ([#168](#168)) - *(prune)* Add more debug info to stats ([#162](#162)) - Add append-only repository mode ([#164](#164)) ### Fixed - parse commands given by arg or env using shell_words ([#240](#240)) - Allow non-value/null xattr ([#235](#235)) - ensure Rust 1.76.0 compiles - backup file even if failed listing extended attributes ([#233](#233)) - Export types so the Repository type can be fully specified ([#229](#229)) - Always sort StringList ([#226](#226)) - *(commands)* Properly finish progress bars - *(commands)* [**breaking**] Fix edge case for repair index ([#219](#219)) - clippy lints ([#220](#220)) - *(errors)* Show filenames in error message coming from ignore source ([#215](#215)) - *(paths)* Handle paths starting with "." correctly ([#213](#213)) - Add warning about unsorted files and sort where neccessary ([#205](#205)) - *(deps)* update rust crate thiserror to 1.0.58 ([#192](#192)) - *(deps)* update rust crate anyhow to 1.0.81 ([#191](#191)) - *(deps)* update rust crate serde_with to 3.7.0 ([#189](#189)) - *(rclone)* Use semver for version checking ([#188](#188)) - *(deps)* update rust crate strum to 0.26.2 ([#187](#187)) - *(deps)* update rust crate clap to 4.5.2 ([#183](#183)) - Set correct content for symlink with parent snapshot ([#174](#174)) - update dependency nix ([#169](#169)) - *(memory)* Limit memory usage for restore when having large pack files ([#165](#165)) - *(prune)* Correct number of repacks ([#167](#167)) - updated msrv and fix clippy lints ([#160](#160)) ### Other - dependency updates - Ensure that MSRV 1.76 works - *(deps)* more version updates ([#237](#237)) - Update MSRV to 1.76.0 - *(deps)* Several version updates ([#234](#234)) - fix clippy lints ([#236](#236)) - Update MSRV (needed by opendal) - update sha2 dependency - add integration tests for `prune` and `ls` ([#221](#221)) - *(error)* Add error sources ([#217](#217)) - add more warnings - make SnapshotFile::cmp_group public ([#210](#210)) - Update MSRV to 1.73.0 - fix clippy lints - add backup integration tests using snapshots ([#175](#175)) - replace dep bitmask-enum by enumset ([#173](#173)) - *(deps)* update dependencies ([#180](#180)) - use release-plz action, remove public api fixtures incl. test and related ci and other release related ci - Add unit tests for extra verification ([#172](#172)) - rustic_config v0.1.0 - add rustic_testing to workspace crates </blockquote> ## `rustic_config` <blockquote> ## [0.2.0](rustic_config-v0.1.0...rustic_config-v0.2.0) - 2024-08-18 ### Added - [**breaking**] move clippy lints to cargo manifest and fix upcoming issues all over the workspace ([#176](#176)) ### Other - Update MSRV to 1.76.0 - Update MSRV (needed by opendal) - Update MSRV to 1.73.0 - add changelog for rustic_config </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
This should fix #232
However I'm not so sure as the error message I encountered is slightly different from what reported by the issue creator.
In both cases I think this PR is worth looking into when you have time.
The idea:
Even on a system that supports extended-attributes, we might encounter some part of the file-system that doesn't.
For instance a
mount
with the right option (streams_interface=windows for instace) will not support xattr.AFAIK it also seems to be the case with a simple sshfs on a debian/ubuntu (as experienced in #232).
Note
I used a debug log to print the XAttr error in order to avoid erroring for every file of an extended attribute incompatible backup.
Testing
I am once again unsure on what tests to add.
Please let me know if anything comes up to your mind.