[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Wed, 10 Nov 2010, Carl Worth wrote:
> I'm thinking of documenting/implementing this such that the
> _flags_to_tags function merges (as a logical OR) the set of flags from
> all filenames for a given email message, and then computes tags from the
> final set. This does assume a particular kind of life-cycle for the
> flags, (that they are additionally unset, and once set for the logical
> message will not want to be cleared again). This lifecycle seems correct
> for things like the flags for "seen" or "replied" as documented in the
> original maildir specification.

Does this mean, that if I want to remove the replied tag (for example) I
can still do it by manipulating flags, i.e. I would remove the R flag
from all messages with the coreposnding Message-ID? Or does it mean that
the only way is to remove the replied tag in notmuch? I'd prefer the
first.

-Michal

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Thu, 11 Nov 2010 16:51:28 +0100, Michal Sojka  wrote:
> Does this mean, that if I want to remove the replied tag (for example) I
> can still do it by manipulating flags, i.e. I would remove the R flag
> from all messages with the coreposnding Message-ID? Or does it mean that
> the only way is to remove the replied tag in notmuch? I'd prefer the
> first.

It is the former, yes. See, for example the following test in the test
suite:

"Removing 'S' flag from existing filename adds 'unread' tag"

Also, here is the latest documentation of
notmuch_message_maildir_flags_to_tags. If this could be made more clear,
please let me know:

/* Add/remove tags according to maildir flags in the message filename(s)
 *
 * This function examines the filenames of 'message' for maildir
 * flags, and adds or removes tags on 'message' as follows when these
 * flags are present:
 *
 *  FlagAction if present
 *  -
 *  'D' Adds the "draft" tag to the message
 *  'F' Adds the "flagged" tag to the message
 *  'P' Adds the "passed" tag to the message
 *  'R' Adds the "replied" tag to the message
 *  'S' Removes the "unread" tag from the message
 *
 * For each flag that is not present, the opposite action (add/remove)
 * is performed for the corresponding tags.
 *
 * Flags are identified as trailing components of the filename after a
 * sequence of ":2,".
 *
 * If there are multiple filenames associated with this message, the
 * flag is considered present if it appears in one or more
 * filenames. (That is, the flags from the multiple filenames are
 * combined with the logical OR operator.)
 *
 * A client can ensure that notmuch database tags remain synchronized
 * with maildir flags by calling this function after each call to
 * notmuch_database_add_message. See also
 * notmuch_message_tags_to_maildir_flags for synchronizing tag changes
 * back to maildir flags.
 */

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Wed, 10 Nov 2010 08:48:54 -0800, Carl Worth  wrote:
> > Great. I've finished the additional tests, which I send as a reply to
> > this mail. Some test are marked as broken because I do not want to touch
> > C sources while you are woking on them.
> 
> Thanks!

I've now got these tests merged in locally, (and updated to work with
the rest of my maildir-sync changes---I'm not reusing filenames like
"msg-003" but instead using filenames associated with particular tests
such as "duplicate-file" to avoid unintended dependencies and side
effects between difference tests).

> I'd like to see a similar test that starts with "some-message:2,RS" and
> then copies it to "some-message". When we add this message would we
> expect the "replied" tag to be removed and the "unread" tag added?

I've added a test for this now. The direction my thoughts are headed is
that adding a file like this will not result in tag changes. See below.

> I'm still trying to work out exactly how the
> notmuch_message_maildir_flags_to_tags function should be documented,
> (whereas notmuch_message_tags_to_maildir_flags seems entirely
> obvious).

I'm thinking of documenting/implementing this such that the
_flags_to_tags function merges (as a logical OR) the set of flags from
all filenames for a given email message, and then computes tags from the
final set. This does assume a particular kind of life-cycle for the
flags, (that they are additionally unset, and once set for the logical
message will not want to be cleared again). This lifecycle seems correct
for things like the flags for "seen" or "replied" as documented in the
original maildir specification.

This behavior might be less appropriate if the flag support were
generalized to a user-customizable set of flag<->tag mappings. But we're
not dealing with this now, so I'll avoid getting bogged down by it.

Further suggestions from anyone are welcome.

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Wed, 10 Nov 2010, Carl Worth wrote:
> > This only fails if the message is in */new and there is no */cur.
> 
> Right. I think that's a little too severe.
> 
> > I do not know if MH format has something special or it is just plain
> > files in plain directories. If the latter, the synchronzation should
> > work unless one of the directories is named 'new'.
> 
> The MH format is plain files in plain directories.
> 
> But in this case, I would contend that we don't _want_ the
> synchronization to work. The files shouldn't be getting renamed at all
> unless we are dealing with maildir.

Yes, I think this could be easily implemented.

> Neither maildir nor mh give us anything extremely reliable that we can
> use to unambiguously distinguish between them. But I think a heuristic
> of:
> 
>   if "cur" and "new" sub-directories exist:
>   then this directory is maildir;
> 
> And only when this heuristic passes should we be fiddling with filenames
> in maildir-specified ways.

Agreed.

> Meanwhile, I ran into one problem with my proposal. We can't use
> notmuch_message_sync_with_maildir_flags since notmuch_message_sync is an
> internal interface. The corresponding public interface actually consists
> of three or four different functions (notmuch_message_add_tag,
> notmuch_message_remove_tag, and notmuch_message_freeze/thaw). I think it
> would be quite crazy to add _with_maildir_flags variants of all of
> those.
> 
> So maybe we will need a new function for the purpose of synchronizing
> the current tags of a message to a maildir filename. So that would be,
> perhaps, notmuch_message_tags_to_maildir_flags or so?

This sounds good and allows us to get rid of
NOTMUCH_MESSAGE_FLAG_TAGS_INVALID.

> Finally, I'm also a bit unsettled about the handling of the "S"
> flag. For all the other flags it is easy to document that
> notmuch_database_add_message_with_maildir_flags simply adds the
> corresponding tag to the message. But we can't say that the presence of
> the "S" tag prevents this function from adding the "unread" tag, since
> this function never does add an "unread" tag.

But can we say the the function _removes_ the unread tag if 'S' is
present and adds it otherwise?

> Instead, the setting of "unread" is taking place at a higher-level,
> (inside "notmuch new"). So perhaps the right answer is for the library
> function to add a "seen" tag, (making the handling of 'T' consistent
> with all other tags and dropping the "inverse" field from the
> table). Then, the "notmuch new" program can query the "seen" tag to
> decide whether it should add its configured new_tags, ("inbox" and
> "unread" by default).
> 
> I do want something like that anyway, because I want to make it so that
> someone that first starts with notmuch and a large collection of maildir
> messages doesn't end up with every message tagged as "inbox" after their
> first run of "notmuch new".

I understand your point but this change would break how I use notmuch
now. My new_tags contains only "new" tag and if this tag is not added
during notmuch new the message would not be properly tagged by my
initial tagging script. I want to avoid the situation that I accidentaly
view a message in a web-mail client, which adds the 'S' flag and this
will lead to exclusion of the message from initial tagging.

If we leave the things the way they are now, it should be easy for the
user to run

  notmuch tag -inbox not tag:unread

We could also show this hint at the end of notmuch new when it is run
for the first time and all messages are tagged by inbox.

> 
> So anyway, I'm currently working on implementing what I described
> above. And I may change my mind slightly as I work through things.
> 
> I'll let you know,

Great. I've finished the additional tests, which I send as a reply to
this mail. Some test are marked as broken because I do not want to touch
C sources while you are woking on them.

-Michal

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Wed, 10 Nov 2010 11:26:40 +0100, Michal Sojka  wrote:
> > So maybe we will need a new function for the purpose of synchronizing
> > the current tags of a message to a maildir filename. So that would be,
> > perhaps, notmuch_message_tags_to_maildir_flags or so?
> 
> This sounds good and allows us to get rid of
> NOTMUCH_MESSAGE_FLAG_TAGS_INVALID.

Yes. And I'm now pursuing simply two new synchronization functions, (as
opposed to the _with_maildir_flags variant of _add_message I proposed
earlier). So the two new functions are:

notmuch_message_maildir_flags_to_tags
and notmuch_message_tags_to_maildir_flags

> But can we say the the function _removes_ the unread tag if 'S' is
> present and adds it otherwise?

I think so. I ended up getting close to this with the code I was writing
yesterday. I eventually had the original (non-maildir-related)
notmuch_database_add_message adding the "unread" tag (which is a new
behavior) and the _add_message_with_maildir_flags removing it if
necessary.

The semantics will be slightly different with the explicit flags_to_tags
function, but I think I'll get this right.

> I understand your point but this change would break how I use notmuch
> now. My new_tags contains only "new" tag and if this tag is not added
> during notmuch new the message would not be properly tagged by my
> initial tagging script.

And of course I found the same problem with my own setup. ;-)

So the tag named configured in [new.tags] will need to be applied
unconditionally to all new messages, (as currently documented). I was
getting concerned when my patches started adding an "unread" tag to new
messages (even without [maildir.synchronize_flags] configuration) and
the existing configuration wouldn't allow a user to prevent that.

But perhaps with the new functions proposed above we can avoid that
problem.

>   notmuch tag -inbox not tag:unread
> 
> We could also show this hint at the end of notmuch new when it is run
> for the first time and all messages are tagged by inbox.

Yes. That might be the right answer.

> Great. I've finished the additional tests, which I send as a reply to
> this mail. Some test are marked as broken because I do not want to touch
> C sources while you are woking on them.

Thanks!

In addition to this test:

   +test_begin_subtest 'Duplicated message is tagged according to the 
duplicate'
   +cp "$MAIL_DIR/cur/msg-003" "$MAIL_DIR/cur/msg-003-dup:2,RS" 

I'd like to see a similar test that starts with "some-message:2,RS" and
then copies it to "some-message". When we add this message would we
expect the "replied" tag to be removed and the "unread" tag added?

I'm still trying to work out exactly how the
notmuch_message_maildir_flags_to_tags function should be documented,
(whereas notmuch_message_tags_to_maildir_flags seems entirely obvious).

Thanks,

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Tue, 09 Nov 2010, Carl Worth wrote:
> The updating I've done here only goes as far as just before "Add a
> message to new/ without info". And it looks like one change I made
> inadvertently broke a later test, so it's expected that "Check that
> removing info did not change tags" currently fails. [And this failure
> shows what I don't like about test_expect_success---if we were using
> test_expect_equal it would be trivial to see what problem I made
> here.]

Did you try ./maildyr-sync -v? In fact, this is what I don't like about
test_begin_subtest. test_begin_subtest does not hide debug output that
goes to stdout and should only be shown with -v. I admit that
test_expect_success is not ideal though. Without -v, it should
automatically show stdout when a test fails. I'll send patch for this in
another mail.

> 
> Finally, when going through these tests I saw:
> 
>   "Removing of unread tag should fail without cur/"
> 
> And that's behavior I do not want. Adding and removing tags should be
> reliable whether or not the maildir synchronization can succeed. In this
> specific case, the right answer is probably to say that a directory
> without "new" and "cur" is not a maildir so no synchronization should be
> done.

This only fails if the message is in */new and there is no */cur.

> Notmuch does need to be able to support things like mh format still. Do
> the current patches break that by doing maildir-style renaming in
> non-maildir directories?

I do not know if MH format has something special or it is just plain
files in plain directories. If the latter, the synchronzation should
work unless one of the directories is named 'new'. See the tests with
fakenew directory [fakenew is probably not the most obvious name for a
simple non-maildir directory].

> If so, we'll need to fix that as well. And that might require an
> "is_maildir" term to be stored for directory documents in the
> database.
> 
> Again, that's something I can help with.

If you think that what we have now is not sufficient, I'd need some help
with this.

-Michal

P.S. Because of my work on test suite, I didn't do any additional tests
for maildir synchronization, so maybe tomorrow :-(

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Wed, 10 Nov 2010 11:26:40 +0100, Michal Sojka sojk...@fel.cvut.cz wrote:
  So maybe we will need a new function for the purpose of synchronizing
  the current tags of a message to a maildir filename. So that would be,
  perhaps, notmuch_message_tags_to_maildir_flags or so?
 
 This sounds good and allows us to get rid of
 NOTMUCH_MESSAGE_FLAG_TAGS_INVALID.

Yes. And I'm now pursuing simply two new synchronization functions, (as
opposed to the _with_maildir_flags variant of _add_message I proposed
earlier). So the two new functions are:

notmuch_message_maildir_flags_to_tags
and notmuch_message_tags_to_maildir_flags

 But can we say the the function _removes_ the unread tag if 'S' is
 present and adds it otherwise?

I think so. I ended up getting close to this with the code I was writing
yesterday. I eventually had the original (non-maildir-related)
notmuch_database_add_message adding the unread tag (which is a new
behavior) and the _add_message_with_maildir_flags removing it if
necessary.

The semantics will be slightly different with the explicit flags_to_tags
function, but I think I'll get this right.

 I understand your point but this change would break how I use notmuch
 now. My new_tags contains only new tag and if this tag is not added
 during notmuch new the message would not be properly tagged by my
 initial tagging script.

And of course I found the same problem with my own setup. ;-)

So the tag named configured in [new.tags] will need to be applied
unconditionally to all new messages, (as currently documented). I was
getting concerned when my patches started adding an unread tag to new
messages (even without [maildir.synchronize_flags] configuration) and
the existing configuration wouldn't allow a user to prevent that.

But perhaps with the new functions proposed above we can avoid that
problem.

   notmuch tag -inbox not tag:unread
 
 We could also show this hint at the end of notmuch new when it is run
 for the first time and all messages are tagged by inbox.

Yes. That might be the right answer.

 Great. I've finished the additional tests, which I send as a reply to
 this mail. Some test are marked as broken because I do not want to touch
 C sources while you are woking on them.

Thanks!

In addition to this test:

   +test_begin_subtest 'Duplicated message is tagged according to the 
duplicate'
   +cp $MAIL_DIR/cur/msg-003 $MAIL_DIR/cur/msg-003-dup:2,RS 

I'd like to see a similar test that starts with some-message:2,RS and
then copies it to some-message. When we add this message would we
expect the replied tag to be removed and the unread tag added?

I'm still trying to work out exactly how the
notmuch_message_maildir_flags_to_tags function should be documented,
(whereas notmuch_message_tags_to_maildir_flags seems entirely obvious).

Thanks,

-Carl

-- 
carl.d.wo...@intel.com


pgpPwGFw2teI6.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Wed, 10 Nov 2010 00:39:31 +0100, Michal Sojka  wrote:
> Did you try ./maildyr-sync -v?

Ah, no. That's always seemed really noisy. What I want is simply to see
the information related to a failure *when a failure occurs*, and not a
lot of noise, (nor having to re-run with extra options to see the output
I want).

> test_expect_success is not ideal though. Without -v, it should
> automatically show stdout when a test fails. I'll send patch for this in
> another mail.

Cool. I'll look for that.

> This only fails if the message is in */new and there is no */cur.

Right. I think that's a little too severe.

> I do not know if MH format has something special or it is just plain
> files in plain directories. If the latter, the synchronzation should
> work unless one of the directories is named 'new'.

The MH format is plain files in plain directories.

But in this case, I would contend that we don't _want_ the
synchronization to work. The files shouldn't be getting renamed at all
unless we are dealing with maildir.

Neither maildir nor mh give us anything extremely reliable that we can
use to unambiguously distinguish between them. But I think a heuristic
of:

if "cur" and "new" sub-directories exist:
then this directory is maildir;

And only when this heuristic passes should we be fiddling with filenames
in maildir-specified ways.

> If you think that what we have now is not sufficient, I'd need some help
> with this.

I'll look.

