Add SetExtendedAttrs to Client #583
Conversation
client.go
Outdated
| @@ -593,6 +593,16 @@ func (c *Client) Truncate(path string, size int64) error { | |||
| return c.setstat(path, sshFileXferAttrSize, uint64(size)) | |||
| } | |||
|
|
|||
| // SetExtendedAttrs sets the extended attributes of the named file. | |||
| func (c *Client) SetExtendedAttrs(path string, extended []StatExtended) error { | |||
There was a problem hiding this comment.
It seems like the common abbreviation for “extended attributes” is xattr So, that would suggest SetXattr, which would match the setxattr call in OpenSSH. But I don’t think we want our name that short. SetExtAttrs() might be a good inbetween? But I’m still fine with this longer name as well.
Maybe we could also implement this as SetExtAttrs(path string, xattrs ...StatExtended) error … though checking it on go playground, it seems this doesn’t allow for the type-elided shorthand cl.SetExtAttr(path, { ExtType: "foo", ExtData: "bar" }) to be used. 🙁 Meanwhile, cl.SetExtAttrs(path, []sftp.StatExtended{ { ExtType: "foo", ExtData: "bar" } }) only needs the one sftp.StatExtended type name to be used for any number of xattrs.
A common API design that some have picked is SetExtAttrs(path string, xattrs ...string) and then provide alternating type and data values that get turned into the []sftp.StatExtended internally. But I’m not a big fan of this easy-to-misalign approach. But then again, it is integrated into the standard slog library, so… 🤷♀️
In the alternative, we could provide two functions: SetExtAttrs(path string, xattrs []StatExtended) and SetExtAttr(path, typ, data string) thus anyone wanting to set a single attribute can avoid the StatExtended altogether, and then for more than one, they can make use of the []StatExtended{ {}, {} } type elision.
There was a problem hiding this comment.
Following the draft RFCs the extended attributes in the SFTP protocol do not necessarily match to extended atributes on the system, so I would avoid the very short xattr to avoid confusion. Maybe SetExtendedData is a better name to correspond with the FileInfoExtendedData interface ?
Another catch is that it is very unspecified what setstat actually does with extended attributes. It might, in case of real extended attributes on files, just replace all current attributes.
There was a problem hiding this comment.
Added long comment to clarify the possible use cases.
There was a problem hiding this comment.
Second checked the standard, and the field is called extended_data, so 👍
peterverraedt
force-pushed
the
marshal-extended-client
branch
from
07fd48c to
e24bd0a
Compare
Add function to set extended attributes in the sftp client. Signed-off-by: Peter Verraedt <[email protected]> Add longer comment Signed-off-by: Peter Verraedt <[email protected]>
peterverraedt
force-pushed
the
marshal-extended-client
branch
from
e24bd0a to
0814039
Compare
Signed-off-by: Peter Verraedt <[email protected]>
Add function to set extended attributes in the sftp client.