Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available

[Daniel Kahn Gillmor <dkg@fifthhorseman.net>]

Hi Bremner--

Thanks for the review!

On Tue 2017-11-14 09:02:08 -0400, David Bremner wrote:

> Since you wrote this, I've deprecated GMime 2.6. I'm not sure that
> changes anything here, but seems worth mentioning

well, i'm happy to hear that -- i've got no problem with deprecating
GMime 2.6, and would be fine with maintaining GMime 3.0 in
stretch-backports if that would make you feel more comfortable about the
decision.

Still, I'll be kind of bummed to have to rewrite this series to strip
out the 2.6 support: i originally wrote it only with 3.0 support, and
then went back in and added 2.6 support because at the time, you didn't
want to deprecate 2.6 :( our coding cadence isn't very well synced :/

> Its a nitpick, but I don't really understand/like including = with the
> property name.  That will break, e.g. for anyone attempting to use it
> from the API.

I don't mind changing the documentation to use ``session-key`` instead
of ``session-key=``.  *shrug*

> The way the diff works out, I was pretty confused by seeing several
> "wrong" calls to _notmuch_crypto_decrypt before the actual change. It
> would be nice to telegraph that somehow, perhaps in the commit message.

sure, i can add to the commit message that _notmuch_crypto_decrypt is
growing a new parameter.

> Personally I would be fine with (and probably happier) only supporting
> new features when using gmime-3.0.  Debugging crypto related stuff is
> always hard (see recent discussion about _mime_node_create, where we
> still don't know what's wrong, and are just papering over the problem),
> and it seems worth striving for simplicity as much as possible.  I also
> don't know how motivated gmime upstream is to fix bugs in 2.6; I could
> certainly understand if the answer was "not very".

I believe the answer is "not very" -- but if there are serious bugs (i
don't think we've talked about any of this stuff as bugs in gmime) then
we should probably try to raise them with him.

> There is, by the way, a function notmuch_built_with that can be used to
> introspect the library as to what optional features it is built
> with. It's used in notmuch_config to report back to the user about the
> presence of optional features.

Is there any naming convention for these features?  do you want me to
add a "session-key" label with a future revision of this branch?  or are
you asking for something else?

    --dkg