Skip to content

RFC: more fine grained crate structure #6284

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

Open
thoughtpolice opened this issue Apr 8, 2025 · 6 comments
Open

RFC: more fine grained crate structure #6284

thoughtpolice opened this issue Apr 8, 2025 · 6 comments

Comments

@thoughtpolice
Copy link
Member

One of the things I've been thinking about for a bit is something along the lines of restructuring the crate graph to look like this (an arrow A -> B means A depends on B):

Image


Red boxes are our libraries. Green boxes are backend crates that:

  • Contain a Backend impl
  • And also contain a cli extension so that their commands can be added to the resulting binary

Therefore, backends need both jj-lib and jj-cli in order to compile, and those are then finally linked into the binary too. I think this makes sense: a backend crate needs to provide all the functionality for the backend, from storage to UX.

I think a crate structure like this will be nicer overall in the long term because it can (hopefully) reduce some compile times, and it will also split up the dependency tree to be a bit more fine grained e.g. there is a requirement for blake2 in jj-lib, but only for SimpleBackend, while in the above diagram we could move that a layer below and into the simplebackend crate.

We could then even think about adding some more boxes and arrows to the above diagram to make things more exciting. But I think this is probably the first "big" restructuring we would need.

There are a couple things standing in the way of this, but I think the biggest roadblock is: how do we test this stuff? We need to restructure the test setup a bit.

  • #[test] blocks, which exist inside jj-lib now and are fine
  • We have some tests that run both with the simple backend and the local backend to test common functionality
    • Other backends, both our own and 3rd party ones, might want access to these?
    • Should we export those tests from jj-lib in some way so that they can be used in downstream testsuites?
  • Moving things around this way means that most of the tests are probably better off in the actual crate that ships them; e.g. git related tests should be inside the hypothetical jj-git-backend crate.
  • Worth noting that many tests practically just use the Git backend when they could maybe not, but that is a massive amount of work to fix
    • It would probably be OK to either leave those as-is, or occasionally rewrite them slowly over time and move them into better places.
  • Currently the biggest problem is that the jj-lib crate uses testutils that has a hardcoded enum that describes what backend is in use when it constructs a TestEnvironment, IIRC. This means that we currently cannot move the Git backend out of the jj-lib crate and push it downward. This needs to be reworked completely, but I haven't thought much of it.

I'm interested in some other thoughts here, especially from Yuya since this has brushed up on a few PRs we did before. /cc @jj-vcs/maintainers

@emilazy
Copy link
Contributor

emilazy commented Apr 8, 2025

I would sorta like it if the lib and cli parts of backends were split. We shouldn’t be entrenching jj-cli as a library crate further, IMO. Would there be a big obstacle to doing that?

@thoughtpolice
Copy link
Member Author

thoughtpolice commented Apr 8, 2025

I don't think there's much of a practical difference in the short term. You have to depend on jj_cli as a library today anyway if you write any CLI functionality at all since you need types like WorkspaceCommandHelper. We would need to move more code out of jj-cli and push it downward in the dependency tree (perhaps into a new exciting jj-core crate that sits between lib and cli) before we could really get any benefit from having those things be separate or reducing the dependency on that. In practice any backend is going to have to depend on both, somewhere.

But, I also think it's better to just split up the crates slowly in big chunky pieces and not break them down too far at the beginning. That's just my gut feeling, though.

@joyously
Copy link

joyously commented Apr 8, 2025

The arrows make it look circular, which I doubt you intended.
What would it look like for something like gg which simply wants to use jj-lib and not jj-cli?

@yuja
Copy link
Contributor

yuja commented Apr 8, 2025

I think the arrow to "jj (binary)" is inverted (or is test dependency?)

Currently the biggest problem is that the jj-lib crate uses testutils that has a hardcoded enum that describes what backend is in use when it constructs a TestEnvironment, IIRC. This means that we currently cannot move the Git backend out of the jj-lib crate and push it downward.

I don't think we need to split testutils into smaller crates. It's nice that we can easily set up test workspace/repository without using various functions from various crates. Tests are independent artifacts. If we have to avoid project-level dependency cycle, I think we can just extract the tests to separate crate (e.g. jj-git-backend-tests -> {jj-git-backend .., testutils -> jj-git-backend ..}.)

@PhilipMetzger
Copy link
Contributor

I generally like the idea, yet think its still to early to split jj-lib into something like the lower level jj-core. Since there's a bunch of missing API-design decisions to make jj-lib comfortable for other downstream projects. I think we can keep this open wrt. #5685 since it's in my opinion still blocked on moving most of cli/src/cli_util.rs into jj-lib.

@martinvonz
Copy link
Member

there is a requirement for blake2 in jj-lib, but only for SimpleBackend

nit: It's used in many other places too (SimpleOpStore, StackedTable, default_index/).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants