This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11728

dirstate: use a single closure for get_flags
ClosedPublic

Authored by marmoute on Oct 28 2021, 5:14 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG91f07430db8c: dirstate: use a single closure for get_flags
Summary

The previous code was overlooking fallback when neither symlink not exec was
supported.

The number of "variants" is getting too high, so I am consolidating this in a
single closure that should be easier to maintains.

This also ensure that fallback flags are always taken into account.

(they are not user code yet, but small experimentation shown that the feature
was working as intended.)

A a small side effect we need to check for symlink support more lazily and this
show up in the test in a couple of places.

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

marmoute created this revision.Oct 28 2021, 5:14 PM
Alphare added a subscriber: Alphare.EditedOct 29 2021, 3:43 AM

The code looks good, but I'd prefer if there were a small docstring explaining what the two fallback mechanisms (e.g. the dirstatev2 one and the manifest one) are.

marmoute added a comment.Oct 29 2021, 4:13 AM
In D11728#179984, @Alphare wrote:

The code looks good, but I'd prefer if there were a small docstring explaining what the two fallback mechanism (e.g. the dirstatev2 one and the manifest one) are.

good idea, pushing this.

Alphare added a comment.Oct 29 2021, 8:36 AM

This is probably stuck somewhere isn't it?

marmoute added a comment.Oct 29 2021, 8:37 AM
In D11728#179990, @Alphare wrote:

This is probably stuck somewhere isn't it?

I refresh the flaky bits of the CI, and I hoping for the automatic refresh to work this time.

baymax updated this revision to Diff 31004.Oct 29 2021, 8:45 AM

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

Alphare accepted this revision.Oct 29 2021, 8:46 AM

Thanks. I'll amend the typos but this looks good.

This revision is now accepted and ready to land.Oct 29 2021, 8:46 AM
marmoute added a commit: rHG91f07430db8c: dirstate: use a single closure for get_flags.Oct 29 2021, 9:51 AM
Closed by commit rHG91f07430db8c: dirstate: use a single closure for get_flags (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstate.py (97 lines)
Mtests/test-share.t (4 lines)
Mtests/test-subrepo.t (4 lines)

Diff 31007

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 not (self._checklink and self._checkexec): """build a callable that returns flags associated with a filename
fallback = buildfallback()
def check_both(x): The information is extracted from three possible layers:
"""This platform supports symlinks and exec permissions""" 1. the file system if it supports the information
2. the "fallback" information stored in the dirstate if any
3. a more expensive mechanism inferring the flags from the parents.
"""
# small hack to cache the result of buildfallback()
fallback_func = []
def get_flags(x):
entry = None
fallback_value = None
try: try:
st = os.lstat(self._join(x)) st = os.lstat(self._join(x))
if util.statislink(st):
return b'l'
if util.statisexec(st):
return b'x'
except OSError: except OSError:
pass
return b''
def check_link(x):
"""This platform only supports symlinks"""
if os.path.islink(self._join(x)):
return b'l'
entry = self.get_entry(x)
if entry.has_fallback_exec:
if entry.fallback_exec:
return b'x'
elif b'x' in fallback(x):
return b'x'
return b'' return b''
def check_exec(x): if self._checklink:
"""This platform only supports exec permissions""" if util.statislink(st):
if b'l' in fallback(x):
return b'l' return b'l'
else:
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)): else:
return b'x' if not fallback_func:
return b'' fallback_func.append(buildfallback())
fallback_value = fallback_func[0](x)
if b'l' in fallback_value:
return b'l'
def check_fallback(x): if self._checkexec:
"""This platform supports neither symlinks nor exec permissions, so if util.statisexec(st):
check the fallback in the dirstate if it exists, otherwise figure it return b'x'
out the more expensive way from the parents.""" else:
if entry is None:
entry = self.get_entry(x) entry = self.get_entry(x)
if entry.has_fallback_symlink:
if entry.fallback_symlink:
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: else:
if fallback_value is None:
if not fallback_func:
fallback_func.append(buildfallback())
fallback_value = fallback_func[0](x)
if b'x' in fallback_value:
return b'x'
return b'' return b''
return fallback(x)
if self._checklink and self._checkexec: return get_flags
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()

tests/test-share.t

$ hg branches $ hg branches
default 0:d3873e73d99e default 0:d3873e73d99e
$ hg tags $ hg tags
tip 0:d3873e73d99e tip 0:d3873e73d99e
$ test -d .hg/cache $ test -d .hg/cache
[1] [1]
$ ls -1 .hg/wcache || true $ ls -1 .hg/wcache || true
checkisexec (execbit !) checkisexec (execbit !)
checklink (symlink !) checklink (symlink no-rust !)
checklink-target (symlink !) checklink-target (symlink no-rust !)
manifestfulltextcache (reporevlogstore !) manifestfulltextcache (reporevlogstore !)
$ ls -1 ../repo1/.hg/cache $ ls -1 ../repo1/.hg/cache
branch2-served branch2-served
rbc-names-v1 rbc-names-v1
rbc-revs-v1 rbc-revs-v1
tags2-visible tags2-visible
Cloning a shared repo should pick up the full cache dir on the other hand. Cloning a shared repo should pick up the full cache dir on the other hand.

tests/test-subrepo.t

../shared/subrepo-2/.hg/cache/storehash ../shared/subrepo-2/.hg/cache/storehash
../shared/subrepo-2/.hg/cache/storehash/* (glob) ../shared/subrepo-2/.hg/cache/storehash/* (glob)
../shared/subrepo-2/.hg/dirstate ../shared/subrepo-2/.hg/dirstate
../shared/subrepo-2/.hg/hgrc ../shared/subrepo-2/.hg/hgrc
../shared/subrepo-2/.hg/requires ../shared/subrepo-2/.hg/requires
../shared/subrepo-2/.hg/sharedpath ../shared/subrepo-2/.hg/sharedpath
../shared/subrepo-2/.hg/wcache ../shared/subrepo-2/.hg/wcache
../shared/subrepo-2/.hg/wcache/checkisexec (execbit !) ../shared/subrepo-2/.hg/wcache/checkisexec (execbit !)
../shared/subrepo-2/.hg/wcache/checklink (symlink !) ../shared/subrepo-2/.hg/wcache/checklink (symlink no-rust !)
../shared/subrepo-2/.hg/wcache/checklink-target (symlink !) ../shared/subrepo-2/.hg/wcache/checklink-target (symlink no-rust !)
../shared/subrepo-2/.hg/wcache/manifestfulltextcache (reporevlogstore !) ../shared/subrepo-2/.hg/wcache/manifestfulltextcache (reporevlogstore !)
../shared/subrepo-2/file ../shared/subrepo-2/file
$ hg -R ../shared in $ hg -R ../shared in
abort: repository default not found abort: repository default not found
[255] [255]
$ hg -R ../shared/subrepo-2 showconfig paths $ hg -R ../shared/subrepo-2 showconfig paths
paths.default=$TESTTMP/subrepo-status/subrepo-2 paths.default=$TESTTMP/subrepo-status/subrepo-2
$ hg -R ../shared/subrepo-1 sum --remote $ hg -R ../shared/subrepo-1 sum --remote