feat: cmd/ipfs: Make it possible to depend on cmd/ipfs #10219
Conversation
Stebalien
force-pushed
the
magiks-ribs-hacks
branch
3 times, most recently
from
25036fc
to
66d0f6b
Compare
| // - run the command invocation | ||
| // - output the response | ||
| // - if anything fails, print error, maybe with help. | ||
| func Start(buildEnv func(ctx context.Context, req *cmds.Request) (cmds.Environment, error)) (exitCode int) { |
There was a problem hiding this comment.
To be honest, this feels like the wrong thing to export. We should just be exporting a way to start the daemon that's entirely independent of the commandline tool.
There was a problem hiding this comment.
Maybe, although I think for what @magik6k was looking for he wanted both the CLI and daemon and so this seems natural.
IIUC it's basically a way to do preloaded plugins with a fairly small and Go versioned codebase that uses imports rather than what effectively operates like a kubo fork.
For example, instead of:
- git pull the latest version, add your plugins as a dependency, compile
- update kubo dependency in your repo, compile
It's got some tradeoffs with regards to a standard preloaded plugin build, but they seem like reasonable ones for someone to make.
Were you thinking to expose two different things, one for starting the daemon and another for the CLI?
There was a problem hiding this comment.
In my case I want to have the whole CLI with the plugin, doing it with start meant the least amount of code needed to get the cli + plugin
aschmahmann
force-pushed
the
magiks-ribs-hacks
branch
from
d01f209
to
0b20004
Compare
aschmahmann
force-pushed
the
magiks-ribs-hacks
branch
from
0b20004
to
287444b
Compare
There was a problem hiding this comment.
Thanks @magik6k and @Stebalien. Sorry it's taken so long to get to this 😬.
Generally LGTM, but had a few questions on some changes I made to make sure everything still works as the original PR intended. Thanks 🙏
| return err | ||
| } | ||
|
|
||
| func getRepoPath(req *cmds.Request) (string, error) { |
There was a problem hiding this comment.
@magik6k you previously exported this function, but I didn't see it used https://github.com/FILCAT/ribs/blob/main/integrations/kuri/cmd/kuri/main.go and wasn't sure it was useful otherwise. Did I miss something?
There was a problem hiding this comment.
I believe I ended up using a separate path, private is good.
|
|
||
| type PluginPreloader func(*loader.PluginLoader) error | ||
|
|
||
| func loadPlugins(repoPath string, preload PluginPreloader) (*loader.PluginLoader, error) { |
There was a problem hiding this comment.
@magik6k you previously exported this function, but I didn't see it used https://github.com/FILCAT/ribs/blob/main/integrations/kuri/cmd/kuri/main.go and wasn't sure it was useful otherwise. Did I miss something?
There was a problem hiding this comment.
Probably made it exported for tinkering, so private makes sense
| // BuildEnv creates an environment to be used with the kubo CLI. Note: the plugin preloader should only call functions | ||
| // associated with preloaded plugins (i.e. Load). | ||
| func BuildEnv(pl PluginPreloader) func(ctx context.Context, req *cmds.Request) (cmds.Environment, error) { |
There was a problem hiding this comment.
Added some comments here, but wondering if this should be something more restrictive (e.g. instead of taking *loader.PluginLoader pass something that really only operates on Load(plugin.Plugin) error like an interface or a function).
Thoughts?
No particularly strong feelings here, this seems like something easy to change later so documenting the edges doesn't seem so bad.
There was a problem hiding this comment.
Not sure how much value there is in restricting more, for me I only really care about being able to load my plugin
There was a problem hiding this comment.
Cool, fine leaving it for now can always change later and it shouldn't be a burden downstream
| @@ -36,6 +36,12 @@ For more information about the different types of Kubo plugins, see [plugins.md] | |||
|
|
|||
| Kubo plugins can also be injected at runtime using Go plugins (see below), but these are hard to use and not well supported by Go, so we don't recommend them. | |||
|
|
|||
| ### Kubo binary imports | |||
There was a problem hiding this comment.
Thoughts on a better name here (please don't say kubo-as-a-library, we already have some things there which are in need of deleting)?
| // - run the command invocation | ||
| // - output the response | ||
| // - if anything fails, print error, maybe with help. | ||
| func Start(buildEnv func(ctx context.Context, req *cmds.Request) (cmds.Environment, error)) (exitCode int) { |
There was a problem hiding this comment.
Maybe, although I think for what @magik6k was looking for he wanted both the CLI and daemon and so this seems natural.
IIUC it's basically a way to do preloaded plugins with a fairly small and Go versioned codebase that uses imports rather than what effectively operates like a kubo fork.
For example, instead of:
- git pull the latest version, add your plugins as a dependency, compile
- update kubo dependency in your repo, compile
It's got some tradeoffs with regards to a standard preloaded plugin build, but they seem like reasonable ones for someone to make.
Were you thinking to expose two different things, one for starting the daemon and another for the CLI?
Today there are two ways to run kubo with plugins, one on non-linux systems:
This PR adds a third way - by depending on
cmd/ipfs/kuboit's now possible to depend on this repo to create kubo-derived nodes with user-specified plugins.I needed this for RIBS, which is a highly-advanced blockstore, that couldn't justifiably "just be integrated" into upstream kubo.
mainusing this functionality: https://github.com/lotus-web3/ribs/blob/a8d8aff2953416bc267debd0ee05f591cca582d9/integrations/kuri/cmd/kuri/main.go#L16-L20Review probably easiest commit-by-commit.