Skip to content

Commit

Permalink
git: on abandoning unreachable commits, don't count HEAD ref
Browse files Browse the repository at this point in the history
This basically reverts 20eb9ec "git: don't abandon HEAD commit when it
loses a branch." I think the new behavior is more consistent because the Git
HEAD is equivalent to @- in jj, so it shouldn't be considered a named ref.

Note that we've made old HEAD branch not considered at 92cfffd "git: on
external HEAD move, do not abandon old branch."

#4108
  • Loading branch information
yuja committed Jul 24, 2024
1 parent eb332b1 commit bafb357
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* [The "fileset" language](docs/filesets.md) is now enabled by default. It can
still be disable by setting `ui.allow-filesets=false`.

* On `jj git fetch`/`import`, commits referred to by `HEAD@git` are no longer
preserved. If a checked-out named branch gets deleted locally or remotely, the
corresponding commits will be abandoned.

### Deprecations

### New features
Expand Down
12 changes: 7 additions & 5 deletions cli/tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ fn test_git_colocated_rebase_on_import() {
// refs/heads/master we just exported
test_env.jj_cmd_ok(&workspace_root, &["st"]);

// Move `master` and HEAD backwards, which should result in commit2 getting
// hidden, and a new working-copy commit at the new position.
// Move `master` backwards, which should result in commit2 getting hidden,
// and the working-copy commit rebased.
let commit2_oid = git_repo
.find_branch("master", git2::BranchType::Local)
.unwrap()
Expand All @@ -292,16 +292,18 @@ fn test_git_colocated_rebase_on_import() {
let commit2 = git_repo.find_commit(commit2_oid).unwrap();
let commit1 = commit2.parents().next().unwrap();
git_repo.branch("master", &commit1, true).unwrap();
git_repo.set_head("refs/heads/master").unwrap();
let (stdout, stderr) = get_log_output_with_stderr(&test_env, &workspace_root);
insta::assert_snapshot!(stdout, @r###"
@ 5539e55eb3690b85a7ebd4a37a5e3b57f469ee94
@ 15b1d70c5e33b5d2b18383292b85324d5153ffed
○ 47fe984daf66f7bf3ebf31b9cb3513c995afb857 master HEAD@git add a file
◆ 0000000000000000000000000000000000000000
"###);
insta::assert_snapshot!(stderr, @r###"
Reset the working copy parent to the new Git HEAD.
Abandoned 1 commits that are no longer reachable.
Rebased 1 descendant commits off of commits rewritten from git
Working copy now at: zsuskuln 15b1d70c (empty) (no description set)
Parent commit : qpvuntsm 47fe984d master | add a file
Added 0 files, modified 1 files, removed 0 files
Done importing changes from the underlying Git repo.
"###);
}
Expand Down
7 changes: 3 additions & 4 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
use std::default::Default;
use std::io::Read;
use std::path::PathBuf;
use std::{fmt, iter, str};
use std::{fmt, str};

use git2::Oid;
use itertools::Itertools;
Expand Down Expand Up @@ -496,16 +496,15 @@ fn default_remote_ref_state_for(ref_name: &RefName, git_settings: &GitSettings)
}
}

/// Commits referenced by local branches, tags, or HEAD@git.
/// Commits referenced by local branches or tags.
///
/// On `import_refs()`, this is similar to collecting commits referenced by
/// `view.git_refs()`. Main difference is that local branches can be moved by
/// tracking remotes, and such mutation isn't applied to `view.git_refs()` yet.
fn pinned_commit_ids(view: &View) -> Vec<CommitId> {
itertools::chain!(
itertools::chain(
view.local_branches().map(|(_, target)| target),
view.tags().values(),
iter::once(view.git_head()),
)
.flat_map(|target| target.added_ids())
.cloned()
Expand Down
8 changes: 4 additions & 4 deletions lib/tests/test_git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ fn test_import_refs_reimport_head_removed() {
}

#[test]
fn test_import_refs_reimport_git_head_counts() {
// Test that if a branch is removed but the Git HEAD points to the commit (or a
// descendant of it), we still keep it alive.
fn test_import_refs_reimport_git_head_does_not_count() {
// Test that if a branch is removed, the corresponding commit is abandoned
// no matter if the Git HEAD points to the commit (or a descendant of it.)
let settings = testutils::user_settings();
let git_settings = GitSettings::default();
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
Expand All @@ -379,7 +379,7 @@ fn test_import_refs_reimport_git_head_counts() {
git::import_head(tx.mut_repo()).unwrap();
git::import_refs(tx.mut_repo(), &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit)));
assert!(!tx.mut_repo().view().heads().contains(&jj_id(&commit)));
}

#[test]
Expand Down

0 comments on commit bafb357

Please sign in to comment.