From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 164E76DE010F for ; Tue, 14 Nov 2017 05:59:33 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[AWL=0.000] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BtDr9qbSjp5Z for ; Tue, 14 Nov 2017 05:59:31 -0800 (PST) Received: from che.mayfirst.org (che.mayfirst.org [162.247.75.118]) by arlo.cworth.org (Postfix) with ESMTP id A57FE6DE014D for ; Tue, 14 Nov 2017 05:59:31 -0800 (PST) Received: from fifthhorseman.net (unknown [118.200.9.16]) by che.mayfirst.org (Postfix) with ESMTPSA id 3EB5BF99D; Tue, 14 Nov 2017 08:59:30 -0500 (EST) Received: by fifthhorseman.net (Postfix, from userid 1000) id 9A46621C5E; Tue, 14 Nov 2017 21:54:59 +0800 (+08) From: Daniel Kahn Gillmor To: David Bremner , Notmuch Mail Subject: Re: [PATCH 03/18] crypto: use stashed session-key properties for decryption, if available In-Reply-To: <87r2t19fov.fsf@tethera.net> References: <20171025065203.24403-1-dkg@fifthhorseman.net> <20171025065203.24403-4-dkg@fifthhorseman.net> <87r2t19fov.fsf@tethera.net> Date: Tue, 14 Nov 2017 21:54:59 +0800 Message-ID: <87mv3px8wc.fsf@fifthhorseman.net> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Nov 2017 13:59:33 -0000 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