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

os: TestUserConfigDir and TestUserCacheDir expect writable $HOME on linux #64990

Closed
zhsj opened this issue Jan 6, 2024 · 2 comments
Closed
Assignees
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@zhsj
Copy link
Contributor

zhsj commented Jan 6, 2024

Go version

go version go1.22rc1 linux/amd64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/zsj/.cache/go-build'
GOENV='/home/zsj/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/zsj/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/zsj/go'
GOPRIVATE=''
GOPROXY='https://goproxy.cn'
GOROOT='/home/zsj/Workspaces/debian/package/pkg-go/build-area/x/usr/lib/go-1.22'
GOSUMDB='sum.golang.google.cn'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/zsj/Workspaces/debian/package/pkg-go/build-area/x/usr/lib/go-1.22/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22rc1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3035930955=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Building Go 1.22 on Debian build system.

What did you see happen?

https://buildd.debian.org/status/fetch.php?pkg=golang-1.22&arch=arm64&ver=1.22%7Erc1-1&stamp=1704505334&raw=0

--- FAIL: TestUserConfigDir (0.00s)
    os_test.go:2874: could not create UserConfigDir: mkdir /sbuild-nonexistent: permission denied
--- FAIL: TestUserCacheDir (0.00s)
    os_test.go:2851: could not create UserCacheDir: mkdir /sbuild-nonexistent: permission denied

What did you expect to see?

Build successfully without distro patch.

While reading #57630 and #57638, @bcmills said

Perhaps it should return an error if the chosen directory is not writable

However UserConfigDir and UserCacheDir don't report such error, and only the tests expect the returned paths are writable.

The Debian build system is intentionally configured to have a non-writable $HOME dir. Previously the Debian package carries a patch to skip TestUserHomeDir which failed if $HOME is not writable. However this is fixed in bb4ea80. But for the new added TestUserConfigDir and TestUserCacheDir, they fail again.

@bcmills
Copy link
Contributor

bcmills commented Jan 8, 2024

Hmm, the XDG spec does seem to explicitly allow non-writable directories:

If, when attempting to write a file, the destination directory is non-existent an attempt should be made to create it with permission 0700. If the destination directory exists already the permissions should not be changed. The application should be prepared to handle the case where the file could not be written, either because the directory was non-existent and could not be created, or for any other reason. In such case it may choose to present an error message to the user.

So perhaps the tests should be converted to examples instead.

@bcmills bcmills self-assigned this Jan 8, 2024
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. labels Jan 8, 2024
@bcmills bcmills added this to the Go1.22 milestone Jan 8, 2024
@gopherbot
Copy link

Change https://go.dev/cl/554815 mentions this issue: os: relax tests and add examples for UserCacheDir and UserConfigDir

@bcmills bcmills added release-blocker FixPending Issues that have a fix which has not yet been reviewed or submitted. labels Jan 8, 2024
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
Per https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html:
“If, when attempting to write a file, the destination directory is
non-existent an attempt should be made to create it with permission
0700. […] The application should be prepared to handle the case where
the file could not be written […]. In such case it may choose to
present an error message to the user.”

In certain CI environments, these directories have well-defined
locations but do not exist and cannot be created. In that case,
we now choose to log and return from the test without failing it.

To prevent the functions from falling back to being entirely untested,
we still fail the test (and “present an error message to the user”) if
either function returns an empty string without an error, or returns a
path that refers to a non-directory or results in an error other than
ErrNotExist.

In addition, since the tests themselves no longer create subdirectories,
we add examples illustrating the suggested pattern of usage.

Fixes golang#64990.

Change-Id: Ie72106424f5ebe36eaf9288c22710d74bb14a462
Reviewed-on: https://go-review.googlesource.com/c/go/+/554815
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Development

No branches or pull requests

3 participants