Re: [PATCH 1/2] doc: replace use of environment variables with a generated config

2020-07-14 Thread Floris Bruynooghe
On Sat 11 Jul 2020 at 17:00 +0300, Tomi Ollila wrote:

> On Sat, Jul 11 2020, David Bremner wrote:
>
>> I don't love the use of exec, but it is getting unwieldy to pass
>> configuration options on the sphinx-build command line, and I
>> anticipate further use of conditionals.
>
> Perhaps less "opinions" in commit message.
>
> (and as I think I don't comment 2/2, s/seperate/separate/ there)
>
>
>> ---
>>  configure  |  8 
>>  doc/Makefile.local |  2 +-
>>  doc/conf.py| 11 ---
>>  3 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 80cbac4f..177432db 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1548,6 +1548,14 @@ NOTMUCH_HAVE_PYTHON3_PYTEST=${have_python3_pytest}
>>  PLATFORM=${platform}
>>  EOF
>>  
>> +cat > sphinx.config <> +# Generate by configure, run from doc/conf.py
>> +EOF
>> +if [ $WITH_EMACS = "1" ]; then
>> +printf "tags.add('WITH_EMACS')\n" >> sphinx.config
>> +fi
>> +printf "rsti_dir = '%s'\n" $(realpath emacs) >> sphinx.config
>> +
>
> perhaps instead of multiple redirections to the file, 
>
> {
> echo "# Generated by configure, run from doc/conf.py"
> echo
> if [ $WITH_EMACS = "1" ]; then
> printf "tags.add('WITH_EMACS')\n"
> fi 
> printf "rsti_dir = '%s'\n" "$(realpath emacs)"
>
> } > sphinx.config
> 
> alternative (someone might think less readable... ;/):
>
>  exec 3>&1 1> sphinx.config
>
>  echo "# Generated by configure, run from doc/conf.py"
>  ...
>
>  exec 1>&3 3>&-

I'd probably prefer this last one, but I don't mind any of the solutions or
even the current one.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH] doc: set up for autoapi / readthedocs compatibility

2020-07-14 Thread Floris Bruynooghe
On Sun 12 Jul 2020 at 09:02 -0300, David Bremner wrote:

> sphinx-autoapi seems nicer conceptually (it parses the docs rather
> than importing them),

TIL about sphinx-autoapi, agree it's nicer conceptually.

> but it also generates a ton of warnings, so
> leave the default as autodoc.
> ---
>
> You can see the results of this (for now) at 
> https://notmuch.readthedocs.io/en/rtdtest/
> We'd presumable want to integrate the whole tree somehow into notmuchmail.org

Saldy it seems to struggle with a fair bit of things.  E.g. it does
manage to create a Database.MODE attribute, but it doesn't figure out
that this is the Mode enum and thus doesn't document the fact you have
two options: MODE.READ_ONLY and MODE.READ_WRITE.  There are obviously a
bunch of other enums where this matters.

It could be that these things are solvable by using more autodoc-style
directives:
https://sphinx-autoapi.readthedocs.io/en/latest/reference/directives.html

But I wonder if the autodoc one will always be better because of the
dynamic nature.  E.g. mapping notmuch2._database.Mode to
notmuch2.Database.MODE on the actual Public API.

I didn't actually try out how much better autodoc does, I should
probably try that too before commenting further.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH 2/2] doc: add new python bindings to main documentatation tree.

2020-07-14 Thread Floris Bruynooghe
Oh, this is very nice!  I've been thinking for a while I should attempt
this.  Great to see it being done!

On Sat 11 Jul 2020 at 10:20 -0300, David Bremner wrote:

> A seperate conf.py and doc directory will be needed if someone wants
> to build the bindings docs separately from notmuch.
> ---
>  configure   | 4 
>  doc/conf.py | 8 
>  doc/index.rst   | 1 +
>  doc/python-bindings.rst | 5 +
>  4 files changed, 18 insertions(+)
>  create mode 100644 doc/python-bindings.rst
>
> diff --git a/configure b/configure
> index 177432db..36fe4a9d 100755
> --- a/configure
> +++ b/configure
> @@ -801,6 +801,7 @@ if [ $have_python3 -eq 1 ]; then
>  if "$python" -c 'import cffi,setuptools; cffi.FFI().verify()' >/dev/null 
> 2>&1; then
>  printf "Yes.\n"
>  have_python3_cffi=1
> +WITH_PYTHON_DOCS=1
>  else
>  printf "No (will not install CFFI-based python bindings).\n"
>  fi
> @@ -1554,6 +1555,9 @@ EOF
>  if [ $WITH_EMACS = "1" ]; then
>  printf "tags.add('WITH_EMACS')\n" >> sphinx.config
>  fi
> +if [ $WITH_PYTHON_DOCS = "1" ]; then
> +printf "tags.add('WITH_PYTHON')\n" >> sphinx.config
> +fi
>  printf "rsti_dir = '%s'\n" $(realpath emacs) >> sphinx.config
>  
>  # Finally, after everything configured, inform the user how to continue.
> diff --git a/doc/conf.py b/doc/conf.py
> index fdff2a2c..94e266af 100644
> --- a/doc/conf.py
> +++ b/doc/conf.py
> @@ -4,6 +4,8 @@
>  import sys
>  import os
>  
> +extensions = [ 'sphinx.ext.autodoc' ]
> +
>  # The suffix of source filenames.
>  source_suffix = '.rst'
>  
> @@ -22,6 +24,9 @@ for pathdir in ['.', '..']:
>  with open(version_file,'r') as infile:
>  version=infile.read().replace('\n','')
>  
> +# for autodoc
> +sys.path.insert(0, os.path.join(location, '..', 'bindings', 'python-cffi', 
> 'notmuch2'))
> +
>  # read generated config
>  for pathdir in ['.', '..']:
>  conf_file = os.path.join(location,pathdir,'sphinx.config')
> @@ -50,6 +55,9 @@ else:
>  # the docstring include files
>  exclude_patterns.append('notmuch-emacs.rst')
>  
> +if not tags.has('WITH_PYTHON'):
> +exclude_patterns.append('python-bindings.rst')
> +
>  # The name of the Pygments (syntax highlighting) style to use.
>  pygments_style = 'sphinx'
>  
> diff --git a/doc/index.rst b/doc/index.rst
> index 4440d93a..a3bf3480 100644
> --- a/doc/index.rst
> +++ b/doc/index.rst
> @@ -26,6 +26,7 @@ Contents:
> man7/notmuch-search-terms
> man1/notmuch-show
> man1/notmuch-tag
> +   python-bindings
>  
>  Indices and tables
>  ==
> diff --git a/doc/python-bindings.rst b/doc/python-bindings.rst
> new file mode 100644
> index ..e1ad26ad
> --- /dev/null
> +++ b/doc/python-bindings.rst
> @@ -0,0 +1,5 @@
> +Python Bindings
> +===
> +
> +.. automodule:: notmuch2
> +   :members:
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH 1/2] doc: replace use of environment variables with a generated config

2020-07-14 Thread Floris Bruynooghe
On Sat 11 Jul 2020 at 10:20 -0300, David Bremner wrote:

> I don't love the use of exec, but it is getting unwieldy to pass

It's already a config file written in Python which is a terrible sin.
So no need to apologise, I think it makes sense in this context.

> configuration options on the sphinx-build command line, and I
> anticipate further use of conditionals.
> ---
>  configure  |  8 
>  doc/Makefile.local |  2 +-
>  doc/conf.py| 11 ---
>  3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 80cbac4f..177432db 100755
> --- a/configure
> +++ b/configure
> @@ -1548,6 +1548,14 @@ NOTMUCH_HAVE_PYTHON3_PYTEST=${have_python3_pytest}
>  PLATFORM=${platform}
>  EOF
>  
> +cat > sphinx.config < +# Generate by configure, run from doc/conf.py

Minor insignificant typo: Generated

> +EOF
> +if [ $WITH_EMACS = "1" ]; then
> +printf "tags.add('WITH_EMACS')\n" >> sphinx.config
> +fi
> +printf "rsti_dir = '%s'\n" $(realpath emacs) >> sphinx.config
> +
>  # Finally, after everything configured, inform the user how to continue.
>  cat <  
> diff --git a/doc/Makefile.local b/doc/Makefile.local
> index 769438ed..598b6028 100644
> --- a/doc/Makefile.local
> +++ b/doc/Makefile.local
> @@ -4,7 +4,7 @@ dir := doc
>  
>  # You can set these variables from the command line.
>  SPHINXOPTS:= -q
> -SPHINXBUILD   = WITH_EMACS=${WITH_EMACS} RSTI_DIR=$(realpath emacs) 
> sphinx-build
> +SPHINXBUILD   = sphinx-build
>  DOCBUILDDIR  := $(dir)/_build
>  
>  # Internal variables.
> diff --git a/doc/conf.py b/doc/conf.py
> index 70987ac5..fdff2a2c 100644
> --- a/doc/conf.py
> +++ b/doc/conf.py
> @@ -22,6 +22,13 @@ for pathdir in ['.', '..']:
>  with open(version_file,'r') as infile:
>  version=infile.read().replace('\n','')
>  
> +# read generated config
> +for pathdir in ['.', '..']:
> +conf_file = os.path.join(location,pathdir,'sphinx.config')
> +if os.path.exists(conf_file):
> +with open(conf_file,'r') as infile:
> +exec(''.join(infile.readlines()))
> +
>  # The full version, including alpha/beta/rc tags.
>  release = version
>  
> @@ -29,12 +36,10 @@ release = version
>  # directories to ignore when looking for source files.
>  exclude_patterns = ['_build']
>  
> -if os.environ.get('WITH_EMACS') == '1':
> +if tags.has('WITH_EMACS'):
>  # Hacky reimplementation of include to workaround limitations of
>  # sphinx-doc
>  lines = ['.. include:: /../emacs/rstdoc.rsti\n\n'] # in the source tree
> -rsti_dir = os.environ.get('RSTI_DIR')
> -# the other files are from the build tree
>  for file in ('notmuch.rsti', 'notmuch-lib.rsti', 'notmuch-show.rsti', 
> 'notmuch-tag.rsti'):
>  lines.extend(open(rsti_dir+'/'+file))
>  rst_epilog = ''.join(lines)
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [RFC PATCH] lib: document new database_open API

2020-07-08 Thread Floris Bruynooghe
On Sat 04 Jul 2020 at 14:01 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>>> + *
>>> + * - in the environment variable NOTMUCH_DATABASE, if non-empty
>>> + *
>>> + * - by $XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE where XDG_DATA_HOME
>>> + *   defaults to "$HOME/.local/share" and NOTMUCH_PROFILE defaults to
>>> + *   "default"
>>
>> I like the profile support, is the plan for
>> $XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE to be written when creating the
>> database?
>
> Yes, with "NOTMUCH_PROFILE=default" by default.
>
>> It's nice that the environment variable handling is done in the library,
>> should make it more consistent for bindings.  As long as it can be
>> overwritten I guess.
>
> Overwritten how? By passing parameters?

yes, that's what I meant.  Which I think you design here allows.  Just
takes a while to figure out what the right parameter combination
is... ;)

>> The API is rather complex though, perhaps easier when split across
>> several functions?  Maybe a notmuch_database_open_profile(const char
>> *profile, notmuch_database_t**) is useful as the simple one which always
>> does the right thing when called with NULL for profile.  Not sure what
>> other combinations would be needed.
>
> I have no objections to a "do the write thing" wrapper or two. I don't
> think that increases maintence cost too much.
>
> d
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH 2/4] lib: catch error from closed db in n_m_get_message_id

2020-07-08 Thread Floris Bruynooghe
On Sun 05 Jul 2020 at 08:17 -0300, David Bremner wrote:

> David Bremner  writes:
>
>> Floris Bruynooghe  writes:
>>
>>> notmuch_database_get_version currently returns and unsigned int and
>>> segfaults on use with a closed db.
>>
>> Yes, the ones without a proper status value are going to be a bit work.
>>
>> In the next series I just posted [1], I started providing status value
>> returning version (see notmuch_message_get_flag_st). We've been through
>> a few of these migrations and it has not been too painful.
>>
>
> I thought of another variation for the boolean valued functions. We
> could embed the boolean values in the notmuch_status_t value by adding
> one or more new status values corresponding to TRUE and FALSE. I'm not
> sure if that would be much simpler, but it would avoid the use of output
> parameters.

This also seems very reasonable.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH 2/4] lib: catch error from closed db in n_m_get_message_id

2020-07-08 Thread Floris Bruynooghe
On Sat 04 Jul 2020 at 14:17 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>
>>> - * This function will not return NULL since Notmuch ensures that every
>>> - * message has a unique message ID, (Notmuch will generate an ID for a
>>> - * message if the original file does not contain one).
>>> + * This function will return NULL if triggers an unhandled Xapian
>>> + * exception.
>
>> How much of a departure from the existing API is this?  Will this be
>> possible with all functions?  I had a quick look and tried some other
>> functions that don't return notmuch_status_t:
>
> It's upward compatible in that any code which crashes because it was not
> expecting a NULL pointer, will already be crashing in the same
> circumstances because of an uncaught exception / call to abort.

Oh yes, that is a very good point.  This choice seems very reason then.


Cheers,
Floris
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [RFC PATCH] lib: document new database_open API

2020-07-04 Thread Floris Bruynooghe
On Fri 03 Jul 2020 at 10:43 -0300, David Bremner wrote:

> Several aspects of this are potentially controversial:
>
> 1) The use of environment variables as fallback. I understand the
> discomfort with having a library function check the environment, but
> this seems to be functionality people want, and it is better to
> implement it once.
>
> 2) The use of both NULL and "" to do different things for config_path.
>
> 3) The new API is pretty complex, compared to the previous one.
> ---
>
> There is not much code to back this so far. This is just me thinking
> out loud at this point.  The location calculation is done (and also
> easy).  The challenging part is probably updating
> notmuch_database_get_config to do what this comment promises.
>
> I suspect database_create will probably need to be updated to match.
>
> Another question is if we should have an opaque set of options to pass
> to open (in the style notmuch_indexopts_t).  Currently this is just
> future proofing as far as I know.
>
>  lib/notmuch.h | 138 --
>  1 file changed, 111 insertions(+), 27 deletions(-)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index ceb5a018..6a46b80a 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -301,52 +301,136 @@ typedef enum {
>  } notmuch_database_mode_t;
>  
>  /**
> - * Open an existing notmuch database located at 'path'.
> + * Deprecated alias for notmuch_database_open_with_config with
> + * config_path=error_message=NULL
> + * @deprecated Deprecated as of libnotmuch 5.2 (notmuch 0.31)
> + */
> +NOTMUCH_DEPRECATED(5, 2)
> +notmuch_status_t
> +notmuch_database_open (const char *path,
> +notmuch_database_mode_t mode,
> +notmuch_database_t **database);
> +/**
> + * Deprecated alias for notmuch_database_open_with_config with
> + * config_path=NULL
> + *
> + * @deprecated Deprecated as of libnotmuch 5.2 (notmuch 0.31)
> + *
> + */
> +NOTMUCH_DEPRECATED(5, 2)
> +notmuch_status_t
> +notmuch_database_open_verbose (const char *path,
> +notmuch_database_mode_t mode,
> +notmuch_database_t **database,
> +char **error_message);
> +
> +/**
> + * Open an existing notmuch database located at 'database_path', using
> + * configuration in 'config_path'.
> + *
> + * @param[in]database_path
> + * @parblock
> + * Path to existing database.
> + *
> + * A notmuch database is a Xapian database containing appropriate
> + * metadata.
>   *
>   * The database should have been created at some time in the past,
>   * (not necessarily by this process), by calling
> - * notmuch_database_create with 'path'. By default the database should be
> - * opened for reading only. In order to write to the database you need to
> - * pass the NOTMUCH_DATABASE_MODE_READ_WRITE mode.
> + * notmuch_database_create.
> + *
> + * If 'database_path' is NULL, use the location specified
> + *
> + * - in the environment variable NOTMUCH_DATABASE, if non-empty
> + *
> + * - by $XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE where XDG_DATA_HOME
> + *   defaults to "$HOME/.local/share" and NOTMUCH_PROFILE defaults to
> + *   "default"

I like the profile support, is the plan for
$XDG_DATA_HOME/notmuch/$NOTMUCH_PROFILE to be written when creating the
database?

> + *
> + * If 'database_path' is non-NULL, but does not appear to be a Xapian
> + * database, check for a directory '.notmuch/xapian' below
> + * 'database_path'.
> + *
> + * @endparblock
> + * @param[in]mode
> + * @parblock
> + * Mode to open database
> + *
> + * By default the database will be opened for reading only. In order
> + * to write to the database you need to pass the
> + * #NOTMUCH_DATABASE_MODE_READ_WRITE mode.
> + *
> + * @endparblock
> + * @param[in]  config_path
> + * @parblock
> + * Path to config file.
> + *
> + * Config file is key-value, with mandatory sections. See
> + * notmuch-config(5) for more information. The key-value pair
> + * overrides the corresponding configuration data stored in the
> + * database (see notmuch_database_get_config)
>   *
> - * An existing notmuch database can be identified by the presence of a
> - * directory named ".notmuch" below 'path'.
> + * If config_path is NULL use the path specified
> + *
> + * - in environment variable NOTMUCH_CONFIG, if non-empty
> + *
> + * - by  XDG_CONFIG_HOME/notmuch/ where
> + *   XDG_CONFIG_HOME defaults to "$HOME/.config".
> + *
> + * - by $HOME/.notmuch-config
> + *
> + * If config_path is "" (empty string) then do not
> + * open any configuration file.
> + * @endparblock
> + * @param[in] profile:
> + * @parblock
> + * Name of profile (configuration/database variant).
> + *
> + * If non-NULL, append to the directory / file path determined by
> + * config_path.
> + *
> + * If NULL then use
> + * - environment variable NOTMUCH_PROFILE if defined,
> + * - otherwise "default" for directories and "" (empty string) for paths.
> + *
> + * 

Re: [PATCH 2/4] lib: catch error from closed db in n_m_get_message_id

2020-07-04 Thread Floris Bruynooghe
Nice.

On Mon 29 Jun 2020 at 22:14 -0300, David Bremner wrote:
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index ceb5a018..0dc89547 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -1363,9 +1363,8 @@ notmuch_message_get_database (const notmuch_message_t 
> *message);
>   * message is valid, (which is until the query from which it derived
>   * is destroyed).
>   *
> - * This function will not return NULL since Notmuch ensures that every
> - * message has a unique message ID, (Notmuch will generate an ID for a
> - * message if the original file does not contain one).
> + * This function will return NULL if triggers an unhandled Xapian
> + * exception.
>   */
>  const char *
>  notmuch_message_get_message_id (notmuch_message_t *message);

How much of a departure from the existing API is this?  Will this be
possible with all functions?  I had a quick look and tried some other
functions that don't return notmuch_status_t:

notmuch_database_get_version currently returns and unsigned int and
segfaults on use with a closed db.

notmuch_needs_upgrade returns notmuch_bool_t and seems to return a valid
bool with a closed db.  I'm not sure if this is always the right answer
though.

I wonder if a backwards-compatible errno-style API could work,
notmuch_last_status(notmuch_database_t* database) or so.  This kind of
thing is probably easy to adopt in bindings but harder for direct users
of the API.  It's also an extra API call for everything that doesn't
return notmuch_status_t.  But I'll leave the judgement to you, I'm not
as experienced with the API.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: Usage after database close

2020-06-29 Thread Floris Bruynooghe
On Sun 28 Jun 2020 at 19:11 -0300, David Bremner wrote:
> You need to add a seperate repo for the new style debug symbols in
> Debian:
> $ (git)-[master]-% apt policy libxapian30-dbgsym
> libxapian30-dbgsym:
>   Installed: 1.4.15-1
>   Candidate: 1.4.15-1
>   Version table:
>  *** 1.4.15-1 500
> 500 http://debug.mirrors.debian.org/debian-debug testing-debug/main 
> amd64 Packages
> 500 http://debug.mirrors.debian.org/debian-debug unstable-debug/main 
> amd64 Packages
> 100 /var/lib/dpkg/status

My goodness, I completely missed the dbgsym memo.  Thanks!
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 2/2] bindings/python-cffi: update version from global version.

2020-06-29 Thread Floris Bruynooghe
On Thu 25 Jun 2020 at 10:34 -0300, David Bremner wrote:

> David Bremner  writes:
>
>> Copy machinery from the older python bindings
>
>>  
>> +# get the notmuch version number without importing the notmuch module
>> +version_file = os.path.join(os.path.dirname(__file__),
>> +'notmuch2', 'version.py')
>> +exec(compile(open(version_file).read(), version_file, 'exec'))
>> +assert '__VERSION__' in globals(), \
>> +'Failed to read the notmuch binding version number'
>
> I wrote a cover letter for this, but that seems to have gotten lost. My
> main point was I'm not sure why this is better than Floris's version,
> since they both read a file when setup.py is run. I don't understand (or
> use) pip, so someone else will have to figure this out. If the
> constraint is that the version has to be hardcoded in setup.py then (as
> much as that sounds like a design mistake), we can apply similar sed
> hackery directly to setup.py. Perhaps someone can remember why we didn't
> do that for the old python bindings.

For some reason this is the only mail in this thread I have, so I don't
actually know the patch.

I think it can be simpler though, is it possible to copy the toplevel
version file into bindings/python-cffi/version in the part of the build
that would otherwise do the sed magic?  Then setup.py only needs to look
for the version file in the same directory as itself instead of finding
the toplevel of the repo.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Usage after database close

2020-06-28 Thread Floris Bruynooghe
On Sun 28 Jun 2020 at 13:19 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>> Hi,
>>
>> I started writing some test cases to define better what you can do with
>> a closed database and make sure that the python bindings do not behave
>> unexpectedly here too.
>>
>> One of the first things I tried ends up with xapian calling
>> exit_group(2) directly, terminating the process.  So I'm wondering if
>> I'm approaching this entirely the wrong way.  My understanding is that
>> we should generally be allowed to use anything after the database has
>> been closed, as long as nothing has been destroyed.
>>
>> Below is a minimal reproducible example of what I'm testing so far.  I
>> must admit I'm generally lazy here and usually just test with notmuch
>> that is currently in Debian testing.
>
> Funny that you should mention lazy, that's basically what the problem is
> here ;). notmuch_message_get_message_id is lazily trying to read the
> information from the database. This is a bit surprising here because of
> the query, but that's not really visible once the message object is
> created.
>
> In principle it could be documented what parts of the API can trigger
> access to the database, but I'm not sure about the benefit of the extra
> complexity. It might be safer to assume that only access to already
> fetched information is safe. In particular if you move
>
> messageid = notmuch_message_get_message_id(msg);
>
> before you close the database, then printing it out afterwards works. I
> didn't run it valgrind to make sure, but afaik, that should be perfectly
> legal.

Ok, I forgot the "expected behaviour" part of the bug report ;) I think
that this doesn't work is fine and I'm not surprised by and your
description of fetching it first is very reasonable.  However I was
expecting NOTMUCH_STATUS_XAPIAN_EXEPTION instead of bluntly getting
terminated.  This is what the notmuch_database_close() docs say after
all.

I had a little look and this seems to be caused by the
message->doc.termlist_begin() call in
_notmuch_message_ensure_metadata(), I didn't have xapian debug symbols
and am not familiar with xapian to quickly have an idea of whether this
case can be improved or not.  (-dbg debian packages for notmuch and
xapian would be very handy ;))

But part of my question is, *should* this be improved?  Am I
interpreting notmuch's intended API correctly?

> The original motivation (see 7864350c938944276c1a378539da7670c211b9b5)
> to allow long running processes to release the lock on the
> database. This is not a pattern we use in the CLI, so it's not as well
> tested as it could be. In particular the work to export
> notmuch_database_reopen (tests, documentation) has not happened yet.
>
> d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Usage after database close

2020-06-28 Thread Floris Bruynooghe
Hi,

I started writing some test cases to define better what you can do with
a closed database and make sure that the python bindings do not behave
unexpectedly here too.

One of the first things I tried ends up with xapian calling
exit_group(2) directly, terminating the process.  So I'm wondering if
I'm approaching this entirely the wrong way.  My understanding is that
we should generally be allowed to use anything after the database has
been closed, as long as nothing has been destroyed.

Below is a minimal reproducible example of what I'm testing so far.  I
must admit I'm generally lazy here and usually just test with notmuch
that is currently in Debian testing.

Cheers,
Floris

Here the script:

#!/bin/sh

MAILDIR=$(mktemp --directory)
export MAILDIR
echo $MAILDIR

mkdir $MAILDIR/tmp
mkdir $MAILDIR/new
mkdir $MAILDIR/cur

cat > $MAILDIR/notmuch-config < $MAILDIR/cur/1593342032.M818430P304029Q3.powell <
Date: Sun, 28 Jun 2020 13:00:32 -
From: s...@example.com
To: d...@example.com
Subject: Test mail
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

This is a test mail
EOF

notmuch new

cat >$MAILDIR/test.c <
#include 

int main(int argc, char** argv) {
notmuch_status_t status;
notmuch_database_t* db;
notmuch_message_t* msg;
const char* messageid;
printf("Opening db\n");
status = notmuch_database_open(argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, 
);
if (status != NOTMUCH_STATUS_SUCCESS) {
return status;
}
printf("Finding msg\n");
status = notmuch_database_find_message(db, "0...@powell.devork.be", );
if (status != NOTMUCH_STATUS_SUCCESS) {
return status;
}
printf("Closing db\n");
status = notmuch_database_close(db);
if (status != NOTMUCH_STATUS_SUCCESS) {
return status;
}
printf("Get messageid\n");
messageid = notmuch_message_get_message_id(msg);
if (messageid == NULL) {
return 1;
}
printf("Messageid: %s\n", messageid);
printf("The end.\n");
}
EOF

gcc $MAILDIR/test.c -lnotmuch -o $MAILDIR/test

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


Re: [PATCH] python-cffi: read version from notmuch version file

2020-06-23 Thread Floris Bruynooghe
On Tue 23 Jun 2020 at 13:43 +0300, Frank LENORMAND wrote:

> On Tue Jun 23 12:33:36 2020, David Bremner wrote:
>> Frank LENORMAND  writes:
>> > For example, 0.30.1, with the first two numbers coming from the main
>> > repository, and the last one acting as major for the bindings.
>> >
>> > 0.29.3 → 0.29.1
>> > 0.30-rc2 → 0.30.1-rc2
>> > etc.
>> >
>> 
>> I'm mainly interested in supporting two use cases for notmuch: building
>> everything from source, and binary packages of released versions. We've
>> already gone to some trouble to tell Emacs users that try to mix and
>> match versions that they are on their own, and this seems to apply even
>> more strongly to bindings users.
>>
>> With that said, if Floris thinks some hierarchical version is useful,
>> and is willing to maintain it, I can live with it. I would ask that:
>> 
>> 1) You keep the whole "upstream" version number. So the first example
>> would be 0.29.3.1.  0.29.1 is a previous version of notmuch, and that
>> ambiguity can only cause trouble.
>
> The idea was that the bindings will work with the X.Y version they were
> released for, since the last component in X.Y.Z is for minor changes that
> shouldn't affect the API.

Minor nitpicking, but API is not strong enough here, you'd need to
ensure ABI compatibility.

> So we can keep X.Y from NotMuch itself, and append some information that
> hint at the state of the bindings.
[...]
> Or the exact same version number, but then what should happen to it when
> the bindings are modified, but not NotMuch?

If it was bad enough to need a new release then I guess everyone gets
the same version bump as the entire project gets a bugfix release?

I honestly like the simplicity of just having the same version number
and not having to think about maintaining it separately.  It also means
we mostly don't have to worry about how setuptools/pip is going to view
the version number.

The only way I think this could break is if we want to break backwards
compatibility in the bindings, but we're not supposed to do that
(realistically an impossible task in Python if you ask me, but we can
aim for at least avoiding doing this knowingly).

The most likely version number sin is that the python bindings get a new
feature while libnotmuch only gets bugfix.  I also don't think this is
terrible, but that's perhaps unusual and frowned upon.  Maybe this
warrants a README in the bindings to warn the version number just tracks
libnotmuch and as far as python goes can only be used to order the
releases.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python-cffi: read version from notmuch version file

2020-06-22 Thread Floris Bruynooghe
On Fri 19 Jun 2020 at 15:26 +0300, Frank LENORMAND wrote:

> On Fri Jun 19 12:46:28 2020, Floris Bruynooghe wrote:
>> This keeps it in sync with the main notmuch version which is less
>> confusing to users.
>> ---
>>  bindings/python-cffi/setup.py | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/bindings/python-cffi/setup.py b/bindings/python-cffi/setup.py
>> index 37918e3d..1effcfc6 100644
>> --- a/bindings/python-cffi/setup.py
>> +++ b/bindings/python-cffi/setup.py
>> @@ -1,9 +1,17 @@
>> +import pathlib
>> +
>>  import setuptools
>>  
>>  
>> +THIS_FILE = pathlib.Path(__file__).absolute()
>> +PROJECT_ROOT = THIS_FILE.parent.parent.parent
>> +with open(PROJECT_ROOT.joinpath('version')) as fp:
>> +VERSION = fp.read().strip()
>> +
>> +
>>  setuptools.setup(
>>  name='notmuch2',
>> -version='0.1',
>> +version=VERSION,
>>  description='Pythonic bindings for the notmuch mail database using 
>> CFFI',
>>  author='Floris Bruynooghe',
>>  author_email='f...@devork.be',
>> -- 
>> 2.27.0
>
> It seems that this strategy doesn't work well when the user runs
> `pip install .` in the `bindings/python-cffi` directory.
>
> Apparently all the files are copied to a temporary directory first:
>
> https://travis-ci.com/github/pazz/alot/jobs/351377760#L708-L710
>
> It doesn't happen with the original bindings, probably because the version
> number is stored in `bindings/python/notmuch/version.py`, which is also
> copied when `pip` runs.

Ouch, I only tested pip install -e, which does work.  But indeed a plain
pip install no longer works which is pretty bad.

I guess we could either revert this and do the same sed hackery as the
other bindings, or copy the version file into bindings/python-cffi and
have it loaded in the same way as now.  It would still have to be kept
in sync there sadly.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: python/notmuch2 on PyPI

2020-06-22 Thread Floris Bruynooghe
On Fri 19 Jun 2020 at 09:30 -0300, David Bremner wrote:
> Patrick Totzke  writes:
>> Just to clarify: alot does not, and will not, depend on packages being on 
>> PyPI

Ah, my bad.  I got some github threads mixed up and assumed this had to
do with alot.

> Notmuch as a project does not currently distribute any binary packages,
> whether for linux distros or for PyPi, or fancy new things like flatpaks
> or snaps.

Also fair enough, I think that's a very reasonable stance to take.


I was really hoping to hear from Thore on their motivation and how they
envision this to ideally work.  I'm curious how much work the syncing is
and how to keep it up to date etc.

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


Re: [PATCH] python-cffi: read version from notmuch version file

2020-06-19 Thread Floris Bruynooghe
On Fri 19 Jun 2020 at 07:20 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
> BTW I noticed something (setuptools?) translates "0.30~rc2" to
> "0.30-rc2". I assume that is as intended, and there are some stricter
> rules for python module versions. It should only affect pre-release
> versions in any case.

Supposedly setuptools uses packaging.version:

   $ python3
   Python 3.8.3 (default, May 14 2020, 11:03:12)
   [GCC 9.3.0] on linux
   Type "help", "copyright", "credits" or "license" for more information.
   >>> import packaging.version
   >>> rc1 = packaging.version.parse('0.30~rc1')
   >>> rc1
   
   >>> rc2 = packaging.version.parse('0.30~rc2')
   >>> maj = packaging.version.parse('0.30')
   >>> maj
   
   >>> rc1 < rc2 < maj
   True

So I think this is ok?  Just for completeness:

   >>> rc1b = packaging.version.parse('0.30-rc1')
   >>> rc1b
   
   >>> rc1b < maj
   True

Apparently PEP440 has the full details of how Python wants to see
version numbers work, but it's too many years ago that I read that
thing ;).  This seems good enough to me.

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


python/notmuch2 on PyPI

2020-06-19 Thread Floris Bruynooghe
Hi Thore, notmuch folks,

I noticed that Thore published notmuch2 on PyPI.  I think this is
because alot needs it's users to be able to pull it in as a dependency
using the normal Python mechanisms?

It seems this is currently published from a fork at
https://github.com/weilbith/notmuch2-python-bindings and I wondered if
it was possible to publish this directly from the main notmuch repo or
even integrate this into the normal notmuch release process.  What are
the pros and cons of this?  Is it a bad idea to tie these two publishing
mechanisms too close together?  To difficult to do bugfix releases?  Is
it too hard to make pypi publishing frictionless enough for the main
release process?

An cool stretch goal would be to publish manylinux wheels with the
library included.  But let's not get too hung up on that, small steps
are great.

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


[PATCH] python-cffi: read version from notmuch version file

2020-06-19 Thread Floris Bruynooghe
This keeps it in sync with the main notmuch version which is less
confusing to users.
---
 bindings/python-cffi/setup.py | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/bindings/python-cffi/setup.py b/bindings/python-cffi/setup.py
index 37918e3d..1effcfc6 100644
--- a/bindings/python-cffi/setup.py
+++ b/bindings/python-cffi/setup.py
@@ -1,9 +1,17 @@
+import pathlib
+
 import setuptools
 
 
+THIS_FILE = pathlib.Path(__file__).absolute()
+PROJECT_ROOT = THIS_FILE.parent.parent.parent
+with open(PROJECT_ROOT.joinpath('version')) as fp:
+VERSION = fp.read().strip()
+
+
 setuptools.setup(
 name='notmuch2',
-version='0.1',
+version=VERSION,
 description='Pythonic bindings for the notmuch mail database using CFFI',
 author='Floris Bruynooghe',
 author_email='f...@devork.be',
-- 
2.27.0

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


python-cffi: read version number from notmuch

2020-06-19 Thread Floris Bruynooghe
This reads the version from the toplevel notmuch version file.
The main assumption is obviously that setup.py is always in
bindings/python-cffi/setup.py together with the rest of the
notmuch git repo.


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


Re: [python-cffi] Version number for the `notmuch2` bindings

2020-06-19 Thread Floris Bruynooghe
On Thu 18 Jun 2020 at 16:56 -0300, David Bremner wrote:

> Frank LENORMAND  writes:
>
>> Hi,
>>
>> The original Python bindings follow the entire repository's version
>> number[1]. The new Python bindings use `0.1`[2].
>>
>> The Debian package[3] follows the same version number as well, but
>> it's starting to confuse maintainers of packages for other environments
>> (e.g. Pypi[4]), who use `0.1` because that's what's in the code.
>
> Floris might have some good reason in mind for the divergence. I will
> say it's a pain in Debian to have different binary packages (.deb's)
> built from the same source with different version numbers. So I'd need
> to be convinced.

There is no good reason, it was overlooked when merging the cffi
bindings into notmuch proper.

Is there any reason we can not directly read the toplevel version file
from inside setup.py instead of having to add sed hackery?

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


[PATCH 2/2] python config access: fix style and KeyError bug

2020-06-15 Thread Floris Bruynooghe
This fixes some minor style/pep8 things and adds tests for the new
config support.  Also fixes a bug where KeyError was never raised
on a missing key.
---
 bindings/python-cffi/notmuch2/_config.py  |  9 ++--
 bindings/python-cffi/tests/test_config.py | 56 +++
 2 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 bindings/python-cffi/tests/test_config.py

diff --git a/bindings/python-cffi/notmuch2/_config.py 
b/bindings/python-cffi/notmuch2/_config.py
index 58383c16..29de6495 100644
--- a/bindings/python-cffi/notmuch2/_config.py
+++ b/bindings/python-cffi/notmuch2/_config.py
@@ -4,9 +4,12 @@ import notmuch2._base as base
 import notmuch2._capi as capi
 import notmuch2._errors as errors
 
+
 __all__ = ['ConfigMapping']
 
+
 class ConfigIter(base.NotmuchIter):
+
 def __init__(self, parent, iter_p):
 super().__init__(
 parent, iter_p,
@@ -19,6 +22,7 @@ class ConfigIter(base.NotmuchIter):
 item = super().__next__()
 return base.BinString.from_cffi(item)
 
+
 class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
 """The config key/value pairs stored in the database.
 
@@ -50,11 +54,10 @@ class ConfigMapping(base.NotmuchObject, 
collections.abc.MutableMapping):
 ret = capi.lib.notmuch_database_get_config(self._ptr(), key, val_pp)
 if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
 raise errors.NotmuchError(ret)
-if val_pp[0] == "":
-capi.lib.free(val_pp[0])
-raise KeyError
 val = base.BinString.from_cffi(val_pp[0])
 capi.lib.free(val_pp[0])
+if val == '':
+raise KeyError
 return val
 
 def __setitem__(self, key, val):
diff --git a/bindings/python-cffi/tests/test_config.py 
b/bindings/python-cffi/tests/test_config.py
new file mode 100644
index ..1b2695f5
--- /dev/null
+++ b/bindings/python-cffi/tests/test_config.py
@@ -0,0 +1,56 @@
+import collections.abc
+
+import pytest
+
+import notmuch2._database as dbmod
+
+import notmuch2._config as config
+
+
+class TestIter:
+
+@pytest.fixture
+def db(self, maildir):
+with dbmod.Database.create(maildir.path) as db:
+yield db
+
+def test_type(self, db):
+assert isinstance(db.config, collections.abc.MutableMapping)
+assert isinstance(db.config, config.ConfigMapping)
+
+def test_alive(self, db):
+assert db.config.alive
+
+def test_set_get(self, maildir):
+# Ensure get-set works from different db objects
+with dbmod.Database.create(maildir.path) as db0:
+db0.config['spam'] = 'ham'
+with dbmod.Database(maildir.path) as db1:
+assert db1.config['spam'] == 'ham'
+
+def test_get_keyerror(self, db):
+with pytest.raises(KeyError):
+val = db.config['not-a-key']
+print(repr(val))
+
+def test_iter(self, db):
+assert list(db.config) == []
+db.config['spam'] = 'ham'
+db.config['eggs'] = 'bacon'
+assert set(db.config) == {'spam', 'eggs'}
+assert set(db.config.keys()) == {'spam', 'eggs'}
+assert set(db.config.values()) == {'ham', 'bacon'}
+assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')}
+
+def test_len(self, db):
+assert len(db.config) == 0
+db.config['spam'] = 'ham'
+assert len(db.config) == 1
+db.config['eggs'] = 'bacon'
+assert len(db.config) == 2
+
+def test_del(self, db):
+db.config['spam'] = 'ham'
+assert db.config.get('spam') == 'ham'
+del db.config['spam']
+assert db.config.get('spam') is None
-- 
2.27.0

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


[PATCH 1/2] python/notmuch2: add bindings for the database config strings

2020-06-15 Thread Floris Bruynooghe
From: Anton Khirnov 

---
 bindings/python-cffi/notmuch2/_build.py| 17 +
 bindings/python-cffi/notmuch2/_config.py   | 84 ++
 bindings/python-cffi/notmuch2/_database.py | 23 ++
 3 files changed, 124 insertions(+)
 create mode 100644 bindings/python-cffi/notmuch2/_config.py

diff --git a/bindings/python-cffi/notmuch2/_build.py 
b/bindings/python-cffi/notmuch2/_build.py
index 5e1fcac1..f269f2a1 100644
--- a/bindings/python-cffi/notmuch2/_build.py
+++ b/bindings/python-cffi/notmuch2/_build.py
@@ -314,6 +314,23 @@ ffibuilder.cdef(
 notmuch_indexopts_get_decrypt_policy (const notmuch_indexopts_t 
*indexopts);
 void
 notmuch_indexopts_destroy (notmuch_indexopts_t *options);
+
+notmuch_status_t
+notmuch_database_set_config (notmuch_database_t *db, const char *key, 
const char *value);
+notmuch_status_t
+notmuch_database_get_config (notmuch_database_t *db, const char *key, char 
**value);
+notmuch_status_t
+notmuch_database_get_config_list (notmuch_database_t *db, const char 
*prefix, notmuch_config_list_t **out);
+notmuch_bool_t
+notmuch_config_list_valid (notmuch_config_list_t *config_list);
+const char *
+notmuch_config_list_key (notmuch_config_list_t *config_list);
+const char *
+notmuch_config_list_value (notmuch_config_list_t *config_list);
+void
+notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
+void
+notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 """
 )
 
diff --git a/bindings/python-cffi/notmuch2/_config.py 
b/bindings/python-cffi/notmuch2/_config.py
new file mode 100644
index ..58383c16
--- /dev/null
+++ b/bindings/python-cffi/notmuch2/_config.py
@@ -0,0 +1,84 @@
+import collections.abc
+
+import notmuch2._base as base
+import notmuch2._capi as capi
+import notmuch2._errors as errors
+
+__all__ = ['ConfigMapping']
+
+class ConfigIter(base.NotmuchIter):
+def __init__(self, parent, iter_p):
+super().__init__(
+parent, iter_p,
+fn_destroy=capi.lib.notmuch_config_list_destroy,
+fn_valid=capi.lib.notmuch_config_list_valid,
+fn_get=capi.lib.notmuch_config_list_key,
+fn_next=capi.lib.notmuch_config_list_move_to_next)
+
+def __next__(self):
+item = super().__next__()
+return base.BinString.from_cffi(item)
+
+class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
+"""The config key/value pairs stored in the database.
+
+The entries are exposed as a :class:`collections.abc.MutableMapping` 
object.
+Note that setting a value to an empty string is the same as deleting it.
+
+:param parent: the parent object
+:param ptr_name: the name of the attribute on the parent which will
+   return the memory pointer.  This allows this object to
+   access the pointer via the parent's descriptor and thus
+   trigger :class:`MemoryPointer`'s memory safety.
+"""
+
+def __init__(self, parent, ptr_name):
+self._parent = parent
+self._ptr = lambda: getattr(parent, ptr_name)
+
+@property
+def alive(self):
+return self._parent.alive
+
+def _destroy(self):
+pass
+
+def __getitem__(self, key):
+if isinstance(key, str):
+key = key.encode('utf-8')
+val_pp = capi.ffi.new('char**')
+ret = capi.lib.notmuch_database_get_config(self._ptr(), key, val_pp)
+if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
+raise errors.NotmuchError(ret)
+if val_pp[0] == "":
+capi.lib.free(val_pp[0])
+raise KeyError
+val = base.BinString.from_cffi(val_pp[0])
+capi.lib.free(val_pp[0])
+return val
+
+def __setitem__(self, key, val):
+if isinstance(key, str):
+key = key.encode('utf-8')
+if isinstance(val, str):
+val = val.encode('utf-8')
+ret = capi.lib.notmuch_database_set_config(self._ptr(), key, val)
+if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
+raise errors.NotmuchError(ret)
+
+def __delitem__(self, key):
+self[key] = ""
+
+def __iter__(self):
+"""Return an iterator over the config items.
+
+:raises NullPointerError: If the iterator can not be created.
+"""
+configlist_pp = capi.ffi.new('notmuch_config_list_t**')
+ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', 
configlist_pp)
+if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
+raise errors.NotmuchError(ret)
+return ConfigIter(self._parent, configlist_pp[0])
+
+def __len__(self):
+return sum(1 for t in self)
diff --git a/bindings/python-cffi/notmuch2/_database.py 
b/bindings/python-cffi/notmuch2/_database.py
index 95f59ca0..3c06402d 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -7,6 +7,7 @@ import pathlib
 import weakref

python: config API

2020-06-15 Thread Floris Bruynooghe
This is a followup on the patch from Anton Khirnov adding config
API support to the python bindings.

I can not help myself but point out that I did not spot the bug
until not only I had written tests, but until I looked at the
test coverage to see what was not yet executed and added more
tests.  Tests and test coverage is good!


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


[PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

2020-06-15 Thread Floris Bruynooghe
From: Anton Khirnov 

Any messages retrieved from a query - either directly via
search_messages() or indirectly via thread objects - are owned by that
query. Retrieving the same message (i.e. corresponding to the same
message ID / database object) several times will always yield the same
C object.

The caller is allowed to destroy message objects owned by a query before
the query itself - which can save memory for long-lived queries.
However, that message must then never be retrieved again from that
query.

The python-notmuch2 bindings will currently destroy every message object
in Message._destroy(), which will lead to an invalid free if the same
message is then retrieved again. E.g. the following python program leads
to libtalloc abort()ing:

import notmuch2
db   = notmuch2.Database(mode = notmuch2.Database.MODE.READ_ONLY)
t= next(db.threads('*'))
msgs = list(zip(t.toplevel(), t.toplevel()))
msgs = list(zip(t.toplevel(), t.toplevel()))

Fix this issue by creating a subclass of Message, which is used for
"standalone" message which have to be freed by the caller. Message class
is then used only for messages descended from a query, which do not need
to be freed by the caller.
---
 bindings/python-cffi/notmuch2/_database.py |  6 ++--
 bindings/python-cffi/notmuch2/_message.py  | 42 ++
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py 
b/bindings/python-cffi/notmuch2/_database.py
index 95f59ca0..f14eac78 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -399,7 +399,7 @@ class Database(base.NotmuchObject):
   capi.lib.NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID]
 if ret not in ok:
 raise errors.NotmuchError(ret)
-msg = message.Message(self, msg_pp[0], db=self)
+msg = message.StandaloneMessage(self, msg_pp[0], db=self)
 if sync_flags:
 msg.tags.from_maildir_flags()
 return self.AddedMessage(
@@ -468,7 +468,7 @@ class Database(base.NotmuchObject):
 msg_p = msg_pp[0]
 if msg_p == capi.ffi.NULL:
 raise LookupError
-msg = message.Message(self, msg_p, db=self)
+msg = message.StandaloneMessage(self, msg_p, db=self)
 return msg
 
 def get(self, filename):
@@ -501,7 +501,7 @@ class Database(base.NotmuchObject):
 msg_p = msg_pp[0]
 if msg_p == capi.ffi.NULL:
 raise LookupError
-msg = message.Message(self, msg_p, db=self)
+msg = message.StandaloneMessage(self, msg_p, db=self)
 return msg
 
 @property
diff --git a/bindings/python-cffi/notmuch2/_message.py 
b/bindings/python-cffi/notmuch2/_message.py
index c5fdbf6d..416ce7ca 100644
--- a/bindings/python-cffi/notmuch2/_message.py
+++ b/bindings/python-cffi/notmuch2/_message.py
@@ -14,7 +14,7 @@ __all__ = ['Message']
 
 
 class Message(base.NotmuchObject):
-"""An email message stored in the notmuch database.
+"""An email message stored in the notmuch database retrieved via a query.
 
 This should not be directly created, instead it will be returned
 by calling methods on :class:`Database`.  A message keeps a
@@ -61,22 +61,10 @@ class Message(base.NotmuchObject):
 
 @property
 def alive(self):
-if not self._parent.alive:
-return False
-try:
-self._msg_p
-except errors.ObjectDestroyedError:
-return False
-else:
-return True
-
-def __del__(self):
-self._destroy()
+return self._parent.alive
 
 def _destroy(self):
-if self.alive:
-capi.lib.notmuch_message_destroy(self._msg_p)
-self._msg_p = None
+pass
 
 @property
 def messageid(self):
@@ -375,6 +363,30 @@ class Message(base.NotmuchObject):
 if isinstance(other, self.__class__):
 return self.messageid == other.messageid
 
+class StandaloneMessage(Message):
+"""An email message stored in the notmuch database.
+
+This subclass of Message is used for messages that are retrieved from the
+database directly and are not owned by a query.
+"""
+@property
+def alive(self):
+if not self._parent.alive:
+return False
+try:
+self._msg_p
+except errors.ObjectDestroyedError:
+return False
+else:
+return True
+
+def __del__(self):
+self._destroy()
+
+def _destroy(self):
+if self.alive:
+capi.lib.notmuch_message_destroy(self._msg_p)
+self._msg_p = None
 
 class FilenamesIter(base.NotmuchIter):
 """Iterator for binary filenames objects."""
-- 
2.27.0

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


[PATCH 2/2] Make messages returned by Thread objects owned

2020-06-15 Thread Floris Bruynooghe
This reverses the logic of StandaloneMessage to instead create a
OwnedMessage.  Only the Thread class allows retrieving messages more
then once so it can explicitly create such messages.

The added test fails with SIGABRT without the fix for the message
re-use in threads being present.
---
 bindings/python-cffi/notmuch2/_database.py  |  6 +--
 bindings/python-cffi/notmuch2/_message.py   | 55 -
 bindings/python-cffi/notmuch2/_thread.py|  8 ++-
 bindings/python-cffi/tests/test_database.py | 11 +
 4 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py 
b/bindings/python-cffi/notmuch2/_database.py
index f14eac78..95f59ca0 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -399,7 +399,7 @@ class Database(base.NotmuchObject):
   capi.lib.NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID]
 if ret not in ok:
 raise errors.NotmuchError(ret)
-msg = message.StandaloneMessage(self, msg_pp[0], db=self)
+msg = message.Message(self, msg_pp[0], db=self)
 if sync_flags:
 msg.tags.from_maildir_flags()
 return self.AddedMessage(
@@ -468,7 +468,7 @@ class Database(base.NotmuchObject):
 msg_p = msg_pp[0]
 if msg_p == capi.ffi.NULL:
 raise LookupError
-msg = message.StandaloneMessage(self, msg_p, db=self)
+msg = message.Message(self, msg_p, db=self)
 return msg
 
 def get(self, filename):
@@ -501,7 +501,7 @@ class Database(base.NotmuchObject):
 msg_p = msg_pp[0]
 if msg_p == capi.ffi.NULL:
 raise LookupError
-msg = message.StandaloneMessage(self, msg_p, db=self)
+msg = message.Message(self, msg_p, db=self)
 return msg
 
 @property
diff --git a/bindings/python-cffi/notmuch2/_message.py 
b/bindings/python-cffi/notmuch2/_message.py
index 416ce7ca..02de50ad 100644
--- a/bindings/python-cffi/notmuch2/_message.py
+++ b/bindings/python-cffi/notmuch2/_message.py
@@ -47,9 +47,7 @@ class Message(base.NotmuchObject):
 :type db: Database
 :param msg_p: The C pointer to the ``notmuch_message_t``.
 :type msg_p: 
-
 :param dup: Whether the message was a duplicate on insertion.
-
 :type dup: None or bool
 """
 _msg_p = base.MemoryPointer()
@@ -61,10 +59,22 @@ class Message(base.NotmuchObject):
 
 @property
 def alive(self):
-return self._parent.alive
+if not self._parent.alive:
+return False
+try:
+self._msg_p
+except errors.ObjectDestroyedError:
+return False
+else:
+return True
+
+def __del__(self):
+self._destroy()
 
 def _destroy(self):
-pass
+if self.alive:
+capi.lib.notmuch_message_destroy(self._msg_p)
+self._msg_p = None
 
 @property
 def messageid(self):
@@ -363,30 +373,26 @@ class Message(base.NotmuchObject):
 if isinstance(other, self.__class__):
 return self.messageid == other.messageid
 
-class StandaloneMessage(Message):
-"""An email message stored in the notmuch database.
 
-This subclass of Message is used for messages that are retrieved from the
-database directly and are not owned by a query.
+class OwnedMessage(Message):
+"""An email message owned by parent thread object.
+
+This subclass of Message is used for messages that are retrieved
+from the notmuch database via a parent :class:`notmuch2.Thread`
+object, which "owns" this message.  This means that when this
+message object is destroyed, by calling :func:`del` or
+:meth:`_destroy` directly or indirectly, the message is not freed
+in the notmuch API and the parent :class:`notmuch2.Thread` object
+can return the same object again when needed.
 """
+
 @property
 def alive(self):
-if not self._parent.alive:
-return False
-try:
-self._msg_p
-except errors.ObjectDestroyedError:
-return False
-else:
-return True
-
-def __del__(self):
-self._destroy()
+return self._parent.alive
 
 def _destroy(self):
-if self.alive:
-capi.lib.notmuch_message_destroy(self._msg_p)
-self._msg_p = None
+pass
+
 
 class FilenamesIter(base.NotmuchIter):
 """Iterator for binary filenames objects."""
@@ -690,8 +696,9 @@ collections.abc.ValuesView.register(PropertiesValuesView)
 
 class MessageIter(base.NotmuchIter):
 
-def __init__(self, parent, msgs_p, *, db):
+def __init__(self, parent, msgs_p, *, db, msg_cls=Message):
 self._db = db
+self._msg_cls = msg_cls
 super().__init__(parent, msgs_p,
  fn_destroy=capi.lib.notmuch_messages_destroy,
  fn_valid=capi.lib.notmuch_messages_valid,
@@ -700,4 +707,4 @@ class 

python: Continuing message re-use fix

2020-06-15 Thread Floris Bruynooghe
Hi,

This builds on the patch by Anton Khirnov to fix the message re-use
that is possible when accessing messages from a thread.  I started
with just addressing my own comments on this patch, but evolved it
into switching the logic around and leave the normal Message object
untouched.  Instead I created a new OwnedMessage which is used by
the Thread which does not free itself on __del__().  I think this
is preferable because the other iterators, mainly Database.messages(),
do not allow retrieving messages more than once since the query object
is hidden from the API.

I've left the original commit in this patch series to not alter any
contributions.

Cheers,
Floris


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


Re: [PATCH] Update tox.ini for python3.8 and fix pypy3.6

2020-06-15 Thread Floris Bruynooghe
On Mon 15 Jun 2020 at 07:06 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>>  [testenv]
>>  deps =
>> @@ -14,3 +14,6 @@ commands = pytest --cov={envsitepackagesdir}/notmuch2 
>> {posargs}
>>  
>>  [testenv:pypy35]
>>  basepython = pypy3.5
>> +
>> +[testenv:pypy36]
>> +basepython = pypy3.6
>
> I'm not an expert, but should python 3.7 and python 3.8 have similar
> clauses? Or do they work by default?

They do work by default.  But to be honest these two pypy ones are not
fully standardised and rely on me manually ensuring I have both a
pypy3.5 and pypy3.6 binary on my path.  The default thing that works is
just a "pypy3" which doesn't give you any guarantees over the version
you get (other than not pypy2).
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Support aborting the atomic context

2020-06-15 Thread Floris Bruynooghe
On Mon 15 Jun 2020 at 07:35 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>> This is an implementation of what was suggested in
>> id:87v9k96xtl@powell.devork.be It closes the database as that is
>> the only safe way to do this afaik.
>>
>> Currently when the database is closed there are still a bunch of
>> operations which can result in segfaults.

I realise that this is perhaps not a very helpful comment.  I'll see if
I can narrow this down into a proper bug report.

>> Yet the API also promises
>> that some operations are still valid, basically those which only
>> access already previously retrieved information.  It would be nice to
>> find a good solution for this in the python bindings, but I don't yet
>> know what this would be.
>
> There is a Xapian method WritableDatabase::cancel_transaction(), but it
> is not currently exposed by notmuch.  I think it could be added, but
> getting the testing working right is not something I want to tackle at
> this point in the release cycle.  I guess this should be upwardly
> compatible, as all code correct for your current implementation should
> still work if we replace notmuch_database_close with
> notmuch_cancel_atomic. Thoughts?

I agree with your reasoning here that such a change would be upwardly
compatible.  Though I can think of some really convoluted scenarios
where this could be false, e.g. after aborting a transaction code could
rely on handling NOTMUCH_STATUS_XAPIAN_EXCEPTION it gets from a the
closed database.  This seems so much a corner case though, that I'd be
willing to ignore it.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: difficulties with notmuch2 python bindings for alot

2020-06-15 Thread Floris Bruynooghe
On Sun 14 Jun 2020 at 19:44 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>> One thing that they encountered and don't yet understand is that they
>> reported issues with leaking filedescriptors.  They used the bindings in
>> a way where I expect it to only call notmuch_database_destroy() when
>> they are done with it.  From reading notmuch.h I think that's correct
>> and there's no need to call notmuch_database_close() first.  Yet someone
>> reported that explicitly calling close helped.  Is the assumption I made
>> of only calling destroy correct?
>
> The first thing destroy does is call close. My read of the
> notmuch_database_close code is that it is idempotent (calling multiple
> times does not change anything).

Thanks for confirming, so that should be fine.

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


Re: difficulties with notmuch2 python bindings for alot

2020-06-14 Thread Floris Bruynooghe
Hi Daniel,

On Tue 09 Jun 2020 at 09:19 -0400, Daniel Kahn Gillmor wrote:
> I see over on github that alot is trying to port to the notmuch2
> bindings, and having a few problems with it:
>
>  https://github.com/pazz/alot/pull/1511
>
> alot is an important consumer of the notmuch python bindings, and it
> would be really great to see them successfully transition to the
> notmuch2 module.
>
> Floris, if you (or anyone else with this particular knowledge) has a
> chance to take a look and help them sort out the remaining issues, that
> would be much appreciated!

Thanks for the pointer, I've pinged the issue offering help with the
bindings and had a look through the existing things they discussed.

One thing that they encountered and don't yet understand is that they
reported issues with leaking filedescriptors.  They used the bindings in
a way where I expect it to only call notmuch_database_destroy() when
they are done with it.  From reading notmuch.h I think that's correct
and there's no need to call notmuch_database_close() first.  Yet someone
reported that explicitly calling close helped.  Is the assumption I made
of only calling destroy correct?

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


python: Update tox.ini for python 3.8

2020-06-14 Thread Floris Bruynooghe
This was released a while ago, we should support it.


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


[PATCH] Update tox.ini for python3.8 and fix pypy3.6

2020-06-14 Thread Floris Bruynooghe
Python 3.8 has been released for a while now, make sure we keep
supporting it correctly.

PyPy 3.6 wasn not configured correctly.
---
 bindings/python-cffi/tox.ini | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/bindings/python-cffi/tox.ini b/bindings/python-cffi/tox.ini
index 34148a11..7cf93be0 100644
--- a/bindings/python-cffi/tox.ini
+++ b/bindings/python-cffi/tox.ini
@@ -3,7 +3,7 @@ minversion = 3.0
 addopts = -ra --cov=notmuch2 --cov=tests
 
 [tox]
-envlist = py35,py36,py37,pypy35,pypy36
+envlist = py35,py36,py37,py38,pypy35,pypy36
 
 [testenv]
 deps =
@@ -14,3 +14,6 @@ commands = pytest --cov={envsitepackagesdir}/notmuch2 
{posargs}
 
 [testenv:pypy35]
 basepython = pypy3.5
+
+[testenv:pypy36]
+basepython = pypy3.6
-- 
2.27.0

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


[PATCH] Add missing set methods to tagsets

2020-06-14 Thread Floris Bruynooghe
Even though we use collections.abc.Set which implements all these
methods under their operator names, the actual named variations of
these methods are shockingly missing.  So let's add them manually.
---
 bindings/python-cffi/notmuch2/_tags.py  | 21 +
 bindings/python-cffi/tests/test_tags.py | 62 +
 2 files changed, 83 insertions(+)

diff --git a/bindings/python-cffi/notmuch2/_tags.py 
b/bindings/python-cffi/notmuch2/_tags.py
index 212852a8..3b14c981 100644
--- a/bindings/python-cffi/notmuch2/_tags.py
+++ b/bindings/python-cffi/notmuch2/_tags.py
@@ -110,6 +110,27 @@ class ImmutableTagSet(base.NotmuchObject, 
collections.abc.Set):
 def __eq__(self, other):
 return tuple(sorted(self.iter())) == tuple(sorted(other.iter()))
 
+def issubset(self, other):
+return self <= other
+
+def issuperset(self, other):
+return self >= other
+
+def union(self, other):
+return self | other
+
+def intersection(self, other):
+return self & other
+
+def difference(self, other):
+return self - other
+
+def symmetric_difference(self, other):
+return self ^ other
+
+def copy(self):
+return set(self)
+
 def __hash__(self):
 return hash(tuple(self.iter()))
 
diff --git a/bindings/python-cffi/tests/test_tags.py 
b/bindings/python-cffi/tests/test_tags.py
index f12fa1e6..faf3947b 100644
--- a/bindings/python-cffi/tests/test_tags.py
+++ b/bindings/python-cffi/tests/test_tags.py
@@ -50,6 +50,22 @@ class TestImmutable:
 assert 'unread' in tagset
 assert 'foo' not in tagset
 
+def test_isdisjoint(self, tagset):
+assert tagset.isdisjoint(set(['spam', 'ham']))
+assert not tagset.isdisjoint(set(['inbox']))
+
+def test_issubset(self, tagset):
+assert {'inbox'} <= tagset
+assert {'inbox'}.issubset(tagset)
+assert tagset <= {'inbox', 'unread', 'spam'}
+assert tagset.issubset({'inbox', 'unread', 'spam'})
+
+def test_issuperset(self, tagset):
+assert {'inbox', 'unread', 'spam'} >= tagset
+assert {'inbox', 'unread', 'spam'}.issuperset(tagset)
+assert tagset >= {'inbox'}
+assert tagset.issuperset({'inbox'})
+
 def test_iter(self, tagset):
 expected = sorted(['unread', 'inbox'])
 found = []
@@ -78,18 +94,30 @@ class TestImmutable:
 assert isinstance(common, set)
 assert isinstance(common, collections.abc.Set)
 assert common == {'unread'}
+common = tagset.intersection({'unread'})
+assert isinstance(common, set)
+assert isinstance(common, collections.abc.Set)
+assert common == {'unread'}
 
 def test_or(self, tagset):
 res = tagset | {'foo'}
 assert isinstance(res, set)
 assert isinstance(res, collections.abc.Set)
 assert res == {'unread', 'inbox', 'foo'}
+res = tagset.union({'foo'})
+assert isinstance(res, set)
+assert isinstance(res, collections.abc.Set)
+assert res == {'unread', 'inbox', 'foo'}
 
 def test_sub(self, tagset):
 res = tagset - {'unread'}
 assert isinstance(res, set)
 assert isinstance(res, collections.abc.Set)
 assert res == {'inbox'}
+res = tagset.difference({'unread'})
+assert isinstance(res, set)
+assert isinstance(res, collections.abc.Set)
+assert res == {'inbox'}
 
 def test_rsub(self, tagset):
 res = {'foo', 'unread'} - tagset
@@ -102,6 +130,10 @@ class TestImmutable:
 assert isinstance(res, set)
 assert isinstance(res, collections.abc.Set)
 assert res == {'inbox', 'foo'}
+res = tagset.symmetric_difference({'unread', 'foo'})
+assert isinstance(res, set)
+assert isinstance(res, collections.abc.Set)
+assert res == {'inbox', 'foo'}
 
 def test_rxor(self, tagset):
 res = {'unread', 'foo'} ^ tagset
@@ -109,6 +141,12 @@ class TestImmutable:
 assert isinstance(res, collections.abc.Set)
 assert res == {'inbox', 'foo'}
 
+def test_copy(self, tagset):
+res = tagset.copy()
+assert isinstance(res, set)
+assert isinstance(res, collections.abc.Set)
+assert res == {'inbox', 'unread'}
+
 
 class TestMutableTagset:
 
@@ -175,3 +213,27 @@ class TestMutableTagset:
 msg.tags.to_maildir_flags()
 flags = msg.path.name.split(',')[-1]
 assert 'F' not in flags
+
+def test_isdisjoint(self, tagset):
+assert tagset.isdisjoint(set(['spam', 'ham']))
+assert not tagset.isdisjoint(set(['inbox']))
+
+def test_issubset(self, tagset):
+assert {'inbox'} <= tagset
+assert {'inbox'}.issubset(tagset)
+assert not {'spam'} <= tagset
+assert not {'spam'}.issubset(tagset)
+assert tagset <= {'inbox', 'unread', 'spam'}
+assert tagset.issubset({'inbox', 'unread', 'spam'})
+assert 

python-cffi: add missing tagset methods

2020-06-14 Thread Floris Bruynooghe
This issue was found by alot's porting efforts.  It seems these
were simply missing.


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


[PATCH] Support aborting the atomic context

2020-06-14 Thread Floris Bruynooghe
Since it is possible to use an atomic context to abort a number of
changes support this usage.  Because the only way to actually abort
the transaction is to close the database this must also do so.
---
 bindings/python-cffi/notmuch2/_database.py  | 16 +++-
 bindings/python-cffi/tests/test_database.py |  5 +
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py 
b/bindings/python-cffi/notmuch2/_database.py
index 95f59ca0..c851f0a5 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -641,6 +641,7 @@ class AtomicContext:
 def __init__(self, db, ptr_name):
 self._db = db
 self._ptr = lambda: getattr(db, ptr_name)
+self._exit_fn = lambda: None
 
 def __del__(self):
 self._destroy()
@@ -656,13 +657,17 @@ class AtomicContext:
 ret = capi.lib.notmuch_database_begin_atomic(self._ptr())
 if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
 raise errors.NotmuchError(ret)
+self._exit_fn = self._end_atomic
 return self
 
-def __exit__(self, exc_type, exc_value, traceback):
+def _end_atomic(self):
 ret = capi.lib.notmuch_database_end_atomic(self._ptr())
 if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
 raise errors.NotmuchError(ret)
 
+def __exit__(self, exc_type, exc_value, traceback):
+self._exit_fn()
+
 def force_end(self):
 """Force ending the atomic section.
 
@@ -681,6 +686,15 @@ class AtomicContext:
 if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
 raise errors.NotmuchError(ret)
 
+def abort(self):
+"""Abort the transaction.
+
+Aborting a transaction will not commit any of the changes, but
+will also implicitly close the database.
+"""
+self._exit_fn = lambda: None
+self._db.close()
+
 
 @functools.total_ordering
 class DbRevision:
diff --git a/bindings/python-cffi/tests/test_database.py 
b/bindings/python-cffi/tests/test_database.py
index e3a8344d..aa2cbdc7 100644
--- a/bindings/python-cffi/tests/test_database.py
+++ b/bindings/python-cffi/tests/test_database.py
@@ -127,6 +127,11 @@ class TestAtomic:
 with pytest.raises(errors.UnbalancedAtomicError):
 ctx.force_end()
 
+def test_abort(self, db):
+with db.atomic() as txn:
+txn.abort()
+assert db.closed
+
 
 class TestRevision:
 
-- 
2.27.0

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


[PATCH] Support aborting the atomic context

2020-06-14 Thread Floris Bruynooghe
This is an implementation of what was suggested in
id:87v9k96xtl@powell.devork.be It closes the database as that is
the only safe way to do this afaik.

Currently when the database is closed there are still a bunch of
operations which can result in segfaults.  Yet the API also promises
that some operations are still valid, basically those which only
access already previously retrieved information.  It would be nice to
find a good solution for this in the python bindings, but I don't yet
know what this would be.


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


Re: Python 3.x bindings support which versions, exactly?

2020-06-05 Thread Floris Bruynooghe
On Tue 26 May 2020 at 00:41 +0300, Tomi Ollila wrote:

> On Mon, May 25 2020, Floris Bruynooghe wrote:
>
>> In tox.ini the earliest version is 3.5 and if memory serves me right
>> there's a reasonably good reason for that.  I think the notmuch2
>> bindings use some features not yet present in 3.4.  But anything >= 3.5
>> should be supported and if not a bug.  I guess tox.ini should be updated
>> with 3.8 sometime :)
>
> I've been long thinking whether our current python3 check is "strict"
> enough, as it, in configure, is:
>
> if "$python" -c 'import sys; assert sys.version_info >= (3,0)' > /dev/null 
> 2>&1
>
> Perhaps that should be updated to (3,5) ?

If this check is only used to decide to build the python-cffi/notmuch2
bindings, then yes.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: status of the new python bindings

2020-06-02 Thread Floris Bruynooghe
Hi Anton,

Great that you're looking at this API!  My apologies for the late
response, this slipped by me probably as I was bulk marking things as
read when I came back from a few weeks away.

On Thu 07 May 2020 at 15:57 +0200, Anton Khirnov wrote:
> 1) What is the logic behind choosing whether something is exported as
> a property or as a method? E.g. Database.needs_upgrade is a property,
> while Database.revision() is a method. In my own python code, I tend to
> use @property for things that are "cheap" - i.e. do not involve
> (significant) IO or heavy computation and methods for those that do. But
> both of the above attributes involve library calls, presumably(?) of
> similar complexity. Would be nice if this was consistent.

Choosing when something should be a property is sometimes indeed tricky
and in general I agree you're right to expect property calls to be not
too expensive.  But I wouldn't go so far to consider every call into
libnotmuch as expensive.  I think there is also an element of how static
something is.  Usually I expect properties to behave more like
attributes, that is after all the point of them, and thus I don't expect
them to change often without explicitly manipulating them.

So that could justifies why I choose Database.version as a property
while Database.revision() is a method.  The revision changes all the
time as a side-effect of other things.  I think it also justifies
generally why tags are exposed as properties.

Database.needs_upgrade is indeed a more questionable choice, especially
given its naming which is more of a question being asked and thus
probably more suitable as a method.  It does change rarely, but does so
by interaction with another method - Database.upgrade().  So I would
agree that this perhaps wasn't the best choice and maybe that was better
as a method.

> 2) Atomic transactions are now exported as a context manager, which is
> nice and convenient for the usual use cases, but AFAIU does not have the
> same power. E.g. my tagging script does the tagging as a single atomic
> transaction and has a "dry-run" mode in which it omits the end_atomic()
> call, which is documented to throw away all the changes. This seems to
> not be possible with the new bindings.
> Would it be okay to add bindings for explicitly calling
> start/end_atomic()? Or is my approach considered invalid?

This is indeed a good usecase that I overlooked, I stopped reading at
the part where is says being and end atomic must always be used in
pairs...  But yes, we should add support for this too.  What would you
think of a .abort() on the context manager instead of directly exposing
the start/end?  E.g.:

db = notmuch2.Database()
with db.atomic() as txn:
# do stuff
txn.abort()

However if I read the docs correctly the transaction only actually is
aborted once the database is closed?  But when I try this:

with db.atomic():
# do stuff
db.close()

I discover that after calling .close() it's very easy to segfault by
making other calls, e.g. the above currently segfaults.  I thought this
wasn't so bad but perhaps .close() should immediately destroy the
database too.  This would again disallow some funky behaviour that the
raw C API allows I guess where some objects might still be valid to use.
But this is really hard to get right I think and I'd prefer to vote on
the side of safety in the Python bindings.

Still, it could be reasonable to make AtomicContext.abort() also close
the database and then make sure it doesn't call
notmuch_database_end_atomic() on exit.  I tried this locally and it
seems reasonable.  What do you think?


Lastly you can do this today by calling the C API directly:

db = notmuch2.Database()
notmuch2._capi.notmuch_database_being_atomic(db._db_p)

Yes, that uses a lot of internal stuff but it's also pretty advanced
usage.  I'm only suggesting you this to maybe unblock you in the short
term.


> 3) There seem to be no bindings for notmuch_database_set_config().

There are still bits of the API missing indeed.  Would be great to add
them but would be even better to have someone who needs it to validate
the API.  There were recently some good patches posted to expose the
config stuff, would be good to see that merged indeed.

> 4) The setup for building the documentation seems to be missing.

Hmm, yes.  The docstrings are written to work with sphinx autodoc but a
doc directory with sphinx configured never got added.  Perhaps we should
do this so that people can at least build docs locally.

> Anything else of note that remains to be implemented?

Probably.


Thanks for caring!

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


Re: Python 3.x bindings support which versions, exactly?

2020-05-25 Thread Floris Bruynooghe
On Sun 24 May 2020 at 16:29 -0300, David Bremner wrote:

> Ralph Seichter  writes:
>
>> Hello,
>>
>> having examined the source code, I assume that Notmuch's Python bindings
>> should support Python 3.x for any given 'x'. However, reading the Travis
>> logs at https://travis-ci.org/notmuch/notmuch and .travis.yml, the CI
>> tests apparently run on Ubuntu 18.04 and use only Python 3.6.
>>
>> While I hope there won't be any problems with Python 3.7, 3.8 and 3.9,
>> I wanted to ask here first if anybody has tested this yet?
>
> the old (modulename = notmuch) bindings actually do have memory
> management issues with python > 3.6 (maybe also with 3.6). The new
> bindings (modulename= notmuch2) should work with any python recent
> 3.x.

In tox.ini the earliest version is 3.5 and if memory serves me right
there's a reasonably good reason for that.  I think the notmuch2
bindings use some features not yet present in 3.4.  But anything >= 3.5
should be supported and if not a bug.  I guess tox.ini should be updated
with 3.8 sometime :)

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


Re: python-cffi and ruby test suites fail in out-of-tree builds

2020-05-23 Thread Floris Bruynooghe
Hi,

On Thu 21 May 2020 at 21:29 -0400, Daniel Kahn Gillmor wrote:

> Hey folks--
>
> I just did a bit of testing and cleanup for out-of-tree builds (see the
> minor patches that should have landed on the list in the last hour or
> two).

It probably is indeed the unfortunate case that copying the python
source is currently the easiest.  I had a quick look and it seemed like
one'd have to dig into the cffi setuptools support to make this work and
I'm not sure how successful that would be, but I admit I didn't feel
like trying.

> For me, "make check" in an out-of-tree build works fine now, with the
> exception of T391-python-cffi.sh and T395-ruby.sh.

I checked out the python one, and maybe that's not too hard.  The
following patch made this work for both in tree and out of tree (on top
of your other patch):

modified   test/T391-python-cffi.sh
@@ -8,7 +8,7 @@ fi
 
 
 test_begin_subtest "python cffi tests"
-pytest_dir=$NOTMUCH_SRCDIR/bindings/python-cffi/build/stage
+pytest_dir=$NOTMUCH_BUILDDIR/bindings/python-cffi/build/stage
 printf "[pytest]\nminversion = 3.0\naddopts = -ra\n" > $pytest_dir/pytest.ini
 test_expect_success "(cd $pytest_dir && ${NOTMUCH_PYTHON} -m pytest 
--log-file=$TMP_DIRECTORY/test.output)"
 test_done



Hope that's helpful.

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


Re: [PATCH 2/2] python/notmuch2: add bindings for the database config strings

2020-05-21 Thread Floris Bruynooghe
Thanks for adding more of the API!

This mostly is fine as well, again I'd mainly ask to add tests however.
At least the things which are implemented directly I guess: setitem,
getitem, iter and len.


On Sat 09 May 2020 at 07:05 +0200, Anton Khirnov wrote:

> ---
>  bindings/python-cffi/notmuch2/_build.py| 17 +
>  bindings/python-cffi/notmuch2/_config.py   | 84 ++
>  bindings/python-cffi/notmuch2/_database.py | 23 ++
>  3 files changed, 124 insertions(+)
>  create mode 100644 bindings/python-cffi/notmuch2/_config.py
>
> diff --git a/bindings/python-cffi/notmuch2/_build.py 
> b/bindings/python-cffi/notmuch2/_build.py
> index 5e1fcac1..f269f2a1 100644
> --- a/bindings/python-cffi/notmuch2/_build.py
> +++ b/bindings/python-cffi/notmuch2/_build.py
> @@ -314,6 +314,23 @@ ffibuilder.cdef(
>  notmuch_indexopts_get_decrypt_policy (const notmuch_indexopts_t 
> *indexopts);
>  void
>  notmuch_indexopts_destroy (notmuch_indexopts_t *options);
> +
> +notmuch_status_t
> +notmuch_database_set_config (notmuch_database_t *db, const char *key, 
> const char *value);
> +notmuch_status_t
> +notmuch_database_get_config (notmuch_database_t *db, const char *key, 
> char **value);
> +notmuch_status_t
> +notmuch_database_get_config_list (notmuch_database_t *db, const char 
> *prefix, notmuch_config_list_t **out);
> +notmuch_bool_t
> +notmuch_config_list_valid (notmuch_config_list_t *config_list);
> +const char *
> +notmuch_config_list_key (notmuch_config_list_t *config_list);
> +const char *
> +notmuch_config_list_value (notmuch_config_list_t *config_list);
> +void
> +notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
> +void
> +notmuch_config_list_destroy (notmuch_config_list_t *config_list);
>  """
>  )
>  
> diff --git a/bindings/python-cffi/notmuch2/_config.py 
> b/bindings/python-cffi/notmuch2/_config.py
> new file mode 100644
> index ..58383c16
> --- /dev/null
> +++ b/bindings/python-cffi/notmuch2/_config.py
> @@ -0,0 +1,84 @@
> +import collections.abc
> +
> +import notmuch2._base as base
> +import notmuch2._capi as capi
> +import notmuch2._errors as errors
> +
> +__all__ = ['ConfigMapping']
> +

Same style thing about 2 blank lines

> +class ConfigIter(base.NotmuchIter):
> +def __init__(self, parent, iter_p):
> +super().__init__(
> +parent, iter_p,
> +fn_destroy=capi.lib.notmuch_config_list_destroy,
> +fn_valid=capi.lib.notmuch_config_list_valid,
> +fn_get=capi.lib.notmuch_config_list_key,
> +fn_next=capi.lib.notmuch_config_list_move_to_next)
> +
> +def __next__(self):
> +item = super().__next__()
> +return base.BinString.from_cffi(item)
> +
> +class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
> +"""The config key/value pairs stored in the database.
> +
> +The entries are exposed as a :class:`collections.abc.MutableMapping` 
> object.
> +Note that setting a value to an empty string is the same as deleting it.
> +
> +:param parent: the parent object
> +:param ptr_name: the name of the attribute on the parent which will
> +   return the memory pointer.  This allows this object to
> +   access the pointer via the parent's descriptor and thus
> +   trigger :class:`MemoryPointer`'s memory safety.
> +"""
> +
> +def __init__(self, parent, ptr_name):
> +self._parent = parent
> +self._ptr = lambda: getattr(parent, ptr_name)
> +
> +@property
> +def alive(self):
> +return self._parent.alive
> +
> +def _destroy(self):
> +pass
> +
> +def __getitem__(self, key):
> +if isinstance(key, str):
> +key = key.encode('utf-8')
> +val_pp = capi.ffi.new('char**')
> +ret = capi.lib.notmuch_database_get_config(self._ptr(), key, val_pp)
> +if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +raise errors.NotmuchError(ret)
> +if val_pp[0] == "":
> +capi.lib.free(val_pp[0])
> +raise KeyError
> +val = base.BinString.from_cffi(val_pp[0])
> +capi.lib.free(val_pp[0])
> +return val
> +
> +def __setitem__(self, key, val):
> +if isinstance(key, str):
> +key = key.encode('utf-8')
> +if isinstance(val, str):
> +val = val.encode('utf-8')
> +ret = capi.lib.notmuch_database_set_config(self._ptr(), key, val)
> +if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +raise errors.NotmuchError(ret)
> +
> +def __delitem__(self, key):
> +self[key] = ""
> +
> +def __iter__(self):
> +"""Return an iterator over the config items.
> +
> +:raises NullPointerError: If the iterator can not be created.
> +"""
> +configlist_pp = capi.ffi.new('notmuch_config_list_t**')
> +ret = 

Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query

2020-05-21 Thread Floris Bruynooghe
Hi Anton,

Thanks for improving the bindings!  Any my apologies for the late
response, I failed to spot this mail the first time round.

Also, this is a pretty serious bug, thanks for finding it.

This looks pretty solid, a few small style comments that aren't very
important notwithstanding.  Though I do think you should be able to add
a test for this in the pytest tests.  I think just triggering the
use-after-free you demonstrate in the commit message is fine as it will
work with your patch.  The fact it will crash rather then fail without
is unfortunate but probably fine.

On Sat 09 May 2020 at 07:05 +0200, Anton Khirnov wrote:

> Any messages retrieved from a query - either directly via
> search_messages() or indirectly via thread objects - are owned by that
> query. Retrieving the same message (i.e. corresponding to the same
> message ID / database object) several times will always yield the same
> C object.
>
> The caller is allowed to destroy message objects owned by a query before
> the query itself - which can save memory for long-lived queries.
> However, that message must then never be retrieved again from that
> query.
>
> The python-notmuch2 bindings will currently destroy every message object
> in Message._destroy(), which will lead to an invalid free if the same
> message is then retrieved again. E.g. the following python program leads
> to libtalloc abort()ing:
>
> import notmuch2
> db   = notmuch2.Database(mode = notmuch2.Database.MODE.READ_ONLY)
> t= next(db.threads('*'))
> msgs = list(zip(t.toplevel(), t.toplevel()))
> msgs = list(zip(t.toplevel(), t.toplevel()))
>
> Fix this issue by creating a subclass of Message, which is used for
> "standalone" message which have to be freed by the caller. Message class
> is then used only for messages descended from a query, which do not need
> to be freed by the caller.
> ---
>  bindings/python-cffi/notmuch2/_database.py |  6 ++--
>  bindings/python-cffi/notmuch2/_message.py  | 42 ++
>  2 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_database.py 
> b/bindings/python-cffi/notmuch2/_database.py
> index 95f59ca0..f14eac78 100644
> --- a/bindings/python-cffi/notmuch2/_database.py
> +++ b/bindings/python-cffi/notmuch2/_database.py
> @@ -399,7 +399,7 @@ class Database(base.NotmuchObject):
>capi.lib.NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID]
>  if ret not in ok:
>  raise errors.NotmuchError(ret)
> -msg = message.Message(self, msg_pp[0], db=self)
> +msg = message.StandaloneMessage(self, msg_pp[0], db=self)
>  if sync_flags:
>  msg.tags.from_maildir_flags()
>  return self.AddedMessage(
> @@ -468,7 +468,7 @@ class Database(base.NotmuchObject):
>  msg_p = msg_pp[0]
>  if msg_p == capi.ffi.NULL:
>  raise LookupError
> -msg = message.Message(self, msg_p, db=self)
> +msg = message.StandaloneMessage(self, msg_p, db=self)
>  return msg
>  
>  def get(self, filename):
> @@ -501,7 +501,7 @@ class Database(base.NotmuchObject):
>  msg_p = msg_pp[0]
>  if msg_p == capi.ffi.NULL:
>  raise LookupError
> -msg = message.Message(self, msg_p, db=self)
> +msg = message.StandaloneMessage(self, msg_p, db=self)
>  return msg
>  
>  @property
> diff --git a/bindings/python-cffi/notmuch2/_message.py 
> b/bindings/python-cffi/notmuch2/_message.py
> index c5fdbf6d..416ce7ca 100644
> --- a/bindings/python-cffi/notmuch2/_message.py
> +++ b/bindings/python-cffi/notmuch2/_message.py
> @@ -14,7 +14,7 @@ __all__ = ['Message']
>  
>  
>  class Message(base.NotmuchObject):
> -"""An email message stored in the notmuch database.
> +"""An email message stored in the notmuch database retrieved via a query.
>  
>  This should not be directly created, instead it will be returned
>  by calling methods on :class:`Database`.  A message keeps a
> @@ -61,22 +61,10 @@ class Message(base.NotmuchObject):
>  
>  @property
>  def alive(self):
> -if not self._parent.alive:
> -return False
> -try:
> -self._msg_p
> -except errors.ObjectDestroyedError:
> -return False
> -else:
> -return True
> -
> -def __del__(self):
> -self._destroy()
> +return self._parent.alive
>  
>  def _destroy(self):
> -if self.alive:
> -capi.lib.notmuch_message_destroy(self._msg_p)
> -self._msg_p = None
> +pass

I feel like some more description from the commit message could live
here to explain why this isn't destroying the message rather than having
to find it in the commit message.

>  
>  @property
>  def messageid(self):
> @@ -375,6 +363,30 @@ class Message(base.NotmuchObject):
>  if isinstance(other, self.__class__):
>  return self.messageid == other.messageid
>  
> +class 

Re: [PATCH 1/2] Show which notmuch command and version is being used

2019-11-18 Thread Floris Bruynooghe
On Mon 18 Nov 2019 at 07:43 -0400, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>> This add the notmuch version and absolute path of the binary used
>> in the pytest header.  This is nice when running the tests
>> interactively as you get confirmation you're testing the version you
>> thought you were testing.
>> ---
>>  bindings/python-cffi/tests/conftest.py | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/bindings/python-cffi/tests/conftest.py 
>> b/bindings/python-cffi/tests/conftest.py
>> index aa940947..674c7218 100644
>> --- a/bindings/python-cffi/tests/conftest.py
>> +++ b/bindings/python-cffi/tests/conftest.py
>> @@ -10,6 +10,13 @@ import os
>>  import pytest
>>  
>>  
>> +def pytest_report_header():
>> +vers = subprocess.run(['notmuch', '--version'], stdout=subprocess.PIPE)
>> +which = subprocess.run(['which', 'notmuch'], stdout=subprocess.PIPE)
>
> I think it would be better to use "shutil.which()" here, to avoid
> variations in shell builtin vs executable which. If you agree I can make
> the change.

Oh nice, I always forget about shutil.  Please do make the change.

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


[PATCH] Move from _add_message to _index_file API

2019-11-17 Thread Floris Bruynooghe
This moves away from the deprecated notmuch_database_add_message API
and instead uses the notmuch_database_index_file API.  This means
instroducing a class to manage the index options and bumping the
library version requirement to 5.1.
---
 bindings/python-cffi/notdb/_build.py| 26 +-
 bindings/python-cffi/notdb/_database.py | 88 -
 bindings/python-cffi/tests/test_database.py | 24 ++
 3 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/bindings/python-cffi/notdb/_build.py 
b/bindings/python-cffi/notdb/_build.py
index 6be7e5b1..642ef77a 100644
--- a/bindings/python-cffi/notdb/_build.py
+++ b/bindings/python-cffi/notdb/_build.py
@@ -12,6 +12,9 @@ ffibuilder.set_source(
 #if LIBNOTMUCH_MAJOR_VERSION < 5
 #error libnotmuch version not supported by notdb
 #endif
+#if LIBNOTMUCH_MINOR_VERSION < 1
+#ERROR libnotmuch  version < 5.1 not supported
+#endif
 """,
 include_dirs=['../../lib'],
 library_dirs=['../../lib'],
@@ -68,6 +71,12 @@ ffibuilder.cdef(
 NOTMUCH_EXCLUDE_FALSE,
 NOTMUCH_EXCLUDE_ALL
 } notmuch_exclude_t;
+typedef enum {
+NOTMUCH_DECRYPT_FALSE,
+NOTMUCH_DECRYPT_TRUE,
+NOTMUCH_DECRYPT_AUTO,
+NOTMUCH_DECRYPT_NOSTASH,
+} notmuch_decryption_policy_t;
 
 // These are fully opaque types for us, we only ever use pointers.
 typedef struct _notmuch_database notmuch_database_t;
@@ -81,6 +90,7 @@ ffibuilder.cdef(
 typedef struct _notmuch_directory notmuch_directory_t;
 typedef struct _notmuch_filenames notmuch_filenames_t;
 typedef struct _notmuch_config_list notmuch_config_list_t;
+typedef struct _notmuch_indexopts notmuch_indexopts_t;
 
 const char *
 notmuch_status_to_string (notmuch_status_t status);
@@ -118,9 +128,10 @@ ffibuilder.cdef(
 notmuch_database_get_revision (notmuch_database_t *notmuch,
const char **uuid);
 notmuch_status_t
-notmuch_database_add_message (notmuch_database_t *database,
-  const char *filename,
-  notmuch_message_t **message);
+notmuch_database_index_file (notmuch_database_t *database,
+ const char *filename,
+ notmuch_indexopts_t *indexopts,
+ notmuch_message_t **message);
 notmuch_status_t
 notmuch_database_remove_message (notmuch_database_t *database,
  const char *filename);
@@ -294,6 +305,15 @@ ffibuilder.cdef(
 notmuch_filenames_move_to_next (notmuch_filenames_t *filenames);
 void
 notmuch_filenames_destroy (notmuch_filenames_t *filenames);
+notmuch_indexopts_t *
+notmuch_database_get_default_indexopts (notmuch_database_t *db);
+notmuch_status_t
+notmuch_indexopts_set_decrypt_policy (notmuch_indexopts_t *indexopts,
+  notmuch_decryption_policy_t 
decrypt_policy);
+notmuch_decryption_policy_t
+notmuch_indexopts_get_decrypt_policy (const notmuch_indexopts_t 
*indexopts);
+void
+notmuch_indexopts_destroy (notmuch_indexopts_t *options);
 """
 )
 
diff --git a/bindings/python-cffi/notdb/_database.py 
b/bindings/python-cffi/notdb/_database.py
index d414082a..8fa631f2 100644
--- a/bindings/python-cffi/notdb/_database.py
+++ b/bindings/python-cffi/notdb/_database.py
@@ -45,6 +45,13 @@ class QueryExclude(enum.Enum):
 ALL = capi.lib.NOTMUCH_EXCLUDE_ALL
 
 
+class DecryptionPolicy(enum.Enum):
+FALSE = capi.lib.NOTMUCH_DECRYPT_FALSE
+TRUE = capi.lib.NOTMUCH_DECRYPT_TRUE
+AUTO = capi.lib.NOTMUCH_DECRYPT_AUTO
+NOSTASH = capi.lib.NOTMUCH_DECRYPT_NOSTASH
+
+
 class Database(base.NotmuchObject):
 """Toplevel access to notmuch.
 
@@ -332,7 +339,17 @@ class Database(base.NotmuchObject):
 def get_directory(self, path):
 raise NotImplementedError
 
-def add(self, filename, *, sync_flags=False):
+def default_indexopts(self):
+"""Returns default index options for the database.
+
+:raises ObjectDestoryedError: if used after destroyed.
+
+:returns: :class:`IndexOptions`.
+"""
+opts = capi.lib.notmuch_database_get_default_indexopts(self._db_p)
+return IndexOptions(self, opts)
+
+def add(self, filename, *, sync_flags=False, indexopts=None):
 """Add a message to the database.
 
 Add a new message to the notmuch database.  The message is
@@ -346,6 +363,11 @@ class Database(base.NotmuchObject):
 :param sync_flags: Whether to sync the known maildir flags to
notmuch tags.  See :meth:`Message.flags_to_tags` for
details.
+:type sync_flags: bool
+:param indexopts: The indexing options, see
+   :meth:`default_indexopts`.  Leave as `None` to use the
+   default options configured in the database.
+:type 

Adressing deprecation warnings

2019-11-17 Thread Floris Bruynooghe
This addresses the deprecation warning pointed out in another
thread on this list.  It's also a nice self-contained example
of adding an API including a new object.

This patch is against the wip/cffi branch, it does not rely
on my other patch against this branch which renames things.
Conflicts with that renaming patch should be trivial if any.


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


Re: Python3 cffi bindings

2019-11-17 Thread Floris Bruynooghe
Hi Gaute,

Thanks for trying this out!

On Mon 04 Nov 2019 at 11:27 +0100, Gaute Hope wrote:
> I just checked out the wip/cffi branch on git.notmuch.org with the
> purpose of porting Lieer (https://github.com/gauteh/lieer). There
> seems to be some missing functionality: `Database.get_directory()`
> specifically.

Yeah, I didn't add that yet because I don't fully understand how it
should be used.  Specifically I don't know where one might get a
pathname from to pass to .get_directory() and thus whether the API would
be cleaner to just return a reasonable directory object from whatever
location that might be.  Maybe notmuch_database_get_path() is the only
entrypoint here and you can get further by listing files and directories
from it?  But maybe people then use the filesystem directly to find a
directory and create the directories ad-hoc.

I grepped lieer but I think you only use it in one place?  And if I
understand it correctly you only do this to check if your mailstore/cwd
is inside the notmuch database.  I.e. this is equivalent to checking if
your mailstore/cwd has notmuch2.Database.path as prefix which you could
easily do directly rather than using the FileError exception from
.get_directory().

So is anyone else aware of some code which uses db.get_directory() to
give an idea of how and why this is used?

> I also ran into a couple of warning when building
> (included below).

Thanks for pointing these out.  I guess if the bindings are in the main
repo only the latest library version can be supported without any
further concerns.

> By the way, it does not seem that the API is very far from the
> previous python API. If it is close enough, perhaps it is possible to
> get away with a bug version bump in the bindings rather than creating
> a new package. I understand the need for a new package, but it would
> be nice if we could avoid the future confusion of two python binding
> packages (if at all possible).

While I'm glad to hear that you think a migration wouldn't be to painful
for you I am very weary of knowingly breaking APIs.  I'd rather have
people have an easy migration rather than unexpected breakage after an
upgrade.


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


Rename and small tweaks to wip/cffi

2019-11-17 Thread Floris Bruynooghe
Hi,

This is a patch against the wip/cffi branch.  It does the rename
which was discussed before and it adds a small improvment (to me)
to the test runner to show which notmuch version is being tested.

Let me know if this the right or wrong way to contribute to this
branch.

Cheers,
Floris


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


[PATCH 2/2] Rename package to notmuch2

2019-11-17 Thread Floris Bruynooghe
s/python-cffi/notdb/_errors.py 
b/bindings/python-cffi/notmuch2/_errors.py
similarity index 99%
rename from bindings/python-cffi/notdb/_errors.py
rename to bindings/python-cffi/notmuch2/_errors.py
index 924e722f..1c88763b 100644
--- a/bindings/python-cffi/notdb/_errors.py
+++ b/bindings/python-cffi/notmuch2/_errors.py
@@ -1,4 +1,4 @@
-from notdb import _capi as capi
+from notmuch2 import _capi as capi
 
 
 class NotmuchError(Exception):
diff --git a/bindings/python-cffi/notdb/_message.py 
b/bindings/python-cffi/notmuch2/_message.py
similarity index 99%
rename from bindings/python-cffi/notdb/_message.py
rename to bindings/python-cffi/notmuch2/_message.py
index 9b2b037f..bb561426 100644
--- a/bindings/python-cffi/notdb/_message.py
+++ b/bindings/python-cffi/notmuch2/_message.py
@@ -4,10 +4,10 @@ import os
 import pathlib
 import weakref
 
-import notdb._base as base
-import notdb._capi as capi
-import notdb._errors as errors
-import notdb._tags as tags
+import notmuch2._base as base
+import notmuch2._capi as capi
+import notmuch2._errors as errors
+import notmuch2._tags as tags
 
 
 __all__ = ['Message']
diff --git a/bindings/python-cffi/notdb/_query.py 
b/bindings/python-cffi/notmuch2/_query.py
similarity index 93%
rename from bindings/python-cffi/notdb/_query.py
rename to bindings/python-cffi/notmuch2/_query.py
index 613aaf12..1db6ec96 100644
--- a/bindings/python-cffi/notdb/_query.py
+++ b/bindings/python-cffi/notmuch2/_query.py
@@ -1,8 +1,8 @@
-from notdb import _base as base
-from notdb import _capi as capi
-from notdb import _errors as errors
-from notdb import _message as message
-from notdb import _thread as thread
+from notmuch2 import _base as base
+from notmuch2 import _capi as capi
+from notmuch2 import _errors as errors
+from notmuch2 import _message as message
+from notmuch2 import _thread as thread
 
 
 __all__ = []
diff --git a/bindings/python-cffi/notdb/_tags.py 
b/bindings/python-cffi/notmuch2/_tags.py
similarity index 99%
rename from bindings/python-cffi/notdb/_tags.py
rename to bindings/python-cffi/notmuch2/_tags.py
index a25a2264..fe422a79 100644
--- a/bindings/python-cffi/notdb/_tags.py
+++ b/bindings/python-cffi/notmuch2/_tags.py
@@ -1,8 +1,8 @@
 import collections.abc
 
-import notdb._base as base
-import notdb._capi as capi
-import notdb._errors as errors
+import notmuch2._base as base
+import notmuch2._capi as capi
+import notmuch2._errors as errors
 
 
 __all__ = ['ImmutableTagSet', 'MutableTagSet', 'TagsIter']
diff --git a/bindings/python-cffi/notdb/_thread.py 
b/bindings/python-cffi/notmuch2/_thread.py
similarity index 97%
rename from bindings/python-cffi/notdb/_thread.py
rename to bindings/python-cffi/notmuch2/_thread.py
index e1ef6b07..a754749f 100644
--- a/bindings/python-cffi/notdb/_thread.py
+++ b/bindings/python-cffi/notmuch2/_thread.py
@@ -1,11 +1,11 @@
 import collections.abc
 import weakref
 
-from notdb import _base as base
-from notdb import _capi as capi
-from notdb import _errors as errors
-from notdb import _message as message
-from notdb import _tags as tags
+from notmuch2 import _base as base
+from notmuch2 import _capi as capi
+from notmuch2 import _errors as errors
+from notmuch2 import _message as message
+from notmuch2 import _tags as tags
 
 
 __all__ = ['Thread']
diff --git a/bindings/python-cffi/setup.py b/bindings/python-cffi/setup.py
index 7baf63cf..37918e3d 100644
--- a/bindings/python-cffi/setup.py
+++ b/bindings/python-cffi/setup.py
@@ -2,7 +2,7 @@ import setuptools
 
 
 setuptools.setup(
-name='notdb',
+name='notmuch2',
 version='0.1',
 description='Pythonic bindings for the notmuch mail database using CFFI',
 author='Floris Bruynooghe',
@@ -10,7 +10,7 @@ setuptools.setup(
 setup_requires=['cffi>=1.0.0'],
 install_requires=['cffi>=1.0.0'],
 packages=setuptools.find_packages(exclude=['tests']),
-cffi_modules=['notdb/_build.py:ffibuilder'],
+cffi_modules=['notmuch2/_build.py:ffibuilder'],
 classifiers=[
 'Development Status :: 3 - Alpha',
 'Intended Audience :: Developers',
diff --git a/bindings/python-cffi/tests/test_base.py 
b/bindings/python-cffi/tests/test_base.py
index b6d3d62c..d3280a67 100644
--- a/bindings/python-cffi/tests/test_base.py
+++ b/bindings/python-cffi/tests/test_base.py
@@ -1,7 +1,7 @@
 import pytest
 
-from notdb import _base as base
-from notdb import _errors as errors
+from notmuch2 import _base as base
+from notmuch2 import _errors as errors
 
 
 class TestNotmuchObject:
diff --git a/bindings/python-cffi/tests/test_database.py 
b/bindings/python-cffi/tests/test_database.py
index 02de0f41..e3a8344d 100644
--- a/bindings/python-cffi/tests/test_database.py
+++ b/bindings/python-cffi/tests/test_database.py
@@ -5,10 +5,10 @@ import pathlib
 
 import pytest
 
-import notdb
-import notdb._errors as errors
-import notdb._database as dbmod
-import notdb._message as message
+import notmuch2
+import notmuch2._errors as errors
+import notmuch2._database as dbmod
+import notmuch2._

[PATCH 1/2] Show which notmuch command and version is being used

2019-11-17 Thread Floris Bruynooghe
This add the notmuch version and absolute path of the binary used
in the pytest header.  This is nice when running the tests
interactively as you get confirmation you're testing the version you
thought you were testing.
---
 bindings/python-cffi/tests/conftest.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/bindings/python-cffi/tests/conftest.py 
b/bindings/python-cffi/tests/conftest.py
index aa940947..674c7218 100644
--- a/bindings/python-cffi/tests/conftest.py
+++ b/bindings/python-cffi/tests/conftest.py
@@ -10,6 +10,13 @@ import os
 import pytest
 
 
+def pytest_report_header():
+vers = subprocess.run(['notmuch', '--version'], stdout=subprocess.PIPE)
+which = subprocess.run(['which', 'notmuch'], stdout=subprocess.PIPE)
+return ['{} ({})'.format(vers.stdout.decode(errors='replace').strip(),
+ which.stdout.decode(errors='replace').strip())]
+
+
 @pytest.fixture(scope='function')
 def tmppath(tmpdir):
 """The tmpdir fixture wrapped in pathlib.Path."""
-- 
2.24.0

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


Re: python CFFI bindings integration into notmuch build/test

2019-11-17 Thread Floris Bruynooghe
On Sat 16 Nov 2019 at 10:51 -0500, David Bremner wrote:

> Floris Bruynooghe  writes:
>> Anyway, this looks good.  Would you like some changes, e.g. the rename
>> to notmuch2 or so as patches?  What's the next step.
>
> If you could look at the rename that would be great. Patches on top of
> wip/cffi, or a ref for me to pull both sound fine.
>
>> Kind of unrelated, but in my attempt to run the full notmuch test suite
>> (cd tests; make test) I somehow ended up with lots of weird tags in my
>> notmuch database and made the emacs UI pretty horrible - though it seems
>> I haven't lost any email.  I'm sure that was my fault somehow even
>> though I was just trying to follow the readme.  But if anyone has any
>> hints how to recover from this that could save me some time :)
>
> Hmm. I've never encountered that, but maybe it has to do with killing
> the environment? The test harness uses the environment variable
> NOTMUCH_CONFIG to point to a test database, and if that was deleted, it
> would look in ~/.notmuch-config.

Yeah, I probably managed to make it use ~/.notmuch-config somehow.

> Still that would really only be tags from your pytest tests, and you'd
> probably recognize those?

The were a bunch of weird characters that looked like unicode
test-cases, the pytest tests don't try to test notmuch only the bindings
so doesn't really make up weird strings IIRC.  I recovered with
something like "notmuch dump | cleanup_tags.py | notmuch restore", oh
well.

I still have a bunch of non-existing messages with weird unicode message
IDs in them, but no associated filename just an empty string.  AFAIK
notmuch_database_remove_message() needs a filename though, I've tried
removing the empty file which does not fail but doesn't clear those
database entries.  The messages are not ghosts either.  Any idea how I
might be able to clear those bad messages from the database?

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


Re: python CFFI bindings integration into notmuch build/test

2019-11-14 Thread Floris Bruynooghe
On Thu 14 Nov 2019 at 23:24 +0100, Floris Bruynooghe wrote:

> On Thu 14 Nov 2019 at 22:20 +0200, Tomi Ollila wrote:
>> In git://notmuchmail.org/git/notmuch David has ref origin/wip/cffi
>> which contains related changes -- You can fetch the code while waiting
>> for more collaboration instructions from David.
>
> Aha, thanks!

So I can run the tests using `make` and `cd tests;
./T-391-pytest-cffi.sh`.  I can also run the tests still individually
using something like:

make
set PATH (pwd) $PATH  # I use fish
pushd bindings/python-cffi
pew workon notmuch  # how I manage python virtualenvs
pip install -e .
pip install pytest pytest-cov
pytest test/test_base.py

And all works.  I added a little function to the conftest.py to show
which notmuch it's testing with:

def pytest_report_header():
vers = subprocess.run(['notmuch', '--version'],
  capture_output=True, text=True)
which = subprocess.run(['which', 'notmuch'],
   capture_output=True, text=True)
return ['{} ({})'.format(vers.stdout.strip(), which.stdout.strip())]

Maybe you find that useful too.

Anyway, this looks good.  Would you like some changes, e.g. the rename
to notmuch2 or so as patches?  What's the next step.



Kind of unrelated, but in my attempt to run the full notmuch test suite
(cd tests; make test) I somehow ended up with lots of weird tags in my
notmuch database and made the emacs UI pretty horrible - though it seems
I haven't lost any email.  I'm sure that was my fault somehow even
though I was just trying to follow the readme.  But if anyone has any
hints how to recover from this that could save me some time :)

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


Re: python CFFI bindings integration into notmuch build/test

2019-11-14 Thread Floris Bruynooghe
On Thu 14 Nov 2019 at 22:20 +0200, Tomi Ollila wrote:
> In git://notmuchmail.org/git/notmuch David has ref origin/wip/cffi
> which contains related changes -- You can fetch the code while waiting
> for more collaboration instructions from David.

Aha, thanks!

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


Re: python CFFI bindings integration into notmuch build/test

2019-11-14 Thread Floris Bruynooghe
Hi,

Thanks for carrying on with this!

I'm a little confused with how to follow this, is the current state of
the code somewhere in a repo/branch where I can try things out and make
changes from?

On Tue 05 Nov 2019 at 22:22 -0400, David Bremner wrote:

> Tomi Ollila  writes:
>
>
>> alternative:
>>
>>...
>>print('Invoking: {}'.format(' '.join(cmd)))
>> 
>>def preexec_fn(): os.environ['NOTMUCH_CONFIG'] = str(cfg_fname) 
>>
>>proc = subprocess.run(cmd, timeout=5, preexec_fn=preexec_fn)
>>...
>>
>> The unix fork ... (here preexec_fn called in child) ... exec model is
>> superior to any other alternative ! =D
>
> I don't consider myself a good judge of python style, so I'll defer to
> Floris on that one.

I'd have gone with what David wrote to be fair, mostly because it's
pretty simple to see what's going on where for the preexec_fn I'd have
to look at the docs.  Not sure if this has much to do with Python style
though, more with how much you like Unix tricks.


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


Re: Python3 cffi bindings

2019-10-25 Thread Floris Bruynooghe
On Tue 22 Oct 2019 at 13:32 -0300, David Bremner wrote:

> David Bremner  writes:
>
>> The LD_LIBRARY_PATH is already set by the test harness, as is PATH (to
>> find notmuch). It looks like your function notmuch is not respecting
>> PATH (see attached log). if I hack something like
>>
>> diff --git a/bindings/python-cffi/tests/conftest.py 
>> b/bindings/python-cffi/tests/conftest.py
>> index 1b7bbc35..ac17397c 100644
>> --- a/bindings/python-cffi/tests/conftest.py
>> +++ b/bindings/python-cffi/tests/conftest.py
>> @@ -31,7 +31,7 @@ def notmuch(maildir):
>>  accidentally do this in the unittests.
>>  """
>>  cfg_fname = maildir.path / 'notmuch-config'
>> -cmd = ['notmuch'] + list(args)
>> +cmd = ['../../notmuch'] + list(args)
>>  print('Invoking: {}'.format(' '.join(cmd)))
>>  proc = subprocess.run(cmd,
>>
>
> I think I figured it out. Your 'run' function completely overrides the
> environment. But just adding PATH back seems to do the trick. I'm not
> sure if this is the most idomatic change, but it works:
>
> diff --git a/bindings/python-cffi/tests/conftest.py 
> b/bindings/python-cffi/tests/conftest.py
> index 1b7bbc35..6a81aa18 100644
> --- a/bindings/python-cffi/tests/conftest.py
> +++ b/bindings/python-cffi/tests/conftest.py
> @@ -33,9 +33,11 @@ def notmuch(maildir):
>  cfg_fname = maildir.path / 'notmuch-config'
>  cmd = ['notmuch'] + list(args)
>  print('Invoking: {}'.format(' '.join(cmd)))
>proc = subprocess.run(cmd,
>timeout=5,
> -  env={'NOTMUCH_CONFIG': str(cfg_fname)})
> +  
> env={'PATH':os.environ["PATH"],'NOTMUCH_CONFIG': str(cfg_fname)})
>  proc.check_returncode()
>  return run
>  

This seems reasonable, perhaps even a "env = os.environ.copy();
env['NOTMUCH_CONFIG'] = src(cfg_fname)" is better here so that
LD_LIBRARY_PATH and anything else is kept around.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Python3 cffi bindings

2019-10-17 Thread Floris Bruynooghe
On Mon 14 Oct 2019 at 09:42 -0300, David Bremner wrote:

> David Bremner  writes:
>
>> The shim in
>> T391-python-cffi.sh doesn't work for me, it doesn't manage to set
>> PYTHONPATH so that notdb is importable.

Ah yes, I tested this shim while activating a venv with the extension
installed using `pip -e .`.

> I should have mentioned that if I manually set python path with
> something like
>
> $ PYTHONPATH=`pwd`/build/lib.linux-x86_64-3.7:$PYTHONPATH pytest-3
>
> it works OK.  Is there a simple/reliable way of calculating the path
> lib.linux-x86_64-3.7?

It is possible to run this without installing, but it does need a build
step since cffi (in the mode used - which is the recommended mode) needs
to build an extension module.  I did something like this, using my
debian testing system-installed python

$ export PYTHONPATH=$(pwd)/bindings/python-cffi
$ pushd bindings/python-cffi
$ python3 notdb/_build.py  # creates notdb/_capi.cpython-37m-x86_64-linux-gnu.so
$ popd
$ pushd test
$ ./T391-pytest.sh

Does that more or less work?  One problem with this is that it will pick
up the system-wide installed notmuch though.  I guess the way to change
this is by tweaking CFLAGS=-I... LDFLAGS=-L... or so when building?  But
than you also have the whole RPATH/LD_LIBRARY_PATH stuff going on as
well.  Does notmuch abstract any of this away already for it's test
suite?

Cheers,
Floris

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


Re: Python3 cffi bindings

2019-10-09 Thread Floris Bruynooghe
On Tue 08 Oct 2019 at 19:24 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>> Anyway, I found the code, checked things work, updated tests on new
>> python versions, added a very basic intergration with the test
>> framework and squashed the commits.  Otherwise the attached patch
>> is just a plain dump of the current state so interested people have
>> at least a copy of the code again which can be made to work.
>
> I think you missed the attachement. Other than that, sounds interesting ;).

I used git send-email as per the contributing nodes, so it was supposed
to be a followup email.  It does seem like I got a message back saying
the patch mail is being held in the moderator queue as it's too large...
Perhaps you have moderation powers?

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


Python3 cffi bindings

2019-10-08 Thread Floris Bruynooghe
Hi all,

IIRC there was a thread in August about another attempt at bringing
the CFFI-based bindings on board as a Python3-only version.  I
believe there was a desire to re-name things but my searching-fu is
failing me and I can no longer find the email thread.

Anyway, I found the code, checked things work, updated tests on new
python versions, added a very basic intergration with the test
framework and squashed the commits.  Otherwise the attached patch
is just a plain dump of the current state so interested people have
at least a copy of the code again which can be made to work.

IIRC this probably wants to be renamed to "notmuch2" instead of
"notdb".  Otherwise I'm pretty sure this doesn't cover all the
current features either.

So maybe this can be used as a start to figure out how to merge
this if there's still an interest in this.

Cheers,
Floris


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


Re: segfault using python bindings

2019-08-26 Thread Floris Bruynooghe
On Wed 21 Aug 2019 at 12:02 -0400, Daniel Kahn Gillmor wrote:

> On Tue 2019-08-20 19:20:30 +0200, Floris Bruynooghe wrote:
>> For path series what did you have in mind?  One single patch with the
>> whole lot?  The original history at https:://github/flub/notdb?
>> Something in between?
>
> I would be happy with a series of patches, but the cited github history
> has only one commit in the first place :P

hah!  Perhaps the actual history is somewhere in a hg repo, but I can't
even find that.  Not that it matters much.

>> I also recall some comments about the naming not being liked much
>> (notdb).  I'm open to some bikeshedding on the naming if it bothers
>> people.  I was only aiming for something short but under a different
>> import name, which are probably still useful features to keep.
>
> fwiw, "notdb" doesn't really resonate with me, but i wouldn't block you
> if you decided to stick with it.
>
> I'd generally prefer something like "notmuch2" because it makes it much
> clearer that it is associated with notmuch, and that it is the successor
> to the original notmuch bindings.

>From the ensuing conversation I'm concluding that "notmuch2" is the most
liked option, so I'll go with that (and yes, I share many of the
disappointed sentiments raised on that naming convention, but it's how
things are in python).


I'll try and send a first patch series in the coming week.

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


Re: segfault using python bindings

2019-08-20 Thread Floris Bruynooghe
On Thu 15 Aug 2019 at 09:28 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>> On Wed 14 Aug 2019 at 16:20 -0300, David Bremner wrote:
>>>
>>> Can you remind me what the percieved blockers are for merging into the
>>> main notmuch tree? I'm less hung up on python2 compatibility than I used
>>> to be, fwiw. I'd be ok with shipping the old python2 bindings in contrib
>>> for a bit for those who still need/want them, but concentrate our
>>> maintenance effort on the cffi bindings.
>>
>> IIRC it was mostly about how to support transitioning from one API to
>> the other since currently there's no compatibility.  I guess there's
>> nothing stopping one from using both APIs in one codebase though, AFAIK
>> Xapian handles the required locking.  But some of the discussions
>> suggested being able to create a new Message object from an old one etc,
>> allowing you to do more mixing during a transition period.  This is the
>> part that I said is possible but a lot of work and questionable if no
>> one thought they'd be using it.
>>
>
> Ah right. Well, given the impending removal of python2 from various
> places (e.g. debian testing), I don't think we should be that
> fussy/ambitious.
>
> I'd propose
>
> - add the new cffi based bindings, using a distinct name (as you already
>   have).
>
> - deprecate the old ones
>
> - port any internal dependencies to the new bindings
>
> - finally drop the old bindings or move them to contrib/ for people slow
>   in switching.
>
> I think for the first step we only need a reasonable looking patch
> (series?) from you.

Sounds reasonable, just a quick note that I'm on holiday at the moment
and generally won't be too quick.  But I guess there's no rush.  I was
also trying to improve the docs but got sidetracked at some point.
There should be already reasonable good docstrings though.

For path series what did you have in mind?  One single patch with the
whole lot?  The original history at https:://github/flub/notdb?
Something in between?

I also recall some comments about the naming not being liked much
(notdb).  I'm open to some bikeshedding on the naming if it bothers
people.  I was only aiming for something short but under a different
import name, which are probably still useful features to keep.

Cheers,
Floris

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


Re: segfault using python bindings

2019-08-15 Thread Floris Bruynooghe
On Wed 14 Aug 2019 at 16:20 -0300, David Bremner wrote:

> Floris Bruynooghe  writes:
>
>> These are at https://github.com/flub/notmuch/tree/cffi/bindings/python-cffi
>>
>> I'm not really convinced of the way forward last time it was discussed
>> on how to get them merged into notmuch itself so have failed to put in
>> the not insignificant effort.
>>
>> I've since wondered if just getting them standalone on pypi is perhaps a
>> useful service in the mean time as it's relatively little effort.  And
>> if there eventually is a desire again to get them merged in some way
>> that could still be done.
>
> Can you remind me what the percieved blockers are for merging into the
> main notmuch tree? I'm less hung up on python2 compatibility than I used
> to be, fwiw. I'd be ok with shipping the old python2 bindings in contrib
> for a bit for those who still need/want them, but concentrate our
> maintenance effort on the cffi bindings.

IIRC it was mostly about how to support transitioning from one API to
the other since currently there's no compatibility.  I guess there's
nothing stopping one from using both APIs in one codebase though, AFAIK
Xapian handles the required locking.  But some of the discussions
suggested being able to create a new Message object from an old one etc,
allowing you to do more mixing during a transition period.  This is the
part that I said is possible but a lot of work and questionable if no
one thought they'd be using it.

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


Re: segfault using python bindings

2018-11-16 Thread Floris Bruynooghe
On Fri 16 Nov 2018 at 07:15 -0500, Daniel Kahn Gillmor wrote:

> On Fri 2018-11-16 06:27:12 -0400, David Bremner wrote:
>> Floris Bruynooghe  writes:
>>
>>> These are at https://github.com/flub/notmuch/tree/cffi/bindings/python-cffi
>>>
>>> I'm not really convinced of the way forward last time it was discussed
>>> on how to get them merged into notmuch itself so have failed to put in
>>> the not insignificant effort.
>>>
>>> I've since wondered if just getting them standalone on pypi is perhaps a
>>> useful service in the mean time as it's relatively little effort.  And
>>> if there eventually is a desire again to get them merged in some way
>>> that could still be done.
>>
>> What effort are you referring to specifically? Integration with the
>> notmuch test suite?
>
> My recollection is that the main question was about supporting the old
> python interface with the new bindings, so that consumers would have a
> smooth upgrade path.  Is that not right?

That's indeed what I was referring to, integration with the test suite
is fine as was discussed last time imho.

> Floris, i really appreciate the work you put in here, and i'd love to
> see notmuch be able to adopt it directly.   can we figure out what is
> needed to take these changes?

Thanks.  I think mainly when the technical approach was discussed [0] no
actual users of the current Python API weighed in with if they'd be
interested in a migration of the API and if so, how it might work for
them.  So while the gradual approach described there is technically
somewhat nice I have no idea if anyone would benefit from it, or whether
the benefits outweigh all the work involved.

As I was recently thinking however, maybe there's nothing wrong with new
bindings being published as a 3rd party package on pypi.  It'd make it
more discoverable and if people start to adopt it maybe there'd be more
demand for integrating it back with more clarity over how smooth a
transition path needs to be.

Also lastly an apology.  I could have done more to move this forward,
but I simply haven't found^Wmade the time for it.

Cheers,
Floris


[0] id:py3imv2cjp28@devork.be
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: segfault using python bindings

2018-11-16 Thread Floris Bruynooghe
On Fri 16 Nov 2018 at 06:29 -0400, David Bremner wrote:

> Brian May  writes:
>
>> Floris Bruynooghe  writes:
>>
>>> I've since wondered if just getting them standalone on pypi is perhaps a
>>> useful service in the mean time as it's relatively little effort.  And
>>> if there eventually is a desire again to get them merged in some way
>>> that could still be done.
>>
>> Standalone on pypi would be my preferred option.
>>
>> It is defacto Python standard to refer to all dependancies in something
>> like requirements.txt or Pipfile from pypi.
>
> As I mentioned last time this was discussed, the python bindings are
> currently more or less a core part of notmuch as both the test
> suite and developement need them.

Sure, I think pypi publishing is orthogonal to this however.  Either or
both versions of the bindings could be published on pypi in addition to
being in the main repo.  As Brian mentions it would improve
discoverability and improves integration on the python side.  There's
even tooling to bundle the library these days with the manylinux1
wheels.  So there's no need to stop anyone who'd like to do this.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: segfault using python bindings

2018-11-15 Thread Floris Bruynooghe
Hi,

On Sun 11 Nov 2018 at 16:16 -0400, David Bremner wrote:

> David Čepelík  writes:
>
>> Hello Notmuch devs,
>>
>> I'm facing an issue trying to use the Python bindings. This trivial
>> piece of code segfaults:
>>
>> import notmuch
>
> I don't remember the details [1], but there are known conflicts between
> recent versions of python3 and the way the notmuch python bindings
> manage memory. So it could be that. There was also an initiative to
> rewrite at (python3 only?) version of the bindings that did not have
> this problem. I haven't heard much about that recently.

These are at https://github.com/flub/notmuch/tree/cffi/bindings/python-cffi

I'm not really convinced of the way forward last time it was discussed
on how to get them merged into notmuch itself so have failed to put in
the not insignificant effort.

I've since wondered if just getting them standalone on pypi is perhaps a
useful service in the mean time as it's relatively little effort.  And
if there eventually is a desire again to get them merged in some way
that could still be done.


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


Re: [PATCH 2/2] test: pytest runner for the test suite

2018-04-13 Thread Floris Bruynooghe
On Sun 08 Apr 2018 at 19:14 -0300, David Bremner wrote:

> Floris Bruynooghe <f...@devork.be> writes:
>
>> This series looks good to me, would be great to have!  Do you want to
>> commit them this or should I just incorporate it and submit together
>> with tests once actual tests exist.  You could always commit with a ``def
>> test_dummy(): assert True`` or something if you like.
>>
>
> For now, why don't you just incorporate them. Maybe you'll discover some
> issues with them as you work up some real tests.

Sure, that works.

> BTW, it seems like a
> reasonable plan to get a set of unit tests for the existing bindings
> first to help with migration. Is that what you had in mind?

Yes that's what I had in mind.  To safely swap out ctypes for cffi
adding tests to ensure the existing API behaviour remains is a must.

Which reminds me that it's probably worth calling this out explicitly
again: this works implies that cffi will become an external dependency
[0] for the existing bindings.  Just want to make sure this doesn't
become an issue further down the line.

Cheers,
Floris

[0] Technically only on cpython, it's bundled with pypy.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 2/2] test: pytest runner for the test suite

2018-04-08 Thread Floris Bruynooghe
This series looks good to me, would be great to have!  Do you want to
commit them this or should I just incorporate it and submit together
with tests once actual tests exist.  You could always commit with a ``def
test_dummy(): assert True`` or something if you like.

Thanks!
Floris


On Sat 07 Apr 2018 at 18:39 -0300, David Bremner wrote:

> The 'test_subtest_known_broken' should be removed when there are
> actual tests to run.
>
> Based on a function from Tomi [1]
>
> [1]: id:m2r2nq23r9@guru.guru-group.fi
> ---
>  test/T391-pytest.sh | 14 ++
>  1 file changed, 14 insertions(+)
>  create mode 100755 test/T391-pytest.sh
>
> diff --git a/test/T391-pytest.sh b/test/T391-pytest.sh
> new file mode 100755
> index ..9ac7aabe
> --- /dev/null
> +++ b/test/T391-pytest.sh
> @@ -0,0 +1,14 @@
> +#!/usr/bin/env bash
> +test_description="python bindings (pytest)"
> +. $(dirname "$0")/test-lib.sh || exit 1
> +
> +test_require_external_prereq ${NOTMUCH_PYTHON}
> +
> +for bin in ${NOTMUCH_PYTEST_PYTHONS}; do
> +test_begin_subtest "pytest ($bin)"
> +  test_subtest_known_broken
> +   
> PYTHONPATH="$NOTMUCH_SRCDIR/bindings/python${PYTHONPATH:+:$PYTHONPATH}" \
> + test_expect_success "$bin -m pytest 
> $NOTMUCH_SRCDIR/bindings/python"
> +done
> +
> +test_done
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


experimenting with pytest tests

2018-04-07 Thread Floris Bruynooghe
Hi,

From another conversation on this list I've dug up my earlier attempts
at integrating a pytest run into the notmuch testing suite.  I'd be
grateful for some guidance on whether this is the right way to go about
things.

Here's the diff of configure:

diff --git a/configure b/configure
index c5e2ffed..8aa57b83 100755
--- a/configure
+++ b/configure
@@ -567,6 +567,28 @@ if [ $have_python -eq 0 ]; then
 errors=$((errors + 1))
 fi
 
+check_python() {
+local bin=$1
+local var=$(echo $bin | tr -d '.')
+printf "Checking for $bin (with: pytest)... "
+if command -v $bin > /dev/null; then
+if $bin -c 'import pytest' >/dev/null 2>&1; then
+eval have_$var=1
+eval $var=$bin
+printf "Yes.\n"
+else
+printf "No (skipping $bin tests).\n"
+fi
+else
+printf "No (skipping $bin tests).\n"
+fi
+}
+
+check_python python2.7
+check_python python3.5
+check_python python3.6
+check_python pypy3.5
+
 printf "Checking for valgrind development files... "
 if pkg-config --exists valgrind; then
 printf "Yes.\n"
@@ -1209,6 +1231,10 @@ NOTMUCH_HAVE_MAN=$((have_sphinx))
 
 # Name of python interpreter
 NOTMUCH_PYTHON=${python}
+NOTMUCH_PYTHON27=${python27-}
+NOTMUCH_PYTHON35=${python35-}
+NOTMUCH_PYTHON36=${python36-}
+NOTMUCH_PYPY35=${pypy35-}


And I was then also trying to introduce a test/T391-pytest.sh file.
What I'm trying to do in this file, but it doesn't currently work, is to
have one test, with 4 subtests where each subtest is a pytest run with a
particular python version.  But if the NOTMUCH_PYTHON27 (etc) is not
found in sh.config then the subtest should be skipped.  I'm not really
familiar enough with test-lib.sh to do this correctly without a bunch of
more looking into it, but maybe someone else knows how to do this?
Anyway, here my failing attempt:

#!/usr/bin/env bash
test_description="python unittests"
. ./test-lib.sh || exit 1


test_require_external_prereq "${NOTMUCH_PYTHON27}" && {
test_begin_subtest "${NOTMUCH_PYTHON27}"
(
cd "$NOTMUCH_SRCDIR/bindings/python"
PYTHONPATH=".${PYTHONPATH:+:$PYTHONPATH}" \
$NOTMUCH_PYTHON27 -m pytest
)
test_expect_equal $? 0
}


test_require_external_prereq ${NOTMUCH_PYTHON35} && {
test_begin_subtest "${NOTMUCH_PYTHON35}"
(
cd "$NOTMUCH_SRCDIR/bindings/python"
PYTHONPATH=".${PYTHONPATH:+:$PYTHONPATH}" \
$NOTMUCH_PYTHON35 -m pytest
)
test_expect_equal $? 0
}


test_done


Any tips on whether this is the right direction?

Many thanks,
Floris
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: New Python bindings (was: Crash with Python bindings)

2018-03-28 Thread Floris Bruynooghe
On Wed, Mar 28 2018, David Bremner wrote:

> Brian May  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.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: New Python bindings (was: Crash with Python bindings)

2018-03-28 Thread Floris Bruynooghe
On Wed, Mar 28 2018, Brian May wrote:

> Justus Winter  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.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: New Python bindings

2018-03-28 Thread Floris Bruynooghe
On Wed, Mar 28 2018, Justus Winter wrote:

> Floris Bruynooghe <f...@devork.be> writes:
>
>> On Wed, Mar 21 2018, Justus Winter wrote:
>>>
>>> Floris Bruynooghe <f...@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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: pytest integration for the notmuch test suite

2018-03-26 Thread Floris Bruynooghe
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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/3] configure: check for pytest binary

2018-03-26 Thread Floris Bruynooghe
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.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: New Python bindings (was: Crash with Python bindings)

2018-03-26 Thread Floris Bruynooghe
On Wed, Mar 21 2018, Justus Winter wrote:
>
> Floris Bruynooghe <f...@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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Crash with Python bindings

2018-03-18 Thread Floris Bruynooghe
Daniel Kahn Gillmor <d...@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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Crash with Python bindings

2018-03-16 Thread Floris Bruynooghe
Hi all,

David Bremner  writes:

> "W. Trevor King"  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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


LIBNOTMUCH_MINOR_VERSION for 0.26

2018-01-15 Thread Floris Bruynooghe
Hi,

I was looking to update my bindings with the latest release and am
somewhat confused on the value of the LIBNOTMUCH_MINOR_VERSION value.
It seems to be suggested in e.g. the documentation of
notmuch_database_index_file that for notmuch 0.26 this should be 1,
however the header still seems to include it as 0.

Was this accidentally not updated or is there another explanation that I
haven't figured out yet?

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


Re: [PATCH] NEWS: library changes for 0.26

2017-12-30 Thread Floris Bruynooghe
David Bremner  writes:

> These are just "my" changes, as arbited by "git shortlog", which
> sometimes lies.
> ---
>  NEWS | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index 3b6404e7..1c5edf4c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -41,6 +41,27 @@ Indexing cleartext of encrypted e-mails
>that the notmuch index itself is adequately protected.  DO NOT USE
>this feature without considering the security of your index.
>  
> +Library Changes
> +---
> +
> +Indexing files with duplicate message-id
> +
> +  Files with message-id's are now indexed, and searchable via terms

"Files with duplicate message-IDs are now ..."?  
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Update on python-cffi bindings

2017-12-29 Thread Floris Bruynooghe
Hi all,

Daniel Kahn Gillmor <d...@fifthhorseman.net> writes:
> On Thu 2017-12-21 12:30:39 +0100, Floris Bruynooghe wrote:
>> The API changes a lot and there is no easy migration.  And history has
>> shown that's a terrible way to get something new adopted.  Last time I
>> suggested a possible multi-tiered approach (maybe not as explicit):
>>
>> 1 I think it's possible to move the memory management technique to the
>>   existing API without API breakage.  It would still allow users to call
>>   functions in the wrong order etc, but that's not any regression.
>>
>> 2 It's probably possible to either switch the existing API to use CFFI
>>   or create a drop-in replacement for it based on CFFI.  The benefit
>>   here is two-fold: users get better PyPy performance and I believe it
>>   becomes easier to maintain the bindings, e.g. all you need to do to
>>   call notmuch_database_get_path is
>>   capi.ffi.string(capi.lib.notmuch_database_get_path(dbptr)) (see
>>   
>> https://github.com/flub/notmuch/blob/cffi/bindings/python-cffi/notdb/_database.py#L263)
>>   for an actual example).  But this really depends on what everyone else
>>   here that maintains the Python bindings here thinks.  I'd encourage to
>>   have a look at the CFFI implementation to get an idea of this.
>
> these two steps on their own seem like they give us:
>
>   * better and safer memory management
>
>   * better performance on PyPy
>
>   * arguably, easier maintenance of the bindings.
>
> These seem like unambiguous wins, and there is no downside -- people
> using the notmuch module can just upgrade to the new version and it's
> done.  I'd love to see these changes happen.
>
> The only thing it doesn't do is provide more idiomatic bindings for new
> users, which you describe as:
>
>> 3 As last step I still think providing the more idiomatic bindings is
>>   useful, especially for new users.  It does take the burden of
>>   correctly calling the C functions somewhat more.  This could then
>>   either treat a notmuch_cffi as a lower level API which more directly
>>   maps the C API or it could call C directly as it does now.  I'm not
>>   currently sure on which is more feasible or better here.
>>
>>   An additional thing that could be done here to ease migration is to
>>   allow creating the new objects from the old ones allowing a codebase
>>   to gradually adopt the newer API where appropriate.  E.g.:
>>
>>  db = notmuch.Database(...)
>>  msg = db.find_message(...)
>>  new_db = notdb.Database.from_notmuch(db)
>>  new_msg = notdb.Message.from_notmuch(msg)
>>  print('Tags not on msg: {}'.format(new_db.tags - new_msg.tags))
>>
>>   This would rely on the existing API to migrate to CFFI, otherwise it
>>   could still be possible but would be very hairy.
>
> I'm not sure i understand this last sentence.  Are you saying that step
> 3 depends on step 2 happening?

The mixing of a new API with the existing API would be possible if we
can successfully migrate the existing notmuch API to use CFFI.  If not
it may theoretically be possible but could be a lot of pain.

> I'm not sure about the name "notdb"

I'm in no way attached to this name, just needed something to work with.
Naming things is hard, if anyone has a different idea please share.

> but i don't mind the idea of there
> being some other "more-pythonic" notmuch bindings.  New users would
> likely prefer it, and that'd be fine.
>
>> - For exposing completely new APIs, sure building the
>>   notdb.ImmutableTagSet and MutableTagSet was not straight forward,
>>   likewise for the PropertiesMap.  Many other things are much easier
>>   though.  One possible nice way to alleviate this with the idea of the
>>   existing notmuch API being the lower-level layer nearly mimicking the
>>   C-API directly.  That way adding new APIs there is more or less
>>   straight forward and there is less time pressure to add them to the
>>   higher level API.  Especially if mixing the APIs is supported.
>
> I think this is in line with the approach described above.  I like it.

I would really like to hear from some of the existing python binding
users on all of the above and what they thing of this approach.  Is this
work worth it to you?  Will it make you more or less likely to want to
migrate to more pythonic bindings?

In particular I'm not that convinced in the suitability of the existing
library as a low-level binding as it currently sits somewhere above
basic wrapping of libnotmuch.  Anyway, I would appreciate any feedback
on the proposed APIs and way forward.


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


Re: Update on python-cffi bindings

2017-12-21 Thread Floris Bruynooghe
Daniel Kahn Gillmor <d...@fifthhorseman.net> writes:

> Hi Floris--
>
> On Sun 2017-12-17 19:08:18 +0100, Floris Bruynooghe wrote:
>
> i've heard reported, and i also appreciate your attention to performance
> concerns on different python platforms (e.g. making sure things are
> performant on both CPython and PyPy).

Oh btw, I can happily report this has paid off.  These bindings perform
much better on PyPy while performing slightly worse on CPython.  I
haven't proven this but my guess is most of the performance loss on
CPython is due to the memory management which unfortunately involves a
lot of calls on each object destruction, also each object needs to be
destroyed individually so when destroying a parent all children will get
destroyed first which is a waste from libnotmuch's point of view.  This
can be improved on by using a weakref.finalizer-modeled approach, but
is some more work.

> I confess that i haven't read the series in full, but i have two main
> concerns that i'd generally use to evaluate proposals like this:
>
>  0) how much does the API change?  that is, if we're expecting existing
> users of the notmuch python bindings to adopt this new approach, how
> much pain are we putting them through?  How much of an effort has
> been made to reduce that pain, and do we have a clear and
> comprehensive porting guide?

The API changes a lot and there is no easy migration.  And history has
shown that's a terrible way to get something new adopted.  Last time I
suggested a possible multi-tiered approach (maybe not as explicit):

1 I think it's possible to move the memory management technique to the
  existing API without API breakage.  It would still allow users to call
  functions in the wrong order etc, but that's not any regression.

2 It's probably possible to either switch the existing API to use CFFI
  or create a drop-in replacement for it based on CFFI.  The benefit
  here is two-fold: users get better PyPy performance and I believe it
  becomes easier to maintain the bindings, e.g. all you need to do to
  call notmuch_database_get_path is
  capi.ffi.string(capi.lib.notmuch_database_get_path(dbptr)) (see
  
https://github.com/flub/notmuch/blob/cffi/bindings/python-cffi/notdb/_database.py#L263)
  for an actual example).  But this really depends on what everyone else
  here that maintains the Python bindings here thinks.  I'd encourage to
  have a look at the CFFI implementation to get an idea of this.

3 As last step I still think providing the more idiomatic bindings is
  useful, especially for new users.  It does take the burden of
  correctly calling the C functions somewhat more.  This could then
  either treat a notmuch_cffi as a lower level API which more directly
  maps the C API or it could call C directly as it does now.  I'm not
  currently sure on which is more feasible or better here.

  An additional thing that could be done here to ease migration is to
  allow creating the new objects from the old ones allowing a codebase
  to gradually adopt the newer API where appropriate.  E.g.:

 db = notmuch.Database(...)
 msg = db.find_message(...)
 new_db = notdb.Database.from_notmuch(db)
 new_msg = notdb.Message.from_notmuch(msg)
 print('Tags not on msg: {}'.format(new_db.tags - new_msg.tags))

  This would rely on the existing API to migrate to CFFI, otherwise it
  could still be possible but would be very hairy.

So do people reckon following this path would be more desirable and
worth the extra effort?  Would there be a willingness to change the
exising notmuch API to a CFFI implementation?  I didn't go down this
path yet as last time there was no feedback on this suggestion while
there was some moderate curiosity for a more idiomatic API.

>  1) how accessible is the new implementation for contributions from
> other developers?  For example, a transition to a highly idiomatic
> style of python that no other developers would be able to improve
> would put a large maintenance burden on you.

- For the CFFI-part I believe this is easier then the existing ctypes as
  I tried to say above.

- For exposing completely new APIs, sure building the
  notdb.ImmutableTagSet and MutableTagSet was not straight forward,
  likewise for the PropertiesMap.  Many other things are much easier
  though.  One possible nice way to alleviate this with the idea of the
  existing notmuch API being the lower-level layer nearly mimicking the
  C-API directly.  That way adding new APIs there is more or less
  straight forward and there is less time pressure to add them to the
  higher level API.  Especially if mixing the APIs is supported.


> Do you have any thoughts about these questions?
>
> For example, for point 0, have you tried to run alot or some other
> python-based notmuch MUA against this newer python binding?  what
> changes were necessary?
>
> For example, for point 1, c

Update on python-cffi bindings

2017-12-17 Thread Floris Bruynooghe
Hi all,

Thanks for all the feedback on an early post of my CFFI-based libnotmuch
Python bindings.  I've now completed these somewhat more and they now
have most of the functionality.  Here's what's new since last time:

- All tests pass on Python 3.5, 3.6 and pypy3.5

  I could probably add 3.4 as well, but I got the impression from people
  this was less important.  Likewise it seemed there was a more-or-less
  consensus that missing python 2.7 is no big deal.

- properties supported as a mapping

  This behaves as a normal mapping, with one extra method:
  getall(prefix='', *, exact=False).  Which returns an iterator rather
  then just one item.

- queries and threads

  There's now a Database.messages(query, *,
  omit_excluded=EXCLUDE.TRUE,
  sort=SORT.UNSORTED,
  exclude=tags=None)
  function.  It behaves as a generator, i.e. an iterator is returned.
  Likewise for Database.threads().  Also Database.count_messages() and
  Database.count_threads() all have the same signature (well, they
  return ints).

- Message.frozen() as a context manager

  with msg.frozen():
  msg.clear()
  raise Exception('oops')

  just works, that's less obvious to make this work then it looks but
  seemed worth it to look native in Python.

I've pushed all changes to github.com/flub/notmuch on the cffi branch.
I'd be keen to hear more feedback from other people.

Things still to do:

- Nice docs, there's comprehensive docstrings now but sphinx docs are
  clearly superior.

- A few API bits, e.g. Database.upgrade() and some others.  They're
  probably less important and I'll likely only do them as people request
  them.

- Improve finalization.  It'd be nice to give users direct access to
  .destroy() methods to free the memory, or even turn everything into
  (optional) context managers which would automatically call .destroy():

  with db.threads('tag:foo') as threads:
  for thread in threads:
  pass

  This is probably nice for long-running applications which may want to
  be somewhat careful about their memory.  But finalizers need to be
  implemented with a slightly more extended version of
  weakref.finalizer() for this to be safe.

Let me know what you think.  I'd also be happy to re-post a patch, but
it's huge and also the commits not very clean currently.  Please let me
know the best way to proceed.

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


Re: DRAFT Introduce CFFI-based Python bindings

2017-12-01 Thread Floris Bruynooghe
Florian Klink  writes:

>>> 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.
>>
>>I am generally in favour of modernizing the notmuch python bindings,
>>especially when it comes to memory management and exception handling.
>>
>>At the moment, the alot interface officially only supports python v2.7
>>but our dependencies have now mostly been updated and we are working on
>>port to python 3, see here: https://github.com/pazz/alot/pull/1055
>
> afew maintainer here ;-)
>
> I'm also very much in favor of a more modern and pythonic interface, and would
> gladly support retiring python 2, moving to the new interface.
>
> I had a quick glimpse on the code, and would like to do some annotations. I 
> fear
> it's a bit awkward to do this inside the huge patch, which might already have
> changed, and send back via email.
> Did you publish a changeset to github, or somewhere else where I could comment
> on it?

I now also pushed it to github.com/flub/notmuch in the now, it's in the
cffi branch and all code is in bindings/notmuch-cffi.  Not sure if this
helps with commenting, I can make a pull-request to somewhere if you
want - just let me know where.

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


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

2017-11-29 Thread Floris Bruynooghe
Daniel Kahn Gillmor  writes:

> 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:

I would probably model properties as a dictionary in notdb, making this
a collections.abc.MutableMapping implementation with a .get_all(prop)
method inspired from the stdlib email.message package.  This kind of
also implies making properties access a property rather then a method
call:

msg.properties['prop'] = 'foo'
msg.properties['prop'] = 'bar'
msg.properties['prop'] == 'foo'  # pot luck
msg.properties.get_all('prop') == {'foo', 'bar'}  # properties are unsorted are 
they?

This also has the binary question problem, is this returned as bytes or
as str?  Current Python bindings seem to go for .decode('utf-8',
errors='ignore') afaik which is somewhat lossy.


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


Re: DRAFT Introduce CFFI-based Python bindings

2017-11-29 Thread Floris Bruynooghe
Patrick Totzke <patricktot...@gmail.com> writes:

> Quoting David Bremner (2017-11-28 23:59:26)
>> Floris Bruynooghe <f...@devork.be> 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
>
>
> I am generally in favour of modernizing the notmuch python bindings,
> especially when it comes to memory management and exception handling.
>
> At the moment, the alot interface officially only supports python v2.7
> but our dependencies have now mostly been updated and we are working on
> port to python 3, see here: https://github.com/pazz/alot/pull/1055
>
> @Floris, you are welcome to join #alot on freenode if you want to
> discuss details on that.
>
> You mention that your new API breaks compatibility with the existing
> ones. Do you have some demo code that uses the new API for reference?

Short, untested, example which works with what's posted:

db = notdb.Database.create()
# or
db = notdb.Database(path=None, mode=notdb.Database.MODE.READ_WRITE)
print(db.path) -> pathlib.Path (a py34 dependency)
if 'unread' in db.tags:  # tags behaves like a set
print('unread mail!')
with db.atomic():
msg = db.add('/path/to/file')
mdg = db.get('/path/to/file')
msg = db.find('some-msgid')
db.remove('path/to/other/file')
# sorry, don't have a query interface yet
assert 'unread' in msg.tags
for tag in msg.tags:
print(f'a tag: {tag}')
with msg.frozen():  # Message.frozen() not yet implemented
msg.tags.clear()  # all set operations supported
msg.tags.add('atag')
msg.tags_to_flags()

I imagine the query interface would be something like:

with db.query('tag:atag') as query:
print(f'results: {query.count}')
for msg in query:
print(msg.path)

But to be honest I've been spending most time on getting the
memory-safety figured (which I hope I finally did) so far and I think
the tag handling is so far the nicest thing to show off.  They're
completely normal Python sets with no special behaviour at all (well,
that's not true - there's the binary interface, see the code posted for
this).

Actually, this last point is kind of important and I failed to mention
it before too.  The existing Python bindings convert many bytes from
libnotmuch to Python strings, that is unicode on Python 3.  For many it
uses b'bytes'.decode('utf-8', errors='ignore') which is a sane default
if you want to display things.  But if you need to round-trip a tag and
store it again you might be changing the tag.  I've not found the right
way to handle binary data (e.g. also needed for messageid) everywhere
yet but for tags I've gone with:

for tag in iter(msg.tags):  # iter() normally called implicitly by for loop
print(f'All unicode this: {tag}')
for tag in msg.tags.iter(encoding=None):
other_msg.tags.add(tag)  # This passes pure bytes around, loses nothing
# What I've used for message ID for now is a "BinString" type
print(f'All just unicode: {msg.messageid}')
binary_msgid = bytes(msg.messageid)  # No lossy conversion

This BinString stuff is somewhat hacky, not sure how sane that is.  The
second iterator on tags feels somewhat cleaner.  Likewise tags could be
BinString as well instead of plain str.

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


Re: DRAFT Introduce CFFI-based Python bindings

2017-11-29 Thread Floris Bruynooghe
David Bremner <da...@tethera.net> writes:

> Floris Bruynooghe <f...@devork.be> 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.

So on earlier Python versions, sure this is possible at not too much
cost.

- Python 3.4+ would just cost the use of some f-strings.  Not major, was
  just nice to use.
- Python <3.4 afaik would only need a tweak to the Database.tags and
  Message.tags properties.  I *think* swapping the caching of these
  using a weakref should suffice and not break the brittle
  Python-libnotmuch memory management.
  Mind you I think Python 3.0-3.3 are pretty old and not much point in
  supporting them.  But this would also apply for 2.7 support.
- Python 2.7 is probably the worst, in that keyword-only arguments would
  be gone.  If python 2.7 is required I'd be much keener to have another
  go at a drop-in replacement with the memory safety features and then
  build the "notdb" API on top off it.  But for that to be worth it
  people need to be convinced enough that maintaining a CFFI version is
  nicer than a ctypes version I guess.

Kind regards,
Floris
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Introduce CFFI-based python bindings

2017-11-28 Thread Floris Bruynooghe
From: Floris Bruynooghe <f...@google.com>

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 obj

DRAFT Introduce CFFI-based Python bindings

2017-11-28 Thread 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