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

embed: warn about dotfiles in embed.FS documentation #42321

Open
carlmjohnson opened this issue Nov 1, 2020 · 9 comments
Open

embed: warn about dotfiles in embed.FS documentation #42321

carlmjohnson opened this issue Nov 1, 2020 · 9 comments
Assignees
Milestone

Comments

@carlmjohnson
Copy link
Contributor

@carlmjohnson carlmjohnson commented Nov 1, 2020

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

Tip.

$ gotip version
go version devel +48be3ed Sat Oct 31 00:35:33 2020 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Tried out the new embed.FS feature.

example.go:

package main

import (
	"embed"
	"fmt"
	"io/fs"
)

func main() {
	//go:embed example/*
	var files embed.FS
	des, _ := fs.ReadDir(files, "example")
	for _, de := range des {
		fmt.Printf("%q\n", de.Name())
	}
}

example:

total 1504
drwxr-xr-x@ 6 adhoc  staff   192 Oct 30 21:49 .
drwx------  8 adhoc  staff   256 Nov  1 13:34 ..
-rw-r--r--@ 1 adhoc  staff  6148 Oct 30 21:49 .DS_Store
-rw-r--r--  1 adhoc  staff     0 Oct 30 21:47 .dot
-rw-r--r--@ 1 adhoc  staff     0 Oct 30 21:48 Icon
-rw-r--r--  1 adhoc  staff     0 Oct 30 21:47 one.txt

What did you expect to see?

$ gotip run .
"one.txt"

What did you see instead?

$ gotip run .
".DS_Store"
".dot"
"Icon\r"
"one.txt"

There should be a warning in the documentation that dotfiles and hidden files are included.

@inliquid
Copy link

@inliquid inliquid commented Nov 1, 2020

This is not a "documentation", but design issue, so this won't solve... anything?

@carlmjohnson
Copy link
Contributor Author

@carlmjohnson carlmjohnson commented Nov 1, 2020

I think documenting that dotfiles are included will solve 90% of this issue: it can be a surprising behavior. For the final 10%, I think there should be a way of excluding files, but I need to open another issue for that.

@zikaeroh
Copy link
Contributor

@zikaeroh zikaeroh commented Nov 1, 2020

I think this speaks to a potential problem where the embedded data could differ between using someone else's module with embedded data (determined by whatever git/module serving rules dictate those contents) and checking code out locally to use it.

I'm also wondering if this means that a build can differ depending on whether or not GOPROXY was used, since now more than just the .go/.s/.c/.h files matter as they are laid out on disk.

@andig
Copy link
Contributor

@andig andig commented Nov 1, 2020

I would actually like the ability of exluding specific files. even if we document dot file inclusion it would be good to have a way to ecxlude them, same as excluding something like assets.go would be nice.

@SamWhited
Copy link
Member

@SamWhited SamWhited commented Nov 1, 2020

I would actually like the ability of exluding specific files. even if we document dot file inclusion it would be good to have a way to ecxlude them, same as excluding something like assets.go would be nice.

I tentatively disagree. It seems to me that if you wanted to exclude certain files you shouldn't have selected those files in the first place using the glob.

I suspect allowing excluding individual files will end up with projects having giant lists of temp files to be excluded, one person excludes the .swp files that they accidentally included, then the next excludes the .DS_Store files their operating system included, then the next person excludes <your editor of choice's temp files>, etc. (rather like how we see projects with huge .gitignore files instead of people putting their specific editors in their user or system wide gitignore file). Instead I'd suggest it's better to not use a glob or configure your build to check for files you don't want.

@carlmjohnson
Copy link
Contributor Author

@carlmjohnson carlmjohnson commented Nov 1, 2020

This issue can be the proposal for documentation, and #42325 can be a proposal for excluding files.

@zikaeroh
Copy link
Contributor

@zikaeroh zikaeroh commented Nov 1, 2020

I worry that attempting to split this up into many disparate specific proposals will cloud discussion trying to better understand implications and solutions to the various types of "unintended" files appearing in embedded filesystems.

@toothrot
Copy link
Contributor

@toothrot toothrot commented Nov 2, 2020

/cc @rsc

@rsc
Copy link
Contributor

@rsc rsc commented Nov 4, 2020

We should certainly document for now that dot-files are included in a match of star.
We can use #42328 to discuss whether to keep that behavior.

@rsc rsc added the NeedsFix label Nov 4, 2020
@rsc rsc self-assigned this Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.