workspaces: Allow retrieving the path of another workspace

[pksunkara]

Implements #6854

This allows jj workspace root command to take an optional workspace arg to print the root directory of that workspace, while defaulting to the current workspace.

We allow this by creating a new workspace_store which has a separate entry for each workspace containing its name and root directory. This store is created during initiation of the repo and is maintained whenever add, forget & rename workspace subcommands are executed.

CAUTION: Moving a workspace path will not sync the path stored in the workspace store.

CAUTION: Specifying jj workspace root with a workspace created before this will result in an error.

Checklist

If applicable:

• [x] I have updated CHANGELOG.md
• [x] I have updated the documentation (README.md, docs/, demos/)
• [x] I have added/updated tests to cover my changes

[martinvonz]

The PR title gets me confused all the time. Is it okay if we change it to be the same as that of the FR it implements (i.e. "Allow retrieving the path of another workspace")?

[avamsi]

Driving by from https://github.com/jj-vcs/jj/issues/7485 -- is the main blocker here to align on where to persist the mapping (@martinvonz)? @pksunkara are you still willing to take this to completion if unblocked?

[martinvonz]

Yes, I think that's correct.

[pksunkara]

Yes, I would like to finish this once a consensus has been reached

[martinvonz]

Sorry that it took me so long to get back to this. What do you think about keeping a protobuf file with a map from workspace id to path (bytes)? That could be stored in something like .jj/repo/workspaces.

[pksunkara]

I don't mind doing it like that. The current PR does it as part of config.toml. Can I get a quick review to give me some information on the files that need to be changed to start this protobuf approach. I am afraid that I don't have full context of the internals.

[martinvonz]

I have very limited time until after Git Merge (two weeks from now). Maybe someone else can review?

[pksunkara]

@martinvonz Do you have time now to do a quick review?

[martinvonz]

Thanks for the reminder. It looks like the commit description is just a single line. Could you add some information about how the feature works and what the trade-offs are? For example, explain that the path is stored in the repo-level config file (iiuc) and what the limitations of that are (if any). That should make it easier for reviewers to get through the review. Sorry to have to ask for this. As with many open-source projects, reviewer time is often the bottleneck.

[pksunkara]

Sorry, updated the PR description.

[martinvonz]

Sorry, updated the PR description.

Could you move that into the commit description? Thanks. (See https://jj-vcs.github.io/jj/latest/contributing/#commit-guidelines)

[pksunkara]

Could you move that into the commit description? Thanks. (See jj-vcs.github.io/jj/latest/contributing#commit-guidelines)

I am going to redo this PR to use the protobuf approach as discussed before. And will make sure to do this at that point.

1. I assumed you reviewed that the commands that need to interact with the workspaces data is as implemented in this PR (add, forget, rename)? Or am I missing anything else?
2. Do I write the initial state of default workspace on jj initiation? Or when a new one is created?
3. I am assuming that we are ignoring situations where user has moved workspace paths after creating them. Can you confirm?

[avamsi]

If you’re moving the mapping to protobufs (or any other opaque storage), I think you’ll want to surface it in workspace list as well. Also, I’m not super familiar with the internals of workspaces (so take this with a grain of salt), but I kinda liked the idea of seeing them in the repo config -- at least initially. We can always move it once we have a more opinionated UX.

[yuja]

Abusing config file seems a bad idea because it is user-controlled data.

I think you’ll want to surface it in workspace list as well

+1. I expect it will be implemented by a follow-up PR.

[martinvonz]

• I assumed you reviewed that the commands that need to interact with the workspaces data is as implemented in this PR (add, forget, rename)? Or am I missing anything else?

I don't think I have reviewed this PR yet if that's what you mean.

• Do I write the initial state of default workspace on jj initiation? Or when a new one is created?

I think it makes sense to try to always keep it populated even if there's a single workspace by adding it in jj init. That seems cleaner than filling in the first workspace when the second one is added.

• I am assuming that we are ignoring situations where user has moved workspace paths after creating them. Can you confirm?

Yes, I think we can ignore that at least to start with. Once we have the paths recorded, we can follow up with tools to fix it up after moves. I think Git has tools for that.

[pksunkara]

@martinvonz Updated the PR to use protobuf approach. Things needed:

• [x] Need to resolve the 2 comments I asked.
• [x] Review
• [x] CI & tests

[pksunkara]

Updated. Please review the implementation.

[pksunkara]

Address the comments. Please review again. Thanks!

[pksunkara]

I have updated the protobuf to be a single file containing array of workspaces.

[pksunkara]

Addressed all the review comments.

[pksunkara]

Addressed all the review comments.

[pksunkara]

Addressed the nits. I can merge once approved.

[pksunkara]

Abusing config file seems a bad idea because it is user-controlled data.

I think you’ll want to surface it in workspace list as well

+1. I expect it will be implemented by a follow-up PR.

I have re-opened #7114 for the surfacing of path in workspace_list template.