( )⚙ D9643 copies-rust: add methods to build and update CopySource

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9643

copies-rust: add methods to build and update CopySource
ClosedPublic

Authored by marmoute on Dec 21 2020, 5:28 PM.

Details

Reviewers
pulkit
Alphare
Group Reviewers
hg-reviewers
Commits
rHGb6f65d90e8af: copies-rust: add methods to build and update CopySource
rHG7d2486644528: copies-rust: add methods to build and update CopySource
Summary

Having explicit method with clear semantic help to clarify the code and prepare
an update to the underlying documentation without too much disruption.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.Dec 21 2020, 5:28 PM
pulkit added a subscriber: pulkit.Dec 22 2020, 9:10 AM
pulkit added inline comments.
rust/hg-core/src/copy_tracing.rs
687

Seems like an unrelated change that should be in a different patch.

711

This too looks like an unrelated change as far as this patch is concerned.

marmoute added inline comments.Dec 22 2020, 9:15 PM
rust/hg-core/src/copy_tracing.rs
687

Good catch ! I forgot this was there. Updated version pushed

baymax updated this revision to Diff 24507.Dec 23 2020, 3:50 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit accepted this revision.Dec 28 2020, 3:16 AM
This revision is now accepted and ready to land.Dec 28 2020, 3:16 AM
baymax updated this revision to Diff 24577.Jan 5 2021, 3:52 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 24599.Jan 5 2021, 9:17 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.Jan 8 2021, 6:15 AM
baymax updated this revision to Diff 24640.Jan 8 2021, 10:26 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 25688.Feb 22 2021, 9:04 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 25747.Feb 22 2021, 11:17 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 25782.Feb 22 2021, 3:16 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute added a commit: rHG7d2486644528: copies-rust: add methods to build and update CopySource.Feb 24 2021, 11:01 AM
Closed by commit rHG7d2486644528: copies-rust: add methods to build and update CopySource (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/copy_tracing.rs (129 lines)

Diff 24467

rust/hg-core/src/copy_tracing.rs

use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::HashMap; use std::collections::HashMap;
use std::convert::TryInto; use std::convert::TryInto;
pub type PathCopies = HashMap<HgPathBuf, HgPathBuf>; pub type PathCopies = HashMap<HgPathBuf, HgPathBuf>;
type PathToken = usize; type PathToken = usize;
#[derive(Clone, Debug, PartialEq, Copy)] #[derive(Clone, Debug, PartialEq)]
struct CopySource { struct CopySource {
/// revision at which the copy information was added /// revision at which the copy information was added
rev: Revision, rev: Revision,
/// the copy source, (Set to None in case of deletion of the associated /// the copy source, (Set to None in case of deletion of the associated
/// key) /// key)
path: Option<PathToken>, path: Option<PathToken>,
} }
impl CopySource {
/// create a new CopySource
///
/// Use this when no previous copy source existed.
fn new(rev: Revision, path: Option<PathToken>) -> Self {
Self { rev, path }
}
/// create a new CopySource from merging two others
///
/// Use this when merging two InternalPathCopies requires active merging of
/// some entries.
fn new_from_merge(rev: Revision, winner: &Self, loser: &Self) -> Self {
Self {
rev,
path: winner.path,
}
}
/// Update the value of a pre-existing CopySource
///
/// Use this when recording copy information from parent → child edges
fn overwrite(&mut self, rev: Revision, path: Option<PathToken>) {
self.rev = rev;
self.path = path;
}
/// Mark pre-existing copy information as "dropped" by a file deletion
///
/// Use this when recording copy information from parent → child edges
fn mark_delete(&mut self, rev: Revision) {
self.rev = rev;
self.path = None;
}
}
/// maps CopyDestination to Copy Source (+ a "timestamp" for the operation) /// maps CopyDestination to Copy Source (+ a "timestamp" for the operation)
type InternalPathCopies = OrdMap<PathToken, CopySource>; type InternalPathCopies = OrdMap<PathToken, CopySource>;
/// hold parent 1, parent 2 and relevant files actions. /// hold parent 1, parent 2 and relevant files actions.
pub type RevInfo<'a> = (Revision, Revision, ChangedFiles<'a>); pub type RevInfo<'a> = (Revision, Revision, ChangedFiles<'a>);
/// represent the files affected by a changesets /// represent the files affected by a changesets
/// ///
entry = Some(source.to_owned()); entry = Some(source.to_owned());
} }
// Each new entry is introduced by the children, we // Each new entry is introduced by the children, we
// record this information as we will need it to take // record this information as we will need it to take
// the right decision when merging conflicting copy // the right decision when merging conflicting copy
// information. See merge_copies_dict for details. // information. See merge_copies_dict for details.
match copies.entry(dest) { match copies.entry(dest) {
Entry::Vacant(slot) => { Entry::Vacant(slot) => {
let ttpc = CopySource { let ttpc = CopySource::new(current_rev, entry);
rev: current_rev,
path: entry,
};
slot.insert(ttpc); slot.insert(ttpc);
} }
Entry::Occupied(mut slot) => { Entry::Occupied(mut slot) => {
let mut ttpc = slot.get_mut(); let mut ttpc = slot.get_mut();
oracle.record_overwrite(ttpc.rev, current_rev); oracle.record_overwrite(ttpc.rev, current_rev);
ttpc.rev = current_rev; ttpc.overwrite(current_rev, entry);
ttpc.path = entry;
} }
} }
} }
Action::Removed(deleted_path) => { Action::Removed(deleted_path) => {
// We must drop copy information for removed file. // We must drop copy information for removed file.
// //
// We need to explicitly record them as dropped to // We need to explicitly record them as dropped to
// propagate this information when merging two // propagate this information when merging two
// InternalPathCopies object. // InternalPathCopies object.
let deleted = path_map.tokenize(deleted_path); let deleted = path_map.tokenize(deleted_path);
copies.entry(deleted).and_modify(|old| { copies.entry(deleted).and_modify(|old| {
oracle.record_overwrite(old.rev, current_rev); oracle.record_overwrite(old.rev, current_rev);
old.rev = current_rev; old.mark_delete(current_rev);
old.path = None;
}); });
} }
} }
} }
copies copies
} }
/// merge two copies-mapping together, minor and major /// merge two copies-mapping together, minor and major
major.insert(dest, src_minor); major.insert(dest, src_minor);
} }
Some(src_major) => { Some(src_major) => {
let (pick, overwrite) = let (pick, overwrite) =
cmp_value(oracle, &dest, &src_minor, src_major); cmp_value(oracle, &dest, &src_minor, src_major);
if overwrite { if overwrite {
oracle.record_overwrite(src_minor.rev, current_merge); oracle.record_overwrite(src_minor.rev, current_merge);
oracle.record_overwrite(src_major.rev, current_merge); oracle.record_overwrite(src_major.rev, current_merge);
let path = match pick { let src = match pick {
MergePick::Major => src_major.path, MergePick::Major => CopySource::new_from_merge(
MergePick::Minor => src_minor.path, current_merge,
MergePick::Any => src_major.path, src_major,
}; &src_minor,
let src = CopySource { ),
rev: current_merge, MergePick::Minor => CopySource::new_from_merge(
path, current_merge,
&src_minor,
src_major,
),
MergePick::Any => CopySource::new_from_merge(
current_merge,
src_major,
&src_minor,
),
}; };
major.insert(dest, src); major.insert(dest, src);
} else { } else {
match pick { match pick {
MergePick::Any | MergePick::Major => None, MergePick::Any | MergePick::Major => None,
MergePick::Minor => major.insert(dest, src_minor), MergePick::Minor => major.insert(dest, src_minor),
}; };
} }
} }
}; };
} }
major major
} else if major.len() * 2 < minor.len() { } else if major.len() * 2 < minor.len() {
// This use the same rational than the previous block. // This use the same rational than the previous block.
// (Check previous block documentation for details.) // (Check previous block documentation for details.)
for (dest, src_major) in major { for (dest, src_major) in major {
let src_minor = minor.get(&dest); let src_minor = minor.get(&dest);
match src_minor { match src_minor {
None => { None => {
minor.insert(dest, src_major); minor.insert(dest, src_major);
} }
Some(src_minor) => { Some(src_minor) => {
let (pick, overwrite) = let (pick, overwrite) =
cmp_value(oracle, &dest, &src_major, src_minor); cmp_value(oracle, &dest, src_minor, &src_major);
pulkitUnsubmitted
Not Done

Seems like an unrelated change that should be in a different patch.

pulkit: Seems like an unrelated change that should be in a different patch.
marmouteAuthorUnsubmitted
Done

Good catch ! I forgot this was there. Updated version pushed

marmoute: Good catch ! I forgot this was there. Updated version pushed
if overwrite { if overwrite {
oracle.record_overwrite(src_minor.rev, current_merge); oracle.record_overwrite(src_minor.rev, current_merge);
oracle.record_overwrite(src_major.rev, current_merge); oracle.record_overwrite(src_major.rev, current_merge);
let path = match pick { let src = match pick {
MergePick::Major => src_minor.path, MergePick::Major => CopySource::new_from_merge(
MergePick::Minor => src_major.path, current_merge,
MergePick::Any => src_major.path, &src_major,
}; src_minor,
let src = CopySource { ),
rev: current_merge, MergePick::Minor => CopySource::new_from_merge(
path, current_merge,
src_minor,
&src_major,
),
MergePick::Any => CopySource::new_from_merge(
current_merge,
&src_major,
src_minor,
),
}; };
minor.insert(dest, src); minor.insert(dest, src);
} else { } else {
match pick { match pick {
MergePick::Any | MergePick::Major => None, MergePick::Any | MergePick::Minor => None,
pulkitUnsubmitted
Not Done

This too looks like an unrelated change as far as this patch is concerned.

pulkit: This too looks like an unrelated change as far as this patch is concerned.
MergePick::Minor => minor.insert(dest, src_major), MergePick::Major => minor.insert(dest, src_major),
}; };
} }
} }
}; };
} }
minor minor
} else { } else {
let mut override_minor = Vec::new(); let mut override_minor = Vec::new();
DiffItem::Update { old, new } => { DiffItem::Update { old, new } => {
let (dest, src_major) = new; let (dest, src_major) = new;
let (_, src_minor) = old; let (_, src_minor) = old;
let (pick, overwrite) = let (pick, overwrite) =
cmp_value(oracle, dest, src_minor, src_major); cmp_value(oracle, dest, src_minor, src_major);
if overwrite { if overwrite {
oracle.record_overwrite(src_minor.rev, current_merge); oracle.record_overwrite(src_minor.rev, current_merge);
oracle.record_overwrite(src_major.rev, current_merge); oracle.record_overwrite(src_major.rev, current_merge);
let path = match pick { let src = match pick {
MergePick::Major => src_major.path, MergePick::Major => CopySource::new_from_merge(
MergePick::Minor => src_minor.path, current_merge,
// If the two entry are identical, no need to do src_major,
// anything (but diff should not have yield them) src_minor,
MergePick::Any => src_major.path, ),
}; MergePick::Minor => CopySource::new_from_merge(
let src = CopySource { current_merge,
rev: current_merge, src_minor,
path, src_major,
),
MergePick::Any => CopySource::new_from_merge(
current_merge,
src_major,
src_minor,
),
}; };
to_minor(dest, &src); to_minor(dest, &src);
to_major(dest, &src); to_major(dest, &src);
} else { } else {
match pick { match pick {
MergePick::Major => to_minor(dest, src_major), MergePick::Major => to_minor(dest, src_major),
MergePick::Minor => to_major(dest, src_minor), MergePick::Minor => to_major(dest, src_minor),
// If the two entry are identical, no need to do // If the two entry are identical, no need to do