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

cmd/go: mod tidy considers directory starting with _ #53765

Closed
tsdtsdtsd opened this issue Jul 9, 2022 · 3 comments
Closed

cmd/go: mod tidy considers directory starting with _ #53765

tsdtsdtsd opened this issue Jul 9, 2022 · 3 comments
Assignees
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@tsdtsdtsd
Copy link

What version of Go are you using (go version)?

Tested with:

  • go1.17.6 linux/amd64
  • go1.18.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/tsd/.cache/go-build"
GOENV="/home/tsd/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/tsd/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/tsd/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build572917264=/tmp/go-build -gno-record-gcc-switches"

What did you do?

pwd
# /tmp/go-test

go mod init example.com/user/go-test
# go: creating new go.mod: module example.com/user/go-test

mkdir _data
sudo chown root:tsd _data
sudo chmod 700 _data
go mod tidy
# pattern all: open /tmp/go-test/_data: permission denied

What did you expect to see?

Successful execution of go mod tidy

While it's correct that permission denying rules are a fact in this situation, the go tool should still ignore/skip this folder.
The Go Modules Reference states:

Note that go mod tidy will not consider packages in the main module in directories named testdata or with names that start with . or _ unless those packages are explicitly imported by other packages.

What did you see instead?

The tool exits with pattern all: open /tmp/go-test/_data: permission denied without changing go.mod/go.sum

@bcmills
Copy link
Member

bcmills commented Jul 11, 2022

Neat find! I think this is closely related to CL 416179.

@bcmills bcmills added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 11, 2022
@bcmills bcmills added this to the Go1.20 milestone Jul 11, 2022
@bcmills
Copy link
Member

bcmills commented Jul 11, 2022

Indeed, looks like that change will fix this issue. Thanks for reporting!

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 11, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 11, 2022
@bcmills bcmills modified the milestones: Go1.20, Go1.19 Jul 11, 2022
@gopherbot
Copy link

Change https://go.dev/cl/416179 mentions this issue: cmd/go: avoid spurious readdir during fsys.Walk

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
fsys.Walk is cloned from filepath.Walk, which has always handled
a walk of a directory by reading the full directory before calling the
callback on the directory itself. So if the callback returns fs.SkipDir,
those entries are thrown away, but the expense of reading them was
still incurred. (Worse, this is the expensive directory read that also
calls Stat on every entry.) On machines with slow file system I/O,
these reads are particularly annoying. For example, if I do

	go list m...

there is a call to filepath.Walk that is told about $GOROOT/src/archive
and responds by returning filepath.SkipDir because archive does not
start with m, but it only gets the chance to do that after the archive
directory has been read. (Same for all the other top-level directories.)
Even something like go list github.com/foo/bar/... reads every top-level
$GOPATH/src directory.

When we designed filepath.WalkDir, one of the changes we made was
to allow calling the callback twice for a directory: once before reading it,
and then possibly again if the read produces an error (uncommon).
This CL changes fsys.Walk to use that same model. None of the callbacks
need changing, but now the $GOROOT/src/archive and other top-level
directories won't be read when evaluating a pattern like 'm...'.

For golang#53577.
Fixes golang#53765.

Change-Id: Idfa3b9e2cc335417bfd9d66dd584cb16f92bd12e
Reviewed-on: https://go-review.googlesource.com/c/go/+/416179
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants