-
Notifications
You must be signed in to change notification settings - Fork 117
FYI #20
Comments
Would be nice if we could create tests for both to see if there would be situations that |
@kevinburke: for some reason I got Here's my code:
This |
@joonas-fi Are you using |
here's the logic in go 1.11 that leads to an error u := &User{
Uid: currentUID(),
Gid: currentGID(),
Username: os.Getenv("USER"),
Name: "", // ignored
HomeDir: os.Getenv("HOME"),
}
// On NaCL and Android, return a dummy user instead of failing.
switch runtime.GOOS {
case "nacl":
if u.Uid == "" {
u.Uid = "1"
}
if u.Username == "" {
u.Username = "nacl"
}
if u.HomeDir == "" {
u.HomeDir = "/"
}
case "android":
if u.Uid == "" {
u.Uid = "1"
}
if u.Username == "" {
u.Username = "android"
}
if u.HomeDir == "" {
u.HomeDir = "/sdcard"
}
}
// cgo isn't available, but if we found the minimum information
// without it, use it:
if u.Uid != "" && u.Username != "" && u.HomeDir != "" {
return u, nil
}
return u, fmt.Errorf("user: Current not implemented on %s/%s", runtime.GOOS, runtime.GOARCH) note there is a patch to this logic incoming in go1.12, the patch is here: golang/go@3a18f0ecb57 the logic in go 1.12 will look like: uid := currentUID()
// $USER and /etc/passwd may disagree; prefer the latter if we can get it.
// See issue 27524 for more information.
u, err := lookupUserId(uid)
if err == nil {
return u, nil
}
u = &User{
Uid: uid,
Gid: currentGID(),
Username: os.Getenv("USER"),
Name: "", // ignored
HomeDir: os.Getenv("HOME"),
}
// On NaCL and Android, return a dummy user instead of failing.
switch runtime.GOOS {
case "nacl":
if u.Uid == "" {
u.Uid = "1"
}
if u.Username == "" {
u.Username = "nacl"
}
if u.HomeDir == "" {
u.HomeDir = "/"
}
case "android":
if u.Uid == "" {
u.Uid = "1"
}
if u.Username == "" {
u.Username = "android"
}
if u.HomeDir == "" {
u.HomeDir = "/sdcard"
}
}
// cgo isn't available, but if we found the minimum information
// without it, use it:
if u.Uid != "" && u.Username != "" && u.HomeDir != "" {
return u, nil
}
return u, fmt.Errorf("user: Current not implemented on %s/%s", runtime.GOOS, runtime.GOARCH) |
@Ilyes512 unfortunately my image is not public, but my image's first public parent is @kevinburke thanks for responding so quickly, and with the code. The issue seems to be here:
The code you shared seems to require that The upcoming patch doesn't seem to mitigate this exact case. EDIT: Here's a repro:
|
Go 1.12 supports |
I noted this somewhere else but can't find it... but the Go stdlib implementation is quite simplistic. It only checks env vars. This will work in most cases (90+%?) so if it works for you that's great and you can remove a dep! In the nearly decade (!!!) we've had some of our software like Vagrant, there has been enough situations where that is not enough so we have extra logic here in go-homedir to try other ways to detect the home directory. We haven't had a bug reported in home directory detection in a long time... So if you want something more robust then keep using this lib, otherwise you can use the stdlib! |
Correct me if I'm mistaken, but having an |
No, its non-standard. Just the realities of delivering software to many people... you hit some truly “wtf” environments. |
The bit I hit just now, @mitchellh, is the difference between %HOME% and %USERPROFILE% on windows. AWS appears to specifically use %USERPROFILE% which is fine in most cases, but if %HOME% is defined as well, go-homedir prefers it. |
Hey @jhulten. Its been long enough that I don't have full context here, but if there is a well established practice that USERPROFILE is preferred, then we should/can switch to that as a preference for Windosws. |
Probably would be a breaking change for a lot of people, but the ability to provide lookup order instead of it being hardcoded would be a plus. |
Go 1.9 ships with os/user.LookupId, os/user.Lookup, os/user.LookupGroup, os/user.LookupGroupId that work without cgo. If you compile without cgo enabled, those functions will read /etc/passwd and /etc/group respectively to find answers to the questions you ask. Which may be good enough for your use case.
(I wrote the functions in question, and I'm curious how well they work for folks. I would appreciate feedback on how well they work or don't. If they don't work, I'd love to help fix them. Thanks!)
The text was updated successfully, but these errors were encountered: