This is an archive of the discontinued Mercurial Phabricator instance.

rust/hg-cpython/src/dirstate/dirstate_map.rs
140–142	This strongly suggests that `DirstateMapMethods::add_file` should no longer take a `DirstateEntry` parameter. Let’s refactor that later together with its boolean parameters.

marmoute added inline comments.Jul 5 2021, 4:43 AM

rust/hg-cpython/src/dirstate/dirstate_map.rs
140–142	Yep, we can build something better dirstatemap side and that should lead use to something simpler on the hg-core side too.

pulkit added a subscriber: pulkit.Jul 6 2021, 12:47 PM

pulkit added inline comments.

mercurial/dirstatemap.py
184	Hm, is this an intended change?

marmoute added inline comments.Jul 6 2021, 12:52 PM

mercurial/dirstatemap.py
184	Yes, since we dropped the argument we still need it defined in all code path. That code path is the the 'n' case.
rust/hg-core/src/dirstate/dirstate_map.rs
102	See the similar change in the rust code.

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

pulkit accepted this revision.Jul 6 2021, 2:50 PM

This revision is now accepted and ready to land.Jul 6 2021, 2:50 PM

marmoute added a commit: rHGa1745a292885: dirstate: drop `state` to `_addpath`.Jul 6 2021, 3:12 PM

Closed by commit rHGa1745a292885: dirstate: drop `state` to `_addpath` (authored by marmoute). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/dirstate.py (4 lines)
M			mercurial/dirstatemap.py (5 lines)
M			rust/hg-core/src/dirstate/dirstate_map.rs (1 line)
M			rust/hg-core/src/dirstate_tree/dirstate_map.rs (1 line)
M			rust/hg-cpython/src/dirstate/dirstate_map.rs (14 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	28793		Jul 4 2021, 5:51 PM	★	★
Diff 2	28874		Jul 6 2021, 2:09 PM	★	★
Diff 3	28911	rHGa1745a29288502a35d8600c86cf13415bf3209cc	Jul 3 2021, 7:59 PM	★	★

Commit	Parents	Author	Summary	Date
49c23564efde	6462fea59215	Pierre-Yves David		Jul 3 2021, 7:59 PM

Status	Author	Revision
Closed	marmoute	D11077 mergestate: use dirstate.update_file API in recordupdates()
Closed	marmoute	D11076 dirstate: add dedicated function for updating data of a file
Closed	marmoute	D11075 dirstate: add a `update_file` function
Closed	marmoute	D11074 resolve: use the `parentchange` context manager to apply merge action
Closed	marmoute	D11033 sparse: adjust the temporary includes within a `parentchange` context
Closed	marmoute	D11032 amend: adjust the dirstate within a `parentchange` context
Closed	marmoute	D11031 dirstate: use the right internal API in a test script
Closed	marmoute	D11029 sparse: clear rules in the context of a `parentchanges` context
Closed	marmoute	D11028 sparse: make sure we adjust the dirstate at the same time as the parent
Closed	marmoute	D11027 narrow: update narrow spec within a dirstate.parentchange context
Closed	marmoute	D11026 revert: use `set_untracked` instead of `drop` when applicable
Closed	marmoute	D11022 revert: use `set_untracked` when performing a revert
Closed	marmoute	D11021 mq: use `set_untracked` in `qrename`
Closed	marmoute	D11020 context: use `dirstate.set_untracked` in `context.forget`
Closed	marmoute	D11019 dirstate: add a `set_untracked` method for "hg remove"-like usage
Closed	marmoute	D11018 mq: use `set_tracked` in `qrename`
Closed	marmoute	D11017 mq: update the dirstate and its parent within a `parentchange` context
Closed	marmoute	D11016 context: use `dirstate.set_tracked` for `revert`
Closed	marmoute	D11015 context: use `dirstate.set_tracked` in context.copy
Closed	marmoute	D11014 context: use `dirstate.set_tracked` in `context.add`
Closed	marmoute	D11013 dirstate: add a `set_tracked` method for "hg add"-like usage
Closed	marmoute	D11012 dirstate: add a function to update tracking status while "moving" parents
Closed	marmoute	D11023 dirstate: introduce an internal `_drop` method
Closed	marmoute	D11011 dirstate: introduce an internal `_remove` method
Closed	marmoute	D11010 dirstate: introduce an internal `_add` method
Closed	marmoute	D10993 dirstatenonnormalcheck: fix some bytes formating on python3
Closed	marmoute	D10992 dirstate-item: deprecate tuple access on the class
Closed	marmoute	D10935 dirstate: document the dirstatetuple content
Closed	marmoute	D10982 dirstate-item: rename the class to DirstateItem
Closed	marmoute	D10981 dirstate-entry: add a `mtime` property
Closed	marmoute	D10980 dirstate-entry: add a `size` property
Closed	marmoute	D10979 dirstate-entry: add a `mode` property
Closed	marmoute	D10978 dirstate: no longer pass `oldstate` to the `dropfile`
Closed	marmoute	D10987 dirstate-item: use the properties in fakedirstatewritetime
Closed	marmoute	D10986 dirstate-item: use the properties in dirstatemap
Closed	marmoute	D10985 dirstate-item: use the properties in pathutil
Closed	marmoute	D10984 dirstate-item: use need_delay when packing dirstate
Closed	marmoute	D10991 dirstate-item: use the properties in `contrib/dirstatenonnormalcheck.py`
Closed	marmoute	D10990 dirstate-item: use the properties in largefiles
Closed	marmoute	D10989 dirstate-item: use the `state` property in debugpathcomplete
Closed	marmoute	D10988 dirstate-item: use the v1_serialization method in debugstate
Closed	marmoute	D10983 dirstate-item: also build DistateItem in dirstate.directories()
Closed	marmoute	D10977 dirstate: drop the last explicit `state` usage in the module
Closed	marmoute	D10976 dirstate: drop last explicite `state` usage in status
Closed	marmoute	D10975 dirstate: reorder "state" checking conditional
Closed	marmoute	D10974 dirstate-entry: add a `need_delay` method
Closed	marmoute	D10973 dirstate-entry: add a `tracked` property
Closed	marmoute	D10972 dirstate: drop `state` to `_addpath`
Closed	marmoute	D10971 dirstate: infer the 'n' state from `from_p2`
Closed	marmoute	D10970 dirstate: infer the 'n' state from `possibly_dirty`
Closed	marmoute	D10969 dirstate: use a `merged` parameter to _addpath
Closed	marmoute	D10968 dirstate-entry: add a `added` property
Closed	marmoute	D10967 dirstate: use a `added` parameter to _addpath
Closed	marmoute	D10966 dirstate: no longer pass the `oldstate` value to the dirstatemap
Closed	marmoute	D10965 dirstate: stop using `oldstate` in `dirstate._addpath`
Closed	marmoute	D10964 dirstate: drop the now unused magic constants for the dirstate module
Closed	marmoute	D10963 dirstate: move the handling of special case within the dirstatemap
Closed	marmoute	D10962 rust: bump rust-cpython version for 0.6.0
Closed	marmoute	D10961 rust-dirstate: make the MTIME_UNSET public
Closed	marmoute	D10960 rust-dirstatemap: expand the wrapping code a bit
Closed	marmoute	D10959 dirstate-entry: restrict `from_p2` property to tracked file
Closed	marmoute	D10958 dirstate-entry: `merged_removed` and `from_p2_removed` properties
Closed	marmoute	D10957 dirstate-entry: add a `removed` property
Closed	marmoute	D10956 dirstate-entry: add a `from_p2` property
Closed	marmoute	D10955 dirstate-entry: add a `merged` property
Closed	marmoute	D10954 dirstate-entry: add a `state` property (and use it)
Closed	marmoute	D10953 dirstate: move most of the `remove` logic with dirstatemap `removefile`
Closed	marmoute	D10952 dirstate: add a `in_merge` property
Closed	marmoute	D10951 dirstate-entry: introduce dedicated accessors for v1 serialization
Closed	marmoute	D10950 dirstate-entry: goes through the `dirstatetuple` constructor in all cases
Closed	marmoute	D10949 dirstate-entry: turn dirstate tuple into a real object (like in C)
Closed	marmoute	D10934 dirstate: split dirstatemap in its own file
Closed	marmoute	D10933 dirstate: explicitely deal with the `added` case in `_addpath`
Closed	marmoute	D10932 dirstate: add an explicit `possibly_dirty` parameter to `_addpath`
Closed	marmoute	D10931 dirstate: add an explicit `from_p2` parameter to `_addpath`
Closed	marmoute	D10930 dirstate: add default value to _addpath
Closed	marmoute	D10929 dirstate: move the _rangemask filtering closer to its storage
Closed	marmoute	D10928 dirstate: introduce a symbolic constant for the AMBIGUOUS_TIME marker
Closed	marmoute	D10927 dirstate: introduce a symbolic constant for the NONNORMAL marker
Closed	marmoute	D10926 dirstate: introduce a symbolic constant for the FROM_P2 marker
Closed	marmoute	D10925 dirstate: split a not-so-one-liner
Closed	marmoute	D10924 dirstate: split a not-so-one-liner
Closed	marmoute	D10923 dirstate: split a not-so-one-liner

Diff 28874

mercurial/dirstate.py

	return self._map.copymap.get(file, None)			return self._map.copymap.get(file, None)

	def copies(self):			def copies(self):
	return self._map.copymap			return self._map.copymap

	def _addpath(			def _addpath(
	self,			self,
	f,			f,
	state=None,
	mode=0,			mode=0,
	size=None,			size=None,
	mtime=None,			mtime=None,
	added=False,			added=False,
	merged=False,			merged=False,
	from_p2=False,			from_p2=False,
	possibly_dirty=False,			possibly_dirty=False,
	):			):
	if entry is not None and not entry.removed:			if entry is not None and not entry.removed:
	msg = _(b'file %r in dirstate clashes with %r')			msg = _(b'file %r in dirstate clashes with %r')
	msg %= (pycompat.bytestr(d), pycompat.bytestr(f))			msg %= (pycompat.bytestr(d), pycompat.bytestr(f))
	raise error.Abort(msg)			raise error.Abort(msg)
	self._dirty = True			self._dirty = True
	self._updatedfiles.add(f)			self._updatedfiles.add(f)
	self._map.addfile(			self._map.addfile(
	f,			f,
	state=state,
	mode=mode,			mode=mode,
	size=size,			size=size,
	mtime=mtime,			mtime=mtime,
	added=added,			added=added,
	merged=merged,			merged=merged,
	from_p2=from_p2,			from_p2=from_p2,
	possibly_dirty=possibly_dirty,			possibly_dirty=possibly_dirty,
	)			)
	moment where the file was determined to be clean and now."""			moment where the file was determined to be clean and now."""
	if parentfiledata:			if parentfiledata:
	(mode, size, mtime) = parentfiledata			(mode, size, mtime) = parentfiledata
	else:			else:
	s = os.lstat(self._join(f))			s = os.lstat(self._join(f))
	mode = s.st_mode			mode = s.st_mode
	size = s.st_size			size = s.st_size
	mtime = s[stat.ST_MTIME]			mtime = s[stat.ST_MTIME]
	self._addpath(f, b'n', mode, size, mtime)			self._addpath(f, mode=mode, size=size, mtime=mtime)
	self._map.copymap.pop(f, None)			self._map.copymap.pop(f, None)
	if f in self._map.nonnormalset:			if f in self._map.nonnormalset:
	self._map.nonnormalset.remove(f)			self._map.nonnormalset.remove(f)
	if mtime > self._lastnormaltime:			if mtime > self._lastnormaltime:
	# Remember the most recent modification timeslot for status(),			# Remember the most recent modification timeslot for status(),
	# to make sure we won't miss future size-preserving file content			# to make sure we won't miss future size-preserving file content
	# modifications that happen within the same timeslot.			# modifications that happen within the same timeslot.
	self._lastnormaltime = mtime			self._lastnormaltime = mtime

mercurial/dirstatemap.py


	def preload(self):			def preload(self):
	"""Loads the underlying data, if it's not already loaded"""			"""Loads the underlying data, if it's not already loaded"""
	self._map			self._map

	def addfile(			def addfile(
	self,			self,
	f,			f,
	state=None,
	mode=0,			mode=0,
	size=None,			size=None,
	mtime=None,			mtime=None,
	added=False,			added=False,
	merged=False,			merged=False,
	from_p2=False,			from_p2=False,
	possibly_dirty=False,			possibly_dirty=False,
	):			):
	state = b'n'			state = b'n'
	size = FROM_P2			size = FROM_P2
	mtime = AMBIGUOUS_TIME			mtime = AMBIGUOUS_TIME
	elif possibly_dirty:			elif possibly_dirty:
	state = b'n'			state = b'n'
	size = NONNORMAL			size = NONNORMAL
	mtime = AMBIGUOUS_TIME			mtime = AMBIGUOUS_TIME
	else:			else:
	assert state != b'a'
	assert size != FROM_P2			assert size != FROM_P2
	assert size != NONNORMAL			assert size != NONNORMAL
				state = b'n'
				pulkitUnsubmitted Not Done Hm, is this an intended change? pulkit: Hm, is this an intended change?
				marmouteAuthorUnsubmitted Done Yes, since we dropped the argument we still need it defined in all code path. That code path is the the 'n' case. marmoute: Yes, since we dropped the argument we still need it defined in all code path. That code path is…
	size = size & rangemask			size = size & rangemask
	mtime = mtime & rangemask			mtime = mtime & rangemask
	assert state is not None			assert state is not None
	assert size is not None			assert size is not None
	assert mtime is not None			assert mtime is not None
	old_entry = self.get(f)			old_entry = self.get(f)
	if (			if (
	old_entry is None or old_entry.removed			old_entry is None or old_entry.removed
	b"experimental",			b"experimental",
	b"dirstate-tree.in-memory",			b"dirstate-tree.in-memory",
	False,			False,
	)			)

	def addfile(			def addfile(
	self,			self,
	f,			f,
	state=None,
	mode=0,			mode=0,
	size=None,			size=None,
	mtime=None,			mtime=None,
	added=False,			added=False,
	merged=False,			merged=False,
	from_p2=False,			from_p2=False,
	possibly_dirty=False,			possibly_dirty=False,
	):			):
	return self._rustmap.addfile(			return self._rustmap.addfile(
	f,			f,
	state,
	mode,			mode,
	size,			size,
	mtime,			mtime,
	added,			added,
	merged,			merged,
	from_p2,			from_p2,
	possibly_dirty,			possibly_dirty,
	)			)

rust/hg-core/src/dirstate/dirstate_map.rs

	entry.state = EntryState::Normal;			entry.state = EntryState::Normal;
	entry.size = SIZE_FROM_OTHER_PARENT;			entry.size = SIZE_FROM_OTHER_PARENT;
	entry.mtime = MTIME_UNSET;			entry.mtime = MTIME_UNSET;
	} else if possibly_dirty {			} else if possibly_dirty {
	entry.state = EntryState::Normal;			entry.state = EntryState::Normal;
	entry.size = SIZE_NON_NORMAL;			entry.size = SIZE_NON_NORMAL;
	entry.mtime = MTIME_UNSET;			entry.mtime = MTIME_UNSET;
	} else {			} else {
				entry.state = EntryState::Normal;
				marmouteAuthorUnsubmitted Done See the similar change in the rust code. marmoute: See the similar change in the rust code.
	entry.size = entry.size & V1_RANGEMASK;			entry.size = entry.size & V1_RANGEMASK;
	entry.mtime = entry.mtime & V1_RANGEMASK;			entry.mtime = entry.mtime & V1_RANGEMASK;
	}			}
	let old_state = match self.get(filename) {			let old_state = match self.get(filename) {
	Some(e) => e.state,			Some(e) => e.state,
	None => EntryState::Unknown,			None => EntryState::Unknown,
	};			};
	if old_state == EntryState::Unknown \|\| old_state == EntryState::Removed			if old_state == EntryState::Unknown \|\| old_state == EntryState::Removed

rust/hg-core/src/dirstate_tree/dirstate_map.rs

	entry.state = EntryState::Normal;			entry.state = EntryState::Normal;
	entry.size = SIZE_FROM_OTHER_PARENT;			entry.size = SIZE_FROM_OTHER_PARENT;
	entry.mtime = MTIME_UNSET;			entry.mtime = MTIME_UNSET;
	} else if possibly_dirty {			} else if possibly_dirty {
	entry.state = EntryState::Normal;			entry.state = EntryState::Normal;
	entry.size = SIZE_NON_NORMAL;			entry.size = SIZE_NON_NORMAL;
	entry.mtime = MTIME_UNSET;			entry.mtime = MTIME_UNSET;
	} else {			} else {
				entry.state = EntryState::Normal;
	entry.size = entry.size & V1_RANGEMASK;			entry.size = entry.size & V1_RANGEMASK;
	entry.mtime = entry.mtime & V1_RANGEMASK;			entry.mtime = entry.mtime & V1_RANGEMASK;
	}			}

	let old_state = match self.get(filename)? {			let old_state = match self.get(filename)? {
	Some(e) => e.state,			Some(e) => e.state,
	None => EntryState::Unknown,			None => EntryState::Unknown,
	};			};

rust/hg-cpython/src/dirstate/dirstate_map.rs

	},			},
	None => Ok(default)			None => Ok(default)
	}			}
	}			}

	def addfile(			def addfile(
	&self,			&self,
	f: PyObject,			f: PyObject,
	state: PyObject,
	mode: PyObject,			mode: PyObject,
	size: PyObject,			size: PyObject,
	mtime: PyObject,			mtime: PyObject,
	added: PyObject,			added: PyObject,
	merged: PyObject,			merged: PyObject,
	from_p2: PyObject,			from_p2: PyObject,
	possibly_dirty: PyObject,			possibly_dirty: PyObject,
	) -> PyResult<PyObject> {			) -> PyResult<PyObject> {
	let f = f.extract::<PyBytes>(py)?;			let f = f.extract::<PyBytes>(py)?;
	let filename = HgPath::new(f.data(py));			let filename = HgPath::new(f.data(py));
	let state = if state.is_none(py) {
	// Arbitrary default value
	EntryState::Normal
	} else {
	state.extract::<PyBytes>(py)?.data(py)[0]
	.try_into()
	.map_err(\|e: HgError\| {
	PyErr::new::<exc::ValueError, _>(py, e.to_string())
	})?
	};
	let mode = if mode.is_none(py) {			let mode = if mode.is_none(py) {
	// fallback default value			// fallback default value
	0			0
	} else {			} else {
	mode.extract(py)?			mode.extract(py)?
	};			};
	let size = if size.is_none(py) {			let size = if size.is_none(py) {
	// fallback default value			// fallback default value
	SIZE_NON_NORMAL			SIZE_NON_NORMAL
	} else {			} else {
	size.extract(py)?			size.extract(py)?
	};			};
	let mtime = if mtime.is_none(py) {			let mtime = if mtime.is_none(py) {
	// fallback default value			// fallback default value
	MTIME_UNSET			MTIME_UNSET
	} else {			} else {
	mtime.extract(py)?			mtime.extract(py)?
	};			};
	let entry = DirstateEntry {			let entry = DirstateEntry {
	state: state,			// XXX Arbitrary default value since the value is determined later
				state: EntryState::Normal,
	mode: mode,			mode: mode,
				SimonSapinUnsubmitted Not Done This strongly suggests that `DirstateMapMethods::add_file` should no longer take a `DirstateEntry` parameter. Let’s refactor that later together with its boolean parameters. SimonSapin: This strongly suggests that `DirstateMapMethods::add_file` should no longer take a…
				marmouteAuthorUnsubmitted Done Yep, we can build something better dirstatemap side and that should lead use to something simpler on the hg-core side too. marmoute: Yep, we can build something better dirstatemap side and that should lead use to something…
	size: size,			size: size,
	mtime: mtime,			mtime: mtime,
	};			};
	let added = added.extract::<PyBool>(py)?.is_true();			let added = added.extract::<PyBool>(py)?.is_true();
	let merged = merged.extract::<PyBool>(py)?.is_true();			let merged = merged.extract::<PyBool>(py)?.is_true();
	let from_p2 = from_p2.extract::<PyBool>(py)?.is_true();			let from_p2 = from_p2.extract::<PyBool>(py)?.is_true();
	let possibly_dirty = possibly_dirty.extract::<PyBool>(py)?.is_true();			let possibly_dirty = possibly_dirty.extract::<PyBool>(py)?.is_true();
	self.inner(py).borrow_mut().add_file(			self.inner(py).borrow_mut().add_file(

This is an archive of the discontinued Mercurial Phabricator instance.

dirstate: drop `state` to `_addpath`ClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List