> P.S. Because of my work on test suite, I didn't do any additional tests
> for maildir synchronization, so maybe tomorrow :-(

No problem. I appreciate your help improving the test-suite
infrastructure!

Meanwhile, I ran into one problem with my proposal. We can't use
notmuch_message_sync_with_maildir_flags since notmuch_message_sync is an
internal interface. The corresponding public interface actually consists
of three or four different functions (notmuch_message_add_tag,
notmuch_message_remove_tag, and notmuch_message_freeze/thaw). I think it
would be quite crazy to add _with_maildir_flags variants of all of
those.

So maybe we will need a new function for the purpose of synchronizing
the current tags of a message to a maildir filename. So that would be,
perhaps, notmuch_message_tags_to_maildir_flags or so?

Finally, I'm also a bit unsettled about the handling of the "S"
flag. For all the other flags it is easy to document that
notmuch_database_add_message_with_maildir_flags simply adds the
corresponding tag to the message. But we can't say that the presence of
the "S" tag prevents this function from adding the "unread" tag, since
this function never does add an "unread" tag.

Instead, the setting of "unread" is taking place at a higher-level,
(inside "notmuch new"). So perhaps the right answer is for the library
function to add a "seen" tag, (making the handling of 'T' consistent
with all other tags and dropping the "inverse" field from the
table). Then, the "notmuch new" program can query the "seen" tag to
decide whether it should add its configured new_tags, ("inbox" and
"unread" by default).

I do want something like that anyway, because I want to make it so that
someone that first starts with notmuch and a large collection of maildir
messages doesn't end up with every message tagged as "inbox" after their
first run of "notmuch new".

So anyway, I'm currently working on implementing what I described
above. And I may change my mind slightly as I work through things.

I'll let you know,

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Tue, 09 Nov 2010 11:06:30 +0100, Michal Sojka  wrote:
> This sounds good. Still it will be neccessary to synchronize with all
> files, not only the first one.

OK. I'll add that to the list of things I'll fix up.

> > I'd like to get things merged today, so I plan to take your patches and
> > then add new commits on top to implement the functions I described
> > above.
> 
> Great! Seems that it is still not finished and I do not want to do
> duplicite work, so let me know what is the current status and whether
> you need some help from me (e.g. tests for multiple messages with same
> ID). I should be on IRC today.

Yesterday ended up being very busy, but I'm working on this stuff
now. My plan is to merge it and then release notmuch 0.5. (And yes, if I
had a more strict release manager then 0.5 would have been released
yesterday as-is---but it turns out my release manager had an equally
busy day yesterday).

If you wanted to write some more tests, then that would be very useful.

I've been changing the way the tests are written, so you might want to
look at the attached file to see what I'm doing and match it. Notable
differences:

* Using test_begin_subtest and test_expect_equal rather than the
  list-of-commands to test_expect_success.

* Not emitting separate line-item results for things that are
  already tested in other scripts, (like "add message" and
  "search for message", etc.)

The updating I've done here only goes as far as just before "Add a
message to new/ without info". And it looks like one change I made
inadvertently broke a later test, so it's expected that "Check that
removing info did not change tags" currently fails. [And this failure
shows what I don't like about test_expect_success---if we were using
test_expect_equal it would be trivial to see what problem I made here.]

Finally, when going through these tests I saw:

"Removing of unread tag should fail without cur/"

And that's behavior I do not want. Adding and removing tags should be
reliable whether or not the maildir synchronization can succeed. In this
specific case, the right answer is probably to say that a directory
without "new" and "cur" is not a maildir so no synchronization should be
done.

Notmuch does need to be able to support things like mh format still. Do
the current patches break that by doing maildir-style renaming in
non-maildir directories? If so, we'll need to fix that as well. And that
might require an "is_maildir" term to be stored for directory documents
in the database.

Again, that's something I can help with.

Thanks again,

-Carl

-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

-- next part --
An embedded and charset-unspecified text was scrubbed...
Name: maildir-sync
URL: 

-- next part --

-- 
carl.d.worth at intel.com

[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Mon, 08 Nov 2010, Carl Worth wrote:
> On Sun, 07 Nov 2010 02:46:08 +0100, Michal Sojka  
> wrote:
> > The current implementation renames only the file whose name is stored
> > first in the database. I have a TODO comment there to add a loop through
> > all file names, but I have never realized that deleted flag could be so
> > dangerous.
> 
> I think what I'd like to do for now is to simply remove "deleted" from
> the set of tags being manipulated by this support. 

This sounds good. Still it will be neccessary to synchronize with all
files, not only the first one. Currently, I experience a problem with
messages sent by myself to a list. The S flag is sometimes added to
the message in sent folder insted of to the message in inbox.

> This is the similar to what Sebastian decided to do with notmuchsync
> when he described in detail the same scenario I was concerned with:
> 
>   id:87eickhor1.fsf at SSpaeth.de
> 
> Sebastian's solution was slightly different, ("deleted" was not
> synchronized by default but could be explicitly requested). I propose
> simply not synchronizing "deleted" until it can be made safe.
> 
> > It will take me probably a few days until I find time to work on this.
> > So let me now in that time whether you have some preferences in the
> > above.
> 
> I'd like to get things merged today, so I plan to take your patches and
> then add new commits on top to implement the functions I described
> above.

Great! Seems that it is still not finished and I do not want to do
duplicite work, so let me know what is the current status and whether
you need some help from me (e.g. tests for multiple messages with same
ID). I should be on IRC today.

