Patchwork D8432: dirstate: convert _checkexec to a bool when calling rustmod.status

login
register
mail settings
Submitter phabricator
Date April 15, 2020, 8:58 p.m.
Message ID <differential-rev-PHID-DREV-bunau75w4jfzu4ocrpe3-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46137/
State Superseded
Headers show

Comments

phabricator - April 15, 2020, 8:58 p.m.
mplamann created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  posix.checkexec can return True, False, or None. The rust status
  implementation expects a boolean, so convert to bool before passing
  _checkexec into rust.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS




To: mplamann, #hg-reviewers
Cc: mercurial-devel
phabricator - April 15, 2020, 9:38 p.m.
marmoute added a comment.


  That looks valid, but can we get a fix the assignment too ?

REPOSITORY
  rHG Mercurial

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

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

To: mplamann, #hg-reviewers
Cc: marmoute, mercurial-devel
phabricator - April 16, 2020, 8:56 p.m.
mplamann added a comment.


  What do you mean by fix the assignment?

REPOSITORY
  rHG Mercurial

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

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

To: mplamann, #hg-reviewers
Cc: marmoute, mercurial-devel
phabricator - April 16, 2020, 9:21 p.m.
marmoute added a comment.


  In D8432#126079 <https://phab.mercurial-scm.org/D8432#126079>, @mplamann wrote:
  
  > What do you mean by fix the assignment?
  
  thre are some code somewhere putting a None in this attribute. given you issue you face. It seems reasonable to make that code set a False instead.
  
  In practice this means doing this instead
  
    diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
    --- a/mercurial/dirstate.py
    +++ b/mercurial/dirstate.py
    @@ -180,7 +180,7 @@ class dirstate(object):
     
         @propertycache
         def _checkexec(self):
    -        return util.checkexec(self._root)
    +        return bool(util.checkexec(self._root))
     
         @propertycache
         def _checkcase(self):
  
  (we could also consider going up to the "plateform" file, but this seems a good enough spot.

REPOSITORY
  rHG Mercurial

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

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

To: mplamann, #hg-reviewers
Cc: marmoute, mercurial-devel
phabricator - April 20, 2020, 7:48 a.m.
Alphare added a comment.
Alphare accepted this revision.


  Thanks for the fix, I probably forgot about this because I remember writing a similar fix.

REPOSITORY
  rHG Mercurial

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

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

To: mplamann, #hg-reviewers, marmoute, Alphare
Cc: Alphare, marmoute, mercurial-devel

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -1112,7 +1112,7 @@ 
             matcher,
             self._rootdir,
             self._ignorefiles(),
-            self._checkexec,
+            bool(self._checkexec),
             self._lastnormaltime,
             bool(list_clean),
             bool(list_ignored),