unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Crash with Python bindings
@ 2016-01-12  9:41 Konrad Hinsen
  2016-01-12 13:11 ` David Bremner
                   ` (2 more replies)
  0 siblings, 3 replies; 38+ messages in thread
From: Konrad Hinsen @ 2016-01-12  9:41 UTC (permalink / raw)
  To: notmuch

Hi everyone,

I have been writing quite a few Python scripts for notmuch before 
running into a strange bug. Here is a minimal script producing it:

--------------------------------------------------
from notmuch import Query, Database

def foo(bar):
     pass

db = Database()
q = Query(db, "*")
db.close()
--------------------------------------------------

Running this script (Python 3.5, MacOS X) yields:

[1]    22478 abort      pydev3 Temp/notmuch_test.py

The crash actually happens *after* db.close(), when the Python 
interpreter exists, and therefore I suspect that no data is lost, but I 
hesitate to use scripts with that behavior in production use.

The strange part is that what causes the crash is the presence of the 
function foo(), even though it is never called. Remove foo and the 
script runs fine. It is also necessary to create a Query object. The 
combination of a function definition (any) and the creation of a Query 
object yields the crash. This looks like a memory management issue, but 
I didn't explore it any further.

Cheers,
   Konrad.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12  9:41 Crash with Python bindings Konrad Hinsen
@ 2016-01-12 13:11 ` David Bremner
  2016-01-12 14:21   ` Konrad Hinsen
       [not found] ` <20160112102329.4269.20741@thinkbox.jade-hamburg.de>
  2016-01-12 18:08 ` Crash with Python bindings W. Trevor King
  2 siblings, 1 reply; 38+ messages in thread
From: David Bremner @ 2016-01-12 13:11 UTC (permalink / raw)
  To: Konrad Hinsen, notmuch

Konrad Hinsen <konrad.hinsen@fastmail.net> writes:

> Hi everyone,
>
> I have been writing quite a few Python scripts for notmuch before 
> running into a strange bug. Here is a minimal script producing it:
>
> --------------------------------------------------
> from notmuch import Query, Database
>
> def foo(bar):
>      pass
>
> db = Database()
> q = Query(db, "*")
> db.close()

Do you really call the constructor without a path? Or are you censoring
the script for some reason?

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12 13:11 ` David Bremner
@ 2016-01-12 14:21   ` Konrad Hinsen
  2016-01-12 15:26     ` David Bremner
  0 siblings, 1 reply; 38+ messages in thread
From: Konrad Hinsen @ 2016-01-12 14:21 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

>> from notmuch import Query, Database
>>
>> def foo(bar):
>>      pass
>>
>> db = Database()
>> q = Query(db, "*")
>> db.close()
>
> Do you really call the constructor without a path? Or are you censoring
> the script for some reason?

No path means path=None, which stands for the path from
~/.notmuch-config. That's exactly what I want. Is there some reason not
to rely on this mechanism?

Cheers,
  Konrad.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
       [not found] ` <20160112102329.4269.20741@thinkbox.jade-hamburg.de>
@ 2016-01-12 14:23   ` Konrad Hinsen
  2016-01-12 18:51     ` W. Trevor King
  0 siblings, 1 reply; 38+ messages in thread
From: Konrad Hinsen @ 2016-01-12 14:23 UTC (permalink / raw)
  To: notmuch

Hi Justus,

> So I guess what happens is that Python3 changed how the interpreter
> environment is torn down and they actually destroy the 'q' object.  If
> that is so, then your data is indeed safe.

That reminds me of a recent change in object finalization in Python 3:

  https://www.python.org/dev/peps/pep-0442/

I'll look into that, thanks for the gdb exploration!

Cheers,
  Konrad.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12 14:21   ` Konrad Hinsen
@ 2016-01-12 15:26     ` David Bremner
  2016-01-12 19:03       ` David Bremner
  0 siblings, 1 reply; 38+ messages in thread
From: David Bremner @ 2016-01-12 15:26 UTC (permalink / raw)
  To: Konrad Hinsen, notmuch

Konrad Hinsen <konrad.hinsen@fastmail.net> writes:

> David Bremner <david@tethera.net> writes:
>
>>> from notmuch import Query, Database
>>>
>>> def foo(bar):
>>>      pass
>>>
>>> db = Database()
>>> q = Query(db, "*")
>>> db.close()
>>
>> Do you really call the constructor without a path? Or are you censoring
>> the script for some reason?
>
> No path means path=None, which stands for the path from
> ~/.notmuch-config. That's exactly what I want. Is there some reason not
> to rely on this mechanism?

Oh sorry, I'm (obviously) not that familiar with the python bindings.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12  9:41 Crash with Python bindings Konrad Hinsen
  2016-01-12 13:11 ` David Bremner
       [not found] ` <20160112102329.4269.20741@thinkbox.jade-hamburg.de>
@ 2016-01-12 18:08 ` W. Trevor King
  2 siblings, 0 replies; 38+ messages in thread
From: W. Trevor King @ 2016-01-12 18:08 UTC (permalink / raw)
  To: Konrad Hinsen; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Tue, Jan 12, 2016 at 10:41:57AM +0100, Konrad Hinsen wrote:
> --------------------------------------------------
> from notmuch import Query, Database
> 
> def foo(bar):
>      pass
> 
> db = Database()
> q = Query(db, "*")
> db.close()
> --------------------------------------------------
> 
> Running this script (Python 3.5, MacOS X) yields:
>
> [1]    22478 abort      pydev3 Temp/notmuch_test.py
> …
> The strange part is that what causes the crash is the presence of the 
> function foo(), even though it is never called. Remove foo and the 
> script runs fine. It is also necessary to create a Query object.

Adding some more data-points, I see the same results with Python 3.4.3
on Gentoo, although stderr just gets “Aborted”.  All permutations seem
to work on Python 3.3.5.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12 14:23   ` Crash with Python bindings Konrad Hinsen
@ 2016-01-12 18:51     ` W. Trevor King
  2018-03-16 11:59       ` David Bremner
  0 siblings, 1 reply; 38+ messages in thread
From: W. Trevor King @ 2016-01-12 18:51 UTC (permalink / raw)
  To: Konrad Hinsen; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Tue, Jan 12, 2016 at 03:23:46PM +0100, Konrad Hinsen wrote:
> Hi Justus,
> 
> > So I guess what happens is that Python3 changed how the
> > interpreter environment is torn down and they actually destroy the
> > 'q' object.  If that is so, then your data is indeed safe.
> 
> That reminds me of a recent change in object finalization in Python
> 3:
> 
>   https://www.python.org/dev/peps/pep-0442/

I'm pretty sure that is what's going on.  The PEP landed in Python
3.4, explaining why I don't see this issue in 3.3 [1].  And it has
[2]:

   In particular, this PEP obsoletes the current guideline that
   "objects with a __del__ method should not be part of a reference
   cycle".

I'm not sure what the best way is to fix __del__ in our Python
bindings, but if you manage them explicitly:

  db = Database()
  q = Query(db, "*")
  del q
  db.close()
  del db

you can avoid the abort (which happens when q.__del__ is called after
db.__del__).  We could make that sort of cleanup easier with context
managers for Query objects (we have them for databases since [3]), and
they look like the only object that keep an internal database
reference:

  with Database() as db:
    with Query(db, "*") as q:
      # do something with q
    db.close()

Cheers,
Trevor

[1]: id:20160112180813.GA20499@odin.tremily.us
[2]: https://www.python.org/dev/peps/pep-0442/#impact
[3]: 36ce7e3c (python: implement the context manager protocol for
     database objects, 2012-02-15, v0.12)

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12 15:26     ` David Bremner
@ 2016-01-12 19:03       ` David Bremner
  2016-01-12 19:13         ` Binding access to ~/.notmuch-config (was: Crash with Python bindings) W. Trevor King
  0 siblings, 1 reply; 38+ messages in thread
From: David Bremner @ 2016-01-12 19:03 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

>> No path means path=None, which stands for the path from
>> ~/.notmuch-config. That's exactly what I want. Is there some reason not
>> to rely on this mechanism?
>
> Oh sorry, I'm (obviously) not that familiar with the python bindings.

Nothing to do with Konrad's crash, but I consider the fact that the
python bindings read ~/.notmuch-config to be a kind of layering
violation, since that file belongs to the CLI, while the bindings are
supposed to provide access to libnotmuch. Whether this is a real problem
or just an aesthetic one, I'm not sure, but I thought I'd mention it
since we are thinking of various config related issues. Obviously the
location of the database is not one of the things it makes sense to
store in the database. I can imagine scenarios where the bindings might
be usable without the CLI, but they seem fairly artificial so far, since
it seems like almost everyone needs notmuch-new / notmuch-insert.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Binding access to ~/.notmuch-config (was: Crash with Python bindings)
  2016-01-12 19:03       ` David Bremner
@ 2016-01-12 19:13         ` W. Trevor King
  2016-01-13 11:25           ` Binding access to ~/.notmuch-config Konrad Hinsen
  0 siblings, 1 reply; 38+ messages in thread
From: W. Trevor King @ 2016-01-12 19:13 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On Tue, Jan 12, 2016 at 03:03:15PM -0400, David Bremner wrote:
> Nothing to do with Konrad's crash, but I consider the fact that the
> python bindings read ~/.notmuch-config to be a kind of layering
> violation, since that file belongs to the CLI, while the bindings
> are supposed to provide access to libnotmuch.

I think of ~/.notmuch-config as being shared between all client code,
and in that view it makes sense to have both the CLI and Python
bindings (and other bindings) access it to figure out how to configure
their library access calls.  Having a separate config file for each
client to point at the default database path seems like more trouble
than it's worth, as does adding a library function for “reach into
some local config and return the default database path”.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Binding access to ~/.notmuch-config
  2016-01-12 19:13         ` Binding access to ~/.notmuch-config (was: Crash with Python bindings) W. Trevor King
@ 2016-01-13 11:25           ` Konrad Hinsen
  2016-01-13 12:25             ` David Bremner
  0 siblings, 1 reply; 38+ messages in thread
From: Konrad Hinsen @ 2016-01-13 11:25 UTC (permalink / raw)
  To: notmuch

On 12/01/16 20:13, W. Trevor King wrote:

> On Tue, Jan 12, 2016 at 03:03:15PM -0400, David Bremner wrote:
>> Nothing to do with Konrad's crash, but I consider the fact that the
>> python bindings read ~/.notmuch-config to be a kind of layering
>> violation, since that file belongs to the CLI, while the bindings
>> are supposed to provide access to libnotmuch.
>
> I think of ~/.notmuch-config as being shared between all client code,
> and in that view it makes sense to have both the CLI and Python
> bindings (and other bindings) access it to figure out how to configure

I agree. I see notmuch as a collection of CLI tools, some of which are 
part of the distribution and others are written by myself for my 
specific needs. I'd like them all to share a single configuration file. 
In fact, I'd love to be able to add sections specific to my Python scripts.

Cheers,
   Konrad.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Binding access to ~/.notmuch-config
  2016-01-13 11:25           ` Binding access to ~/.notmuch-config Konrad Hinsen
@ 2016-01-13 12:25             ` David Bremner
  2016-01-13 17:23               ` W. Trevor King
  0 siblings, 1 reply; 38+ messages in thread
From: David Bremner @ 2016-01-13 12:25 UTC (permalink / raw)
  To: Konrad Hinsen, notmuch

Konrad Hinsen <konrad.hinsen@fastmail.net> writes:

> I agree. I see notmuch as a collection of CLI tools, some of which are 
> part of the distribution and others are written by myself for my 
> specific needs. I'd like them all to share a single configuration file. 
> In fact, I'd love to be able to add sections specific to my Python scripts.

Yes, I understand that it's convenient, but the current set up is not
really very robust.  The python bindings are making assumptions about a
file format that has never been documented, and whose stability has
never been promised. It's roughly like calling private functions not
part of the published API. Sometimes it's the only way to do something,
but that doesn't mean it won't break. I think we'll eventually want to
provide some config related API, but having having arbitrary programs
reading and writing this file isn't it, IMHO.  In particular upcoming
changes may move some configuration items out of this file and into a
library level configuration API.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Binding access to ~/.notmuch-config
  2016-01-13 12:25             ` David Bremner
@ 2016-01-13 17:23               ` W. Trevor King
  0 siblings, 0 replies; 38+ messages in thread
From: W. Trevor King @ 2016-01-13 17:23 UTC (permalink / raw)
  To: David Bremner; +Cc: Konrad Hinsen, notmuch

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On Wed, Jan 13, 2016 at 08:25:10AM -0400, David Bremner wrote:
> Konrad Hinsen writes:
> > I agree. I see notmuch as a collection of CLI tools, some of which
> > are part of the distribution and others are written by myself for
> > my specific needs. I'd like them all to share a single
> > configuration file.  In fact, I'd love to be able to add sections
> > specific to my Python scripts.
> 
> Yes, I understand that it's convenient, but the current set up is
> not really very robust…  In particular upcoming changes may move
> some configuration items out of this file and into a library level
> configuration API.

I think you mean [1].  And that's fine, since Python scripts, etc.,
can use that API to set and access config settings, be they standard
or script-specific (as far as I can tell, I haven't reviewed that
series in detail).  Docs on any standard settings would be nice, but I
guess they'd land as features moved out of ~/.notmuch-config and into
the database.  The only setting that can't move into the database (as
you pointed out earlier [2]) is the path to the database.  That's
currently all the Python bindings extract now, and making that route
the officially blessed way to find the default database path makes
sense to me.

Cheers,
Trevor

[1]: id:1452654610-22864-1-git-send-email-david@tethera.net
     http://thread.gmane.org/gmane.mail.notmuch.general/21643
[2]: id:8760yy4o3w.fsf@tesseract.cs.unb.ca
     http://article.gmane.org/gmane.mail.notmuch.general/21639

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2016-01-12 18:51     ` W. Trevor King
@ 2018-03-16 11:59       ` David Bremner
  2018-03-16 12:12         ` Justus Winter
  2018-03-16 18:30         ` Floris Bruynooghe
  0 siblings, 2 replies; 38+ messages in thread
From: David Bremner @ 2018-03-16 11:59 UTC (permalink / raw)
  To: W. Trevor King, Justus Winter; +Cc: notmuch

"W. Trevor King" <wking@tremily.us> writes:

> you can avoid the abort (which happens when q.__del__ is called after
> db.__del__).  We could make that sort of cleanup easier with context
> managers for Query objects (we have them for databases since [3]), and
> they look like the only object that keep an internal database
> reference:
>
>   with Database() as db:
>     with Query(db, "*") as q:
>       # do something with q
>     db.close()
>

I'm reminded [1] that this problem still exists. If noone has any idea
of a fix, should we document one of the workarounds?

      
[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=893057

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2018-03-16 11:59       ` David Bremner
@ 2018-03-16 12:12         ` Justus Winter
  2018-03-16 18:30         ` Floris Bruynooghe
  1 sibling, 0 replies; 38+ messages in thread
