checkout: use action map to construct CheckoutPlan

Summary: This is step towards unifying native merge/rebase structs with native checkout - we now construct native checkout plan from the action map, instead of directly making it from the diff

Reviewed By: quark-zju

Differential Revision: D28078156

fbshipit-source-id: 318d7e419ca9fef15a4aebf7494451f69a3bbbe5
This commit is contained in:
Andrey Chursin 2021-04-30 13:03:56 -07:00 committed by Facebook GitHub Bot
parent dd5909abe8
commit ba0ad33d20
4 changed files with 140 additions and 189 deletions

View File

@ -60,16 +60,17 @@ py_class!(class checkoutplan |py| {
let current = current_manifest.borrow_underlying(py);
let target = target_manifest.borrow_underlying(py);
let diff = Diff::new(&current, &target, &matcher);
let vfs = VFS::new(root.to_path_buf()).map_pyerr(py)?;
let checkout = Checkout::from_config(vfs, &config).map_pyerr(py)?;
let mut plan = checkout.plan_diff(diff).map_pyerr(py)?;
if let Some(progress_path) = progress_path {
plan.add_progress(progress_path.to_path_buf()).map_pyerr(py)?;
}
let mut actions = ActionMap::from_diff(diff).map_pyerr(py)?;
if let Some((old_sparse_matcher, new_sparse_matcher)) = sparse_change {
let old_matcher = Box::new(PythonMatcher::new(py, old_sparse_matcher));
let new_matcher = Box::new(PythonMatcher::new(py, new_sparse_matcher));
plan = plan.with_sparse_profile_change(&old_matcher, &new_matcher, &*target).map_pyerr(py)?;
actions = actions.with_sparse_profile_change(&old_matcher, &new_matcher, &*target).map_pyerr(py)?;
}
let vfs = VFS::new(root.to_path_buf()).map_pyerr(py)?;
let checkout = Checkout::from_config(vfs, &config).map_pyerr(py)?;
let mut plan = checkout.plan_action_map(actions);
if let Some(progress_path) = progress_path {
plan.add_progress(progress_path.to_path_buf()).map_pyerr(py)?;
}
checkoutplan::create_instance(py, plan)
}

View File

@ -6,29 +6,29 @@
*/
use anyhow::Result;
use manifest::{DiffType, FileMetadata, FileType, Manifest};
use pathmatcher::AlwaysMatcher;
use std::collections::HashMap;
use manifest::{DiffEntry, DiffType, FileMetadata, FileType, Manifest};
use pathmatcher::{Matcher, XorMatcher};
use std::collections::{hash_map::Entry, HashMap};
use std::fmt;
use std::ops::{Deref, DerefMut};
use types::RepoPathBuf;
/// Map of simple actions that needs to be performed to move between revisions without conflicts.
#[derive(Default, Clone)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct ActionMap {
map: HashMap<RepoPathBuf, Action>,
}
/// Basic update action.
/// Diff between regular(no conflict checkin) commit generates list of such actions.
#[derive(Clone, Copy)]
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum Action {
Update(UpdateAction),
Remove,
UpdateExec(bool),
}
#[derive(Clone, Copy)]
#[derive(Debug, Clone, Copy, PartialEq)]
pub struct UpdateAction {
pub from: Option<FileMetadata>,
pub to: FileMetadata,
@ -37,9 +37,7 @@ pub struct UpdateAction {
impl ActionMap {
// This is similar to CheckoutPlan::new
// Eventually CheckoutPlan::new will migrate to take (Conflict)ActionMap instead of a Diff and there won't be code duplication
pub fn from_diff<M: Manifest>(src: &M, dst: &M) -> Result<Self> {
let matcher = AlwaysMatcher::new();
let diff = src.diff(dst, &matcher);
pub fn from_diff<D: Iterator<Item = Result<DiffEntry>>>(diff: D) -> Result<Self> {
let mut map = HashMap::new();
for entry in diff {
let entry = entry?;
@ -66,6 +64,58 @@ impl ActionMap {
}
Ok(Self { map })
}
pub fn with_sparse_profile_change(
mut self,
old_matcher: &impl Matcher,
new_matcher: &impl Matcher,
new_manifest: &impl Manifest,
) -> Result<Self> {
// First - remove all the files that were scheduled for update, but actually aren't in new sparse profile
let mut result = Ok(());
self.map.retain(|path, action| {
if result.is_err() {
return true;
}
if matches!(action, Action::Remove) {
return true;
}
match new_matcher.matches_file(path.as_ref()) {
Ok(v) => v,
Err(err) => {
result = Err(err);
true
}
}
});
result?;
// Second - handle files in a new manifest, that were affected by sparse profile change
let xor_matcher = XorMatcher::new(old_matcher, new_matcher);
for file in new_manifest.files(&xor_matcher) {
let file = file?;
if new_matcher.matches_file(&file.path)? {
match self.map.entry(file.path) {
Entry::Vacant(va) => {
va.insert(Action::Update(UpdateAction::new(None, file.meta)));
}
Entry::Occupied(_) => {}
}
} else {
// by definition of xor matcher this means old_matcher.matches_file==true
self.map.insert(file.path, Action::Remove);
}
}
Ok(self)
}
#[cfg(test)]
pub fn empty() -> Self {
Self {
map: Default::default(),
}
}
}
impl Deref for ActionMap {
@ -131,3 +181,66 @@ fn pyflags(t: &FileType) -> &'static str {
FileType::Executable => "x",
}
}
#[cfg(test)]
mod tests {
use super::*;
use manifest_tree::testutil::make_tree_manifest_from_meta;
use pathmatcher::TreeMatcher;
use types::HgId;
#[test]
fn test_with_sparse_profile_change() -> Result<()> {
let a = (rp("a"), FileMetadata::regular(hgid(1)));
let b = (rp("b"), FileMetadata::regular(hgid(2)));
let c = (rp("c"), FileMetadata::regular(hgid(3)));
let ab_profile = TreeMatcher::from_rules(["a", "b"].iter())?;
let ac_profile = TreeMatcher::from_rules(["a", "c"].iter())?;
let manifest = make_tree_manifest_from_meta(vec![a, b, c]);
let actions =
ActionMap::empty().with_sparse_profile_change(&ab_profile, &ab_profile, &manifest)?;
assert_eq!("", &actions.to_string());
let mut expected_actions = ActionMap::empty();
expected_actions.map.insert(rp("b"), Action::Remove);
expected_actions.map.insert(
rp("c"),
Action::Update(UpdateAction::new(None, FileMetadata::regular(hgid(3)))),
);
let actions =
ActionMap::empty().with_sparse_profile_change(&ab_profile, &ac_profile, &manifest)?;
assert_eq!(expected_actions, actions);
let mut actions = ActionMap::empty();
actions.map.insert(
rp("b"),
Action::Update(UpdateAction::new(None, FileMetadata::regular(hgid(10)))),
);
actions.map.insert(rp("b"), Action::UpdateExec(true));
let actions = actions.with_sparse_profile_change(&ab_profile, &ac_profile, &manifest)?;
assert_eq!(expected_actions, actions);
let mut actions = ActionMap::empty();
actions.map.insert(
rp("c"),
Action::Update(UpdateAction::new(None, FileMetadata::regular(hgid(3)))),
);
let actions = actions.with_sparse_profile_change(&ab_profile, &ac_profile, &manifest)?;
assert_eq!(expected_actions, actions);
Ok(())
}
fn rp(p: &str) -> RepoPathBuf {
RepoPathBuf::from_string(p.to_string()).unwrap()
}
fn hgid(p: u8) -> HgId {
let mut r = HgId::default().into_byte_array();
r[0] = p;
HgId::from_byte_array(r)
}
}

View File

@ -9,10 +9,9 @@ use anyhow::{anyhow, bail, format_err, Result};
use futures::channel::mpsc;
use futures::channel::mpsc::UnboundedReceiver;
use futures::{stream, try_join, Stream, StreamExt};
use manifest::{DiffEntry, DiffType, FileMetadata, FileType, Manifest};
use manifest::{FileMetadata, FileType, Manifest};
use minibytes::Bytes;
use parking_lot::Mutex;
use pathmatcher::{Matcher, XorMatcher};
use progress_model::ProgressBar;
use progress_model::Registry;
use revisionstore::{
@ -20,7 +19,7 @@ use revisionstore::{
StoreKey, StoreResult,
};
use std::boxed::Box;
use std::collections::{HashMap, HashSet};
use std::collections::HashMap;
use std::fmt;
use std::fs::{File, OpenOptions};
use std::io::{BufRead, BufReader, Write};
@ -128,13 +127,6 @@ impl Checkout {
pub fn plan_action_map(&self, map: ActionMap) -> CheckoutPlan {
CheckoutPlan::from_action_map(self.clone(), map)
}
pub fn plan_diff<D: Iterator<Item = Result<DiffEntry>>>(
&self,
iter: D,
) -> Result<CheckoutPlan> {
CheckoutPlan::from_diff(self.clone(), iter)
}
}
impl CheckoutPlan {
@ -162,53 +154,6 @@ impl CheckoutPlan {
}
}
/// Processes diff into checkout plan.
/// Left in the diff is a current commit.
/// Right is a commit to be checked out.
fn from_diff<D: Iterator<Item = Result<DiffEntry>>>(
checkout: Checkout,
iter: D,
) -> Result<Self> {
let mut remove = vec![];
let mut update_content = vec![];
let mut update_meta = vec![];
for item in iter {
let item: DiffEntry = item?;
match item.diff_type {
DiffType::LeftOnly(_) => remove.push(item.path),
DiffType::RightOnly(meta) => {
update_content.push(UpdateContentAction::new(item.path, meta, true))
}
DiffType::Changed(old, new) => {
match (old.hgid == new.hgid, old.file_type, new.file_type) {
(true, FileType::Executable, FileType::Regular) => {
update_meta.push(UpdateMetaAction {
path: item.path,
set_x_flag: false,
});
}
(true, FileType::Regular, FileType::Executable) => {
update_meta.push(UpdateMetaAction {
path: item.path,
set_x_flag: true,
});
}
_ => {
update_content.push(UpdateContentAction::new(item.path, new, false));
}
}
}
};
}
Ok(Self {
remove,
update_content,
update_meta,
progress: None,
checkout,
})
}
pub fn add_progress(&mut self, path: PathBuf) -> Result<()> {
let vfs = &self.checkout.vfs;
let progress = if path.exists() {
@ -226,38 +171,6 @@ impl CheckoutPlan {
Ok(())
}
/// Updates current plan to account for sparse profile change
pub fn with_sparse_profile_change(
mut self,
old_matcher: &impl Matcher,
new_matcher: &impl Matcher,
new_manifest: &impl Manifest,
) -> Result<Self> {
// First - remove all the files that were scheduled for update, but actually aren't in new sparse profile
retain_paths(&mut self.update_content, new_matcher)?;
retain_paths(&mut self.update_meta, new_matcher)?;
let updated_content: HashSet<_> =
self.update_content.iter().map(|a| a.path.clone()).collect();
// Second - handle files in a new manifest, that were affected by sparse profile change
let xor_matcher = XorMatcher::new(old_matcher, new_matcher);
for file in new_manifest.files(&xor_matcher) {
let file = file?;
if new_matcher.matches_file(&file.path)? {
if !updated_content.contains(&file.path) {
self.update_content
.push(UpdateContentAction::new(file.path, file.meta, true));
}
} else {
// by definition of xor matcher this means old_matcher.matches_file==true
self.remove.push(file.path);
}
}
Ok(self)
}
/// Applies plan to the root using store to fetch data.
/// This async function offloads file system operation to tokio blocking thread pool.
/// It limits number of concurrent fs operations to Checkout::concurrency.
@ -863,23 +776,6 @@ impl UpdateContentAction {
}
}
fn retain_paths<T: AsRef<RepoPath>>(v: &mut Vec<T>, matcher: impl Matcher) -> Result<()> {
let mut result = Ok(());
v.retain(|p| {
if result.is_err() {
return true;
}
match matcher.matches_file(p.as_ref()) {
Ok(v) => v,
Err(err) => {
result = Err(err);
true
}
}
});
result
}
impl AsRef<RepoPath> for UpdateContentAction {
fn as_ref(&self) -> &RepoPath {
&self.path
@ -901,7 +797,7 @@ mod test {
use anyhow::Context;
use manifest_tree::testutil::make_tree_manifest_from_meta;
use manifest_tree::Diff;
use pathmatcher::{AlwaysMatcher, TreeMatcher};
use pathmatcher::AlwaysMatcher;
use quickcheck::{Arbitrary, StdGen};
use rand::SeedableRng;
use rand_chacha::ChaChaRng;
@ -957,68 +853,6 @@ mod test {
Ok(())
}
#[test]
fn test_with_sparse_profile_change() -> Result<()> {
let a = (rp("a"), FileMetadata::regular(hgid(1)));
let b = (rp("b"), FileMetadata::regular(hgid(2)));
let c = (rp("c"), FileMetadata::regular(hgid(3)));
let ab_profile = TreeMatcher::from_rules(["a/**", "b/**"].iter())?;
let ac_profile = TreeMatcher::from_rules(["a/**", "c/**"].iter())?;
let manifest = make_tree_manifest_from_meta(vec![a, b, c]);
let tempdir = tempfile::tempdir()?;
let working_path = tempdir.path().to_path_buf().join("workingdir");
create_dir(working_path.as_path()).unwrap();
let vfs = VFS::new(working_path.clone())?;
let plan = CheckoutPlan::empty(vfs.clone()).with_sparse_profile_change(
&ab_profile,
&ab_profile,
&manifest,
)?;
assert_eq!("", &plan.to_string());
let plan = CheckoutPlan::empty(vfs.clone()).with_sparse_profile_change(
&ab_profile,
&ac_profile,
&manifest,
)?;
assert_eq!(
"rm b\nup c=>0300000000000000000000000000000000000000\n",
&plan.to_string()
);
let mut plan = CheckoutPlan::empty(vfs.clone());
plan.update_content.push(UpdateContentAction::new(
rp("b"),
FileMetadata::regular(hgid(10)),
true,
));
plan.update_meta.push(UpdateMetaAction {
path: rp("b"),
set_x_flag: true,
});
let plan = plan.with_sparse_profile_change(&ab_profile, &ac_profile, &manifest)?;
assert_eq!(
"rm b\nup c=>0300000000000000000000000000000000000000\n",
&plan.to_string()
);
let mut plan = CheckoutPlan::empty(vfs.clone());
plan.update_content.push(UpdateContentAction::new(
rp("c"),
FileMetadata::regular(hgid(3)),
true,
));
let plan = plan.with_sparse_profile_change(&ab_profile, &ac_profile, &manifest)?;
assert_eq!(
"rm b\nup c=>0300000000000000000000000000000000000000\n",
&plan.to_string()
);
Ok(())
}
#[test]
fn test_progress_parsing() -> Result<()> {
let tempdir = tempfile::tempdir()?;
@ -1112,8 +946,7 @@ mod test {
let vfs = VFS::new(working_path.clone())?;
let checkout = Checkout::default_config(vfs);
let plan = checkout
.plan_diff(diff)
.context("Plan construction failed")?;
.plan_action_map(ActionMap::from_diff(diff).context("Plan construction failed")?);
// Use clean vfs for test
plan.apply_stream(dummy_fs)

View File

@ -10,6 +10,7 @@ use crate::conflict::{Conflict, ConflictState};
use anyhow::bail;
use anyhow::Result;
use manifest::{FileType, FsNodeMetadata, Manifest};
use pathmatcher::AlwaysMatcher;
use std::collections::HashSet;
use std::fmt;
use types::RepoPathBuf;
@ -44,8 +45,11 @@ impl Merge {
dest: &M,
base: &M,
) -> Result<MergeResult<M>> {
let dest_actions = ActionMap::from_diff(base, dest)?;
let src_actions = ActionMap::from_diff(base, src)?;
let matcher = AlwaysMatcher::new();
let diff = base.diff(dest, &matcher);
let dest_actions = ActionMap::from_diff(diff)?;
let diff = base.diff(src, &matcher);
let src_actions = ActionMap::from_diff(diff)?;
let dest_files: HashSet<_> = dest_actions.keys().collect();
let src_files = src_actions.keys().collect();
let union = dest_files.union(&src_files);