| durin42 | |
| martinvonz |
| hg-reviewers |
In upcoming patches, to implement Share Safe plan we will be introducing
requires file in store. We need to route all callers to a single function
to check for a share-safe requirement and if present, write requirements to
.hg/store/requires instead.
After this patch, callers directly calling scmutil.writerequires() are only
those where we don't have the repo object, for example when initializing
the repository object itself.
| No Linters Available |
| No Unit Test Coverage |
"callers directly calling scmutil.writerequires() are once
where" Do you mean "only" ?
The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
In D8631#129003, @marmoute wrote:"callers directly calling scmutil.writerequires() are once
where" Do you mean "only" ?
Yes.
The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
There are some places where we don't have a repo object. So those we route to scmutil.writerequires which accepts a vfs. Did you mean updating scmutil.writerequires or updating the new method I added?
In D8631#129129, @pulkit wrote:In D8631#129003, @marmoute wrote:"callers directly calling scmutil.writerequires() are once
where" Do you mean "only" ?Yes.
The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
There are some places where we don't have a repo object. So those we route to scmutil.writerequires which accepts a vfs. Did you mean updating scmutil.writerequires or updating the new method I added?
what I mean is "not having a new method on repo"
In D8631#129134, @marmoute wrote:In D8631#129129, @pulkit wrote:In D8631#129003, @marmoute wrote:"callers directly calling scmutil.writerequires() are once
where" Do you mean "only" ?Yes.
The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
There are some places where we don't have a repo object. So those we route to scmutil.writerequires which accepts a vfs. Did you mean updating scmutil.writerequires or updating the new method I added?
what I mean is "not having a new method on repo"
I tried that. It looks a bit ugly because that function will then call repo._writerequires().
In D8631#129152, @pulkit wrote:In D8631#129134, @marmoute wrote:In D8631#129129, @pulkit wrote:In D8631#129003, @marmoute wrote:"callers directly calling scmutil.writerequires() are once
where" Do you mean "only" ?Yes.
The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
There are some places where we don't have a repo object. So those we route to scmutil.writerequires which accepts a vfs. Did you mean updating scmutil.writerequires or updating the new method I added?
what I mean is "not having a new method on repo"
I tried that. It looks a bit ugly because that function will then call repo._writerequires().
I don't get it, why do we need to have a repo method at all ?
In D8631#129153, @marmoute wrote:In D8631#129152, @pulkit wrote:In D8631#129134, @marmoute wrote:In D8631#129129, @pulkit wrote:In D8631#129003, @marmoute wrote:"callers directly calling scmutil.writerequires() are once
where" Do you mean "only" ?Yes.
The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
There are some places where we don't have a repo object. So those we route to scmutil.writerequires which accepts a vfs. Did you mean updating scmutil.writerequires or updating the new method I added?
what I mean is "not having a new method on repo"
I tried that. It looks a bit ugly because that function will then call repo._writerequires().
I don't get it, why do we need to have a repo method at all ?
The patch was updated to not have a method on repo.
| Path | Packages | |||
|---|---|---|---|---|
| M | mercurial/interfaces/repository.py (3 lines) | |||
| M | mercurial/localrepo.py (4 lines) | |||
| M | mercurial/sparse.py (7 lines) | |||
| M | mercurial/upgrade.py (7 lines) |
| Commit | Parents | Author | Summary | Date |
|---|---|---|---|---|
| ed3b1f592da9 | a4438263b228 | Pulkit Goyal | Apr 14 2020, 7:13 AM |
| pass | pass | ||||
| def debugwireargs(one, two, three=None, four=None, five=None): | def debugwireargs(one, two, three=None, four=None, five=None): | ||||
| pass | pass | ||||
| def savecommitmessage(text): | def savecommitmessage(text): | ||||
| pass | pass | ||||
| def writerequirements(requirements): | |||||
| """ writes current repository requirements """ | |||||
| class completelocalrepository( | class completelocalrepository( | ||||
| ilocalrepositorymain, ilocalrepositoryfilestorage | ilocalrepositorymain, ilocalrepositoryfilestorage | ||||
| ): | ): | ||||
| """Complete interface for a local repository.""" | """Complete interface for a local repository.""" | ||||
| class iwireprotocolcommandcacher(interfaceutil.Interface): | class iwireprotocolcommandcacher(interfaceutil.Interface): | ||||
| if self.ui.configbool(b'experimental', b'bundle2-advertise'): | if self.ui.configbool(b'experimental', b'bundle2-advertise'): | ||||
| caps = set(caps) | caps = set(caps) | ||||
| capsblob = bundle2.encodecaps( | capsblob = bundle2.encodecaps( | ||||
| bundle2.getrepocaps(self, role=b'client') | bundle2.getrepocaps(self, role=b'client') | ||||
| ) | ) | ||||
| caps.add(b'bundle2=' + urlreq.quote(capsblob)) | caps.add(b'bundle2=' + urlreq.quote(capsblob)) | ||||
| return caps | return caps | ||||
| def writerequirements(self, requirements): | |||||
| self.requirements = requirements | |||||
| self._writerequirements() | |||||
| def _writerequirements(self): | def _writerequirements(self): | ||||
| scmutil.writerequires(self.vfs, self.requirements) | scmutil.writerequires(self.vfs, self.requirements) | ||||
| # Don't cache auditor/nofsauditor, or you'll end up with reference cycle: | # Don't cache auditor/nofsauditor, or you'll end up with reference cycle: | ||||
| # self -> auditor -> self._checknested -> self | # self -> auditor -> self._checknested -> self | ||||
| @property | @property | ||||
| def auditor(self): | def auditor(self): | ||||
| ) | ) | ||||
| from . import ( | from . import ( | ||||
| error, | error, | ||||
| match as matchmod, | match as matchmod, | ||||
| merge as mergemod, | merge as mergemod, | ||||
| mergestate as mergestatemod, | mergestate as mergestatemod, | ||||
| pathutil, | pathutil, | ||||
| pycompat, | pycompat, | ||||
| scmutil, | |||||
| util, | util, | ||||
| ) | ) | ||||
| from .utils import hashutil | from .utils import hashutil | ||||
| # Whether sparse features are enabled. This variable is intended to be | # Whether sparse features are enabled. This variable is intended to be | ||||
| # temporary to facilitate porting sparse to core. It should eventually be | # temporary to facilitate porting sparse to core. It should eventually be | ||||
| # a per-repo option, possibly a repo requirement. | # a per-repo option, possibly a repo requirement. | ||||
| enabled = False | enabled = False | ||||
| # because that will invalidate the matcher cache and force a | # because that will invalidate the matcher cache and force a | ||||
| # re-read. We ideally want to update the cached matcher on the | # re-read. We ideally want to update the cached matcher on the | ||||
| # repo instance then flush the new config to disk once wdir is | # repo instance then flush the new config to disk once wdir is | ||||
| # updated. But this requires massive rework to matcher() and its | # updated. But this requires massive rework to matcher() and its | ||||
| # consumers. | # consumers. | ||||
| if b'exp-sparse' in oldrequires and removing: | if b'exp-sparse' in oldrequires and removing: | ||||
| repo.requirements.discard(b'exp-sparse') | repo.requirements.discard(b'exp-sparse') | ||||
| scmutil.writerequires(repo.vfs, repo.requirements) | repo.writerequirements(repo.requirements) | ||||
| elif b'exp-sparse' not in oldrequires: | elif b'exp-sparse' not in oldrequires: | ||||
| repo.requirements.add(b'exp-sparse') | repo.requirements.add(b'exp-sparse') | ||||
| scmutil.writerequires(repo.vfs, repo.requirements) | repo.writerequirements(repo.requirements) | ||||
| try: | try: | ||||
| writeconfig(repo, includes, excludes, profiles) | writeconfig(repo, includes, excludes, profiles) | ||||
| return refreshwdir(repo, oldstatus, oldmatch, force=force) | return refreshwdir(repo, oldstatus, oldmatch, force=force) | ||||
| except Exception: | except Exception: | ||||
| if repo.requirements != oldrequires: | if repo.requirements != oldrequires: | ||||
| repo.requirements.clear() | repo.requirements.clear() | ||||
| repo.requirements |= oldrequires | repo.requirements |= oldrequires | ||||
| scmutil.writerequires(repo.vfs, repo.requirements) | repo.writerequirements(repo.requirements) | ||||
| writeconfig(repo, oldincludes, oldexcludes, oldprofiles) | writeconfig(repo, oldincludes, oldexcludes, oldprofiles) | ||||
| raise | raise | ||||
| def clearrules(repo, force=False): | def clearrules(repo, force=False): | ||||
| """Clears include/exclude rules from the sparse config. | """Clears include/exclude rules from the sparse config. | ||||
| The remaining sparse config only has profiles, if defined. The working | The remaining sparse config only has profiles, if defined. The working | ||||
| error, | error, | ||||
| filelog, | filelog, | ||||
| hg, | hg, | ||||
| localrepo, | localrepo, | ||||
| manifest, | manifest, | ||||
| metadata, | metadata, | ||||
| pycompat, | pycompat, | ||||
| revlog, | revlog, | ||||
| scmutil, | |||||
| util, | util, | ||||
| vfs as vfsmod, | vfs as vfsmod, | ||||
| ) | ) | ||||
| from .utils import compression | from .utils import compression | ||||
| # list of requirements that request a clone of all revlog if added/removed | # list of requirements that request a clone of all revlog if added/removed | ||||
| RECLONES_REQUIREMENTS = { | RECLONES_REQUIREMENTS = { | ||||
| # better than allowing a client to continue while the repository is in | # better than allowing a client to continue while the repository is in | ||||
| # an inconsistent state. | # an inconsistent state. | ||||
| ui.status( | ui.status( | ||||
| _( | _( | ||||
| b'marking source repository as being upgraded; clients will be ' | b'marking source repository as being upgraded; clients will be ' | ||||
| b'unable to read from repository\n' | b'unable to read from repository\n' | ||||
| ) | ) | ||||
| ) | ) | ||||
| scmutil.writerequires( | srcrepo.writerequirements(srcrepo.requirements | {b'upgradeinprogress'}) | ||||
| srcrepo.vfs, srcrepo.requirements | {b'upgradeinprogress'} | |||||
| ) | |||||
| ui.status(_(b'starting in-place swap of repository data\n')) | ui.status(_(b'starting in-place swap of repository data\n')) | ||||
| ui.status(_(b'replaced files will be backed up at %s\n') % backuppath) | ui.status(_(b'replaced files will be backed up at %s\n') % backuppath) | ||||
| # Now swap in the new store directory. Doing it as a rename should make | # Now swap in the new store directory. Doing it as a rename should make | ||||
| # the operation nearly instantaneous and atomic (at least in well-behaved | # the operation nearly instantaneous and atomic (at least in well-behaved | ||||
| # environments). | # environments). | ||||
| ui.status(_(b'replacing store...\n')) | ui.status(_(b'replacing store...\n')) | ||||
| # We first write the requirements file. Any new requirements will lock | # We first write the requirements file. Any new requirements will lock | ||||
| # out legacy clients. | # out legacy clients. | ||||
| ui.status( | ui.status( | ||||
| _( | _( | ||||
| b'finalizing requirements file and making repository readable ' | b'finalizing requirements file and making repository readable ' | ||||
| b'again\n' | b'again\n' | ||||
| ) | ) | ||||
| ) | ) | ||||
| scmutil.writerequires(srcrepo.vfs, requirements) | srcrepo.writerequirements(requirements) | ||||
| # The lock file from the old store won't be removed because nothing has a | # The lock file from the old store won't be removed because nothing has a | ||||
| # reference to its new location. So clean it up manually. Alternatively, we | # reference to its new location. So clean it up manually. Alternatively, we | ||||
| # could update srcrepo.svfs and other variables to point to the new | # could update srcrepo.svfs and other variables to point to the new | ||||
| # location. This is simpler. | # location. This is simpler. | ||||
| backupvfs.unlink(b'store/lock') | backupvfs.unlink(b'store/lock') | ||||
| return backuppath | return backuppath | ||||