Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add imap support #251

Merged
merged 10 commits into from
Nov 13, 2023
Merged

add imap support #251

merged 10 commits into from
Nov 13, 2023

Conversation

DubbleClick
Copy link
Contributor

@DubbleClick DubbleClick commented Oct 31, 2023

this was considerably easier with the musl toolchains since it allows debian/ubuntu to build slx, which doesn't require libpam

closes #202

tests:

  • x86 alpine
  • x86 rhel
  • x86 debian
  • aarch64 alpine
  • aarch64 rhel
  • aarch64 debian

todo: (have no experience with bsd, don't own a mac or aarch64 hardware)

  • bsd x86
  • bsd aarch64
  • mac aarch64

@crazywhalecc crazywhalecc added enhancement New feature or request kind/extension Issues related to extensions labels Oct 31, 2023
@DubbleClick
Copy link
Contributor Author

I can't do aarch64 debian testing (only alma & alpine) so I'm not sure if libpam will be required for aarch64 debian. I don't think so, though. And if it's not required, it's not worth including it imo.

@DubbleClick
Copy link
Contributor Author

If you can create a debian aarch64 vhdx that I can upload to azure, I'll be able to test aarch64 debian in the future, by the way. Otherwise, it will have to wait until the 12c snapdragon releases - will finally get a laptop then.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Nov 12, 2023

I'm writing a manually triggered testing actions here. It can specify different branches and repositories to test the extension combinations in the matrix of PHP8.0-8.2, Linux, macOS (using the official runner and local arm matrix).

I am now separating the download file process to avoid repeated test failures due to accidental download errors, but I don’t know why only arm-macos cannot hit the cache and cannot get the output of the previous workflow. I'm working on it.

FreeBSD is not a major platform, but it has good support for some common libraries. I currently reuse a lot of unix builds. However, imap is a relatively special library and may not be well suited to directly reusing unix compilation scripts for compilation. It may not be the main consideration for the current branch.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Nov 12, 2023

Debian aarch64 is good. But macOS support is still bad:

./tcp_unix.c:535:19: error: variable has incomplete type 'struct pollfd'
    struct pollfd pfd;
                  ^
./tcp_unix.c:535:12: note: forward declaration of 'struct pollfd'
    struct pollfd pfd;
           ^
./tcp_unix.c:545:20: error: use of undeclared identifier 'POLLIN'
      pfd.events = POLLIN;
                   ^
./tcp_unix.c:550:6: error: call to undeclared function 'poll'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        i = poll (&pfd, 1, ti ? tmo * 1000 : -1);
            ^
./tcp_unix.c:591:17: error: variable has incomplete type 'struct pollfd'
  struct pollfd pfd;
                ^
./tcp_unix.c:591:10: note: forward declaration of 'struct pollfd'
  struct pollfd pfd;
         ^
./tcp_unix.c:602:18: error: use of undeclared identifier 'POLLIN'
    pfd.events = POLLIN;
                 ^
./tcp_unix.c:606:11: error: call to undeclared function 'poll'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      i = poll (&pfd, 1, ti ? tmo * 1000 : -1);
          ^
./tcp_unix.c:659:17: error: variable has incomplete type 'struct pollfd'
  struct pollfd pfd;
                ^
./tcp_unix.c:659:10: note: forward declaration of 'struct pollfd'
  struct pollfd pfd;
         ^
./tcp_unix.c:670:18: error: use of undeclared identifier 'POLLOUT'
    pfd.events = POLLOUT;
                 ^
./tcp_unix.c:674:11: error: call to undeclared function 'poll'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
      i = poll (&pfd, 1, ti ? tmo * 1000 : -1);
          ^

@crazywhalecc
Copy link
Owner

Updates:

I found that macOS is basically caused by the lack of system headers. Among them, tcp_unix.c file is missing #include <poll.h>, and at least 10 other files are missing #include <time.h> and #include <utime.h>. After adding them, use osx instead of slx to compile successfully.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Nov 12, 2023

But this causes a problem. I don't want to repeat so much repetitive content in the patch, and making the patch file seems to conflict with the existing patch.

I wonder if we can directly fork uw-imap/imap and then apply the relevant patches to the code (except environment-related ones, such as SSL and CC). For macOS, a special .patch file will be run before compilation to fix this error.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Nov 12, 2023

Currently, it has been tested and runs well on Linux and macOS. Is it time to merge? BTW it would be better if you had checking code for imap extension.

@DubbleClick
Copy link
Contributor Author

Checking code for imap extension? The default check with php --ri "imap" should work, right?

@crazywhalecc
Copy link
Owner

Checking code for imap extension? The default check with php --ri "imap" should work, right?

In fact, it is a runtime test script, such as gd.

If we don’t add it, and just use --ri to make sure it can run, then it doesn’t matter if we don’t add this test. It’s just an option.

@DubbleClick
Copy link
Contributor Author

DubbleClick commented Nov 13, 2023

Yeah we don't need to write a test here I think. Just the default check is fine, since it won't show up if imap isn't supported.

If MacOS works fine this is good to merge for me!

@crazywhalecc crazywhalecc merged commit a8f2b00 into crazywhalecc:main Nov 13, 2023
4 checks passed
@DubbleClick DubbleClick deleted the imap branch November 13, 2023 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kind/extension Issues related to extensions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

imap support
2 participants