Re: [9front] [patch] upas: plumb flag updates on messages.

[ori@eigenstate.org]

>> ok, small remarks.
>> 
>> +			if(doplumb){
>> +				if((m->cstate & Cnew) && ensurecache(mb, m) == 0)
>>  					msgdecref(mb, m);
>> 
>> i think you want to do the msgdecref() of the message *after* mailplumb(),
>> not before. otherwise its like free before use. i know with the current
>> code it will still work but it is semantially wrong. if we later change
>> how cache eviction works that will blow up.
> 
> Ok. I was looking at the code, and it *seemed* like the intent was
> to have even evicted messages be somewhat valid. I think there are a
> number of places where we manipulate seen but not cached messages.
> 
>> +		print("child\n");
>> +		if(mboxfile != nil)
>> +			if(err = newmbox(mboxfile, "mbox", 0, nil))
>> +				sysfatal("opening %s: %s", mboxfile, err);
> 
>> theres a debug print left there and this changes some semantics.
>> i know this is because initial sync from imap takes forever. but
>> people sometimes want to specify mailbox path at the initial
>> command line and expect error status if the initial sync failed
>> (like if mounting a mbox file in a script to parse a message).
> 
> Right, forgot about this change. Reverted -- It doesn't even
> make things respon faster, since the mount doesn't return until
> after we do the initial sync.
> 
>> i think a good compromise would be to add a commandline flag to
>> specify if you want initial sync in the background or not
>> (can then be added to /rc/bin/startupasfs).
>>
>> if we do background syncing, then even if the initial sync
>> fails, it should try to continue. imagine your imap server
>> is currently down or network is unavailable, you want it to
>> continue trying in that case. and sysfataling here makes no
>> sense as nobody will be listening.
> 
> Yes. background sync will need a bunch more work, and a lot
> more thought to get right.
> 
> Updated patch below, slipped in 2 small changes:
> 
>  - Acme Mail => Next button to take you to the next msg
>  - Imap => small consistency tweak: flags are uint.
> 
> Can back them out if you care about separating them.
> 


+			if((m->cstate & Cnew|Cmod) && ensurecache(mb, m) == 0){

And I attached the wrong version of the patch. The one I tested
has (m->cstate & (Cnew|Cmod))