[PATCH 1/1] build: remove trailing '/.' when doing mkdir -p .deps/.
On Sun, Nov 03 2013, Jed Brown wrote: > Tomi Ollila writes: > >> %.o: %.cc $(global_deps) >> -@mkdir -p .deps/$(@D) >> +@mkdir -p $(patsubst %/.,%,.deps/$(@D)) >> $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ >> -MD -MP -MF .deps/$*.d > > An alternative approach is to use directory marker files [1] to clean up > the recipes that need output directories and to satisfy Paul's second > rule of makefiles [2]. > > .SECONDEXPANSION: > > %.o: %.cc $(global_deps) | .deps/$$(@D)/.DIR > $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ > -MD -MP -MF .deps/$*.d > > %/.DIR: > @mkdir -p $(patsubst %/.,%,$(@D)) > @touch $@ > > .PRECIOUS: %.DIR Hmm, nice suggestion... the diff to be reviewed is just soo much bigger ;/ Now that I learned new things [11] yet another alternative is: diff --git a/Makefile.local b/Makefile.local index 72524eb..cc1a0cb 100644 --- a/Makefile.local +++ b/Makefile.local @@ -235,12 +235,15 @@ endif # Otherwise, print the full command line. quiet ?= $($(shell echo $1 | sed -e s'/ .*//')) -%.o: %.cc $(global_deps) - @mkdir -p .deps/$(@D) +depdirs = $(subdirs:%=.deps/%) + +$(depdirs): + @mkdir -p $(depdirs) + +%.o: %.cc $(global_deps) | $(depdirs) $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d -%.o: %.c $(global_deps) - @mkdir -p .deps/$(@D) +%.o: %.c $(global_deps) | $(depdirs) $(call quiet,CC $(CPPFLAGS) $(CFLAGS)) -c $(FINAL_CFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d .PHONY : clean still, for the time being I'd still use the patch I originally proposed due to the triviality I change... [11] http://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html Tomi > [1] http://www.cmcrossroads.com/article/making-directories-gnu-make > [2] http://make.paulandlesley.org/rules.html
Getting the right root mail of the thread
On Sun, 03 Nov 2013, Jesse Rosenthal wrote: > The current behavior can be annoying, but the old behavior could make > the MUA quite unusable in a number of circumstances. (And yes, an MUA > that fails on reading mail from senders with bad emailing practices is > unusable for me.) I wouldn't have suggested changing the current default behaviour anyway, but thanks for reminding us there are users depending on it! > Maybe there should be a "show original subject" toggle? That wouldn't be > too hard, though it would require another call to the library and > regenerating the search results. IMO the best approach would be including the first (or oldest, or original) subject in the structured thread summary output, in addition to the first matching subject that we have now. BR, Jani.
Getting the right root mail of the thread
On Sun, 03 Nov 2013, Felipe Contreras wrote:
> On Sat, Nov 2, 2013 at 8:58 AM, Jani Nikula wrote:
>> On Sat, 02 Nov 2013, Felipe Contreras wrote:
>>> On Sat, Nov 2, 2013 at 7:50 AM, Jani Nikula wrote:
On Sat, 02 Nov 2013, Felipe Contreras
wrote:
>>>
> I think there should be a way to get the root mail of a thread,
> irrespective of the search order.
Largely agreed. It's just that nobody's gotten around to doing this
yet. At the cli level I think the consensus is that the structured
(sexp/json) output format should contain multiple (or all) subjects.
>>>
>>> What about the default? (--format=text). What about user-interfaces
>>> that must display a summary of a thread?
>>
>> To be honest, we haven't had much interest in adding new content or
>> features to the text output formats. It's just much easier to do this in
>> a backwards compatible way in the structured output formats, for which
>> we also have the format versioning. The emacs ui uses the sexp format
>> for the thread summaries.
>
> How is this any better?
>
> (:thread "826a" :timestamp 1383396519 :date_relative
> "Yest. 06:48" :matched 45 :total 45 :authors "Felipe Contreras"
> :subject "[PATCH/TEST 44/44] request-pull: rewrite to C" :tags ("sent"
> "to-me"))
I made no claims it would contain more or better information *now*. But
if someone were to add new things, the structured output is where it
would likely be added. We could add :subject_first, :subject_oldest, or
whatever *in addition* to :subject.
BR,
Jani.
Re: [PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag'
On Tue, Oct 22 2013, Austin Clements wrote: > We no longer use this, since we've lifted all interactive behavior to > the appropriate interactive entry points. Because of this, > `notmuch-tag' also no longer needs to return the tag changes list, > since the caller always passes it in. > --- > emacs/notmuch-tag.el | 19 --- > 1 file changed, 4 insertions(+), 15 deletions(-) > > diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el > index f9c1740..feee17c 100644 > --- a/emacs/notmuch-tag.el > +++ b/emacs/notmuch-tag.el > @@ -249,27 +249,18 @@ from TAGS if present." > (error "Changed tag must be of the form `+this_tag' or > `-that_tag'") > (sort result-tags 'string<))) > > -(defun notmuch-tag (query &optional tag-changes) > +(defun notmuch-tag (query tag-changes) >"Add/remove tags in TAG-CHANGES to messages matching QUERY. > > QUERY should be a string containing the search-terms. > -TAG-CHANGES can take multiple forms. If TAG-CHANGES is a list of > -strings of the form \"+tag\" or \"-tag\" then those are the tag > -changes applied. If TAG-CHANGES is a string then it is > -interpreted as a single tag change. If TAG-CHANGES is the string > -\"-\" or \"+\", or null, then the user is prompted to enter the > -tag changes. > +TAG-CHANGES is a list of strings of the form \"+tag\" or > +\"-tag\" to add or remove tags, respectively. > > Note: Other code should always use this function alter tags of > messages instead of running (notmuch-call-notmuch-process \"tag\" ..) > directly, so that hooks specified in notmuch-before-tag-hook and > notmuch-after-tag-hook will be run." >;; Perform some validation > - (if (string-or-null-p tag-changes) > - (if (or (string= tag-changes "-") (string= tag-changes "+") (null > tag-changes)) > - (setq tag-changes (notmuch-read-tag-changes > - (notmuch-tag-completions query) nil tag-changes)) > - (setq tag-changes (list tag-changes Hey folks. After a recent upgrade to 0.16+120~gfd733a4+1 I found that my custom tagging functions weren't working, which I traced back to the removal of this section. Previously notmuch-tag accepted tag changes in two forms: as a list of tag changes: (list "+foo" "-bar") or as a space-separated string: "+foo -bar" This removed section is what would turn the space-separated string into a list. I have custom tagging functions that were written like this: (notmuch-search-tag "+spam") notmuch-search-tag passes the TAG-CHANGES to notmuch-tag, which was changing it to a list in this removed section. Since its removal, all of my custom functions started throwing the following error: Wrong type argument: stringp, 43 I didn't pay super close attention to the rest of the patch series, but From a backwards compatibility point of view, do we really need to remove this section? Is there a problem with specifying tag changes as a string? I think it was actually me that last modified notmuch-tag, and I vaguely remember spending time thinking about how to preserve that particular way to specify the changes, so that things would be simpler for simple tag operations. In any event, some amount of backwards compatibility has been removed, so if we do want to keep it removed, we should add a good NEWS item to explain that peoples custom bindings might break. jamie. >(mapc (lambda (tag-change) > (unless (string-match-p "^[-+]\\S-+$" tag-change) > (error "Tag must be of the form `+this_tag' or `-that_tag'"))) > @@ -278,9 +269,7 @@ notmuch-after-tag-hook will be run." > (run-hooks 'notmuch-before-tag-hook) > (apply 'notmuch-call-notmuch-process "tag" > (append tag-changes (list "--" query))) > -(run-hooks 'notmuch-after-tag-hook)) > - ;; in all cases we return tag-changes as a list > - tag-changes) > +(run-hooks 'notmuch-after-tag-hook))) > > (defun notmuch-tag-change-list (tags &optional reverse) >"Convert TAGS into a list of tag changes. > -- > 1.8.4.rc3 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch pgpULmMmtjhHP.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 8/8] emacs: Remove interactive behavior of `notmuch-tag'
On Tue, Oct 22 2013, Austin Clements wrote: > We no longer use this, since we've lifted all interactive behavior to > the appropriate interactive entry points. Because of this, > `notmuch-tag' also no longer needs to return the tag changes list, > since the caller always passes it in. > --- > emacs/notmuch-tag.el | 19 --- > 1 file changed, 4 insertions(+), 15 deletions(-) > > diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el > index f9c1740..feee17c 100644 > --- a/emacs/notmuch-tag.el > +++ b/emacs/notmuch-tag.el > @@ -249,27 +249,18 @@ from TAGS if present." > (error "Changed tag must be of the form `+this_tag' or > `-that_tag'") > (sort result-tags 'string<))) > > -(defun notmuch-tag (query &optional tag-changes) > +(defun notmuch-tag (query tag-changes) >"Add/remove tags in TAG-CHANGES to messages matching QUERY. > > QUERY should be a string containing the search-terms. > -TAG-CHANGES can take multiple forms. If TAG-CHANGES is a list of > -strings of the form \"+tag\" or \"-tag\" then those are the tag > -changes applied. If TAG-CHANGES is a string then it is > -interpreted as a single tag change. If TAG-CHANGES is the string > -\"-\" or \"+\", or null, then the user is prompted to enter the > -tag changes. > +TAG-CHANGES is a list of strings of the form \"+tag\" or > +\"-tag\" to add or remove tags, respectively. > > Note: Other code should always use this function alter tags of > messages instead of running (notmuch-call-notmuch-process \"tag\" ..) > directly, so that hooks specified in notmuch-before-tag-hook and > notmuch-after-tag-hook will be run." >;; Perform some validation > - (if (string-or-null-p tag-changes) > - (if (or (string= tag-changes "-") (string= tag-changes "+") (null > tag-changes)) > - (setq tag-changes (notmuch-read-tag-changes > - (notmuch-tag-completions query) nil tag-changes)) > - (setq tag-changes (list tag-changes Hey folks. After a recent upgrade to 0.16+120~gfd733a4+1 I found that my custom tagging functions weren't working, which I traced back to the removal of this section. Previously notmuch-tag accepted tag changes in two forms: as a list of tag changes: (list "+foo" "-bar") or as a space-separated string: "+foo -bar" This removed section is what would turn the space-separated string into a list. I have custom tagging functions that were written like this: (notmuch-search-tag "+spam") notmuch-search-tag passes the TAG-CHANGES to notmuch-tag, which was changing it to a list in this removed section. Since its removal, all of my custom functions started throwing the following error: Wrong type argument: stringp, 43 I didn't pay super close attention to the rest of the patch series, but
[PATCH 1/1] build: remove trailing '/.' when doing mkdir -p .deps/.
When make variable $@ does not contain directory part, $(@D) resolves as '.'. In this case .deps/$(@D) is '.deps/.' In some systems `mkdir [-p] directory/.` fails. To make this compatible with more system substitute trailing '/.' (slashdot) with '' (empty string) whenever it occurs there. --- Makefile.local | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.local b/Makefile.local index 72524eb..c85e09c 100644 --- a/Makefile.local +++ b/Makefile.local @@ -236,11 +236,11 @@ endif quiet ?= $($(shell echo $1 | sed -e s'/ .*//')) %.o: %.cc $(global_deps) - @mkdir -p .deps/$(@D) + @mkdir -p $(patsubst %/.,%,.deps/$(@D)) $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d %.o: %.c $(global_deps) - @mkdir -p .deps/$(@D) + @mkdir -p $(patsubst %/.,%,.deps/$(@D)) $(call quiet,CC $(CPPFLAGS) $(CFLAGS)) -c $(FINAL_CFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d .PHONY : clean -- 1.8.3.1
Getting the right root mail of the thread
On Sun, Nov 3, 2013 at 2:35 PM, Jesse Rosenthal wrote: > Jani Nikula writes: >> I think it's actually worse than what your example demonstrates. It's >> the subject of the newest/oldest *matching* message that gets used. In >> your example, the first/last messages in the thread apparently match >> your query. > > The behavior is there because subjects frequently change in long > threads, and this allows the subject to refer to the most recent unread > message (if we're sorting in the default order). The > reason I requested and wrote in this behavior five years ago or so (my > only c contribution ever) was that numerous business associates would > keep email lists by replying and changing the subject. This is *very* > common outside of programming circles. Even in programming circles, > subjects frequently change on mailing list (with a "[was: ...]" > appended). Yes but how important is it to keep track of that? I say it is much more important to track threads like this properly: foobar patch 0 (usually a summary/overview) +-foobar patch 1 | +-comment on patch 1 +-foobar patch 2 +-foobar patch 3 | +-comment on patch 3 +-foobar patch 4 +-foobar patch 5 But fine, let's concentrate on the common user scenario (which is not common for notmuch users at all). We can have a thread like this: No work on Friday + Shall we go for some beers? (was: No work on Friday) + What about project X? (was: No work on Friday) So which is the correct summary of the thread? The fact of the matter is that we are talking about three threads now. Gmail does this correctly. Each time the subject is changed, it's considered a new thread. > The current behavior can be annoying, but the old behavior could make > the MUA quite unusable in a number of circumstances. (And yes, an MUA > that fails on reading mail from senders with bad emailing practices is > unusable for me.) This is rhetorical warfare. It's wouldn't be "failing on reading mail". If displaying the original subject is failing, then we might be failing when searching with older first. Why would the order of the search affect the thread summary? > Maybe there should be a "show original subject" toggle? That wouldn't be > too hard, though it would require another call to the library and > regenerating the search results. Yes. I say it should be a property of the query. I don't see why anybody would want it any other way, but it wouldn't hurt to make it an option. -- Felipe Contreras
Getting the right root mail of the thread
Jani Nikula writes: > I think it's actually worse than what your example demonstrates. It's > the subject of the newest/oldest *matching* message that gets used. In > your example, the first/last messages in the thread apparently match > your query. The behavior is there because subjects frequently change in long threads, and this allows the subject to refer to the most recent unread message (if we're sorting in the default order). The reason I requested and wrote in this behavior five years ago or so (my only c contribution ever) was that numerous business associates would keep email lists by replying and changing the subject. This is *very* common outside of programming circles. Even in programming circles, subjects frequently change on mailing list (with a "[was: ...]" appended). The current behavior can be annoying, but the old behavior could make the MUA quite unusable in a number of circumstances. (And yes, an MUA that fails on reading mail from senders with bad emailing practices is unusable for me.) Maybe there should be a "show original subject" toggle? That wouldn't be too hard, though it would require another call to the library and regenerating the search results.
[PATCH] test: fix compact backup / restore test
On Sat, 02 Nov 2013, David Bremner wrote:
> It was looking in completely the wrong place for the backup and the
> (test) xapian database. Unfortunately test_begin_subtest hides the
> relevant errors.
I included this, unchanged, in my series [1] that depends on it. It
makes no difference which one is pushed.
BR,
Jani.
[1] id:cover.1383481295.git.jani at nikula.org
> ---
>
> I found this bug because
>
> id:9ee3f2334a117b0a1b88650f44432423cbe95fd7.1383315568.git.jani at
> nikula.org
>
> did _not_ break any tests. Which was puzzling.
>
> test/compact | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/test/compact b/test/compact
> index 5bb5cea..afab537 100755
> --- a/test/compact
> +++ b/test/compact
> @@ -19,10 +19,11 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One
> (inbox tag1 unread)
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2
> unread)
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
>
> -test_begin_subtest "Restoring backup"
> -rm -Rf ${TEST_TMPDIR}/mail/xapian
> -mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian
> +test_expect_success 'Restoring Backup' \
> +'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
> + mv ${MAIL_DIR}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
>
> +test_begin_subtest "Checking restored backup"
> output=$(notmuch search \* | notmuch_search_sanitize)
> test_expect_equal "$output" "\
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
> --
> 1.8.4.rc3
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 11/11] man: document notmuch compact --verbose and --backup=DIRECTORY options
--- man/man1/notmuch-compact.1 | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/man/man1/notmuch-compact.1 b/man/man1/notmuch-compact.1 index 1aeed22..dfffccb 100644 --- a/man/man1/notmuch-compact.1 +++ b/man/man1/notmuch-compact.1 @@ -4,6 +4,8 @@ notmuch-compact \- compact the notmuch database .SH SYNOPSIS .B notmuch compact +.RI "[ --verbose ]" +.RI "[ --backup=<" directory "> ]" .SH DESCRIPTION @@ -14,11 +16,35 @@ the space required by the database and improve lookup performance. The compacted database is built in a temporary directory and is later moved into the place of the origin database. The original uncompacted -database is preserved to be deleted by the user as desired. +database is discarded, unless the +.BR "\-\-backup=" +option is used. Note that the database write lock will be held during the compaction process (which may be quite long) to protect data integrity. +Supported options for +.B compact +include + +.RS 4 +.TP 4 +.BR "\-\-backup=" + +Save the current database to the given directory before replacing it +with the compacted database. The backup directory must not exist and +it must reside on the same mounted filesystem as the current database. + +.RE + +.RS 4 +.TP 4 +.BR \-\-verbose + +Report database compaction progress to stdout. + +.RE + .RE .SH ENVIRONMENT The following environment variables can be used to control the -- 1.8.4.rc3
[PATCH v2 10/11] cli: add compact --verbose option and silence output without it
If there's nothing surprising to say, don't say anything. Unless asked
for by the user.
---
notmuch-compact.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/notmuch-compact.c b/notmuch-compact.c
index 359acfc..1e7808c 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -32,27 +32,33 @@ notmuch_compact_command (notmuch_config_t *config, int
argc, char *argv[])
const char *path = notmuch_config_get_database_path (config);
const char *backup_path = NULL;
notmuch_status_t ret;
+notmuch_bool_t verbose;
int opt_index;
notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_STRING, &backup_path, "backup", 0, 0 },
+ { NOTMUCH_OPT_BOOLEAN, &verbose, "verbose", 'v', 0 },
};
opt_index = parse_arguments (argc, argv, options, 1);
if (opt_index < 0)
return 1;
-printf ("Compacting database...\n");
-ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
+if (verbose)
+ printf ("Compacting database...\n");
+ret = notmuch_database_compact (path, backup_path,
+ verbose ? status_update_cb : NULL, NULL);
if (ret) {
fprintf (stderr, "Compaction failed: %s\n",
notmuch_status_to_string(ret));
return 1;
}
-if (backup_path)
- printf ("The old database has been moved to %s.\n", backup_path);
+if (verbose) {
+ if (backup_path)
+ printf ("The old database has been moved to %s.\n", backup_path);
-printf ("Done.\n");
+ printf ("Done.\n");
+}
return 0;
}
--
1.8.4.rc3
[PATCH v2 09/11] cli: add compact --backup=DIRECTORY option, don't backup by default
It's the user's decision. The recommended way is to do a database dump
anyway. Clean up the relevant printfs too.
---
v2: reorder prints
---
notmuch-compact.c | 27 +--
test/compact | 4 ++--
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/notmuch-compact.c b/notmuch-compact.c
index b9461c2..359acfc 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -27,16 +27,19 @@ status_update_cb (const char *msg, unused (void *closure))
}
int
-notmuch_compact_command (notmuch_config_t *config,
-unused (int argc),
-unused (char *argv[]))
+notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])
{
const char *path = notmuch_config_get_database_path (config);
-const char *backup_path;
+const char *backup_path = NULL;
notmuch_status_t ret;
+int opt_index;
-backup_path = talloc_asprintf (config, "%s/xapian.old", path);
-if (! backup_path)
+notmuch_opt_desc_t options[] = {
+ { NOTMUCH_OPT_STRING, &backup_path, "backup", 0, 0 },
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+if (opt_index < 0)
return 1;
printf ("Compacting database...\n");
@@ -46,14 +49,10 @@ notmuch_compact_command (notmuch_config_t *config,
return 1;
}
-printf ("\n");
-printf ("\n");
-printf ("The old database has been moved to %s", backup_path);
-printf ("\n");
-printf ("To delete run,\n");
-printf ("\n");
-printf ("rm -R %s\n", backup_path);
-printf ("\n");
+if (backup_path)
+ printf ("The old database has been moved to %s.\n", backup_path);
+
+printf ("Done.\n");
return 0;
}
diff --git a/test/compact b/test/compact
index afab537..ac174ce 100755
--- a/test/compact
+++ b/test/compact
@@ -10,7 +10,7 @@ notmuch tag +tag1 \*
notmuch tag +tag2 subject:Two
notmuch tag -tag1 +tag3 subject:Three
-test_expect_success "Running compact" "notmuch compact"
+test_expect_success "Running compact" "notmuch compact
--backup=${TEST_DIRECTORY}/xapian.old"
test_begin_subtest "Compact preserves database"
output=$(notmuch search \* | notmuch_search_sanitize)
@@ -21,7 +21,7 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three
(inbox tag3 unread)"
test_expect_success 'Restoring Backup' \
'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
- mv ${MAIL_DIR}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
+ mv ${TEST_DIRECTORY}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
test_begin_subtest "Checking restored backup"
output=$(notmuch search \* | notmuch_search_sanitize)
--
1.8.4.rc3
[PATCH v2 08/11] cli: return error status if compaction fails
As is customary for any tool.
---
notmuch-compact.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/notmuch-compact.c b/notmuch-compact.c
index 55dc731..b9461c2 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -43,16 +43,17 @@ notmuch_compact_command (notmuch_config_t *config,
ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
if (ret) {
fprintf (stderr, "Compaction failed: %s\n",
notmuch_status_to_string(ret));
-} else {
- printf ("\n");
- printf ("\n");
- printf ("The old database has been moved to %s", backup_path);
- printf ("\n");
- printf ("To delete run,\n");
- printf ("\n");
- printf ("rm -R %s\n", backup_path);
- printf ("\n");
+ return 1;
}
+printf ("\n");
+printf ("\n");
+printf ("The old database has been moved to %s", backup_path);
+printf ("\n");
+printf ("To delete run,\n");
+printf ("\n");
+printf ("rm -R %s\n", backup_path);
+printf ("\n");
+
return 0;
}
--
1.8.4.rc3
[PATCH v2 07/11] lib: use the compaction backup path provided by the caller
The extra path component added by the lib is a magic value that the
caller just has to know. This is demonstrated by the current code,
which indeed has "xapian.old" both sides of the interface. Use the
backup path provided by the lib caller verbatim, without adding
anything to it.
---
v2: add xapian.old in cli
---
lib/database.cc | 14 --
notmuch-compact.c | 10 +++---
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 5a01703..a021bf1 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -868,7 +868,6 @@ notmuch_database_compact (const char* path,
{
void *local;
char *notmuch_path, *xapian_path, *compact_xapian_path;
-char *old_xapian_path = NULL;
notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
notmuch_database_t *notmuch = NULL;
struct stat statbuf;
@@ -898,13 +897,8 @@ notmuch_database_compact (const char* path,
}
if (backup_path != NULL) {
- if (! (old_xapian_path = talloc_asprintf (local, "%s/xapian.old",
backup_path))) {
- ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
- goto DONE;
- }
-
- if (stat(old_xapian_path, &statbuf) != -1) {
- fprintf (stderr, "Backup path already exists: %s\n",
old_xapian_path);
+ if (stat(backup_path, &statbuf) != -1) {
+ fprintf (stderr, "Backup path already exists: %s\n", backup_path);
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
@@ -928,8 +922,8 @@ notmuch_database_compact (const char* path,
goto DONE;
}
-if (old_xapian_path != NULL) {
- if (rename(xapian_path, old_xapian_path)) {
+if (backup_path) {
+ if (rename(xapian_path, backup_path)) {
fprintf (stderr, "Error moving old database out of the way\n");
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
diff --git a/notmuch-compact.c b/notmuch-compact.c
index ee7afcf..55dc731 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -32,9 +32,13 @@ notmuch_compact_command (notmuch_config_t *config,
unused (char *argv[]))
{
const char *path = notmuch_config_get_database_path (config);
-const char *backup_path = path;
+const char *backup_path;
notmuch_status_t ret;
+backup_path = talloc_asprintf (config, "%s/xapian.old", path);
+if (! backup_path)
+ return 1;
+
printf ("Compacting database...\n");
ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
if (ret) {
@@ -42,11 +46,11 @@ notmuch_compact_command (notmuch_config_t *config,
} else {
printf ("\n");
printf ("\n");
- printf ("The old database has been moved to %s/xapian.old",
backup_path);
+ printf ("The old database has been moved to %s", backup_path);
printf ("\n");
printf ("To delete run,\n");
printf ("\n");
- printf ("rm -R %s/xapian.old\n", backup_path);
+ printf ("rm -R %s\n", backup_path);
printf ("\n");
}
--
1.8.4.rc3
[PATCH v2 06/11] lib: update documentation of callback functions for database_compact and database_upgrade.
From: David Bremner Compact was missing callback documentation entirely, and upgrade did not discuss the closure parameter. --- lib/notmuch.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/notmuch.h b/lib/notmuch.h index cd301a4..82fd599 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -227,6 +227,9 @@ typedef void (*notmuch_compact_status_cb_t)(const char *message, void *closure); * The database will be opened with NOTMUCH_DATABASE_MODE_READ_WRITE * during the compaction process to ensure no writes are made. * + * If the optional callback function 'status_cb' is non-NULL, it will + * be called with diagnostic and informational messages. The argument + * 'closure' is passed verbatim to any callback invoked. */ notmuch_status_t notmuch_database_compact (const char* path, @@ -270,7 +273,8 @@ notmuch_database_needs_upgrade (notmuch_database_t *database); * provide progress indication to the user. If non-NULL it will be * called periodically with 'progress' as a floating-point value in * the range of [0.0 .. 1.0] indicating the progress made so far in - * the upgrade process. + * the upgrade process. The argument 'closure' is passed verbatim to + * any callback invoked. */ notmuch_status_t notmuch_database_upgrade (notmuch_database_t *database, -- 1.8.4.rc3
[PATCH v2 05/11] lib: add closure parameter to compact status update callback
This provides much more flexibility for the caller.
---
lib/database.cc | 14 +-
lib/notmuch.h | 5 +++--
notmuch-compact.c | 8 +++-
3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index eadf8a7..5a01703 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -821,9 +821,11 @@ static int rmtree (const char *path)
class NotmuchCompactor : public Xapian::Compactor
{
notmuch_compact_status_cb_t status_cb;
+void *status_closure;
public:
-NotmuchCompactor(notmuch_compact_status_cb_t cb) : status_cb(cb) { }
+NotmuchCompactor(notmuch_compact_status_cb_t cb, void *closure) :
+ status_cb(cb), status_closure(closure) { }
virtual void
set_status (const std::string &table, const std::string &status)
@@ -842,7 +844,7 @@ public:
return;
}
- status_cb(msg);
+ status_cb(msg, status_closure);
talloc_free(msg);
}
};
@@ -861,7 +863,8 @@ public:
notmuch_status_t
notmuch_database_compact (const char* path,
const char* backup_path,
- notmuch_compact_status_cb_t status_cb)
+ notmuch_compact_status_cb_t status_cb,
+ void *closure)
{
void *local;
char *notmuch_path, *xapian_path, *compact_xapian_path;
@@ -913,7 +916,7 @@ notmuch_database_compact (const char* path,
}
try {
- NotmuchCompactor compactor(status_cb);
+ NotmuchCompactor compactor(status_cb, closure);
compactor.set_renumber(false);
compactor.add_source(xapian_path);
@@ -953,7 +956,8 @@ DONE:
notmuch_status_t
notmuch_database_compact (unused (const char* path),
unused (const char* backup_path),
- unused (notmuch_compact_status_cb_t status_cb))
+ unused (notmuch_compact_status_cb_t status_cb),
+ unused (void *closure))
{
fprintf (stderr, "notmuch was compiled against a xapian version lacking
compaction support.\n");
return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9dab555..cd301a4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -219,7 +219,7 @@ notmuch_database_close (notmuch_database_t *database);
/* A callback invoked by notmuch_database_compact to notify the user
* of the progress of the compaction process.
*/
-typedef void (*notmuch_compact_status_cb_t)(const char*);
+typedef void (*notmuch_compact_status_cb_t)(const char *message, void
*closure);
/* Compact a notmuch database, backing up the original database to the
* given path.
@@ -231,7 +231,8 @@ typedef void (*notmuch_compact_status_cb_t)(const char*);
notmuch_status_t
notmuch_database_compact (const char* path,
const char* backup_path,
- notmuch_compact_status_cb_t status_cb);
+ notmuch_compact_status_cb_t status_cb,
+ void *closure);
/* Destroy the notmuch database, closing it if necessary and freeing
* all associated resources.
diff --git a/notmuch-compact.c b/notmuch-compact.c
index bfda40e..ee7afcf 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -20,10 +20,8 @@
#include "notmuch-client.h"
-void status_update_cb (const char *msg);
-
-void
-status_update_cb (const char *msg)
+static void
+status_update_cb (const char *msg, unused (void *closure))
{
printf("%s\n", msg);
}
@@ -38,7 +36,7 @@ notmuch_compact_command (notmuch_config_t *config,
notmuch_status_t ret;
printf ("Compacting database...\n");
-ret = notmuch_database_compact (path, backup_path, status_update_cb);
+ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
if (ret) {
fprintf (stderr, "Compaction failed: %s\n",
notmuch_status_to_string(ret));
} else {
--
1.8.4.rc3
[PATCH v2 04/11] lib: do not leak the database in compaction
Destroy instead of close the database after compaction, and also on error path, to not leak the database. --- lib/database.cc | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 7a8702e..eadf8a7 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -941,10 +941,12 @@ notmuch_database_compact (const char* path, goto DONE; } -notmuch_database_close(notmuch); - DONE: +if (notmuch) + notmuch_database_destroy (notmuch); + talloc_free(local); + return ret; } #else -- 1.8.4.rc3
[PATCH v2 03/11] lib: check talloc success in compact
In line with the allocation checks all around.
---
lib/database.cc | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 3dfea0f..7a8702e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -863,13 +863,17 @@ notmuch_database_compact (const char* path,
const char* backup_path,
notmuch_compact_status_cb_t status_cb)
{
-void *local = talloc_new (NULL);
+void *local;
char *notmuch_path, *xapian_path, *compact_xapian_path;
char *old_xapian_path = NULL;
notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
notmuch_database_t *notmuch = NULL;
struct stat statbuf;
+local = talloc_new (NULL);
+if (! local)
+ return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
ret = notmuch_database_open(path, NOTMUCH_DATABASE_MODE_READ_WRITE,
¬much);
if (ret) {
goto DONE;
--
1.8.4.rc3
[PATCH v2 02/11] lib: construct compactor within try block to catch any exceptions
Constructors may also throw exceptions. Catch them.
---
lib/database.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 20e5ec2..3dfea0f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -864,7 +864,6 @@ notmuch_database_compact (const char* path,
notmuch_compact_status_cb_t status_cb)
{
void *local = talloc_new (NULL);
-NotmuchCompactor compactor(status_cb);
char *notmuch_path, *xapian_path, *compact_xapian_path;
char *old_xapian_path = NULL;
notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
@@ -910,6 +909,8 @@ notmuch_database_compact (const char* path,
}
try {
+ NotmuchCompactor compactor(status_cb);
+
compactor.set_renumber(false);
compactor.add_source(xapian_path);
compactor.set_destdir(compact_xapian_path);
--
1.8.4.rc3
[PATCH v2 01/11] test: fix compact backup / restore test
From: David Bremner
It was looking in completely the wrong place for the backup and the
(test) xapian database. Unfortunately test_begin_subtest hides the
relevant errors.
---
test/compact | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/test/compact b/test/compact
index 5bb5cea..afab537 100755
--- a/test/compact
+++ b/test/compact
@@ -19,10 +19,11 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One
(inbox tag1 unread)
thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread)
thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
-test_begin_subtest "Restoring backup"
-rm -Rf ${TEST_TMPDIR}/mail/xapian
-mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian
+test_expect_success 'Restoring Backup' \
+'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
+ mv ${MAIL_DIR}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
+test_begin_subtest "Checking restored backup"
output=$(notmuch search \* | notmuch_search_sanitize)
test_expect_equal "$output" "\
thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
--
1.8.4.rc3
[PATCH v2 00/11] compactor changes v2
Hi all, this is v2 of [1], incorporating compact related patches from David, some changes from David's review, some new patches, man page update, test update. Cheers, Jani. [1] id:cover.1383315568.git.jani at nikula.org David Bremner (2): test: fix compact backup / restore test lib: update documentation of callback functions for database_compact and database_upgrade. Jani Nikula (9): lib: construct compactor within try block to catch any exceptions lib: check talloc success in compact lib: do not leak the database in compaction lib: add closure parameter to compact status update callback lib: use the compaction backup path provided by the caller cli: return error status if compaction fails cli: add compact --backup=DIRECTORY option, don't backup by default cli: add compact --verbose option and silence output without it man: document notmuch compact --verbose and --backup=DIRECTORY options lib/database.cc| 41 +++-- lib/notmuch.h | 11 --- man/man1/notmuch-compact.1 | 28 +++- notmuch-compact.c | 46 +++--- test/compact | 9 + 5 files changed, 90 insertions(+), 45 deletions(-) -- 1.8.4.rc3
Getting the right root mail of the thread
On 11/02/2013 09:08 AM, Felipe Contreras wrote: > Either way this doesn't make any sense to me. Each thread has a single origin > mail, why would anybody would like to show a message other than that while > displaying the summary of the tread? Even more, why isn't there an option to > fetch that information easily? It's entirely possible to make a message that has a References: header that includes message IDs from two different threads. For a message like this, there is not a single "origin mail", right? For threads that derive from this message, the whole thread has more than one origin. --dkg
Getting the right root mail of the thread
On Sun, Nov 3, 2013 at 1:22 PM, Daniel Kahn Gillmor wrote: > On 11/02/2013 09:08 AM, Felipe Contreras wrote: >> >> Either way this doesn't make any sense to me. Each thread has a single >> origin >> mail, why would anybody would like to show a message other than that while >> displaying the summary of the tread? Even more, why isn't there an option >> to >> fetch that information easily? > > > It's entirely possible to make a message that has a References: header that > includes message IDs from two different threads. Right, there could be two or more roots. But we don't get either in the summary, we can only get the first or the last one. I say the summary should contain the earliest root, always. Or at least have an option to get it. -- Felipe Contreras
Getting the right root mail of the thread
On Sat, Nov 2, 2013 at 8:58 AM, Jani Nikula wrote:
> On Sat, 02 Nov 2013, Felipe Contreras wrote:
>> On Sat, Nov 2, 2013 at 7:50 AM, Jani Nikula wrote:
>>> On Sat, 02 Nov 2013, Felipe Contreras wrote:
>>
I think there should be a way to get the root mail of a thread,
irrespective of the search order.
>>>
>>> Largely agreed. It's just that nobody's gotten around to doing this
>>> yet. At the cli level I think the consensus is that the structured
>>> (sexp/json) output format should contain multiple (or all) subjects.
>>
>> What about the default? (--format=text). What about user-interfaces
>> that must display a summary of a thread?
>
> To be honest, we haven't had much interest in adding new content or
> features to the text output formats. It's just much easier to do this in
> a backwards compatible way in the structured output formats, for which
> we also have the format versioning. The emacs ui uses the sexp format
> for the thread summaries.
How is this any better?
(:thread "826a" :timestamp 1383396519 :date_relative
"Yest. 06:48" :matched 45 :total 45 :authors "Felipe Contreras"
:subject "[PATCH/TEST 44/44] request-pull: rewrite to C" :tags ("sent"
"to-me"))
--
Felipe Contreras
Re: [PATCH 1/1] build: remove trailing '/.' when doing mkdir -p .deps/.
On Sun, Nov 03 2013, Jed Brown wrote: > Tomi Ollila writes: > >> %.o: %.cc $(global_deps) >> -@mkdir -p .deps/$(@D) >> +@mkdir -p $(patsubst %/.,%,.deps/$(@D)) >> $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ >> -MD -MP -MF .deps/$*.d > > An alternative approach is to use directory marker files [1] to clean up > the recipes that need output directories and to satisfy Paul's second > rule of makefiles [2]. > > .SECONDEXPANSION: > > %.o: %.cc $(global_deps) | .deps/$$(@D)/.DIR > $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ > -MD -MP -MF .deps/$*.d > > %/.DIR: > @mkdir -p $(patsubst %/.,%,$(@D)) > @touch $@ > > .PRECIOUS: %.DIR Hmm, nice suggestion... the diff to be reviewed is just soo much bigger ;/ Now that I learned new things [11] yet another alternative is: diff --git a/Makefile.local b/Makefile.local index 72524eb..cc1a0cb 100644 --- a/Makefile.local +++ b/Makefile.local @@ -235,12 +235,15 @@ endif # Otherwise, print the full command line. quiet ?= $($(shell echo $1 | sed -e s'/ .*//')) -%.o: %.cc $(global_deps) - @mkdir -p .deps/$(@D) +depdirs = $(subdirs:%=.deps/%) + +$(depdirs): + @mkdir -p $(depdirs) + +%.o: %.cc $(global_deps) | $(depdirs) $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d -%.o: %.c $(global_deps) - @mkdir -p .deps/$(@D) +%.o: %.c $(global_deps) | $(depdirs) $(call quiet,CC $(CPPFLAGS) $(CFLAGS)) -c $(FINAL_CFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d .PHONY : clean still, for the time being I'd still use the patch I originally proposed due to the triviality I change... [11] http://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html Tomi > [1] http://www.cmcrossroads.com/article/making-directories-gnu-make > [2] http://make.paulandlesley.org/rules.html ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
On Sun, Nov 3, 2013 at 2:35 PM, Jesse Rosenthal wrote: > Jani Nikula writes: >> I think it's actually worse than what your example demonstrates. It's >> the subject of the newest/oldest *matching* message that gets used. In >> your example, the first/last messages in the thread apparently match >> your query. > > The behavior is there because subjects frequently change in long > threads, and this allows the subject to refer to the most recent unread > message (if we're sorting in the default order). The > reason I requested and wrote in this behavior five years ago or so (my > only c contribution ever) was that numerous business associates would > keep email lists by replying and changing the subject. This is *very* > common outside of programming circles. Even in programming circles, > subjects frequently change on mailing list (with a "[was: ...]" > appended). Yes but how important is it to keep track of that? I say it is much more important to track threads like this properly: foobar patch 0 (usually a summary/overview) +-foobar patch 1 | +-comment on patch 1 +-foobar patch 2 +-foobar patch 3 | +-comment on patch 3 +-foobar patch 4 +-foobar patch 5 But fine, let's concentrate on the common user scenario (which is not common for notmuch users at all). We can have a thread like this: No work on Friday + Shall we go for some beers? (was: No work on Friday) + What about project X? (was: No work on Friday) So which is the correct summary of the thread? The fact of the matter is that we are talking about three threads now. Gmail does this correctly. Each time the subject is changed, it's considered a new thread. > The current behavior can be annoying, but the old behavior could make > the MUA quite unusable in a number of circumstances. (And yes, an MUA > that fails on reading mail from senders with bad emailing practices is > unusable for me.) This is rhetorical warfare. It's wouldn't be "failing on reading mail". If displaying the original subject is failing, then we might be failing when searching with older first. Why would the order of the search affect the thread summary? > Maybe there should be a "show original subject" toggle? That wouldn't be > too hard, though it would require another call to the library and > regenerating the search results. Yes. I say it should be a property of the query. I don't see why anybody would want it any other way, but it wouldn't hurt to make it an option. -- Felipe Contreras ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
On Sun, 03 Nov 2013, Jesse Rosenthal wrote: > The current behavior can be annoying, but the old behavior could make > the MUA quite unusable in a number of circumstances. (And yes, an MUA > that fails on reading mail from senders with bad emailing practices is > unusable for me.) I wouldn't have suggested changing the current default behaviour anyway, but thanks for reminding us there are users depending on it! > Maybe there should be a "show original subject" toggle? That wouldn't be > too hard, though it would require another call to the library and > regenerating the search results. IMO the best approach would be including the first (or oldest, or original) subject in the structured thread summary output, in addition to the first matching subject that we have now. BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
Jani Nikula writes: > I think it's actually worse than what your example demonstrates. It's > the subject of the newest/oldest *matching* message that gets used. In > your example, the first/last messages in the thread apparently match > your query. The behavior is there because subjects frequently change in long threads, and this allows the subject to refer to the most recent unread message (if we're sorting in the default order). The reason I requested and wrote in this behavior five years ago or so (my only c contribution ever) was that numerous business associates would keep email lists by replying and changing the subject. This is *very* common outside of programming circles. Even in programming circles, subjects frequently change on mailing list (with a "[was: ...]" appended). The current behavior can be annoying, but the old behavior could make the MUA quite unusable in a number of circumstances. (And yes, an MUA that fails on reading mail from senders with bad emailing practices is unusable for me.) Maybe there should be a "show original subject" toggle? That wouldn't be too hard, though it would require another call to the library and regenerating the search results. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
On Sun, 03 Nov 2013, Felipe Contreras wrote:
> On Sat, Nov 2, 2013 at 8:58 AM, Jani Nikula wrote:
>> On Sat, 02 Nov 2013, Felipe Contreras wrote:
>>> On Sat, Nov 2, 2013 at 7:50 AM, Jani Nikula wrote:
On Sat, 02 Nov 2013, Felipe Contreras wrote:
>>>
> I think there should be a way to get the root mail of a thread,
> irrespective of the search order.
Largely agreed. It's just that nobody's gotten around to doing this
yet. At the cli level I think the consensus is that the structured
(sexp/json) output format should contain multiple (or all) subjects.
>>>
>>> What about the default? (--format=text). What about user-interfaces
>>> that must display a summary of a thread?
>>
>> To be honest, we haven't had much interest in adding new content or
>> features to the text output formats. It's just much easier to do this in
>> a backwards compatible way in the structured output formats, for which
>> we also have the format versioning. The emacs ui uses the sexp format
>> for the thread summaries.
>
> How is this any better?
>
> (:thread "826a" :timestamp 1383396519 :date_relative
> "Yest. 06:48" :matched 45 :total 45 :authors "Felipe Contreras"
> :subject "[PATCH/TEST 44/44] request-pull: rewrite to C" :tags ("sent"
> "to-me"))
I made no claims it would contain more or better information *now*. But
if someone were to add new things, the structured output is where it
would likely be added. We could add :subject_first, :subject_oldest, or
whatever *in addition* to :subject.
BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
On Sun, Nov 3, 2013 at 1:22 PM, Daniel Kahn Gillmor wrote: > On 11/02/2013 09:08 AM, Felipe Contreras wrote: >> >> Either way this doesn't make any sense to me. Each thread has a single >> origin >> mail, why would anybody would like to show a message other than that while >> displaying the summary of the tread? Even more, why isn't there an option >> to >> fetch that information easily? > > > It's entirely possible to make a message that has a References: header that > includes message IDs from two different threads. Right, there could be two or more roots. But we don't get either in the summary, we can only get the first or the last one. I say the summary should contain the earliest root, always. Or at least have an option to get it. -- Felipe Contreras ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
On Sat, Nov 2, 2013 at 8:58 AM, Jani Nikula wrote:
> On Sat, 02 Nov 2013, Felipe Contreras wrote:
>> On Sat, Nov 2, 2013 at 7:50 AM, Jani Nikula wrote:
>>> On Sat, 02 Nov 2013, Felipe Contreras wrote:
>>
I think there should be a way to get the root mail of a thread,
irrespective of the search order.
>>>
>>> Largely agreed. It's just that nobody's gotten around to doing this
>>> yet. At the cli level I think the consensus is that the structured
>>> (sexp/json) output format should contain multiple (or all) subjects.
>>
>> What about the default? (--format=text). What about user-interfaces
>> that must display a summary of a thread?
>
> To be honest, we haven't had much interest in adding new content or
> features to the text output formats. It's just much easier to do this in
> a backwards compatible way in the structured output formats, for which
> we also have the format versioning. The emacs ui uses the sexp format
> for the thread summaries.
How is this any better?
(:thread "826a" :timestamp 1383396519 :date_relative
"Yest. 06:48" :matched 45 :total 45 :authors "Felipe Contreras"
:subject "[PATCH/TEST 44/44] request-pull: rewrite to C" :tags ("sent"
"to-me"))
--
Felipe Contreras
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: Getting the right root mail of the thread
On 11/02/2013 09:08 AM, Felipe Contreras wrote: Either way this doesn't make any sense to me. Each thread has a single origin mail, why would anybody would like to show a message other than that while displaying the summary of the tread? Even more, why isn't there an option to fetch that information easily? It's entirely possible to make a message that has a References: header that includes message IDs from two different threads. For a message like this, there is not a single "origin mail", right? For threads that derive from this message, the whole thread has more than one origin. --dkg ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/1] build: remove trailing '/.' when doing mkdir -p .deps/.
Tomi Ollila writes: > %.o: %.cc $(global_deps) > - @mkdir -p .deps/$(@D) > + @mkdir -p $(patsubst %/.,%,.deps/$(@D)) > $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ > -MD -MP -MF .deps/$*.d An alternative approach is to use directory marker files [1] to clean up the recipes that need output directories and to satisfy Paul's second rule of makefiles [2]. .SECONDEXPANSION: %.o: %.cc $(global_deps) | .deps/$$(@D)/.DIR $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d %/.DIR: @mkdir -p $(patsubst %/.,%,$(@D)) @touch $@ .PRECIOUS: %.DIR [1] http://www.cmcrossroads.com/article/making-directories-gnu-make [2] http://make.paulandlesley.org/rules.html -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20131103/6cd2f078/attachment.pgp>
Re: [PATCH 1/1] build: remove trailing '/.' when doing mkdir -p .deps/.
Tomi Ollila writes: > %.o: %.cc $(global_deps) > - @mkdir -p .deps/$(@D) > + @mkdir -p $(patsubst %/.,%,.deps/$(@D)) > $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ > -MD -MP -MF .deps/$*.d An alternative approach is to use directory marker files [1] to clean up the recipes that need output directories and to satisfy Paul's second rule of makefiles [2]. .SECONDEXPANSION: %.o: %.cc $(global_deps) | .deps/$$(@D)/.DIR $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d %/.DIR: @mkdir -p $(patsubst %/.,%,$(@D)) @touch $@ .PRECIOUS: %.DIR [1] http://www.cmcrossroads.com/article/making-directories-gnu-make [2] http://make.paulandlesley.org/rules.html pgpBRYtw3Lebn.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/1] build: remove trailing '/.' when doing mkdir -p .deps/.
When make variable $@ does not contain directory part, $(@D) resolves as '.'. In this case .deps/$(@D) is '.deps/.' In some systems `mkdir [-p] directory/.` fails. To make this compatible with more system substitute trailing '/.' (slashdot) with '' (empty string) whenever it occurs there. --- Makefile.local | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.local b/Makefile.local index 72524eb..c85e09c 100644 --- a/Makefile.local +++ b/Makefile.local @@ -236,11 +236,11 @@ endif quiet ?= $($(shell echo $1 | sed -e s'/ .*//')) %.o: %.cc $(global_deps) - @mkdir -p .deps/$(@D) + @mkdir -p $(patsubst %/.,%,.deps/$(@D)) $(call quiet,CXX $(CPPFLAGS) $(CXXFLAGS)) -c $(FINAL_CXXFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d %.o: %.c $(global_deps) - @mkdir -p .deps/$(@D) + @mkdir -p $(patsubst %/.,%,.deps/$(@D)) $(call quiet,CC $(CPPFLAGS) $(CFLAGS)) -c $(FINAL_CFLAGS) $< -o $@ -MD -MP -MF .deps/$*.d .PHONY : clean -- 1.8.3.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] test: fix compact backup / restore test
On Sat, 02 Nov 2013, David Bremner wrote:
> It was looking in completely the wrong place for the backup and the
> (test) xapian database. Unfortunately test_begin_subtest hides the
> relevant errors.
I included this, unchanged, in my series [1] that depends on it. It
makes no difference which one is pushed.
BR,
Jani.
[1] id:cover.1383481295.git.j...@nikula.org
> ---
>
> I found this bug because
>
> id:9ee3f2334a117b0a1b88650f44432423cbe95fd7.1383315568.git.j...@nikula.org
>
> did _not_ break any tests. Which was puzzling.
>
> test/compact | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/test/compact b/test/compact
> index 5bb5cea..afab537 100755
> --- a/test/compact
> +++ b/test/compact
> @@ -19,10 +19,11 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One
> (inbox tag1 unread)
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2
> unread)
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
>
> -test_begin_subtest "Restoring backup"
> -rm -Rf ${TEST_TMPDIR}/mail/xapian
> -mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian
> +test_expect_success 'Restoring Backup' \
> +'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
> + mv ${MAIL_DIR}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
>
> +test_begin_subtest "Checking restored backup"
> output=$(notmuch search \* | notmuch_search_sanitize)
> test_expect_equal "$output" "\
> thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
> --
> 1.8.4.rc3
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 11/11] man: document notmuch compact --verbose and --backup=DIRECTORY options
--- man/man1/notmuch-compact.1 | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/man/man1/notmuch-compact.1 b/man/man1/notmuch-compact.1 index 1aeed22..dfffccb 100644 --- a/man/man1/notmuch-compact.1 +++ b/man/man1/notmuch-compact.1 @@ -4,6 +4,8 @@ notmuch-compact \- compact the notmuch database .SH SYNOPSIS .B notmuch compact +.RI "[ --verbose ]" +.RI "[ --backup=<" directory "> ]" .SH DESCRIPTION @@ -14,11 +16,35 @@ the space required by the database and improve lookup performance. The compacted database is built in a temporary directory and is later moved into the place of the origin database. The original uncompacted -database is preserved to be deleted by the user as desired. +database is discarded, unless the +.BR "\-\-backup=" +option is used. Note that the database write lock will be held during the compaction process (which may be quite long) to protect data integrity. +Supported options for +.B compact +include + +.RS 4 +.TP 4 +.BR "\-\-backup=" + +Save the current database to the given directory before replacing it +with the compacted database. The backup directory must not exist and +it must reside on the same mounted filesystem as the current database. + +.RE + +.RS 4 +.TP 4 +.BR \-\-verbose + +Report database compaction progress to stdout. + +.RE + .RE .SH ENVIRONMENT The following environment variables can be used to control the -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 08/11] cli: return error status if compaction fails
As is customary for any tool.
---
notmuch-compact.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/notmuch-compact.c b/notmuch-compact.c
index 55dc731..b9461c2 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -43,16 +43,17 @@ notmuch_compact_command (notmuch_config_t *config,
ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
if (ret) {
fprintf (stderr, "Compaction failed: %s\n",
notmuch_status_to_string(ret));
-} else {
- printf ("\n");
- printf ("\n");
- printf ("The old database has been moved to %s", backup_path);
- printf ("\n");
- printf ("To delete run,\n");
- printf ("\n");
- printf ("rm -R %s\n", backup_path);
- printf ("\n");
+ return 1;
}
+printf ("\n");
+printf ("\n");
+printf ("The old database has been moved to %s", backup_path);
+printf ("\n");
+printf ("To delete run,\n");
+printf ("\n");
+printf ("rm -R %s\n", backup_path);
+printf ("\n");
+
return 0;
}
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 07/11] lib: use the compaction backup path provided by the caller
The extra path component added by the lib is a magic value that the
caller just has to know. This is demonstrated by the current code,
which indeed has "xapian.old" both sides of the interface. Use the
backup path provided by the lib caller verbatim, without adding
anything to it.
---
v2: add xapian.old in cli
---
lib/database.cc | 14 --
notmuch-compact.c | 10 +++---
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 5a01703..a021bf1 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -868,7 +868,6 @@ notmuch_database_compact (const char* path,
{
void *local;
char *notmuch_path, *xapian_path, *compact_xapian_path;
-char *old_xapian_path = NULL;
notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
notmuch_database_t *notmuch = NULL;
struct stat statbuf;
@@ -898,13 +897,8 @@ notmuch_database_compact (const char* path,
}
if (backup_path != NULL) {
- if (! (old_xapian_path = talloc_asprintf (local, "%s/xapian.old",
backup_path))) {
- ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
- goto DONE;
- }
-
- if (stat(old_xapian_path, &statbuf) != -1) {
- fprintf (stderr, "Backup path already exists: %s\n",
old_xapian_path);
+ if (stat(backup_path, &statbuf) != -1) {
+ fprintf (stderr, "Backup path already exists: %s\n", backup_path);
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
@@ -928,8 +922,8 @@ notmuch_database_compact (const char* path,
goto DONE;
}
-if (old_xapian_path != NULL) {
- if (rename(xapian_path, old_xapian_path)) {
+if (backup_path) {
+ if (rename(xapian_path, backup_path)) {
fprintf (stderr, "Error moving old database out of the way\n");
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
diff --git a/notmuch-compact.c b/notmuch-compact.c
index ee7afcf..55dc731 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -32,9 +32,13 @@ notmuch_compact_command (notmuch_config_t *config,
unused (char *argv[]))
{
const char *path = notmuch_config_get_database_path (config);
-const char *backup_path = path;
+const char *backup_path;
notmuch_status_t ret;
+backup_path = talloc_asprintf (config, "%s/xapian.old", path);
+if (! backup_path)
+ return 1;
+
printf ("Compacting database...\n");
ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
if (ret) {
@@ -42,11 +46,11 @@ notmuch_compact_command (notmuch_config_t *config,
} else {
printf ("\n");
printf ("\n");
- printf ("The old database has been moved to %s/xapian.old",
backup_path);
+ printf ("The old database has been moved to %s", backup_path);
printf ("\n");
printf ("To delete run,\n");
printf ("\n");
- printf ("rm -R %s/xapian.old\n", backup_path);
+ printf ("rm -R %s\n", backup_path);
printf ("\n");
}
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 09/11] cli: add compact --backup=DIRECTORY option, don't backup by default
It's the user's decision. The recommended way is to do a database dump
anyway. Clean up the relevant printfs too.
---
v2: reorder prints
---
notmuch-compact.c | 27 +--
test/compact | 4 ++--
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/notmuch-compact.c b/notmuch-compact.c
index b9461c2..359acfc 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -27,16 +27,19 @@ status_update_cb (const char *msg, unused (void *closure))
}
int
-notmuch_compact_command (notmuch_config_t *config,
-unused (int argc),
-unused (char *argv[]))
+notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])
{
const char *path = notmuch_config_get_database_path (config);
-const char *backup_path;
+const char *backup_path = NULL;
notmuch_status_t ret;
+int opt_index;
-backup_path = talloc_asprintf (config, "%s/xapian.old", path);
-if (! backup_path)
+notmuch_opt_desc_t options[] = {
+ { NOTMUCH_OPT_STRING, &backup_path, "backup", 0, 0 },
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+if (opt_index < 0)
return 1;
printf ("Compacting database...\n");
@@ -46,14 +49,10 @@ notmuch_compact_command (notmuch_config_t *config,
return 1;
}
-printf ("\n");
-printf ("\n");
-printf ("The old database has been moved to %s", backup_path);
-printf ("\n");
-printf ("To delete run,\n");
-printf ("\n");
-printf ("rm -R %s\n", backup_path);
-printf ("\n");
+if (backup_path)
+ printf ("The old database has been moved to %s.\n", backup_path);
+
+printf ("Done.\n");
return 0;
}
diff --git a/test/compact b/test/compact
index afab537..ac174ce 100755
--- a/test/compact
+++ b/test/compact
@@ -10,7 +10,7 @@ notmuch tag +tag1 \*
notmuch tag +tag2 subject:Two
notmuch tag -tag1 +tag3 subject:Three
-test_expect_success "Running compact" "notmuch compact"
+test_expect_success "Running compact" "notmuch compact
--backup=${TEST_DIRECTORY}/xapian.old"
test_begin_subtest "Compact preserves database"
output=$(notmuch search \* | notmuch_search_sanitize)
@@ -21,7 +21,7 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three
(inbox tag3 unread)"
test_expect_success 'Restoring Backup' \
'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
- mv ${MAIL_DIR}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
+ mv ${TEST_DIRECTORY}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
test_begin_subtest "Checking restored backup"
output=$(notmuch search \* | notmuch_search_sanitize)
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 10/11] cli: add compact --verbose option and silence output without it
If there's nothing surprising to say, don't say anything. Unless asked
for by the user.
---
notmuch-compact.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/notmuch-compact.c b/notmuch-compact.c
index 359acfc..1e7808c 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -32,27 +32,33 @@ notmuch_compact_command (notmuch_config_t *config, int
argc, char *argv[])
const char *path = notmuch_config_get_database_path (config);
const char *backup_path = NULL;
notmuch_status_t ret;
+notmuch_bool_t verbose;
int opt_index;
notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_STRING, &backup_path, "backup", 0, 0 },
+ { NOTMUCH_OPT_BOOLEAN, &verbose, "verbose", 'v', 0 },
};
opt_index = parse_arguments (argc, argv, options, 1);
if (opt_index < 0)
return 1;
-printf ("Compacting database...\n");
-ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
+if (verbose)
+ printf ("Compacting database...\n");
+ret = notmuch_database_compact (path, backup_path,
+ verbose ? status_update_cb : NULL, NULL);
if (ret) {
fprintf (stderr, "Compaction failed: %s\n",
notmuch_status_to_string(ret));
return 1;
}
-if (backup_path)
- printf ("The old database has been moved to %s.\n", backup_path);
+if (verbose) {
+ if (backup_path)
+ printf ("The old database has been moved to %s.\n", backup_path);
-printf ("Done.\n");
+ printf ("Done.\n");
+}
return 0;
}
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 05/11] lib: add closure parameter to compact status update callback
This provides much more flexibility for the caller.
---
lib/database.cc | 14 +-
lib/notmuch.h | 5 +++--
notmuch-compact.c | 8 +++-
3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index eadf8a7..5a01703 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -821,9 +821,11 @@ static int rmtree (const char *path)
class NotmuchCompactor : public Xapian::Compactor
{
notmuch_compact_status_cb_t status_cb;
+void *status_closure;
public:
-NotmuchCompactor(notmuch_compact_status_cb_t cb) : status_cb(cb) { }
+NotmuchCompactor(notmuch_compact_status_cb_t cb, void *closure) :
+ status_cb(cb), status_closure(closure) { }
virtual void
set_status (const std::string &table, const std::string &status)
@@ -842,7 +844,7 @@ public:
return;
}
- status_cb(msg);
+ status_cb(msg, status_closure);
talloc_free(msg);
}
};
@@ -861,7 +863,8 @@ public:
notmuch_status_t
notmuch_database_compact (const char* path,
const char* backup_path,
- notmuch_compact_status_cb_t status_cb)
+ notmuch_compact_status_cb_t status_cb,
+ void *closure)
{
void *local;
char *notmuch_path, *xapian_path, *compact_xapian_path;
@@ -913,7 +916,7 @@ notmuch_database_compact (const char* path,
}
try {
- NotmuchCompactor compactor(status_cb);
+ NotmuchCompactor compactor(status_cb, closure);
compactor.set_renumber(false);
compactor.add_source(xapian_path);
@@ -953,7 +956,8 @@ DONE:
notmuch_status_t
notmuch_database_compact (unused (const char* path),
unused (const char* backup_path),
- unused (notmuch_compact_status_cb_t status_cb))
+ unused (notmuch_compact_status_cb_t status_cb),
+ unused (void *closure))
{
fprintf (stderr, "notmuch was compiled against a xapian version lacking
compaction support.\n");
return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9dab555..cd301a4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -219,7 +219,7 @@ notmuch_database_close (notmuch_database_t *database);
/* A callback invoked by notmuch_database_compact to notify the user
* of the progress of the compaction process.
*/
-typedef void (*notmuch_compact_status_cb_t)(const char*);
+typedef void (*notmuch_compact_status_cb_t)(const char *message, void
*closure);
/* Compact a notmuch database, backing up the original database to the
* given path.
@@ -231,7 +231,8 @@ typedef void (*notmuch_compact_status_cb_t)(const char*);
notmuch_status_t
notmuch_database_compact (const char* path,
const char* backup_path,
- notmuch_compact_status_cb_t status_cb);
+ notmuch_compact_status_cb_t status_cb,
+ void *closure);
/* Destroy the notmuch database, closing it if necessary and freeing
* all associated resources.
diff --git a/notmuch-compact.c b/notmuch-compact.c
index bfda40e..ee7afcf 100644
--- a/notmuch-compact.c
+++ b/notmuch-compact.c
@@ -20,10 +20,8 @@
#include "notmuch-client.h"
-void status_update_cb (const char *msg);
-
-void
-status_update_cb (const char *msg)
+static void
+status_update_cb (const char *msg, unused (void *closure))
{
printf("%s\n", msg);
}
@@ -38,7 +36,7 @@ notmuch_compact_command (notmuch_config_t *config,
notmuch_status_t ret;
printf ("Compacting database...\n");
-ret = notmuch_database_compact (path, backup_path, status_update_cb);
+ret = notmuch_database_compact (path, backup_path, status_update_cb, NULL);
if (ret) {
fprintf (stderr, "Compaction failed: %s\n",
notmuch_status_to_string(ret));
} else {
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 06/11] lib: update documentation of callback functions for database_compact and database_upgrade.
From: David Bremner Compact was missing callback documentation entirely, and upgrade did not discuss the closure parameter. --- lib/notmuch.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/notmuch.h b/lib/notmuch.h index cd301a4..82fd599 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -227,6 +227,9 @@ typedef void (*notmuch_compact_status_cb_t)(const char *message, void *closure); * The database will be opened with NOTMUCH_DATABASE_MODE_READ_WRITE * during the compaction process to ensure no writes are made. * + * If the optional callback function 'status_cb' is non-NULL, it will + * be called with diagnostic and informational messages. The argument + * 'closure' is passed verbatim to any callback invoked. */ notmuch_status_t notmuch_database_compact (const char* path, @@ -270,7 +273,8 @@ notmuch_database_needs_upgrade (notmuch_database_t *database); * provide progress indication to the user. If non-NULL it will be * called periodically with 'progress' as a floating-point value in * the range of [0.0 .. 1.0] indicating the progress made so far in - * the upgrade process. + * the upgrade process. The argument 'closure' is passed verbatim to + * any callback invoked. */ notmuch_status_t notmuch_database_upgrade (notmuch_database_t *database, -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 04/11] lib: do not leak the database in compaction
Destroy instead of close the database after compaction, and also on error path, to not leak the database. --- lib/database.cc | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 7a8702e..eadf8a7 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -941,10 +941,12 @@ notmuch_database_compact (const char* path, goto DONE; } -notmuch_database_close(notmuch); - DONE: +if (notmuch) + notmuch_database_destroy (notmuch); + talloc_free(local); + return ret; } #else -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 03/11] lib: check talloc success in compact
In line with the allocation checks all around.
---
lib/database.cc | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 3dfea0f..7a8702e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -863,13 +863,17 @@ notmuch_database_compact (const char* path,
const char* backup_path,
notmuch_compact_status_cb_t status_cb)
{
-void *local = talloc_new (NULL);
+void *local;
char *notmuch_path, *xapian_path, *compact_xapian_path;
char *old_xapian_path = NULL;
notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
notmuch_database_t *notmuch = NULL;
struct stat statbuf;
+local = talloc_new (NULL);
+if (! local)
+ return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
ret = notmuch_database_open(path, NOTMUCH_DATABASE_MODE_READ_WRITE,
¬much);
if (ret) {
goto DONE;
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 00/11] compactor changes v2
Hi all, this is v2 of [1], incorporating compact related patches from David, some changes from David's review, some new patches, man page update, test update. Cheers, Jani. [1] id:cover.1383315568.git.j...@nikula.org David Bremner (2): test: fix compact backup / restore test lib: update documentation of callback functions for database_compact and database_upgrade. Jani Nikula (9): lib: construct compactor within try block to catch any exceptions lib: check talloc success in compact lib: do not leak the database in compaction lib: add closure parameter to compact status update callback lib: use the compaction backup path provided by the caller cli: return error status if compaction fails cli: add compact --backup=DIRECTORY option, don't backup by default cli: add compact --verbose option and silence output without it man: document notmuch compact --verbose and --backup=DIRECTORY options lib/database.cc| 41 +++-- lib/notmuch.h | 11 --- man/man1/notmuch-compact.1 | 28 +++- notmuch-compact.c | 46 +++--- test/compact | 9 + 5 files changed, 90 insertions(+), 45 deletions(-) -- 1.8.4.rc3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 02/11] lib: construct compactor within try block to catch any exceptions
Constructors may also throw exceptions. Catch them.
---
lib/database.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 20e5ec2..3dfea0f 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -864,7 +864,6 @@ notmuch_database_compact (const char* path,
notmuch_compact_status_cb_t status_cb)
{
void *local = talloc_new (NULL);
-NotmuchCompactor compactor(status_cb);
char *notmuch_path, *xapian_path, *compact_xapian_path;
char *old_xapian_path = NULL;
notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
@@ -910,6 +909,8 @@ notmuch_database_compact (const char* path,
}
try {
+ NotmuchCompactor compactor(status_cb);
+
compactor.set_renumber(false);
compactor.add_source(xapian_path);
compactor.set_destdir(compact_xapian_path);
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 01/11] test: fix compact backup / restore test
From: David Bremner
It was looking in completely the wrong place for the backup and the
(test) xapian database. Unfortunately test_begin_subtest hides the
relevant errors.
---
test/compact | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/test/compact b/test/compact
index 5bb5cea..afab537 100755
--- a/test/compact
+++ b/test/compact
@@ -19,10 +19,11 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One
(inbox tag1 unread)
thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread)
thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
-test_begin_subtest "Restoring backup"
-rm -Rf ${TEST_TMPDIR}/mail/xapian
-mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian
+test_expect_success 'Restoring Backup' \
+'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
+ mv ${MAIL_DIR}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
+test_begin_subtest "Checking restored backup"
output=$(notmuch search \* | notmuch_search_sanitize)
test_expect_equal "$output" "\
thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
--
1.8.4.rc3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
