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

git: on abandoning unreachable commits, don't count HEAD ref #4143

Merged
merged 1 commit into from
Jul 24, 2024
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
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