Patchwork D6741: interfaces: create a new folder for interfaces and move repository.py in it

login
register
mail settings
Submitter phabricator
Date Aug. 17, 2019, 11:45 p.m.
Message ID <differential-rev-PHID-DREV-vfcfyiuel5ffp37hlxs6-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41347/
State Superseded
Headers show

Comments

phabricator - Aug. 17, 2019, 11:45 p.m.
pulkit created this revision.
Herald added a reviewer: indygreg.
Herald added a reviewer: durin42.
Herald added a reviewer: martinvonz.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I was trying to understand current interfaces and write new ones and I realized
  we need to improve how current interfaces are organised. This creates a
  dedicated folder for defining interfaces and move `repository.py` which defines
  all the current interfaces inside it.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  contrib/import-checker.py
  hgext/lfs/__init__.py
  hgext/lfs/wrapper.py
  hgext/narrow/__init__.py
  hgext/narrow/narrowbundle2.py
  hgext/narrow/narrowcommands.py
  hgext/sqlitestore.py
  mercurial/changegroup.py
  mercurial/exchange.py
  mercurial/exchangev2.py
  mercurial/filelog.py
  mercurial/hg.py
  mercurial/httppeer.py
  mercurial/interfaces/__init__.py
  mercurial/interfaces/repository.py
  mercurial/localrepo.py
  mercurial/manifest.py
  mercurial/narrowspec.py
  mercurial/repository.py
  mercurial/revlog.py
  mercurial/revlogutils/constants.py
  mercurial/streamclone.py
  mercurial/testing/storage.py
  mercurial/utils/storageutil.py
  mercurial/wireprotov1peer.py
  setup.py
  tests/notcapable
  tests/pullext.py
  tests/simplestorerepo.py
  tests/test-check-interfaces.py
  tests/wireprotosimplecache.py

CHANGE DETAILS




To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 19, 2019, 10:48 a.m.
martinvonz added a comment.


  > I was trying to understand current interfaces and write new ones and I realized
  > we need to improve how current interfaces are organised.
  
  And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6741/new/

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

To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 20, 2019, 3:04 p.m.
pulkit added a comment.


  In D6741#98947 <https://phab.mercurial-scm.org/D6741#98947>, @martinvonz wrote:
  
  >> I was trying to understand current interfaces and write new ones and I realized
  >> we need to improve how current interfaces are organised.
  >
  > And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better?
  
  Looking through Augie's hgit patch, I found we need to add more interfaces and decided to work on adding for `store.basicstore`. I found all the current interfaces in repository.py which has grown very large. I decided to create a new file to have interface for the store class, and maybe a new one for dirstate too, and having them in a separate folder dedicated to interfaces will be nice.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6741/new/

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

To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 20, 2019, 3:10 p.m.
durin42 added a comment.


  In D6741#99023 <https://phab.mercurial-scm.org/D6741#99023>, @pulkit wrote:
  
  > In D6741#98947 <https://phab.mercurial-scm.org/D6741#98947>, @martinvonz wrote:
  >
  >>> I was trying to understand current interfaces and write new ones and I realized
  >>> we need to improve how current interfaces are organised.
  >>
  >> And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better?
  >
  > Looking through Augie's hgit patch, I found we need to add more interfaces and decided to work on adding for `store.basicstore`. I found all the current interfaces in repository.py which has grown very large. I decided to create a new file to have interface for the store class, and maybe a new one for dirstate too, and having them in a separate folder dedicated to interfaces will be nice.
  
  I'm inclined to agree - the repository.py file full of interfaces has gotten crufty. It'd be nice to split it out into more files by topic.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6741/new/

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

