Skip to content

Commit d7dd75a

Browse files
committed
Add a dry_run mode to LockedWorkingCopy snapshot method.
At Google, we are implementing a tool that needs to know the state of the jj repo constantly. Always snapshotting and updating .jj/working_copy/checkout increases the likelihood of write races. To fix this, I'd like to add a --dry-run flag to SnapshotOptions to make it not update the working copy. This is similar to the request in jj-vcs#2562 Implement
1 parent fae66ac commit d7dd75a

File tree

5 files changed

+59
-7
lines changed

5 files changed

+59
-7
lines changed

cli/src/cli_util.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,7 @@ to the current parents may contain changes from multiple commits.
14091409
start_tracking_matcher,
14101410
max_new_file_size,
14111411
conflict_marker_style,
1412+
dry_run: false,
14121413
})
14131414
}
14141415

cli/src/merge_tools/diff_working_copies.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ diff editing in mind and be a little inaccurate.
299299
start_tracking_matcher: &EverythingMatcher,
300300
max_new_file_size: u64::MAX,
301301
conflict_marker_style,
302+
dry_run: false,
302303
})?;
303304
Ok(output_tree_state.current_tree_id().clone())
304305
}

lib/src/local_working_copy.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -966,8 +966,8 @@ impl TreeState {
966966
start_tracking_matcher,
967967
max_new_file_size,
968968
conflict_marker_style,
969+
dry_run,
969970
} = options;
970-
971971
let sparse_matcher = self.sparse_matcher();
972972

973973
let fsmonitor_clock_needs_save = *fsmonitor_settings != FsmonitorSettings::None;
@@ -985,6 +985,15 @@ impl TreeState {
985985
if matcher.visit(RepoPath::root()).is_nothing() {
986986
// No need to load the current tree, set up channels, etc.
987987
self.watchman_clock = watchman_clock;
988+
if dry_run {
989+
return Ok((
990+
false,
991+
SnapshotStats {
992+
dry_run_tree_id: Some(self.tree_id.clone()),
993+
..SnapshotStats::default()
994+
},
995+
));
996+
}
988997
return Ok((is_dirty, SnapshotStats::default()));
989998
}
990999

@@ -1024,8 +1033,9 @@ impl TreeState {
10241033
snapshotter.into_result()
10251034
})?;
10261035

1027-
let stats = SnapshotStats {
1036+
let mut stats = SnapshotStats {
10281037
untracked_paths: untracked_paths_rx.into_iter().collect(),
1038+
dry_run_tree_id: None,
10291039
};
10301040
let mut tree_builder = MergedTreeBuilder::new(self.tree_id.clone());
10311041
trace_span!("process tree entries").in_scope(|| {
@@ -1041,6 +1051,18 @@ impl TreeState {
10411051
}
10421052
deleted_files
10431053
});
1054+
trace_span!("write tree").in_scope(|| {
1055+
let new_tree_id = tree_builder.write_tree(&self.store).unwrap();
1056+
if dry_run {
1057+
stats.dry_run_tree_id = Some(new_tree_id);
1058+
} else {
1059+
is_dirty |= new_tree_id != self.tree_id;
1060+
self.tree_id = new_tree_id;
1061+
}
1062+
});
1063+
if dry_run {
1064+
return Ok((false, stats));
1065+
}
10441066
trace_span!("process file states").in_scope(|| {
10451067
let changed_file_states = file_states_rx
10461068
.iter()
@@ -1050,11 +1072,6 @@ impl TreeState {
10501072
self.file_states
10511073
.merge_in(changed_file_states, &deleted_files);
10521074
});
1053-
trace_span!("write tree").in_scope(|| {
1054-
let new_tree_id = tree_builder.write_tree(&self.store).unwrap();
1055-
is_dirty |= new_tree_id != self.tree_id;
1056-
self.tree_id = new_tree_id;
1057-
});
10581075
if cfg!(debug_assertions) {
10591076
let tree = self.current_tree().unwrap();
10601077
let tree_paths: HashSet<_> = tree

lib/src/working_copy.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ pub struct SnapshotOptions<'a> {
222222
pub max_new_file_size: u64,
223223
/// Expected conflict marker style for checking for changed files.
224224
pub conflict_marker_style: ConflictMarkerStyle,
225+
/// If true, skip any updates to the working copy metadata files when
226+
/// snapshotting.
227+
pub dry_run: bool,
225228
}
226229

227230
impl SnapshotOptions<'_> {
@@ -234,6 +237,7 @@ impl SnapshotOptions<'_> {
234237
start_tracking_matcher: &EverythingMatcher,
235238
max_new_file_size: u64::MAX,
236239
conflict_marker_style: ConflictMarkerStyle::default(),
240+
dry_run: false,
237241
}
238242
}
239243
}
@@ -246,6 +250,8 @@ pub type SnapshotProgress<'a> = dyn Fn(&RepoPath) + 'a + Sync;
246250
pub struct SnapshotStats {
247251
/// List of new (previously untracked) files which are still untracked.
248252
pub untracked_paths: BTreeMap<RepoPathBuf, UntrackedReason>,
253+
/// The tree id generated in dry_run mode. Set iff dry_run is enabled.
254+
pub dry_run_tree_id: Option<MergedTreeId>,
249255
}
250256

251257
/// Reason why the new path isn't tracked.

lib/tests/test_local_working_copy.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,33 @@ fn test_snapshot_file_directory_transition() {
838838
assert_eq!(new_tree.id(), tree1.id());
839839
}
840840

841+
#[test]
842+
fn test_snapshot_dry_run() {
843+
let mut test_workspace = TestWorkspace::init();
844+
let workspace_root = test_workspace.workspace.workspace_root().to_owned();
845+
let file_path = workspace_root.join("file");
846+
std::fs::write(&file_path, "contents".as_bytes()).unwrap();
847+
let repo = &test_workspace.repo;
848+
let mut locked_ws = test_workspace
849+
.workspace
850+
.start_working_copy_mutation()
851+
.unwrap();
852+
let (new_tree_id, stats) = locked_ws
853+
.locked_wc()
854+
.snapshot(&SnapshotOptions {
855+
dry_run: true,
856+
..SnapshotOptions::empty_for_test()
857+
})
858+
.unwrap();
859+
assert_eq!(new_tree_id, repo.store().empty_merged_tree_id());
860+
assert_ne!(
861+
stats
862+
.dry_run_tree_id
863+
.expect("dry_run_tree_id must be set in dry_run mode"),
864+
repo.store().empty_merged_tree_id()
865+
);
866+
}
867+
841868
#[test]
842869
fn test_materialize_snapshot_conflicted_files() {
843870
let mut test_workspace = TestWorkspace::init();

0 commit comments

Comments
 (0)