In D6731#98871, @pulkit wrote:

In such cases, I like the idea of having fix as two patches, first which demonstrates the bug and the second which fixes the bug. What do you think?

In D6479#98757, @navaneeth.suresh wrote:

In D6479#98737, @durin42 wrote:

The only thing I'm curious about really is why we have extrastorage and usualstorge. Can we get away with only one of those choices instead of having more options?

The ultimate goal is to store the mergestate info in the changeset extras only. But, now , it has no support to store the local version of the files which are stored in the mergestate. This lacks info when the user gets conflicts other than a usual hg merge.

In such cases, I like the idea of having fix as two patches, first which demonstrates the bug and the second which fixes the bug. What do you think?

In D6730#98852, @av6 wrote:

The changes in tests that this patch causes are actually already present in the D6709, so let's fold this patch into it as well.

The changes in tests that this patch causes are actually already present in the D6709, so let's fold this patch into it as well.

@pulkit This one is ready too.

+impl PySharedState {
+ pub fn borrow_mut<'a, T>(
+ &'a self,
+ py: Python<'a>,
+ pyrefmut: RefMut<'a, T>,
+ ) -> PyResult<PyRefMut<'a, T>> {
+ if self.mutably_borrowed.get() {
+ return Err(AlreadyBorrowed::new(
+ py,
+ "Cannot borrow mutably while there exists another \
+ mutable reference in a Python object",
+ ));
+ }
+ match self.leak_count.get() {
+ 0 => {
+ self.mutably_borrowed.replace(true);
+ Ok(PyRefMut::new(py, pyrefmut, self))
+ }
+ TODO
+ For now, this works differently than Python references
+ in the case of iterators.
+ Python does not complain when the data an iterator
+ points to is modified if the iterator is never used
+ afterwards.
+ Here, we are stricter than this by refusing to give a
+ mutable reference if it is already borrowed.
+ While the additional safety might be argued for, it
+ breaks valid programming patterns in Python and we need
+ to fix this issue down the line.
+ _ => Err(AlreadyBorrowed::new(
+ py,
+ "Cannot borrow mutably while there are \
+ immutable references in Python objects",
+ )),
+ }
+ }
+
+ / Return a reference to the wrapped data with an artificial static
+ / lifetime.
+ / We need to be protected by the GIL for thread-safety.
+ pub fn leak_immutable<T>(
+ &self,
+ py: Python,
+ data: &RefCell<T>,
+ ) -> PyResult<&'static T> {
+ if self.mutably_borrowed.get() {
+ return Err(AlreadyBorrowed::new(
+ py,
+ "Cannot borrow immutably while there is a \
+ mutable reference in Python objects",
+ ));
+ }
+ let ptr = data.as_ptr();
+ self.leak_count.replace(self.leak_count.get() + 1);
+ unsafe { Ok(&*ptr) }
+ }

+ if let Some(ref mut file_fold_map) = self.file_fold_map {
+ file_fold_map
+ .remove::<Vec<u8>>(filename.to_ascii_uppercase().as_ref());

+ def getdirs(&self) -> PyResult<Dirs> {
+ TODO don't copy, share the reference
+ self.inner(py).borrow_mut().set_dirs();
+ Dirs::from_inner(
+ py,
+ DirsMultiset::new(
+ DirsIterable::Dirstate(&self.inner(py).borrow()),
+ Some(EntryState::Removed),
+ ),
+ )
+ }
+ def getalldirs(&self) -> PyResult<Dirs> {
+ TODO don't copy, share the reference
+ self.inner(py).borrow_mut().set_all_dirs();
+ Dirs::from_inner(
+ py,
+ DirsMultiset::new(
+ DirsIterable::Dirstate(&self.inner(py).borrow()),
+ None,
+ ),
+ )
+ }

It looks fine to me, one thing that could be improved is the output format of list values. For example, progress.format: its default value is shown to be ['topic', 'bar', 'number', 'estimate'], but it doesn't match hg help config.progress.format (it says default: topic bar number estimate), and that is also not the format that users would need to use for the actual value in hgrc or using --config flag.

In D6479#98737, @durin42 wrote:

The only thing I'm curious about really is why we have extrastorage and usualstorge. Can we get away with only one of those choices instead of having more options?

Let's rename this to transplant --stop instead.

Looks good to me, but I want someone else to put eyes on it before it gets pushed.