D7521: amend: check for file modifications when updating dirstate (issue6233)

[spectral (Kyle Lippincott)]

spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, we called dirstate.normal(f), which would put information into the
  dirstate claiming that the file on disk is what it "should be" for the current
  checkout, and it would have the size and timestamp of the most recent
  modification to the file (which is not necessarily the one we just committed).
  If the file was modified while the commit message editor was open, we would 
put
  incorrect information into the dirstate.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/cmdutil.py
  tests/test-amend.t

CHANGE DETAILS

diff --git a/tests/test-amend.t b/tests/test-amend.t
--- a/tests/test-amend.t
+++ b/tests/test-amend.t
@@ -476,3 +476,35 @@
a |  2 +-
b |  2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
+
+Modifying a file while the editor is open can cause dirstate corruption
+(issue6233)
+
+  $ cd $TESTTMP
+  $ hg init modify-during-amend; cd modify-during-amend
+  $ echo r0 > foo; hg commit -qAm "r0"
+  $ echo alpha > foo; hg commit -qm "alpha"
+  $ echo beta >> foo
+  $ cat > $TESTTMP/sleepy_editor < #!/bin/bash
+  > echo hi > "\$1"
+  > sleep 3
+  > EOF
+  $ chmod +x $TESTTMP/sleepy_editor
+  $ HGEDITOR=$TESTTMP/sleepy_editor hg commit --amend &
+  $ sleep 1
+  $ echo delta >> foo
+  $ sleep 3
+  $ if (hg diff -c . | grep -q 'delta') || [[ -n "$(hg status)" ]]; then
+  >   echo "OK."
+  > else
+  >   echo "Bug detected. 'delta' is not part of the commit OR the wdir"
+  >   echo "Diff and status before rebuild:"
+  >   hg diff
+  >   hg status
+  >   hg debugrebuilddirstate
+  >   echo "Diff and status after rebuild:"
+  >   hg diff
+  >   hg status
+  > fi
+  OK.
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3054,11 +3054,13 @@
 # selectively update the dirstate only for the amended files.
 dirstate = repo.dirstate
 
-# Update the state of the files which were added and
-# and modified in the amend to "normal" in the dirstate.
+# Update the state of the files which were added and modified in the
+# amend to "normal" in the dirstate. We need to use "normallookup" 
since
+# the files may have changed since the command started; using "normal"
+# would mark them as clean but with uncommitted contents.
 normalfiles = set(wctx.modified() + wctx.added()) & filestoamend
 for f in normalfiles:
-dirstate.normal(f)
+dirstate.normallookup(f)
 
 # Update the state of files which were removed in the amend
 # to "removed" in the dirstate.



To: spectral, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7512: exchange: guard against method invocation on `b2caps=None` args

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D7512#110590 , @mharbison72 
wrote:
  
  > @marmoute?
  
  I will have a look.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7512/new/

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

To: mharbison72, #hg-reviewers, dlax, indygreg
Cc: marmoute, yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7459: lock: pass "success" boolean to _afterlock callbacks

[spectral (Kyle Lippincott)]

Closed by commit rHG4b065b019b4e: lock: pass success boolean to 
_afterlock callbacks (authored by spectral).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7459?vs=18248=18393

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7459/new/

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

AFFECTED FILES
  hgext/remotefilelog/__init__.py
  mercurial/bundle2.py
  mercurial/changegroup.py
  mercurial/localrepo.py
  mercurial/lock.py
  mercurial/manifest.py
  tests/test-lock.py

CHANGE DETAILS

diff --git a/tests/test-lock.py b/tests/test-lock.py
--- a/tests/test-lock.py
+++ b/tests/test-lock.py
@@ -65,7 +65,7 @@
 def releasefn(self):
 self._releasecalled = True
 
-def postreleasefn(self):
+def postreleasefn(self, success):
 self._postreleasecalled = True
 
 def assertacquirecalled(self, called):
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1572,7 +1572,11 @@
 reporef = weakref.ref(repo)
 manifestrevlogref = weakref.ref(self)
 
