From 326049150cf57477817b2ca069de9b0ac12e11ee Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 4 Nov 2023 22:25:50 -0700 Subject: [PATCH 1/2] git: extract a function for abandoning unreachable commits This motivation for this is so we can easily skip calling the function if the user has opted out of the propagation of abandoned commits we usually do (#2504). However, it seems like a good piece of code to extract regardless of that feature. --- lib/src/git.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 6e4a85c5ab..2bca1c9a95 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -313,18 +313,24 @@ pub fn import_some_refs( } } - // Find commits that are no longer referenced in the git repo and abandon them - // in jj as well. + let abandoned_commits = abandon_unreachable_commits(mut_repo, &changed_remote_refs); + let stats = GitImportStats { abandoned_commits }; + Ok(stats) +} + +/// Finds commits that used to be reachable in git that no longer are reachable. +/// Those commits will be recorded as abandoned in the `MutableRepo`. +fn abandon_unreachable_commits( + mut_repo: &mut MutableRepo, + changed_remote_refs: &BTreeMap, +) -> Vec { let hidable_git_heads = changed_remote_refs .values() .flat_map(|(old_remote_ref, _)| old_remote_ref.target.added_ids()) .cloned() .collect_vec(); if hidable_git_heads.is_empty() { - let stats = GitImportStats { - abandoned_commits: vec![], - }; - return Ok(stats); + return vec![]; } let pinned_heads = itertools::chain!( changed_remote_refs @@ -335,10 +341,6 @@ pub fn import_some_refs( ) .cloned() .collect_vec(); - // We could use mut_repo.record_rewrites() here but we know we only need to care - // about abandoned commits for now. We may want to change this if we ever - // add a way of preserving change IDs across rewrites by `git` (e.g. by - // putting them in the commit message). let abandoned_expression = RevsetExpression::commits(pinned_heads) .range(&RevsetExpression::commits(hidable_git_heads)) .intersection(&RevsetExpression::visible_heads().ancestors()); @@ -352,9 +354,7 @@ pub fn import_some_refs( for abandoned_commit in &abandoned_commits { mut_repo.record_abandoned_commit(abandoned_commit.clone()); } - - let stats = GitImportStats { abandoned_commits }; - Ok(stats) + abandoned_commits } /// Calculates diff of git refs to be imported. From a0dbfb8f82ac5b66443b352b375c0eb5c6d788c1 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 4 Nov 2023 22:15:41 -0700 Subject: [PATCH 2/2] git: add config to disable abandoning of unreachable commits Some users prefer to have commits not get abandoned when importing refs. This adds a config option for that. Closes #2504. --- CHANGELOG.md | 4 ++++ cli/src/config-schema.json | 5 +++++ docs/config.md | 14 ++++++++++++++ lib/src/git.rs | 6 +++++- lib/src/settings.rs | 5 +++++ lib/tests/test_git.rs | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 66 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96f647b8db..5ed154d005 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 create a new workspace with its working-copy commit on top of all the parents, as if you had run `jj new r1 r2 r3 ...`. +* You can now set `git.abandon-unreachable-commits = false` to disable the + usual behavior where commits that became unreachable in the Git repo are + abandoned ([#2504](https://github.com/martinvonz/jj/pull/2504)). + ### Fixed bugs diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 8c30f64bc3..14e71edf97 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -225,6 +225,11 @@ "description": "Whether jj creates a local branch with the same name when it imports a remote-tracking branch from git. See https://github.com/martinvonz/jj/blob/main/docs/config.md#automatic-local-branch-creation", "default": true }, + "abandon-unreachable-commits": { + "type": "boolean", + "description": "Whether jj should abandon commits that became unreachable in Git.", + "default": true + }, "push-branch-prefix": { "type": "string", "description": "Prefix used when pushing a change ID as a new branch", diff --git a/docs/config.md b/docs/config.md index 8615df90c0..ebd3fd6cc4 100644 --- a/docs/config.md +++ b/docs/config.md @@ -525,6 +525,20 @@ jj branch delete gh-pages jj branch untrack gh-pages@upstream ``` +### Abandon commits that became unreachable in Git + +By default, when `jj` imports refs from Git, it will look for commits that used +to be [reachable][reachable] but no longer are reachable. Those commits will +then be abandoned, and any descendant commits will be rebased off of them (as +usual when commits are abandoned). You can disable this behavior and instead +leave the Git-unreachable commits in your repo by setting: + +```toml +git.abandon-unreachable-commits = false +``` + +[reachable]: https://git-scm.com/docs/gitglossary/#Documentation/gitglossary.txt-aiddefreachableareachable + ### Prefix for generated branches on push `jj git push --change` generates branch names with a prefix of "push-" by diff --git a/lib/src/git.rs b/lib/src/git.rs index 2bca1c9a95..bd3a69700a 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -313,7 +313,11 @@ pub fn import_some_refs( } } - let abandoned_commits = abandon_unreachable_commits(mut_repo, &changed_remote_refs); + let abandoned_commits = if git_settings.abandon_unreachable_commits { + abandon_unreachable_commits(mut_repo, &changed_remote_refs) + } else { + vec![] + }; let stats = GitImportStats { abandoned_commits }; Ok(stats) } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index b60f657548..a56bc83f9a 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -40,12 +40,16 @@ pub struct RepoSettings { #[derive(Debug, Clone)] pub struct GitSettings { pub auto_local_branch: bool, + pub abandon_unreachable_commits: bool, } impl GitSettings { pub fn from_config(config: &config::Config) -> Self { GitSettings { auto_local_branch: config.get_bool("git.auto-local-branch").unwrap_or(true), + abandon_unreachable_commits: config + .get_bool("git.abandon-unreachable-commits") + .unwrap_or(true), } } } @@ -54,6 +58,7 @@ impl Default for GitSettings { fn default() -> Self { GitSettings { auto_local_branch: true, + abandon_unreachable_commits: true, } } } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 1e9cd80eae..23d00d944f 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -708,6 +708,7 @@ fn test_import_refs_reimport_with_moved_untracked_remote_ref() { let settings = testutils::user_settings(); let git_settings = GitSettings { auto_local_branch: false, + ..Default::default() }; let test_workspace = TestRepo::init_with_backend(TestRepoBackend::Git); let repo = &test_workspace.repo; @@ -819,6 +820,37 @@ fn test_import_refs_reimport_all_from_root_removed() { assert!(!tx.mut_repo().view().heads().contains(&jj_id(&commit))); } +#[test] +fn test_import_refs_reimport_abandoning_disabled() { + // Test that we don't abandoned unreachable commits if configured not to + let settings = testutils::user_settings(); + let git_settings = GitSettings { + abandon_unreachable_commits: false, + ..Default::default() + }; + let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git); + let repo = &test_repo.repo; + let git_repo = get_git_repo(repo); + + let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]); + let commit2 = empty_git_commit(&git_repo, "refs/heads/delete-me", &[&commit1]); + let mut tx = repo.start_transaction(&settings, "test"); + git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + // Test the setup + assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit2))); + + // Remove the `delete-me` branch and re-import + git_repo + .find_reference("refs/heads/delete-me") + .unwrap() + .delete() + .unwrap(); + git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit2))); +} + #[test] fn test_import_refs_reimport_conflicted_remote_branch() { let settings = testutils::user_settings(); @@ -1426,6 +1458,7 @@ fn test_import_export_non_tracking_branch() { let test_data = GitRepoData::create(); let mut git_settings = GitSettings { auto_local_branch: false, + ..Default::default() }; let git_repo = test_data.git_repo; let commit_main_t0 = empty_git_commit(&git_repo, "refs/remotes/origin/main", &[]);