Conversation
jacobsa
left a comment
jacobsa
left a comment
There was a problem hiding this comment.
Thanks for doing this. Please add tests to the appropriate cases in samples/memfs/memfs_test.go, and probably add a new case for Chown.
manvalls
force-pushed
the
feature/better-uid-gid
branch
3 times, most recently
from
b4c41b3 to
c8197cf
Compare
|
Test case for chown turned to be a pain in the ass, as you need to be root to actually use it. You can see my latest attempt here, it worked locally on my desktop but travis wasn't happy about it and neither was osx. Also some test cases seem to hang on linux, no clue about those, seems to be unrelated to the change. Anyway, that's as far as I go, feel free to pick it up from here. |
|
Would you mind rebasing this on the current master branch please? We fixed the travis config in the meantime. |
manvalls
force-pushed
the
feature/better-uid-gid
branch
from
c8197cf to
58a7930
Compare
The purpose of this PR is to allow file systems to: 1. Create inodes with the UID and GID of the calling process 2. Support chown
manvalls
force-pushed
the
feature/better-uid-gid
branch
from
58a7930 to
13353bc
Compare
| switch inMsg.Header().Opcode { | ||
| header := inMsg.Header() | ||
|
|
||
| switch header.Opcode { |
There was a problem hiding this comment.
nit: use switch header := inMsg.Header(); header.Opcode to scope the header variable to the switch block only (smaller scopes are generally preferable)
| Mode *os.FileMode | ||
| Atime *time.Time | ||
| Mtime *time.Time | ||
| Uid *uint32 |
There was a problem hiding this comment.
Is there a reason why you’re adding Uid/Gid only to some operations, but not to all? Isn’t the uid/gid field set by the kernel for all requests?
3 participants
The purpose of this PR is to allow file systems to: