termux / termux-packages

A package build system for Termux.
https://termux.dev
Other
13.28k stars 3.06k forks source link

[fdm] Can't fetch mail to Maildir on non-rooted devices #11778

Closed lisaev1 closed 2 years ago

lisaev1 commented 2 years ago

When instructed to deliver to Maildir, fdm uses safemove() (in file.c) to move fetched mail-file from tmp/ to new/:

safemove(const char *oldpath, const char *newpath)
{
        int             ret;
        int             errsave;
        struct stat     sb;

        ret = link(oldpath, newpath);
        if (ret != 0 && errno == EXDEV) {
                ret = stat(newpath, &sb);
                if (ret == -1) {
                        if (errno == ENOENT)
                                ret = rename(oldpath, newpath);
                } else {
                        ret = -1;
                        errno = EEXIST;
                }
        }

        errsave = errno;
        if (unlink(oldpath) != 0 && errno != ENOENT)
                fatal("unlink failed");
        errno = errsave;

        return (ret);
}

Unfortunately, on a non-rooted device this would fail because Android doesn't allow creation of hardlinks.

Do I correctly understand that in Termux, fdm is effectively limited to mbox (one file for all messages)? And would it be desirable to patch fdm to perhaps use rename()/renameat()?

Thanks!

sylirre commented 2 years ago

Yes, it should be patched to use rename function.

lisaev1 commented 2 years ago

So... safemove() does fall back to rename(), but only when link() fails with EXDEV (per fdm commit a607552325dc5aad84bce061c431156f4e95008a). I would think that the only change in file.c is to replace errno == EXDEV with (errno == EXDEV || errno == EPERM)? I can also ask at fdm if this can be upstreamed...

sylirre commented 2 years ago

Package update with the fix should be published (packages.termux.dev). Check whether the issue is now resolved.

lisaev1 commented 2 years ago

Hmm, It isn't fixed yet, and this is totally my fault... Apologies!

Instead of EPERM, it should have been EACCES. I rebuilt fdm locally, with the following modification of your patch:

~/termux-packages $ git diff
diff --git a/packages/fdm/fix-safemove.patch b/packages/fdm/fix-safemove.patch
index 01c83e2f3..439c40721 100644
--- a/packages/fdm/fix-safemove.patch
+++ b/packages/fdm/fix-safemove.patch
@@ -6,7 +6,7 @@ diff -uNr fdm-2.1/file.c fdm-2.1.mod/file.c

        ret = link(oldpath, newpath);
 -      if (ret != 0 && errno == EXDEV) {
-+      if (ret != 0 && (errno == EXDEV || errno == EPERM)) {
++      if (ret != 0 && (errno == EXDEV || errno == EACCES)) {
                ret = stat(newpath, &sb);
                if (ret == -1) {
                        if (errno == ENOENT)

(a printf inserted in file.c printed Permission denied, which maps to EACCES, not EPERM). With the updated patch, I was able to fetch into maildir:

~ $ fdm -v fetch
WARNING: linker: /data/data/com.termux/files/usr/bin/fdm: unsupported flags DT_FLAGS_1=0x8000001
version is: fdm 2.1, started at: Thu Sep  1 08:51:45 2022
comcast: got message 1 of 1: size 2798, body 2717
comcast: message 1 delivered (rule 0, maildir) in 0.007 seconds
comcast: deleting message 1
comcast: 1 messages processed (0 kept) in 2.262 seconds (average 2.262)

Sorry again for making you do a wrong commit... On the bright side, I discovered that Termux's build system is amazing :)

PS: If this matters, here is my build env:

~/termux-packages $ termux-info
Termux Variables:
TERMUX_APP_PACKAGE_MANAGER=apt
TERMUX_MAIN_PACKAGE_FORMAT=debian
TERMUX_VERSION=0.118.0
Packages CPU architecture:
arm
Subscribed repositories:
# sources.list
deb https://packages.termux.dev/apt/termux-main stable main
Updatable packages:
All packages up to date
termux-tools version:
1.29.2
Android version:
7.1.2
Kernel build information:
Linux localhost 3.4.0-g2c2be66 #1 SMP PREEMPT Sat Jan 26 14:40:26 UTC 2019 armv7l Android
Device manufacturer:
LGE
Device model:
Nexus 5

Thanks!

sylirre commented 2 years ago

Pushed an update which should be available for installation soon.

lisaev1 commented 2 years ago

Yes, it fixes the bug. Thank you!