New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: os: Chown expects int, but os/user uses strings #6495

Open
gopherbot opened this Issue Sep 27, 2013 · 11 comments

Comments

Projects
None yet
5 participants
@gopherbot

gopherbot commented Sep 27, 2013

[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).
@bradfitz

This comment has been minimized.

Member

bradfitz commented Sep 27, 2013

Comment 1:

I agree this is not ideal.  But we can't change this until Go 2.x.
Marking long-term.

Labels changed: added go2, removed priority-triage.

Status changed to LongTerm.

@gopherbot

This comment has been minimized.

gopherbot commented Sep 27, 2013

Comment 2:

Also, how would one do this on Windows? It uses the file_posix.go implementation of
Chown (does not have an all-errors stub like plan9), but on Windows, os/user contains
SIDs, not stringified ints. It seems a chown call is not possible on Windows without
syscall.
@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Sep 27, 2013

Comment 3:

What are you trying to do? Chown is Unix API. What is the task you are trying to
accomplish on Windows?
Alex
@gopherbot

This comment has been minimized.

gopherbot commented Sep 28, 2013

Comment 4:

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
@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Sep 29, 2013

Comment 5:

> ... 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
@rsc

This comment has been minimized.

Contributor

rsc commented Nov 27, 2013

Comment 6:

Labels changed: added go1.3maybe.

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 4, 2013

Comment 7:

Labels changed: added release-none, removed go1.3maybe.

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 4, 2013

Comment 8:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@rsc rsc changed the title from os: Chown wants ints, os/user only gives strings to os: Chown expects int, but os/user uses strings Jun 17, 2017

@rsc

This comment has been minimized.

Contributor

rsc commented Jun 17, 2017

cf #8537

@rsc rsc changed the title from os: Chown expects int, but os/user uses strings to proposal: os: Chown expects int, but os/user uses strings Jun 17, 2017

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Dec 6, 2017

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.

@bradfitz

This comment has been minimized.

Member

bradfitz commented Dec 6, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment