upas/fs plumb modify for self-made flag changes

upas/fs plumb modify for self-made flag changes

[theinicke]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

Currently upas/fs plumbs modify messages only if the flag changes are made by another imap connection. If the flag changes are made within the running upas/fs no modify message is plumbed.

Assuming this is not intentionally and we want to get a modify plumb message after flags were changed in running upas/fs the attached patch fixes this by making sure to send a modify for pending changes upon mbox sync.

The modify could be plumbed earlier, but this way it costs us almost nothing and changed flags from outside upas/fs are noticed in the same time interval anyway.

--
Tobias Heinicke

[-- Attachment #2.1: Type: text/plain, Size: 326 bytes --]

from postmaster@9pi:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Disposition: attachment; filename=upasfs.patch
	Content-Type: text/plain; charset="US-ASCII"
	Content-Transfer-Encoding: 7bit

[-- Attachment #2.2: upasfs.patch.suspect --]
[-- Type: application/octet-stream, Size: 306 bytes --]

diff -r 8857059a1805 sys/src/cmd/upas/fs/fs.c
--- a/sys/src/cmd/upas/fs/fs.c	Fri Jan 31 09:25:39 2020 -0800
+++ b/sys/src/cmd/upas/fs/fs.c	Sat Feb 01 13:22:38 2020 +0100
@@ -1197,6 +1197,7 @@
 			mb->modflags(mb, m, f);
 		m->flags = f;
 		m->cstate |= Cidxstale;
+		m->cstate |= Cmod;
 	}
 	return nil;
 }

Re: [9front] upas/fs plumb modify for self-made flag changes

[ori]

> Currently upas/fs plumbs modify messages only if the flag changes are made by another imap connection. If the flag changes are made within the running upas/fs no modify message is plumbed.
> 
> Assuming this is not intentionally and we want to get a modify plumb message after flags were changed in running upas/fs the attached patch fixes this by making sure to send a modify for pending changes upon mbox sync.
> 
> The modify could be plumbed earlier, but this way it costs us almost nothing and changed flags from outside upas/fs are noticed in the same time interval anyway.
> 
> --
> Tobias Heinicke

This is probably the right thing to do.

Acme Mail both modifies messages and watches for modification
notifications. Have you made sure sure that it does the right
thing in all cases?

Re: [9front] upas/fs plumb modify for self-made flag changes

[theinicke]

Thanks for your reply and sorry for the noise related to not inlining the patch.

> This is probably the right thing to do.

Glad to hear this.

> Acme Mail both modifies messages and watches for modification
> notifications. Have you made sure sure that it does the right
> thing in all cases?

I have tested it; even with multiple acme Mail instances running at the same time, it did the right thing for all the cases I have tried.
Having this in my tree for more than a week and using acme Mail as my daily driver I am fairly confident, that this does not introduce any regression.

--
Tobias Heinicke

Re: [9front] upas/fs plumb modify for self-made flag changes

[ori]

> Thanks for your reply and sorry for the noise related to not inlining the patch.
> 
>> This is probably the right thing to do.
> 
> Glad to hear this.
> 
>> Acme Mail both modifies messages and watches for modification
>> notifications. Have you made sure sure that it does the right
>> thing in all cases?
> 
> I have tested it; even with multiple acme Mail instances running at the same time, it did the right thing for all the cases I have tried.
> Having this in my tree for more than a week and using acme Mail as my daily driver I am fairly confident, that this does not introduce any regression.
> 

Sorry for the slow response -- I'm looking again,
and trying to decide if there's a possible bug
here, which existed before your change:

		if(mb->modflags != nil)
			mb->modflags(mb, m, f);
		m->flags = f;
		m->cstate |= Cidxstale;

In imap4modflags, we have:

		imap4cmd(imap, "uid store %lud flags (%s)", (ulong)m->imapuid, buf);
		imap4resp0(imap, mb, m);

imap4resp0 may modfiy the flags to match what's on the
server. I think we want the server to have the final word
after we set the flags, so we want to set them first:

		m->flags = f;
		m->cstate |= Cidxstale;
		m->cstate |= Cmod;
		if(mb->modflags != nil)
			mb->modflags(mb, m, f);

Does this sound right?

Re: [9front] upas/fs plumb modify for self-made flag changes

[theinicke]

> Sorry for the slow response -- I'm looking again,
> and trying to decide if there's a possible bug
> here, which existed before your change:
> 
> 		if(mb->modflags != nil)
> 			mb->modflags(mb, m, f);
> 		m->flags = f;
> 		m->cstate |= Cidxstale;
> 
> In imap4modflags, we have:
> 
> 		imap4cmd(imap, "uid store %lud flags (%s)", (ulong)m->imapuid, buf);
> 		imap4resp0(imap, mb, m);
> 
> imap4resp0 may modfiy the flags to match what's on the
> server. I think we want the server to have the final word
> after we set the flags, so we want to set them first:
> 
> 		m->flags = f;
> 		m->cstate |= Cidxstale;
> 		m->cstate |= Cmod;
> 		if(mb->modflags != nil)
> 			mb->modflags(mb, m, f);
> 
> Does this sound right?

Yes you are right, this looks good to me.
I have applied this to my tree and it works as expected.
Thank you - please commit this change, if no one objects.

Re: [9front] upas/fs plumb modify for self-made flag changes

[ori]

> Yes you are right, this looks good to me.
> I have applied this to my tree and it works as expected.
> Thank you - please commit this change, if no one objects.

Ok, committed. Thanks!

Re: [9front] upas/fs plumb modify for self-made flag changes

[Lyndon Nerenberg]

ori@eigenstate.org writes:

> imap4resp0 may modfiy the flags to match what's on the
> server. I think we want the server to have the final word
> after we set the flags, so we want to set them first:

That's correct.  You have to think of an IMAP session as an
all-encompassing state machine.  Everything an IMAP client sends
to a server is merely a suggestion.  Nothing changes until the
server sends something to the client saying a change has taken
place.

E.g., the client cannot mark a message as "deleted" unless and until
the server sends it something stating the \Deleted flag has been
set on that message, no matter how many times the client has tried
setting that flag.

--lyndon

Re: [9front] upas/fs plumb modify for self-made flag changes

[Stanley Lieber]

On February 5, 2020 5:16:03 PM EST, Lyndon Nerenberg <lyndon@orthanc.ca> wrote:
>ori@eigenstate.org writes:
>
>> imap4resp0 may modfiy the flags to match what's on the
>> server. I think we want the server to have the final word
>> after we set the flags, so we want to set them first:
>
>That's correct.  You have to think of an IMAP session as an
>all-encompassing state machine.  Everything an IMAP client sends
>to a server is merely a suggestion.  Nothing changes until the
>server sends something to the client saying a change has taken
>place.
>
>E.g., the client cannot mark a message as "deleted" unless and until
>the server sends it something stating the \Deleted flag has been
>set on that message, no matter how many times the client has tried
>setting that flag.
>
>--lyndon

don't i know it.

sl

Re: [9front] upas/fs plumb modify for self-made flag changes

[Stanley Lieber]

On February 5, 2020 5:16:03 PM EST, Lyndon Nerenberg <lyndon@orthanc.ca> wrote:
>ori@eigenstate.org writes:
>
>> imap4resp0 may modfiy the flags to match what's on the
>> server. I think we want the server to have the final word
>> after we set the flags, so we want to set them first:
>
>That's correct.  You have to think of an IMAP session as an
>all-encompassing state machine.  Everything an IMAP client sends
>to a server is merely a suggestion.  Nothing changes until the
>server sends something to the client saying a change has taken
>place.
>
>E.g., the client cannot mark a message as "deleted" unless and until
>the server sends it something stating the \Deleted flag has been
>set on that message, no matter how many times the client has tried
>setting that flag.
>
>--lyndon

don't i know it.

sl