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: a /.. after a ... in an importpath pattern is confusing #37552

Open
perillo opened this issue Feb 28, 2020 · 11 comments
Open

cmd/go: a /.. after a ... in an importpath pattern is confusing #37552

perillo opened this issue Feb 28, 2020 · 11 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@perillo
Copy link
Contributor

perillo commented Feb 28, 2020

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

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

Yes. It also reproduces with all versions of go (tested up to go1.5).

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY="github.com/perillo/go-init"
GONOSUMDB="github.com/perillo/go-init"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE="github.com/perillo/go-init"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build295792619=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14
uname -sr: Linux 5.5.6-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.31.
gdb --version: GNU gdb (GDB) 9.1

What did you do?

From inside the root directory of a module:

$ mkdir build
$ cd build
$ go list .../../
can't load package: package .: no Go files in /home/manlio/src/go/src/github.com/perillo/atexit/build

$ go list .../../../                                                                                                                                    [0]
github.com/perillo/atexit

How are /.. interpreted when after a ... in a pattern?

@bcmills
Copy link
Contributor

bcmills commented Feb 28, 2020

Ooh, that's a neat find!

I think we should reject patterns that include enough instances of /.. to back into a .... Anything else seems too confusing.

CC @jayconrod @matloob

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Feb 28, 2020
@bcmills bcmills added this to the Backlog milestone Feb 28, 2020
@howjmay
Copy link
Contributor

howjmay commented Mar 8, 2020

may I try to take this issue?

@jayconrod
Copy link
Contributor

@howjmay Sure, please feel free to work on this.

cmd/go/internal/search.Match is probably the right place to start.

@howjmay
Copy link
Contributor

howjmay commented Mar 9, 2020

@jayconrod Thank you so much for indicating ><

@howjmay
Copy link
Contributor

howjmay commented Mar 22, 2020

@bcmills @jayconrod
I was reading the code under /search. So I should allow only one /.. after ...right?

@jayconrod
Copy link
Contributor

@howjmay It's fine if there is a /.. after ..., though not immediately after.

Take a look at path.Clean. Among other things, it removes .. elements and the preceding path element. We want to ensure that no path element with ... is removed. A simple check would be counting the number of ... wildcards in a pattern before and after path.Clean and verifying the counts are the same.

@howjmay
Copy link
Contributor

howjmay commented Apr 5, 2020

To summarze it, I have to implement 2 more rules here.

  1. No /.. immediately after .... If there is /.. right after ..., function MatchPattern() will return false
  2. No ... is removed during processing

Am I right?

@jayconrod
Copy link
Contributor

Only the second rule is needed.

If a .. element follows an element with ..., path.Clean would eliminate both.

@tpaschalis
Copy link
Contributor

tpaschalis commented May 29, 2020

Hey @howjmay! Sorry for pinging you, but are you still working on this issue, or is it okay for me to take a stab?

@howjmay
Copy link
Contributor

howjmay commented May 29, 2020

Hi @tpaschalis may you give me one week more? I am finally figuring it out

howjmay added a commit to howjmay/go that referenced this issue May 31, 2020
howjmay added a commit to howjmay/go that referenced this issue May 31, 2020
howjmay added a commit to howjmay/go that referenced this issue May 31, 2020
@gopherbot
Copy link

Change https://golang.org/cl/235878 mentions this issue: path/path: Avoid removing wildcard in front of /..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants