diff --git a/rust/hg-core/src/copy_tracing.rs b/rust/hg-core/src/copy_tracing.rs --- a/rust/hg-core/src/copy_tracing.rs +++ b/rust/hg-core/src/copy_tracing.rs @@ -110,9 +110,6 @@ /// maps CopyDestination to Copy Source (+ a "timestamp" for the operation) type InternalPathCopies = OrdMap; -/// hold parent 1, parent 2 and relevant files actions. -pub type RevInfo<'a> = (Revision, Revision, ChangedFiles<'a>); - /// represent the files affected by a changesets /// /// This hold a subset of mercurial.metadata.ChangingFiles as we do not need @@ -334,22 +331,6 @@ } } -/// A small struct whose purpose is to ensure lifetime of bytes referenced in -/// ChangedFiles -/// -/// It is passed to the RevInfoMaker callback who can assign any necessary -/// content to the `data` attribute. The copy tracing code is responsible for -/// keeping the DataHolder alive at least as long as the ChangedFiles object. -pub struct DataHolder { - /// RevInfoMaker callback should assign data referenced by the - /// ChangedFiles struct it return to this attribute. The DataHolder - /// lifetime will be at least as long as the ChangedFiles one. - pub data: Option, -} - -pub type RevInfoMaker<'a, D> = - Box Fn(Revision, &'r mut DataHolder) -> RevInfo<'r> + 'a>; - /// A small "tokenizer" responsible of turning full HgPath into lighter /// PathToken /// @@ -382,32 +363,28 @@ } /// Same as mercurial.copies._combine_changeset_copies, but in Rust. -/// -/// Arguments are: -/// -/// revs: all revisions to be considered -/// children: a {parent ? [childrens]} mapping -/// target_rev: the final revision we are combining copies to -/// rev_info(rev): callback to get revision information: -/// * first parent -/// * second parent -/// * ChangedFiles -/// isancestors(low_rev, high_rev): callback to check if a revision is an -/// ancestor of another -pub fn combine_changeset_copies( - revs: Vec, - mut children_count: HashMap, - target_rev: Revision, - rev_info: RevInfoMaker, -) -> PathCopies { - let mut all_copies = HashMap::new(); +pub struct CombineChangesetCopies { + all_copies: HashMap, + path_map: TwoWayPathMap, + children_count: HashMap, +} - let mut path_map = TwoWayPathMap::default(); +impl CombineChangesetCopies { + pub fn new(children_count: HashMap) -> Self { + Self { + all_copies: HashMap::new(), + path_map: TwoWayPathMap::default(), + children_count, + } + } - for rev in revs { - let mut d: DataHolder = DataHolder { data: None }; - let (p1, p2, changes) = rev_info(rev, &mut d); - + pub fn add_revision( + &mut self, + rev: Revision, + p1: Revision, + p2: Revision, + changes: ChangedFiles<'_>, + ) { // We will chain the copies information accumulated for the parent with // the individual copies information the curent revision. Creating a // new TimeStampedPath for each `rev` → `children` vertex. @@ -415,49 +392,61 @@ let p1_copies = match p1 { NULL_REVISION => None, _ => get_and_clean_parent_copies( - &mut all_copies, - &mut children_count, + &mut self.all_copies, + &mut self.children_count, p1, ), // will be None if the vertex is not to be traversed }; let p2_copies = match p2 { NULL_REVISION => None, _ => get_and_clean_parent_copies( - &mut all_copies, - &mut children_count, + &mut self.all_copies, + &mut self.children_count, p2, ), // will be None if the vertex is not to be traversed }; // combine it with data for that revision - let (p1_copies, p2_copies) = - chain_changes(&mut path_map, p1_copies, p2_copies, &changes, rev); + let (p1_copies, p2_copies) = chain_changes( + &mut self.path_map, + p1_copies, + p2_copies, + &changes, + rev, + ); let copies = match (p1_copies, p2_copies) { (None, None) => None, (c, None) => c, (None, c) => c, (Some(p1_copies), Some(p2_copies)) => Some(merge_copies_dict( - &path_map, rev, p2_copies, p1_copies, &changes, + &self.path_map, + rev, + p2_copies, + p1_copies, + &changes, )), }; if let Some(c) = copies { - all_copies.insert(rev, c); + self.all_copies.insert(rev, c); } } - // Drop internal information (like the timestamp) and return the final - // mapping. - let tt_result = all_copies - .remove(&target_rev) - .expect("target revision was not processed"); - let mut result = PathCopies::default(); - for (dest, tt_source) in tt_result { - if let Some(path) = tt_source.path { - let path_dest = path_map.untokenize(dest).to_owned(); - let path_path = path_map.untokenize(path).to_owned(); - result.insert(path_dest, path_path); + pub fn finish(mut self, target_rev: Revision) -> PathCopies { + // Drop internal information (like the timestamp) and return the final + // mapping. + let tt_result = self + .all_copies + .remove(&target_rev) + .expect("target revision was not processed"); + let mut result = PathCopies::default(); + for (dest, tt_source) in tt_result { + if let Some(path) = tt_source.path { + let path_dest = self.path_map.untokenize(dest).to_owned(); + let path_path = self.path_map.untokenize(path).to_owned(); + result.insert(path_dest, path_path); + } } + result } - result } /// fetch previous computed information diff --git a/rust/hg-cpython/src/copy_tracing.rs b/rust/hg-cpython/src/copy_tracing.rs --- a/rust/hg-cpython/src/copy_tracing.rs +++ b/rust/hg-cpython/src/copy_tracing.rs @@ -8,11 +8,8 @@ use cpython::PyTuple; use cpython::Python; -use hg::copy_tracing::combine_changeset_copies; use hg::copy_tracing::ChangedFiles; -use hg::copy_tracing::DataHolder; -use hg::copy_tracing::RevInfo; -use hg::copy_tracing::RevInfoMaker; +use hg::copy_tracing::CombineChangesetCopies; use hg::Revision; /// Combines copies information contained into revision `revs` to build a copy @@ -26,64 +23,41 @@ target_rev: Revision, rev_info: PyObject, ) -> PyResult { - let revs: PyResult<_> = - revs.iter(py).map(|r| Ok(r.extract(py)?)).collect(); - - // Wrap the `rev_info_maker` python callback as a Rust closure - // - // No errors are expected from the Python side, and they will should only - // happens in case of programing error or severe data corruption. Such - // errors will raise panic and the rust-cpython harness will turn them into - // Python exception. - let rev_info_maker: RevInfoMaker = - Box::new(|rev: Revision, d: &mut DataHolder| -> RevInfo { - let res: PyTuple = rev_info - .call(py, (rev,), None) - .expect("rust-copy-tracing: python call to `rev_info` failed") - .cast_into(py) - .expect( - "rust-copy_tracing: python call to `rev_info` returned \ - unexpected non-Tuple value", - ); - let p1 = res.get_item(py, 0).extract(py).expect( - "rust-copy-tracing: rev_info return is invalid, first item \ - is a not a revision", - ); - let p2 = res.get_item(py, 1).extract(py).expect( - "rust-copy-tracing: rev_info return is invalid, first item \ - is a not a revision", - ); - - let files = match res.get_item(py, 2).extract::(py) { - Ok(raw) => { - // Give responsability for the raw bytes lifetime to - // hg-core - d.data = Some(raw); - let addrs = d.data.as_ref().expect( - "rust-copy-tracing: failed to get a reference to the \ - raw bytes for copy data").data(py); - ChangedFiles::new(addrs) - } - // value was presumably None, meaning they was no copy data. - Err(_) => ChangedFiles::new_empty(), - }; - - (p1, p2, files) - }); - let children_count: PyResult<_> = children_count + let children_count = children_count .items(py) .iter() .map(|(k, v)| Ok((k.extract(py)?, v.extract(py)?))) - .collect(); + .collect::>()?; + + /// (Revision number, parent 1, parent 2, copy data for this revision) + type RevInfo = (Revision, Revision, Revision, Option); + + let revs_info = revs.iter(py).map(|rev_py| -> PyResult { + let rev = rev_py.extract(py)?; + let tuple: PyTuple = + rev_info.call(py, (rev_py,), None)?.cast_into(py)?; + let p1 = tuple.get_item(py, 0).extract(py)?; + let p2 = tuple.get_item(py, 1).extract(py)?; + let opt_bytes = tuple.get_item(py, 2).extract(py)?; + Ok((rev, p1, p2, opt_bytes)) + }); - let res = combine_changeset_copies( - revs?, - children_count?, - target_rev, - rev_info_maker, - ); + let mut combine_changeset_copies = + CombineChangesetCopies::new(children_count); + + for rev_info in revs_info { + let (rev, p1, p2, opt_bytes) = rev_info?; + let files = match &opt_bytes { + Some(bytes) => ChangedFiles::new(bytes.data(py)), + // value was presumably None, meaning they was no copy data. + None => ChangedFiles::new_empty(), + }; + + combine_changeset_copies.add_revision(rev, p1, p2, files) + } + let path_copies = combine_changeset_copies.finish(target_rev); let out = PyDict::new(py); - for (dest, source) in res.into_iter() { + for (dest, source) in path_copies.into_iter() { out.set_item( py, PyBytes::new(py, &dest.into_vec()),