From: Austin Clements Date: Wed, 23 Oct 2013 19:32:09 +0000 (+2000) Subject: Re: [PATCH 1/3] cli: add insert --must-index option X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=d0e894b5b41a886f02cb663742044706da688ed5;p=notmuch-archives.git Re: [PATCH 1/3] cli: add insert --must-index option --- diff --git a/f6/c44e4eb220a6ce7ed9df2299da28a0ab63ca3a b/f6/c44e4eb220a6ce7ed9df2299da28a0ab63ca3a new file mode 100644 index 000000000..82d7ce19d --- /dev/null +++ b/f6/c44e4eb220a6ce7ed9df2299da28a0ab63ca3a @@ -0,0 +1,212 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id A7CA7431FCF + for ; Wed, 23 Oct 2013 12:32:20 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id Uvyvt3ZzfiBI for ; + Wed, 23 Oct 2013 12:32:15 -0700 (PDT) +Received: from dmz-mailsec-scanner-1.mit.edu (dmz-mailsec-scanner-1.mit.edu + [18.9.25.12]) + by olra.theworths.org (Postfix) with ESMTP id 23484431FC2 + for ; Wed, 23 Oct 2013 12:32:15 -0700 (PDT) +X-AuditID: 1209190c-b7fd38e0000009aa-c9-5268243efa3b +Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) + by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP + id F4.F3.02474.E3428625; Wed, 23 Oct 2013 15:32:14 -0400 (EDT) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id r9NJWCTZ008799; + Wed, 23 Oct 2013 15:32:13 -0400 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r9NJWAfD029830 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Wed, 23 Oct 2013 15:32:11 -0400 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1VZ4A5-0006eV-OK; Wed, 23 Oct 2013 15:32:09 -0400 +Date: Wed, 23 Oct 2013 15:32:09 -0400 +From: Austin Clements +To: Tomi Ollila +Subject: Re: [PATCH 1/3] cli: add insert --must-index option +Message-ID: <20131023193209.GF20337@mit.edu> +References: <1374365254-13227-1-git-send-email-novalazy@gmail.com> + <87ip048gbj.fsf@qmul.ac.uk> + <20130727151510.GA13750@hili.localdomain> + <87hadtxfrr.fsf@qmul.ac.uk> + <20130912001349.GA18821@hili.localdomain> + <87zjqhv264.fsf@zancas.localnet> + <87bo2xtdp2.fsf@unb.ca> + +MIME-Version: 1.0 +Content-Type: text/plain; charset=iso-8859-1 +Content-Disposition: inline +Content-Transfer-Encoding: 8bit +In-Reply-To: +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFlrOKsWRmVeSWpSXmKPExsUixCmqrWunkhFkcO+DisWN1m5Gi+s3ZzJb + vFk5j9WB2ePw14UsHs9W3WL22HLoPXMAcxSXTUpqTmZZapG+XQJXRnfXd7aCryoVn95OYG5g + PCrTxcjJISFgIvFn8VYmCFtM4sK99WxdjFwcQgL7GCXOr3zADuFsZJS48mMnM4Rzmkmi+9Zc + RghnCaPEtjnzwPpZBFQlDsx9xQ5iswloSGzbv5wRxBYRUJF40LaeFcRmFrCTOPK9CywuLGAj + 0T37JRuIzSugI/H0yBqoofeZJJbdaWOGSAhKnJz5hAWiWUdi59Y7QA0cQLa0xPJ/HBBheYnm + rbPByjkFDCTeHd8Ndo8o0N4pJ7exTWAUnoVk0iwkk2YhTJqFZNICRpZVjLIpuVW6uYmZOcWp + ybrFyYl5ealFuoZ6uZkleqkppZsYwfEhybOD8c1BpUOMAhyMSjy8Gh/Sg4RYE8uKK3MPMUpy + MCmJ8uopZwQJ8SXlp1RmJBZnxBeV5qQWH2KU4GBWEuF98geonDclsbIqtSgfJiXNwaIkznuT + wz5ISCA9sSQ1OzW1ILUIJivDwaEkwWsBMlSwKDU9tSItM6cEIc3EwQkynAdouBlIDW9xQWJu + cWY6RP4Uo6KUOO9VJaCEAEgiozQPrheWvl4xigO9IszLAdLOA0x9cN2vgAYzAQ2esiQNZHBJ + IkJKqoGxy/uv5+ojzNcuKaxx9VoxP2XersytIa330+I6k1X8Gp7sVNV+YOYdePFR2pltgv4J + N3K0f15tYeA3d1omulPPJih8ctBE6UD+rT2Cd2MXpOf+1D7iHSL7UuNHvyBX2pRfX5eskC8p + /mKQLfN/oq3rvZWnwr3lOzpsd63/VtuwfRVb/fmPjIuVWIozEg21mIuKEwEz2/mLOgMAAA== +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +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: Wed, 23 Oct 2013 19:32:20 -0000 + +Quoth Tomi Ollila on Oct 23 at 10:05 pm: +> On Thu, Oct 10 2013, David Bremner wrote: +> +> > Tomi Ollila writes: +> >>> I'm not opposed to doing an SONAME bump for 0.17. Are there other ABI +> >>> breaking changes that we have been holding back on? Can these maybe go +> >>> through at the same time? +> >> +> >> Maybe something along these lines... +> >> +> >> (Quick draft for the API part; to start discussion before working too much +> >> for something that is going to be dropped...) +> >> +> >> notmuch_status_t +> >> -notmuch_database_create (const char *path, notmuch_database_t **database); +> >> +notmuch_database_create (const char *path, +> >> + notmuch_loghook_t *loghook, +> >> + notmuch_database_t **database); +> > +> > Another idea floated (by Austin?) was to pass in an options struct, to +> > allow future options to be added without changing the function +> > signature. I guess with some care this could be done in an upwardly +> > compatible way. +> +> Maybe something like +> +> #define NOTMUCH_API_OPTIONS_VERSION 1 +> typedef struct { +> int options_version; +> void (*log)(void *, int level, int status, const char * format, ...); +> void * logdata; +> } notmuch_options_t; +> +> ... +> +> notmuch_status_t +> notmuch_database_create (const char *path, +> notmuch_options_t *options, +> notmuch_database_t **database); +> ... +> +> notmuch_status_t +> notmuch_database_open (const char *path, +> notmuch_database_mode_t mode, +> notmuch_options_t *options, +> notmuch_database_t **database); +> +> then in use: +> +> notmuch_options_t options = { +> .options_version = NOTMUCH_API_OPTIONS_VERSION, +> +> .. äsh, this has problem that the macro changes in header file +> but the structure initialization is not following automatically +> (in other fields than that). Therefore perhaps "fixing" +> the version macros: +> +> #define NOTMUCH_API_OPTIONS_VERSION_1 1 +> /* #define NOTMUCH_API_OPTIONS_VERSION_2 2 // added in the future */ +> +> notmuch_options_t options = { +> .options_version = NOTMUCH_API_OPTIONS_VERSION_1, +> .log = log_to_stderr, +> .logdata = NULL +> }; +> +> Well, this is one idea (does not sound as good as I initially +> thought...) how does other software tackle this kind of issues... +> +> If something like this is finally chosen we could provide easy +> transition path to allow NULL as options -- making notmuch CLI +> behave as it used to be (log to stderr...). + +Using a version number on the options makes it tricky to maintain +backwards compatibility, since you need code to read all past versions +of the structure. A similar but, I think, better way would be to take +the size of the structure in the structure. Something like: + +typedef struct { + size_t options_length; + ... +} notmuch_options_t; + +#define NOTMUCH_OPTIONS_INIT { sizeof(notmuch_options_t) } +// or without pre-processor, but requiring GCC attributes +static __attribute__((__unused__)) +notmuch_options_t notmuch_options_init = { sizeof(notmuch_options_t) }; + +NOTMUCH_OPTIONS_INIT/notmuch_options_init could also contain other +defaults, though it's best if the defaults are simply the zero +initializations of the various fields. + + +Then, in code calling libnotmuch, you'd do something like + +notmuch_options_t options = NOTMUCH_OPTIONS_INIT; +options.log = log_to_stderr; +// ... + + +And in libnotmuch, we would do something like + +notmuch_status_t +notmuch_database_open (const char *path, + notmuch_database_mode_t mode, + const notmuch_options_t *options, + notmuch_database_t **database) +{ + notmuch_option_t real_options = NOTMUCH_OPTIONS_INIT; + if (real_options.options_length < options.options_length) + return error; + memmove(&real_options, options, options.options_length); + // ... +} + +This approach makes it free to add fields and we can always deprecate +fields as long as we leave the space in the structure. This is based +roughly on how the Linux kernel deals with various structures that +have grown over time in the syscall ABI. + +Another much more verbose but also more robust approach would be to +make notmuch_options_t opaque and provide setters (and getters, if +we're feeling benevolent). I kind of like the simplicity of a struct. + +One thing to consider is what works best with bindings. Presumably +the opaque structure wouldn't be a problem. I've never had to wrap +anything like the struct approach, though, so I don't know if that's +easy or hard.