D4642: localrepo: iteratively derive local repository type

mharbison72 (Matt Harbison) phabricator at mercurial-scm.org
Mon Sep 24 23:56:28 EDT 2018


mharbison72 added a comment.


  In https://phab.mercurial-scm.org/D4642#71399, @indygreg wrote:
  
  > In https://phab.mercurial-scm.org/D4642#71269, @yuja wrote:
  >
  > > > +    Extensions should wrap these factory functions to customize repository type
  > > >  +    creation. Note that an extension's wrapped function may be called even if
  > > >  +    that extension is not loaded for the repo being constructed. Extensions
  > > >  +    should check if their ``__name__`` appears in the
  > > >  +    ``extensionmodulenames`` set passed to the factory function and no-op if
  > > >  +    not.
  > >
  > > I assume this will be revisited later. I think it's a source of bugs to
  > >  relying on extensions to check if they are enabled. It's also cumbersome
  > >  to wrap a function referenced from another table.
  >
  >
  > I agree it is not great and I hope to revisit this problem.
  >
  > FWIW we have similar problems with `extensions.wrapfunction()`. Many function wrappings bury their head in the sand with regards to universal function wrapping in multi-repo contexts. e.g. in hgweb, repo A can load an extension which wraps a function. Repo B doesn't want that extension loaded but the process still has the function wrapped. I'm not sure how to best handle this :/
  
  
  I like the `@decorator` style registration that evolve does.  I wonder if that can be combined with the repository "features" functionality, such that the extension can still globally wrap functions, and add features in say, reposetup().  The decorator could be declared with a feature that needs to be present, and that backing code check for it by default before either calling the wrapper or orig().  I guess one problem is that not every function takes a repo.  But without a repo, I'm not sure how any decision can be made outside of individual extensions.
  
  Even if this only covers 75% of the cases, it at least gives visibility to the problem to extension authors.  I didn't realize it was a problem until I ran into it with LFS.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4642

To: indygreg, #hg-reviewers
Cc: mharbison72, yuja, mercurial-devel


More information about the Mercurial-devel mailing list