Skip to content

Add Chmod() method to File#357

Open
owbone wants to merge 1 commit into
spf13:masterfrom
owbone:master
Open

Add Chmod() method to File#357
owbone wants to merge 1 commit into
spf13:masterfrom
owbone:master

Conversation

@owbone

@owbone owbone commented May 20, 2022

Copy link
Copy Markdown

os.File offers a Chmod() method. This is often safer and more direct
to use than os.Chmod() because it operates on an open file descriptor
rather than having to lookup the file by name. Without this, it's
possible for the target file to be renamed, in which case an
os.Chmod() would either fail or apply to any file that's taken its
place.

Therefore, add the Chmod() method to the File interface, and
implement it for all File implementations. The bulk of this change is
in MemMapFs, which required moving the chmod functionality down into
the mem package so that it can be shared between both mem.File and
MemMapFs.

@CLAassistant

CLAassistant commented May 20, 2022

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

`os.File` offers a `Chmod()` method. This is often safer and more direct
to use than `os.Chmod()` because it operates on an open file descriptor
rather than having to lookup the file by name. Without this, it's
possible for the target file to be renamed, in which case an
`os.Chmod()` would either fail or apply to any file that's taken its
place.

Therefore, add the `Chmod()` method to the `File` interface, and
implement it for all `File` implementations. The bulk of this change is
in `MemMapFs`, which required moving the chmod functionality down into
the `mem` package so that it can be shared between both `mem.File` and
`MemMapFs`.
@spf13

spf13 commented May 7, 2026

Copy link
Copy Markdown
Owner

The underlying need is real and the implementation within this repo is correct. The problem is adding Chmod to the File interface: that breaks every third-party afero.File implementation at compile time, which is too high a cost for a method most callers never need.

The right pattern — already established in this repo — is a narrower optional interface:

// In symlink.go or a new chmod.go:
type ChmodFile interface {
    Chmod(mode os.FileMode) error
}

Callers that need it type-assert:

if cf, ok := f.(afero.ChmodFile); ok {
    return cf.Chmod(mode)
}
return afero.ErrNoChmod // or similar

This keeps the File interface stable for all existing implementations while making the capability discoverable for those that support it. Please re-submit with that structure.

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.

3 participants