`jj git colocate` #4833

cormacrelf · 2024-11-11T14:12:26Z

Fixes #4624

Open to bikesheds on what to call the inverse command. jj git delocate?

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

joyously · 2024-11-11T15:42:31Z

Open to bikesheds on what to call the inverse command. jj git delocate?

Would it be isolate?

neongreen · 2024-11-11T16:13:36Z

jj git dismantle? jj git purify?

cormacrelf · 2024-11-11T22:22:48Z

If you're turning off git, you are really turning a page on what once was. One might type it with a renewed sense of purpose, confidence in your knowledge of the JJ way, and independence of the crutch you once needed. In that vein I suggest

jj git renounce, my favourite; or
jj git disavow

(Also like jj git dismantle.)

martinvonz · 2024-11-11T22:32:47Z

jj git internalize might be okay. I think of the operation as making the .git/ dir internal to .jj/. But it's not as cheeky as some of the other options :)

emilazy · 2024-11-11T22:36:24Z

jj git conceal keeps everything nicely under a co‐ namespace. (Not serious. Or am I?)

joyously · 2024-11-11T22:44:52Z

separate comes to mind.
Or jj git colocate --reverse
I think of most of the git subcommands as actual Git commands, but this is not, so it seems a bit different. (both colocate and its reverse)

lib/src/workspace.rs

We need this to differentiate primary/secondary workspaces when choosing how to colocate them (and for now, doing nothing for workspaces as worktrees have not landed).

essiene · 2024-11-12T12:19:48Z

separate comes to mind. Or jj git colocate --reverse I think of most of the git subcommands as actual Git commands, but this is not, so it seems a bit different. (both colocate and its reverse)

I really like colocate --reverse since unlike other suggestions, I don't have to think about what this does.

If there must be a command under jj git, a close second of colocate --reverse would be jj git decolocate, which is not a real word to be sure, but also communicates intent clearly.

fwiw ;-)

cormacrelf · 2024-11-12T12:32:19Z

cli/src/commands/git/colocate.rs

+        let old_path = workspace_command.repo_path().join("store").join("git");
+        let new_path = workspace.workspace_root().join(".git");
+        fs::rename(&old_path, &new_path).context(old_path)?;


Ah, sorry, we have to check that old_path exists (and is a directory, and is not a symlink, and also that git_target points to it) before we commit to writing out core.bare=false.

As long as we check this, I don't think we need to handle the large variety of git_target scenarios outside of this, because there will soon be an alternative, which is to just add a worktree. Which will work in many more scenarios. git_target can point into outer space for all we care.

The idea is to have --prefer-worktree switch to using worktrees in the primary workspace, as well as secondary ones.

Initially, we can only colocate the primary workspace. See <https://martinvonz.github.io/jj/latest/git-compatibility/#converting-a-repo-into-a-co-located-repo> for the steps. Adds a Workspace::reload() function to reload the workspace from the file system, which is technically not necessary for this diff, but will become necessary soon, when we make the GitBackend colocation-aware.

PhilipMetzger · 2024-11-12T17:41:20Z

Open to bikesheds on what to call the inverse command. jj git delocate?

I'd like to have the inverse in the native backend with something like adopt but that can wait. Otherwise internalize is a fine name.

yuja · 2024-11-15T11:23:50Z

cli/src/commands/git/colocate.rs