-def persistmanifestcache():
+def persistmanifestcache(success):
+# Repo is in an unknown state, do not persist.
+if not success:
+return
+
 repo = reporef()
 self = manifestrevlogref()
 if repo is None or self is None:
diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -233,7 +233,8 @@
 return self
 
 def __exit__(self, exc_type, exc_value, exc_tb):
-self.release()
+success = all(a is None for a in (exc_type, exc_value, exc_tb))
+self.release(success=success)
 
 def __del__(self):
 if self.held:
@@ -408,7 +409,7 @@
 self.acquirefn()
 self._inherited = False
 
-def release(self):
+def release(self, success=True):
 """release the lock and execute callback function if any
 
 If the lock has been acquired multiple times, the actual release is
@@ -433,7 +434,7 @@
 # at all.
 if not self._parentheld:
 for callback in self.postrelease:
-callback()
+callback(success)
 # Prevent double usage and help clear cycles.
 self.postrelease = None
 
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2203,7 +2203,7 @@
 # fixes the function accumulation.
 hookargs = tr2.hookargs
 
-def hookfunc():
+def hookfunc(unused_success):
 repo = reporef()
 if hook.hashook(repo.ui, b'txnclose-bookmark'):
 bmchanges = sorted(tr.changes[b'bookmarks'].items())
@@ -2615,7 +2615,7 @@
 l.postrelease.append(callback)
 break
 else:  # no lock have been found.
-callback()
+callback(True)
 
 def lock(self, wait=True):
 '''Lock the repository store (.hg/store) and return a weak reference
@@ -2953,7 +2953,7 @@
 )
 raise
 
-def commithook():
+def commithook(unused_success):
 # hack for command that use a temporary commit (eg: histedit)
 # temporary commit got stripped before hook release
 if self.changelog.hasnode(ret):
@@ -3399,7 +3399,7 @@
 self.ui.debug(b'pushing key for "%s:%s"\n' % (namespace, key))
 ret = pushkey.push(self, namespace, key, old, new)
 
-def runhook():
+def runhook(unused_success):
 self.hook(
 b'pushkey',
 namespace=namespace,
diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -436,7 +436,7 @@
 
 if changesets > 0:
 
-def runhooks():
+def runhooks(unused_success):
 # These hooks run when the lock releases, not when the
 # transaction closes. So it's possible for the changelog
 # to have changed since we last saw it.
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -2372,7 +2372,7 @@
 
 if pushkeycompat:
 
-def runhook():
+def runhook(unused_success):
 for hookargs in allhooks:
 op.repo.hook(b'pushkey', **pycompat.strkwargs(hookargs))
 
diff --git a/hgext/remotefilelog/__init__.py b/hgext/remotefilelog/__init__.py
--- a/hgext/remotefilelog/__init__.py
+++ b/hgext/remotefilelog/__init__.py
@@ -1063,7 +1063,7 @@
 # update a revset with a date limit
 bgprefetchrevs = revdatelimit(ui, 

D7518: revlog: fix revset in reachableroots docstring

[quark (Jun Wu)]

Closed by commit rHG1a42f8451a92: revlog: fix revset in reachableroots 
docstring (authored by quark).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7518?vs=18390=18394

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7518/new/

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

AFFECTED FILES
  mercurial/revlog.py

CHANGE DETAILS

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1279,7 +1279,7 @@
 return bool(self.reachableroots(a, [b], [a], includepath=False))
 
 def reachableroots(self, minroot, heads, roots, includepath=False):
-"""return (heads(:: and ::))
+"""return (heads(::( and ::)))
 
 If includepath is True, return (::)."""
 try:



To: quark, indygreg, #hg-reviewers, pulkit
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7520: dateutil: correct default for Ymd in parsedate

[quark (Jun Wu)]

Closed by commit rHGaef7b91dba51: dateutil: correct default for Ymd in 
parsedate (authored by quark).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7520?vs=18392=18396

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7520/new/

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

AFFECTED FILES
  mercurial/utils/dateutil.py

CHANGE DETAILS

diff --git a/mercurial/utils/dateutil.py b/mercurial/utils/dateutil.py
--- a/mercurial/utils/dateutil.py
+++ b/mercurial/utils/dateutil.py
@@ -209,6 +209,8 @@
 True
 >>> tz == strtz
 True
+>>> parsedate(b'2000 UTC', formats=extendeddateformats)
+(946684800, 0)
 """
 if bias is None:
 bias = {}
@@ -244,7 +246,8 @@
 if part[0:1] in b"HMS":
 b = b"00"
 else:
-b = b"0"
+# year, month, and day start from 1
+b = b"1"
 
 # this piece is for matching the generic end to today's date
 n = datestr(now, b"%" + part[0:1])



To: quark, #hg-reviewers, pulkit
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7519: test-doctest: include dateutil

[quark (Jun Wu)]

Closed by commit rHG92518ca66c76: test-doctest: include dateutil (authored by 
quark).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7519?vs=18391=18395

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7519/new/

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

AFFECTED FILES
  tests/test-doctest.py

CHANGE DETAILS

diff --git a/tests/test-doctest.py b/tests/test-doctest.py
--- a/tests/test-doctest.py
+++ b/tests/test-doctest.py
@@ -82,6 +82,7 @@
 testmod('mercurial.url')
 testmod('mercurial.util')
 testmod('mercurial.util', testtarget='platform')
+testmod('mercurial.utils.dateutil')
 testmod('mercurial.utils.stringutil')
 testmod('hgext.convert.convcmd')
 testmod('hgext.convert.cvsps')



To: quark, #hg-reviewers, pulkit
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7512: exchange: guard against method invocation on `b2caps=None` args

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.
mharbison72 added a subscriber: marmoute.


  In D7512#110589 , @yuja wrote:
  
  >>   >>   """add a changegroup part to the requested bundle"""
  >>   >>
  >>   >> - if not kwargs.get('cg', True):
  >>   >>
  >>   >> +if not kwargs.get('cg', True) or not b2caps:
  >>   >>
  >>   >>   return
  >>   >
  >>   > Is it valid to call these functions with `b2caps=None`? I suspect it 
would
  >>   > be a bug or a data corruption.
  >>   The only caller I can find[1] will indeed pass something, even if it is 
`{}`.  I can change these to asserts if you want.
  >>   [1] 
https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448
  >
  > Well, I have no expertise around this module, so I have no idea which is
  > better. I just don't know if it's valid to return successfully if b2caps
  > is None.
  
  @marmoute?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7512/new/

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

To: mharbison72, #hg-reviewers, dlax, indygreg
Cc: marmoute, yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7512: exchange: guard against method invocation on `b2caps=None` args

[yuja (Yuya Nishihara)]

yuja added a comment.


  >   >>   """add a changegroup part to the requested bundle"""
  >   >>
  >   >> - if not kwargs.get('cg', True):
  >   >>
  >   >> +if not kwargs.get('cg', True) or not b2caps:
  >   >>
  >   >>   return
  >   >
  >   > Is it valid to call these functions with `b2caps=None`? I suspect it 
would
  >   > be a bug or a data corruption.
  >   The only caller I can find[1] will indeed pass something, even if it is 
`{}`.  I can change these to asserts if you want.
  >   [1] 
https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448
  
  Well, I have no expertise around this module, so I have no idea which is
  better. I just don't know if it's valid to return successfully if b2caps
  is None.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7512/new/

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

To: mharbison72, #hg-reviewers, dlax, indygreg
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D7512: exchange: guard against method invocation on `b2caps=None` args

[Yuya Nishihara]

>   >>   """add a changegroup part to the requested bundle"""
>   >>
>   >> - if not kwargs.get('cg', True):
>   >>
>   >> +if not kwargs.get('cg', True) or not b2caps:
>   >>
>   >>   return
>   >
>   > Is it valid to call these functions with `b2caps=None`? I suspect it would
>   > be a bug or a data corruption.
>   
>   The only caller I can find[1] will indeed pass something, even if it is 
> `{}`.  I can change these to asserts if you want.
>   
>   [1] 
> https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/exchange.py#l2448

Well, I have no expertise around this module, so I have no idea which is
better. I just don't know if it's valid to return successfully if b2caps
is None.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel