[PATCH 04 of 10] localrepo: move ui loading to baselocalrepository

[Kevin Bullock]

> On Feb 13, 2017, at 19:09, Jun Wu <quark at fb.com> wrote:
> 
> Whatever we will end up with, I want to make sure:
> 
>  1. There is a "repo" object passed to preload functions, not a "repopath"
> 
>     "repopath" is just not practically useful as too many checks need to be
>     done on it. The repo object does not have to be a full repo - a minimal
>     version could only have vfs and svfs, so preload functions can read
>     contents properly.
> 
>  2. Avoid unexpected reposetup() runs
> 
>    - In the future, chg does not run uisetup()s, for correctness. Because
>      uisetup() runs only once and will take incorrect configs.
>    - Extensions do not expect reposetup() runs when uisetup() does not run.
>    - So repo.__init__ should not call reposetup()s.
> 
>    (this could be done alternatively by ignoring reposetup()s for
>     extensions that haven't run uisetup()s)
> 
>  3. Avoid unwanted side effects
> 
>    - An extension starts a new SSH connection when repo.__init__ is called
>    - An extension writes to some log file when repo.__init__ is called 
> 
> So I think moving part of "localrepository" to a smaller object is
> reasonable. An alternative I can think of is to disable side-effect methods
> in a subclass of localrepository:
> 
>    class purerepo(localrepository):
>        def __init__(..., create=False):
>            super(..., create=False)
> 
>        def lock(self):
>            raise ProgrammingError('cannot lock')
> 
>        def transaction(self):
>            raise ProgrammingError('cannot do transaction')
> 
> If that looks more reasonable, I can go that approach, which will probably
> be more compatible, while less confident about the side-effect-free part.

There's another alternative we might consider: can we move all of the "side effects" out of the localrepository class entirely? That is, instead of resorting to inheritance, could we create a lower-level "repo storage" class that has enough logic to make vfs and svfs available, but doesn't call reposetup()?

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock