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

//go:embed <path to file in parent directory> doesn't work #46056

Closed
gucio321 opened this issue May 8, 2021 · 25 comments
Closed

//go:embed <path to file in parent directory> doesn't work #46056

gucio321 opened this issue May 8, 2021 · 25 comments

Comments

@gucio321
Copy link
Contributor

gucio321 commented May 8, 2021

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

go version go1.16.4 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/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.16.4"
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-build3225948580=/tmp/go-build -gno-record-gcc-switches"

What did you do?

There is a following package:

  • main.go
  • README.md
  • assets:
    • assets.go

assets.go contains package assets loaded with //go:embed directive.
I'd like to add README.md file to assets.go, but //go:embed ../README.md doesn't work

What did you expect to see?

the program should compile and work fine.

What did you see instead?

assets/assets.go:59:12: pattern ../README.md: invalid pattern syntax
@seankhliao
Copy link
Member

This is intentional and documnted

Patterns may not contain ‘.’ or ‘..’ or empty path elements, nor may they begin or end with a slash.

@gucio321
Copy link
Contributor Author

@seankhliao is it possible to make it a feature request?

@seankhliao
Copy link
Member

It still can't cross module boundaries, and if it's in the same module you can just put the embed directive in a go file in the parent directory

@gucio321
Copy link
Contributor Author

yah, this solution is possible, but it would be a bit crazy.
I'd need to add go:embed directive in main.go and pass on embeded variable to my app? it doesn't seem right.

@gucio321
Copy link
Contributor Author

@seankhliao maybe go:embed in module area could be allowed?

@seankhliao
Copy link
Member

This was an explicit design decision:

Because embed.Files implements fs.FS, it cannot provide access to files with names beginning with .., so files in parent directories are also disallowed entirely, even when the parent directory named by .. does happen to be in the same module.

ref: https://go.googlesource.com/proposal/+/master/design/draft-embed.md

