Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

[Daniel Kahn Gillmor]

On Tue 2017-11-28 23:46:11 +0100, Ruben Pollan wrote:
> Message.get_property (prop) returns a string with the value of the property 
> and
> Message.get_properties (prop, exact=False) returns a list [(key, value)]

This looks like a sensible approach to me.  I'd be curious to hear what
others think of this.

In considering the API design space here, it occurs to me that it might
be more pythonic for get_properties to return a dict like:

   { key: [ value, … ], key: [ value, … ] }

Any reason you chose one over the other?  My python-fu is shallow, so
please don't take my aesthetic guesswork as authoritative; but i'm
imagining a user wanting to grab a bunch of properties and then easily
access them by key, and the dict seems like the simple way to do that.

Also, does get_properties() work with prop=None to fetch all properties?
if so, maybe that should be the default?

To be clear, I'd be fine with a response that disagrees with these
suggestions (especially if it explains why) and then adopting this
patch; i just want to make sure they've been considered before we lock
in the API.

Thanks for working on this, Meskio!

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: DRAFT Introduce CFFI-based Python bindings

[David Bremner]

Floris Bruynooghe  writes:

>
> Lastly there are some downsides to the choices I made:
> - I ended up going squarely for CPython 3.6+.  Choosing Python
>   3 allowed better API design, e.g. with keyword-only parameters
>   etc.  Choosing CPython 3.4+ restricts the madness that can
>   happen with __del__ and gives some newer (tho now unused)
>   features in weakref.finalizer.
> - This is no longer drop-in compatible.
> - I haven't got to a stage where my initial goal of speed has
>   been proven yet.

I guess you'll have to convince the maintainers / users of alot and afew
that this makes sense before we go much further. I'd point out that
Debian stable is only at python 3.5, so that makes me a bit wary of this
(being able to run the test suite on debian stable and similar aged
distros useful for me, and I suspect other developers).

I know there are issues with memory management in the current bindings,
so that may be a strong reason to push to python 3.6; it seems to need
more investigation at the moment.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

[meskio]

