Skip to content

[plugins] Fix init hooks for custom plugins #1500

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

Merged
merged 2 commits into from
Sep 22, 2023
Merged

[plugins] Fix init hooks for custom plugins #1500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59331f2
[plugins] Fix init hooks for custom plugins
mikeland73 Sep 22, 2023
cf3ab26
Fix indent
mikeland73 Sep 22, 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
5 changes: 5 additions & 0 deletions examples/plugins/local/my-plugin/my-plugin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,10 @@
"{{ .Virtenv }}/some-file": "some-file.txt",
"{{ .DevboxDir }}/some-file.txt": "some-file.txt",
"{{ .Virtenv }}/process-compose.yaml": "process-compose.yaml"
},
"shell": {
"init_hook": [
"export MY_INIT_HOOK_VAR=BAR"
]
}
}
9 changes: 8 additions & 1 deletion examples/plugins/local/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,12 @@ if [ -z "$MY_FOO_VAR" ]; then
echo "MY_FOO_VAR environment variable is not set."
exit 1
else
echo "MY_FOO_VAR is set to '$MY_FOO_VAR'"
echo "MY_FOO_VAR is set to '$MY_FOO_VAR'"
fi

if [ -z "$MY_INIT_HOOK_VAR" ]; then
echo "MY_INIT_HOOK_VAR environment variable is not set."
exit 1
else
echo "MY_INIT_HOOK_VAR is set to '$MY_INIT_HOOK_VAR'"
fi
18 changes: 16 additions & 2 deletions internal/plugin/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,24 @@ import (
"go.jetpack.io/devbox/internal/devpkg"
)

func InitHooks(pkgs []*devpkg.Package, projectDir string) ([]string, error) {
func (m *Manager) InitHooks(
pkgs []*devpkg.Package,
includes []string,
) ([]string, error) {
hooks := []string{}
allPkgs := []Includable{}
for _, pkg := range pkgs {
c, err := getConfigIfAny(pkg, projectDir)
allPkgs = append(allPkgs, pkg)
}
for _, include := range includes {
name, err := m.ParseInclude(include)
if err != nil {
return nil, err
}
allPkgs = append(allPkgs, name)
}
for _, pkg := range allPkgs {
c, err := getConfigIfAny(pkg, m.ProjectDir())
if err != nil {
return nil, err
}
Expand Down
5 changes: 4 additions & 1 deletion internal/shellgen/scripts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ func WriteScriptsToFiles(devbox devboxer) error {

// Write all hooks to a file.
written := map[string]struct{}{} // set semantics; value is irrelevant
pluginHooks, err := plugin.InitHooks(devbox.InstallablePackages(), devbox.ProjectDir())
pluginHooks, err := devbox.PluginManager().InitHooks(
devbox.InstallablePackages(),
devbox.Config().Include,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: should this field be plural: Includes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this ship has sailed, has been in the devbox.json for a while now.

FWIW, I think we chatted about this an decided to use as a verb instead of noun (similar to import instead of imports). I don't feel super strongly, but it seems like it would be difficult to change (but I guess we could support both)

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh yeah, I do remember chatting about it being a verb...

)
if err != nil {
return errors.WithStack(err)
}
Expand Down