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

Allow listing and resolving @git branches #1687

Merged
merged 4 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj debug completion`, `jj debug mangen` and `jj debug config-schema` have
been moved from `jj debug` to `jj util`.

* `jj` will no longer parse `br` as a git_ref `refs/heads/br` when a branch `br`
does not exist but the git_ref does (this is rare). Use `br@git` instead.

### New features

* `jj git push --deleted` will remove all locally deleted branches from the remote.
Expand Down Expand Up @@ -101,6 +104,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
now shorter within the default log revset. You can override the default by
setting the `revsets.short-prefixes` config to a different revset.

* The last seen state of branches in the underlying git repo is now presented by
`jj branch list` as a remote called `git` (e.g. `main@git`). They can also be
referenced in revsets. Such branches exist in colocated repos or if you use
`jj git export`.

### Fixed bugs

* Modify/delete conflicts now include context lines
Expand Down
16 changes: 15 additions & 1 deletion lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::op_store::RefTarget;
use crate::repo::{MutableRepo, Repo};
use crate::revset;
use crate::settings::GitSettings;
use crate::view::RefName;
use crate::view::{RefName, View};

#[derive(Error, Debug, PartialEq)]
pub enum GitImportError {
Expand Down Expand Up @@ -60,6 +60,20 @@ fn local_branch_name_to_ref_name(branch: &str) -> String {
format!("refs/heads/{branch}")
}

// TODO: Eventually, git-tracking branches should no longer be stored in
// git_refs but with the other remote-tracking branches in BranchTarget. Note
// that there are important but subtle differences in behavior for, e.g. `jj
// branch forget`.
pub fn git_tracking_branches(view: &View) -> impl Iterator<Item = (&str, &RefTarget)> {
view.git_refs().iter().filter_map(|(ref_name, target)| {
ref_name_to_local_branch_name(ref_name).map(|branch_name| (branch_name, target))
})
}

pub fn get_git_tracking_branch<'a>(view: &'a View, branch: &str) -> Option<&'a RefTarget> {
view.git_refs().get(&local_branch_name_to_ref_name(branch))
}

