Patchwork D10485: rust: Add a Timestamp struct instead of abusing Duration

login
register
mail settings
Submitter phabricator
Date April 20, 2021, 2:18 p.m.
Message ID <differential-rev-PHID-DREV-jwy6iaulp2e4mcqjvyf5-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48808/
State Superseded
Headers show

Comments

phabricator - April 20, 2021, 2:18 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  `SystemTime` would be the standard library type semantically appropriate
  instead of `Duration`.
  
  But since the value is coming from Python as a plain integer and used in
  dirstate packing code as an integer, let’s make a type that contains a single
  integer instead of using one with sub-second precision.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10485

AFFECTED FILES
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate/parsers.rs
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-core/src/lib.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/parsers.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs
--- a/rust/hg-cpython/src/parsers.rs
+++ b/rust/hg-cpython/src/parsers.rs
@@ -14,13 +14,13 @@ 
     PythonObject, ToPyObject,
 };
 use hg::{
-    pack_dirstate, parse_dirstate, utils::hg_path::HgPathBuf, DirstateEntry,
-    DirstateParents, FastHashMap, PARENT_SIZE,
+    dirstate::parsers::Timestamp, pack_dirstate, parse_dirstate,
+    utils::hg_path::HgPathBuf, DirstateEntry, DirstateParents, FastHashMap,
+    PARENT_SIZE,
 };
 use std::convert::TryInto;
 
 use crate::dirstate::{extract_dirstate, make_dirstate_tuple};
-use std::time::Duration;
 
 fn parse_dirstate_wrapper(
     py: Python,
@@ -98,7 +98,7 @@ 
             p1: p1.try_into().unwrap(),
             p2: p2.try_into().unwrap(),
         },
