just playing with tangled
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

local_working_copy: store materialized conflict marker length

Storing the conflict marker length in the working copy makes conflict
parsing more consistent, and it allows us to parse valid conflict hunks
even if the user left some invalid conflict markers in the file while
resolving the conflicts.

authored by

Scott Taylor and committed by
Scott Taylor
6baa4362 b11ce6bd

+409 -38
+2 -1
cli/src/commands/debug/local_working_copy.rs
··· 40 40 for (file, state) in wc.file_states()? { 41 41 writeln!( 42 42 ui.stdout(), 43 - "{:?} {:13?} {:10?} {:?}", 43 + "{:?} {:13?} {:10?} {:?} {:?}", 44 44 state.file_type, 45 45 state.size, 46 46 state.mtime.0, 47 + state.materialized_conflict_data, 47 48 file 48 49 )?; 49 50 }
+18 -2
cli/src/merge_tools/external.rs
··· 12 12 use jj_lib::backend::MergedTreeId; 13 13 use jj_lib::backend::TreeValue; 14 14 use jj_lib::conflicts; 15 - use jj_lib::conflicts::materialize_merge_result_to_bytes; 15 + use jj_lib::conflicts::choose_materialized_conflict_marker_len; 16 + use jj_lib::conflicts::materialize_merge_result_to_bytes_with_marker_len; 16 17 use jj_lib::conflicts::ConflictMarkerStyle; 18 + use jj_lib::conflicts::MIN_CONFLICT_MARKER_LEN; 17 19 use jj_lib::gitignore::GitIgnoreFile; 18 20 use jj_lib::matchers::Matcher; 19 21 use jj_lib::merge::Merge; ··· 181 183 .conflict_marker_style 182 184 .unwrap_or(default_conflict_marker_style); 183 185 186 + // If the merge tool doesn't get conflict markers pre-populated in the output 187 + // file, we should default to accepting MIN_CONFLICT_MARKER_LEN since the 188 + // merge tool is unlikely to know about our rules for conflict marker length. 189 + // In the future, we may want to add a "$markerLength" variable. 190 + let conflict_marker_len = if editor.merge_tool_edits_conflict_markers { 191 + choose_materialized_conflict_marker_len(&content) 192 + } else { 193 + MIN_CONFLICT_MARKER_LEN 194 + }; 184 195 let initial_output_content = if editor.merge_tool_edits_conflict_markers { 185 - materialize_merge_result_to_bytes(&content, conflict_marker_style) 196 + materialize_merge_result_to_bytes_with_marker_len( 197 + &content, 198 + conflict_marker_style, 199 + conflict_marker_len, 200 + ) 186 201 } else { 187 202 BString::default() 188 203 }; ··· 257 272 repo_path, 258 273 output_file_contents.as_slice(), 259 274 conflict_marker_style, 275 + conflict_marker_len, 260 276 ) 261 277 .block_on()? 262 278 } else {
+180
cli/tests/test_working_copy.rs
··· 13 13 // limitations under the License. 14 14 15 15 use indoc::indoc; 16 + use regex::Regex; 16 17 17 18 use crate::common::TestEnvironment; 18 19 ··· 262 263 2: invalid utf-8 sequence of 1 bytes from index 0 263 264 "##); 264 265 } 266 + 267 + #[test] 268 + fn test_conflict_marker_length_stored_in_working_copy() { 269 + let test_env = TestEnvironment::default(); 270 + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); 271 + let repo_path = test_env.env_root().join("repo"); 272 + 273 + // Create a conflict in the working copy with long markers on one side 274 + let conflict_file = repo_path.join("file"); 275 + std::fs::write( 276 + &conflict_file, 277 + indoc! {" 278 + line 1 279 + line 2 280 + line 3 281 + "}, 282 + ) 283 + .unwrap(); 284 + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "base"]); 285 + std::fs::write( 286 + &conflict_file, 287 + indoc! {" 288 + line 1 289 + line 2 - left 290 + line 3 - left 291 + "}, 292 + ) 293 + .unwrap(); 294 + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "side-a"]); 295 + test_env.jj_cmd_ok(&repo_path, &["new", "description(base)", "-m", "side-b"]); 296 + std::fs::write( 297 + &conflict_file, 298 + indoc! {" 299 + line 1 300 + ======= fake marker 301 + line 2 - right 302 + ======= fake marker 303 + line 3 304 + "}, 305 + ) 306 + .unwrap(); 307 + test_env.jj_cmd_ok( 308 + &repo_path, 309 + &["new", "description(side-a)", "description(side-b)"], 310 + ); 311 + 312 + // File should be materialized with long conflict markers 313 + insta::assert_snapshot!(std::fs::read_to_string(&conflict_file).unwrap(), @r##" 314 + line 1 315 + <<<<<<<<<<< Conflict 1 of 1 316 + %%%%%%%%%%% Changes from base to side #1 317 + -line 2 318 + -line 3 319 + +line 2 - left 320 + +line 3 - left 321 + +++++++++++ Contents of side #2 322 + ======= fake marker 323 + line 2 - right 324 + ======= fake marker 325 + line 3 326 + >>>>>>>>>>> Conflict 1 of 1 ends 327 + "##); 328 + 329 + // The timestamps in the `jj debug local-working-copy` output change, so we want 330 + // to remove them before asserting the snapshot 331 + let timestamp_regex = Regex::new(r"\b\d{10,}\b").unwrap(); 332 + // On Windows, executable is always `()`, but on Unix-like systems, it's `true` 333 + // or `false`, so we want to remove it from the output as well 334 + let executable_regex = Regex::new("executable: [^ ]+").unwrap(); 335 + 336 + let redact_output = |output: &str| { 337 + let output = timestamp_regex.replace_all(output, "<timestamp>"); 338 + let output = executable_regex.replace_all(&output, "<executable>"); 339 + output.into_owned() 340 + }; 341 + 342 + // Working copy should contain conflict marker length 343 + let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "local-working-copy"]); 344 + insta::assert_snapshot!(redact_output(&stdout), @r#" 345 + Current operation: OperationId("6feb53603f9f7324085d2d89dca19a6dac93fef6795cfd5d57090ff803d404ab1196b45d5b97faa641f6a78302ac0fbd149f5e5a880d1fd64d6520c31beab213") 346 + Current tree: Merge(Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("f56b8223da0dab22b03b8323ced4946329aeb4e0")])) 347 + Normal { <executable> } 249 <timestamp> Some(MaterializedConflictData { conflict_marker_len: 11 }) "file" 348 + "#); 349 + 350 + // Update the conflict with more fake markers, and it should still parse 351 + // correctly (the markers should be ignored) 352 + std::fs::write( 353 + &conflict_file, 354 + indoc! {" 355 + line 1 356 + <<<<<<<<<<< Conflict 1 of 1 357 + %%%%%%%%%%% Changes from base to side #1 358 + -line 2 359 + -line 3 360 + +line 2 - left 361 + +line 3 - left 362 + +++++++++++ Contents of side #2 363 + <<<<<<< fake marker 364 + ||||||| fake marker 365 + line 2 - right 366 + ======= fake marker 367 + line 3 368 + >>>>>>> fake marker 369 + >>>>>>>>>>> Conflict 1 of 1 ends 370 + "}, 371 + ) 372 + .unwrap(); 373 + 374 + // The file should still be conflicted, and the new content should be saved 375 + let stdout = test_env.jj_cmd_success(&repo_path, &["st"]); 376 + insta::assert_snapshot!(stdout, @r#" 377 + Working copy changes: 378 + M file 379 + There are unresolved conflicts at these paths: 380 + file 2-sided conflict 381 + Working copy : mzvwutvl 3a981880 (conflict) (no description set) 382 + Parent commit: rlvkpnrz ce613b49 side-a 383 + Parent commit: zsuskuln 7b2b03ab side-b 384 + "#); 385 + insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]), @r##" 386 + diff --git a/file b/file 387 + --- a/file 388 + +++ b/file 389 + @@ -6,8 +6,10 @@ 390 + +line 2 - left 391 + +line 3 - left 392 + +++++++++++ Contents of side #2 393 + -======= fake marker 394 + +<<<<<<< fake marker 395 + +||||||| fake marker 396 + line 2 - right 397 + ======= fake marker 398 + line 3 399 + +>>>>>>> fake marker 400 + >>>>>>>>>>> Conflict 1 of 1 ends 401 + "##); 402 + 403 + // Working copy should still contain conflict marker length 404 + let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "local-working-copy"]); 405 + insta::assert_snapshot!(redact_output(&stdout), @r#" 406 + Current operation: OperationId("205bc702428a522e0b175938a51c51b59741c854a609ba63c89de76ffda6e5eff6fcc00725328b1a91f448401769773cefcff01fac3448190d2cea4e137d2166") 407 + Current tree: Merge(Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("3329c18c95f7b7a55c278c2259e9c4ce711fae59")])) 408 + Normal { <executable> } 289 <timestamp> Some(MaterializedConflictData { conflict_marker_len: 11 }) "file" 409 + "#); 410 + 411 + // Resolve the conflict 412 + std::fs::write( 413 + &conflict_file, 414 + indoc! {" 415 + line 1 416 + <<<<<<< fake marker 417 + ||||||| fake marker 418 + line 2 - left 419 + line 2 - right 420 + ======= fake marker 421 + line 3 - left 422 + >>>>>>> fake marker 423 + "}, 424 + ) 425 + .unwrap(); 426 + 427 + let stdout = test_env.jj_cmd_success(&repo_path, &["st"]); 428 + insta::assert_snapshot!(stdout, @r#" 429 + Working copy changes: 430 + M file 431 + Working copy : mzvwutvl 1aefd866 (no description set) 432 + Parent commit: rlvkpnrz ce613b49 side-a 433 + Parent commit: zsuskuln 7b2b03ab side-b 434 + "#); 435 + 436 + // When the file is resolved, the conflict marker length is removed from the 437 + // working copy 438 + let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "local-working-copy"]); 439 + insta::assert_snapshot!(redact_output(&stdout), @r#" 440 + Current operation: OperationId("2206ce3c108b1573df0841138c226bba1ab3cff900a5899ed31ac69162c7d6f30d37fb5ab43da60dba88047b8ab22d453887fff688f26dfcf04f2c99420a5563") 441 + Current tree: Merge(Resolved(TreeId("6120567b3cb2472d549753ed3e4b84183d52a650"))) 442 + Normal { <executable> } 130 <timestamp> None "file" 443 + "#); 444 + }
+24 -2
lib/src/conflicts.rs
··· 344 344 } 345 345 } 346 346 347 - fn materialize_merge_result_with_marker_len<T: AsRef<[u8]>>( 347 + pub fn materialize_merge_result_with_marker_len<T: AsRef<[u8]>>( 348 348 single_hunk: &Merge<T>, 349 349 conflict_marker_style: ConflictMarkerStyle, 350 350 conflict_marker_len: usize, ··· 368 368 MergeResult::Resolved(content) => content, 369 369 MergeResult::Conflict(hunks) => { 370 370 let conflict_marker_len = choose_materialized_conflict_marker_len(single_hunk); 371 + let mut output = Vec::new(); 372 + materialize_conflict_hunks( 373 + &hunks, 374 + conflict_marker_style, 375 + conflict_marker_len, 376 + &mut output, 377 + ) 378 + .expect("writing to an in-memory buffer should never fail"); 379 + output.into() 380 + } 381 + } 382 + } 383 + 384 + pub fn materialize_merge_result_to_bytes_with_marker_len<T: AsRef<[u8]>>( 385 + single_hunk: &Merge<T>, 386 + conflict_marker_style: ConflictMarkerStyle, 387 + conflict_marker_len: usize, 388 + ) -> BString { 389 + let merge_result = files::merge(single_hunk); 390 + match merge_result { 391 + MergeResult::Resolved(content) => content, 392 + MergeResult::Conflict(hunks) => { 371 393 let mut output = Vec::new(); 372 394 materialize_conflict_hunks( 373 395 &hunks, ··· 824 846 path: &RepoPath, 825 847 content: &[u8], 826 848 conflict_marker_style: ConflictMarkerStyle, 849 + conflict_marker_len: usize, 827 850 ) -> BackendResult<Merge<Option<FileId>>> { 828 851 let simplified_file_ids = file_ids.clone().simplify(); 829 852 let simplified_file_ids = &simplified_file_ids; ··· 835 858 // copy. 836 859 let mut old_content = Vec::with_capacity(content.len()); 837 860 let merge_hunk = extract_as_single_hunk(simplified_file_ids, store, path).await?; 838 - let conflict_marker_len = choose_materialized_conflict_marker_len(&merge_hunk); 839 861 materialize_merge_result_with_marker_len( 840 862 &merge_hunk, 841 863 conflict_marker_style,
+87 -11
lib/src/local_working_copy.rs
··· 66 66 use crate::backend::TreeValue; 67 67 use crate::commit::Commit; 68 68 use crate::conflicts; 69 - use crate::conflicts::materialize_merge_result_to_bytes; 69 + use crate::conflicts::choose_materialized_conflict_marker_len; 70 + use crate::conflicts::materialize_merge_result_to_bytes_with_marker_len; 70 71 use crate::conflicts::materialize_tree_value; 71 72 use crate::conflicts::ConflictMarkerStyle; 72 73 use crate::conflicts::MaterializedTreeValue; 74 + use crate::conflicts::MIN_CONFLICT_MARKER_LEN; 73 75 use crate::file_util::check_symlink_support; 74 76 use crate::file_util::try_symlink; 75 77 #[cfg(feature = "watchman")] ··· 125 127 GitSubmodule, 126 128 } 127 129 130 + #[derive(Debug, PartialEq, Eq, Clone, Copy)] 131 + pub struct MaterializedConflictData { 132 + pub conflict_marker_len: u32, 133 + } 134 + 128 135 #[derive(Debug, PartialEq, Eq, Clone)] 129 136 pub struct FileState { 130 137 pub file_type: FileType, 131 138 pub mtime: MillisSinceEpoch, 132 139 pub size: u64, 140 + pub materialized_conflict_data: Option<MaterializedConflictData>, 133 141 /* TODO: What else do we need here? Git stores a lot of fields. 134 142 * TODO: Could possibly handle case-insensitive file systems keeping an 135 143 * Option<PathBuf> with the actual path here. */ 136 144 } 137 145 138 146 impl FileState { 147 + /// Check whether a file state appears clean compared to a previous file 148 + /// state, ignoring materialized conflict data. 149 + pub fn is_clean(&self, old_file_state: &Self) -> bool { 150 + self.file_type == old_file_state.file_type 151 + && self.mtime == old_file_state.mtime 152 + && self.size == old_file_state.size 153 + } 154 + 139 155 /// Indicates that a file exists in the tree but that it needs to be 140 156 /// re-stat'ed on the next snapshot. 141 157 fn placeholder() -> Self { ··· 147 163 file_type: FileType::Normal { executable }, 148 164 mtime: MillisSinceEpoch(0), 149 165 size: 0, 166 + materialized_conflict_data: None, 150 167 } 151 168 } 152 169 153 - fn for_file(executable: bool, size: u64, metadata: &Metadata) -> Self { 170 + fn for_file( 171 + executable: bool, 172 + size: u64, 173 + metadata: &Metadata, 174 + materialized_conflict_data: Option<MaterializedConflictData>, 175 + ) -> Self { 154 176 #[cfg(windows)] 155 177 let executable = { 156 178 // Windows doesn't support executable bit. ··· 160 182 file_type: FileType::Normal { executable }, 161 183 mtime: mtime_from_metadata(metadata), 162 184 size, 185 + materialized_conflict_data, 163 186 } 164 187 } 165 188 ··· 171 194 file_type: FileType::Symlink, 172 195 mtime: mtime_from_metadata(metadata), 173 196 size: metadata.len(), 197 + materialized_conflict_data: None, 174 198 } 175 199 } 176 200 ··· 179 203 file_type: FileType::GitSubmodule, 180 204 mtime: MillisSinceEpoch(0), 181 205 size: 0, 206 + materialized_conflict_data: None, 182 207 } 183 208 } 184 209 } ··· 418 443 file_type, 419 444 mtime: MillisSinceEpoch(proto.mtime_millis_since_epoch), 420 445 size: proto.size, 446 + materialized_conflict_data: proto.materialized_conflict_data.as_ref().map(|data| { 447 + MaterializedConflictData { 448 + conflict_marker_len: data.conflict_marker_len, 449 + } 450 + }), 421 451 } 422 452 } 423 453 ··· 436 466 proto.file_type = file_type as i32; 437 467 proto.mtime_millis_since_epoch = file_state.mtime.0; 438 468 proto.size = file_state.size; 469 + proto.materialized_conflict_data = file_state.materialized_conflict_data.map(|data| { 470 + crate::protos::working_copy::MaterializedConflictData { 471 + conflict_marker_len: data.conflict_marker_len, 472 + } 473 + }); 439 474 proto 440 475 } 441 476 ··· 676 711 file_type, 677 712 mtime, 678 713 size, 714 + materialized_conflict_data: None, 679 715 } 680 716 }) 681 717 } ··· 1305 1341 path: RepoPathBuf, 1306 1342 disk_path: &Path, 1307 1343 maybe_current_file_state: Option<&FileState>, 1308 - new_file_state: FileState, 1344 + mut new_file_state: FileState, 1309 1345 ) -> Result<(), SnapshotError> { 1310 1346 let update = self.get_updated_tree_value( 1311 1347 &path, ··· 1313 1349 maybe_current_file_state, 1314 1350 &new_file_state, 1315 1351 )?; 1352 + // Preserve materialized conflict data for normal, non-resolved files 1353 + if matches!(new_file_state.file_type, FileType::Normal { .. }) 1354 + && !update.as_ref().is_some_and(|update| update.is_resolved()) 1355 + { 1356 + new_file_state.materialized_conflict_data = 1357 + maybe_current_file_state.and_then(|state| state.materialized_conflict_data); 1358 + } 1316 1359 if let Some(tree_value) = update { 1317 1360 self.tree_entries_tx.send((path.clone(), tree_value)).ok(); 1318 1361 } ··· 1370 1413 Some(current_file_state) => { 1371 1414 // If the file's mtime was set at the same time as this state file's own mtime, 1372 1415 // then we don't know if the file was modified before or after this state file. 1373 - current_file_state == new_file_state 1416 + new_file_state.is_clean(current_file_state) 1374 1417 && current_file_state.mtime < self.tree_state.own_mtime 1375 1418 } 1376 1419 }; ··· 1391 1434 }; 1392 1435 let new_tree_values = match new_file_type { 1393 1436 FileType::Normal { executable } => self 1394 - .write_path_to_store(repo_path, disk_path, &current_tree_values, executable) 1437 + .write_path_to_store( 1438 + repo_path, 1439 + disk_path, 1440 + &current_tree_values, 1441 + executable, 1442 + maybe_current_file_state.and_then(|state| state.materialized_conflict_data), 1443 + ) 1395 1444 .block_on()?, 1396 1445 FileType::Symlink => { 1397 1446 let id = self ··· 1419 1468 disk_path: &Path, 1420 1469 current_tree_values: &MergedTreeValue, 1421 1470 executable: FileExecutableFlag, 1471 + materialized_conflict_data: Option<MaterializedConflictData>, 1422 1472 ) -> Result<MergedTreeValue, SnapshotError> { 1423 1473 if let Some(current_tree_value) = current_tree_values.as_resolved() { 1424 1474 #[cfg(unix)] ··· 1449 1499 repo_path, 1450 1500 &content, 1451 1501 self.conflict_marker_style, 1502 + materialized_conflict_data.map_or(MIN_CONFLICT_MARKER_LEN, |data| { 1503 + data.conflict_marker_len as usize 1504 + }), 1452 1505 ) 1453 1506 .block_on()?; 1454 1507 match new_file_ids.into_resolved() { ··· 1552 1605 let metadata = file 1553 1606 .metadata() 1554 1607 .map_err(|err| checkout_error_for_stat_error(err, disk_path))?; 1555 - Ok(FileState::for_file(executable, size, &metadata)) 1608 + Ok(FileState::for_file(executable, size, &metadata, None)) 1556 1609 } 1557 1610 1558 1611 fn write_symlink(&self, disk_path: &Path, target: String) -> Result<FileState, CheckoutError> { ··· 1576 1629 disk_path: &Path, 1577 1630 conflict_data: Vec<u8>, 1578 1631 executable: bool, 1632 + materialized_conflict_data: Option<MaterializedConflictData>, 1579 1633 ) -> Result<FileState, CheckoutError> { 1580 1634 let mut file = OpenOptions::new() 1581 1635 .write(true) ··· 1595 1649 let metadata = file 1596 1650 .metadata() 1597 1651 .map_err(|err| checkout_error_for_stat_error(err, disk_path))?; 1598 - Ok(FileState::for_file(executable, size, &metadata)) 1652 + Ok(FileState::for_file( 1653 + executable, 1654 + size, 1655 + &metadata, 1656 + materialized_conflict_data, 1657 + )) 1599 1658 } 1600 1659 1601 1660 #[cfg_attr(windows, allow(unused_variables))] ··· 1786 1845 contents, 1787 1846 executable, 1788 1847 } => { 1789 - let data = 1790 - materialize_merge_result_to_bytes(&contents, conflict_marker_style).into(); 1791 - self.write_conflict(&disk_path, data, executable)? 1848 + let conflict_marker_len = choose_materialized_conflict_marker_len(&contents); 1849 + let data = materialize_merge_result_to_bytes_with_marker_len( 1850 + &contents, 1851 + conflict_marker_style, 1852 + conflict_marker_len, 1853 + ) 1854 + .into(); 1855 + let materialized_conflict_data = MaterializedConflictData { 1856 + conflict_marker_len: conflict_marker_len.try_into().unwrap_or(u32::MAX), 1857 + }; 1858 + self.write_conflict( 1859 + &disk_path, 1860 + data, 1861 + executable, 1862 + Some(materialized_conflict_data), 1863 + )? 1792 1864 } 1793 1865 MaterializedTreeValue::OtherConflict { id } => { 1794 1866 // Unless all terms are regular files, we can't do much 1795 1867 // better than trying to describe the merge. 1796 1868 let data = id.describe().into_bytes(); 1797 1869 let executable = false; 1798 - self.write_conflict(&disk_path, data, executable)? 1870 + self.write_conflict(&disk_path, data, executable, None)? 1799 1871 } 1800 1872 }; 1801 1873 changed_file_states.push((path, file_state)); ··· 1851 1923 file_type, 1852 1924 mtime: MillisSinceEpoch(0), 1853 1925 size: 0, 1926 + materialized_conflict_data: None, 1854 1927 }; 1855 1928 changed_file_states.push((path, file_state)); 1856 1929 } ··· 2310 2383 }, 2311 2384 mtime: MillisSinceEpoch(0), 2312 2385 size, 2386 + materialized_conflict_data: None, 2313 2387 }; 2314 2388 let new_static_entry = |path: &'static str, size| (repo_path(path), new_state(size)); 2315 2389 let new_owned_entry = |path: &str, size| (repo_path(path).to_owned(), new_state(size)); ··· 2358 2432 }, 2359 2433 mtime: MillisSinceEpoch(0), 2360 2434 size, 2435 + materialized_conflict_data: None, 2361 2436 }; 2362 2437 let new_proto_entry = |path: &str, size| { 2363 2438 file_state_entry_to_proto(repo_path(path).to_owned(), &new_state(size)) ··· 2422 2497 }, 2423 2498 mtime: MillisSinceEpoch(0), 2424 2499 size, 2500 + materialized_conflict_data: None, 2425 2501 }; 2426 2502 let new_proto_entry = |path: &str, size| { 2427 2503 file_state_entry_to_proto(repo_path(path).to_owned(), &new_state(size))
+6
lib/src/protos/working_copy.proto
··· 24 24 GitSubmodule = 4; 25 25 } 26 26 27 + message MaterializedConflictData { 28 + // TODO: maybe we should store num_sides here as well 29 + uint32 conflict_marker_len = 1; 30 + } 31 + 27 32 message FileState { 28 33 int64 mtime_millis_since_epoch = 1; 29 34 uint64 size = 2; 30 35 FileType file_type = 3; 31 36 // Set only if file_type is Conflict 32 37 bytes conflict_id = 4 [deprecated = true]; 38 + MaterializedConflictData materialized_conflict_data = 5; 33 39 } 34 40 35 41 message FileStateEntry {
+9
lib/src/protos/working_copy.rs
··· 1 1 // This file is @generated by prost-build. 2 2 #[allow(clippy::derive_partial_eq_without_eq)] 3 3 #[derive(Clone, PartialEq, ::prost::Message)] 4 + pub struct MaterializedConflictData { 5 + /// TODO: maybe we should store num_sides here as well 6 + #[prost(uint32, tag = "1")] 7 + pub conflict_marker_len: u32, 8 + } 9 + #[allow(clippy::derive_partial_eq_without_eq)] 10 + #[derive(Clone, PartialEq, ::prost::Message)] 4 11 pub struct FileState { 5 12 #[prost(int64, tag = "1")] 6 13 pub mtime_millis_since_epoch: i64, ··· 12 19 #[deprecated] 13 20 #[prost(bytes = "vec", tag = "4")] 14 21 pub conflict_id: ::prost::alloc::vec::Vec<u8>, 22 + #[prost(message, optional, tag = "5")] 23 + pub materialized_conflict_data: ::core::option::Option<MaterializedConflictData>, 15 24 } 16 25 #[allow(clippy::derive_partial_eq_without_eq)] 17 26 #[derive(Clone, PartialEq, ::prost::Message)]
+83 -22
lib/tests/test_conflicts.rs
··· 15 15 use indoc::indoc; 16 16 use itertools::Itertools; 17 17 use jj_lib::backend::FileId; 18 + use jj_lib::conflicts::choose_materialized_conflict_marker_len; 18 19 use jj_lib::conflicts::extract_as_single_hunk; 19 20 use jj_lib::conflicts::materialize_merge_result_to_bytes; 20 21 use jj_lib::conflicts::parse_conflict; ··· 590 591 for materialize_style in all_styles { 591 592 let materialized = materialize_conflict_string(store, path, &conflict, materialize_style); 592 593 for parse_style in all_styles { 593 - let parsed = 594 - update_from_content(&conflict, store, path, materialized.as_bytes(), parse_style) 595 - .block_on() 596 - .unwrap(); 594 + let parsed = update_from_content( 595 + &conflict, 596 + store, 597 + path, 598 + materialized.as_bytes(), 599 + parse_style, 600 + MIN_CONFLICT_MARKER_LEN, 601 + ) 602 + .block_on() 603 + .unwrap(); 597 604 598 605 assert_eq!( 599 606 parsed, conflict, ··· 1630 1637 let materialized = 1631 1638 materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); 1632 1639 let parse = |content| { 1633 - update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) 1634 - .block_on() 1635 - .unwrap() 1640 + update_from_content( 1641 + &conflict, 1642 + store, 1643 + path, 1644 + content, 1645 + ConflictMarkerStyle::Diff, 1646 + MIN_CONFLICT_MARKER_LEN, 1647 + ) 1648 + .block_on() 1649 + .unwrap() 1636 1650 }; 1637 1651 assert_eq!(parse(materialized.as_bytes()), conflict); 1638 1652 ··· 1680 1694 let materialized = 1681 1695 materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); 1682 1696 let parse = |content| { 1683 - update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) 1684 - .block_on() 1685 - .unwrap() 1697 + update_from_content( 1698 + &conflict, 1699 + store, 1700 + path, 1701 + content, 1702 + ConflictMarkerStyle::Diff, 1703 + MIN_CONFLICT_MARKER_LEN, 1704 + ) 1705 + .block_on() 1706 + .unwrap() 1686 1707 }; 1687 1708 assert_eq!(parse(materialized.as_bytes()), conflict); 1688 1709 ··· 1736 1757 let materialized_simplified = 1737 1758 materialize_conflict_string(store, path, &simplified_conflict, ConflictMarkerStyle::Diff); 1738 1759 let parse = |content| { 1739 - update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) 1740 - .block_on() 1741 - .unwrap() 1760 + update_from_content( 1761 + &conflict, 1762 + store, 1763 + path, 1764 + content, 1765 + ConflictMarkerStyle::Diff, 1766 + MIN_CONFLICT_MARKER_LEN, 1767 + ) 1768 + .block_on() 1769 + .unwrap() 1742 1770 }; 1743 1771 insta::assert_snapshot!( 1744 1772 materialized, ··· 1896 1924 ); 1897 1925 1898 1926 // The conflict should be materialized using long conflict markers 1927 + let materialized_marker_len = choose_materialized_conflict_marker_len( 1928 + &extract_as_single_hunk(&conflict, store, path) 1929 + .block_on() 1930 + .unwrap(), 1931 + ); 1932 + assert!(materialized_marker_len > MIN_CONFLICT_MARKER_LEN); 1899 1933 let materialized = 1900 1934 materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Snapshot); 1901 1935 insta::assert_snapshot!(materialized, @r##" ··· 1923 1957 // to avoid the two versions of the file being obviously identical, so that we 1924 1958 // can test the actual parsing logic. 1925 1959 let parse = |conflict, content| { 1926 - update_from_content(conflict, store, path, content, ConflictMarkerStyle::Diff) 1927 - .block_on() 1928 - .unwrap() 1960 + update_from_content( 1961 + conflict, 1962 + store, 1963 + path, 1964 + content, 1965 + ConflictMarkerStyle::Diff, 1966 + materialized_marker_len, 1967 + ) 1968 + .block_on() 1969 + .unwrap() 1929 1970 }; 1930 1971 assert_eq!(parse(&conflict, materialized.as_bytes()), conflict); 1931 1972 ··· 1999 2040 new_conflict 2000 2041 ); 2001 2042 2043 + // If we add back the second conflict, it should still be parsed correctly 2044 + // (the fake conflict markers shouldn't be interpreted as conflict markers 2045 + // still, since they aren't the longest ones in the file). 2046 + assert_eq!(parse(&new_conflict, materialized.as_bytes()), conflict); 2047 + 2002 2048 // If the new conflict is materialized again, it should have shorter 2003 2049 // conflict markers now 2004 2050 insta::assert_snapshot!( ··· 2062 2108 vec![Some(left_file_id.clone()), Some(right_file_id.clone())], 2063 2109 ); 2064 2110 2111 + // The conflict should be materialized with normal markers 2112 + let materialized_marker_len = choose_materialized_conflict_marker_len( 2113 + &extract_as_single_hunk(&conflict, store, path) 2114 + .block_on() 2115 + .unwrap(), 2116 + ); 2117 + assert!(materialized_marker_len == MIN_CONFLICT_MARKER_LEN); 2118 + 2065 2119 let materialized = 2066 2120 materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); 2067 2121 insta::assert_snapshot!(materialized, @r##" ··· 2086 2140 ); 2087 2141 2088 2142 let parse = |conflict, content| { 2089 - update_from_content(conflict, store, path, content, ConflictMarkerStyle::Diff) 2090 - .block_on() 2091 - .unwrap() 2143 + update_from_content( 2144 + conflict, 2145 + store, 2146 + path, 2147 + content, 2148 + ConflictMarkerStyle::Diff, 2149 + materialized_marker_len, 2150 + ) 2151 + .block_on() 2152 + .unwrap() 2092 2153 }; 2093 2154 assert_eq!(parse(&conflict, materialized.as_bytes()), conflict); 2094 2155 ··· 2164 2225 line 5 2165 2226 "##); 2166 2227 2167 - // Snapshotting again will cause the conflict to appear resolved 2228 + // Even though the file now contains markers of length 7, the materialized 2229 + // markers of length 7 are still parsed 2168 2230 let second_snapshot = parse(&new_conflict, new_conflict_contents.as_bytes()); 2169 - assert_ne!(second_snapshot, new_conflict); 2170 - assert!(second_snapshot.is_resolved()); 2231 + assert_eq!(second_snapshot, new_conflict); 2171 2232 } 2172 2233 2173 2234 fn materialize_conflict_string(