From: Justus Winter @ 2018-03-16 12:12 UTC (permalink / raw)
  To: David Bremner, W. Trevor King; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

David Bremner <david@tethera.net> writes:

> "W. Trevor King" <wking@tremily.us> writes:
>
>> you can avoid the abort (which happens when q.__del__ is called after
>> db.__del__).  We could make that sort of cleanup easier with context
>> managers for Query objects (we have them for databases since [3]), and
>> they look like the only object that keep an internal database
>> reference:
>>
>>   with Database() as db:
>>     with Query(db, "*") as q:
>>       # do something with q
>>     db.close()

So while this shouldn't crash of course, this code is wrong.  The
context manager closes the database, so doing db.close() at the end of
the block is superfluous.

> I'm reminded [1] that this problem still exists. If noone has any idea
> of a fix, should we document one of the workarounds?

I don't remember the details, but the different semantics of garbage
collection and talloc was problematic.  In essence, every wrapping
Python object must keep a reference to its parent (as in parent in the
talloc hierarchy).

The bug report [1] sounds like that the crash happens at interpreter
shutdown, where iirc the objects destructors are called in arbitrary
order.


Justus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2018-03-16 11:59       ` David Bremner
  2018-03-16 12:12         ` Justus Winter
@ 2018-03-16 18:30         ` Floris Bruynooghe
  2018-03-16 22:40           ` Daniel Kahn Gillmor
  2018-03-21 10:16           ` New Python bindings (was: Crash with Python bindings) Justus Winter
  1 sibling, 2 replies; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-16 18:30 UTC (permalink / raw)
  To: David Bremner, W. Trevor King, Justus Winter; +Cc: notmuch

Hi all,

David Bremner <david@tethera.net> writes:

> "W. Trevor King" <wking@tremily.us> writes:
>
>> you can avoid the abort (which happens when q.__del__ is called after
>> db.__del__).  We could make that sort of cleanup easier with context
>> managers for Query objects (we have them for databases since [3]), and
>> they look like the only object that keep an internal database
>> reference:
>>
>>   with Database() as db:
>>     with Query(db, "*") as q:
>>       # do something with q
>>     db.close()
>>
>
> I'm reminded [1] that this problem still exists. If noone has any idea
> of a fix, should we document one of the workarounds?

This is exactly what I have fixed in my alternative bindings which I
created around the end of last year [0].  So we do have an idea of how
to fix this, at the time I said I do believe that it's possible to also
do this for the existing bindings even though it is a lot of work.
After some talking between dkg and me we got to a way forward which
proposed this, but I must admit that after messing a little with getting
a pytest run integrated into the notmuch test-suite instead of using tox
I lost momentum on the project and didn't advance any further.

If someone can hook pytest runs with various python versions into the
notmuch test suit I'd be very much obliged and probably have another go
at this as it's still an interesting problem and gives a nice way
forward.

Cheers,
Floris

[0] https://github.com/flub/notmuch/tree/cffi/bindings/python-cffi

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2018-03-16 18:30         ` Floris Bruynooghe
@ 2018-03-16 22:40           ` Daniel Kahn Gillmor
  2018-03-18  8:01             ` Floris Bruynooghe
  2018-03-21 10:16           ` New Python bindings (was: Crash with Python bindings) Justus Winter
  1 sibling, 1 reply; 38+ messages in thread
From: Daniel Kahn Gillmor @ 2018-03-16 22:40 UTC (permalink / raw)
  To: Floris Bruynooghe, David Bremner, W. Trevor King, Justus Winter; +Cc: notmuch

On Fri 2018-03-16 19:30:37 +0100, Floris Bruynooghe wrote:
> If someone can hook pytest runs with various python versions into the
> notmuch test suit I'd be very much obliged and probably have another go
> at this as it's still an interesting problem and gives a nice way
> forward.

I don't really know what this request means -- so maybe that means that
i'm not the right person for the task, which is fine.

but it's also possible that the right person for the task *also* doesn't
know what you're asking for, so if you could elaborate a bit further
i think that would be super helpful :)

thanks for looking into this further!

  --dkg

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: Crash with Python bindings
  2018-03-16 22:40           ` Daniel Kahn Gillmor
@ 2018-03-18  8:01             ` Floris Bruynooghe
  0 siblings, 0 replies; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-18  8:01 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, David Bremner, W. Trevor King, Justus Winter; +Cc: notmuch

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> On Fri 2018-03-16 19:30:37 +0100, Floris Bruynooghe wrote:
>> If someone can hook pytest runs with various python versions into the
>> notmuch test suit I'd be very much obliged and probably have another go
>> at this as it's still an interesting problem and gives a nice way
>> forward.
>
> I don't really know what this request means -- so maybe that means that
> i'm not the right person for the task, which is fine.
>
> but it's also possible that the right person for the task *also* doesn't
> know what you're asking for, so if you could elaborate a bit further
> i think that would be super helpful :)

Fair enough :)
Here a somewhat more long-form version of this:

Before even attempting to refactor the existing bindings to use cffi as
a backend instead off ctypes and/or adding the changes needed to track
the lifetime of objects correctly I would like to be able to write full
unitttest-level tests for the bindings to be able to guarantee that no
user-level APIs are broken.  In my version of the bindings I did this
the traditional Python way: using pytest for writing unittest and using
tox to invoke the tests for the various supported versions of python.

One of the feedback items I got from the patch I sent last time was that
the project would be reluctant to adopt this and would like to avoid
virtualenv and pip with their behaviour of downloading things over the
network.  Instead wishing for it to use a system python which should
have the available tools already installed (i.e. pytest).  And all this
just integrated in the existing test suite.

So my last attempt at this looks like I made a test/T391-pytest.sh file
with the idea of running a subtest for each python version, with the
subtest being a ``pythonX.Y -m pytest bindings/python/tests/`` so it'd
run the entire test.  To be nice this also needs to be hooked up so that
the subtests get skipped when a python version is not available, or is
missing pytest itself.

So while trying to figure this out is where I got distracted last time
and started working more on other things.


Kind Regards,
Floris

^ permalink raw reply	[flat|nested] 38+ messages in thread

* New Python bindings (was: Crash with Python bindings)
  2018-03-16 18:30         ` Floris Bruynooghe
  2018-03-16 22:40           ` Daniel Kahn Gillmor
@ 2018-03-21 10:16           ` Justus Winter
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
                               ` (2 more replies)
  1 sibling, 3 replies; 38+ messages in thread
From: Justus Winter @ 2018-03-21 10:16 UTC (permalink / raw)
  To: Floris Bruynooghe, David Bremner, W. Trevor King; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Hi Floris :)

Floris Bruynooghe <flub@devork.be> writes:

> This is exactly what I have fixed in my alternative bindings which I
> created around the end of last year [0].  So we do have an idea of how
> to fix this, at the time I said I do believe that it's possible to also
> do this for the existing bindings even though it is a lot of work.
> After some talking between dkg and me we got to a way forward which
> proposed this, but I must admit that after messing a little with getting
> a pytest run integrated into the notmuch test-suite instead of using tox
> I lost momentum on the project and didn't advance any further.

I'm sorry that I didn't speak up when you announced your work.  I'm
actually excited about a new set of bindings for Python.  I agree with
using cffi.  I briefly looked at the code, and I believe it is much
nicer than what we currently have.

I trust that it works fine with Python 3, does it?

The testsuite cannot depend on pulling stuff from the net simply because
build servers typically do not have access to it.  That is a hard
requirement.

I don't remember what kind of tests there are for the current bindings
(are there any...?), but shouldn't these just continue to work for the
time being?


Thanks,
Justus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* pytest integration for the notmuch test suite
  2018-03-21 10:16           ` New Python bindings (was: Crash with Python bindings) Justus Winter
