[Lars Luthman] Bug#826881: Calls to notmuch_directory_get_mtime() don't return the recently set mtime

[Lars Luthman] Bug#826881: Calls to notmuch_directory_get_mtime() don't return the recently set mtime

[David Bremner]

[-- Attachment #0: Type: message/rfc822, Size: 7491 bytes --]

[-- Attachment #1.1: Type: text/plain, Size: 1306 bytes --]

Package: libnotmuch-dev
Version: 0.18.2-1
Severity: normal
Tags: patch

notmuch_directory_set_mtime() sets the new mtime in the database, but
doesn't update the cached value in the notmuch_directory_t object, which
is what notmuch_directory_get_mtime() returns. This means that this
sequence of calls:

  notmuch_directory_set_mtime(dir, 12345);
  assert(notmuch_directory_get_mtime(dir) == 12345);

will fail, which can be very confusing - I was writing unit tests for a
library wrapper and it took a while to figure out what was going on.

Destroying the directory object and creating a new one will cause
notmuch_directory_get_mtime() to return the correct value.

If this is the intended behaviour it should probably be documented. If
it's not, the attached patch should fix it. 

-- System Information:
Debian Release: 8.5
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 3.16.0-4-686-pae (SMP w/2 CPU cores)
Locale: LANG=en_GB.utf8, LC_CTYPE=en_GB.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages libnotmuch-dev depends on:
ii  libnotmuch3  0.18.2-1

libnotmuch-dev recommends no packages.

libnotmuch-dev suggests no packages.

-- no debconf information

[-- Attachment #1.2: set_cached_mtime.diff --]
[-- Type: text/x-patch, Size: 410 bytes --]

diff -ru notmuch-0.18.2-old/lib/directory.cc notmuch-0.18.2/lib/directory.cc
--- notmuch-0.18.2-old/lib/directory.cc	2016-06-09 19:45:49.870660808 +0200
+++ notmuch-0.18.2/lib/directory.cc	2016-06-09 19:46:43.670658075 +0200
@@ -230,6 +230,7 @@
 	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 
+    directory->mtime = mtime;
     return NOTMUCH_STATUS_SUCCESS;
 }
 
Only in notmuch-0.18.2/lib: directory.cc~

[PATCH] lib: update cached mtime in notmuch_directory_set_mtime

[David Bremner]

Without this change, the following code fails

  notmuch_directory_set_mtime(dir, 12345);
  assert(notmuch_directory_get_mtime(dir) == 12345);
---
 lib/directory.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/directory.cc b/lib/directory.cc
index a19f777..5de3319 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -227,6 +227,9 @@ notmuch_directory_set_mtime (notmuch_directory_t *directory,
 				   Xapian::sortable_serialise (mtime));
 
 	db->replace_document (directory->document_id, directory->doc);
+
+	directory->mtime = mtime;
+
     } catch (const Xapian::Error &error) {
 	_notmuch_database_log (notmuch,
 		 "A Xapian exception occurred setting directory mtime: %s.\n",
-- 
2.8.1

Re: [PATCH] lib: update cached mtime in notmuch_directory_set_mtime

[David Bremner]

David Bremner <david@tethera.net> writes:

> Without this change, the following code fails
>
>   notmuch_directory_set_mtime(dir, 12345);
>   assert(notmuch_directory_get_mtime(dir) == 12345);

pushed.

Hopefully nobody relied on the old, weird, behaviour.

d