-        Duration::from_secs(now.as_object().extract::<u64>(py)?),
+        Timestamp(now.as_object().extract::<u64>(py)?),
     ) {
         Ok(packed) => {
             for (filename, entry) in dirstate_map.iter() {
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -10,7 +10,6 @@ 
 
 use std::cell::{Ref, RefCell};
 use std::convert::TryInto;
-use std::time::Duration;
 
 use cpython::{
     exc, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr, PyList,
@@ -27,6 +26,7 @@ 
     parsers::dirstate_parents_to_pytuple,
 };
 use hg::{
+    dirstate::parsers::Timestamp,
     dirstate_tree::dispatch::DirstateMapMethods,
     errors::HgError,
     revlog::Node,
@@ -312,7 +312,7 @@ 
         p2: PyObject,
         now: PyObject
     ) -> PyResult<PyBytes> {
-        let now = Duration::new(now.extract(py)?, 0);
+        let now = Timestamp(now.extract(py)?);
         let parents = DirstateParents {
             p1: extract_node_id(py, &p1)?,
             p2: extract_node_id(py, &p2)?,
diff --git a/rust/hg-core/src/lib.rs b/rust/hg-core/src/lib.rs
--- a/rust/hg-core/src/lib.rs
+++ b/rust/hg-core/src/lib.rs
@@ -8,7 +8,7 @@ 
 pub mod dagops;
 pub mod errors;
 pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors};
-mod dirstate;
+pub mod dirstate;
 pub mod dirstate_tree;
 pub mod discovery;
 pub mod requirements;
diff --git a/rust/hg-core/src/dirstate_tree/dispatch.rs b/rust/hg-core/src/dirstate_tree/dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs
@@ -1,6 +1,6 @@ 
 use std::path::PathBuf;
-use std::time::Duration;
 
+use crate::dirstate::parsers::Timestamp;
 use crate::matchers::Matcher;
 use crate::utils::hg_path::{HgPath, HgPathBuf};
 use crate::CopyMapIter;
@@ -90,7 +90,7 @@ 
     fn pack(
         &mut self,
         parents: DirstateParents,
-        now: Duration,
+        now: Timestamp,
     ) -> Result<Vec<u8>, DirstateError>;
 
     fn build_file_fold_map(&mut self) -> &FastHashMap<HgPathBuf, HgPathBuf>;
@@ -254,7 +254,7 @@ 
     fn pack(
         &mut self,
         parents: DirstateParents,
-        now: Duration,
+        now: Timestamp,
     ) -> Result<Vec<u8>, DirstateError> {
         self.pack(parents, now)
     }
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -1,10 +1,10 @@ 
 use std::collections::BTreeMap;
 use std::path::PathBuf;
-use std::time::Duration;
 
 use super::path_with_basename::WithBasename;
 use crate::dirstate::parsers::parse_dirstate_entries;
 use crate::dirstate::parsers::parse_dirstate_parents;
+use crate::dirstate::parsers::Timestamp;
 
 use crate::matchers::Matcher;
 use crate::revlog::node::NULL_NODE;
@@ -328,7 +328,7 @@ 
     fn pack(
         &mut self,
         _parents: DirstateParents,
-        _now: Duration,
+        _now: Timestamp,
     ) -> Result<Vec<u8>, DirstateError> {
         let _ = self.iter_node_data_mut();
         todo!()
diff --git a/rust/hg-core/src/dirstate/parsers.rs b/rust/hg-core/src/dirstate/parsers.rs
--- a/rust/hg-core/src/dirstate/parsers.rs
+++ b/rust/hg-core/src/dirstate/parsers.rs
@@ -13,7 +13,6 @@ 
 use bytes_cast::BytesCast;
 use micro_timer::timed;
 use std::convert::{TryFrom, TryInto};
-use std::time::Duration;
 
 /// Parents are stored in the dirstate as byte hashes.
 pub const PARENT_SIZE: usize = 20;
@@ -83,15 +82,17 @@ 
     Ok(parents)
 }
 
-/// `now` is the duration in seconds since the Unix epoch
+/// Seconds since the Unix epoch
+pub struct Timestamp(pub u64);
+
 pub fn pack_dirstate(
     state_map: &mut StateMap,
     copy_map: &CopyMap,
     parents: DirstateParents,
-    now: Duration,
+    now: Timestamp,
 ) -> Result<Vec<u8>, HgError> {
     // TODO move away from i32 before 2038.
-    let now: i32 = now.as_secs().try_into().expect("time overflow");
+    let now: i32 = now.0.try_into().expect("time overflow");
 
     let expected_size: usize = state_map
         .iter()
@@ -171,7 +172,7 @@ 
             p1: b"12345678910111213141".into(),
             p2: b"00000000000000000000".into(),
         };
-        let now = Duration::new(15000000, 0);
+        let now = Timestamp(15000000);
         let expected = b"1234567891011121314100000000000000000000".to_vec();
 
         assert_eq!(
@@ -202,7 +203,7 @@ 
             p1: b"12345678910111213141".into(),
             p2: b"00000000000000000000".into(),
         };
-        let now = Duration::new(15000000, 0);
+        let now = Timestamp(15000000);
         let expected = [
             49, 50, 51, 52, 53, 54, 55, 56, 57, 49, 48, 49, 49, 49, 50, 49,
             51, 49, 52, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48,
@@ -242,7 +243,7 @@ 
             p1: b"12345678910111213141".into(),
             p2: b"00000000000000000000".into(),
         };
-        let now = Duration::new(15000000, 0);
+        let now = Timestamp(15000000);
         let expected = [
             49, 50, 51, 52, 53, 54, 55, 56, 57, 49, 48, 49, 49, 49, 50, 49,
             51, 49, 52, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48,
@@ -282,7 +283,7 @@ 
             p1: b"12345678910111213141".into(),
             p2: b"00000000000000000000".into(),
         };
-        let now = Duration::new(15000000, 0);
+        let now = Timestamp(15000000);
         let result =
             pack_dirstate(&mut state_map, &copymap, parents.clone(), now)
                 .unwrap();
@@ -360,7 +361,7 @@ 
             p1: b"12345678910111213141".into(),
             p2: b"00000000000000000000".into(),
         };
-        let now = Duration::new(15000000, 0);
+        let now = Timestamp(15000000);
         let result =
             pack_dirstate(&mut state_map, &copymap, parents.clone(), now)
                 .unwrap();
@@ -406,7 +407,7 @@ 
             p1: b"12345678910111213141".into(),
             p2: b"00000000000000000000".into(),
         };
-        let now = Duration::new(15000000, 0);
+        let now = Timestamp(15000000);
         let result =
             pack_dirstate(&mut state_map, &copymap, parents.clone(), now)
                 .unwrap();
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -5,6 +5,7 @@ 
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
+use crate::dirstate::parsers::Timestamp;
 use crate::errors::HgError;
 use crate::revlog::node::NULL_NODE;
 use crate::{
@@ -22,7 +23,6 @@ 
 use std::convert::TryInto;
 use std::iter::FromIterator;
 use std::ops::Deref;
-use std::time::Duration;
 
 pub type FileFoldMap = FastHashMap<HgPathBuf, HgPathBuf>;
 
@@ -389,7 +389,7 @@ 
     pub fn pack(
         &mut self,
         parents: DirstateParents,
-        now: Duration,
+        now: Timestamp,
     ) -> Result<Vec<u8>, DirstateError> {
         let packed =
             pack_dirstate(&mut self.state_map, &self.copy_map, parents, now)?;