@ 2018-03-25 17:40             ` David Bremner
  2018-03-25 17:40               ` [PATCH 1/3] configure: check for pytest binary David Bremner
                                 ` (4 more replies)
  2018-03-26 20:47             ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
  2018-03-28  7:20             ` New Python bindings (was: Crash with Python bindings) Brian May
  2 siblings, 5 replies; 38+ messages in thread
From: David Bremner @ 2018-03-25 17:40 UTC (permalink / raw)
  To: Justus Winter, Floris Bruynooghe, David Bremner, W. Trevor King; +Cc: notmuch

Here's one approach. A given pytest "file" can be embedded in a normal
(for us) test script.  As I write this, it occurs to me you might be
thinking of embedding unit tests in the bindings source files; that
would be easy to add, something along the lines of

test_begin_subtest "python bindings embedded unit tests"
test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch"

You could also run one source file of tests with

test_begin_subtest "python bindings foo tests"
test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch/test_foo.py"

that would give a less granular result, at the cost of more boilerplate

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/3] configure: check for pytest binary
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
@ 2018-03-25 17:40               ` David Bremner
  2018-03-26 20:55                 ` Floris Bruynooghe
  2018-03-25 17:40               ` [PATCH 2/3] test: add new test_expect_pytest_success David Bremner
                                 ` (3 subsequent siblings)
  4 siblings, 1 reply; 38+ messages in thread
From: David Bremner @ 2018-03-25 17:40 UTC (permalink / raw)
  To: Justus Winter, Floris Bruynooghe, David Bremner, W. Trevor King; +Cc: notmuch

This is to support future use of pytest in the test suite
---
 configure | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/configure b/configure
index b177b141..ab45878d 100755
--- a/configure
+++ b/configure
@@ -62,6 +62,7 @@ CXXFLAGS=${CXXFLAGS:-\$(CFLAGS)}
 LDFLAGS=${LDFLAGS:-}
 XAPIAN_CONFIG=${XAPIAN_CONFIG:-}
 PYTHON=${PYTHON:-}
+PYTEST=${PYTEST:-}
 
 # We don't allow the EMACS or GZIP Makefile variables inherit values
 # from the environment as we do with CC and CXX above. The reason is
@@ -118,6 +119,8 @@ Other environment variables can be used to control configure itself,
 			library. [$XAPIAN_CONFIG]
 	PYTHON		Name of python command to use in
 			configure and the test suite.
+        PYTEST		Name of pytest command to use in
+                        the test suite.
 
 Additionally, various options can be specified on the configure
 command line.
@@ -571,6 +574,24 @@ if [ $have_python -eq 0 ]; then
     errors=$((errors + 1))
 fi
 
+pytest=""
+if [ $have_python -eq 1 ]; then
+    printf "Checking for pytest... "
+    have_pytest=0
+
+    for name in ${PYTEST} pytest-3 pytest pytest-2; do
+        if command -v $name > /dev/null; then
+            have_pytest=1
+            pytest=$name
+            printf "Yes (%s).\n" $pytest
+            break
+        fi
+    done
+    if [ $have_pytest -eq 0 ]; then
+        printf "No (some tests may be skipped).\n"
+    fi
+fi
+
 printf "Checking for valgrind development files... "
 if pkg-config --exists valgrind; then
     printf "Yes.\n"
@@ -1234,6 +1255,9 @@ NOTMUCH_HAVE_MAN=$((have_sphinx))
 # Name of python interpreter
 NOTMUCH_PYTHON=${python}
 
+# Name of pytest runner
+NOTMUCH_PYTEST=${pytest}
+
 # Are the ruby development files (and ruby) available? If not skip
 # building/testing ruby bindings.
 NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/3] test: add new test_expect_pytest_success
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
  2018-03-25 17:40               ` [PATCH 1/3] configure: check for pytest binary David Bremner
@ 2018-03-25 17:40               ` David Bremner
  2018-03-25 17:40               ` [PATCH 3/3] test: add example test using pytest David Bremner
                                 ` (2 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: David Bremner @ 2018-03-25 17:40 UTC (permalink / raw)
  To: Justus Winter, Floris Bruynooghe, David Bremner, W. Trevor King; +Cc: notmuch

As the name suggests, this is something of a cross between
test_python (reading a script from stdin) and test expect success.

It seemed somewhat redundant to allow our usual kind of file
comparison with pytest scripts, although that will make it tougher to
compare output with the CLI.
---
 test/test-lib.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 5b212514..fd0e9647 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1002,6 +1002,13 @@ test_python() {
 	$NOTMUCH_PYTHON -B - > OUTPUT
 }
 
+test_expect_pytest_success() {
+    test_file="test_${test_count}.py"
+    cat > ${test_file}
+    PYTHONPATH="$NOTMUCH_SRCDIR/bindings/python${PYTHONPATH:+:$PYTHONPATH}" \
+              test_expect_success "$NOTMUCH_PYTEST ${test_file}"
+}
+
 test_ruby() {
     MAIL_DIR=$MAIL_DIR ruby -I $NOTMUCH_SRCDIR/bindings/ruby> OUTPUT
 }
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 3/3] test: add example test using pytest
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
  2018-03-25 17:40               ` [PATCH 1/3] configure: check for pytest binary David Bremner
  2018-03-25 17:40               ` [PATCH 2/3] test: add new test_expect_pytest_success David Bremner
