Colocated workspaces using git worktrees - #4588
Conversation
aaebbbb to
44d144f
Compare
44d144f to
4919138
Compare
963b134 to
72283b8
Compare
| .iter() | ||
| .flat_map(|ext| ext.as_ref().new_resolvers(repo)) | ||
| .collect(), | ||
| workspace_ctx, |
There was a problem hiding this comment.
HEAD@git would have to be translated to CommitRef::GitHead(WorkspaceId) or something at lower_expression() (where we know the current workspace.)
There was a problem hiding this comment.
BTW, it might be okay to deprecate HEAD@git symbol since it can no longer be abstracted as a remote ref of the local @git remote. It serves as an indicator of colocated repo, but I've never used the HEAD@git symbol, which should usually point to @-.
There was a problem hiding this comment.
I had to add the workspace id to the resolver anyway, because the resolver needs to be able to generate HEAD@git as a did-you-mean suggestion, and must do that only when the current workspace is colocated. Since it had to be added to the resolver anyway, it was simple to resolve GitHead without changing CommitRef.
There was a problem hiding this comment.
Hmm, good point. I lean towards deprecating HEAD@git then.
As I said, it's no longer a valid abstraction, and the HEAD revision can be queried by git_head(). IIRC, HEAD@git resolution was added just because git_head template is rendered as such, not because we have a real use case.
You should be able to undo that operation. Maybe it's enough that we print a hint about that? Or was there some other consequence I'm missing? |
0a149ed to
cd66e02
Compare
…rktrees This also tests that new workspaces area colocated if the repo is. Creating the worktree will not be enough to satisfy this test -- we need JJ to advance commits and use the worktrees.
PhilipMetzger
left a comment
PhilipMetzger
left a comment
There was a problem hiding this comment.
Here's another batch of mostly minor nits.
| /// This flag works even if a repository was not | ||
| /// originally initialised with `--colocate`. |
There was a problem hiding this comment.
Nit: Remove this note and write it positively in the second paragraph.
There was a problem hiding this comment.
Updated, following a bit of the wording from jj git init --help.
There was a problem hiding this comment.
As a TODO until the PR is ready but this should have a good motivation for the change. See https://github.com/martinvonz/jj/blob/main/docs/contributing.md#code-reviews particularly this section
We are not particularly strict about the style, but please do explain the reason for the change unless it's obvious.
43ef037 to
a2ca03c
Compare
|
The main substantive question I have left is, when we import and export from git, should this always do it for every single worktree? Or just the current one? The approach I have taken so far is to do it for the current one. But I think this is spreading out the import over too many operations when the worktrees are, in fact, all right there, readable, and iterable. |
And workspaces might not always be accessible. They might be on USB stick or some network drive. So no, not all worktrees, but just the current one. |
Previously, only the default workspace could be colocated. This adds logic to the colocation-detection code to support finding a worktree as well as a full-on .git directory. Secondly, in a couple of places, this discovered worktree is opened instead of always opening the backing repo. This is so that when you import HEAD, you import it from the current worktree. This work can't be finished until we can store multiple git HEADs in the view, but is continued in the following few diffs.
…t worktree Now that workspaces exist, it is necessary to distinguish the two kinds of git repository `jj` might open. There's the git backend (which is either a bare repository or a colocated one) and the worktrees for all the colocated workspaces. Both of these are a `gix/git2::Repository`, the difference is just which path you opened. So you have to distinguish them with names. The worktree git repo is now mostly used instead of always opening the backend repo. This is so that when you import HEAD, you import it from the current worktree.
a2ca03c to
19d7bdf
Compare
This forms the basis of `jj workspace add --colocate`. The libraries don't quite have the functionality we need - gix doesn't do it at all - git2 does (Repository::worktree(name, opts)), but it always has to check out a commit. This implementation - can create in an already-existing directory; - writes a dummy HEAD and index; and - does not check out any files. JJ handles the rest.
E.g. you create a workspace, but there is already a git worktree of the same name. JJ should give you a hint as to what to do.
64c334e to
c6ad91e
Compare
New flag: jj workspace add --colocate. We simply add a worktree.
Previously, JJ's view of the git head (HEAD@git) could only track one HEAD, for only one workspace, specifically the one colocated with the main .jj/repo. Since we now allow workspaces to be colocated, we need to track a separate git HEAD per workspace. This allows us to import HEAD for only the current workspace, and export HEAD to the current workspace when @- moves. Thereby allowing git tools to behave correctly in those workspaces. This affects a lot of stuff. And a lot of tests. A lot more things need to know the current workspace ID, including e.g. the revset resolvers, and the commit templater.
This comes with a couple of tweaks to make `cargo insta review`-ing rehashes easier in future.
…spaces Pretty straightforward. Previously there was only `git_head()`, which a previous diff updated to use the HEAD for the current workspace.
Fails on the second snapshot assertion. The problem with not removing HEADs is that they'll just hang around forever if you don't cull them.
This fixes the test in the previous diff. But we do not remove when merging views, yet. I have another series of patches to do what the TODO describes, but it fails too many tests in pretty perplexing ways, so that's not in this PR.
We need this for when we forget colocated workspaces, so that users do not have to `git worktree remove` manually. The validation / removal separation is so we can two-phase-commit the removals during a multi-workspace forget operation, checking they're all valid and able to be removed during the transaction and removing the worktrees afterwards. Missing worktrees (like when the user has ALREADY run `git worktree remove`) are ignored.
The CLI analogue of the previous diff. We need this for when we forget colocated workspaces, so that users do not have to `git worktree remove` manually.
`jj workspace rename` exists now. This means that people currently using JJ with colocated repos and renaming their default workspace may lose their git HEAD briefly when they next upgrade. This guesswork limits that to cases where they both: - rename their default workspace - AND create a second workspace (which cannot be --colocate as that is being added in the same PR as View.git_heads) But it's not a huge deal -- JJ will re-add the git HEAD when it next does almost anything.
It's long-winded, but at least now we can test deprecated fields in the View end to end.
|
Is there anyone who can test on windows? I don't have a machine handy. Edit: never mind, I made a failing test and got the CI to tell me how git does it. DetailsThat path, de-backslashed, is which is a Verbatim path prefix. I suspect Git likes its paths in the gitfile ( ; git init repo
; cd repo
; touch file.txt && git add file.txt && git commit -m 'initial commit'
; git workspace add ../abc
; cat ../abc/.git
; cat .git/worktrees/abc/commondir
; cat .git/worktrees/abc/gitdir |
aadc9e1 to
3db4851
Compare
We're getting paths like this, I think.
gitdir: \\?\C:\Users\runneradmin\AppData\Local\Temp\jj-test-fy26a2\second
But I don't know what git does, and I don't have access to a windows machine.
Adding this so the CI windows build can tell us what's wrong.
Before
gitdir: \\?\C:\Users\runneradmin\AppData\Local\Temp\jj-test-fy26a2\second
After
gitdir: C:/Users/runneradmin/AppData/Local/Temp/jj-test-fy26a2/second
And same for the other worktree bits and pieces. We also change the way
the tests run git commands, because of yet another error to do with
Git being unable to handle verbatim paths:
command=`"git" "worktree" "add" "\\\\?\\C:\\Users\\runneradmin\\AppData\\Local\\Temp\\jj-test-ti0MHE\\second"`
code=128
stdout=""
stderr=```
Preparing worktree (new branch \'second\')
fatal: could not create leading directories of \'//?/C:/Users/runneradmin/AppData/Local/Temp/jj-test-ti0MHE/second/.git\': Invalid argument
3db4851 to
6047297
Compare
cormacrelf
changed the title
|
Could you somehow split this PR into two or three units? This looks too big to run multiple review rounds, and GitHub UI doesn't support per-patch acceptance. One idea (not sure if this is the easiest/simplest option):
|
|
Closing to split in half, next is #4644 |
100-line research doc for auri. Same shape as broker-lifetime-fix. Tracking issue: jj-vcs/jj#8052 (ONGOING since 2025-11-17). Original PR jj-vcs/jj#4588 was closed; work continues in smaller chunks at #4644. No ETA. Constraint today: only one workspace per repo can be colocated with git. 'jj workspace add' creates pure-jj workspaces with no .git in their directory; native git commands don't work there. 'jj git init --colocate' refuses to run inside an existing git worktree. Workarounds ranked: (1) Per-agent pure-jj workspaces backed by the main colocated repo. RECOMMENDED. jj fully works, native git doesn't — acceptable since claude-bus activity is jj-based; gh pr create runs from main workspace anyway. Forward-compatible with #8052 when it lands. (2) Git worktree per agent, drop jj. Regression on the existing workflow. Skip. (3) Separate clones per agent. Object-store duplication, manual coordination. Overkill. (4) Status quo + commit attribution (bast ideation pick). Diagnostic, not preventative. Ship complementarily. What'd change: bin/agent-launch ensures $BUS_ROOT/.workspaces/ <agent> exists via 'jj workspace add'; cd there before exec claude. .gitignore the .workspaces/ dir. Hooks already pass through payload.cwd correctly; no change there. CMake build stays in main, binary shared. Open questions for sulin: workspace location ($BUS_ROOT .workspaces/ vs /tmp), auto-create on first launch vs explicit init, shared op log vs per-workspace heads. Surfaced rather than guessed.
5 participants
Fixes #4436
Checklist
If applicable:
CHANGELOG.md