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: modfetch.Unzip checks that all filenames are acceptable, causing problems for test fixture directories #26672

Closed
dgodd opened this issue Jul 29, 2018 · 5 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dgodd
Copy link

dgodd commented Jul 29, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.11beta2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/dgodd/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/dgodd/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/golang1.11beta2"
GOTMPDIR=""
GOTOOLDIR="/usr/local/golang1.11beta2/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/53/t04mmfdn0qq68l978jgz64cc0000gn/T/go-build523317176=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

If you import a module which includes files which don't pass module.CheckFilePath then you can't use that module. A good example of this is https://github.com/bmatcuk/doublestar. Doublestar has a test directory with several very strangely named files to make sure that it works on such directories. However, that means I can not use go modules to import doublestar

module github.com/dgodd/gomodfilenames

require (
  github.com/bmatcuk/doublestar v1.0.9
)

What did you expect to see?

I expect to be able to import doublestar. I'm not entirely sure what should change. Possibly modules should be able to specify files/directories which will not be imported. If such a system existed then doublestar could state that the test directory should not be extracted by modfetch.Unzip, this seems to me to be good since as a user of the module I don't need it anyway.

I should mention that go get github.com/bmatcuk/doublestar does work.

What did you see instead?

go: extracting github.com/bmatcuk/doublestar v1.0.9
-> unzip /Users/dgodd/go/src/mod/cache/download/github.com/bmatcuk/doublestar/@v/v1.0.9.zip: malformed file path "test/a☺b": invalid char '☺'
go: import "github.com/dgodd/gomodfilenames" ->
	import "github.com/bmatcuk/doublestar": cannot find module providing package github.com/bmatcuk/doublestar
@oiooj oiooj added the modules label Jul 29, 2018
@ianlancetaylor ianlancetaylor changed the title modfetch.Unzip checks that all filenames are acceptable. This causes problems for test fixture directories. cmd/go: modfetch.Unzip checks that all filenames are acceptable, causing problems for test fixture directories Aug 1, 2018
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 1, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Aug 1, 2018
@bcmills
Copy link
Contributor

bcmills commented Aug 1, 2018

Possibly modules should be able to specify files/directories which will not be imported.

You should be able to put a go.mod file in any directory you want to exclude from the parent module.
(That makes it logically a separate module, although I'm not 100% certain that that will exclude it from the zip file.)

@michilu
Copy link

michilu commented Aug 3, 2018

I got the same problem in my case:

go: extracting github.com/antlr/antlr4 v0.0.0-20180728001836-7d0787e29ca8
-> unzip go/src/mod/cache/download/github.com/antlr/antlr4/@v/v0.0.0-20180728001836-7d0787e29ca8.zip: malformed file path "runtime/Cpp/demo/Windows/antlr4-cpp-demo/antlr4-cpp-demo-vs2015.vcxproj..filters": double dot

I think to should be acceptable the normal cases that like the double dot.

@jamie-digital
Copy link

A decent compromise would be for such packages to create their fixtures when the tests are run, if they don't already exist, rather than checking them into source control. The same already applies for fixtures with unusual mode bits, since ZIP archives don't necessarily preserve them.

@dgodd
Copy link
Author

dgodd commented Aug 7, 2018

Thank you all. It turns out that @bcmills suggestion works wonderfully. I have created a PR to doublestar (bmatcuk/doublestar#16) and am closing this issue. Thank you all.

@vitarb
Copy link

vitarb commented Oct 4, 2018

Please note that creating mod files in those repos would be slow and painful process to follow for consumers as some packages might be not under active development.
Also it seems that we are putting restriction that any package that has special symbols in file names (even those that are not critical for core build) that are illegal on SOME platform couldn't be used on ANY platform.
Also although solution that was proposed above may help in many cases, creating mod files won't help if there are go files in there (either mixed or in sub-folders) and that module needs to be consumed at the end.

tony84727 added a commit to ortery/cockroach that referenced this issue Jan 16, 2019
robertgzr added a commit to robertgzr/gtoggl-api that referenced this issue Jun 27, 2019
Adds go.mod files to the mock subdirs to avoid
golang/go#26672
DavidGamba added a commit to DavidGamba/ffind that referenced this issue Aug 14, 2019
DavidGamba added a commit to DavidGamba/cli-bookmarks that referenced this issue Aug 26, 2019
Move test_tree into test_files dir and create a module for it:
See golang/go#26672 (comment)
@golang golang locked and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

8 participants