+    }
+
+    let git_repo = git_backend.git_repo();
+    if workspace.is_primary_workspace() {


Can we detect this somehow by caller or git layer?

I don't think "is primary workspace" can be a common concept across working-copy/workspace-loader backends.

yuja · 2024-11-15T11:24:49Z

cli/src/commands/git/colocate.rs

+
+    if workspace_command.working_copy_shared_with_git() {
+        return Err(user_error(format!(
+            "Workspace '{}' is already colocated.",


nit: error message doesn't usually contain period

yuja · 2024-11-15T11:26:32Z

cli/src/commands/git/colocate.rs

+        let old_path_canon = old_path.canonicalize().ok();
+
+        let mut config =
+            gix::config::File::from_git_dir(common_dir.to_path_buf()).map_err(internal_error)?;


nit: maybe this should be a user error (assuming it would do some I/O)?

BTW, the gix' fetch/clone implementation might help understand their config API. I'm not familiar with that.
https://github.com/GitoxideLabs/gitoxide/blob/main/gix/src/clone/fetch/util.rs

yuja · 2024-11-15T11:28:25Z

cli/src/commands/git/colocate.rs

+    //
+    // This way we end up with a git HEAD written immediately, rather than
+    // next time @ moves. And you can immediately start running git commands.
+    let workspace = workspace


Maybe we can use command.workspace_helper_no_snapshot()?

yuja · 2024-11-15T11:29:46Z

cli/src/commands/git/colocate.rs

+
+    git::reset_head(tx.repo_mut(), &git2_repo, &wc_commit)?;
+
+    tx.finish(ui, format!("Colocated existing workspace {name}"))?;


nit: Colocated -> colocate

yuja · 2024-11-15T11:31:43Z

cli/src/commands/git/colocate.rs

+        if old_path.is_symlink()
+            || !old_path.is_dir()


nit: use .symlink_metadata() instead of stat()ing twice?

yuja · 2024-11-15T11:34:42Z

jj git colocate --reverse

I don't like the name --reverse, but it's nice that we don't have to add two separate commands which would be rarely used.

cormacrelf force-pushed the git-colocate branch from 16ec8cb to 6091659 Compare November 11, 2024 14:17

cormacrelf force-pushed the git-colocate branch 2 times, most recently from 14df9e7 to c5ad1b8 Compare November 11, 2024 15:58

cormacrelf commented Nov 11, 2024

View reviewed changes

lib/src/workspace.rs Outdated Show resolved Hide resolved

cormacrelf force-pushed the git-colocate branch from c5ad1b8 to e9de61d Compare November 12, 2024 09:09

workspace: Add Workspace::is_primary_workspace()

879288c

We need this to differentiate primary/secondary workspaces when choosing how to colocate them (and for now, doing nothing for workspaces as worktrees have not landed).

cormacrelf force-pushed the git-colocate branch 2 times, most recently from c510233 to 335ac54 Compare November 12, 2024 09:43

cormacrelf requested a review from yuja November 12, 2024 09:58

cormacrelf commented Nov 12, 2024

View reviewed changes

cormacrelf force-pushed the git-colocate branch 3 times, most recently from 60c3272 to db0a1eb Compare November 12, 2024 14:26

cormacrelf force-pushed the git-colocate branch from db0a1eb to 5fb6856 Compare November 12, 2024 14:52

yuja reviewed Nov 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`jj git colocate` #4833

`jj git colocate` #4833

cormacrelf commented Nov 11, 2024

joyously commented Nov 11, 2024

neongreen commented Nov 11, 2024

cormacrelf commented Nov 11, 2024 •

edited

Loading

martinvonz commented Nov 11, 2024

emilazy commented Nov 11, 2024

joyously commented Nov 11, 2024

essiene commented Nov 12, 2024 •

edited

Loading

cormacrelf Nov 12, 2024

cormacrelf Nov 12, 2024

PhilipMetzger commented Nov 12, 2024

yuja Nov 15, 2024

yuja Nov 15, 2024

yuja Nov 15, 2024

yuja Nov 15, 2024

yuja Nov 15, 2024

yuja Nov 15, 2024

yuja commented Nov 15, 2024


		git::reset_head(tx.repo_mut(), &git2_repo, &wc_commit)?;

		tx.finish(ui, format!("Colocated existing workspace {name}"))?;

jj git colocate #4833

Are you sure you want to change the base?

jj git colocate #4833

Conversation

cormacrelf commented Nov 11, 2024

Checklist

joyously commented Nov 11, 2024

neongreen commented Nov 11, 2024

cormacrelf commented Nov 11, 2024 • edited Loading

martinvonz commented Nov 11, 2024

emilazy commented Nov 11, 2024

joyously commented Nov 11, 2024

essiene commented Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhilipMetzger commented Nov 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuja commented Nov 15, 2024

`jj git colocate` #4833

`jj git colocate` #4833

cormacrelf commented Nov 11, 2024 •

edited

Loading

essiene commented Nov 12, 2024 •

edited

Loading