mergify bot pushed a commit to aws/copilot-cli that referenced this issue Aug 4, 2021
To start using the new ✨ [embed pkg](https://pkg.go.dev/embed) and deprecate our use of packr we need to move all of our templates under the `internal/pkg/template` pkg. This is because unfortunately we can't refer to the current "templates" directory by doing `"../../../templates"`:

> Because embed.Files implements fs.FS, it cannot provide access to files with names beginning with .., so files in parent directories are also disallowed entirely, even when the parent directory named by .. does happen to be in the same module.
>
> golang/go#46056 (comment)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
xar-tol pushed a commit to xar-tol/copilot-cli that referenced this issue Aug 5, 2021
…#2691)

To start using the new ✨ [embed pkg](https://pkg.go.dev/embed) and deprecate our use of packr we need to move all of our templates under the `internal/pkg/template` pkg. This is because unfortunately we can't refer to the current "templates" directory by doing `"../../../templates"`:

> Because embed.Files implements fs.FS, it cannot provide access to files with names beginning with .., so files in parent directories are also disallowed entirely, even when the parent directory named by .. does happen to be in the same module.
>
> golang/go#46056 (comment)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
nathannaveen added a commit to nathannaveen/simpleiot that referenced this issue Sep 4, 2021
* Removed installing go genesis.
* Removed the existing genesis file handler.
* Changed to go embed for file serving.
* Updated envsetup.sh to generate frontend within assets package.
* Had to generate within assets package because go embed does not support path traversing golang/go#46056
nathannaveen added a commit to nathannaveen/simpleiot that referenced this issue Sep 4, 2021
* Removed installing go genesis.
* Removed the existing genesis file handler.
* Changed to go embed for file serving.
* Updated envsetup.sh to generate frontend within assets package.
* Had to generate within assets package because go embed does not support path traversing golang/go#46056
@lonnyzhang423
Copy link

It still can't cross module boundaries, and if it's in the same module you can just put the embed directive in a go file in the parent directory

@gucio321 There is no need to put embed files into child directories, * can be used to match everything int the current directory.

I found this in embed draft.
https://go.googlesource.com/proposal/+/master/design/draft-embed.md#go_embed-directives

@purpleidea
Copy link

There's a major bug with this:

I have the COPYING license file in my project root. But my CLI package which wants to display that text is in a subdir. And so I can't ../COPYING to embed it. I don't think you should be able to get out of the package root, but this blocks the obvious use case that we used go-bindata for :/

@korya
Copy link

korya commented Oct 8, 2021

I agree. It makes sense to limit go:embed access to the whole directory of the current module rather than the current directory. One should be able to embed files within the whole "sub-tree" of the root directory of the current module. Right now, I end up applying the following pattern:

//go:generate cp -r ../../assets ./local-asset-dir
//go:embed local-asset-dir
var assetFs embed.FS

This works but it looks bad and I would like to avoid it.

@sam-hoffman
Copy link

@korya , does your solution work to embed one file into a slice of bytes?

@gucio321
Copy link
Contributor Author

@korya what do you mean by go:generate ../../assets ./local-asset-dir?
is assets some executable or just file/directory?

@korya
Copy link

korya commented Oct 21, 2021

@korya , does your solution work to embed one file into a slice of bytes?

It should work.

@korya what do you mean by go:generate ../../assets ./local-asset-dir?

Sorry, there is a typo. I missed cp -r. It should be //go:generate cp -r ../../assets ./local-asset-dir. I've fixed my original post.

@gucio321
Copy link
Contributor Author

gucio321 commented Oct 21, 2021

hmm, right, it may work, but it will not be portable :-(.
I don't remember, how will it work on windows, but ln -s could probably be used.

@gunsluo
Copy link

gunsluo commented Oct 26, 2021

I have the same issue, go:generate ../../assets ./local-asset-dir doesn't look good

@JuanigTorres
Copy link

JuanigTorres commented Jan 3, 2022

Wouldn't it be nice if embed could support predefined variables like 'dirname' or 'projectname' to avoid some unexpected bug and thus avoid the '..' or '.'?

@gucio321
Copy link
Contributor Author

gucio321 commented Jan 4, 2022

it sounds nice!
Maybe we should make a new issue @JuanigTorres ?

@JuanigTorres
Copy link

Sounds good to me!

@AlexeyInc
Copy link

it sounds nice! Maybe we should make a new issue @JuanigTorres ?

If it's still relevant I found good answer here.

syncom added a commit to syncom/hermit that referenced this issue Jun 7, 2022
We remove the `--schema` flag from the `gen-installer` command, for it's
no longer perceived to be useful.

We move the tempate file `install.sh.tmpl` to the `files/` directory for
consistency. Due to a golang constraint (cf.
golang/go#46056), we created a package `files`
to make the installer template available to other packages.

Thank @alecthomas and @jvmakine for their feedback.
alecthomas pushed a commit to cashapp/hermit that referenced this issue Jun 8, 2022
Add hermit gen-installer subcommand. It prints the SHA-256 digest of the generated installer script to stdout.

We also perform a cross-check in CI to make sure `cmd/geninstaller` and
`hermit gen-installer` produce identical installer scripts.

We remove the `--schema` flag from the `gen-installer` command, for it's
no longer perceived to be useful.

We move the tempate file `install.sh.tmpl` to the `files/` directory for
consistency. Due to a golang constraint (cf.
golang/go#46056), we created a package `files`
to make the installer template available to other packages.
@BapiGso
Copy link

BapiGso commented Dec 6, 2022

maybe do this,Create a new assets.go in the directory that needs to be packaged

package assets

import "embed"

//go:embed *
var Assets embed.FS

Then use this variable in another package

a :=&assets.Assets

thrau pushed a commit to localstack/copilot-cli-local that referenced this issue Dec 9, 2022
…#2691)

To start using the new ✨ [embed pkg](https://pkg.go.dev/embed) and deprecate our use of packr we need to move all of our templates under the `internal/pkg/template` pkg. This is because unfortunately we can't refer to the current "templates" directory by doing `"../../../templates"`:

> Because embed.Files implements fs.FS, it cannot provide access to files with names beginning with .., so files in parent directories are also disallowed entirely, even when the parent directory named by .. does happen to be in the same module.
>
> golang/go#46056 (comment)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
@KernelDeimos
Copy link

I want my webpack bundle to go in a different folder than the golang source code. I want to have a build folder that's .gitignored in the root of my repository so it's easy to keep track of transient files vs persistent ones. The fact that go:embed won't let me do this is arbitrary and stupid. Just let me specify a "target folder" in the virtual (embedded) filesystem instead of assuming I want to copy the file structure relative to my source file (which I almost never want to do as it is)

@gucio321
Copy link
Contributor Author

gucio321 commented Feb 12, 2023

I think, we needs to "construct" a proposal issue to solve this problem.
Here is what I think:
I understand maintainers' point of view

Because embed.Files implements fs.FS, it cannot provide access to files with names beginning with .., so files in parent directories are also disallowed entirely, even when the parent directory named by .. does happen to be in the same module.

In my opinion its not a good justification - it sounds like saying "we can't add this, since it is not implemented"

It still can't cross module boundaries, and if it's in the same module you can just put the embed directive in a go file in the parent directory

First of all let me note that putting go:embed directive in a parent directory does not look good at all. Over that, it may cause "cyclic import error" (or anyhow it is called in go - cant remember exacltly).
I agree that crossing module boundries wouldn't be good.
However, in my opinion, "shouldn't cross module boundries" does not equal "could not reffer to parent directory" - thats the point here. I think that compiler should accept .. pattern and should compile successfully as long as go:embed doesn't reffer to file above go.mod's location.

Any thoughts?

@seankhliao
Copy link
Member

As mentioned before, not having .. isn't that it's just not implemented, it goes against the contract specified by io/fs.FS.

@gucio321
Copy link
Contributor Author

As mentioned before, not having .. isn't that it's just not implemented, it goes against the contract specified by io/fs.FS.

well, but go:embed does not need to use fs.FS in fact, does it? Maybe embed.FS should implement fs.FS, but in some extended form so that it could support .. pattern?

@tigerinus
Copy link

The best practice here is to create a package in parent level with go:embed var in there; then reference to this var in the child level packages.

@fripSide

This comment was marked as spam.

@golang golang locked as resolved and limited conversation to collaborators Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests