From e98b2f1dc07f7aca26f488ffa3c16a2ad87981de Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sun, 24 Aug 2014 20:58:50 +2000 Subject: [PATCH] Re: [PATCH v3 04/13] lib: Database version 3: Introduce fine-grained "features" --- 99/de4b9c98ca72e03fe4c9d6924eda8940a991dd | 140 ++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 99/de4b9c98ca72e03fe4c9d6924eda8940a991dd diff --git a/99/de4b9c98ca72e03fe4c9d6924eda8940a991dd b/99/de4b9c98ca72e03fe4c9d6924eda8940a991dd new file mode 100644 index 000000000..662962aeb --- /dev/null +++ b/99/de4b9c98ca72e03fe4c9d6924eda8940a991dd @@ -0,0 +1,140 @@ +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 AB503431FBC + for ; Sat, 23 Aug 2014 17:59:00 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -2.3 +X-Spam-Level: +X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_MED=-2.3] 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 2Hr1+1xpMa5l for ; + Sat, 23 Aug 2014 17:58:53 -0700 (PDT) +Received: from dmz-mailsec-scanner-1.mit.edu (dmz-mailsec-scanner-1.mit.edu + [18.9.25.12]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 84C23431FB6 + for ; Sat, 23 Aug 2014 17:58:53 -0700 (PDT) +X-AuditID: 1209190c-f795e6d000006c66-ef-53f938cc5799 +Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) + (using TLS with cipher AES256-SHA (256/256 bits)) + (Client did not present a certificate) + by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP + id 94.84.27750.CC839F35; Sat, 23 Aug 2014 20:58:52 -0400 (EDT) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id s7O0wqZq009560; + Sat, 23 Aug 2014 20:58:52 -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 s7O0woqS027618 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Sat, 23 Aug 2014 20:58:51 -0400 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1XLM8w-0001tA-HW; Sat, 23 Aug 2014 20:58:50 -0400 +From: Austin Clements +To: David Bremner , notmuch@notmuchmail.org +Subject: Re: [PATCH v3 04/13] lib: Database version 3: Introduce fine-grained + "features" +In-Reply-To: <87ppfqsv8s.fsf@maritornes.cs.unb.ca> +References: <1406859003-11561-1-git-send-email-amdragon@mit.edu> + <1406859003-11561-5-git-send-email-amdragon@mit.edu> + <87ppfqsv8s.fsf@maritornes.cs.unb.ca> +User-Agent: Notmuch/0.18.1+76~g58c9570 (http://notmuchmail.org) Emacs/23.4.1 + (i486-pc-linux-gnu) +Date: Sat, 23 Aug 2014 20:58:50 -0400 +Message-ID: <87fvgmg0tx.fsf@awakening.csail.mit.edu> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFnrAIsWRmVeSWpSXmKPExsUixG6nonvG4mewwdJflhY3WrsZLa7fnMns + wOTxbNUtZo8th94zBzBFcdmkpOZklqUW6dslcGXMnfybueC3cMX0Ca8ZGxif83cxcnJICJhI + LPv7gQXCFpO4cG89WxcjF4eQwGwmiU0r1rBAOBsZJZbfvscOUiUkcJpJ4sQKLojEEkaJ3lMN + YAk2AQ2J37cWM4HYIgJ2Ej0XzoPFhQUiJOZ83wO2glPASOLOsmmsEINmM0q07UoFsUUFEiRO + 9h9g7GLk4GARUJXo/c4FEuYFuu7t0jZ2CFtQ4uTMJ2BjmAW0JG78e8k0gVFgFpLULCSpBYxM + qxhlU3KrdHMTM3OKU5N1i5MT8/JSi3QN9XIzS/RSU0o3MYICklOSZwfjm4NKhxgFOBiVeHg/ + XPwRLMSaWFZcmXuIUZKDSUmU95XGz2AhvqT8lMqMxOKM+KLSnNTiQ4wSHMxKIrzFZkA53pTE + yqrUonyYlDQHi5I471trq2AhgfTEktTs1NSC1CKYrAwHh5IE70xzoEbBotT01Iq0zJwShDQT + ByfIcB6g4Y9BaniLCxJzizPTIfKnGBWlxHk3giQEQBIZpXlwvbCE8YpRHOgVYd6VIFU8wGQD + 1/0KaDAT0ODpM76CDC5JREhJNTDanjqs9WKrRoZorE7+kmNHBfintS9/HbN3Ildo5raZlzfI + PmJT75z0fhOz8QXu9PJ3N2oVF5bNOrCoW0zs0uTpKz/sFZKZ0N/0T9Hi2/pXy1NyjB7/W7Ri + /vfpC9bcrPr46+aXP5win9bsZTWPTLvy6tER6Zt3XzfcWVraFSF48cuxTde/nPc+ZaHEUpyR + aKjFXFScCAB2WsD38wIAAA== +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: Sun, 24 Aug 2014 00:59:00 -0000 + +On Sat, 23 Aug 2014, David Bremner wrote: +> Austin Clements writes: +>> +>> + /* Bit mask of features used by this database. Features are +>> + * named, independent aspects of the database schema. This is a +>> + * bitwise-OR of NOTMUCH_FEATURE_* values (below). */ +>> + unsigned int features; +> +> Should we be using a fixed size integer (uint_32t or whatever) for +> features? iirc the metadata in the database is actually a string, so I +> guess arbitrary precision there. + +Right; this doesn't matter for the on-disk format because these don't +appear on disk. But you're right that in principle we could overflow +this, leading to subtle bugs. I moved the enum above struct +_notmuch_database, gave it a name and bitwise operators for C++, and +used that enum name everywhere, so precision should never be a problem. + +>> +/* Bit masks for _notmuch_database::features. */ +>> +enum { +>> + /* If set, file names are stored in "file-direntry" terms. If +>> + * unset, file names are stored in document data. +>> + * +>> + * Introduced: version 1. Implementation support: both for read; +>> + * required for write. */ +>> + NOTMUCH_FEATURE_FILE_TERMS = 1 << 0, +> +> I agree with Jani that the Implementation support: part is a bit +> mystifying without the commit message. Maybe part of the commit message +> could migrate here? Or maybe just add a pointer to the comment in database.cc. + +I stripped these out because I don't think they're maintainable. See my +reply to Jani. + +>> + if (! *incompat_out) +> +> Should we support passing NULL for incompat_out? or at least check for +> it? + +Added a guard so it's safe to pass NULL. + +>> @@ -1048,7 +1164,8 @@ notmuch_database_get_version (notmuch_database_t *notmuch) +>> notmuch_bool_t +>> notmuch_database_needs_upgrade (notmuch_database_t *notmuch) +>> { +>> - return notmuch->needs_upgrade; +>> + return notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE && +>> + (NOTMUCH_FEATURES_CURRENT & ~notmuch->features); +>> } +> +> Maybe I'm not thinking hard enough here, but how does this deal with a +> feature that is needed to open a database in read only mode? Maybe it +> needs a comment for people not as clever as Austin ;). + +I'm not quite sure what you mean. notmuch_database_needs_upgrade +returns false for read-only databases because you can't upgrade a +read-only database. This was true before this patch, too, though it was +less obvious. (Maybe that's not what you're asking?) -- 2.26.2