need checks or tests that compiled extensions are loaded properly

Bug #406113 reported by Martin Pool
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
High
Martin Pool

Bug Description

In cases like bug 405653 the compiled extensions were not being loaded properly. This can cause a large performance regression but it may not be obvious that performance is low because of an easily fixed problem.

We want to keep the ability to run bzr using only Python implementations because: being able to cross-test against the Python implementations may be useful for debugging, and because it's useful to run bzr in environments where it's not possible to build extensions. Both of these are usual situations and people should know what they're getting in to.

We'd like a red flag when either:

 * someone commits a change (eg to pqm) that causes extensions not to be loaded when they should be
 * a problem in the source, in packaging, or installations causes extensions not to be loaded at run time when they should be

I'm suggesting these are different aspects of the bug because it's quite possible to have either failures in the source that mean it doesn't correctly attempt to load the extension even if it's built, or possible to have problems in the build scripts, installation or packaging that may mean users experience this problem even if there's no problem in the upstream tree.

At the moment our protection against this is comprised of: per-implementation tests that can raise dependency not present results; and a check in setup.py that causes it to fail unless the extensions are built.

Related branches

Revision history for this message
Martin Pool (mbp) wrote :

The problem as I see it is that if you manage, in whatever way, to get a copy of bzr that doesn't load its extensions, it will silently fall back on every run.

If you really want just the python code obviously this is desirable but I think that's the abnormal case.

How about: we will always give a warning (or even error) if failing to load them, unless an environment variable or config setting is given to turn it off?

Martin Pool (mbp)
Changed in bzr:
status: Confirmed → In Progress
Revision history for this message
Martin Pool (mbp) wrote :

See also https://code.edge.launchpad.net/~mbp/bzr/406113-extension-warnings/+merge/9410

John points out we should give just one message, not one for every missing extension.

Plugins may add compiled extensions, so ideally we would not have a static list of 'the extensions for Bazaar'.

Extensions may fail to load for reasons other than the file not being present.

At the moment we have no concept of 'all the extensions bzr uses'. We could poke through the bzrlib directory and find either .c, .pyx or extension libraries, though that might not cover extensions needed by plugins. Alternatively, we can have a statically maintained list,

Bug 131019 asks that we show the extensions in --version. That seems to imply probing for extensions that wouldn't normally be loaded to show the version.

Revision history for this message
Martin Pool (mbp) wrote :

I'm updating this branch and you now get something like this:

[0] mbp@grace% ./bzr st
modified:
  bzrlib/commands.py
  bzrlib/osutils.py
unknown:
  tmp.diff
  bzrlib/tmp.diff
/home/mbp/bzr/406113-extension-warnings/bzrlib/osutils.py:928: UserWarning: bzr: warning: Failed to load compiled extensions:
    No module named _chunks_to_lines_pyx
    cannot import name _btree_serializer_pyx
    No module named _rio_pyx
    No module named _bencode_pyx
    No module named _dirstate_helpers_pyx
    No module named _chk_map_pyx
    No module named _annotator_pyx
    No module named _known_graph_pyx
    No module named _knit_load_data_pyx
    No module named _groupcompress_pyx
    No module named _readdir_pyx
    Bazaar can run, but performance may be reduced.
    Check Bazaar is correctly installed or set ignore_missing_extensions
  % '\n '.join(_extension_load_failures,))

in some ways we should just give the module name, but it's possible that the error message will be of interest.

Revision history for this message
Martin Pool (mbp) wrote :

It turns out that ImportError has no structure, just a message, and I don't really feel like parsing apart the message to make the warning prettier.

Revision history for this message
John A Meinel (jameinel) wrote : Re: [Bug 406113] Re: need checks or tests that compiled extensions are loaded properly

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> I'm updating this branch and you now get something like this:
>
> [0] mbp@grace% ./bzr st
> modified:
> bzrlib/commands.py
> bzrlib/osutils.py
> unknown:
> tmp.diff
> bzrlib/tmp.diff
> /home/mbp/bzr/406113-extension-warnings/bzrlib/osutils.py:928: UserWarning: bzr: warning: Failed to load compiled extensions:
> No module named _chunks_to_lines_pyx
> cannot import name _btree_serializer_pyx
> No module named _rio_pyx
> No module named _bencode_pyx
> No module named _dirstate_helpers_pyx
> No module named _chk_map_pyx
> No module named _annotator_pyx
> No module named _known_graph_pyx
> No module named _knit_load_data_pyx
> No module named _groupcompress_pyx
> No module named _readdir_pyx
> Bazaar can run, but performance may be reduced.
> Check Bazaar is correctly installed or set ignore_missing_extensions
> % '\n '.join(_extension_load_failures,))
>
> in some ways we should just give the module name, but it's possible that
> the error message will be of interest.
>

^- I'm noticing that the indentation seems less than optimal. Especially
seeing the '%'\n....' line at the end is a bit ugly.

Anyway, it is better than the status quo. :)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqqVTUACgkQJdeBCYSNAAPB0wCbBEpYDZoxblrAqY1TRuKGmYPS
HcYAn16KQAvlGP7SSm7ciRxSgWap/sfW
=3jML
-----END PGP SIGNATURE-----

Revision history for this message
Martin Pool (mbp) wrote : Re: [Bug 406113] Re: need checks or tests that compiled extensions are loaded properly

2009/9/11 John A Meinel <email address hidden>:

> ^- I'm noticing that the indentation seems less than optimal. Especially
> seeing the '%'\n....' line at the end is a bit ugly.
>
> Anyway, it is better than the status quo. :)

I think for some classes of warning that are not api deprecations, we
probably don't want to bother giving the source code for the line it
originated from.

--
Martin <http://launchpad.net/~mbp/>

Revision history for this message
Martin Pool (mbp) wrote :

This was merged post 2.0. We're getting some reports that it's giving warnings when running from source so it is apparently active.

More fixes to get the right balance are probably needed but I'll mark this closed for now.

Changed in bzr:
status: In Progress → Fix Released
milestone: none → 2.1
Vincent Ladeuil (vila)
Changed in bzr:
milestone: 2.1.0 → 2.1b1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.