cli: add new wc only to workspaces that became immutable this transaction#9331
cli: add new wc only to workspaces that became immutable this transaction#9331jjt wants to merge 2 commits intojj-vcs:mainfrom
Conversation
|
I think I invited you to jj-vcs/collaborators. I suspect you accepted that invitation. Did you get a notification soon thereafter that you were removed from that team? I suspect there something is wrong (or something I just don't understand) with the automation around this. |
That means the other workspaces will only be updated after the next operation is run from them, so any current changes in the other workspace's working copy will go into its current commit. I wonder if we should create the new commit in those workspaces just before we snapshot the working copy. The feature was added in #3600 but I didn't find any discussion about this there, but I also didn't look very hard. |
@martinvonz I did get an invitation and 30 mins later a revocation, all before I had a chance to accept. But, a couple hours ago I received a new one and accepted it before it could be revoked again.
Could you contextualize this a bit wrt this PR? This is my first time diving into jj source, so I'm not sure about where and under what conditions we'd want to create new commit(s) before snapshotting the current workspace's wc. See also this discord thread in #development about this change for any more context/etc. |
|
#9338 seems to be exacerbated by this PR's changes when I test localy, since it won't proactively put working copies on top of other workspaces when |
|
Going to put this into draft state, since it seems to be a deeper problem of tracking if another workspace's wc actually became immutable during this transaction, or started out immutable in the first place. As an example, if we're in |
0b2c7dc to
c892438
Compare
3b83f86 to
5a044d0
Compare
Splits `find_immutable_commit` into a thin wrapper over a new `find_immutable_commit_with` that takes the immutable expression as a parameter, so callers can evaluate immutability under an expression other than the current workspace's parsed one.
`finish_transaction` iterated every workspace's working copy and stacked a new commit on top of any that were immutable in the post-transaction repo. With workspace-dependent immutability (e.g., `immutable_heads() = working_copies() ~ @`), every other workspace's working copy looks immutable from the current workspace, so running `jj new` spuriously created empty commits on workspaces the user wasn't touching. Evaluate a workspace's wc immutability from that workspace's own perspective when deciding whether to stack: re-parse `immutable_heads()` with the target workspace as the revset context. For non-current workspaces, skip the stacking step if the wc was already immutable under its perspective in the pre-transaction repo. Under `working_copies() ~ @` this correctly excludes the target workspace's own `@` (so it isn't perpetually immutable to itself), while still treating it as immutable once something else in the transaction (e.g., a tag on its `@`) makes it so. The current workspace is unchanged. Two cases are covered by tests: (1) `jj new` from one workspace no longer cascades empty commits onto another workspace when `immutable_heads() = working_copies() ~ @`; (2) tagging another workspace's `@` does stack a new commit on top, including under configs that combine `builtin_immutable_heads()` with `working_copies() ~ @`. Fixes jj-vcs#7751
|
I've been testing this out locally and still hitting some edge cases with certain workspace/wc/config changes. Will continue to iterate. |
finish_transactioniterated every workspace's working copy and stacked a new commit on top of any that were immutable in the post-transaction repo. With workspace-dependent immutability (e.g.immutable_heads() = working_copies() ~ @), every other workspace's wc looks immutable from the current workspace, so runningjj newspuriously created empty commits on workspaces the user wasn't touching.Now, we evaluate each workspace's wc immutability from that workspace's own perspective when deciding whether to stack:
immutable_heads()is re-parsed in the context of the target workspace.For non-current workspaces, the stacking step is skipped if the wc was already immutable under its own perspective in the pre-transaction repo. Under
immutable_heads() = working_copies() ~ @this correctly excludes the target workspace's own wc while still treating it as immutable if something else in the transaction (e.g. a tag on its@) makes it so.Fixes #7751
See also this discord thread in #development.
Checklist
If applicable:
CHANGELOG.mdhow it works, how it's organized), including any code drafted by an LLM.
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.