Merge lp://staging/~spiv/bzr-loom/plugin-init into lp://staging/bzr-loom

Proposed by Andrew Bennetts
Status: Work in progress
Proposed branch: lp://staging/~spiv/bzr-loom/plugin-init
Merge into: lp://staging/bzr-loom
Diff against target: 836 lines (+244/-220)
2 files modified
branch.py (+89/-210)
formats.py (+155/-10)
To merge this branch: bzr merge lp://staging/~spiv/bzr-loom/plugin-init
Reviewer Review Type Date Requested Status
Loom Developers Pending
Review via email: mp+32559@code.staging.launchpad.net

Description of the change

This branch does a couple of loosely related things:
 * makes sure InterLoomBranch is always registered, by moving its definition to formats.py. I don't think this is a great solution, but it does solve the problem. Certainly by the time formats.register.formats() has been called that InterLoomBranch ought to be installed one way or another I think. This fixes bug 617212.
 * replaces "import bzrlib.branch" etc with "from bzrlib import branch" etc, following bzrlib's own conventions in format.py and branch.py. This also makes explicit a bunch of import dependencies like bzrlib.inventory that were only implicitly imported before.
 * uses lazy_import where appropriate in format.py and branch.py (and don't use it when not helpful... register_formats is always called by __init__.py so bzrlib.branch will unavoidably be imported).

I've verified that bzr with no plugins other than loom this saves 25 open syscalls (out of ~1500, as measured with strace: BZR_PLUGIN_PATH='-user:-core:-site' BZR_PLUGINS_AT=loom@$(pwd) strace -e stat64,open -c bzr.dev st). There's no noticeable time difference on a warm system, but this does suggest it is slightly more efficient this way, or at least no worse.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

On Fri, Aug 13, 2010 at 7:13 PM, Andrew Bennetts
<email address hidden> wrote:
> Andrew Bennetts has proposed merging lp:~spiv/bzr-loom/plugin-init into lp:bzr-loom.
>
> Requested reviews:
>  Loom Developers (bzr-loom-devs)
> Related bugs:
>  #617212 "bzr selftest -s bt.per_interbranch Loom" finds 0 tests
>  https://bugs.launchpad.net/bugs/617212
>
>
> This branch does a couple of loosely related things:
>  * makes sure InterLoomBranch is always registered, by moving its definition to formats.py.  I don't think this is a great solution, but it does solve the problem.  Certainly by the time formats.register.formats() has been called that InterLoomBranch ought to be installed one way or another I think.  This fixes bug 617212.

Probably ok.

>  * replaces "import bzrlib.branch" etc with "from bzrlib import branch" etc, following bzrlib's own conventions in format.py and branch.py.  This also makes explicit a bunch of import dependencies like bzrlib.inventory that were only implicitly imported before.

This is going to be really confusing. Loom has its own 'branch'
module, so please don't do that. (Not to mention that I am not
convinced by the bzr convention anyhow. If you feel you must do that,
please do _mod_bzrlib_branch or something to make in unambiguous.
Personally though, import bzrlib.branch is pretty nice.

>  * uses lazy_import where appropriate in format.py and branch.py (and don't use it when not helpful... register_formats is always called by __init__.py so bzrlib.branch will unavoidably be imported).

I would keep it lazy myself, if we fix bzrlib to not load
bzrlib.branch when registering formats, it would be a shame to have to
rework this because of an arbitrary change

-Rob

Unmerged revisions

127. By Andrew Bennetts

Follow bzrlib conventions for imports, use lazy_imports in branch.py, and register InterLoomBranch in formats.py so that it is always registered for the bzr per_interbranch test suite.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches