Skip to content
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,os/user: stop special-casing Android and iOS in UserHomeDir and user.Current #62148

Open
bcmills opened this issue Aug 18, 2023 · 3 comments
Labels
mobile Android, iOS, and x/mobile OS-Android OS-iOS GOOS=ios Proposal
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 18, 2023

The android GOOS is generally a special case of linux, and the ios GOOS is generally a special case of darwin.

However, the behavior of os.UserHomeDir on android and ios deviates from the standard behavior for linux and darwin.

  • android was special-cased to return / in https://go.dev/cl/139717.
    • The commit message and code-review comments do not explain why, as far as I can tell, and no code comment was added explaining it either.
    • Its path was changed to /sdcard in https://go.dev/cl/169540 for os: The result of UserHomeDir and User.HomeDir is inconsistent. #31070, to match an existing special case in os/user.
      • That special case was added in https://go.dev/cl/37960, with the explanation “return a dummy user instead of failing”. However, it did not explain why failing is not the correct behavior in that configuration, as I would expect it to be — it seems like the code was changed to conform to an erroneous test, instead of changing the test to conform to the previously-correct(?) code.
  • ios was special-cased to return / in https://go.dev/cl/141798.
    • The explanation given was that “The UserHomeDir test succeeds on the builder, but not when run
      manually where HOME is set to the host $HOME.”
    • To me, that seems like a bug in the exec wrapper, not os.UserHomeDir — and, indeed, later (in https://go.dev/cl/167938) the wrapper was changed to avoid propagating $HOME to the device.

A month after the two special cases were added, the signature of UserHomeDir was changed to include an error return value (#28562). It seems to me that the special cases should have been removed at that point, but they weren't, and I think that was an oversight.

I propose that we add a GODEBUG guard for those special cases (mostly for a smoother version-based transition per #56986) and default UserHomeDir and os/user.Current on android and ios to be consistent with linux and darwin respectively as of the next Go version.

(CC @golang/android @golang/ios)

@gopherbot gopherbot added this to the Proposal milestone Aug 18, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Aug 18, 2023

This proposal was motivated by https://go.dev/cl/520262 (for #57638), in which it is turning out to be more difficult than it ought to be to add tests for UserCacheDir and UserConfigDir because of lingering bugs on android and possibly ios.

@bcmills bcmills added mobile Android, iOS, and x/mobile OS-Android OS-iOS GOOS=ios labels Aug 18, 2023
@dmitshur
Copy link
Contributor

android was special-cased to return / in https://go.dev/cl/139717.
The commit message and code-review comments do not explain why, as far as I can tell, and no code comment was added explaining it either.

I'll note that code review comments did include the unanswered question "Or would /data/local/tmp be better?", which adds a little context on how "/" was chosen.

@bcmills
Copy link
Contributor Author

bcmills commented Aug 19, 2023

That does add some context on how "/" was chosen, but unfortunately doesn't shed much light on why a special case was thought to be needed at all. 😅

@bcmills bcmills changed the title proposal: os: stop special-casing Android and iOS in UserHomeDir proposal: os,os/user: stop special-casing Android and iOS in UserHomeDir and user.Current Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Android, iOS, and x/mobile OS-Android OS-iOS GOOS=ios Proposal
Projects
Status: Incoming
Development

No branches or pull requests

3 participants