Skip to content

Commit

Permalink
Draft for import --reset BRANCH.
Browse files Browse the repository at this point in the history
  • Loading branch information
ilyagr committed Jul 10, 2023
1 parent d81c599 commit 935b900
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 15 deletions.
9 changes: 1 addition & 8 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub enum GitImportError {
InternalGitError(#[from] git2::Error),
}

fn parse_git_ref(ref_name: &str) -> Option<RefName> {
pub fn parse_git_ref(ref_name: &str) -> Option<RefName> {
if let Some(branch_name) = ref_name.strip_prefix("refs/heads/") {
Some(RefName::LocalBranch(branch_name.to_string()))
} else if let Some(remote_and_branch) = ref_name.strip_prefix("refs/remotes/") {
Expand Down Expand Up @@ -620,13 +620,6 @@ pub fn fetch(
// Apart from `jj branch forget`, jj doesn't provide commands to manipulate
// remote-tracking branches, and local git branches don't affect fetch
// behaviors. So, it's unnecessary to export anything else.
//
// TODO: Create a command the user can use to reset jj's
// branch state to the git repo's state. In this case, `jj branch forget`
// doesn't work as it tries to delete the latter. One possible name is `jj
// git import --reset BRANCH`.
// TODO: Once the command described above exists, it should be mentioned in `jj
// help branch forget`.
let nonempty_branches: HashSet<_> = mut_repo
.view()
.branches()
Expand Down
9 changes: 9 additions & 0 deletions src/commands/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ pub struct BranchListArgs {
///
/// A forgotten branch will not impact remotes on future pushes. It will be
/// recreated on future pulls if it still exists in the remote.
///
/// When exporting the forgetting of a branch in colocated repositories or
/// during a `jj git export`, jj will delete the corresponding git repo branch
/// if it exists at the expected position.
///
/// If you'd like to instead override jj's branch with the git repo's branch,
/// you can use `jj git import --reset`. That command can also be used in rare
/// cases where a race condition causes the sync between git branches and jj
/// branches to fail because of conflicts.
#[derive(clap::Args, Clone, Debug)]
pub struct BranchForgetArgs {
/// The branches to forget.
Expand Down
65 changes: 58 additions & 7 deletions src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use std::time::Instant;
use clap::{ArgGroup, Subcommand};
use itertools::Itertools;
use jj_lib::backend::{CommitId, ObjectId, TreeValue};
use jj_lib::git::{self, parse_gitmodules, GitFetchError, GitPushError, GitRefUpdate};
use jj_lib::git::{
self, parse_git_ref, parse_gitmodules, GitFetchError, GitPushError, GitRefUpdate,
};
use jj_lib::git_backend::GitBackend;
use jj_lib::op_store::{BranchTarget, RefTarget};
use jj_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate};
Expand All @@ -19,7 +21,7 @@ use jj_lib::repo_path::RepoPath;
use jj_lib::revset::{self, RevsetIteratorExt as _};
use jj_lib::settings::{ConfigResultExt as _, UserSettings};
use jj_lib::store::Store;
use jj_lib::view::View;
use jj_lib::view::{RefName, View};
use jj_lib::workspace::Workspace;
use maplit::hashset;

Expand Down Expand Up @@ -153,7 +155,30 @@ pub struct GitPushArgs {

/// Update repo with changes made in the underlying Git repo
#[derive(clap::Args, Clone, Debug)]
pub struct GitImportArgs {}
pub struct GitImportArgs {
/// Destroy jj's record of BRANCH and then, if possible, restore it from the
/// git repo (can be repeated)
///
/// The state of BRANCH and its remote-tracking branches that was previously
/// recorded in jj is lost. If the git repo has corresponding branches or
/// remote-tracking branches, that state will then be imported into jj.
///
/// Branches other than BRANCH are not imported when --reset is specified.
///
/// This command can be used for recovery if the remote-tracking branches or
/// git-tracking branches become conflicted (this can happen, for example,
/// due to a data race).
///
/// If you'd like to destroy git's record of the BRANCH as well as jj's, do
/// `jj git import --reset BRANCH && jj branch forget BRANCH && jj git
/// export`. This should work even if a plain `jj branch forget BRANCH
/// && jj git export` doesn't work due to conflicts.
//
// We could provide an operation that only destroys jj's record of BRANCH, but it would have
// confusingly different behavior in colocated and non-colocated repos.
#[arg(long, value_name = "BRANCH")]
reset: Vec<String>,
}

/// Update the underlying Git repo with changes made in the repo
#[derive(clap::Args, Clone, Debug)]
Expand Down Expand Up @@ -958,13 +983,39 @@ fn classify_branch_update(
fn cmd_git_import(
ui: &mut Ui,
command: &CommandHelper,
_args: &GitImportArgs,
args: &GitImportArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let repo = workspace_command.repo();
let repo = workspace_command.repo().clone();
let git_repo = get_git_repo(repo.store())?;
let mut tx = workspace_command.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
let mut tx;
if args.reset.is_empty() {
tx = workspace_command.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
} else {
tx = workspace_command.start_transaction("git import --reset");
for branch in args.reset.iter() {
tx.mut_repo().remove_branch(branch);
}
let git_ref_filter = |ref_name: &RefName| match ref_name {
RefName::LocalBranch(branch) => args.reset.contains(branch),
RefName::RemoteBranch { branch, .. } => args.reset.contains(branch),
_ => false,
};
for git_tracking_ref in repo.view().git_refs().keys() {
if let Some(ref_name) = parse_git_ref(git_tracking_ref) {
if git_ref_filter(&ref_name) {
tx.mut_repo().remove_git_ref(git_tracking_ref);
}
}
}
git::import_some_refs(
tx.mut_repo(),
&git_repo,
&command.settings().git_settings(),
git_ref_filter,
)?;
}
tx.finish(ui)?;
Ok(())
}
Expand Down
77 changes: 77 additions & 0 deletions tests/test_git_import_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,83 @@ fn test_git_import_move_export_with_default_undo() {
"###);
}

#[test]
fn test_git_import_resert_conflcited_git_tracking() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let git_repo = git2::Repository::open(repo_path.join(".jj/repo/store/git")).unwrap();

test_env.jj_cmd_success(&repo_path, &["branch", "create", "br"]);
test_env.jj_cmd_success(&repo_path, &["describe", "-m=a"]);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
br: 812679adcf38 a
"###);
// Export a branch `br` when it's at `a`
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");
let opid_with_git_tracking_at_a = current_operation_id(&test_env, &repo_path);

test_env.jj_cmd_success(&repo_path, &["describe", "-m=b"]);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
br: b4a6b8c586dd b
@git (ahead by 1 commits, behind by 1 commits): 812679adcf38 a
"###);
// Export a branch `br` when it's at `b`
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");

// Now, the git repo is at b. We create a conflict by importing at a point of
// time where jj thinks the branch is at a.
let stdout = test_env.jj_cmd_success(
&repo_path,
&["--at-op", &opid_with_git_tracking_at_a, "git", "import"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
Concurrent modification detected, resolving automatically.
br: b4a6b8c586dd b
"###);

// Now we are stuck, export is broken. Forgetting the branch only forgets the
// local branch, and exporting does not work nor fix the git-tracking branches.
test_env.jj_cmd_success(&repo_path, &["branch", "forget", "br"]);
insta::assert_debug_snapshot!(get_git_repo_refs(&git_repo), @r###"
[
(
"refs/heads/br",
CommitId(
"b4a6b8c586ddf9e89264f164bbcf5c47219a5e5a",
),
),
]
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");
// Same as before
insta::assert_debug_snapshot!(get_git_repo_refs(&git_repo), @"[]");
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");

// import --reset removes the conflict
test_env.jj_cmd_success(&repo_path, &["git", "import", "--reset=br"]);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");

// After import --reset, `branch forget` works properly
test_env.jj_cmd_success(&repo_path, &["branch", "forget", "br"]);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");
// Same as before
insta::assert_debug_snapshot!(get_git_repo_refs(&git_repo), @r###"
[
(
"refs/heads/a",
CommitId(
"230dd059e1b059aefc0da06a2e5a7dbf22362f22",
),
),
]
"###);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
a: 230dd059e1b0 (no description set)
"###);
}

fn get_branch_output(test_env: &TestEnvironment, repo_path: &Path) -> String {
test_env.jj_cmd_success(repo_path, &["branch", "list"])
}
Expand Down

0 comments on commit 935b900

Please sign in to comment.