From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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.