-
Notifications
You must be signed in to change notification settings - Fork 302
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
Embedded JavaScript pipeline PoC #1829
base: main
Are you sure you want to change the base?
Conversation
clicommand/pipeline_eval.go
Outdated
process.Enable(runtime) // process.env() | ||
|
||
// provide plugin() as a native module (implemented in Go) | ||
registry.RegisterNativeModule("buildkite/plugin", func(runtime *goja.Runtime, module *goja.Object) { |
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.
Ohhh, this is how you did it. I can imagine expanding on the API would be a bit of a pain if it's all in Golang. Presumably we could RunScript
with some existing gear in the VM first?
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.
Ah right, looks like it's built into goja_nodejs
already https://github.com/dop251/goja_nodejs/blob/master/require/module.go#L114. I can't quite figure out how to enable it 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.
@keithpitt Yeah this registry.RegisterNativeModule("buildkite/plugin", …
is just a proof-of-concept of JS functions implemented in Go, but I think you'd want a very good reason to do that; like interacting with existing/complex Go code/libs.
The require.WithLoader(func(name string) ([]byte, error) { …
~20 lines further up loads require("buildkite/*")
straight out of resources/node_modules/buildkite/*.js
files which get embedded into the buildkite-agent
binary at compile time and exposed via a virtual read-only filesystem.
clicommand/pipeline_eval.go
Outdated
// Add support for require() CommonJS modules. | ||
// require("buildkite/*") is handled by embedded resources/node_modules/buildkite/* filesystem. | ||
// Other paths are loaded from the host filesystem. | ||
registry := require.NewRegistry( | ||
require.WithLoader(func(name string) ([]byte, error) { | ||
if !strings.HasPrefix(name, "node_modules/buildkite/") { | ||
return require.DefaultSourceLoader(name) | ||
} | ||
res := resources.FS | ||
data, err := res.ReadFile(name) | ||
if errors.Is(err, fs.ErrNotExist) { | ||
return nil, require.ModuleFileDoesNotExistError | ||
} else if err != nil { | ||
return nil, err | ||
} | ||
return data, nil | ||
}), | ||
) |
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.
@keithpitt This is the bit which will load require("buildkite/whatever")
from resources.FS
which is a virtual filesystem exposing resources/node_modules/buildkite/*
files embedded in the compiled binary.
clicommand/pipeline_eval.go
Outdated
y, err := yaml.Marshal(v.Export()) | ||
if err != nil { | ||
return err | ||
} |
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.
it might be cool to evaluate the generated pipeline.yml against the pipeline json schema here to safeguard against generating a wonky pipeline
🤔 i wonder if we should do that on pipeline upload as well, even if we just output warnings
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.
Yeah that's be nice on upload too!
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.
Oh, hell yeah! If we can get this thing talking to npm, maybe it makes sense to keep the schema in a package. If we kept it in sync, then our pipeline editor in the UI could validate against it too.
package resources (embedded filesystem) is also folded into package js.
Now require("buildkite/*") will attempt to load from host filesystem if the requested file doesn't exist in the embedded filesystem.
I've done a bit of refactoring, and some improvements to |
I love this idea - I find the yaml quite challenging to work with, and I love the idea of a Turing complete DSL, with factoring and lovely things like that. But why JavaScript vs typescript? With typescript you'd get IDE integration for free, and discoverability would increase 1000%, plus more problems surfaced at compile time instead of runtime |
Hey @davidwheeler123 – glad you stumbled across this! This was purely a first-pass experiment, and the use of javascript here doesn't mean much – we've also played around with an embedded typescript engine :) This is early days, but certainly a direction we're exploring |
Experimental PoC adding a
buildkite-agent pipeline eval buildkite.js
command using github.com/dop251/goja as a JavaScript (“ECMAScript 5.1(+)”) interpreter, plus github.com/dop251/goja_nodejs for CommonJSrequire("…")
and some other conveniences.require("buildkite/plugin")
demonstrates a native module, implementing theplugin(name, ver, config)
JavaScript function as a Go Function.Other
require("buildkite/*")
modules are provided by an embedded filesystem, located in a new packageresources
; it currently containsnode_modules/buildkite/hello.js
which servicesrequire("buildkite/hello")
and demonstratesconsole.log()
working.Other
require("…")
paths are served from the host filesystem (i.e. the build directory).There's an example JavaScript pipeline at
test/fixtures/pipelines/buildkite.js
demonstrating some basic module loading, environment usage, plugin reuse etc:This can be piped into
buildkite-agent pipeline upload
for now, although eventually it would be integrated.$ ./buildkite-agent pipeline eval test/fixtures/pipelines/buildkite.js | ./buildkite-agent pipeline upload