@ 2018-03-25 17:40               ` David Bremner
  2018-03-25 19:14               ` pytest integration for the notmuch test suite Tomi Ollila
  2018-03-26 21:01               ` Floris Bruynooghe
  4 siblings, 0 replies; 38+ messages in thread
From: David Bremner @ 2018-03-25 17:40 UTC (permalink / raw)
  To: Justus Winter, Floris Bruynooghe, David Bremner, W. Trevor King; +Cc: notmuch

It might make sense to remove the non-pytest version of this test, but
that requires other changes to following tests
---
 test/T390-python.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/T390-python.sh b/test/T390-python.sh
index 9f71ce3c..72036004 100755
--- a/test/T390-python.sh
+++ b/test/T390-python.sh
@@ -194,4 +194,16 @@ EOF
 echo "$fname" > EXPECTED
 test_expect_equal_file EXPECTED OUTPUT
 
+if [ -n "${NOTMUCH_PYTEST}" ]; then
+
+    test_begin_subtest "pytest based get_revision"
+    test_expect_pytest_success <<EOF
+import notmuch
+def test_get_revision():
+    db = notmuch.Database(mode=notmuch.Database.MODE.READ_ONLY)
+    (revision, uuid) = db.get_revision()
+    assert revision == 55
+EOF
+
+fi
 test_done
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: pytest integration for the notmuch test suite
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
                                 ` (2 preceding siblings ...)
  2018-03-25 17:40               ` [PATCH 3/3] test: add example test using pytest David Bremner
@ 2018-03-25 19:14               ` Tomi Ollila
  2018-03-26 11:31                 ` David Bremner
  2018-03-26 21:01               ` Floris Bruynooghe
  4 siblings, 1 reply; 38+ messages in thread
From: Tomi Ollila @ 2018-03-25 19:14 UTC (permalink / raw)
  To: David Bremner, Justus Winter, Floris Bruynooghe, David Bremner,
	W. Trevor King
  Cc: notmuch

On Sun, Mar 25 2018, David Bremner wrote:

> Here's one approach. A given pytest "file" can be embedded in a normal
> (for us) test script.  As I write this, it occurs to me you might be
> thinking of embedding unit tests in the bindings source files; that
> would be easy to add, something along the lines of
>
> test_begin_subtest "python bindings embedded unit tests"
> test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch"

Hmm.

Looks a bit strange to embed the pytest snippets into shell script and then
execute each of these individiually. The only thing py.test seems to do here is
"visualizing" assert output. We could just use normal python otherwise, and
just not (necessarily) drop things into functions.

If we had pytest, I'd suggest the tests were written and executed
separately (from one test script) and then results collected somehow
to the final aggregator.

(Recently I've been working with py.test-3 (and am not too happy with it,
perhaps it is better when testing python code, we use it for testing c code
with help of ctypes...) so I have some knowledge of how it works...

BTW: in case of pytest. be careful there is no `conftest.py` in any of the
directories starting / !!!, to mess up with your tests >;/ 

> You could also run one source file of tests with
>
> test_begin_subtest "python bindings foo tests"
> test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch/test_foo.py"

notmuch bätter... ;/

>
> that would give a less granular result, at the cost of more boilerplate


Tomi

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: pytest integration for the notmuch test suite
  2018-03-25 19:14               ` pytest integration for the notmuch test suite Tomi Ollila
@ 2018-03-26 11:31                 ` David Bremner
  0 siblings, 0 replies; 38+ messages in thread
From: David Bremner @ 2018-03-26 11:31 UTC (permalink / raw)
  To: Tomi Ollila, Justus Winter, Floris Bruynooghe, W. Trevor King; +Cc: notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sun, Mar 25 2018, David Bremner wrote:
>
>> Here's one approach. A given pytest "file" can be embedded in a normal
>> (for us) test script.  As I write this, it occurs to me you might be
>> thinking of embedding unit tests in the bindings source files; that
>> would be easy to add, something along the lines of
>>
>> test_begin_subtest "python bindings embedded unit tests"
>> test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch"
>
> Hmm.
>
> Looks a bit strange to embed the pytest snippets into shell script and then
> execute each of these individiually. The only thing py.test seems to do here is
> "visualizing" assert output. We could just use normal python otherwise, and
> just not (necessarily) drop things into functions.

Yes, I had pretty much the same thought. It's also true that pytest
allows mixing tests with regular definition, if desired (i.e. it can
search for all the functions starting test_ or some other marker).
It's also true this doesn't really cost more at run time than the
current test_python; they both exec the python interpreter.

> If we had pytest, I'd suggest the tests were written and executed
> separately (from one test script) and then results collected somehow
> to the final aggregator.

Sure, that sounds nicer. I just don't want to invest a lot in writing
glue before it's clear what is needed. If start by running the pytest
files the crude way, we can always improve the glue later. That does
argue against doing many embedded snippets, I agree.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-21 10:16           ` New Python bindings (was: Crash with Python bindings) Justus Winter
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
@ 2018-03-26 20:47             ` Floris Bruynooghe
  2018-03-27 22:29               ` New Python bindings Justus Winter
  2018-03-28  7:20             ` New Python bindings (was: Crash with Python bindings) Brian May
  2 siblings, 1 reply; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-26 20:47 UTC (permalink / raw)
  To: Justus Winter, David Bremner, W. Trevor King; +Cc: notmuch

On Wed, Mar 21 2018, Justus Winter wrote:
>
> Floris Bruynooghe <flub@devork.be> writes:
>
>> This is exactly what I have fixed in my alternative bindings which I
>> created around the end of last year [0].  So we do have an idea of how
>> to fix this, at the time I said I do believe that it's possible to also
>> do this for the existing bindings even though it is a lot of work.
>> After some talking between dkg and me we got to a way forward which
>> proposed this, but I must admit that after messing a little with getting
>> a pytest run integrated into the notmuch test-suite instead of using tox
>> I lost momentum on the project and didn't advance any further.
>
> I'm sorry that I didn't speak up when you announced your work.  I'm
> actually excited about a new set of bindings for Python.  I agree with
> using cffi.  I briefly looked at the code, and I believe it is much
> nicer than what we currently have.

Nice to hear, thanks!

> I trust that it works fine with Python 3, does it?

The version I made so far *only* works on Python 3.  Mostly because it
was easier, but it also allows some API niceties.

> The testsuite cannot depend on pulling stuff from the net simply because
> build servers typically do not have access to it.  That is a hard
> requirement.

Sure I understand that.  See another part of this thread on this though.


Cheers,
Floris

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/3] configure: check for pytest binary
  2018-03-25 17:40               ` [PATCH 1/3] configure: check for pytest binary David Bremner
@ 2018-03-26 20:55                 ` Floris Bruynooghe
  0 siblings, 0 replies; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-26 20:55 UTC (permalink / raw)
  To: David Bremner, Justus Winter, David Bremner, W. Trevor King; +Cc: notmuch

On Sun, Mar 25 2018, David Bremner wrote:

> This is to support future use of pytest in the test suite

Thanks for having a go at this!

> ---
>  configure | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/configure b/configure
> index b177b141..ab45878d 100755
> --- a/configure
> +++ b/configure
> @@ -62,6 +62,7 @@ CXXFLAGS=${CXXFLAGS:-\$(CFLAGS)}
>  LDFLAGS=${LDFLAGS:-}
>  XAPIAN_CONFIG=${XAPIAN_CONFIG:-}
>  PYTHON=${PYTHON:-}
> +PYTEST=${PYTEST:-}
>  
>  # We don't allow the EMACS or GZIP Makefile variables inherit values
>  # from the environment as we do with CC and CXX above. The reason is
> @@ -118,6 +119,8 @@ Other environment variables can be used to control configure itself,
>  			library. [$XAPIAN_CONFIG]
>  	PYTHON		Name of python command to use in
>  			configure and the test suite.
> +        PYTEST		Name of pytest command to use in
> +                        the test suite.
>  
>  Additionally, various options can be specified on the configure
>  command line.
> @@ -571,6 +574,24 @@ if [ $have_python -eq 0 ]; then
>      errors=$((errors + 1))
>  fi
>  
> +pytest=""
> +if [ $have_python -eq 1 ]; then
> +    printf "Checking for pytest... "
> +    have_pytest=0
> +
> +    for name in ${PYTEST} pytest-3 pytest pytest-2; do

This is kind of not granular enough I think.  It would be better to
invoke pytest as "pythonX.Y -m pytest" which is the safe way to execute
it on all python versions.

> +        if command -v $name > /dev/null; then
> +            have_pytest=1
> +            pytest=$name
> +            printf "Yes (%s).\n" $pytest
> +            break
> +        fi
> +    done
> +    if [ $have_pytest -eq 0 ]; then
> +        printf "No (some tests may be skipped).\n"
> +    fi

The other thing I was trying to achieve was to be able to run the
unittest for each python version, so say 2.7, 3.5 & 3.6 are supported
then I was trying to find all of those instead of just one.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: pytest integration for the notmuch test suite
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
                                 ` (3 preceding siblings ...)
  2018-03-25 19:14               ` pytest integration for the notmuch test suite Tomi Ollila
@ 2018-03-26 21:01               ` Floris Bruynooghe
  2018-03-26 21:25                 ` David Bremner
  4 siblings, 1 reply; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-26 21:01 UTC (permalink / raw)
  To: David Bremner, Justus Winter, David Bremner, W. Trevor King; +Cc: notmuch

On Sun, Mar 25 2018, David Bremner wrote:

> Here's one approach. A given pytest "file" can be embedded in a normal
> (for us) test script.  As I write this, it occurs to me you might be
> thinking of embedding unit tests in the bindings source files; that
> would be easy to add, something along the lines of
>
> test_begin_subtest "python bindings embedded unit tests"
> test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch"

I was trying to construct something where a full pytest run on one
python version was one subtest.  For granularity I think treating an
entire pytest run as a subtest with just checking the return code should
be sufficient,
e.g. `python2.7 -m pytest ${NOTMUCH_SRCDIR}/bindings/python/notmuch`.

But the whole test in this case would be this same subtest but once with
python2.7, python3.5, python3.6 etc.

What do you think of this?

Cheers,
Floris

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: pytest integration for the notmuch test suite
  2018-03-26 21:01               ` Floris Bruynooghe
@ 2018-03-26 21:25                 ` David Bremner
  0 siblings, 0 replies; 38+ messages in thread
From: David Bremner @ 2018-03-26 21:25 UTC (permalink / raw)
  To: Floris Bruynooghe, Justus Winter, W. Trevor King; +Cc: notmuch

Floris Bruynooghe <flub@devork.be> writes:

> On Sun, Mar 25 2018, David Bremner wrote:
>
>> Here's one approach. A given pytest "file" can be embedded in a normal
>> (for us) test script.  As I write this, it occurs to me you might be
>> thinking of embedding unit tests in the bindings source files; that
>> would be easy to add, something along the lines of
>>
>> test_begin_subtest "python bindings embedded unit tests"
>> test_expect_success "${NOTMUCH_PYTEST} ${NOTMUCH_SRCDIR}/bindings/python/notmuch"
>
> I was trying to construct something where a full pytest run on one
> python version was one subtest.  For granularity I think treating an
> entire pytest run as a subtest with just checking the return code should
> be sufficient,
> e.g. `python2.7 -m pytest ${NOTMUCH_SRCDIR}/bindings/python/notmuch`.
>
> But the whole test in this case would be this same subtest but once with
> python2.7, python3.5, python3.6 etc.
>
> What do you think of this?

The notmuch build system currently looks for a single version of
python. I'm pretty sure we don't want to require multiple python
versions to build notmuch, although I guess it wouldn't be that hard to
detect whatever versions exist in the environment and run pytest with
each of them. I just need a list of names to look for (or a better way
to find all the pythons installed). Debian has the handy comand
"py3versions --installed", but I suppose that's completely unportable.

Then each one can be checked to see if it finds the pytest module.

It sounds doable to me; it needs a bit more work in configure, but I can
drop the stuff in test-lib.sh

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings
  2018-03-26 20:47             ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
@ 2018-03-27 22:29               ` Justus Winter
  2018-03-28 22:07                 ` Floris Bruynooghe
  0 siblings, 1 reply; 38+ messages in thread
From: Justus Winter @ 2018-03-27 22:29 UTC (permalink / raw)
  To: Floris Bruynooghe, David Bremner, W. Trevor King; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]

Floris Bruynooghe <flub@devork.be> writes:

> On Wed, Mar 21 2018, Justus Winter wrote:
>>
>> Floris Bruynooghe <flub@devork.be> writes:
>>
>>> This is exactly what I have fixed in my alternative bindings which I
>>> created around the end of last year [0].  So we do have an idea of how
>>> to fix this, at the time I said I do believe that it's possible to also
>>> do this for the existing bindings even though it is a lot of work.
>>> After some talking between dkg and me we got to a way forward which
>>> proposed this, but I must admit that after messing a little with getting
>>> a pytest run integrated into the notmuch test-suite instead of using tox
>>> I lost momentum on the project and didn't advance any further.
>>
>> I'm sorry that I didn't speak up when you announced your work.  I'm
>> actually excited about a new set of bindings for Python.  I agree with
>> using cffi.  I briefly looked at the code, and I believe it is much
>> nicer than what we currently have.
>
> Nice to hear, thanks!

Thanks for all the work :)

>> I trust that it works fine with Python 3, does it?
>
> The version I made so far *only* works on Python 3.  Mostly because it
> was easier, but it also allows some API niceties.

Reasonable choice.  Which versions of Python 3 are supported?  I am also
writing bindings and I am wondering which versions to target.

