mirror of
https://github.com/martinvonz/jj.git
synced 2024-09-11 13:56:29 +03:00
view: don't force commits pointed to by refs to be current heads
If you rewrite a commit that's also available on some remote, you'll currently see both the old version and the new version in the view, which means they're divergent. They're not logically divergent (the rewritten version should replace the old version), so this is a UX bug. I think it indicates that the set of current heads should be redefined to be the *desired* heads. That's also what I had suspected in the TODO removed by this change. I think another indication that we should hide the old heads even if they have e.g. a remote branch pointing to them is that we don't want them to be rebased if we rewrite an ancestor. So that's what I decided to do: let the view's heads be the desired heads. The user can still define revsets for showing non-current commits pointed to by e.g. remote branches.
This commit is contained in:
parent
97612f9e99
commit
33bf6ce1d5
@ -648,24 +648,6 @@ impl MutableRepo {
|
||||
.cloned()
|
||||
.collect();
|
||||
view.head_ids.extend(view.public_head_ids.iter().cloned());
|
||||
for branch_target in view.branches.values() {
|
||||
if let Some(ref_target) = &branch_target.local_target {
|
||||
view.head_ids.extend(ref_target.removes());
|
||||
view.head_ids.extend(ref_target.adds());
|
||||
}
|
||||
for ref_target in branch_target.remote_targets.values() {
|
||||
view.head_ids.extend(ref_target.removes());
|
||||
view.head_ids.extend(ref_target.adds());
|
||||
}
|
||||
}
|
||||
for ref_target in view.tags.values() {
|
||||
view.head_ids.extend(ref_target.removes());
|
||||
view.head_ids.extend(ref_target.adds());
|
||||
}
|
||||
for ref_target in view.git_refs.values() {
|
||||
view.head_ids.extend(ref_target.removes());
|
||||
view.head_ids.extend(ref_target.adds());
|
||||
}
|
||||
view.head_ids = self
|
||||
.index
|
||||
.heads(view.head_ids.iter())
|
||||
|
@ -16,7 +16,6 @@ use std::sync::Arc;
|
||||
|
||||
use jujutsu_lib::backend::{Conflict, ConflictId, ConflictPart, TreeValue};
|
||||
use jujutsu_lib::commit_builder::CommitBuilder;
|
||||
use jujutsu_lib::op_store::RefTarget;
|
||||
use jujutsu_lib::repo_path::RepoPath;
|
||||
use jujutsu_lib::store::Store;
|
||||
use jujutsu_lib::testutils;
|
||||
@ -383,50 +382,6 @@ fn test_remove_head(use_git: bool) {
|
||||
assert!(repo.index().has_id(commit3.id()));
|
||||
}
|
||||
|
||||
#[test_case(false ; "local backend")]
|
||||
// #[test_case(true ; "git backend")]
|
||||
fn test_remove_head_ancestor_git_ref(use_git: bool) {
|
||||
// Test that MutableRepo::remove_head() does not leave the view with a git ref
|
||||
// pointing to a commit that's not reachable by any head.
|
||||
let settings = testutils::user_settings();
|
||||
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
|
||||
|
||||
let mut tx = repo.start_transaction("test");
|
||||
let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
|
||||
let commit1 = graph_builder.initial_commit();
|
||||
let commit2 = graph_builder.commit_with_parents(&[&commit1]);
|
||||
let commit3 = graph_builder.commit_with_parents(&[&commit1]);
|
||||
let commit4 = graph_builder.commit_with_parents(&[&commit1]);
|
||||
let commit5 = graph_builder.commit_with_parents(&[&commit2, &commit3, &commit4]);
|
||||
tx.mut_repo().set_git_ref(
|
||||
"refs/heads/main".to_string(),
|
||||
RefTarget::Conflict {
|
||||
removes: vec![commit2.id().clone()],
|
||||
adds: vec![commit3.id().clone(), commit4.id().clone()],
|
||||
},
|
||||
);
|
||||
let repo = tx.commit();
|
||||
|
||||
let mut tx = repo.start_transaction("test");
|
||||
let mut_repo = tx.mut_repo();
|
||||
let heads = mut_repo.view().heads().clone();
|
||||
assert!(heads.contains(commit5.id()));
|
||||
mut_repo.remove_head(commit5.id());
|
||||
let heads = mut_repo.view().heads().clone();
|
||||
assert!(!heads.contains(commit5.id()));
|
||||
assert!(heads.contains(commit4.id()));
|
||||
assert!(heads.contains(commit3.id()));
|
||||
assert!(heads.contains(commit2.id()));
|
||||
assert!(!heads.contains(commit1.id()));
|
||||
let repo = tx.commit();
|
||||
let heads = repo.view().heads().clone();
|
||||
assert!(!heads.contains(commit5.id()));
|
||||
assert!(heads.contains(commit4.id()));
|
||||
assert!(heads.contains(commit3.id()));
|
||||
assert!(heads.contains(commit2.id()));
|
||||
assert!(!heads.contains(commit1.id()));
|
||||
}
|
||||
|
||||
#[test_case(false ; "local backend")]
|
||||
// #[test_case(true ; "git backend")]
|
||||
fn test_add_public_head(use_git: bool) {
|
||||
|
@ -1109,6 +1109,12 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
|
||||
resolve_commit_ids(mut_repo.as_repo_ref(), "remote_branches()"),
|
||||
vec![commit2.id().clone(), commit1.id().clone()]
|
||||
);
|
||||
// The commits don't have to be in the current set of heads to be included.
|
||||
mut_repo.remove_head(commit2.id());
|
||||
assert_eq!(
|
||||
resolve_commit_ids(mut_repo.as_repo_ref(), "remote_branches()"),
|
||||
vec![commit2.id().clone(), commit1.id().clone()]
|
||||
);
|
||||
// Can get branches when there are conflicted refs
|
||||
mut_repo.set_remote_branch(
|
||||
"branch1".to_string(),
|
||||
|
@ -890,10 +890,11 @@ fn test_rebase_descendants_basic_branch_update_with_non_local_branch() {
|
||||
Some(RefTarget::Normal(commit_b.id().clone()))
|
||||
);
|
||||
|
||||
// Commit B is still visible because the remote branch points to it
|
||||
// Commit B is no longer visible even though the remote branch points to it.
|
||||
// (The user can still see it using e.g. the `remote_branches()` revset.)
|
||||
assert_eq!(
|
||||
*tx.mut_repo().view().heads(),
|
||||
hashset! {repo.view().checkout().clone(), commit_b.id().clone(), commit_b2.id().clone()}
|
||||
hashset! {repo.view().checkout().clone(), commit_b2.id().clone()}
|
||||
);
|
||||
|
||||
tx.discard();
|
||||
@ -983,14 +984,10 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() {
|
||||
})
|
||||
);
|
||||
|
||||
// TODO: We should probably either hide B or indicate in the UI why it's still
|
||||
// visible. Alternatively, we could redefine the view's heads to be only the
|
||||
// desired anonymous heads.
|
||||
assert_eq!(
|
||||
*tx.mut_repo().view().heads(),
|
||||
hashset! {
|
||||
repo.view().checkout().clone(),
|
||||
commit_b.id().clone(),
|
||||
commit_b2.id().clone(),
|
||||
commit_b3.id().clone(),
|
||||
commit_b4.id().clone(),
|
||||
@ -1059,10 +1056,8 @@ fn test_rebase_descendants_rewrite_updates_branch_conflict() {
|
||||
*tx.mut_repo().view().heads(),
|
||||
hashset! {
|
||||
repo.view().checkout().clone(),
|
||||
commit_a.id().clone(),
|
||||
commit_a2.id().clone(),
|
||||
commit_a3.id().clone(),
|
||||
commit_b.id().clone(),
|
||||
commit_b2.id().clone(),
|
||||
commit_b3.id().clone(),
|
||||
commit_c.id().clone(),
|
||||
|
Loading…
Reference in New Issue
Block a user