bzr-svn breaks all commands when subvertpy missing

[Robert Collins]

Please be nicer on missing dependencies.

nicer == can still use bzr.

 affects bzr-svn

  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/bzrdir.py", line
920, in open_containing_tree_or_branch
    bzrdir, relpath = klass.open_containing(location)
  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/bzrdir.py", line
847, in open_containing
    return BzrDir.open_containing_from_transport(transport)
  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/bzrdir.py", line
868, in open_containing_from_transport
    result = BzrDir.open_from_transport(a_transport)
  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/bzrdir.py", line
822, in open_from_transport
    redirected)
  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/lazy_import.py",
line 125, in __call__
    return obj(*args, **kwargs)
  File
"/home/robertc/source/baz/bzr-test-fixes/bzrlib/transport/__init__.py",
line 1680, in do_catching_redirections
    return action(transport)
  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/bzrdir.py", line
809, in find_format
    transport, _server_formats=_server_formats)
  File "/home/robertc/source/baz/bzr-test-fixes/bzrlib/bzrdir.py", line
1662, in find_format
    return format.probe_transport(transport)
  File "/home/robertc/.bazaar/plugins/svn/format.py", line 86, in
probe_transport
    klass._check_versions()
  File "/home/robertc/.bazaar/plugins/svn/format.py", line 56, in
_check_versions
    lazy_check_versions()
  File "/home/robertc/.bazaar/plugins/svn/__init__.py", line 119, in
lazy_check_versions
    init_subvertpy()
  File "/home/robertc/.bazaar/plugins/svn/__init__.py", line 102, in
init_subvertpy
    raise ImportError("bzr-svn: unable to find subvertpy. Please install
from http://launchpad.net/subvertpy.")
ImportError: bzr-svn: unable to find subvertpy. Please install from
http://launchpad.net/subvertpy.

[Jelmer Vernooij (jelmer)]

On Sun, 2009-03-01 at 22:48 +0000, Robert Collins wrote:
> Public bug reported:
>
> Please be nicer on missing dependencies.
>
> nicer == can still use bzr.
What do you mean by nicer exactly? Is there some exception I should be
raising instead that won't make bzr traceback?
--
Jelmer Vernooij <email address hidden> - http://samba.org/~jelmer/
Jabber: <email address hidden>

[Robert Collins (lifeless)]

On Mon, 2009-03-02 at 02:12 +0000, Jelmer Vernooij wrote:
> On Sun, 2009-03-01 at 22:48 +0000, Robert Collins wrote:
> > Public bug reported:
> >
> > Please be nicer on missing dependencies.
> >
> > nicer == can still use bzr.
> What do you mean by nicer exactly? Is there some exception I should be
> raising instead that won't make bzr traceback?

`bzr info` when there is no .svn directory on disk *cannot* be a svn
using control directory - there is no need for subvertpy or bzr-svn at
that point. So I don't think you need to tell the user anything.

If there *is* a .svn directory, thats different, of course it is
reasonable to raise at that point. As for exceptions, define a custom
one (from BzrError) and it will show a one line error rather than a full
backtrace. Or use bzrlib.errors.DependencyNotPresent.

I've been thinking that perhaps a 'comment' field in 'bzr plugins' would
be nice - it would let you probe for subvertpy and report that its not
available in the UI without raising an exception. What do you think?

-Rob

[Jelmer Vernooij (jelmer)]

.svn directories only exist in Subversion working copies, not in Subversion repositories.

For Subversion repositories the only real indication that there is a Subversion repository present is if there is a file named "format". However, it is also possible to address paths that are inside of the repository and don't exist as such, e.g. file:///foo/trunk where "/foo" contains a Subversion repository. os.path.exists("/foo/trunk") is False here, but we can open this URL using bzr-svn.

I've fixed bzr-svn to raise DependencyNotPresent, so it should at least not longer give tracebacks but a single-line error. That's as far as bzr-svn can go with the current infrastructure in bzr.

I don't think hiding the fact that bzr-svn couldn't be loaded in "bzr plugins" would be useful; it will just confuse users who would think that while they have bzr-svn installed it doesn't work.

Rather, I think it would make sense for the format probe code to catch DependencyNotPresent and print a warning if it was raised rather than letting DependencyNotPresent fall through and cause an abort of the probe process.

[Robert Collins (lifeless)]

On Mon, 2009-03-02 at 03:45 +0000, Jelmer Vernooij wrote:
> .svn directories only exist in Subversion working copies, not in
> Subversion repositories.

I'm getting this error in local bzr only working trees. Subversion
repositories have similar data though - a 'format' file for starters.

> I've fixed bzr-svn to raise DependencyNotPresent, so it should at least
> not longer give tracebacks but a single-line error. That's as far as
> bzr-svn can go with the current infrastructure in bzr.

I think thats an improvement. However, I still don't think you need
bzr-svn to be loaded successfully in the cases I'm seeing this. We're
probing for a control directory, not accessing arbitrary files at this
point in the code path. Perhaps I don't understand the issues
sufficiently.

> I don't think hiding the fact that bzr-svn couldn't be loaded in "bzr
> plugins" would be useful; it will just confuse users who would think
> that while they have bzr-svn installed it doesn't work.

Fair enough. I do want us to become more lazy [avoiding work we don't
need to do] and that will tend to hide things anyway.

> Rather, I think it would make sense for the format probe code to catch
> DependencyNotPresent and print a warning if it was raised rather than
> letting DependencyNotPresent fall through and cause an abort of the
> probe process.

We could do this; however it would still be very noisy wouldn't it?

What other info can I give to show that this is being raised when it
doesn't need to be?

-Rob

[Jelmer Vernooij (jelmer)]

You seem to want to be able to have bzr-svn installed without subvertpy;
that's not something I want to support. If Bazaar should be more
tolerant of plugins that are missing dependencies, I think that's
something that should be handled in Bazaar by handling ImportError and
DependencyNotPresent, not in the individual plugins.

However, I'm all for more laziness in the loading of bzr-svn.

On Mon, 2009-03-02 at 04:23 +0000, Robert Collins wrote:
> On Mon, 2009-03-02 at 03:45 +0000, Jelmer Vernooij wrote:
> > I've fixed bzr-svn to raise DependencyNotPresent, so it should at least
> > not longer give tracebacks but a single-line error. That's as far as
> > bzr-svn can go with the current infrastructure in bzr.
>
> I think thats an improvement. However, I still don't think you need
> bzr-svn to be loaded successfully in the cases I'm seeing this. We're
> probing for a control directory, not accessing arbitrary files at this
> point in the code path. Perhaps I don't understand the issues
> sufficiently.
Well, we'd have to check all of the parent paths to see if they exist
and if so if they contain "format" files. That is doable, and probably
worth the code duplication from libsvn here.

> > I don't think hiding the fact that bzr-svn couldn't be loaded in "bzr
> > plugins" would be useful; it will just confuse users who would think
> > that while they have bzr-svn installed it doesn't work.
> Fair enough. I do want us to become more lazy [avoiding work we don't
> need to do] and that will tend to hide things anyway.
I do agree laziness is good in general (one of the reasons I've patched
bzr-{git,hg,gtk,svn} to be lazy), but it shouldn't change the behaviour
of Bazaar in any way (other than being faster, of course). i.e. "bzr
info" on a svn checkout should give some error/warning about subvertpy
being missing.

> > Rather, I think it would make sense for the format probe code to catch
> > DependencyNotPresent and print a warning if it was raised rather than
> > letting DependencyNotPresent fall through and cause an abort of the
> > probe process.
> We could do this; however it would still be very noisy wouldn't it?
Yes, but I think that's a non-issue. You should either install subvertpy
in that case or remove/disable bzr-svn.

Cheers,

Jelmer

--
Jelmer Vernooij <email address hidden> - http://samba.org/~jelmer/
Jabber: <email address hidden>

[Robert Collins (lifeless)]

On Mon, 2009-03-02 at 15:01 +0000, Jelmer Vernooij wrote:
> You seem to want to be able to have bzr-svn installed without subvertpy;
> that's not something I want to support. If Bazaar should be more
> tolerant of plugins that are missing dependencies, I think that's
> something that should be handled in Bazaar by handling ImportError and
> DependencyNotPresent, not in the individual plugins.

Not per se. However:
$ sudo apt-get install subvertpy
Reading package lists... Done
Building dependency tree
Reading state information... Done
E: Couldn't find package subvertpy

so I have subvertpy manually installed. But I have to set my PYTHONPATH
to let bzr-svn find it now that its not bundled; and that is a manual
step. I'd like bzr-svn to not complain in a fatal manner except when I'm
actually trying to use it || it is reasonable for it to have to be
talking to svn to be used.

This has performance ramifications - not loading until its really needed
means less libraries loaded in the general case :)

> However, I'm all for more laziness in the loading of bzr-svn.
..
> > sufficiently.
> Well, we'd have to check all of the parent paths to see if they exist
> and if so if they contain "format" files. That is doable, and probably
> worth the code duplication from libsvn here.

Ah, this is because:
 - open_branch is called on ControlDir
 - SVNDir-for-a-branch-may-have-no-existence-on-disk
 - bzr has no concept of continuing to walk up, and then finding
   something lower.

Just walking up could lead to finding a svn tree above a bzr tree, not
so good. Do you have seperate formats for
svn-has-a/format-or-is-on-weddav-etc, and
svn-has-a-.svn-is-a-tree-only ? If so, modifying the former would appear
safe, as nesting a bzr tree under a svn _repo_ is nuts.

Alternatively, perhaps bzr needs to grow some more flexability or
clarity in these matters. Knowing that we are trying to open a branch
may be a useful hint.

> > > I don't think hiding the fact that bzr-svn couldn't be loaded in "bzr
> > > plugins" would be useful; it will just confuse users who would think
> > > that while they have bzr-svn installed it doesn't work.
> > Fair enough. I do want us to become more lazy [avoiding work we don't
> > need to do] and that will tend to hide things anyway.
> I do agree laziness is good in general (one of the reasons I've patched
> bzr-{git,hg,gtk,svn} to be lazy), but it shouldn't change the behaviour
> of Bazaar in any way (other than being faster, of course). i.e. "bzr
> info" on a svn checkout should give some error/warning about subvertpy
> being missing.

For sure. Failing on warn on something like that would be bad :).

> > > Rather, I think it would make sense for the format probe code to catch
> > > DependencyNotPresent and print a warning if it was raised rather than
> > > letting DependencyNotPresent fall through and cause an abort of the
> > > probe process.
> > We could do this; however it would still be very noisy wouldn't it?
> Yes, but I think that's a non-issue. You should either install subvertpy
> in that case or remove/disable bzr-svn.

Currently I remove bzr-svn, which generally leads to you getting less
bug reports from me when I break ...

[Jelmer Vernooij (jelmer)]

On Mon, Mar 02, 2009 at 08:15:22PM -0000, Robert Collins wrote:
> On Mon, 2009-03-02 at 15:01 +0000, Jelmer Vernooij wrote:
> > You seem to want to be able to have bzr-svn installed without subvertpy;
> > that's not something I want to support. If Bazaar should be more
> > tolerant of plugins that are missing dependencies, I think that's
> > something that should be handled in Bazaar by handling ImportError and
> > DependencyNotPresent, not in the individual plugins.

> $ sudo apt-get install subvertpy
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> E: Couldn't find package subvertpy

> so I have subvertpy manually installed. But I have to set my PYTHONPATH
> to let bzr-svn find it now that its not bundled; and that is a manual
> step. I'd like bzr-svn to not complain in a fatal manner except when I'm
> actually trying to use it || it is reasonable for it to have to be
> talking to svn to be used.
Try python-subvertpy :-) That should be available in the subvertpy PPA, bzr
PPA, in jaunty, NetBSD, MacPorts, FrugalWare Linux, the bzr windows installer
and soon also in Debian sid.

> This has performance ramifications - not loading until its really needed
> means less libraries loaded in the general case :)
Sure, and I think *that* is the reason I haven't closed this bug as
Won't Fix :-)

> > However, I'm all for more laziness in the loading of bzr-svn.
> ..
> > > sufficiently.
> > Well, we'd have to check all of the parent paths to see if they exist
> > and if so if they contain "format" files. That is doable, and probably
> > worth the code duplication from libsvn here.
> Ah, this is because:
> - open_branch is called on ControlDir
> - SVNDir-for-a-branch-may-have-no-existence-on-disk
> - bzr has no concept of continuing to walk up, and then finding
> something lower.

> Just walking up could lead to finding a svn tree above a bzr tree, not
> so good.
That can't really happen with Subversion repositories though, you
would never have bzr branches in there, unless you're putting the svn
berkeley DB in bzr for some reason...

Cheers,

Jelmer

--
Jelmer Vernooij <email address hidden> - http://jelmer.vernstok.nl/

[Robert Collins (lifeless)]

On Mon, 2009-03-02 at 20:40 +0000, Jelmer Vernooij wrote:

> > > However, I'm all for more laziness in the loading of bzr-svn.
> > ..
> > > > sufficiently.
> > > Well, we'd have to check all of the parent paths to see if they exist
> > > and if so if they contain "format" files. That is doable, and probably
> > > worth the code duplication from libsvn here.
> > Ah, this is because:
> > - open_branch is called on ControlDir
> > - SVNDir-for-a-branch-may-have-no-existence-on-disk
> > - bzr has no concept of continuing to walk up, and then finding
> > something lower.
>
> > Just walking up could lead to finding a svn tree above a bzr tree, not
> > so good.
> That can't really happen with Subversion repositories though, you
> would never have bzr branches in there, unless you're putting the svn
> berkeley DB in bzr for some reason...

Right, thats what I asked about in the bit you trimmed :)
svn-tree/bzr-tree is fine
svn-repo/bzr-tree can't happen

If you have separate control dir objects probing for svn trees and svn
repos, we can sanely have the svn repo one walk up.

-Rob

[Jelmer Vernooij (jelmer)]

On Mon, Mar 02, 2009 at 09:00:04PM -0000, Robert Collins wrote:
> On Mon, 2009-03-02 at 20:40 +0000, Jelmer Vernooij wrote:
> > > > However, I'm all for more laziness in the loading of bzr-svn.
> > > ..
> > > > > sufficiently.
> > > > Well, we'd have to check all of the parent paths to see if they exist
> > > > and if so if they contain "format" files. That is doable, and probably
> > > > worth the code duplication from libsvn here.
> > > Ah, this is because:
> > > - open_branch is called on ControlDir
> > > - SVNDir-for-a-branch-may-have-no-existence-on-disk
> > > - bzr has no concept of continuing to walk up, and then finding
> > > something lower.

> > > Just walking up could lead to finding a svn tree above a bzr tree, not
> > > so good.
> > That can't really happen with Subversion repositories though, you
> > would never have bzr branches in there, unless you're putting the svn
> > berkeley DB in bzr for some reason...

> Right, thats what I asked about in the bit you trimmed :)
> svn-tree/bzr-tree is fine
> svn-repo/bzr-tree can't happen

> If you have separate control dir objects probing for svn trees and svn
> repos, we can sanely have the svn repo one walk up.
Yes, these are separate. The working tree one just does a
transport.has(".svn") for the transport it is handed only.

Cheers,

Jelmer

--
Jelmer Vernooij <email address hidden> - http://jelmer.vernstok.nl/
 22:55:21 up 8 days, 5:08, 5 users, load average: 3.44, 3.58, 3.61

[Jelmer Vernooij (jelmer)]

  status fixreleased