Cheers,
Justus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-21 10:16           ` New Python bindings (was: Crash with Python bindings) Justus Winter
  2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
  2018-03-26 20:47             ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
@ 2018-03-28  7:20             ` Brian May
  2018-03-28 13:42               ` David Bremner
  2018-03-28 22:10               ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
  2 siblings, 2 replies; 38+ messages in thread
From: Brian May @ 2018-03-28  7:20 UTC (permalink / raw)
  To: notmuch

Justus Winter <justus@sequoia-pgp.org> writes:

>> This is exactly what I have fixed in my alternative bindings which I
>> created around the end of last year [0].  So we do have an idea of how
>> to fix this, at the time I said I do believe that it's possible to also
>> do this for the existing bindings even though it is a lot of work.
>> After some talking between dkg and me we got to a way forward which
>> proposed this, but I must admit that after messing a little with getting
>> a pytest run integrated into the notmuch test-suite instead of using tox
>> I lost momentum on the project and didn't advance any further.
>
> I'm sorry that I didn't speak up when you announced your work.  I'm
> actually excited about a new set of bindings for Python.  I agree with
> using cffi.  I briefly looked at the code, and I believe it is much
> nicer than what we currently have.

I can into this thread late. However, my priorities for python bindings
would be:

* I don't care about anything less then Python 3.5.
* Reliable Python 3.6 support important.
* Packages should be available from pypi.python.org

I have found the current official bindings unreliable with Python 3.x,
and tend to cause aborts on exiting and/or fail to save updates. As
such, for now I have downgraded to Python 2.7 for now.
-- 
Brian May <brian@linuxpenguins.xyz>
https://linuxpenguins.xyz/brian/

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-28  7:20             ` New Python bindings (was: Crash with Python bindings) Brian May
@ 2018-03-28 13:42               ` David Bremner
  2018-03-28 22:15                 ` Floris Bruynooghe
  2018-03-28 22:37                 ` Brian May
  2018-03-28 22:10               ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
  1 sibling, 2 replies; 38+ messages in thread
From: David Bremner @ 2018-03-28 13:42 UTC (permalink / raw)
  To: Brian May, notmuch

Brian May <brian@linuxpenguins.xyz> writes:

> I can into this thread late. However, my priorities for python bindings
> would be:

[...]
> * Packages should be available from pypi.python.org
>

We tried this before, and it didn't work out very well. Bindings tend to
depend on a strict matching of versions with the underlying library, so
distributing them seperately doesn't really make sense to me. You need
the underlying libraries, so why not get the matching bindings from the
same place?  We found that the situation was exacerbated by the fact
that no-one cared about updating the bindings on pypi.

Projects like numpy seem to get around this by distributing compiled
shared libraries on pypi. That's fine if someone wants to do it, but it
looks like "just another distro" to me, and not really an upstream
problem.  I guess we'd entertain minor tweaks to the build system to
support that, but probably not a wholesale conversion.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings
  2018-03-27 22:29               ` New Python bindings Justus Winter
@ 2018-03-28 22:07                 ` Floris Bruynooghe
  0 siblings, 0 replies; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-28 22:07 UTC (permalink / raw)
  To: Justus Winter, David Bremner, W. Trevor King; +Cc: notmuch

On Wed, Mar 28 2018, Justus Winter wrote:

> Floris Bruynooghe <flub@devork.be> writes:
>
>> On Wed, Mar 21 2018, Justus Winter wrote:
>>>
>>> Floris Bruynooghe <flub@devork.be> writes:
>>>
>>>> This is exactly what I have fixed in my alternative bindings which I
>>>> created around the end of last year [0].  So we do have an idea of how
>>>> to fix this, at the time I said I do believe that it's possible to also
>>>> do this for the existing bindings even though it is a lot of work.
>>>> After some talking between dkg and me we got to a way forward which
>>>> proposed this, but I must admit that after messing a little with getting
>>>> a pytest run integrated into the notmuch test-suite instead of using tox
>>>> I lost momentum on the project and didn't advance any further.
>>>
>>> I'm sorry that I didn't speak up when you announced your work.  I'm
>>> actually excited about a new set of bindings for Python.  I agree with
>>> using cffi.  I briefly looked at the code, and I believe it is much
>>> nicer than what we currently have.
>>
>> Nice to hear, thanks!
>
> Thanks for all the work :)
>
>>> I trust that it works fine with Python 3, does it?
>>
>> The version I made so far *only* works on Python 3.  Mostly because it
>> was easier, but it also allows some API niceties.
>
> Reasonable choice.  Which versions of Python 3 are supported?  I am also
> writing bindings and I am wondering which versions to target.

Personally I consider python3.5, pypy3.5 and python3.6 the ones to
target if I have no other constraints, which was the case here.  For
upstreaming into notmuch proper there are naturally other constraints
;-)  But unless you need something specific I think 3.4 is when py3k
became the better version than 2.7, everything below that is probably
not worth it.  All IMHO obviously.

Cheers,
Floris

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-28  7:20             ` New Python bindings (was: Crash with Python bindings) Brian May
  2018-03-28 13:42               ` David Bremner
@ 2018-03-28 22:10               ` Floris Bruynooghe
  1 sibling, 0 replies; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-28 22:10 UTC (permalink / raw)
  To: Brian May, notmuch

On Wed, Mar 28 2018, Brian May wrote:

> Justus Winter <justus@sequoia-pgp.org> writes:
>
>>> This is exactly what I have fixed in my alternative bindings which I
>>> created around the end of last year [0].  So we do have an idea of how
>>> to fix this, at the time I said I do believe that it's possible to also
>>> do this for the existing bindings even though it is a lot of work.
>>> After some talking between dkg and me we got to a way forward which
>>> proposed this, but I must admit that after messing a little with getting
>>> a pytest run integrated into the notmuch test-suite instead of using tox
>>> I lost momentum on the project and didn't advance any further.
>>
>> I'm sorry that I didn't speak up when you announced your work.  I'm
>> actually excited about a new set of bindings for Python.  I agree with
>> using cffi.  I briefly looked at the code, and I believe it is much
>> nicer than what we currently have.
>
> I can into this thread late. However, my priorities for python bindings
> would be:
>
> * I don't care about anything less then Python 3.5.
> * Reliable Python 3.6 support important.
> * Packages should be available from pypi.python.org

Well, the 1st two should already be covered on my
https://github.com/flub/notmuch/tree/cffi branch.  There's obviously
still room for improvement.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-28 13:42               ` David Bremner
@ 2018-03-28 22:15                 ` Floris Bruynooghe
  2018-03-28 22:37                 ` Brian May
  1 sibling, 0 replies; 38+ messages in thread
From: Floris Bruynooghe @ 2018-03-28 22:15 UTC (permalink / raw)
  To: David Bremner, Brian May, notmuch

On Wed, Mar 28 2018, David Bremner wrote:

> Brian May <brian@linuxpenguins.xyz> writes:
>
>> I can into this thread late. However, my priorities for python bindings
>> would be:
>
> [...]
>> * Packages should be available from pypi.python.org
>>
>
> We tried this before, and it didn't work out very well. Bindings tend to
> depend on a strict matching of versions with the underlying library, so
> distributing them seperately doesn't really make sense to me. You need
> the underlying libraries, so why not get the matching bindings from the
> same place?  We found that the situation was exacerbated by the fact
> that no-one cared about updating the bindings on pypi.

I did build a

#if LIBNOTMUCH_MAJOR_VERSION < 5
    #error libnotmuch version not supported by notdb
#endif

into my current bindings which kind of allows you to do this to some,
hopefully reasonable, level.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-28 13:42               ` David Bremner
  2018-03-28 22:15                 ` Floris Bruynooghe
@ 2018-03-28 22:37                 ` Brian May
  2018-03-28 23:13                   ` David Bremner
  1 sibling, 1 reply; 38+ messages in thread
From: Brian May @ 2018-03-28 22:37 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> We tried this before, and it didn't work out very well. Bindings tend to
> depend on a strict matching of versions with the underlying library, so
> distributing them seperately doesn't really make sense to me. You need
> the underlying libraries, so why not get the matching bindings from the
> same place?  We found that the situation was exacerbated by the fact
> that no-one cared about updating the bindings on pypi.

I believe that is the purpose of Python Wheels.

https://pythonwheels.com/

pypi is the defecto standard for distributing Python code for use in
Python applications. It means packages that use notmuch just need to
list it as a dependancy in 'requirements.txt' and a 'pip install -r
requirements.txt' will install everything required (if inside a
virtualenv no root access required even).

There are also various solutions to get automatic deploys to pypi, for
example through travis:

https://docs.travis-ci.com/user/deployment/pypi/

Unfortunately, I think many people will not even consider using a python
library unless it has up-to-date bindings available on pypi.
-- 
Brian May <brian@linuxpenguins.xyz>
https://linuxpenguins.xyz/brian/

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-28 22:37                 ` Brian May
@ 2018-03-28 23:13                   ` David Bremner
  2018-04-04 22:37                     ` Brian May
  0 siblings, 1 reply; 38+ messages in thread
