Wed, 12 Sep 2018 15:59:26 -0700 localrepo: extract resolving of opener options to standalone functions
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:59:26 -0700] rev 39700
localrepo: extract resolving of opener options to standalone functions Requirements and config options are converted into a dict which is available to the store vfs to consult. This is how storage options are communicated from the repo layer to the storage layer. Currently, we do that option resolution in a private method on the repo instance. And there is a single method doing that resolution. Opener options are logically specific to the storage backend they apply to. And, opener options may wish to influence how the repo object/type is constructed. So it makes sense to have more granular storage option resolution that occurs before the repo object is instantiated. This commit extracts the code for resolving opener options into new module-level functions. These functions are run before the repo instance is constructed. As part of the code move, we split the option resolution into generic and revlog-specific options. After this commit, we no longer add revlog-specific options to repos that don't have a revlog requirement. Some of these opener options and associated config options might make sense on alternate storage backends. We can always reuse config options and opener option names for other backends. But we shouldn't be passing opener options to storage backends that won't recognize them. I haven't done it here, but after this commit it should be possible for store backends to validate the set of opener options it receives. Because localrepository.openerreqs is no longer used after this commit, it has been removed. I'm not super thrilled about the code outside of localrepo that is adding requirements and updating opener options. We'll probably want to create a more formal API for that use case that constructs a new repo instance and poisons the old repo object. But this was a pre-existing issue and can be dealt with later. I have little doubt it will cause me troubles as I continue to refactor how repository objects are instantiated. .. api:: ``localrepository.openerreqs`` has been removed. Override ``localrepo.resolvestorevfsoptions()`` to add custom opener options. .. api:: ``localrepository._applyopenerreqs()`` has been removed. Use ``localrepo.resolvestorevfsoptions()`` to add custom opener options. Differential Revision: https://phab.mercurial-scm.org/D4576
Wed, 12 Sep 2018 15:17:47 -0700 localrepo: use boolean in opener options
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:17:47 -0700] rev 39699
localrepo: use boolean in opener options Not sure why we're using an integer for a flag value here. I'm pretty sure nothing relies on values being 1. While we're here, convert to a dict comprehension. Differential Revision: https://phab.mercurial-scm.org/D4575
Wed, 12 Sep 2018 15:07:27 -0700 localrepo: move store() from store module
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:07:27 -0700] rev 39698
localrepo: move store() from store module I want logic related to requirements handling to be in the localrepo module so it is all in one place. I would have loved to inline this logic. Unfortunately, statichttprepo also calls it. I didn't want to inline it twice. We could potentially refactor statichttppeer. But meh. Differential Revision: https://phab.mercurial-scm.org/D4574
Wed, 12 Sep 2018 15:05:51 -0700 localrepo: resolve store and cachevfs in makelocalrepository()
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:05:51 -0700] rev 39697
localrepo: resolve store and cachevfs in makelocalrepository() This is mostly a code move and refactor. One change is that we now explicitly look for requirements indicating a share is being used rather than blindly try to read from .hg/sharedpath. Requirements *should* be all that is necessary to dictate high-level behavior and I'm not sure why the previous code was doing what it was. The previous code has been in place since 87d1fd40f57e (authored in 2009). And the commit immediately after that (971e38a9344b) introduced ``hg.share()`` and always wrote the ``shared`` requirement. And as far as I can tell, every revision of ``hg.share()`` since has written either the ``shared`` or ``relshared`` requirement. So I'm pretty sure we don't need to maintain BC by always looking for and honoring the ``.hg/sharedpath`` file even if a requirement isn't present. .. bc:: A repository will no longer use shared storage if it has a ``.hg/sharedpath`` file but no entry in ``.hg/requires`` saying it is shared. This change should not have any end-user impact, as all shared repos should have a ``.hg/requires`` file indicating this. Differential Revision: https://phab.mercurial-scm.org/D4573
Wed, 12 Sep 2018 13:10:45 -0700 localrepo: document and test bug around opening shared repos
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 13:10:45 -0700] rev 39696
localrepo: document and test bug around opening shared repos As part of refactoring this code, I realized that we don't validate the requirements of a shared repository. This commit documents that next to the requirements validation code and adds a test demonstrating the buggy behavior. I'm not sure if I'll fix this. But it is definitely a bug that users could encounter, as LFS, narrow, and potentially other extensions dynamically add requirements on first use. One part of this I'm not sure about is how to handle loading the .hg/hgrc of the shared repo. We need to do that in order to load extensions. But we don't want that repo's hgrc to overwrite the current repo's. Differential Revision: https://phab.mercurial-scm.org/D4572
Wed, 12 Sep 2018 15:03:17 -0700 localrepo: move requirements reasonability testing to own function
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:03:17 -0700] rev 39695
localrepo: move requirements reasonability testing to own function Just because we know how to handle each listed requirement doesn't mean that set of requirements is reasonable. This commit introduces an extension-wrappable function to validate that a set of requirements makes sense. We could combine this with ensurerequirementsrecognized(). But I think having a line between basic membership testing and compatibility checking is more powerful as it will help differentiate between missing support and buggy behavior. Differential Revision: https://phab.mercurial-scm.org/D4571
Wed, 12 Sep 2018 15:47:24 -0700 statichttprepo: use new functions for requirements validation
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 15:47:24 -0700] rev 39694
statichttprepo: use new functions for requirements validation The new code in localrepo for requirements gathering and validation is more robust than scmutil.readrequires(). Let's port statichttprepo to it. Since scmutil.readrequires() is no longer used, it has been removed. It is possible extensions were monkeypatching this to supplement the set of supported requirements. But the proper way to do that is to register a featuresetupfuncs. I'm comfortable forcing the API break because featuresetupfuncs is more robust and has been supported for a while. .. api:: ``scmutil.readrequires()`` has been removed. Use ``localrepo.featuresetupfuncs`` to register new repository requirements. Use ``localrepo.ensurerequirementsrecognized()`` to validate them. Differential Revision: https://phab.mercurial-scm.org/D4570
Wed, 12 Sep 2018 14:54:17 -0700 localrepo: validate supported requirements in makelocalrepository()
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 14:54:17 -0700] rev 39693
localrepo: validate supported requirements in makelocalrepository() This should be a glorified code move. I did take the opportunity to refactor things. We now have a separate function for gathering requirements and one for validating them. I also mode cosmetic changes to the code, such as not using abbreviations and using a set instead of list to model missing requirements. Differential Revision: https://phab.mercurial-scm.org/D4569
Wed, 12 Sep 2018 14:45:52 -0700 localrepo: read requirements file in makelocalrepository()
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 14:45:52 -0700] rev 39692
localrepo: read requirements file in makelocalrepository() Previously, scmutil.readrequires() loaded the requirements file and validated its content against what was supported. Requirements translate to repository features and are critical to our plans to dynamically create local repository types. So, we must load them in makelocalrepository() before a repository instance is constructed. This commit moves the reading of the .hg/requires file to makelocalrepository(). Because scmutil.readrequires() was performing I/O and validation, we inlined the validation into localrepository.__init__ and removed scmutil.readrequires(). I plan to remove scmutil.readrequires() in a future commit (we can't do it now because statichttprepo uses it). Differential Revision: https://phab.mercurial-scm.org/D4568
Wed, 12 Sep 2018 12:36:07 -0700 localrepo: check for .hg/ directory in makelocalrepository()
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 12 Sep 2018 12:36:07 -0700] rev 39691
localrepo: check for .hg/ directory in makelocalrepository() As part of this, we move the check to before .hg/hgrc is loaded, as it makes sense to check for the directory before attempting to open a file in it. Differential Revision: https://phab.mercurial-scm.org/D4567
(0) -30000 -10000 -3000 -1000 -300 -100 -10 +10 +100 +300 +1000 +3000 +10000 tip