Skip to content

Add the jj-core crate - #9766

Open
PhilipMetzger wants to merge 8 commits into
mainfrom
pm/xuotnqsxlkpx
Open

Add the jj-core crate#9766
PhilipMetzger wants to merge 8 commits into
mainfrom
pm/xuotnqsxlkpx

Conversation

@PhilipMetzger

Copy link
Copy Markdown
Contributor

These are the simple parts of the jj-core extraction, see #6284 (comment) for what this entails.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

@PhilipMetzger
PhilipMetzger requested a review from a team as a code owner July 6, 2026 16:03
@PhilipMetzger
PhilipMetzger force-pushed the pm/xuotnqsxlkpx branch 5 times, most recently from 9234b2b to 951ed8a Compare July 6, 2026 17:58
Comment thread core/src/lib.rs Outdated
Comment thread core/src/lib.rs Outdated
Comment thread lib/core/src/lib.rs
Comment thread lib/core/proc-macros/src/content_hash.rs
@PhilipMetzger
PhilipMetzger force-pushed the pm/xuotnqsxlkpx branch 3 times, most recently from 0b88216 to ba29d5d Compare July 6, 2026 21:37
Comment thread lib/src/matchers.rs
Comment thread core/.gitignore Outdated
Comment thread lib/core/src/lib.rs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Regarding directory structure, are we going to flatten all sub crates that are re-exported from jj-lib?

cli/
core/
lib/
gen-protos/
testutils/  ??
git-backend/
...

or put them under lib/ as before?

cli/
lib/core/
lib/gen-protos/
lib/testutils/
lib/git-backend/
lib/...

Just wondering. I don't have a preference either way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess this is more a question for @martinvonz. I'd prefer to move to a crates/ directory at some point but Martin said that he prefers the flat structure for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or put them under lib/ as before?

I like this better. Then the crate name could be jj- followed by the path segments joined by -, e.g. jj-cli, jj-lib-core, jj-lib-gen-protos. What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IMO, I can do that although I'd prefer if we didn't prefix lib path to the name since these crates should be quite independent of jj-lib.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that actually sounds good to me. I was thinking that things like jj-gen-protos sounded too generic, but gen-protos/ will presumably be split up and go into local-working-copy/gen-protos/, simple-op-store/gen-protos/ etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking at this PR again, I was confused why the new crate is under lib/. As you said, jj-core is supposed to be independent of jj-lib, so it seems to me like it shouldn't be under lib/. I think my previous concern was just about gen-protos/ and testutils/, but I think the answer is that we have core/, lib/ and core/testutils/ (if useful) and lib/testutils/. I think the crate names should match the directory path (with an implied jj-). Does that make sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So you still prefer variant 1 of #9766 (comment) if I'm understanding correctly? Then I'll need to move everything back again.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I prefer a mix of them:

cli/
core/
core/testutils/
git-commit-backend/
lib/
lib/testutils/
local-working-copy/
local-working-copy/gen-protos/
simple-op-store/
simple-op-store/gen-protos/
...

Do you follow what I mean? What do you (all) think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you follow what I mean?

yes, thanks for clarifying.

What do you (all) think?

I still don't have any strong opinion on the structure. And I would guess the same for Yuya but he should speak for himself.

Comment thread Cargo.lock
Comment thread lib/src/ref_name.rs Outdated

@pksunkara pksunkara left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think str_util, time_util needs to be moved too.

@martinvonz

Copy link
Copy Markdown
Contributor

I think str_util, time_util needs to be moved too.

I think the idea is to move most (?) of jj-lib. This is just a first PR.

@PhilipMetzger PhilipMetzger changed the title Add a jj-core crate Add the jj-core crate Jul 8, 2026
@pksunkara

Copy link
Copy Markdown
Member

👍 If we are doing this incrementally and merge this PR first, I will send the PR to move str_util and time_util and any other things I get to do in the meanwhile.

Comment thread lib/core/proc-macros/src/content_hash.rs
Comment thread lib/core/src/repo_path.rs
Comment thread Cargo.lock
Comment thread lib/src/ref_name.rs Outdated
Comment thread Cargo.toml Outdated
Comment thread lib/core/proc-macros/src/content_hash.rs
Comment thread lib/proc-macros/src/lib.rs Outdated
Comment thread lib/core/src/content_hash.rs
@PhilipMetzger
PhilipMetzger force-pushed the pm/xuotnqsxlkpx branch 2 times, most recently from 06e5493 to 82455c9 Compare July 16, 2026 20:45
Comment thread lib/core/src/lib.rs Outdated
Comment thread lib/src/content_hash.rs Outdated
assert_ne!(hash(&42i32), hash(&[42i32][..]));
}

// TODO: move this over when we lower `hex_util.rs`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not move hex_util.rs before anything else? it looks like it doesn't depend on anything else

Comment thread lib/core/src/file_util.rs Outdated
Comment thread lib/core/src/file_util.rs Outdated
Comment thread lib/core/src/repo_path.rs Outdated
Comment thread lib/core/src/repo_path.rs Outdated
Comment thread lib/core/src/repo_path.rs Outdated
Comment thread lib/src/matchers.rs
This will be common base for building upon `jj` if you only want the internals which 
make the other systems work, like the `Backend` or `WorkingCopy` trait. It should be of utmost importance
to make the crate as low dependency as possible so its not in the critical path during compilation. 

Part of #6284
This part of the library is quite basic and it doesn't have too many dependencies which also need to 
move to make it happen. We also need to move the `ContentHash` macro since all `RepoPath` types depend on it,
the macro is moved to a new `jj-core-proc-macros` crate which mirrors the existing structure in `jj-lib`.
Since we now have `jj-core-proc-macros` this also deprecates `jj-lib-proc-macros` for external consumers.

Part of #6284
This is in preparation of moving `Backend`, `Index` and `Store`. I've also added the `Nothing/EverythingMatcher`
even though it's a clear layering violation but I've granted myself an exception for that since they're quite basic.
Its a simple move since it only depends on `RepoPathBuf`
which already is in the new core crate.

Part of #6284
@PhilipMetzger
PhilipMetzger force-pushed the pm/xuotnqsxlkpx branch 2 times, most recently from a9a1b10 to 3c50ef1 Compare July 31, 2026 21:58
This moves the first interface into the new core library. This also moves the `Signer` struct by 
dropping the `Config` dependency, to preserve the `jj-lib` API it is imported as `CoreSigner`.

Part of #6284
Move the pest grammar, the parser and all DSL related parts to the new crate.
Its needed so we can move the `WorkspaceName` and `WorkspaceNameBuf` newtypes for the `WorkingCopyStore`
trait. See the next patch.

Part of #6284
So `jj-core` users also have access to it, since we already did the same thing for the `RevsetParser`.
This is part of building a new `jj-core` crate.

Part of #6284
Moving this is quite simple and adds another trait to the core crate.

Part of #6284
Comment thread lib/src/repo_path.rs

@martinvonz martinvonz Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems weird to leave behind a repo_path module without RepoPath. I sent #9893 to make this more clean.

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

Successfully merging this pull request may close these issues.

4 participants