-Michal

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Tue, 09 Nov 2010, Carl Worth wrote:
 The updating I've done here only goes as far as just before Add a
 message to new/ without info. And it looks like one change I made
 inadvertently broke a later test, so it's expected that Check that
 removing info did not change tags currently fails. [And this failure
 shows what I don't like about test_expect_success---if we were using
 test_expect_equal it would be trivial to see what problem I made
 here.]

Did you try ./maildyr-sync -v? In fact, this is what I don't like about
test_begin_subtest. test_begin_subtest does not hide debug output that
goes to stdout and should only be shown with -v. I admit that
test_expect_success is not ideal though. Without -v, it should
automatically show stdout when a test fails. I'll send patch for this in
another mail.

 
 Finally, when going through these tests I saw:
 
   Removing of unread tag should fail without cur/
 
 And that's behavior I do not want. Adding and removing tags should be
 reliable whether or not the maildir synchronization can succeed. In this
 specific case, the right answer is probably to say that a directory
 without new and cur is not a maildir so no synchronization should be
 done.

This only fails if the message is in */new and there is no */cur.

 Notmuch does need to be able to support things like mh format still. Do
 the current patches break that by doing maildir-style renaming in
 non-maildir directories?

I do not know if MH format has something special or it is just plain
files in plain directories. If the latter, the synchronzation should
work unless one of the directories is named 'new'. See the tests with
fakenew directory [fakenew is probably not the most obvious name for a
simple non-maildir directory].

 If so, we'll need to fix that as well. And that might require an
 is_maildir term to be stored for directory documents in the
 database.
 
 Again, that's something I can help with.

If you think that what we have now is not sufficient, I'd need some help
with this.

-Michal

P.S. Because of my work on test suite, I didn't do any additional tests
for maildir synchronization, so maybe tomorrow :-(
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Sun, 07 Nov 2010 02:46:08 +0100, Michal Sojka  wrote:
> On Thu, 04 Nov 2010, Carl Worth wrote:
> > I'm not entirely sure I like a big, global state-changing function like
> > that in the library. But if we do want to have that, we need to fix the
> > documentation of all functions that are affected by it to correctly
> > document their current behavior.
> 
> I can imagine two other solutions. One would be to add a parameter
> (probably called flags) to the following functions:
> 
> notmuch_message_add_tag()
> notmuch_message_remove_tag()
> notmuch_message_thaw()
...
> The other option would be to put a flag into notmuch_message_t but this
> is probably not very different from having it in notmuch_database_t.

Here's yet another approach that I have in mind.

We can implement all of the support by adding two new library functions:

notmuch_database_add_message_with_maildir_flags
notmuch_message_sync_with_maildir_flags

These functions would work like the existing
notmuch_database_add_message and notmuch_message_sync except they would
do the maildir-flag synchronization.

This allows a user of the library to do whatever it wants, (no
synchronization, one-way synchronization in either direction, or two-way
synchronization), without having any sort of "configuration" API calls
in the library.

> I think I could make notmuch_message_maildir_to_flags() private and call
> it from notmuch_database_add_message() so that both directions will be
> implemented in the library.

Yes. That's in line with what I propose above.

> The current implementation renames only the file whose name is stored
> first in the database. I have a TODO comment there to add a loop through
> all file names, but I have never realized that deleted flag could be so
> dangerous.

I think what I'd like to do for now is to simply remove "deleted" from
the set of tags being manipulated by this support. This is the similar
to what Sebastian decided to do with notmuchsync when he described in
detail the same scenario I was concerned with:

id:87eickhor1.fsf at SSpaeth.de

Sebastian's solution was slightly different, ("deleted" was not
synchronized by default but could be explicitly requested). I propose
simply not synchronizing "deleted" until it can be made safe.

> It will take me probably a few days until I find time to work on this.
> So let me now in that time whether you have some preferences in the
> above.

I'd like to get things merged today, so I plan to take your patches and
then add new commits on top to implement the functions I described
above.

I'll be interested in any feedback.

Thanks again,

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Sun, 07 Nov 2010 02:46:08 +0100, Michal Sojka sojk...@fel.cvut.cz wrote:
 On Thu, 04 Nov 2010, Carl Worth wrote:
  I'm not entirely sure I like a big, global state-changing function like
  that in the library. But if we do want to have that, we need to fix the
  documentation of all functions that are affected by it to correctly
  document their current behavior.
 
 I can imagine two other solutions. One would be to add a parameter
 (probably called flags) to the following functions:
 
 notmuch_message_add_tag()
 notmuch_message_remove_tag()
 notmuch_message_thaw()
...
 The other option would be to put a flag into notmuch_message_t but this
 is probably not very different from having it in notmuch_database_t.

Here's yet another approach that I have in mind.

We can implement all of the support by adding two new library functions:

notmuch_database_add_message_with_maildir_flags
notmuch_message_sync_with_maildir_flags

These functions would work like the existing
notmuch_database_add_message and notmuch_message_sync except they would
do the maildir-flag synchronization.

This allows a user of the library to do whatever it wants, (no
synchronization, one-way synchronization in either direction, or two-way
synchronization), without having any sort of configuration API calls
in the library.

 I think I could make notmuch_message_maildir_to_flags() private and call
 it from notmuch_database_add_message() so that both directions will be
 implemented in the library.

