( )⚙ D11715 dirstate: group return logic and clarify each function in flagfunc

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11715

dirstate: group return logic and clarify each function in flagfunc
ClosedPublic

Authored by Alphare on Oct 21 2021, 3:42 AM.

Details

Reviewers
spectral
Group Reviewers
hg-reviewers
Commits
rHG0d6a099bba58: dirstate: group return logic and clarify each function in flagfunc
Summary

As suggested by spectral during review of my fix of the missing return, this
makes it more obvious that all cases are covered and improves readability of
the functions with new names and a small docstring.

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

Alphare created this revision.Oct 21 2021, 3:42 AM
baymax updated this revision to Diff 30977.Oct 21 2021, 1:51 PM

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

spectral accepted this revision.Oct 21 2021, 7:50 PM
mharbison72 added a subscriber: mharbison72.Oct 24 2021, 12:29 AM

Now that the fallback function is always being built- do the context.manifest(), dirstate.copies(), etc that get run in context._buildflagfunc() always have an acceptably low overhead?

spectral added a comment.Oct 24 2021, 2:20 AM
In D11715#179846, @mharbison72 wrote:

Now that the fallback function is always being built

I don't think this is the case? It should be gated on the same conditions it was before.

mharbison72 added a comment.Oct 24 2021, 7:14 PM
In D11715#179859, @spectral wrote:
In D11715#179846, @mharbison72 wrote:

Now that the fallback function is always being built

I don't think this is the case? It should be gated on the same conditions it was before.

Ah, right. It was a late night reviewing stuff...

Alphare added a commit: rHG0d6a099bba58: dirstate: group return logic and clarify each function in flagfunc.Oct 24 2021, 8:01 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0d6a099bba58: dirstate: group return logic and clarify each function in flagfunc (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstate.py (107 lines)

Diff 30984

mercurial/dirstate.py

return not util.fscasesensitive(self._join(b'.hg')) return not util.fscasesensitive(self._join(b'.hg'))
def _join(self, f): def _join(self, f):
# much faster than os.path.join() # much faster than os.path.join()
# it's safe because f is always a relative path # it's safe because f is always a relative path
return self._rootdir + f return self._rootdir + f
def flagfunc(self, buildfallback): def flagfunc(self, buildfallback):
if self._checklink and self._checkexec: if not (self._checklink and self._checkexec):
fallback = buildfallback()
def f(x): def check_both(x):
"""This platform supports symlinks and exec permissions"""
try: try:
st = os.lstat(self._join(x)) st = os.lstat(self._join(x))
if util.statislink(st): if util.statislink(st):
return b'l' return b'l'
if util.statisexec(st): if util.statisexec(st):
return b'x' return b'x'
except OSError: except OSError:
pass pass
return b'' return b''
return f def check_link(x):
"""This platform only supports symlinks"""
fallback = buildfallback()
if self._checklink:
def f(x):
if os.path.islink(self._join(x)): if os.path.islink(self._join(x)):
return b'l' return b'l'
entry = self.get_entry(x) entry = self.get_entry(x)
if entry.has_fallback_exec: if entry.has_fallback_exec:
if entry.fallback_exec: if entry.fallback_exec:
return b'x' return b'x'
elif b'x' in fallback(x): elif b'x' in fallback(x):
return b'x' return b'x'
return b'' return b''
return f def check_exec(x):
if self._checkexec: """This platform only supports exec permissions"""
def f(x):
if b'l' in fallback(x): if b'l' in fallback(x):
return b'l' return b'l'
entry = self.get_entry(x) entry = self.get_entry(x)
if entry.has_fallback_symlink: if entry.has_fallback_symlink:
if entry.fallback_symlink: if entry.fallback_symlink:
return b'l' return b'l'
if util.isexec(self._join(x)): if util.isexec(self._join(x)):
return b'x' return b'x'
return b'' return b''
return f def check_fallback(x):
else: """This platform supports neither symlinks nor exec permissions, so
check the fallback in the dirstate if it exists, otherwise figure it
def f(x): out the more expensive way from the parents."""
entry = self.get_entry(x) entry = self.get_entry(x)
if entry.has_fallback_symlink: if entry.has_fallback_symlink:
if entry.fallback_symlink: if entry.fallback_symlink:
return b'l' return b'l'
if entry.has_fallback_exec: if entry.has_fallback_exec:
if entry.fallback_exec: if entry.fallback_exec:
return b'x' return b'x'
elif entry.has_fallback_symlink: elif entry.has_fallback_symlink:
return b'' return b''
return fallback(x) return fallback(x)
return f if self._checklink and self._checkexec:
return check_both
elif self._checklink:
return check_link
elif self._checkexec:
return check_exec
else:
return check_fallback
@propertycache @propertycache
def _cwd(self): def _cwd(self):
# internal config: ui.forcecwd # internal config: ui.forcecwd
forcecwd = self._ui.config(b'ui', b'forcecwd') forcecwd = self._ui.config(b'ui', b'forcecwd')
if forcecwd: if forcecwd:
return forcecwd return forcecwd
return encoding.getcwd() return encoding.getcwd()