expand gomod paths with glob patterns - #1177
Conversation
✅ Deploy Preview for dalec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
58b875c to
c53eee6
Compare
There was a problem hiding this comment.
Pull request overview
Adds glob expansion for Go module generator paths, simplifying multi-module manifests.
Changes:
- Expands GoMod paths against patched source contents.
- Adds BuildKit filesystem glob support.
- Adds unit, integration, and documentation coverage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
source.go |
Adds the source glob callback. |
preprocess.go |
Expands GoMod path patterns. |
preprocess_test.go |
Tests path expansion behavior. |
frontend/gateway.go |
Implements filesystem globbing. |
frontend/debug/handle_gomod.go |
Preprocesses debug GoMod builds. |
test/source_test.go |
Adds integration coverage. |
website/content/sources.md |
Documents glob paths. |
c53eee6 to
f273497
Compare
Signed-off-by: Pedro Tôrres <pedrotorres@microsoft.com>
f273497 to
d1208d0
Compare
|
|
||
| newPaths := make([]string, 0, len(oldPaths)) | ||
| for _, pattern := range oldPaths { | ||
| matches, err := sOpt.Glob(patchedState, filepath.Join(basePath, pattern)) |
There was a problem hiding this comment.
This is globging for all paths, forcing eager evaluation regardless of glob patterns being present.
I'm also thinking we could handle this inside our shell where these paths are handled.
There was a problem hiding this comment.
If no glob pattern is present, this will be a no-op. Are you worried about performance? I don't think there'll be a noticeable slowdown in paths without patterns. Since there's no function to check if a path has a glob pattern, we would have to implement one ourselves and maintain it. This wouldn't be easy, as support for new patterns could be added and given that on Windows the patterns behave differently (there's no escaping support in that OS).
If we don't handle this expansion here, things like gomod edits may not work as expected. Expanding the paths here ensures the rest of Dalec behaves exactly as before, and there's no special handling related to path expansion. The way I implemented it also makes it possible to easily expand this feature to other generators without having to worry too much about the specific package manager of other languages.
There was a problem hiding this comment.
It's not a no-op because we have to do a full evaluation, including fetch the content immediately (actually the current implementation is doing a new solve for every path).
For using the posix shell to handle globs, I'm not sure its a problem to have to handle the globs in 2 places. It's just a prerequisite, same as how we have to tell it to traverse those paths in both places.
What this PR does / why we need it:
Support glob patterns in paths of the GoMod generator, allowing the DALEC manifest to remain unchanged when new modules are introduced or existing ones removed.
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):N/A
Special notes for your reviewer:
N/A