Yes. That's in line with what I propose above.

 The current implementation renames only the file whose name is stored
 first in the database. I have a TODO comment there to add a loop through
 all file names, but I have never realized that deleted flag could be so
 dangerous.

I think what I'd like to do for now is to simply remove deleted from
the set of tags being manipulated by this support. This is the similar
to what Sebastian decided to do with notmuchsync when he described in
detail the same scenario I was concerned with:

id:87eickhor1@sspaeth.de

Sebastian's solution was slightly different, (deleted was not
synchronized by default but could be explicitly requested). I propose
simply not synchronizing deleted until it can be made safe.

 It will take me probably a few days until I find time to work on this.
 So let me now in that time whether you have some preferences in the
 above.

I'd like to get things merged today, so I plan to take your patches and
then add new commits on top to implement the functions I described
above.

I'll be interested in any feedback.

Thanks again,

-Carl

-- 
carl.d.wo...@intel.com


pgpAO0B8AoSxp.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Thu, 04 Nov 2010, Carl Worth wrote:
> Meanwhile, here are some of the things I'm still thinking about in
> regards to this patch. First, the commit message describes the
> synchronization happening at "notmuch new" and "notmuch tag/notmuch
> restore". But the implementation shows that the functionality is within
> the library, not the command-line tool above it.
> 
> I think having this at the library makes sense, but as you certainly
> noticed, the library has historically been entirely unaware of any
> configuration, (which I'd like to keep). Obviously, you maintained that
> separation in your patch series, but you added a new
> notmuch_database_set_maildir_sync function so that the library can be
> informed of the desired behavior.
> 
> I'm not entirely sure I like a big, global state-changing function like
> that in the library. But if we do want to have that, we need to fix the
> documentation of all functions that are affected by it to correctly
> document their current behavior.

I can imagine two other solutions. One would be to add a parameter
(probably called flags) to the following functions:

notmuch_message_add_tag()
notmuch_message_remove_tag()
notmuch_message_thaw()

Since you want to keep ABI this would require to implement second
versions of these functions to keep backward compatibility.

The other option would be to put a flag into notmuch_message_t but this
is probably not very different from having it in notmuch_database_t.

> Also, the synchronization is inherently 2-way, but the two directions
> are implemented differently in the library. One direction, (from tags to
> maildir flags), is implemented implicitly in the library (existing
> functions do the new synchronization under the influence of
> database_set_maildir_sync). But the other direction, (from maildir flags
> to tags), requires an explicit call to a new function
> (notmuch_message_maildir_to_flags). I definitely don't like this.

I think I could make notmuch_message_maildir_to_flags() private and call
it from notmuch_database_add_message() so that both directions will be
implemented in the library.

> Finally, the documentation for notmuch_message_maildir_to_tags ("Add or
> remove tags based on the maildir flags in the file name")
> inadequate. This documentation needs to say which tags are added/removed
> in what conditions. It should also give guidance on when this function
> should be called in order to achieve some particular behavior.
> 
> I recognize that in the above I don't give specific guidance on whether
> the new functionality should be implicit or explicit in the
> library. I'm not certain which is better, and I'm willing to listen to
> justification from someone who has spent some time implementing and
> testing this stuff. But I don't like the current mixed state.

I found the following what I'd like the most: Do not export
notmuch_message_maildir_to_tags() from the library and call it
implicitly from notmuch_database_add_message(). Keep
notmuch_database_set_maildir_sync() and update the documentation of the
affected functions. This sounds to me better than adding explicit flags
parameters to enable/disable synchronization to all of the affected
functions. The additional parameters would make the API harder to use.

> One other issue, how does this support deal with multiple files that
> have the same message ID (and hence a single record in the database)?
> Some bad failure modes I can imagine are cycling of tags/filenames with
> successive runs of notmuch new, or "leaking"[*] of tags from one filename
> to another through the database.
> 
> [*] Imagine, for example, someone using an external client that
> identifies duplicate messages in the mailstore and adds the maildir flag
> corresponding to "deleted" to all but one of each of the
> duplicates. There's then the possibility that notmuch could propagate
> this "deleted" flag through the "deleted" tag in the database, (perhaps
> after a notmuch dump/restore cycle). And that could be a catastrophic
> result if all messages that have duplicates get flagged for deletion!
> 
> What thoughts do you have on this multiple-file issue?

The current implementation renames only the file whose name is stored
first in the database. I have a TODO comment there to add a loop through
all file names, but I have never realized that deleted flag could be so
dangerous.

I will need to extend the test suite for tests with multiple messages
having the same id and during that work I'll hopefully find some sane
solution.

It will take me probably a few days until I find time to work on this.
So let me now in that time whether you have some preferences in the
above.

-Michal

Re: [PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

On Thu, 04 Nov 2010, Carl Worth wrote:
 Meanwhile, here are some of the things I'm still thinking about in
 regards to this patch. First, the commit message describes the
 synchronization happening at notmuch new and notmuch tag/notmuch
 restore. But the implementation shows that the functionality is within
 the library, not the command-line tool above it.
 
 I think having this at the library makes sense, but as you certainly
 noticed, the library has historically been entirely unaware of any
 configuration, (which I'd like to keep). Obviously, you maintained that
 separation in your patch series, but you added a new
 notmuch_database_set_maildir_sync function so that the library can be
 informed of the desired behavior.
 
 I'm not entirely sure I like a big, global state-changing function like
 that in the library. But if we do want to have that, we need to fix the
 documentation of all functions that are affected by it to correctly
 document their current behavior.

I can imagine two other solutions. One would be to add a parameter
(probably called flags) to the following functions:

notmuch_message_add_tag()
notmuch_message_remove_tag()
notmuch_message_thaw()

Since you want to keep ABI this would require to implement second
versions of these functions to keep backward compatibility.

The other option would be to put a flag into notmuch_message_t but this
is probably not very different from having it in notmuch_database_t.

 Also, the synchronization is inherently 2-way, but the two directions
 are implemented differently in the library. One direction, (from tags to
 maildir flags), is implemented implicitly in the library (existing
 functions do the new synchronization under the influence of
 database_set_maildir_sync). But the other direction, (from maildir flags
 to tags), requires an explicit call to a new function
 (notmuch_message_maildir_to_flags). I definitely don't like this.

I think I could make notmuch_message_maildir_to_flags() private and call
it from notmuch_database_add_message() so that both directions will be
implemented in the library.
 
 Finally, the documentation for notmuch_message_maildir_to_tags (Add or
 remove tags based on the maildir flags in the file name)
 inadequate. This documentation needs to say which tags are added/removed
 in what conditions. It should also give guidance on when this function
 should be called in order to achieve some particular behavior.
 
 I recognize that in the above I don't give specific guidance on whether
 the new functionality should be implicit or explicit in the
 library. I'm not certain which is better, and I'm willing to listen to
 justification from someone who has spent some time implementing and
 testing this stuff. But I don't like the current mixed state.

I found the following what I'd like the most: Do not export
notmuch_message_maildir_to_tags() from the library and call it
implicitly from notmuch_database_add_message(). Keep
notmuch_database_set_maildir_sync() and update the documentation of the
affected functions. This sounds to me better than adding explicit flags
parameters to enable/disable synchronization to all of the affected
functions. The additional parameters would make the API harder to use.

 One other issue, how does this support deal with multiple files that
 have the same message ID (and hence a single record in the database)?
 Some bad failure modes I can imagine are cycling of tags/filenames with
 successive runs of notmuch new, or leaking[*] of tags from one filename
 to another through the database.
 
 [*] Imagine, for example, someone using an external client that
 identifies duplicate messages in the mailstore and adds the maildir flag
 corresponding to deleted to all but one of each of the
 duplicates. There's then the possibility that notmuch could propagate
 this deleted flag through the deleted tag in the database, (perhaps
 after a notmuch dump/restore cycle). And that could be a catastrophic
 result if all messages that have duplicates get flagged for deletion!
 
 What thoughts do you have on this multiple-file issue?

The current implementation renames only the file whose name is stored
first in the database. I have a TODO comment there to add a loop through
all file names, but I have never realized that deleted flag could be so
dangerous.

I will need to extend the test suite for tests with multiple messages
having the same id and during that work I'll hopefully find some sane
solution.

It will take me probably a few days until I find time to work on this.
So let me now in that time whether you have some preferences in the
above.

-Michal
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Sun, 31 Oct 2010 22:29:14 +0100, Michal Sojka  wrote:
> This is the next iteration of maildir synchronization patches. The
> changes are:
> - Configuration is now simplified. The synchronization can only be
>   full enabled or disabled. By default it is still disabled.
> - Added test for notmuch restore (with enabled synchronization)
> - Rebased to the current master
> - 'D' flag is mapped to 'deleted' tag instead of 'delete' (this has
>   already been in v3)

This is coming along nicely, Michal. I'm delighted to see the testing
here and the improved configuration support.

I think after merging this, I'd be inclined to change two tiny things:

1. Enable it by default

2. Augment the documentation in the configuration file to say
   exactly which tags are synchronized with which flags.

I could easily do those in follow-up commits, so there's no need to
worry about resending the series for those.

Meanwhile, here are some of the things I'm still thinking about in
regards to this patch. First, the commit message describes the
synchronization happening at "notmuch new" and "notmuch tag/notmuch
restore". But the implementation shows that the functionality is within
the library, not the command-line tool above it.

I think having this at the library makes sense, but as you certainly
noticed, the library has historically been entirely unaware of any
configuration, (which I'd like to keep). Obviously, you maintained that
separation in your patch series, but you added a new
notmuch_database_set_maildir_sync function so that the library can be
informed of the desired behavior.

I'm not entirely sure I like a big, global state-changing function like
that in the library. But if we do want to have that, we need to fix the
documentation of all functions that are affected by it to correctly
document their current behavior.

Also, the synchronization is inherently 2-way, but the two directions
are implemented differently in the library. One direction, (from tags to
maildir flags), is implemented implicitly in the library (existing
functions do the new synchronization under the influence of
database_set_maildir_sync). But the other direction, (from maildir flags
to tags), requires an explicit call to a new function
(notmuch_message_maildir_to_flags). I definitely don't like this.

Finally, the documentation for notmuch_message_maildir_to_tags ("Add or
remove tags based on the maildir flags in the file name")
inadequate. This documentation needs to say which tags are added/removed
in what conditions. It should also give guidance on when this function
should be called in order to achieve some particular behavior.

I recognize that in the above I don't give specific guidance on whether
the new functionality should be implicit or explicit in the
library. I'm not certain which is better, and I'm willing to listen to
justification from someone who has spent some time implementing and
testing this stuff. But I don't like the current mixed state.

One other issue, how does this support deal with multiple files that
have the same message ID (and hence a single record in the database)?
Some bad failure modes I can imagine are cycling of tags/filenames with
successive runs of notmuch new, or "leaking"[*] of tags from one filename
to another through the database.

[*] Imagine, for example, someone using an external client that
identifies duplicate messages in the mailstore and adds the maildir flag
corresponding to "deleted" to all but one of each of the
duplicates. There's then the possibility that notmuch could propagate
this "deleted" flag through the "deleted" tag in the database, (perhaps
after a notmuch dump/restore cycle). And that could be a catastrophic
result if all messages that have duplicates get flagged for deletion!

What thoughts do you have on this multiple-file issue?

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [PATCH v4 0/4] Maildir synchronization

[Carl Worth]

On Sun, 31 Oct 2010 22:29:14 +0100, Michal Sojka sojk...@fel.cvut.cz wrote:
 This is the next iteration of maildir synchronization patches. The
 changes are:
 - Configuration is now simplified. The synchronization can only be
   full enabled or disabled. By default it is still disabled.
 - Added test for notmuch restore (with enabled synchronization)
 - Rebased to the current master
 - 'D' flag is mapped to 'deleted' tag instead of 'delete' (this has
   already been in v3)

This is coming along nicely, Michal. I'm delighted to see the testing
here and the improved configuration support.

I think after merging this, I'd be inclined to change two tiny things:

1. Enable it by default

2. Augment the documentation in the configuration file to say
   exactly which tags are synchronized with which flags.

I could easily do those in follow-up commits, so there's no need to
worry about resending the series for those.

Meanwhile, here are some of the things I'm still thinking about in
regards to this patch. First, the commit message describes the
synchronization happening at notmuch new and notmuch tag/notmuch
restore. But the implementation shows that the functionality is within
the library, not the command-line tool above it.

I think having this at the library makes sense, but as you certainly
noticed, the library has historically been entirely unaware of any
configuration, (which I'd like to keep). Obviously, you maintained that
separation in your patch series, but you added a new
notmuch_database_set_maildir_sync function so that the library can be
informed of the desired behavior.

I'm not entirely sure I like a big, global state-changing function like
that in the library. But if we do want to have that, we need to fix the
documentation of all functions that are affected by it to correctly
document their current behavior.

Also, the synchronization is inherently 2-way, but the two directions
are implemented differently in the library. One direction, (from tags to
maildir flags), is implemented implicitly in the library (existing
functions do the new synchronization under the influence of
database_set_maildir_sync). But the other direction, (from maildir flags
to tags), requires an explicit call to a new function
(notmuch_message_maildir_to_flags). I definitely don't like this.

Finally, the documentation for notmuch_message_maildir_to_tags (Add or
remove tags based on the maildir flags in the file name)
inadequate. This documentation needs to say which tags are added/removed
in what conditions. It should also give guidance on when this function
should be called in order to achieve some particular behavior.

I recognize that in the above I don't give specific guidance on whether
the new functionality should be implicit or explicit in the
library. I'm not certain which is better, and I'm willing to listen to
justification from someone who has spent some time implementing and
testing this stuff. But I don't like the current mixed state.

One other issue, how does this support deal with multiple files that
have the same message ID (and hence a single record in the database)?
Some bad failure modes I can imagine are cycling of tags/filenames with
successive runs of notmuch new, or leaking[*] of tags from one filename
to another through the database.

[*] Imagine, for example, someone using an external client that
identifies duplicate messages in the mailstore and adds the maildir flag
corresponding to deleted to all but one of each of the
duplicates. There's then the possibility that notmuch could propagate
this deleted flag through the deleted tag in the database, (perhaps
after a notmuch dump/restore cycle). And that could be a catastrophic
result if all messages that have duplicates get flagged for deletion!

What thoughts do you have on this multiple-file issue?

-Carl

-- 
carl.d.wo...@intel.com


pgpnLRjHg8Q4g.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v4 0/4] Maildir synchronization

[Michal Sojka]

This is the next iteration of maildir synchronization patches. The
changes are:
- Configuration is now simplified. The synchronization can only be
  full enabled or disabled. By default it is still disabled.
- Added test for notmuch restore (with enabled synchronization)
- Rebased to the current master
- 'D' flag is mapped to 'deleted' tag instead of 'delete' (this has
  already been in v3)

Can be pulled by
git pull git://rtime.felk.cvut.cz/notmuch tags/maildir-sync-v4

Michal Sojka (4):
  lib: Return added message even if it already was in the database
  Maildir synchronization
  Make maildir synchronization configurable
  Tests for maildir synchronization

 lib/database-private.h |2 +-
 lib/database.cc|   19 -
 lib/message.cc |  226 ++
 lib/notmuch-private.h  |4 +
 lib/notmuch.h  |   16 +++-
 notmuch-client.h   |7 ++
 notmuch-config.c   |   49 ++
 notmuch-new.c  |7 ++-
 notmuch-restore.c  |2 +
 notmuch-setup.c|   10 ++
 notmuch-tag.c  |2 +
 test/maildir-sync  |  231 
 test/notmuch-test  |3 +-
 test/test-lib.sh   |   14 +++-
 14 files changed, 585 insertions(+), 7 deletions(-)
 create mode 100755 test/maildir-sync