[Noting down confusion from IRC] Chown takes ints, os/user Lookup returns strings. If someone wants to "chown jdoe file", they're supposed to have platform-specific code to strconv from os/lookup strings to ints? I know backwards-compatibility means changes are hard, but this API does not seem ideal. func Chown(name string, uid, gid int) error http://golang.org/pkg/os/#Chown type User struct { Uid string; ... } http://golang.org/pkg/os/user/#User I understand different platforms do different things -- but surely, if a platform has Chown that operates on ints, surely it should have an API that produces ints too. Or a cross-platform User abstraction that can be passed to Chown (where Chown exists).
Not sure what chown does on Windows, but consider: 1. Windows claims POSIX compatibility 2. The implementation of os.Chown http://golang.org/src/pkg/os/file_posix.go#L98 is explicitly used on Windows: http://golang.org/src/pkg/os/file_posix.go#L5 -- that is, it's not a stub like on Plan 9: http://golang.org/src/pkg/os/file_plan9.go#L408
> ... 1. Windows claims POSIX compatibility You should take it up with Microsoft. > 2. The implementation of os.Chown http://golang.org/src/pkg/os/file_posix.go#L98 > is explicitly used on Windows: http://golang.org/src/pkg/os/file_posix.go#L5 > -- that is, it's not a stub like on Plan 9: > http://golang.org/src/pkg/os/file_plan9.go#L408 Oh, it is the same as plan9: http://golang.org/src/pkg/syscall/syscall_windows.go#L901 And you didn't tell us what you are trying to accomplish with Chown on windows. I don't see us doing anything until we have some plan. Alex
cf #8537
The os package does provide Getuid and Getgid functions that return int (currently, at least), and it seems reasonable that those functions should work well with Chown. So it seems that we shouldn't change Chown. Note that all these functions are Unix-specific and do not work on Windows.
So the question is whether we should add a conversion function from the os/user package representation (string) to the os package representation (int). And, if so, where should that function live.
Perhaps the os/user package should have Unix-specific functions UnixIDFromString and StringFromUnixID that do the conversion between the os/user types and the os types.
It's not clear whether there is any corresponding requirement on Windows.
Rather than focus on how to fix this particular mismatch, I'd rethink the whole os package. There's a ton of weird C/Unix junk in there that does feel like Go and doesn't work nicely on Windows.
I mean, Chown lets you change the ownership, but there's nothing in the standard library to even get the ownership (easily). You have to type-assert the os.FileInfo interface into a non-portable *syscall.Stat_t which is never documented in the os package.
os/user was designed to work with Windows, hence the strings. But os.Chown etc return ErrWindows on Windows.
I'm thinking we should create new interface types for OS users and groups (similar to os.Signal), and then concrete implementations with int & string underlying types, something like:
package xxx
type User interface{
unexported()
}
type UserID int
func (UserID) unexported() {}
type UserName string
func (UserName) unexported() {}
And then use the xxx.User as the type for os/user and os.Chown/etc.
Seems weird to be forced to make string comparisons when:
// On Windows, it returns -1.
func Getuid() int { return syscall.Getuid() }
// Geteuid returns the numeric effective user id of the caller.
//
// On Windows, it returns -1.
func Geteuid() int { return syscall.Geteuid() }
You are really going to cripple everyone but windows for the sake of windows bad choices? Just make a separate library for windows and leave the standard library efficient as possible.
Most helpful comment
Rather than focus on how to fix this particular mismatch, I'd rethink the whole
ospackage. There's a ton of weird C/Unix junk in there that does feel like Go and doesn't work nicely on Windows.I mean,
Chownlets you change the ownership, but there's nothing in the standard library to even get the ownership (easily). You have to type-assert the os.FileInfo interface into a non-portable*syscall.Stat_twhich is never documented in theospackage.