martinvonz / jj

A Git-compatible VCS that is both simple and powerful
https://martinvonz.github.io/jj/
Apache License 2.0
9.25k stars 319 forks source link

FR: JJ extension suppport #3575

Open matts1 opened 6 months ago

matts1 commented 6 months ago

Is your feature request related to a problem? Please describe. I'm trying to make a jj gerrit subcommand which will support gerrit. I imagine that people will make commands like jj github, jj lint, jj fix.

Describe the solution you'd like Not so much what I'd like - I'm not particularly attached to the idea and welcome other ideas, but my thoughts were:

The subprocess would then: 1) Read JJ_SERVER to get the specifications for the grpc channel and create a grpc connection over these. 2) Communicate with the jj parent process via grpc 3) Exit, which the parent process would see, and then kill the grpc server.

Describe alternatives you've considered

Additional context Add any other context or screenshots about the feature request here.

thoughtpolice commented 6 months ago

Note that I am adding Gerrit support in #2845; it has some rough edges still, but I (and several of my colleagues) use it almost every day. I mostly haven't done another round of review yet because I still have 1 or 2 things left to do before the PR is complete (notably adding tests, and documentation for users). I rebase it regularly. Nobody is particularly opposed to it I think, and I believe there is still much interest in adding other specific forge integrations into jj directly (github, etc.) Realistically in the case of forge support, I think having "First-class" support directly is nicer than the typical status quo in the Git world, which is "Everyone just finds some random binary and throws it into their workflow and they all work somewhat differently." That said, it's much more work for us. I suspect we'd probably only ever realistically have a ~handful of integrations, too.

That said, I have no particular argument against the actual FR here, which is for out of tree extensions that can sit somewhere in the command namespace.

Actually, as far as the specific design you suggest, I think exposing most of the jj's existing interfaces over RPC to arbitrary clients is, broadly speaking, an excellent goal in the long run because it not only paves a way for your extensions example — it also solves many of the thornier issues with things like dynamic loading and distributing custom binaries that are built in jj_cli. IMO, an RPC interface is actually vastly more important and broadly applicable than just being able to add subcommands to jj. For example, VSCode integration would work for any version of jj the user has, even if it has many extra additions, like custom backends.

Related issue: https://github.com/martinvonz/jj/issues/3001 — there has also been much more discussion about this over time in various places like Discord, so others should definitely chime in with their thoughts.

martinvonz commented 6 months ago

3034 is also related

matts1 commented 6 months ago

Ah, so the alternative suggested in #3034 would be to define jj-foo, similar to how git does it. I'm personally not in favor of that, since I believe that will just lead to binaries invoking jj commands themselves directly via the CLI. How do other people feel about it?

I quite like the fact that with my idea, jj has already started a gRPC server for you, scoped to that specific invocation. This means you don't need to worry about making sure that a long-running daemon is always running, or invoking jj api many times.

I think we could also provide optional language support, for example:

foo_extension = { binary = "/path/to/foo_ext.py", prelude = "python" }

And then jj itself would invoke a binary that does something along the lines of (pseudocode):

conn = grpc.connect(file_descriptor(os.environ["JJ_SERVER"]))
main = importlib.import("/path/to/foo_ext.py", "main")
main(conn, sys.argv)

This would significantly reduce the barrier to entry for an extension, meaning that all you need to do is:

def main(conn, argv):
    # Start using conn
PhilipMetzger commented 6 months ago

There's also #3219 which is along the same line and should be developed along with this.

noahmayr commented 6 months ago

Imo we would ideally be able to use extensions for backend implementations as well, maybe one of these crates can help with that: Maybe https://github.com/ZettaScaleLabs/stabby or https://github.com/extism/extism

Stabby (a stable rust ABI create) would mean that extensions could only be written in Rust though, while extism would be an embedded wasm runtime which supports a handful of plugin languages. If lua is sufficient as a plugin language, https://github.com/mlua-rs/mlua could also be an option

martinvonz commented 6 months ago

If we do need backends to be added without recompiling the binary, then maybe it would be enough to have a single proxy backend that implements the Backend trait and delegates to a configured gRPC address. I think we can wait and decide about that when we have some real use cases.

thoughtpolice commented 6 months ago

Imo we would ideally be able to use extensions for backend implementations as well, maybe one of these crates can help with that: Maybe https://github.com/ZettaScaleLabs/stabby or https://github.com/extism/extism

This isn't just about new backends, but — I am strongly against an approach leaning on the dynamic linker (stabby) unless we survey the use cases and come to the conclusion that it's absolutely necessary; it brings a very long tail of complications and many supposed benefits do not always materialize in practice on all platforms. extism is more interesting, but I think we'd have to do a lot of work for anything except Rust to be usable anyway (for one, we basically need to write FFI bindings for the jj hostcall APIs for N languages, in order to have anything that is reasonably usable OOTB), and the host<->sandbox bridge implies a lot of other technicalities that are not easy to work around without writing your own crates, anyway. For example, it would probably not be possible to use extism to, say, write a new working copy implementation that used something like FUSE, because the FUSE library would need to be compiled into the host application anyway before you could reasonably expose APIs for it. You end up back at square 1.

Realistically I think adding backends/trait impls is far, far less of a pertinent issue for users, so we should only ever cross that bridge when (if) we get to it and there are no other options.

senekor commented 2 weeks ago

I'm reading through this discussion from the perspective of wanting to convice people that jj util exec is a good idea. (discussion: https://github.com/martinvonz/jj/issues/3001, implemention: https://github.com/martinvonz/jj/pull/4759)

I noticed that jj util exec is basically a subset of the original idea here. It invokes another program, but doesn't expose a gRPC server to it. That is an extension that could later be added. If the gRPC server is cheap to spin up, we could do it for every invocation, even if most people would just use it for little scripts. If that's too expensive, we could add a flag to jj util exec that enabled the gRPC server feature. The original use case could then be solved along the following lines:

[aliases]
gerrit = ["util", "exec", "--with-grpc-server", "--", "/path/to/gerrit/binary"]
#                         ^^^^^^^^^^^^^^^^^^^^
#                    optional, to be discussed