* [9front] Mail rewrite; open path with stored mails @ 2021-02-12 20:54 theinicke 2021-02-12 21:10 ` ori 0 siblings, 1 reply; 12+ messages in thread From: theinicke @ 2021-02-12 20:54 UTC (permalink / raw) To: 9front I have noticed that Mail/Nail does no longer open additional mailboxes if passed a path [containing mails as undecoded MIME messages]. Now I am wondering whether this is by design or it is desirable to reintroduce this. That said I can imagine some possibilities for reintroduction: 1) change argv parsing to interpret argument like old Mail did (e.g. mbox | path) 1a) also reintroduce second optional argument to open mailbox with given mboxname - imho unnecessary, but backwards compatibility? 2) introduce this via an additional parameter 3) just do it using a script I could provide a patch implementing it, but I am not sure whether this was left out by design (e.g. because if one needed it one could easily do this with a script) and which solution to favor, thoughts? -- Tobias Heinicke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-12 20:54 [9front] Mail rewrite; open path with stored mails theinicke @ 2021-02-12 21:10 ` ori 2021-02-13 20:47 ` theinicke 0 siblings, 1 reply; 12+ messages in thread From: ori @ 2021-02-12 21:10 UTC (permalink / raw) To: 9front Quoth theinicke@bss-wf.de: > I have noticed that Mail/Nail does no longer open additional mailboxes if passed a path [containing mails as undecoded MIME messages]. Now I am wondering whether this is by design or it is desirable to reintroduce this. > > That said I can imagine some possibilities for reintroduction: > > 1) change argv parsing to interpret argument like old Mail did (e.g. mbox | path) > 1a) also reintroduce second optional argument to open mailbox with given mboxname - imho unnecessary, but backwards compatibility? > 2) introduce this via an additional parameter > 3) just do it using a script The goal wasn't for perfect compatibility, but I have nothing against the feature. I just didn't use it, and nobody complained that it was gone until now. I'd be happy to take a patch for this. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-12 21:10 ` ori @ 2021-02-13 20:47 ` theinicke 2021-02-13 21:25 ` theinicke 0 siblings, 1 reply; 12+ messages in thread From: theinicke @ 2021-02-13 20:47 UTC (permalink / raw) To: 9front Quoth ori@eigenstate.org: > Quoth theinicke@bss-wf.de: > > I have noticed that Mail/Nail does no longer open additional mailboxes if passed a path [containing mails as undecoded MIME messages]. Now I am wondering whether this is by design or it is desirable to reintroduce this. > > > > That said I can imagine some possibilities for reintroduction: > > > > 1) change argv parsing to interpret argument like old Mail did (e.g. mbox | path) > > 1a) also reintroduce second optional argument to open mailbox with given mboxname - imho unnecessary, but backwards compatibility? > > 2) introduce this via an additional parameter > > 3) just do it using a script > > The goal wasn't for perfect compatibility, > but I have nothing against the feature. I > just didn't use it, and nobody complained > that it was gone until now. > > I'd be happy to take a patch for this. Thank you for your reply and sorry for the noise of accidentally sending multiple mails, I have inlined a patch which introduces it via a new parameter. Compared to old Mail this allows us to work with relative paths (on fs). diff -r 02e3059af5bc sys/man/1/acmemail --- a/sys/man/1/acmemail Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/man/1/acmemail Sat Feb 13 21:40:11 2021 +0100 @@ -20,6 +20,10 @@ .B -o .I outbox ] +[ +.B -d +.I path +] .SH DESCRIPTION .PP @@ -64,6 +68,10 @@ Save a copy of outgoing messages to the mailbox .IR outbox , instead of discarding them after they're enqueued. +.TP +.BI -d " path +Opens new mailbox for the given path, +closes it on exit. .PP Mail presents and acme interface for a upas/fs mailbox. diff -r 02e3059af5bc sys/src/cmd/upas/Mail/mbox.c --- a/sys/src/cmd/upas/Mail/mbox.c Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/src/cmd/upas/Mail/mbox.c Sat Feb 13 21:39:50 2021 +0100 @@ -32,6 +32,8 @@ Reprog *mesgpat; +int openedmbox = 0; + int threadsort = 1; int sender; @@ -51,8 +53,9 @@ Plumbmsg *m; while(1){ - if((m = plumbrecv(fd)) == nil) + if((m = plumbrecv(fd)) == nil) { threadexitsall("plumber gone"); + } sendp(ch, m); } } @@ -672,17 +675,26 @@ } } +static int +openctlfile(void) +{ + int fd; + char *path; + + path = estrjoin(maildir, "/ctl", nil); + fd = open(path, OWRITE); + free(path); + return fd; +} + static void mbflush(char **, int) { int i, j, ln, fd; - char *path; Mesg *m, *p; i = 0; - path = estrjoin(maildir, "/ctl", nil); - fd = open(path, OWRITE); - free(path); + fd = openctlfile(); if(fd == -1) sysfatal("open mbox: %r"); while(i < mbox.nmesg){ @@ -753,6 +765,22 @@ } static void +removeopened(void) +{ + int fd; + char buf[256]; + + if(!openedmbox) + return; + fd = openctlfile(); + if(fd == -1) + return; + + snprint(buf, sizeof buf, "close %s", mailbox); + write(fd, buf, strlen(buf)); +} + +static void quitall(char **, int) { Mesg *m; @@ -768,6 +796,7 @@ for(c = mbox.opencomp; c != nil; c = c->qnext) fprint(c->ctl, "del\n"); fprint(mbox.ctl, "del\n"); + removeopened(); threadexitsall(nil); } @@ -975,15 +1004,81 @@ static void usage(void) { - fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [mbox]\n", argv0); + fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [-d path] [mbox]\n", argv0); exits("usage"); } +static void +openmbox(char *path, char *mboxname) +{ + int fd, i; + char buf[512], err[ERRMAX]; + char *name, *fsname, *s1, *s2, *abspath; + + /* if path is already absolute, leave it as it is - to allow /imap... + otherwise get absolute path from relative one */ + if(*path == '/'){ + i = strlen(path); + if(path[i-1] == '/') + path[i-1] = '\0'; + abspath = estrdup(path); + } + else { + fd = open(path, OREAD); + if(fd < 0) + sysfatal("can't open %s: %r", path); + + if(fd2path(fd, buf, sizeof buf) != 0) + sysfatal("fd2path %s: %r", path); + close(fd); + abspath = estrdup(buf); + } + + if(mboxname != nil) + name = mboxname; + else{ + s1 = strrchr(abspath, '/'); + if(s1 == nil) + name = abspath; + else{ + *s1++ = '\0'; + name = s1; + *--s1 = '/'; + } + } + + fd = openctlfile(); + if(fd == -1) + sysfatal("open mbox: %r"); + fsname = estrdup(name); + s2 = emalloc(5+strlen(abspath)+1+strlen(fsname)+2+1); + for(i=0; i<10; i++){ + sprint(s2, "open %s %s", abspath, fsname); + if(write(fd, s2, strlen(s2)) >= 0) + break; + err[0] = '\0'; + errstr(err, sizeof err); + if(strstr(err, "mbox name in use") == nil) + sysfatal("can't create directory %s for mail: %s", name, err); + free(fsname); + fsname = emalloc(strlen(name)+3); + sprint(fsname, "%s-%d", name, i); + } + if(i == 10) + sysfatal("can't open %s: %r", abspath); + mailbox = fsname; + openedmbox = 1; + free(s2); + free(abspath); + close(fd); +} + void threadmain(int argc, char **argv) { Fmt fmt; char *cmd; + char *path = nil, *mboxname = nil; int i; mbox.view = Vgroup; @@ -1017,6 +1112,9 @@ case 'o': savebox = EARGF(usage()); break; + case 'd': + path = EARGF(usage()); + break; default: usage(); break; @@ -1024,8 +1122,13 @@ if(argc > 1) usage(); - if(argc == 1) + if(argc == 1) { mailbox = argv[0]; + mboxname = mailbox; + } + + if(path != nil) + openmbox(path, mboxname); mesgpat = regcomp("([0-9]+)(/.*)?"); cwait = threadwaitchan(); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-13 20:47 ` theinicke @ 2021-02-13 21:25 ` theinicke 2021-02-13 23:59 ` Alex Musolino 0 siblings, 1 reply; 12+ messages in thread From: theinicke @ 2021-02-13 21:25 UTC (permalink / raw) To: 9front Sorry - please consider following patch instead, as it also attempts to close opened mailbox, if Mail exits due to plumber being gone. -- Tobias Heinicke diff -r 02e3059af5bc sys/src/cmd/upas/Mail/mbox.c --- a/sys/src/cmd/upas/Mail/mbox.c Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/src/cmd/upas/Mail/mbox.c Sat Feb 13 22:21:29 2021 +0100 @@ -32,6 +32,8 @@ Reprog *mesgpat; +int openedmbox = 0; + int threadsort = 1; int sender; @@ -45,14 +47,44 @@ static void showmesg(Biobuf*, Mesg*, int, int); +static int +openctlfile(void) +{ + int fd; + char *path; + + path = estrjoin(maildir, "/ctl", nil); + fd = open(path, OWRITE); + free(path); + return fd; +} + +static void +removeopened(void) +{ + int fd; + char buf[256]; + + if(!openedmbox) + return; + fd = openctlfile(); + if(fd == -1) + return; + + snprint(buf, sizeof buf, "close %s", mailbox); + write(fd, buf, strlen(buf)); +} + static void plumbloop(Channel *ch, int fd) { Plumbmsg *m; while(1){ - if((m = plumbrecv(fd)) == nil) + if((m = plumbrecv(fd)) == nil) { + removeopened(); threadexitsall("plumber gone"); + } sendp(ch, m); } } @@ -676,13 +708,10 @@ mbflush(char **, int) { int i, j, ln, fd; - char *path; Mesg *m, *p; i = 0; - path = estrjoin(maildir, "/ctl", nil); - fd = open(path, OWRITE); - free(path); + fd = openctlfile(); if(fd == -1) sysfatal("open mbox: %r"); while(i < mbox.nmesg){ @@ -768,6 +797,7 @@ for(c = mbox.opencomp; c != nil; c = c->qnext) fprint(c->ctl, "del\n"); fprint(mbox.ctl, "del\n"); + removeopened(); threadexitsall(nil); } @@ -975,15 +1005,80 @@ static void usage(void) { - fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [mbox]\n", argv0); + fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [-d path] [mbox]\n", argv0); exits("usage"); } +static void +openmbox(char *path, char *mboxname) +{ + int fd, i; + char buf[512], err[ERRMAX]; + char *name, *fsname, *s1, *s2, *abspath; + + /* if path is already absolute, leave it as it is - to allow /imap... + otherwise get absolute path from relative one */ + if(*path == '/'){ + i = strlen(path); + if(path[i-1] == '/') + path[i-1] = '\0'; + abspath = estrdup(path); + }else{ + fd = open(path, OREAD); + if(fd < 0) + sysfatal("can't open %s: %r", path); + + if(fd2path(fd, buf, sizeof buf) != 0) + sysfatal("fd2path %s: %r", path); + close(fd); + abspath = estrdup(buf); + } + + if(mboxname != nil) + name = mboxname; + else{ + s1 = strrchr(abspath, '/'); + if(s1 == nil) + name = abspath; + else{ + *s1++ = '\0'; + name = s1; + *--s1 = '/'; + } + } + + fd = openctlfile(); + if(fd == -1) + sysfatal("open mbox: %r"); + fsname = estrdup(name); + s2 = emalloc(5+strlen(abspath)+1+strlen(fsname)+2+1); + for(i=0; i<10; i++){ + sprint(s2, "open %s %s", abspath, fsname); + if(write(fd, s2, strlen(s2)) >= 0) + break; + err[0] = '\0'; + errstr(err, sizeof err); + if(strstr(err, "mbox name in use") == nil) + sysfatal("can't create directory %s for mail: %s", name, err); + free(fsname); + fsname = emalloc(strlen(name)+3); + sprint(fsname, "%s-%d", name, i); + } + if(i == 10) + sysfatal("can't open %s: %r", abspath); + mailbox = fsname; + openedmbox = 1; + free(s2); + free(abspath); + close(fd); +} + void threadmain(int argc, char **argv) { Fmt fmt; char *cmd; + char *path = nil, *mboxname = nil; int i; mbox.view = Vgroup; @@ -1017,6 +1112,9 @@ case 'o': savebox = EARGF(usage()); break; + case 'd': + path = EARGF(usage()); + break; default: usage(); break; @@ -1024,8 +1122,13 @@ if(argc > 1) usage(); - if(argc == 1) + if(argc == 1) { mailbox = argv[0]; + mboxname = mailbox; + } + + if(path != nil) + openmbox(path, mboxname); mesgpat = regcomp("([0-9]+)(/.*)?"); cwait = threadwaitchan(); diff -r 02e3059af5bc sys/man/1/acmemail --- a/sys/man/1/acmemail Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/man/1/acmemail Sat Feb 13 22:23:35 2021 +0100 @@ -20,6 +20,10 @@ .B -o .I outbox ] +[ +.B -d +.I path +] .SH DESCRIPTION .PP @@ -64,6 +68,10 @@ Save a copy of outgoing messages to the mailbox .IR outbox , instead of discarding them after they're enqueued. +.TP +.BI -d " path +Opens new mailbox for the given path, +closes it on exit. .PP Mail presents and acme interface for a upas/fs mailbox. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-13 21:25 ` theinicke @ 2021-02-13 23:59 ` Alex Musolino 2021-02-14 10:42 ` theinicke 0 siblings, 1 reply; 12+ messages in thread From: Alex Musolino @ 2021-02-13 23:59 UTC (permalink / raw) To: 9front I have a couple of general comments. Firstly, the -d option is typically used as to enable debugging output of some kind. Can you use a different character? Perhaps -p for path? Also, it seems that the options we support and the usage string(s) do not agree. Secondly, you can register the removembox function to run on exit with atexit(2). That'll save you having to call it yourself from any and every place that might result in an exit. At the moment, removembox won't be called if we hit one of the sysfatals. -- Cheers, Alex Musolino ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-13 23:59 ` Alex Musolino @ 2021-02-14 10:42 ` theinicke 2021-02-14 20:57 ` ori 2021-02-14 22:55 ` Alex Musolino 0 siblings, 2 replies; 12+ messages in thread From: theinicke @ 2021-02-14 10:42 UTC (permalink / raw) To: 9front Quoth Alex Musolino <alex@musolino.id.au>: > I have a couple of general comments. > > Firstly, the -d option is typically used as to enable debugging output > of some kind. Can you use a different character? Perhaps -p for > path? Also, it seems that the options we support and the usage > string(s) do not agree. > > Secondly, you can register the removembox function to run on exit with > atexit(2). That'll save you having to call it yourself from any and > every place that might result in an exit. At the moment, removembox > won't be called if we hit one of the sysfatals. > > -- > Cheers, > Alex Musolino > Thank you for your comments, based on your suggestions I have provided the following patch (assuming that you meant the usage string discrepancy that was there before my patch - while at it I have also changed NAIL(1) to ACMEMAIL(1); if this was not what you meant please elaborate). -- Cheers, Tobias Heinicke diff -r 02e3059af5bc sys/man/1/acmemail --- a/sys/man/1/acmemail Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/man/1/acmemail Sun Feb 14 11:36:18 2021 +0100 @@ -1,4 +1,4 @@ -.TH NAIL 1 +.TH ACMEMAIL 1 .SH NAME Mail \- view mail in acme @@ -6,7 +6,7 @@ Mail [ -.B -OsT +.B -TOs ] [ .B -m @@ -20,6 +20,13 @@ .B -o .I outbox ] +[ +.B -p +.I path +] +[ +.I mbox +] .SH DESCRIPTION .PP @@ -64,6 +71,10 @@ Save a copy of outgoing messages to the mailbox .IR outbox , instead of discarding them after they're enqueued. +.TP +.BI -p " path +Opens new mailbox for the given path, +closes it on exit. .PP Mail presents and acme interface for a upas/fs mailbox. diff -r 02e3059af5bc sys/src/cmd/upas/Mail/mbox.c --- a/sys/src/cmd/upas/Mail/mbox.c Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/src/cmd/upas/Mail/mbox.c Sun Feb 14 11:36:18 2021 +0100 @@ -32,6 +32,8 @@ Reprog *mesgpat; +int openedmbox = 0; + int threadsort = 1; int sender; @@ -45,6 +47,34 @@ static void showmesg(Biobuf*, Mesg*, int, int); +int +openctlfile(void) +{ + int fd; + char *path; + + path = estrjoin(maildir, "/ctl", nil); + fd = open(path, OWRITE); + free(path); + return fd; +} + +void +removeopened(void) +{ + int fd; + char buf[256]; + + if(!openedmbox) + return; + fd = openctlfile(); + if(fd == -1) + return; + + snprint(buf, sizeof buf, "close %s", mailbox); + write(fd, buf, strlen(buf)); +} + static void plumbloop(Channel *ch, int fd) { @@ -676,13 +706,10 @@ mbflush(char **, int) { int i, j, ln, fd; - char *path; Mesg *m, *p; i = 0; - path = estrjoin(maildir, "/ctl", nil); - fd = open(path, OWRITE); - free(path); + fd = openctlfile(); if(fd == -1) sysfatal("open mbox: %r"); while(i < mbox.nmesg){ @@ -975,15 +1002,84 @@ static void usage(void) { - fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [mbox]\n", argv0); + fprint(2, "usage: %s [-TOs] [-m maildir] [-f format] [-o outbox] [-p path] [mbox]\n", argv0); exits("usage"); } +static void +openmbox(char *path, char *mboxname) +{ + int fd, i; + char buf[512], err[ERRMAX]; + char *name, *fsname, *s1, *s2, *abspath; + + /* if path is already absolute, leave it as it is - to allow /imap... + otherwise get absolute path from relative one */ + if(*path == '/'){ + i = strlen(path); + if(path[i-1] == '/') + path[i-1] = '\0'; + abspath = estrdup(path); + }else{ + fd = open(path, OREAD); + if(fd < 0) + sysfatal("can't open %s: %r", path); + + if(fd2path(fd, buf, sizeof buf) != 0) + sysfatal("fd2path %s: %r", path); + close(fd); + abspath = estrdup(buf); + } + + if(mboxname != nil) + name = mboxname; + else{ + s1 = strrchr(abspath, '/'); + if(s1 == nil) + name = abspath; + else{ + *s1++ = '\0'; + name = s1; + *--s1 = '/'; + } + } + + fd = openctlfile(); + if(fd == -1) + sysfatal("open mbox: %r"); + fsname = estrdup(name); + s2 = emalloc(5+strlen(abspath)+1+strlen(fsname)+2+1); + for(i=0; i<10; i++){ + sprint(s2, "open %s %s", abspath, fsname); + if(write(fd, s2, strlen(s2)) >= 0) + break; + err[0] = '\0'; + errstr(err, sizeof err); + if(strstr(err, "mbox name in use") == nil) + sysfatal("can't create directory %s for mail: %s", name, err); + free(fsname); + fsname = emalloc(strlen(name)+3); + sprint(fsname, "%s-%d", name, i); + } + if(i == 10) + sysfatal("can't open %s: %r", abspath); + if(atexit(removeopened) == 0) { + removeopened(); + sysfatal("atexit: %r"); + } + mailbox = fsname; + openedmbox = 1; + free(s2); + free(abspath); + close(fd); +} + void threadmain(int argc, char **argv) { Fmt fmt; char *cmd; + char *path = nil, *mboxname = nil; int i; mbox.view = Vgroup; @@ -1017,6 +1113,9 @@ case 'o': savebox = EARGF(usage()); break; + case 'p': + path = EARGF(usage()); + break; default: usage(); break; @@ -1024,8 +1123,13 @@ if(argc > 1) usage(); - if(argc == 1) + if(argc == 1) { mailbox = argv[0]; + mboxname = mailbox; + } + + if(path != nil) + openmbox(path, mboxname); mesgpat = regcomp("([0-9]+)(/.*)?"); cwait = threadwaitchan(); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-14 10:42 ` theinicke @ 2021-02-14 20:57 ` ori 2021-02-14 22:05 ` theinicke 2021-02-14 22:55 ` Alex Musolino 1 sibling, 1 reply; 12+ messages in thread From: ori @ 2021-02-14 20:57 UTC (permalink / raw) To: 9front > Thank you for your comments, based on your suggestions I have provided the following patch > (assuming that you meant the usage string discrepancy that was there before my patch - > while at it I have also changed NAIL(1) to ACMEMAIL(1); if this was not what you meant please elaborate). Some comments -- sorry if this is long: > .SH DESCRIPTION > .PP > @@ -64,6 +71,10 @@ > Save a copy of outgoing messages to the mailbox > .IR outbox , > instead of discarding them after they're enqueued. > +.TP > +.BI -p " path > +Opens new mailbox for the given path, > +closes it on exit. Why not just make this flag modify the interpretation of argv[0]? Mail -o /path/to/mbox > +void > +removeopened(void) closembox()? > + fd = openctlfile(); > + if(fd == -1) > + return; Nitpicking here, but most of the code uses: if((fd = open...) == -1) > + snprint(buf, sizeof buf, "close %s", mailbox); > + write(fd, buf, strlen(buf)); cleaner: fprint(fd, "close %s"); > - fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [mbox]\n", argv0); > + fprint(2, "usage: %s [-TOs] [-m maildir] [-f format] [-o outbox] [-p path] [mbox]\n", argv0); > exits("usage"); > } > > +static void > +openmbox(char *path, char *mboxname) > +{ This function looks pretty ugly; I'd rather just have it take a path to the mbox and not mangle it. > + int fd, i; > + char buf[512], err[ERRMAX]; > + char *name, *fsname, *s1, *s2, *abspath; > + > + /* if path is already absolute, leave it as it is - to allow /imap... > + otherwise get absolute path from relative one */ There's no real harm in skipping this special case. It's one open in code that does an open for every mail in the mailbox. > + if(*path == '/'){ > + i = strlen(path); > + if(path[i-1] == '/') > + path[i-1] = '\0'; > + abspath = estrdup(path); > + }else{ > + fd = open(path, OREAD); > + if(fd < 0) > + sysfatal("can't open %s: %r", path); > + > + if(fd2path(fd, buf, sizeof buf) != 0) > + sysfatal("fd2path %s: %r", path); > + close(fd); > + abspath = estrdup(buf); > + } > + > + if(mboxname != nil) > + name = mboxname; > + else{ > + s1 = strrchr(abspath, '/'); > + if(s1 == nil) > + name = abspath; > + else{ > + *s1++ = '\0'; > + name = s1; > + *--s1 = '/'; Why the termination and unterminaton? > + } > + } > + > + fd = openctlfile(); > + if(fd == -1) > + sysfatal("open mbox: %r"); > + fsname = estrdup(name); > + s2 = emalloc(5+strlen(abspath)+1+strlen(fsname)+2+1); > + for(i=0; i<10; i++){ Why is this being repeated 10 times? I don't see anything that is going to be improved by doing it over and over in rapid succession. If the mbox is in use, it's almost certain to stay in use for longer than this loop runs. > + sprint(s2, "open %s %s", abspath, fsname); sprint should probably not be used in new code; while it's possible to use safely, it's easier to verify snprint and smprint. In this case, smprint is probably the right choice. > + if(write(fd, s2, strlen(s2)) >= 0) > + break; Or better: fprint. > void > threadmain(int argc, char **argv) > { > Fmt fmt; > char *cmd; > + char *path = nil, *mboxname = nil; > int i; > > mbox.view = Vgroup; > @@ -1017,6 +1113,9 @@ > case 'o': > savebox = EARGF(usage()); > break; > + case 'p': > + path = EARGF(usage()); > + break; > default: > usage(); > break; > @@ -1024,8 +1123,13 @@ > > if(argc > 1) > usage(); > - if(argc == 1) > + if(argc == 1) { > mailbox = argv[0]; > + mboxname = mailbox; > + } > + > + if(path != nil) > + openmbox(path, mboxname); > > mesgpat = regcomp("([0-9]+)(/.*)?"); > cwait = threadwaitchan(); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-14 20:57 ` ori @ 2021-02-14 22:05 ` theinicke 0 siblings, 0 replies; 12+ messages in thread From: theinicke @ 2021-02-14 22:05 UTC (permalink / raw) To: 9front Thanks for the write-up. I appreciate it and get most of your points. Here are some notes regarding your comments: > Why not just make this flag modify the interpretation > of argv[0]? Frankly I do not get this - I thought about the new option behaving as opening the desired path at given name, if name is given via argv[0] (otherwise using last path component as name). > closembox()? removeopened is arguably not a nice name, however closembox would be imho not so good either, because we only close previously opened mbox and not in general > There's no real harm in skipping this special case. > It's one open in code that does an open for every > mail in the mailbox. What special case exactly? I thought that we may want to be able to open relative paths (for that I used fd2path(2)); this however would remove us from having the possibility to open another imap (/imaps/mailserver/...) directory - thus if the path is already absolute I decided to leave it as it is (except for 1 possibly trailing slash, as this happens likely; with multiple trailing slashes one may be doomed, but deserves it probably). > Why the termination and unterminaton? Arguably ugly - still use abspath below (read: extract last path component and leave absolute path intact).. > Why is this being repeated 10 times? [...] It tries with a modified mbox name each time - say you got something like $home/mail/9front/archive and $home/mail/9fans/archive Then upon invoking 'Mail -p $home/mail/9front/archive' you would get /mail/fs/archive and if you have this running somewhere and decided to open your other archive (without passing argument) this would be opened as /mail/fs/archive-0 That way we may have up to 11 mailboxes with the same name - 11 may be unnecessary high, but no harm I think? Taken from old Mail. The remaining comments point out some obvious flaws and I'll gladly fix them; just wanted to quickly leave this here and thanks again for the feedback ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-14 10:42 ` theinicke 2021-02-14 20:57 ` ori @ 2021-02-14 22:55 ` Alex Musolino 2021-02-15 8:15 ` theinicke 1 sibling, 1 reply; 12+ messages in thread From: Alex Musolino @ 2021-02-14 22:55 UTC (permalink / raw) To: 9front > + if(atexit(removeopened) == 0) { > + removeopened(); > + sysfatal("atexit: %r"); > + } This is not how atexit(2) works. The idea is that you would do atexit(removeopened) *once* in main which arranges for removeopened to be called on the way out of the program. The following should suffice: if(atexit(removeopened) == 0) sysfatal("atexit: %r"); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-14 22:55 ` Alex Musolino @ 2021-02-15 8:15 ` theinicke 2021-02-15 9:56 ` theinicke 0 siblings, 1 reply; 12+ messages in thread From: theinicke @ 2021-02-15 8:15 UTC (permalink / raw) To: 9front Quoth Alex Musolino <alex@musolino.id.au>: > > + if(atexit(removeopened) == 0) { > > + removeopened(); > > + sysfatal("atexit: %r"); > > + } > > This is not how atexit(2) works. The idea is that you would do > atexit(removeopened) *once* in main which arranges for removeopened to > be called on the way out of the program. The following should > suffice: > > if(atexit(removeopened) == 0) > sysfatal("atexit: %r"); > atexit is only called once, because openmbox is only called once; will move it into main right after openmbox to avoid potential future duplicate calls however, thanks. I decided to call removeopened() if atexit fails, because if we exit the program anyway then we may remove the mbox then and there? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [9front] Mail rewrite; open path with stored mails 2021-02-15 8:15 ` theinicke @ 2021-02-15 9:56 ` theinicke 0 siblings, 0 replies; 12+ messages in thread From: theinicke @ 2021-02-15 9:56 UTC (permalink / raw) To: 9front Ouch.. I should not write code and mails while being tired and doing other things... I apologize to anyone who has wasted time reading through my previous patch. @Ori: Thanks again for your feedback - and also your patience. What follows is a patch that adresses most of the feedback and hopefully looks better: diff -r 02e3059af5bc sys/man/1/acmemail --- a/sys/man/1/acmemail Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/man/1/acmemail Mon Feb 15 10:48:11 2021 +0100 @@ -1,4 +1,4 @@ -.TH NAIL 1 +.TH ACMEMAIL 1 .SH NAME Mail \- view mail in acme @@ -6,7 +6,7 @@ Mail [ -.B -OsT +.B -TOs ] [ .B -m @@ -20,6 +20,13 @@ .B -o .I outbox ] +[ +.B -p +.I path +] +[ +.I mbox +] .SH DESCRIPTION .PP @@ -64,6 +71,10 @@ Save a copy of outgoing messages to the mailbox .IR outbox , instead of discarding them after they're enqueued. +.TP +.BI -p " path +Opens new mailbox for the given path, +closes it on exit. .PP Mail presents and acme interface for a upas/fs mailbox. diff -r 02e3059af5bc sys/src/cmd/upas/Mail/mbox.c --- a/sys/src/cmd/upas/Mail/mbox.c Thu Feb 11 09:37:36 2021 +0100 +++ b/sys/src/cmd/upas/Mail/mbox.c Mon Feb 15 10:48:11 2021 +0100 @@ -45,6 +45,31 @@ static void showmesg(Biobuf*, Mesg*, int, int); +int +openctlfile(void) +{ + int fd; + char *path; + + path = estrjoin(maildir, "/ctl", nil); + fd = open(path, OWRITE); + free(path); + return fd; +} + +void +closembox(void) +{ + int fd; + + fd = openctlfile(); + if(fd == -1) + return; + + fprint(fd, "close %s", mailbox); + close(fd); +} + static void plumbloop(Channel *ch, int fd) { @@ -676,13 +701,10 @@ mbflush(char **, int) { int i, j, ln, fd; - char *path; Mesg *m, *p; i = 0; - path = estrjoin(maildir, "/ctl", nil); - fd = open(path, OWRITE); - free(path); + fd = openctlfile(); if(fd == -1) sysfatal("open mbox: %r"); while(i < mbox.nmesg){ @@ -975,15 +997,91 @@ static void usage(void) { - fprint(2, "usage: %s [-T] [-m mailfs] [-s] [-f format] [mbox]\n", argv0); + fprint(2, "usage: %s [-TOs] [-m maildir] [-f format] [-o outbox] [-p path] [mbox]\n", argv0); exits("usage"); } +static char* +basename(char *path) +{ + char *base; + base = strrchr(path, '/'); + if(base == nil) + base = path; + return base+1; +} + +static char* +absmboxpath(char *path) +{ + char *abspath; + char buf[512]; + int fd, i; + + /* if path is already absolute, leave it as it is - to allow /imap... + otherwise get absolute path from relative one */ + if(*path == '/'){ + i = strlen(path); + while(i > 0 && path[--i] == '/') + path[i] = '\0'; + abspath = estrdup(path); + }else{ + fd = open(path, OREAD); + if(fd < 0) + sysfatal("can't open %s: %r", path); + + if(fd2path(fd, buf, sizeof buf) != 0) + sysfatal("fd2path %s: %r", path); + close(fd); + abspath = estrdup(buf); + } + + return abspath; +} + +static void +openmbox(char *path, char *mboxname) +{ + int fd, i; + char err[ERRMAX]; + char *name, *fsname, *abspath; + + abspath = absmboxpath(path); + + if(mboxname != nil) + name = mboxname; + else{ + name = basename(abspath); + } + + fd = openctlfile(); + if(fd == -1) + sysfatal("open mbox: %r"); + fsname = estrdup(name); + for(i=0; i<10; i++){ + if(fprint(fd, "open %s %s", abspath, fsname) >= 0) + break; + err[0] = '\0'; + errstr(err, sizeof err); + if(strstr(err, "mbox name in use") == nil) + sysfatal("can't create directory %s for mail: %s", name, err); + free(fsname); + fsname = smprint("%s-%d", name, i); + } + if(i == 10) + sysfatal("can't open %s: %r", abspath); + + mailbox = fsname; + free(abspath); + close(fd); +} + void threadmain(int argc, char **argv) { Fmt fmt; char *cmd; + char *path = nil, *mboxname = nil; int i; mbox.view = Vgroup; @@ -1017,6 +1115,9 @@ case 'o': savebox = EARGF(usage()); break; + case 'p': + path = EARGF(usage()); + break; default: usage(); break; @@ -1024,8 +1125,18 @@ if(argc > 1) usage(); - if(argc == 1) + if(argc == 1) { mailbox = argv[0]; + mboxname = mailbox; + } + + if(path != nil) { + openmbox(path, mboxname); + if(atexit(closembox) == 0) { + closembox(); + sysfatal("atexit: %r"); + } + } mesgpat = regcomp("([0-9]+)(/.*)?"); cwait = threadwaitchan(); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [9front] Mail rewrite open path with stored mails @ 2021-02-12 20:50 theinicke 0 siblings, 0 replies; 12+ messages in thread From: theinicke @ 2021-02-12 20:50 UTC (permalink / raw) To: 9front I have noticed that Mail/Nail does no longer open additional mailboxes if passed a path [containing mails as undecoded MIME messages]. Now I am wondering whether this is by design or it is desirable to reintroduce this. That said I can imagine some possibilities for reintroduction: 1) change argv parsing to interpret argument like old Mail did (e.g. mbox | path) 1a) also reintroduce second optional argument to open mailbox with given mboxname - imho unnecessary, but backwards compatibility? 2) introduce this via an additional parameter 3) just do it using a script I could provide a patch implementing it, but I am not sure whether this was left out by design (e.g. because if one needed it one could easily do this with a script) and which solution to favor, thoughts? -- Tobias Heinicke ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-02-15 9:59 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-12 20:54 [9front] Mail rewrite; open path with stored mails theinicke 2021-02-12 21:10 ` ori 2021-02-13 20:47 ` theinicke 2021-02-13 21:25 ` theinicke 2021-02-13 23:59 ` Alex Musolino 2021-02-14 10:42 ` theinicke 2021-02-14 20:57 ` ori 2021-02-14 22:05 ` theinicke 2021-02-14 22:55 ` Alex Musolino 2021-02-15 8:15 ` theinicke 2021-02-15 9:56 ` theinicke -- strict thread matches above, loose matches on Subject: below -- 2021-02-12 20:50 [9front] Mail rewrite " theinicke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).