cmd/go: improve diagnostic and documentation for invalid //go:embed file name #44486

catundercar · 2021-02-22T08:50:01Z

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

$ go version
go version go1.16 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="off"
GOARCH="amd64"
GOBIN="/root/go/bin"
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://goproxy.io"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build162353995=/tmp/go-build -gno-record-gcc-switches"

What did you do?

my directory test tree is:

├── 00|00.test
└── test

0 directories, 2 files

but the fs.ReadDir() only returns single file: test.
the code is:

//go:embed test
var efs embed.FS

func main() {
    efps, _ := efs.ReadDir("test")
    for _, efp := range efps {
        fmt.Println(efp.Name())
    } 
}

What did you expect to see?

like os.ReadDir(), want two files return(return all files).

What did you see instead?

Just return single file: test.

The text was updated successfully, but these errors were encountered:

catundercar · 2021-02-22T09:19:31Z

it seems like the | can not pass the function isBadEmbedName

catundercar · 2021-02-22T09:42:48Z

I edit the source code of go in src/cmd/go/internal/load/pkg.go.I comment the logic module.CheckFilePath() in isBadEmbedName function line 2095-2097 and rebuild a go binary, then use the new binary build my code, and it works~
But I think it's not appropriatly.

catundercar · 2021-02-22T09:52:46Z

@rsc Are there any way to support some special symbol by appropriate way?

beoran · 2021-02-22T15:18:28Z

@Mrliu8023

Is there any reason that the name of the directory has to be 00|00.test? To me, a name like, e.g. 00_00.test seems to be just as useful and less obscure.

jayconrod · 2021-02-22T15:49:53Z

The go command correctly reports an error for this file, but we should improve the error message and the embed documentation to explain why.

Currently the documentation says:

Patterns must not match files outside the package's module, such as ‘.git/*’ or symbolic links. Matches for empty directories are ignored. After that, each pattern in a //go:embed line must match at least one file or non-empty directory.

A file named 00|00.test cannot be included in a module because it contains characters that are meaningful to the shell. That's what module.CheckFilePath is checking. Its documentation explains the restrictions.

CheckFilePath checks that a slash-separated file path is valid. The definition of a valid file path is the same as the definition of a valid import path except that the set of allowed characters is larger: all Unicode letters, ASCII digits, the ASCII space character (U+0020), and the ASCII punctuation characters “!#$%&()+,-.=@[]^_{}~”. (The excluded punctuation characters, " * < > ? ` ' | / \ and :, have special meanings in certain shells or operating systems.)

//go:embed can't match files that aren't part of a module because packages would include different sets of files depending on whether they were built as part of the main module or a dependency.

catundercar · 2021-02-22T16:29:13Z

@Mrliu8023

Is there any reason that the name of the directory has to be 00|00.test? To me, a name like, e.g. 00_00.test seems to be just as useful and less obscure.

Because i can't control the input of users. Might I need give more information in my document.

catundercar · 2021-02-22T16:31:08Z

The go command correctly reports an error for this file, but we should improve the error message and the embed documentation to explain why.

Currently the documentation says:

Patterns must not match files outside the package's module, such as ‘.git/*’ or symbolic links. Matches for empty directories are ignored. After that, each pattern in a //go:embed line must match at least one file or non-empty directory.

A file named 00|00.test cannot be included in a module because it contains characters that are meaningful to the shell. That's what module.CheckFilePath is checking. Its documentation explains the restrictions.

CheckFilePath checks that a slash-separated file path is valid. The definition of a valid file path is the same as the definition of a valid import path except that the set of allowed characters is larger: all Unicode letters, ASCII digits, the ASCII space character (U+0020), and the ASCII punctuation characters “!#$%&()+,-.=@[]^_{}~”. (The excluded punctuation characters, " * < > ? ` ' | / \ and :, have special meanings in certain shells or operating systems.)

//go:embed can't match files that aren't part of a module because packages would include different sets of files depending on whether they were built as part of the main module or a dependency.

Yes.The embed files should have the same rule with .go files.

gopherbot · 2022-06-21T17:47:17Z

Change https://go.dev/cl/413494 mentions this issue: embed: additional embed file name check rules #44486

For #44486 Change-Id: I66af9f7a9f95489a41fd6710e50bdd7878f78b85 Reviewed-on: https://go-review.googlesource.com/c/go/+/413494 Run-TryBot: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>

For golang#44486 Change-Id: I66af9f7a9f95489a41fd6710e50bdd7878f78b85 Reviewed-on: https://go-review.googlesource.com/c/go/+/413494 Run-TryBot: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>

ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 22, 2021

ALTree added this to the Go1.17 milestone Feb 22, 2021

jayconrod added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 22, 2021

jayconrod modified the milestones: Go1.17, backlog, Backlog Feb 22, 2021

jayconrod changed the title ~~embed: not support | or full-width symbol file name~~ cmd/go: improve diagnostic and documentation for invalid //go:embed file name Feb 22, 2021

breml mentioned this issue Apr 8, 2021

embed: Files starting with - (dash) are not embedded (undocumented behavior) #45447

Closed

jayconrod mentioned this issue May 17, 2021

embed: Files starting with : (double dots) are not embedded (undocumented behavior) #46199

Closed

dmitshur modified the milestones: Backlog, Go1.19 Jun 23, 2022

dmitshur added the Documentation label Jun 23, 2022

dmitshur modified the milestones: Go1.19, Backlog Jun 23, 2022

dmitshur removed the Documentation label Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: improve diagnostic and documentation for invalid //go:embed file name #44486

cmd/go: improve diagnostic and documentation for invalid //go:embed file name #44486

catundercar commented Feb 22, 2021

catundercar commented Feb 22, 2021

catundercar commented Feb 22, 2021 •

edited

catundercar commented Feb 22, 2021

beoran commented Feb 22, 2021

jayconrod commented Feb 22, 2021

catundercar commented Feb 22, 2021

catundercar commented Feb 22, 2021

gopherbot commented Jun 21, 2022

cmd/go: improve diagnostic and documentation for invalid //go:embed file name #44486

cmd/go: improve diagnostic and documentation for invalid //go:embed file name #44486

Comments

catundercar commented Feb 22, 2021

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

catundercar commented Feb 22, 2021

catundercar commented Feb 22, 2021 • edited

catundercar commented Feb 22, 2021

beoran commented Feb 22, 2021

jayconrod commented Feb 22, 2021

catundercar commented Feb 22, 2021

catundercar commented Feb 22, 2021

gopherbot commented Jun 21, 2022

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

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

catundercar commented Feb 22, 2021 •

edited