From 759ddd1e60105212c5175ba113d08fbfab4400b8 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 31 Oct 2022 09:48:16 -0700 Subject: [PATCH] git: on initial export, export all branches As I said in the previous patch, I don't know why I made the initial export to Git a no-op. Exporting everything makes more sense to (current-)me. It will make it slightly easier to skip exporting conflicted branches (#463). It also lets us remove a `jj export` call from `test_templater.rs`. --- lib/src/git.rs | 17 ++++++++++------- lib/tests/test_git.rs | 7 ++++--- tests/test_templater.rs | 10 ++++------ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index c41db5544..5dc03f7ed 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -245,13 +245,16 @@ pub fn export_refs( upgrade_old_export_state(repo); let last_export_path = repo.repo_path().join("git_export_view"); - if let Ok(mut last_export_file) = OpenOptions::new().read(true).open(&last_export_path) { - let thrift_view = simple_op_store::read_thrift(&mut last_export_file) - .map_err(|err| GitExportError::ReadStateError(err.to_string()))?; - let last_export_store_view = op_store::View::from(&thrift_view); - let last_export_view = View::new(last_export_store_view); - export_changes(&last_export_view, repo.view(), git_repo)?; - } + let last_export_store_view = + if let Ok(mut last_export_file) = OpenOptions::new().read(true).open(&last_export_path) { + let thrift_view = simple_op_store::read_thrift(&mut last_export_file) + .map_err(|err| GitExportError::ReadStateError(err.to_string()))?; + op_store::View::from(&thrift_view) + } else { + op_store::View::default() + }; + let last_export_view = View::new(last_export_store_view); + export_changes(&last_export_view, repo.view(), git_repo)?; if let Ok(mut last_export_file) = OpenOptions::new() .write(true) .create(true) diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 2396ea1c6..2ac249567 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -403,8 +403,9 @@ fn test_import_refs_detached_head() { } #[test] -fn test_export_refs_initial() { - // The first export doesn't do anything +fn test_export_refs_no_detach() { + // When exporting the branch that's current checked out, don't detach HEAD if + // the target already matches let mut test_data = GitRepoData::create(); let git_repo = test_data.git_repo; let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]); @@ -416,7 +417,7 @@ fn test_export_refs_initial() { .unwrap(); test_data.repo = tx.commit(); - // The first export shouldn't do anything + // Do an initial export to make sure `main` is considered assert_eq!(git::export_refs(&test_data.repo, &git_repo), Ok(())); assert_eq!(git_repo.head().unwrap().name(), Some("refs/heads/main")); assert_eq!( diff --git a/tests/test_templater.rs b/tests/test_templater.rs index 9cf13c57a..9d8bcbf67 100644 --- a/tests/test_templater.rs +++ b/tests/test_templater.rs @@ -27,8 +27,6 @@ fn test_templater_branches() { .join("repo") .join("store") .join("git"); - // TODO: This initial export shouldn't be needed - test_env.jj_cmd_success(&origin_path, &["git", "export"]); // Created some branches on the remote test_env.jj_cmd_success(&origin_path, &["describe", "-m=description 1"]); @@ -68,12 +66,12 @@ fn test_templater_branches() { &["log", "-T", r#"commit_id.short() " " branches"#], ); insta::assert_snapshot!(output, @r###" - o 212985c08a44 branch3? - | @ cbf02da4e154 branch2* new-branch - | | o c794a4eab3b9 branch1* + o 48e0b6c42296 branch3? + | @ 092b2e0283a9 branch2* new-branch + | | o f4a739b1677f branch1* | |/ |/| - | o 8cd8e5dc9595 branch2@origin + | o 752dad8b1718 branch2@origin |/ o 000000000000 "###);