Skip to content

backend/local: fix "operation not supported" when unlocking#5596

Merged
MichaelEischer merged 1 commit into
restic:masterfrom
mikix:chmod-again
Nov 16, 2025
Merged

backend/local: fix "operation not supported" when unlocking#5596
MichaelEischer merged 1 commit into
restic:masterfrom
mikix:chmod-again

Conversation

@mikix

@mikix mikix commented Nov 16, 2025

Copy link
Copy Markdown
Contributor

What does this PR change? What problem does it solve?

If the repo is on a mounted folder that doesn't support chmod (like SMB), it was causing an "operation not supported" error when trying to chmod 666 a file before deleting it.

But it isn't generally needed before deleting a file (the folder permissions matter there, not the file permissions). So, just drop it.

This code path I'm deleting was introduced ten years ago without comment. Is there a platform that Restic supports that does care about the file permissions before deleting? If so, I can fix this another way - by ignoring ErrUnsupported for Unix (like the other recent fix I made did)

The only remaining chmod call I can see that might be a problem is in ResetPermissions in file.go (which could call the file_xxx.go's chmod abstraction, but I think there we actually do want to raise an issue if we can't correctly reset permissions - it's only used when there's an access denied error already. So the fact that chmod is unsupported is relevant when explicitly trying to reset permissions.)

Was the change previously discussed in an issue or on the forum?

Closes #5595

Checklist

  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I'm done! This pull request is ready for review.

@mikix mikix force-pushed the chmod-again branch 2 times, most recently from 52a631a to 1fad4de Compare November 16, 2025 02:18

@MichaelEischer MichaelEischer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We cannot remove the chmod call (see comment). Please just filter out ErrUnsupported as already done by setFileReadonly. (Feel free to create a unified helper for setting file read-only and writeable if you want)

Comment thread internal/backend/local/local.go
@mikix

mikix commented Nov 16, 2025

Copy link
Copy Markdown
Contributor Author

Or maybe - much like we have platform specific wrappers for lots of stuff, make a setFileDeletable wrapper that is a no-op on Unix but calls chmod on windows.

@MichaelEischer

Copy link
Copy Markdown
Member

Or maybe - much like we have platform specific wrappers for lots of stuff, make a setFileDeletable wrapper that is a no-op on Unix but calls chmod on windows.

Then I'd just turn that into a deleteFile wrapper that does the correct thing on each platform.

@mikix mikix force-pushed the chmod-again branch 3 times, most recently from ac117ac to e5c1a87 Compare November 16, 2025 13:07
If the repo is on a mounted folder that doesn't support chmod (like
SMB), it was causing an "operation not supported" error when trying to
chmod 666 a file before deleting it.

But it isn't generally needed before deleting a file (the folder
permissions matter there, not the file permissions). So, just drop it.

@MichaelEischer MichaelEischer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!

@MichaelEischer MichaelEischer merged commit c854338 into restic:master Nov 16, 2025
12 checks passed
@MichaelEischer

Copy link
Copy Markdown
Member

The only remaining chmod call I can see that might be a problem is in ResetPermissions in file.go (which could call the file_xxx.go's chmod abstraction, but I think there we actually do want to raise an issue if we can't correctly reset permissions - it's only used when there's an access denied error already.

Yes, that error must be checked. ResetPermissions is already only called if restore couldn't open a file for writing.

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.

"Operation not supported" on linux SMB mounts when unlocking

2 participants