loader: support "all:" prefix in //go:embed patterns#5213
loader: support "all:" prefix in //go:embed patterns#5213deadprogram merged 1 commit intotinygo-org:devfrom
Conversation
|
The circleci failed test looks like a flaky test/race unrelated to my patch. Could a maintainer please look into my PR? |
loader/loader.go
Outdated
| // The "all:" prefix (if present) is stripped and reflected in includeHidden. | ||
| type embedPattern struct { | ||
| pattern string // the glob pattern (without "all:" prefix) | ||
| includeHidden bool // true if "all:" prefix was present |
There was a problem hiding this comment.
I suspect this data structure is just as efficient:
type embedPattern string
func (e embedPattern) Match(file string) bool {
includeHidden := strings.HasPrefix(s, "all:")
...
}The "all:" prefix in //go:embed directives instructs the compiler to include hidden files (starting with "." or "_") when embedding a directory. Previously, the prefix was passed literally to path.Match, which would never match. Introduce an embedPattern struct that parses and validates the pattern at construction time via newEmbedPattern(). The "all:" prefix is stripped once and stored along with the glob pattern, avoiding repeated string prefix checks. Pattern validation is absorbed into the constructor, guaranteeing that any embedPattern value is valid and eliminating the need for separate validation loops. Fixes embedding with patterns like "//go:embed all:static".
abgoyal
force-pushed
the
fix-embed-all-prefix
branch
from
dfc474a to
4a74cf4
Compare
|
Do you think this one works better? i have incorporated the separate pattern validation into the ep creation itself, and then just use that all over. I think it works well. and instead of striping out the prefix later, I just strip it out once at ep creation time - not really from a performance pov, which i think is same either way, just clarity and non-repetitiveness pov. |
|
Any blockers to merging this? |
|
@eliasnaur any other feedback on this, or is it good to merge? |
|
OK I think it looks good. Thank you very much @abgoyal for your improvement and patience! Now merging. |
|
Thanks! |
3 participants
Summary
Fixes handling of the
all:prefix in//go:embeddirectives. This prefix tells Go to include hidden files (starting with.or_) when embedding a directory.Previously, TinyGo passed patterns like
all:dirnameliterally topath.Match, which never matched any files since no file is actually named "all:dirname". This caused//go:embed all:...to silently embed nothing.Changes
all:prefix before pattern matchingincludeHiddenflag tomatchPatternfunctionincludeHiddenis falseall:prefix before pattern validation inparseGoEmbedall:prefixExample
Test
Added test in testdata/embed/ that verifies: