Matthijs Kooijman wrote on Tue, Feb 07, 2012 at 19:29:36 +0100:
> Hi Daniel,
>
> > > + # ctx.config.config (ctx.config is opaque).
> >
> > What do you mean by that?
> >
> > % PYTHONPATH=$prefix/svn-t1/lib/svn-python/ \
> > python -c 'import svn, svn.core, svn.client;
> > print svn.core.svn_config_get_config(None, None)["config"]'
> > <libsvn.core.svn_config_t; proxy of <Swig Object of type 'svn_config_t *'
> > at 0xb734d998> >
> I meant this:
>
> matthijs@grubby:~/docs/src/upstream/subversion-trunk$ python -c 'import svn,
> svn.core, svn.client;print svn.core.svn_config_get_config(None,
> None)["config"]'
> Traceback (most recent call last):
> File "<string>", line 1, in <module>
> File "/usr/lib/pymodules/python2.7/libsvn/core.py", line 5494, in
> __getattr__
> value = _swig_getattr(self, self.__class__, name)
> File "/usr/lib/pymodules/python2.7/libsvn/core.py", line 51, in
> _swig_getattr
> raise AttributeError(name)
> AttributeError: __getitem__
>
> On trunk, your code seems to work (but there, ctx.config still seems to be
> None, though).
>
> Would using svn.core.svn_config_get_config(None, None)["config"] here be the
> recommended practice?
>
svn_config_get_config() reads from disk. You'd do that just once (and
stash the result in the client context, if svn_cmdline_init() or someone
else doesn't do that for you), and then ["config"] it. (In the C level
that's just an apr_hash_get().)
Have a look at what the svn* binaries do.
> > > +++ subversion-trunk/subversion/bindings/swig/python/tests/client.py
> > > 2012-02-07 16:20:36.914353499 +0100
> > > @@ -374,6 +374,11 @@
> > >
> > > self.assertEqual(readme_text, 'This is a test.\n')
> > >
> > > + def test_platform_providers(self):
> > > + providers =
> > > core.svn_auth_get_platform_specific_client_providers(None, None)
> > > + # Not much more we can test in this minimal environment.
> > > + self.assert_(isinstance(providers, list))
> > > +
> >
> > You could assert
> >
> > all(map(lambda x: isinstance(x, 'svn_auth_provider_object_t *'),
> > list))
> >
> > like the Perl revision you refer to does.
> I couldn't figure out how to do that in the face of swig types. Your
> suggestion
> doesn't work either:
>
> TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and
> types
>
It was pseudocode :-)
If you don't find a better way (even after asking) I'll also be happy
with a test that does a strstr() on the repr() of the objects.
> > I've committed this patch. (I had to rewrite the log message to conform
> > to our style of mentioning every symbol[1].) Please send followups as
> > patches against the new HEAD.
> Thanks! I thought I had a policy-compliant log message already, but it seems
> you've been even more thorough. I'll keep that in mind for further patches.
>
You did mention most/all of the changed symbols, but you didn't do so in
the
* path/to/file
(symbol_name): %s
format.
> Gr.
>
> Matthijs
