This is an archive of the discontinued Mercurial Phabricator instance.

dirstate: force _checkexec to return a bool
ClosedPublic

Authored by mplamann on Apr 15 2020, 4:58 PM.

Details

Summary

posix.checkexec can return True, False, or None. The rust status
implementation expects a boolean, so make sure _checkexec returns a boolean.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mplamann created this revision.Apr 15 2020, 4:58 PM

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

What do you mean by fix the assignment?

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.

mplamann retitled this revision from dirstate: convert _checkexec to a bool when calling rustmod.status to dirstate: force _checkexec to return a bool.Apr 17 2020, 2:01 PM
mplamann edited the summary of this revision. (Show Details)
mplamann updated this revision to Diff 21154.
marmoute accepted this revision.Apr 17 2020, 2:09 PM
Alphare accepted this revision.Apr 20 2020, 3:48 AM
Alphare added a subscriber: Alphare.

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

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.