From: David Bremner @ 2018-03-28 23:13 UTC (permalink / raw)
  To: Brian May, notmuch

Brian May <brian@linuxpenguins.xyz> writes:

> David Bremner <david@tethera.net> writes:
>
>> We tried this before, and it didn't work out very well. Bindings tend to
>> depend on a strict matching of versions with the underlying library, so
>> distributing them seperately doesn't really make sense to me. You need
>> the underlying libraries, so why not get the matching bindings from the
>> same place?  We found that the situation was exacerbated by the fact
>> that no-one cared about updating the bindings on pypi.
>
[...]
> Unfortunately, I think many people will not even consider using a python
> library unless it has up-to-date bindings available on pypi.

That's not an itch I personally have, but as I said in the next
paragraph, if someone want's to take on the project of maintaining a
wheel, we'll render the same kind of assistance we give *BSD/Linux/MacOS
package maintainers.  We're happy to look at (reasonable) things we can
do to make downstream projects life easier.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: New Python bindings (was: Crash with Python bindings)
  2018-03-28 23:13                   ` David Bremner
@ 2018-04-04 22:37                     ` Brian May
  2018-04-05  1:09                       ` Pypi David Bremner
  0 siblings, 1 reply; 38+ messages in thread
From: Brian May @ 2018-04-04 22:37 UTC (permalink / raw)
  To: David Bremner, notmuch

David Bremner <david@tethera.net> writes:

> That's not an itch I personally have, but as I said in the next
> paragraph, if someone want's to take on the project of maintaining a
> wheel, we'll render the same kind of assistance we give *BSD/Linux/MacOS
> package maintainers.  We're happy to look at (reasonable) things we can
> do to make downstream projects life easier.

Fair enough. No problem.

I am going to assume that the notmuch library is reasonable stable, and
backward incompatable changes are kept to a minimum with proper updating
of the shared library SONAME. If this is not the case, ignore the rest
of this email.

Ideally the python bindings should be in a git repository that is
separate from the C library. This means you don't have to release new
python bindings for every new source code release of notmuch. You only
need to make a new release if supporting new features or a new release
that breaks backword compatability. It also will make it easier to build
the python libraries standalone using the installed versions of the C
library, which I suspect might make pypi support a lot easier.

I might be able to get time to look at this sometime myself, if nobody
beats me to it.
-- 
Brian May <brian@linuxpenguins.xyz>
https://linuxpenguins.xyz/brian/

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Pypi
  2018-04-04 22:37                     ` Brian May
@ 2018-04-05  1:09                       ` David Bremner
  0 siblings, 0 replies; 38+ messages in thread
From: David Bremner @ 2018-04-05  1:09 UTC (permalink / raw)
  To: Brian May, notmuch

Brian May <brian@linuxpenguins.xyz> writes:

> Ideally the python bindings should be in a git repository that is
> separate from the C library. This means you don't have to release new
> python bindings for every new source code release of notmuch. You only
> need to make a new release if supporting new features or a new release
> that breaks backword compatability. It also will make it easier to build
> the python libraries standalone using the installed versions of the C
> library, which I suspect might make pypi support a lot easier.

Currently the notmuch test suite uses (and tests) the python
bindings. Having the notmuch library build-depend on a seperate python
bindings package would create (I think) a circular build dependency.

Splitting the repo would also break all the existing distro
packaging of the bindings (e.g. for debian).

So, not extremely keen to do that, at the moment.

d

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2018-04-05  1:09 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-12  9:41 Crash with Python bindings Konrad Hinsen
2016-01-12 13:11 ` David Bremner
2016-01-12 14:21   ` Konrad Hinsen
2016-01-12 15:26     ` David Bremner
2016-01-12 19:03       ` David Bremner
2016-01-12 19:13         ` Binding access to ~/.notmuch-config (was: Crash with Python bindings) W. Trevor King
2016-01-13 11:25           ` Binding access to ~/.notmuch-config Konrad Hinsen
2016-01-13 12:25             ` David Bremner
2016-01-13 17:23               ` W. Trevor King
     [not found] ` <20160112102329.4269.20741@thinkbox.jade-hamburg.de>
2016-01-12 14:23   ` Crash with Python bindings Konrad Hinsen
2016-01-12 18:51     ` W. Trevor King
2018-03-16 11:59       ` David Bremner
2018-03-16 12:12         ` Justus Winter
2018-03-16 18:30         ` Floris Bruynooghe
2018-03-16 22:40           ` Daniel Kahn Gillmor
2018-03-18  8:01             ` Floris Bruynooghe
2018-03-21 10:16           ` New Python bindings (was: Crash with Python bindings) Justus Winter
2018-03-25 17:40             ` pytest integration for the notmuch test suite David Bremner
2018-03-25 17:40               ` [PATCH 1/3] configure: check for pytest binary David Bremner
2018-03-26 20:55                 ` Floris Bruynooghe
2018-03-25 17:40               ` [PATCH 2/3] test: add new test_expect_pytest_success David Bremner
2018-03-25 17:40               ` [PATCH 3/3] test: add example test using pytest David Bremner
2018-03-25 19:14               ` pytest integration for the notmuch test suite Tomi Ollila
2018-03-26 11:31                 ` David Bremner
2018-03-26 21:01               ` Floris Bruynooghe
2018-03-26 21:25                 ` David Bremner
2018-03-26 20:47             ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
2018-03-27 22:29               ` New Python bindings Justus Winter
2018-03-28 22:07                 ` Floris Bruynooghe
2018-03-28  7:20             ` New Python bindings (was: Crash with Python bindings) Brian May
2018-03-28 13:42               ` David Bremner
2018-03-28 22:15                 ` Floris Bruynooghe
2018-03-28 22:37                 ` Brian May
2018-03-28 23:13                   ` David Bremner
2018-04-04 22:37                     ` Brian May
2018-04-05  1:09                       ` Pypi David Bremner
2018-03-28 22:10               ` New Python bindings (was: Crash with Python bindings) Floris Bruynooghe
2016-01-12 18:08 ` Crash with Python bindings W. Trevor King

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).