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.

Sign up for GitHub

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

feat: PoC for passing data to plugins #3498

Closed
wants to merge 6 commits into from
Closed

feat: PoC for passing data to plugins #3498

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0da6600
feat: Pass data to plugin
clockworkgr May 10, 2023
f51152f
feat: ensure go.mod points to the updated cli/manifest
clockworkgr May 10, 2023
7a63ab7
feat: Add module analysis
clockworkgr May 11, 2023
85e27d2
feat: Plugin go.mod to correct commit
clockworkgr May 11, 2023
8401d74
fix: add additional includes
clockworkgr May 16, 2023
0108ea9
feat: Correctly resolve protos in indirect deps
clockworkgr May 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions ignite/pkg/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ func GetModuleList(ctx context.Context, c *chain.Chain) (map[string]string, erro
if err != nil {
return nil, err
}
includePaths, err := g.resolveInclude(c.AppPath())

// Discover any custom modules defined by the user's app
g.appModules, err = g.discoverModules(g.appPath, g.protoDir)
Expand Down Expand Up @@ -140,7 +139,11 @@ func GetModuleList(ctx context.Context, c *chain.Chain) (map[string]string, erro
if err != nil {
return nil, err
}

/*
Regardless of any modules in use directly defined in the go module
add it to the includeDirs for correct proto include resolution
*/
g.includeDirs = append(g.includeDirs, path)
// Discover any modules defined by the package
modules, err := g.discoverModules(path, "")
if err != nil {
Expand All @@ -159,7 +162,11 @@ func GetModuleList(ctx context.Context, c *chain.Chain) (map[string]string, erro

g.thirdModules[modulesInPath.Path] = append(g.thirdModules[modulesInPath.Path], modulesInPath.Modules...)
}

// Perform include resolution AFTER includeDirs has been fully populated
includePaths, err := g.resolveInclude(c.AppPath())
if err != nil {
return nil, err
}
var modulelist []ModulesInPath
modulelist = append(modulelist, ModulesInPath{Path: c.AppPath(), Modules: g.appModules})
for sourcePath, modules := range g.thirdModules {
Expand Down
8 changes: 5 additions & 3 deletions ignite/pkg/gomodule/gomodule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,11 @@ func ResolveDependencies(f *modfile.File) ([]module.Version, error) {
}

for _, req := range f.Require {
if req.Indirect {
continue
}
/* Commented out to include indirect dependencies as proto files may import from them
if req.Indirect {
continue
}
*/
Comment on lines +71 to +75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now I'm not sure if this would create an issue with the current implementation so maybe in this case I would keep the existing behaviour as default to avoid having any side effect in the existing implementation by adding a boolean argument like for example:

func ResolveDependencies(f *modfile.File, includeIndirect bool) ([]module.Version, error)

Ideally we should confirm that the removal of the if res.Indirect condition won't have any side effects.

if !isReplacementAdded(req.Mod) {
versions = append(versions, req.Mod)
}
Expand Down