please-build / go-rules

Golang rules for the Please build system
Apache License 2.0
8 stars 18 forks source link

`Failed to remove temporary directory` warnings from `go_mod_download` build actions #284

Closed chrisnovakovic closed 1 month ago

chrisnovakovic commented 3 months ago

Spotted in the context of a recent run of the python-rules release workflow:

2024-08-15T14:53:50.1363040Z 14:53:50.135 WARNING: Failed to remove temporary directory for //third_party/go:toolchain: failed to remove plz-out/tmp/third_party/go/toolchain._build: exit status 1
2024-08-15T14:53:50.1365760Z Output: rm: cannot remove 'plz-out/tmp/third_party/go/toolchain._build/go/pkg/mod/golang.org/x/telemetry/config@v0.28.0/doc.go': Permission denied
2024-08-15T14:53:50.1368382Z rm: cannot remove 'plz-out/tmp/third_party/go/toolchain._build/go/pkg/mod/golang.org/x/telemetry/config@v0.28.0/LICENSE': Permission denied
2024-08-15T14:53:50.1370737Z rm: cannot remove 'plz-out/tmp/third_party/go/toolchain._build/go/pkg/mod/golang.org/x/telemetry/config@v0.28.0/config.json': Permission denied
2024-08-15T14:53:50.1374174Z rm: cannot remove 'plz-out/tmp/third_party/go/toolchain._build/go/pkg/mod/golang.org/x/telemetry/config@v0.28.0/go.mod': Permission denied
2024-08-15T14:53:50.3373221Z 14:53:50.336 WARNING: Failed to remove temporary directory for ///linux_arm64//third_party/go:_yaml#get: failed to remove plz-out/tmp/linux_arm64/third_party/go/_yaml#get._build: exit status 1
2024-08-15T14:53:50.3376659Z Output: rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_yaml#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/doc.go': Permission denied
2024-08-15T14:53:50.3380931Z rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_yaml#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/LICENSE': Permission denied
2024-08-15T14:53:50.3384028Z rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_yaml#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/config.json': Permission denied
2024-08-15T14:53:50.3387096Z rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_yaml#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/go.mod': Permission denied
2024-08-15T14:53:50.3433882Z 14:53:50.342 WARNING: Failed to remove temporary directory for ///freebsd_amd64//third_party/go:_humanize#get: failed to remove plz-out/tmp/freebsd_amd64/third_party/go/_humanize#get._build: exit status 1
2024-08-15T14:53:50.3437911Z Output: rm: cannot remove 'plz-out/tmp/freebsd_amd64/third_party/go/_humanize#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/doc.go': Permission denied
2024-08-15T14:53:50.3441438Z rm: cannot remove 'plz-out/tmp/freebsd_amd64/third_party/go/_humanize#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/LICENSE': Permission denied
2024-08-15T14:53:50.3445084Z rm: cannot remove 'plz-out/tmp/freebsd_amd64/third_party/go/_humanize#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/config.json': Permission denied
2024-08-15T14:53:50.3476068Z rm: cannot remove 'plz-out/tmp/freebsd_amd64/third_party/go/_humanize#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/go.mod': Permission denied
2024-08-15T14:53:50.3481574Z 14:53:50.344 WARNING: Failed to remove temporary directory for ///linux_arm64//third_party/go:_go-logging.v1#get: failed to remove plz-out/tmp/linux_arm64/third_party/go/_go-logging.v1#get._build: exit status 1
2024-08-15T14:53:50.3485348Z Output: rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_go-logging.v1#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/doc.go': Permission denied
2024-08-15T14:53:50.3489570Z rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_go-logging.v1#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/LICENSE': Permission denied
2024-08-15T14:53:50.3492869Z rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_go-logging.v1#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/config.json': Permission denied
2024-08-15T14:53:50.3495888Z rm: cannot remove 'plz-out/tmp/linux_arm64/third_party/go/_go-logging.v1#get._build/go_mod_download_folder/pkg/mod/golang.org/x/telemetry/config@v0.28.0/go.mod': Permission denied

and so on. Reproducible locally with plz build //third_party/go:_difflib#get in python-rules.

This seems to happen because go mod download isn't respecting the -modcacherw flag, and is creating read-only cache directories for module files:

$ ls -asl plz-out/tmp/third_party/go/toolchain._build/go/pkg/mod/golang.org/x/telemetry/config@v0.28.0/
total 36
 4 dr-xr-xr-x 2 csn users  4096 Aug 15 15:33 ./
 4 drwxrwxr-x 3 csn users  4096 Aug 15 15:33 ../
16 -r--r--r-- 1 csn users 14299 Aug 15 15:33 config.json
 4 -r--r--r-- 1 csn users   473 Aug 15 15:33 doc.go
 4 -r--r--r-- 1 csn users    46 Aug 15 15:33 go.mod
 4 -r--r--r-- 1 csn users  1453 Aug 15 15:33 LICENSE
chrisnovakovic commented 3 months ago

This appears to be a regression in Go 1.23.0 - I can't reproduce it with a Go 1.22.5 toolchain.

chrisnovakovic commented 3 months ago

The culprit is golang.org/x/telemetry itself - the configstore package downloads the telemetry config, which is distributed as a Go module, by shelling out to go without passing -modcacherw:

https://github.com/golang/telemetry/blob/0693e6240b9b888df93a2e280a64431c10d47a63/internal/configstore/download.go#L43

chrisnovakovic commented 3 months ago

After speaking to @peterebden, I think it makes sense to disable this telemetry altogether. Even merely invoking go(1) is sufficient to cause the telemetry config to be downloaded unless telemetry is explicitly disabled (by writing off to ~/.config/go/telemetry/mode). Collecting the telemetry is of questionable benefit anyway given that many invocations of go(1) will occur in a network-isolated sandbox.

peterebden commented 3 months ago

Presumably it is also going to add a bunch of time by re-downloading this in every action, because none of them will ever have a valid Go cache for it.

chrisnovakovic commented 1 month ago

This was fixed in Go 1.23.1.