To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 20, 2019, 3:17 p.m.
martinvonz added a comment.


  In D6741#99030 <https://phab.mercurial-scm.org/D6741#99030>, @durin42 wrote:
  
  > In D6741#99023 <https://phab.mercurial-scm.org/D6741#99023>, @pulkit wrote:
  >
  >> In D6741#98947 <https://phab.mercurial-scm.org/D6741#98947>, @martinvonz wrote:
  >>
  >>>> I was trying to understand current interfaces and write new ones and I realized
  >>>> we need to improve how current interfaces are organised.
  >>>
  >>> And what was the reason we need to improve it? I assume we don't really "need" to change it. Will it somehow help with future patches? Or you just like this structure better?
  >>
  >> Looking through Augie's hgit patch, I found we need to add more interfaces and decided to work on adding for `store.basicstore`. I found all the current interfaces in repository.py which has grown very large. I decided to create a new file to have interface for the store class, and maybe a new one for dirstate too, and having them in a separate folder dedicated to interfaces will be nice.
  >
  > I'm inclined to agree - the repository.py file full of interfaces has gotten crufty. It'd be nice to split it out into more files by topic.
  
  That makes sense to me too.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6741/new/

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

To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 25, 2019, 4:06 p.m.
This revision is now accepted and ready to land.
indygreg added a comment.
indygreg accepted this revision.


  It sounds like we're in agreement that interfaces need to be split. Moving everything to a sub-package seems like a logical first step.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6741/new/

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

To: pulkit, indygreg, durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/wireprotosimplecache.py b/tests/wireprotosimplecache.py
--- a/tests/wireprotosimplecache.py
+++ b/tests/wireprotosimplecache.py
@@ -10,12 +10,14 @@ 
 from mercurial import (
     extensions,
     registrar,
-    repository,
     util,
     wireprotoserver,
     wireprototypes,
     wireprotov2server,
 )
+from mercurial.interfaces import (
+    repository,
+)
 from mercurial.utils import (
     interfaceutil,
     stringutil,
diff --git a/tests/test-check-interfaces.py b/tests/test-check-interfaces.py
--- a/tests/test-check-interfaces.py
+++ b/tests/test-check-interfaces.py
@@ -14,6 +14,9 @@ 
                     'test-repo']):
     sys.exit(80)
 
+from mercurial.interfaces import (
+    repository,
+)
 from mercurial.thirdparty.zope import (
     interface as zi,
 )
@@ -27,7 +30,6 @@ 
     localrepo,
     manifest,
     pycompat,
-    repository,
     revlog,
     sshpeer,
     statichttprepo,
diff --git a/tests/simplestorerepo.py b/tests/simplestorerepo.py
--- a/tests/simplestorerepo.py
+++ b/tests/simplestorerepo.py
@@ -32,11 +32,13 @@ 
     localrepo,
     mdiff,
     pycompat,
-    repository,
     revlog,
     store,
     verify,
 )
+from mercurial.interfaces import (
+    repository,
+)
 from mercurial.utils import (
     cborutil,
     interfaceutil,
diff --git a/tests/pullext.py b/tests/pullext.py
--- a/tests/pullext.py
+++ b/tests/pullext.py
@@ -13,6 +13,8 @@ 
     error,
     extensions,
     localrepo,
+)
+from mercurial.interfaces import (
     repository,
 )
 
diff --git a/tests/notcapable b/tests/notcapable
--- a/tests/notcapable
+++ b/tests/notcapable
@@ -6,7 +6,8 @@ 
 fi
 
 cat > notcapable-$CAP.py << EOF
-from mercurial import extensions, localrepo, repository
+from mercurial import extensions, localrepo
+from mercurial.interfaces import repository
 def extsetup(ui):
     extensions.wrapfunction(repository.peer, 'capable', wrapcapable)
     extensions.wrapfunction(localrepo.localrepository, 'peer', wrappeer)
diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1067,6 +1067,7 @@ 
             'mercurial.cext',
             'mercurial.cffi',
             'mercurial.hgweb',
+            'mercurial.interfaces',
             'mercurial.pure',
             'mercurial.thirdparty',
             'mercurial.thirdparty.attr',
diff --git a/mercurial/wireprotov1peer.py b/mercurial/wireprotov1peer.py
--- a/mercurial/wireprotov1peer.py
+++ b/mercurial/wireprotov1peer.py
@@ -22,10 +22,12 @@ 
     error,
     pushkey as pushkeymod,
     pycompat,
-    repository,
     util,
     wireprototypes,
 )
+from .interfaces import (
+    repository,
+)
 from .utils import (
     interfaceutil,
 )
diff --git a/mercurial/utils/storageutil.py b/mercurial/utils/storageutil.py
--- a/mercurial/utils/storageutil.py
+++ b/mercurial/utils/storageutil.py
@@ -22,8 +22,8 @@ 
     error,
     mdiff,
     pycompat,
-    repository,
 )
+from ..interfaces import repository
 
 _nullhash = hashlib.sha1(nullid)
 
diff --git a/mercurial/testing/storage.py b/mercurial/testing/storage.py
--- a/mercurial/testing/storage.py
+++ b/mercurial/testing/storage.py
@@ -17,6 +17,8 @@ 
 from .. import (
     error,
     mdiff,
+)
+from ..interfaces import (
     repository,
 )
 from ..utils import (
diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -12,13 +12,15 @@ 
 import struct
 
 from .i18n import _
+from .interfaces import (
+    repository,
+)
 from . import (
     cacheutil,
     error,
     narrowspec,
     phases,
     pycompat,
-    repository,
     store,
     util,
 )
diff --git a/mercurial/revlogutils/constants.py b/mercurial/revlogutils/constants.py
--- a/mercurial/revlogutils/constants.py
+++ b/mercurial/revlogutils/constants.py
@@ -9,7 +9,7 @@ 
 
 from __future__ import absolute_import
 
-from .. import (
+from ..interfaces import (
     repository,
 )
 
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -65,10 +65,12 @@ 
     mdiff,
     policy,
     pycompat,
-    repository,
     templatefilters,
     util,
 )
+from .interfaces import (
+    repository,
+)
 from .revlogutils import (
     deltas as deltautil,
     flagutil,
diff --git a/mercurial/narrowspec.py b/mercurial/narrowspec.py
--- a/mercurial/narrowspec.py
+++ b/mercurial/narrowspec.py
@@ -8,11 +8,13 @@ 
 from __future__ import absolute_import
 
 from .i18n import _
+from .interfaces import (
+    repository,
+)
 from . import (
     error,
     match as matchmod,
     merge,
-    repository,
     scmutil,
     sparse,
     util,
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -24,10 +24,12 @@ 
     mdiff,
     policy,
     pycompat,
-    repository,
     revlog,
     util,
 )
+from .interfaces import (
+    repository,
+)
 from .utils import (
     interfaceutil,
 )
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -52,7 +52,6 @@ 
     phases,
     pushkey,
     pycompat,
-    repository,
     repoview,
     revset,
     revsetlang,
@@ -66,6 +65,11 @@ 
     util,
     vfs as vfsmod,
 )
+
+from .interfaces import (
+    repository,
+)
+
 from .utils import (
     interfaceutil,
     procutil,
diff --git a/mercurial/repository.py b/mercurial/interfaces/repository.py
rename from mercurial/repository.py
rename to mercurial/interfaces/repository.py
--- a/mercurial/repository.py
+++ b/mercurial/interfaces/repository.py
@@ -7,11 +7,11 @@ 
 
 from __future__ import absolute_import
 
-from .i18n import _
-from . import (
+from ..i18n import _
+from .. import (
     error,
 )
-from .utils import (
+from ..utils import (
     interfaceutil,
 )
 
diff --git a/mercurial/interfaces/__init__.py b/mercurial/interfaces/__init__.py
new file mode 100644
diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
--- a/mercurial/httppeer.py
+++ b/mercurial/httppeer.py
@@ -16,12 +16,14 @@ 
 import weakref
 
 from .i18n import _
+from .interfaces import (
+    repository,
+)
 from . import (
     bundle2,
     error,
     httpconnection,
     pycompat,
-    repository,
     statichttprepo,
     url as urlmod,
     util,
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -39,7 +39,6 @@ 
     node,
     phases,
     pycompat,
-    repository as repositorymod,
     scmutil,
     sshpeer,
     statichttprepo,
@@ -51,6 +50,10 @@ 
     vfs as vfsmod,
 )
 
+from .interfaces import (
+    repository as repositorymod,
+)
+
 release = lock.release
 
 # shared features
diff --git a/mercurial/filelog.py b/mercurial/filelog.py
--- a/mercurial/filelog.py
+++ b/mercurial/filelog.py
@@ -14,8 +14,10 @@ 
 )
 from . import (
     error,
+    revlog,
+)
+from .interfaces import (
     repository,
-    revlog,
 )
 from .utils import (
     interfaceutil,
diff --git a/mercurial/exchangev2.py b/mercurial/exchangev2.py
--- a/mercurial/exchangev2.py
+++ b/mercurial/exchangev2.py
@@ -22,8 +22,10 @@ 
     narrowspec,
     phases,
     pycompat,
+    setdiscovery,
+)
+from .interfaces import (
     repository,
-    setdiscovery,
 )
 
 def pull(pullop):
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -34,7 +34,6 @@ 
     phases,
     pushkey,
     pycompat,
-    repository,
     scmutil,
     sslutil,
     streamclone,
@@ -42,6 +41,9 @@ 
     util,
     wireprototypes,
 )
+from .interfaces import (
+    repository,
+)
 from .utils import (
     stringutil,
 )
diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -25,8 +25,11 @@ 
     mdiff,
     phases,
     pycompat,
+    util,
+)
+
+from .interfaces import (
     repository,
-    util,
 )
 
 _CHANGEGROUPV1_DELTA_HEADER = struct.Struct("20s20s20s20s")
diff --git a/hgext/sqlitestore.py b/hgext/sqlitestore.py
--- a/hgext/sqlitestore.py
+++ b/hgext/sqlitestore.py
@@ -70,10 +70,12 @@ 
     mdiff,
     pycompat,
     registrar,
-    repository,
     util,
     verify,
 )
+from mercurial.interfaces import (
+    repository,
+)
 from mercurial.utils import (
     interfaceutil,
     storageutil,
diff --git a/hgext/narrow/narrowcommands.py b/hgext/narrow/narrowcommands.py
--- a/hgext/narrow/narrowcommands.py
+++ b/hgext/narrow/narrowcommands.py
@@ -25,12 +25,14 @@ 
     pycompat,
     registrar,
     repair,
-    repository,
     repoview,
     sparse,
     util,
     wireprototypes,
 )
+from mercurial.interfaces import (
+    repository,
+)
 
 table = {}
 command = registrar.command(table)
diff --git a/hgext/narrow/narrowbundle2.py b/hgext/narrow/narrowbundle2.py
--- a/hgext/narrow/narrowbundle2.py
+++ b/hgext/narrow/narrowbundle2.py
@@ -23,10 +23,12 @@ 
     localrepo,
     narrowspec,
     repair,
-    repository,
     util,
     wireprototypes,
 )
+from mercurial.interfaces import (
+    repository,
+)
 from mercurial.utils import (
     stringutil,
 )
diff --git a/hgext/narrow/__init__.py b/hgext/narrow/__init__.py
--- a/hgext/narrow/__init__.py
+++ b/hgext/narrow/__init__.py
@@ -17,6 +17,9 @@ 
 from mercurial import (
     localrepo,
     registrar,
+)
+
+from mercurial.interfaces import (
     repository,
 )
 
diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -21,7 +21,6 @@ 
     exchange,
     exthelper,
     localrepo,
-    repository,
     revlog,
     scmutil,
     upgrade,
@@ -30,6 +29,10 @@ 
     wireprotov1server,
 )
 
+from mercurial.interfaces import (
+    repository,
+)
+
 from mercurial.utils import (
     storageutil,
     stringutil,
diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -141,13 +141,16 @@ 
     minifileset,
     node,
     pycompat,
-    repository,
     revlog,
     scmutil,
     templateutil,
     util,
 )
 
+from mercurial.interfaces import (
+    repository,
+)
+
 from . import (
     blobstore,
     wireprotolfsserver,
diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -28,6 +28,7 @@ 
     'mercurial.hgweb.common',
     'mercurial.hgweb.request',
     'mercurial.i18n',
+    'mercurial.interfaces',
     'mercurial.node',
     # for revlog to re-export constant to extensions
     'mercurial.revlogutils.constants',