-
Notifications
You must be signed in to change notification settings - Fork 21
feat(overlay-files): support {component} placeholder for pattern-discovered components #269
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
Open
liunan-ms
wants to merge
1
commit into
microsoft:main
Choose a base branch
from
liunan-ms:liunan/overlayfiles-pattern
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
217 changes: 217 additions & 0 deletions
217
internal/app/azldev/core/components/pattern_discovery.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,217 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package components | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "log/slog" | ||
| "path" | ||
| "path/filepath" | ||
| "slices" | ||
| "strings" | ||
|
|
||
| "github.com/bmatcuk/doublestar/v4" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" | ||
| ) | ||
|
|
||
| // patternDiscoveredComponent captures the result of expanding a single | ||
| // project-level [projectconfig.ComponentConfig.OverlayFiles] entry that | ||
| // contains the `{component}` placeholder. | ||
| type patternDiscoveredComponent struct { | ||
| // Name of the discovered component (the captured {component} segment). | ||
| Name string | ||
| // ReferenceDir is the absolute directory the pattern captured for this | ||
| // component (i.e. the prefix directory + captured name). Used as the | ||
| // component's reference directory for downstream operations. | ||
| ReferenceDir string | ||
| // OverlayFiles is the deduplicated, sorted list of absolute overlay file | ||
| // paths matched by the winning pattern for this component. | ||
| OverlayFiles []string | ||
| // SourcePattern is the exact entry string that produced this match, used | ||
| // in shadow-lint diagnostics. | ||
| SourcePattern string | ||
| } | ||
|
|
||
| // discoverComponentsFromPatterns walks the project-level | ||
| // default-component-config, expands every 'overlay-files' entry that contains | ||
| // the `{component}` placeholder, and returns the resulting pattern-discovered | ||
| // components. Two entries at project scope producing the same component name | ||
| // is a hard error. | ||
| // | ||
| // The caller is responsible for further precedence against explicitly-declared | ||
| // components (that override happens at the resolver layer; see | ||
| // [Resolver.patternDiscoveredComponents]). | ||
| func discoverComponentsFromPatterns(env *azldev.Env) (map[string]*patternDiscoveredComponent, error) { | ||
| result := make(map[string]*patternDiscoveredComponent) | ||
|
|
||
| config := env.Config() | ||
| if config == nil { | ||
| return result, nil | ||
| } | ||
|
|
||
| var patterns []string | ||
|
|
||
| for _, entry := range config.DefaultComponentConfig.OverlayFiles { | ||
| if strings.Contains(entry, projectconfig.OverlayFilesComponentPlaceholder) { | ||
| patterns = append(patterns, entry) | ||
| } | ||
| } | ||
|
|
||
| if len(patterns) == 0 { | ||
| return result, nil | ||
| } | ||
|
|
||
| for _, pattern := range patterns { | ||
| byName, err := expandSinglePattern(env, pattern) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("project 'overlay-files': %w", err) | ||
| } | ||
|
|
||
| for name, match := range byName { | ||
| if existing, ok := result[name]; ok { | ||
| return nil, fmt.Errorf( | ||
| "%w: component %#q is discovered by both %#q and %#q; "+ | ||
| "restrict one of the patterns or rename one of the directories", | ||
| ErrPatternDiscoveryCollision, name, existing.SourcePattern, match.SourcePattern, | ||
| ) | ||
| } | ||
|
|
||
| result[name] = match | ||
| } | ||
| } | ||
|
|
||
| return result, nil | ||
| } | ||
|
|
||
| // expandSinglePattern expands a single validated pattern and groups matched | ||
| // files by captured component name. | ||
| func expandSinglePattern(env *azldev.Env, pattern string) (map[string]*patternDiscoveredComponent, error) { | ||
| prefix, suffix := projectconfig.SplitOverlayFilesPlaceholder(pattern) | ||
|
|
||
| // Substitute a single-segment wildcard for {component} to build the actual | ||
| // glob. Because {component} is validated as a whole path segment (see | ||
| // ConfigFile.Validate), replacing it with "*" produces a well-formed | ||
| // doublestar pattern that captures exactly one path segment. | ||
| globPattern := prefix + "*" + suffix | ||
|
|
||
| // doublestar.WithFailOnIOErrors() is intentionally not set, mirroring | ||
| // findComponentGroupSpecPaths: we may legitimately hit unreadable subtrees | ||
| // under the project root. Non-IO errors (bad pattern, etc.) are still | ||
| // returned. | ||
| matches, err := fileutils.Glob(env.FS(), globPattern, doublestar.WithFilesOnly()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to expand pattern %#q:\n%w", pattern, err) | ||
| } | ||
|
|
||
| byName := make(map[string]*patternDiscoveredComponent) | ||
|
|
||
| for _, match := range matches { | ||
| name, referenceDir, err := extractCapturedName(pattern, prefix, match) | ||
| if err != nil { | ||
| // Skip matches we can't parse (should not happen with a validated | ||
| // pattern, but guard anyway). | ||
| slog.Debug("pattern-discovery: skipping unparseable match", | ||
| "pattern", pattern, "match", match, "err", err) | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| // Reject matches whose captured segment is not a safe component name | ||
| // (spaces, path traversal, absolute paths, etc). Downstream code uses | ||
| // the name as a filesystem path fragment (see fileutils.ValidateFilename) | ||
| // so we fail fast rather than let it explode later. | ||
| if err := fileutils.ValidateFilename(name); err != nil { | ||
| slog.Debug("pattern-discovery: skipping match with unsafe captured name", | ||
| "pattern", pattern, "match", match, "name", name, "err", err) | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| bucket, ok := byName[name] | ||
| if !ok { | ||
| bucket = &patternDiscoveredComponent{ | ||
| Name: name, | ||
| ReferenceDir: referenceDir, | ||
| SourcePattern: pattern, | ||
| } | ||
| byName[name] = bucket | ||
| } | ||
|
|
||
| bucket.OverlayFiles = append(bucket.OverlayFiles, match) | ||
| } | ||
|
|
||
| // Sort the overlay-files list per component (filename first, then full path). | ||
| for _, bucket := range byName { | ||
| slices.SortFunc(bucket.OverlayFiles, func(left, right string) int { | ||
| if result := strings.Compare(filepath.Base(left), filepath.Base(right)); result != 0 { | ||
| return result | ||
| } | ||
|
|
||
| return strings.Compare(left, right) | ||
| }) | ||
| } | ||
|
|
||
| return byName, nil | ||
| } | ||
|
|
||
| // extractCapturedName strips prefix from match and returns the first path | ||
| // segment (the captured component name) together with the concrete directory | ||
| // that captured segment lives in. The returned ReferenceDir is the natural | ||
| // per-component anchor for downstream operations. | ||
| func extractCapturedName(pattern, prefix, match string) (name, referenceDir string, err error) { | ||
| rel := match | ||
| if prefix != "" { | ||
| if !strings.HasPrefix(match, prefix) { | ||
| return "", "", fmt.Errorf( | ||
| "match %#q does not start with expected prefix %#q for pattern %#q", | ||
| match, prefix, pattern, | ||
| ) | ||
| } | ||
|
|
||
| rel = match[len(prefix):] | ||
| } | ||
|
|
||
| // The captured segment is everything up to (but not including) the first | ||
| // path separator in the residual, or the entire residual if there is no | ||
| // suffix. Patterns are validated to use '/' only. | ||
| sep := strings.IndexByte(rel, '/') | ||
| if sep < 0 { | ||
| name = rel | ||
| } else { | ||
| name = rel[:sep] | ||
| } | ||
|
|
||
| if name == "" { | ||
| return "", "", fmt.Errorf( | ||
| "empty captured component name for match %#q against pattern %#q", | ||
| match, pattern, | ||
| ) | ||
| } | ||
|
|
||
| // The reference dir is the prefix + captured segment (no trailing separator). | ||
| // Use path.Clean (POSIX '/') rather than filepath.Clean so ReferenceDir stays | ||
| // consistent with the placeholder contract, which validates and operates on | ||
| // POSIX-style separators only (filepath.Clean would emit '\' on Windows). | ||
| referenceDir = path.Clean(prefix + name) | ||
|
|
||
| return name, referenceDir, nil | ||
| } | ||
|
|
||
| // logShadowedByDeclaration emits a warning when an explicit component | ||
| // declaration overrides pattern-discovered overlay files for the same name. | ||
| func logShadowedByDeclaration(shadowed *patternDiscoveredComponent) { | ||
| slog.Warn( | ||
| "project 'overlay-files' pattern match shadowed by explicit component declaration; overlay files will not be applied", | ||
| "component", shadowed.Name, | ||
| "shadowed_pattern", shadowed.SourcePattern, | ||
| "shadowed_files", shadowed.OverlayFiles, | ||
| ) | ||
| } | ||
|
|
||
| // ErrPatternDiscoveryCollision wraps project-scope pattern collision errors so | ||
| // tests can assert on them. | ||
| var ErrPatternDiscoveryCollision = errors.New("project 'overlay-files' pattern collision") | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might miss shadowing from project defaults by groups. Not sure if thats something worth an error though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be resolved by the scope restriction, now rejecting overlay-files in distro-version and component-group default-component-config entirely.