Skip to content
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

Make other working copies immutable #4421

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

martinvonz
Copy link
Owner

I'm a bit uneasy about this because of the test changes in the last commit. It's clearly a corner case there (undoing jj workspace forget causing some commits to become immutable), but there may be similar cases. I was wondering if we should create the new working-copy commits when snapshotting instead. I thought we had talked about that in the context of #3201 but I don't see any mention of it there. Maybe we talked about it on Discord. Anyone remembers?

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 tests to cover my changes

It's come up a few times that people want to mark working copies other
than the current one as immutable. We don't have a way of finding such
commits. `working_copies() ~ @` is close, but it's not correct if the
same commit is being edited in the current workspace and in another
workspace. This patch adds a `other_working_copies()` revset for it.
This should avoid other workspaces going stale most of the time.

Also include all working copies in default log since we would
otherwise hide other working copies now.
Copy link
Collaborator

Choose a reason for hiding this comment

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

working_copies() ~ @ is close, but it's not correct if the
same commit is being edited in the current workspace and in another
workspace.

Wouldn't it simpler to allow mutation in that case? If the wc commit is already editing, I wouldn't expect it to become immutable by editing the exact commit in other workspace.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It certainly would be simpler, but I think it seems more correct. Since the goal is to avoid accidentally creating stale workspaces, we should avoid that also if another workspace is editing the same commit (or a descendant).

There's also not currently a way to correctly calculate the set of commits edited in other workspaces, but maybe that doesn't really matter if we ignore this use case (i.e., I haven't thought of other use cases).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the goal is to avoid accidentally creating stale workspaces, we should avoid that also if another workspace is editing the same commit (or a descendant).

That's correct, but in order to edit the same commit, the user would have to use --ignore-immutable edit in another workspace, so I (as a user) wouldn't mind if it made the other workspace ignore immutability of the wc commit. (There might be other ways to enter such situation, but that's rare?)

Copy link
Owner Author

Choose a reason for hiding this comment

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

It could also happen if you do jj edit <parent> in one workspace and jj edit <child> in another workspace. Going back to the first workspace would then result in it becoming immutable. That seems consistent to me. However, since we don't prevent snapshotting even if the commit is immutable, it's already not going to work very well for preventing stale workspaces in that case, so I think it makes sense to simply add working_copies() ~ @ as I think you're suggesting. I'll update this PR to do just that.

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.

2 participants