fn prevent_gc(git_repo: &git2::Repository, id: &CommitId) -> Result<(), git2::Error> {
// If multiple processes do git::import_refs() in parallel, this can fail to
// acquire a lock file even with force=true.
Expand Down
11 changes: 10 additions & 1 deletion lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use thiserror::Error;

use crate::backend::{BackendError, BackendResult, ChangeId, CommitId, ObjectId};
use crate::commit::Commit;
use crate::git::get_git_tracking_branch;
use crate::hex_util::to_forward_hex;
use crate::index::{HexPrefix, PrefixResolution};
use crate::op_store::WorkspaceId;
Expand Down Expand Up @@ -1616,7 +1617,9 @@ pub fn walk_revs<'index>(

fn resolve_git_ref(repo: &dyn Repo, symbol: &str) -> Option<Vec<CommitId>> {
let view = repo.view();
for git_ref_prefix in &["", "refs/", "refs/heads/", "refs/tags/", "refs/remotes/"] {
// TODO: We should remove `refs/remotes` from this list once we have a better
// way to address local git repo's remote-tracking branches.
for git_ref_prefix in &["", "refs/", "refs/tags/", "refs/remotes/"] {
if let Some(ref_target) = view.git_refs().get(&(git_ref_prefix.to_string() + symbol)) {
return Some(ref_target.adds());
}
Expand All @@ -1640,6 +1643,12 @@ fn resolve_branch(repo: &dyn Repo, symbol: &str) -> Option<Vec<CommitId>> {
return Some(target.adds());
}
}
// A remote with name "git" will shadow local-git tracking branches
if remote_name == "git" {
if let Some(target) = get_git_tracking_branch(repo.view(), name) {
return Some(target.adds());
}
}
}
None
}
Expand Down
29 changes: 14 additions & 15 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,8 @@ fn test_resolve_symbol_git_refs() {
// Nonexistent ref
assert_matches!(
resolve_symbol(mut_repo, "nonexistent", None),
Err(RevsetResolutionError::NoSuchRevision{name, candidates}) if name == "nonexistent" && candidates.is_empty()
Err(RevsetResolutionError::NoSuchRevision{name, candidates})
if name == "nonexistent" && candidates.is_empty()
);

// Full ref
Expand All @@ -470,27 +471,25 @@ fn test_resolve_symbol_git_refs() {
"refs/heads/branch".to_string(),
RefTarget::Normal(commit5.id().clone()),
);
mut_repo.set_git_ref(
"refs/tags/branch".to_string(),
RefTarget::Normal(commit4.id().clone()),
);
assert_eq!(
resolve_symbol(mut_repo, "heads/branch", None).unwrap(),
vec![commit5.id().clone()]
);

// Unqualified branch name
mut_repo.set_git_ref(
"refs/heads/branch".to_string(),
RefTarget::Normal(commit3.id().clone()),
// branch alone is not recognized
assert_matches!(
resolve_symbol(mut_repo, "branch", None),
Err(RevsetResolutionError::NoSuchRevision{name, candidates})
if name == "branch" && candidates.is_empty()
);
mut_repo.set_git_ref(
"refs/tags/branch".to_string(),
RefTarget::Normal(commit4.id().clone()),
);
// The *tag* branch is recognized
assert_eq!(
resolve_symbol(mut_repo, "branch", None).unwrap(),
vec![commit3.id().clone()]
vec![commit4.id().clone()]
);
// heads/branch does get resolved to the git ref refs/heads/branch
assert_eq!(
resolve_symbol(mut_repo, "heads/branch", None).unwrap(),
vec![commit5.id().clone()]
);

// Unqualified tag name
Expand Down
32 changes: 26 additions & 6 deletions src/commands/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::collections::BTreeSet;
use clap::builder::NonEmptyStringValueParser;
use itertools::Itertools;
use jujutsu_lib::backend::{CommitId, ObjectId};
use jujutsu_lib::git::git_tracking_branches;
use jujutsu_lib::op_store::RefTarget;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::revset;
Expand Down Expand Up @@ -258,6 +259,28 @@ fn cmd_branch_list(
let workspace_command = command.workspace_helper(ui)?;
let repo = workspace_command.repo();

let mut all_branches = repo.view().branches().clone();
for (branch_name, git_tracking_target) in git_tracking_branches(repo.view()) {
let branch_target = all_branches.entry(branch_name.to_owned()).or_default();
if branch_target.remote_targets.contains_key("git") {
// TODO(#1690): There should be a mechanism to prevent importing a
// remote named "git" in `jj git import`.
// TODO: This is not currently tested
writeln!(
ui.warning(),
"WARNING: Branch {branch_name} has a remote-tracking branch for a remote named \
`git`. Local-git tracking branches for it will not be shown.\nIt is recommended \
to rename that remote, as jj normally reserves the `@git` suffix to denote \
local-git tracking branches."
)?;
} else {
// TODO: `BTreeMap::try_insert` could be used here once that's stabilized.
branch_target
.remote_targets
.insert("git".to_string(), git_tracking_target.clone());
}
}

let print_branch_target =
|formatter: &mut dyn Formatter, target: Option<&RefTarget>| -> Result<(), CommandError> {
match target {
Expand Down Expand Up @@ -293,15 +316,12 @@ fn cmd_branch_list(

let mut formatter = ui.stdout_formatter();
let formatter = formatter.as_mut();
for (name, branch_target) in repo.view().branches() {

for (name, branch_target) in all_branches {
write!(formatter.labeled("branch"), "{name}")?;
print_branch_target(formatter, branch_target.local_target.as_ref())?;

for (remote, remote_target) in branch_target
.remote_targets
.iter()
.sorted_by_key(|(name, _target)| name.to_owned())
{
for (remote, remote_target) in branch_target.remote_targets.iter() {
if Some(remote_target) == branch_target.local_target.as_ref() {
continue;
}
Expand Down
60 changes: 60 additions & 0 deletions tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ fn test_branch_empty_name() {
For more information, try '--help'.
"###);
}

#[test]
fn test_branch_forget_glob() {
let test_env = TestEnvironment::default();
Expand Down Expand Up @@ -113,6 +114,65 @@ fn test_branch_forget_glob() {
"###);
}

#[test]
fn test_branch_forget_export() {
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");

test_env.jj_cmd_success(&repo_path, &["new"]);
test_env.jj_cmd_success(&repo_path, &["branch", "set", "foo"]);
let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "list"]);
insta::assert_snapshot!(stdout, @r###"
foo: 65b6b74e0897 (no description set)
"###);

// Exporting the branch to git creates a local-git tracking branch
let stdout = test_env.jj_cmd_success(&repo_path, &["git", "export"]);
insta::assert_snapshot!(stdout, @"");
let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "forget", "foo"]);
insta::assert_snapshot!(stdout, @"");
// Forgetting a branch does not delete its local-git tracking branch. This is
// the opposite of what happens to remote-tracking branches.
// TODO: Consider allowing forgetting local-git tracking branches as an option
let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "list"]);
insta::assert_snapshot!(stdout, @r###"
foo (deleted)
@git: 65b6b74e0897 (no description set)
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r=foo", "--no-graph"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revision "foo" doesn't exist
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r=foo@git", "--no-graph"]);
insta::assert_snapshot!(stdout, @r###"
rlvkpnrzqnoo [email protected] 2001-02-03 04:05:08.000 +07:00 65b6b74e0897
(empty) (no description set)
"###);

// The presence of the @git branch means that a `jj git import` is a no-op...
let stdout = test_env.jj_cmd_success(&repo_path, &["git", "import"]);
insta::assert_snapshot!(stdout, @r###"
Nothing changed.
"###);
// ... and a `jj git export` will delete the branch from git and will delete the
// git-tracking branch. In a colocated repo, this will happen automatically
// immediately after a `jj branch forget`. This is demonstrated in
// `test_git_colocated_branch_forget` in test_git_colocated.rs
let stdout = test_env.jj_cmd_success(&repo_path, &["git", "export"]);
insta::assert_snapshot!(stdout, @"");
let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "list"]);
insta::assert_snapshot!(stdout, @"");

// Note that if `jj branch forget` *did* delete foo@git, a subsequent `jj
// git export` would be a no-op and a `jj git import` would resurrect
// the branch. In a normal repo, that might be OK. In a colocated repo,
// this would automatically happen before the next command, making `jj
// branch forget` useless.
}

// TODO: Test `jj branch list` with a remote named `git`

fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
let template = r#"branches ++ " " ++ commit_id.short()"#;
test_env.jj_cmd_success(cwd, &["log", "-T", template])
Expand Down
29 changes: 29 additions & 0 deletions tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,35 @@ fn test_git_colocated_branches() {
"###);
}

#[test]
fn test_git_colocated_branch_forget() {
let test_env = TestEnvironment::default();
let workspace_root = test_env.env_root().join("repo");
let _git_repo = git2::Repository::init(&workspace_root).unwrap();
test_env.jj_cmd_success(&workspace_root, &["init", "--git-repo", "."]);
test_env.jj_cmd_success(&workspace_root, &["new"]);
test_env.jj_cmd_success(&workspace_root, &["branch", "set", "foo"]);
insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###"
@ 65b6b74e08973b88d38404430f119c8c79465250 foo
◉ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 master HEAD@git
◉ 0000000000000000000000000000000000000000
"###);
let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
insta::assert_snapshot!(stdout, @r###"
foo: 65b6b74e0897 (no description set)
master: 230dd059e1b0 (no description set)
"###);

let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "forget", "foo"]);
insta::assert_snapshot!(stdout, @"");
// A forgotten branch is deleted in the git repo. For a detailed demo explaining
// this, see `test_branch_forget_export` in `test_branch_command.rs`.
let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
insta::assert_snapshot!(stdout, @r###"
master: 230dd059e1b0 (no description set)
"###);
}

#[test]
fn test_git_colocated_conflicting_git_refs() {
let test_env = TestEnvironment::default();
Expand Down
1 change: 1 addition & 0 deletions tests/test_git_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ fn test_git_fetch_conflicting_branches_colocated() {
rem1 (conflicted):
+ f652c32197cf (no description set)
+ 6a21102783e8 message
@git (behind by 1 commits): f652c32197cf (no description set)
@rem1 (behind by 1 commits): 6a21102783e8 message
"###);
}
Expand Down
48 changes: 48 additions & 0 deletions tests/test_git_import_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,53 @@ use crate::common::{get_stderr_string, TestEnvironment};

pub mod common;

#[test]
fn test_resolution_of_git_tracking_branches() {
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");
test_env.jj_cmd_success(&repo_path, &["branch", "create", "main"]);
test_env.jj_cmd_success(&repo_path, &["describe", "-r", "main", "-m", "old_message"]);

// Create local-git tracking branch
let stdout = test_env.jj_cmd_success(&repo_path, &["git", "export"]);
insta::assert_snapshot!(stdout, @"");
// Move the local branch somewhere else
test_env.jj_cmd_success(&repo_path, &["describe", "-r", "main", "-m", "new_message"]);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
main: 3af370264cdc new_message
@git (ahead by 1 commits, behind by 1 commits): 16d541ca40f4 old_message
"###);

// Test that we can address both revisions
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"log",
"-r=main",
"-T",
r#"commit_id ++ " " ++ description"#,
"--no-graph",
],
);
insta::assert_snapshot!(stdout, @r###"
3af370264cdcbba791762f8ef6bc79b456dcbf3b new_message
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"log",
"-r=main@git",
"-T",
r#"commit_id ++ " " ++ description"#,
"--no-graph",
],
);
insta::assert_snapshot!(stdout, @r###"
16d541ca40f42baf2dea41aa61a0b5f1cbf1f91b old_message
"###);
}

#[test]
fn test_git_export_conflicting_git_refs() {
let test_env = TestEnvironment::default();
Expand Down Expand Up @@ -197,6 +244,7 @@ fn test_git_import_move_export_undo() {
test_env.jj_cmd_success(&repo_path, &["branch", "set", "a"]);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
a: 096dc80da670 (no description set)
@git (behind by 1 commits): 230dd059e1b0 (no description set)
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");

Expand Down