Quoting Daniel Kahn Gillmor (2017-11-17 10:03:09)
> On Wed 2017-11-15 23:29:54 +0100, Ruben Pollan wrote:
> > Message.get_property (prop) returns a string with the value of the property.
> 
> Upon review, this is actually insufficient for making robust use of the
> session-key series :(
> 
> In particular, it only returns the first value for the session key
> returned.

See the last patch. I added the implementation of get_properties (and I use it 
in my session-key branch of alot). Hopefully this solves the problem.

> In the session-key series, i work around this by simply trying each
> session key against an encrypted part until i find one that works. It
> would be "cleaner" (more principled) to somehow associate each session
> key with the part(s) of the message file(s) that it is capable of
> decrypting, but that's a lot of bookkeeping -- i think it's actually
> "cleaner" (less code, less computation in the standard case) to just
> take the current approach.

Fair enough for me, I'm copying this approach in alot as well.

-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

[Ruben Pollan]

Message.get_property (prop) returns a string with the value of the property and
Message.get_properties (prop, exact=False) returns a list [(key, value)]
---
 bindings/python/notmuch/globals.py |  5 +++
 bindings/python/notmuch/message.py | 78 +-
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/bindings/python/notmuch/globals.py 
b/bindings/python/notmuch/globals.py
index 71426c84..801062dc 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -75,6 +75,11 @@ class NotmuchMessageS(Structure):
 NotmuchMessageP = POINTER(NotmuchMessageS)
 
 
+class NotmuchMessagePropertiesS(Structure):
+pass
+NotmuchMessagePropertiesP = POINTER(NotmuchMessagePropertiesS)
+
+
 class NotmuchTagsS(Structure):
 pass
 NotmuchTagsP = POINTER(NotmuchTagsS)
diff --git a/bindings/python/notmuch/message.py 
b/bindings/python/notmuch/message.py
index d5b98e4f..7b737943 100644
--- a/bindings/python/notmuch/message.py
+++ b/bindings/python/notmuch/message.py
@@ -19,7 +19,7 @@ Copyright 2010 Sebastian Spaeth 
 """
 
 
-from ctypes import c_char_p, c_long, c_uint, c_int
+from ctypes import c_char_p, c_long, c_uint, c_int, POINTER, byref
 from datetime import date
 from .globals import (
 nmlib,
@@ -29,6 +29,7 @@ from .globals import (
 NotmuchTagsP,
 NotmuchMessageP,
 NotmuchMessagesP,
+NotmuchMessagePropertiesP,
 NotmuchFilenamesP,
 )
 from .errors import (
@@ -113,6 +114,36 @@ class Message(Python3StringMixIn):
 _maildir_flags_to_tags.argtypes = [NotmuchMessageP]
 _maildir_flags_to_tags.restype = c_int
 
+"""notmuch_message_get_property"""
+_get_property = nmlib.notmuch_message_get_property
+_get_property.argtypes = [NotmuchMessageP, c_char_p, POINTER(c_char_p)]
+_get_property.restype = c_int
+
+"""notmuch_message_get_properties"""
+_get_properties = nmlib.notmuch_message_get_properties
+_get_properties.argtypes = [NotmuchMessageP, c_char_p, c_int]
+_get_properties.restype = NotmuchMessagePropertiesP
+
+"""notmuch_message_properties_valid"""
+_properties_valid = nmlib.notmuch_message_properties_valid
+_properties_valid.argtypes = [NotmuchMessagePropertiesP]
+_properties_valid.restype = bool
+
+"""notmuch_message_properties_value"""
+_properties_value = nmlib.notmuch_message_properties_value
+_properties_value.argtypes = [NotmuchMessagePropertiesP]
+_properties_value.restype = c_char_p
+
+"""notmuch_message_properties_key"""
+_properties_key = nmlib.notmuch_message_properties_key
+_properties_key.argtypes = [NotmuchMessagePropertiesP]
+_properties_key.restype = c_char_p
+
+"""notmuch_message_properties_move_to_next"""
+_properties_move_to_next = nmlib.notmuch_message_properties_move_to_next
+_properties_move_to_next.argtypes = [NotmuchMessagePropertiesP]
+_properties_move_to_next.restype = None
+
 #Constants: Flags that can be set/get with set_flag
 FLAG = Enum(['MATCH'])
 
@@ -433,6 +464,51 @@ class Message(Python3StringMixIn):
 _freeze.argtypes = [NotmuchMessageP]
 _freeze.restype = c_uint
 
+def get_property(self, prop):
+""" Retrieve the value for a single property key
+
+:param prop: The name of the property to get.
+:returns: String with the property value or None if there is no such
+  key. In the case of multiple values for the given key, the
+  first one is retrieved.
+:raises: :exc:`NotInitializedError` if message has not been
+ initialized
+"""
+if not self._msg:
+raise NotInitializedError()
+
+value = c_char_p("")
+status = Message._get_property(self._msg, prop, byref(value))
+if status != 0:
+raise NotmuchError(status)
+
+return value.value
+
+def get_properties(self, prop, exact=False):
+""" Get the properties for *message*, returning
+notmuch_message_properties_t object which can be used to iterate
+over all properties.
+
+:param prop: The name of the property to get.
+:param exact: if True, require exact match with key. Otherwise
+  treat as prefix.
+:returns: [(key, value)]
+:raises: :exc:`NotInitializedError` if message has not been
+ initialized
+"""
+if not self._msg:
+raise NotInitializedError()
+
+properties_list = []
+properties = Message._get_properties(self._msg, prop, exact)
+while Message._properties_valid(properties):
+key = Message._properties_key(properties)
+value = Message._properties_value(properties)
+properties_list.append((key, value))
+Message._properties_move_to_next(properties)
+
+return properties_list
+
 def freeze(self):
 """Freezes the current state of 'message' within the 

[PATCH] Introduce CFFI-based python bindings

[Floris Bruynooghe]

From: Floris Bruynooghe 

This introduces the beginnings of new CFFI-based Python bindings.
The bindings aim at:
- Better performance on pypy
- Easier to use Python-C interface
- More "pythonic"
  - The API should not allow invalid operations
  - Use native object protocol where possible
- Memory safety; whatever you do from python, it should not coredump.
---
 AUTHORS |   1 +
 bindings/python-cffi/notdb/__init__.py  |  52 +++
 bindings/python-cffi/notdb/_base.py | 141 +++
 bindings/python-cffi/notdb/_build.py| 140 +++
 bindings/python-cffi/notdb/_database.py | 577 
 bindings/python-cffi/notdb/_errors.py   | 114 ++
 bindings/python-cffi/notdb/_message.py  | 285 ++
 bindings/python-cffi/notdb/_tags.py | 319 +++
 bindings/python-cffi/setup.py   |  13 +
 bindings/python-cffi/tests/conftest.py  | 138 +++
 bindings/python-cffi/tests/test_base.py | 116 ++
 bindings/python-cffi/tests/test_database.py | 274 +
 bindings/python-cffi/tests/test_tags.py | 141 +++
 13 files changed, 2311 insertions(+)
 create mode 100644 bindings/python-cffi/notdb/__init__.py
 create mode 100644 bindings/python-cffi/notdb/_base.py
 create mode 100644 bindings/python-cffi/notdb/_build.py
 create mode 100644 bindings/python-cffi/notdb/_database.py
 create mode 100644 bindings/python-cffi/notdb/_errors.py
 create mode 100644 bindings/python-cffi/notdb/_message.py
 create mode 100644 bindings/python-cffi/notdb/_tags.py
 create mode 100644 bindings/python-cffi/setup.py
 create mode 100644 bindings/python-cffi/tests/conftest.py
 create mode 100644 bindings/python-cffi/tests/test_base.py
 create mode 100644 bindings/python-cffi/tests/test_database.py
 create mode 100644 bindings/python-cffi/tests/test_tags.py

diff --git a/AUTHORS b/AUTHORS
index 6d0f2de8..9ce9ce6f 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -28,3 +28,4 @@ ideas, inspiration, testing or feedback):
 Martin Krafft
 Keith Packard
 Jamey Sharp
+Google LLC
diff --git a/bindings/python-cffi/notdb/__init__.py 
b/bindings/python-cffi/notdb/__init__.py
new file mode 100644
index ..340f4388
--- /dev/null
+++ b/bindings/python-cffi/notdb/__init__.py
@@ -0,0 +1,52 @@
+"""Pythonic API to the notmuch database.
+
+Creating Objects
+
+
+Only the :class:`Database` object is meant to be created by the user.
+All other objects should be created from this initial object.  Users
+should consider their signatures implementation details.
+
+Errors
+==
+
+All errors occuring due to errors from the underlying notmuch database
+are subclasses of the :exc:`NotmuchError`.  Due to memory management
+it is possible to try and use an object after it has been freed.  In
+this case a :exc:`ObjectDestoryedError` will be raised.
+
+Memory Management
+=
+
+xxx
+"""
+
+from notdb import _capi
+from notdb._database import AtomicContext
+from notdb._database import Database
+from notdb._database import DbRevision
+from notdb._errors import NotmuchError
+from notdb._errors import OutOfMemoryError
+from notdb._errors import ReadOnlyDatabaseError
+from notdb._errors import XapianError
+from notdb._errors import FileError
+from notdb._errors import FileNotEmailError
+from notdb._errors import DuplicateMessageIdError
+from notdb._errors import NullPointerError
+from notdb._errors import TagTooLongError
+from notdb._errors import UnbalancedFreezeThawError
+from notdb._errors import UnbalancedAtomicError
+from notdb._errors import UnsupportedOperationError
+from notdb._errors import UpgradeRequiredError
+from notdb._errors import PathError
+from notdb._errors import IllegalArgumentError
+from notdb._errors import NoSuchHeaderError
+from notdb._errors import NoMessageError
+from notdb._errors import ObjectDestroyedError
+from notdb._message import Message
+from notdb._tags import ImmutableTagSet
+from notdb._tags import MutableTagSet
+from notdb._tags import TagsIter
+
+
+NOTMUCH_TAG_MAX = _capi.lib.NOTMUCH_TAG_MAX
diff --git a/bindings/python-cffi/notdb/_base.py 
b/bindings/python-cffi/notdb/_base.py
new file mode 100644
index ..8229b1a6
--- /dev/null
+++ b/bindings/python-cffi/notdb/_base.py
@@ -0,0 +1,141 @@
+import abc
+
+from notdb import _errors as errors
+
+
+class NotmuchObject(metaclass=abc.ABCMeta):
+"""Base notmuch object syntax.
+
+This base class exists to define the memory management handling
+required to use the notmuch library.  It is meant as an interface
+definition rather than a base class, though you can use it as a
+base class to ensure you don't forget part of the interface.  It
+only concerns you if you are implementing this package itself
+rather then using it.
+
+libnotmuch uses a hierarchical memory allocator, where freeing the
+memory of a parent object also frees the memory of all child
+objects.  To make 

DRAFT Introduce CFFI-based Python bindings

[Floris Bruynooghe]

Hi all,

Here are the beginnings off CFFI-based Python bindings, rather
than the ctypes-based ones currently available.  I started this
work in order to get faster bindings on pypy since a script of
mine was running slower on pypy than CPython.  Initially aiming
for a drop-in replacement of the existing bindings I ended up
abandoning this to help enforce correct usage of the API.

The benefits of this approach are:
- More "Pythonic" API, e.g. tags behave like sets, iterators
  which get consumed can easily be re-created as is usual with
  collections, avoid allowing invalid combinations of args and
  calls on a Python-API level.
- CFFI, this works on both CPython and PyPy, on the latter it
  is (supposed to be) a lot faster as the JIT can cross the
  boundary between C and Python code where it otherwise has
  extra overheads to emulate the C-Python API.  Additionally
  it makes it safer to use compared to ctypes, it works on the
  API level using the compiler to figure out the correct details
  of the platform.  Compared to ctypes which only works on the
  ABI level and you need to rely on knowing the layout of code
  when writing the Python bindings.

Additionaly I belive these bindings fix a memory safety issue,
certain situations in my test-suite would lead to coredump which
is not something which should be possible from within Python.
I believe I have seen similar reports in the list archives so
am not the only one seeing these.  Sadly these are hard to
isolate and I have not managed to re-create this in a nice
minimal example, however I believe the root cause is that in
some situations, mostly interpreter shutdown, the __del__
method can have been called while there are still references
to the object and while child-objects are still alive.  This
effectively results in double-frees as the child object frees
memory already freed by the parent.  These bindings solve this
by adding the .alive property and using this to check parent
objects are still alive before destroying themselves.  This is
somewhat expensive, but works and is easy to implement.

Lastly there are some downsides to the choices I made:
- I ended up going squarely for CPython 3.6+.  Choosing Python
  3 allowed better API design, e.g. with keyword-only parameters
  etc.  Choosing CPython 3.4+ restricts the madness that can
  happen with __del__ and gives some newer (tho now unused)
  features in weakref.finalizer.
- This is no longer drop-in compatible.
- I haven't got to a stage where my initial goal of speed has
  been proven yet.

In theory I think it's possible to create a CFFI-based drop-in
replacement to the bindings, only adding the memory-safety fixes
and keeping the Python 2.7 compatibility.  It would then be
possible to build the API proposed in these bindings on top of
this, but once I was making these bindings safer it felt strange
to still allow the API to be misused.


There are a lot of details about this which can be discussed,
also many finer implementation points and even just getting the
proposed API right (you'll notice large gaps for now).  But
this mail is already too long.  I look forward to your comments
and feedback on the approach taken and on whether some form
of this could make it into the main repo.


Lastly a small note on the AUTHORS file patch, due to my own
unfortunate choice of employer I have strict rules to follow
on how to submit patches.  One of which is to add this line if
an AUTHORS file exists.  Given clearly not everyone is listed
here though maybe this is not appropriate.  I would also rather
receive email on f...@devork.be rather than the address I have
to use in the git commits.


Kind Regards,
Floris


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Possible bug in notmuch_thread_get_authors ?

[David Bremner]

Róman Joost  writes:

> Checking other bindings it seems there is an explicit check for NULL and
> we're wondering if that is really necessary.  Perhaps a patch could
> initialize `*authors` in the _resolve_thread_authors_string function to an
> empty string OR at least mention it in the API documentation that the
> return value can be NULL?

Certainly the documentation update is fine. It's not clear to me without
diving into the code whether returning NULL is signalling an error, or
just a bug.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch