Re: [PATCH 4/5] tests: run python-cffi tests

2019-11-21 Thread David Bremner
Tomi Ollila  writes:

> On Fri, Nov 08 2019, David Bremner wrote:
>
>> Tomi Ollila  writes:
>>
>>>
>>> Right -- just that pytest-3 may not be available -- to iterate (tested)...
>>>
>>> if ${NOTMUCH_PYTHON} -m pytest -c $conf --version >/dev/null 2>&1; then
>>>
>>
>> The problem with this is that it might pass if pytest is only installed
>> for python2. 
>
> What is the minimum python3 version supported -- In the previous email thread 
> all
> references show python 3.7
>
> Anyway, in that wip/cffi "branch" commit 97caf16c15d (check for python cffi 
> module)
>
> could also include check for minimum python version, e.g.
>
> +printf "Checking for python 3 cffi... "
> +if "${NOTMUCH_PYTHON}" -c 'import sys,cffi; assert sys.version_info >= 
> (3,7)' 2>/dev/null; then
> +printf "Yes.\n"
> +have_python3_cffi=1
> +else
> +printf "No.\n"
> +have_python3_cffi=0
> +fi

I (force) pushed an updated version of wip/cffi (8d1f30cb) with several changes
along those lines to the configure / build / test scaffolding. In
particular running the tests from the notmuch suite doesn't require
pytest-cov anymore, and pytest and cffi are both checked for only if
python3 is found.

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


moving the config into the database [was: Re: [PATCH] Display extra headers for emacs-mua - db config option]

2019-11-21 Thread Daniel Kahn Gillmor
On Thu 2019-11-21 23:38:06 +0200, Tomi Ollila wrote:
> How can library open the database if it doesn't read the config file
> -- the config file defines where database is located =D

The *only* thing i think worth keeping in the config file is ultimately
the location of the database, though i would eventually prefer that we
introduce a NOTMUCH_DATABASE env var, and use it if we can't find a
config.

This is the moral equivalent of the NOTMUCH_CONFIG env var, as far as
i'm concerned, but more rationalized.

and *eventually eventually* drop the config file entirely, yes.

> If the library can somehow guess the location of the database without
> reading the config file ;D I think dumping -- editing -- restoring
> the database is tolerable solution -- provided it is clearly documented
> for people like me who wants to edit configuration files w/ text
> editor(*),

Better than documenting, i'd be happy if we would add a "notmuch config
edit" subcommand, which handles the above sequence for you, invoking
$EDITOR at the appropriate time.

The only caveat i see there is if the end user wants to inject comments
in the config file, which would then be stripped out in between these
invocations.  perhaps someone who finds these comments in config files
super important could propose a way to stash them in the db and recover
them during "notmuch config edit" as well :)

> (+) if we dropped the configuration file, then notmuch, and library could
> open database from ~/mail/notmuch/ by default, or from location pointed
> by e.g. NOTMUCH_DATABASE_DIR -- as an additional benefit(?) notmuch
> would pollute user's $HOME by one file less -- the `.notmuch-config`.

You named the env var differently than i did, but i'd be happy to go
with your name, since it seems we arrived at the same design
independently :)

 --dkg


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


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread Daniel Kahn Gillmor
On Thu 2019-11-21 22:56:04 +0100, Johan Parin wrote:
> Here is a taste (not fully tested, but seems to work).

Oof, i see what you mean :(

I haven't tried to implement this a different way, so i don't know
whether there isn't a shorter cut to what we want, but yow it is a lot.

Are we doing something more deeply, architecturally wrong that saddles
us these two rather displeasing choices?

> diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
> index 28487079..0eb59883 100644
> --- a/doc/man1/notmuch-config.rst
> +++ b/doc/man1/notmuch-config.rst
> @@ -204,6 +204,12 @@ The available configuration items are described below.
>  supported. See **notmuch-search-terms(7)** for a list of existing
>  prefixes, and an explanation of probabilistic prefixes.
>  
> +**show.extra_headers**
> +A list of extra headers that will be output by **notmuch show**
> +with ``--format=sexp``, if present in the message.
> +
> +Default: empty list.
> +
>  **built_with.**
>  Compile time feature . Current possibilities include
>  "compact" (see **notmuch-compact(1)**) and "field_processor" (see

Given the implementation choices, i think you want **[STORED IN
DATABASE]** here (see the definitions for query. and
index.header. and index.decrypt).

Thanks for taking the time to sort this out!

   --dkg


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


Re: Debian packaging cleanup

2019-11-21 Thread Daniel Kahn Gillmor
On Thu 2019-11-21 08:08:53 -0400, David Bremner wrote:
> Daniel Kahn Gillmor  writes:
>> Anarcat has reviewed these, but they're still tagged
>> notmuch::needs-review in nmbug.  should i clear these tags now that
>> they've been reviewed by someone who knows something about debian
>> packaging?
>
> Go for it.

done. :)

Thanks, Bremner and Anarcat.

  --dkg


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


Re: Tests failing on master

2019-11-21 Thread Tomi Ollila
On Sun, Nov 17 2019, David Bremner wrote:

> Yes, being root would break some of the tests, e.g. those that rely on
> removing permissions to generate an error. Probably we should just
> document that running the test suite as root is not supported? If
> someone can explain a good reason to run as root (maybe some odd
> auto-builder, the tests in question could probably be modified to do
> something more extreme (e.g. delete a file) rather than chmod.

Yes, rootless podman containers!

if I run e.g

  $ podman run --rm -it -v $HOME:$HOME -w $HOME/vc/ext/notmuch \
 notmuch-buildenv-debian10: /bin/zsh

  [root@e489c80816a9 notmuch]# id
  uid=0(root) gid=0(root) groups=0(root)

  [root@e489c80816a9 notmuch]# pwd
  /home/too/vc/ext/notmuch

  [root@e489c80816a9 build]# ls -l configure 
  -rwxr-xr-x. 1 root root 41920 Nov 17 21:00 configure

  [root@e489c80816a9 build]# exit

  $ ls -l $HOME/vc/ext/notmuch/configure
  -rwxr-xr-x. 1 too too 41920 Nov 17 23:00 /home/too/vc/ext/notmuch/configure

  $ podman unshare cat /proc/self/uid_map
  0   1000  1
  1 10  65536


The last line, would have had same results if run inside container, tells
that uid 0 (root) inside container was mapped to uid 1000 (too) outside, 
on "host" side, and 1 uid was mapped, from uid 1 onwards in container,
uids are mapped starting from 10 outside.

All that means, that I can do, without sudo, all kinds of things when 
using this container technology. Files owned by my uid outside container
are seen as owned by root inside container.

Since last spring, I've been gradually moving all things I used docker
to podman. And everything has (in my use cases) worked better. In Fedora
it just works (already). In Ubuntu 16.04 and RHEL 7.7 it also worked when
I tried, but lack of fuse-overlayfs makes it slow (to start) in these
systems. I hope every system in 2020 has good podman support...


All that long text written, I can take care notmuch tests can be run on
podman containers... 

>
> cheers,
>
> d

YEA,

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


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread Johan Parin

Johan Parin  writes:

> Daniel Kahn Gillmor  writes:
>>
>> Is it that much worse to pass around the notmuch_database_t *?
>>
>
> It has a lot of code impact, it really propagates to a lot of
> places. For instance it also impacts the json and text api, because
> those need to have the same signatures as the json api. Also the
> database needs to be opened in places where it's currently not opened,
> such as in notmuch_search_command().
>

Here is a taste (not fully tested, but seems to work).

/Johan

>From 706a0f784a101b05ac82a462ff6c19c0cef550af Mon Sep 17 00:00:00 2001
From: Johan Parin 
Date: Sun, 17 Nov 2019 13:15:39 +0100
Subject: [PATCH] Display extra headers for emacs-mua - db config option

Modify format_headers_sprinter so that it returns some additional headers in the
sexp, instead of a small fixed set of headers.

The extra header list is configured by the database config option
`show.extra_headers'.

This is required in order for the elisp variable
`notmuch-message-headers' to work.

See this bug report:

  https://notmuchmail.org/pipermail/notmuch/2017/026069.html

This version avoids the file global notmuch database variable in
notmuch-show.c by passing around the database pointer in a lot of
function calls.
---
 doc/man1/notmuch-config.rst |   6 ++
 notmuch-client.h|  12 ++--
 notmuch-config.c|   7 ++-
 notmuch-reply.c |  13 ++--
 notmuch-search.c|  29 ++---
 notmuch-show.c  | 114 
 sprinter-json.c |   3 +-
 sprinter-sexp.c |   3 +-
 sprinter-text.c |  10 +++-
 sprinter.h  |  12 ++--
 10 files changed, 155 insertions(+), 54 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 28487079..0eb59883 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -204,6 +204,12 @@ The available configuration items are described below.
 supported. See **notmuch-search-terms(7)** for a list of existing
 prefixes, and an explanation of probabilistic prefixes.
 
+**show.extra_headers**
+A list of extra headers that will be output by **notmuch show**
+with ``--format=sexp``, if present in the message.
+
+Default: empty list.
+
 **built_with.**
 Compile time feature . Current possibilities include
 "compact" (see **notmuch-compact(1)**) and "field_processor" (see
diff --git a/notmuch-client.h b/notmuch-client.h
index 74690054..d4402231 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -64,8 +64,10 @@ struct sprinter;
 struct notmuch_show_params;
 
 typedef struct notmuch_show_format {
-struct sprinter *(*new_sprinter)(const void *ctx, FILE *stream);
-notmuch_status_t (*part)(const void *ctx, struct sprinter *sprinter,
+struct sprinter *(*new_sprinter)(notmuch_database_t *notmuch,
+ const void *ctx, FILE *stream);
+notmuch_status_t (*part)(notmuch_database_t *notmuch,
+			 const void *ctx, struct sprinter *sprinter,
 			 struct mime_node *node, int indent,
 			 const struct notmuch_show_params *params);
 } notmuch_show_format_t;
@@ -227,12 +229,14 @@ notmuch_status_t
 show_one_part (const char *filename, int part);
 
 void
-format_part_sprinter (const void *ctx, struct sprinter *sp, mime_node_t *node,
+format_part_sprinter (notmuch_database_t *notmuch,
+		  const void *ctx, struct sprinter *sp, mime_node_t *node,
 		  bool output_body,
 		  bool include_html);
 
 void
-format_headers_sprinter (struct sprinter *sp, GMimeMessage *message,
+format_headers_sprinter (notmuch_database_t *notmuch,
+			 struct sprinter *sp, GMimeMessage *message,
 			 bool reply, const _notmuch_message_crypto_t *msg_crypto);
 
 typedef enum {
diff --git a/notmuch-config.c b/notmuch-config.c
index 1b079e85..6554ad9b 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -841,9 +841,10 @@ typedef struct config_key {
 
 static struct config_key
 config_key_table[] = {
-{ "index.decrypt",   true,   false,  NULL },
-{ "index.header.",   true,   true,   validate_field_name },
-{ "query.",  true,   true,   NULL },
+{ "index.decrypt",  true,   false,  NULL },
+{ "index.header.",  true,   true,   validate_field_name },
+{ "query.", true,   true,   NULL },
+{ "show.extra_headers", true,   false,  NULL }
 };
 
 static config_key_info_t *
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2c30f6f9..5d468c88 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -614,7 +614,8 @@ enum {
 };
 
 static int
-do_reply (notmuch_config_t *config,
+do_reply (notmuch_database_t *notmuch,
+	  notmuch_config_t *config,
 	  notmuch_query_t *query,
 	  notmuch_show_params_t *params,
 	  int format,
@@ -640,9 +641,9 @@ do_reply (notmuch_config_t *config,
 	}
 
 	if (format == FORMAT_JSON)
-	sp = sprinter_json_create (config, stdout);
+	sp = sprinter_json_create (notmuch, config, stdout);
 	else
-	sp 

Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

2019-11-21 Thread Sean Whitton
Dear maintainers,

The function `org-notmuch-follow-link' in {org,ol}-notmuch.el calls
`notmuch-show' with an arbitrary notmuch search query.  However, the
docstring for `notmuch-show' specifies that a notmuch thread ID, rather
than an arbitrary notmuch query, should be supplied to `notmuch-show'.

The effect of this is that the variable `notmuch-show-thread-id' may
contain an arbitrary search query rather than a thread ID.  That broke
some code of mine which uses that variable.

`org-notmuch-follow-link' needs to continue to accept an arbitrary query
(as notmuch thread IDs are not stable), but it should convert it to a
thread ID before passing it to `notmuch-show'.

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


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread Tomi Ollila
On Fri, Nov 22 2019, Daniel Kahn Gillmor wrote:

> On Thu 2019-11-21 08:27:04 -0400, David Bremner wrote:
>> Apologies for being late to the discussion of where to store the
>> configuration. So far we have only stored configuration in the database
>> where it affected the behaviour of the library API.
>
> While i'm being ambitious, i'd like also to eventually consider moving
> more of the existing CLI functionality into being accessible from the
> library.  I think this would help downstream MUAs that use notmuch who
> currently can't (or don't want to, for whatever reason) take advantage
> of the existing CLI, but can use the library.
>
> If we stuff more config in the config file, then the behavior of any
> future move to the library will have to grapple with the config move
> (currently the library never reads the config file, it just opens the
> database).

How can library open the database if it doesn't read the config file
-- the config file defines where database is located =D

>> I know some people (e.g. dkg) have suggested it would be better to
>> store all of the configuration in the database for consistency, while
>> others are disgruntled that some of the configuration is not editable
>> with text editor.
>
> It would still editable with a text editor -- you just need to edit the
> output of "notmuch dump --include config" and feed the result back into
> "notmuch restore" :)

If the library can somehow guess the location of the database without
reading the config file ;D I think dumping -- editing -- restoring
the database is tolerable solution -- provided it is clearly documented
for people like me who wants to edit configuration files w/ text editor(*),
for forgets these instructions easily. If, in the future, we still have
configuration file(+), notmuch could also write such instructions to
the config file.

>
>  --dkg

Tomi

(*) like someone may know I'm not too fond of editing configuration files
when there are alternatives available (like in this case) -- however
is someone else(tm) takes the time to create this solution and it uses
configuration files, I'll use it...

(+) if we dropped the configuration file, then notmuch, and library could
open database from ~/mail/notmuch/ by default, or from location pointed
by e.g. NOTMUCH_DATABASE_DIR -- as an additional benefit(?) notmuch
would pollute user's $HOME by one file less -- the `.notmuch-config`.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread Daniel Kahn Gillmor
On Thu 2019-11-21 08:27:04 -0400, David Bremner wrote:
> Apologies for being late to the discussion of where to store the
> configuration. So far we have only stored configuration in the database
> where it affected the behaviour of the library API.

While i'm being ambitious, i'd like also to eventually consider moving
more of the existing CLI functionality into being accessible from the
library.  I think this would help downstream MUAs that use notmuch who
currently can't (or don't want to, for whatever reason) take advantage
of the existing CLI, but can use the library.

If we stuff more config in the config file, then the behavior of any
future move to the library will have to grapple with the config move
(currently the library never reads the config file, it just opens the
database).

> I know some people (e.g. dkg) have suggested it would be better to
> store all of the configuration in the database for consistency, while
> others are disgruntled that some of the configuration is not editable
> with text editor.

It would still editable with a text editor -- you just need to edit the
output of "notmuch dump --include config" and feed the result back into
"notmuch restore" :)

 --dkg


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


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread Johan Parin


Daniel Kahn Gillmor  writes:

>
> I'm a little weirded out by the move to a static notmuch_database_t
> *notmuch object.  Are we doing this because we don't want to pass
> around the database to internal functions?

Yes

> I know that the scope of nomtuch-show.c is basically "global scope",
> but i worry that it makes the code more difficult to read and
> maintain.
>

I agree it's not so nice with globals in general.

>
> Is it that much worse to pass around the notmuch_database_t *?
>

It has a lot of code impact, it really propagates to a lot of
places. For instance it also impacts the json and text api, because
those need to have the same signatures as the json api. Also the
database needs to be opened in places where it's currently not opened,
such as in notmuch_search_command().

I have started on such a patch and can certainly complete that if this
is the direction we want to move. I can warn you that there is a lot of
code changes (although they are cosmetic).


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


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread David Bremner
Johan Parin  writes:

> +
> +if (notmuch_database_get_config (notmuch, "show.extra_headers",
> +  _headers) != NOTMUCH_STATUS_SUCCESS)
> + return;

Apologies for being late to the discussion of where to store the
configuration. So far we have only stored configuration in the database
where it affected the behaviour of the library API. I know some people
(e.g. dkg) have suggested it would be better to store all of the
configuration in the database for consistency, while others are
disgruntled that some of the configuration is not editable with text
editor. The approach here would represent a change, which I'm not
necessarily against, but I think we need to think it through.

> +
> +header_list  = g_mime_object_get_header_list (GMIME_OBJECT(message));
> +
> +tofree = extra_headers;

It's somewhat a matter of taste, but I would rather introduce a variable
for the pointer to be mutated by strsep, and call  free (extra_headers) below
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Display extra headers for emacs-mua - db config option

2019-11-21 Thread David Bremner
Daniel Kahn Gillmor  writes:

>
> I'm a little weirded out by the move to a static notmuch_database_t
> *notmuch object.  Are we doing this because we don't want to pass around
> the database to internal functions?  I know that the scope of
> nomtuch-show.c is basically "global scope", but i worry that it makes
> the code more difficult to read and maintain.

I had a similar reaction.

>
> It's also not a common idiom in the rest of the codebase (at least not
> one that i've seen).
>
> Is it that much worse to pass around the notmuch_database_t *?

There are some call chains 3 or 4 deep that would need to be modified. I
_think_ that there is always a notmuch_config_t available, so one option
would be to stash the headers or the database there. We do have the
convention other places (mainly in  lib/) of objects having a pointer to
their "parent" database.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: Debian packaging cleanup

2019-11-21 Thread David Bremner
Daniel Kahn Gillmor  writes:

> On Tue 2019-11-12 10:47:33 -0500, Antoine Beaupré wrote:
>> On 2019-11-10 12:37:42, Daniel Kahn Gillmor wrote:
>>> This series offers a set of simple and small changes to the debian
>>> packaging for notmuch.  they apply to the master branch.
>>
>> Looks good to me.
>
> Anarcat has reviewed these, but they're still tagged
> notmuch::needs-review in nmbug.  should i clear these tags now that
> they've been reviewed by someone who knows something about debian
> packaging?
>

Go for it.

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