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

Remove ext/imap — it has been moved to PECL #13190

Merged
merged 13 commits into from
Jan 22, 2024
Merged

Remove ext/imap — it has been moved to PECL #13190

merged 13 commits into from
Jan 22, 2024

Conversation

derickr
Copy link
Member

@derickr derickr commented Jan 18, 2024

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The codeowner entry for IMAP should also be removed.

The only thing I'm concerned about is that we don't have test coverage for the regression tests of the mail() function.

But don't know what we could do to intercept the mail delivery for a test

@SakiTakamachi
Copy link
Member

Maybe "mailhog" would be good?

@bukka
Copy link
Member

bukka commented Jan 18, 2024

The only thing I'm concerned about is that we don't have test coverage for the regression tests of the mail() function.

That's my concern too. From checking the tests, it seems to me that it is completely removing mail coverage on Windows which is is not good if it's really the case. I think we should find replacement before we are going to proceed with this removal.

@bukka
Copy link
Member

bukka commented Jan 18, 2024

MailHog could work from a quick look as it offers quite nice API to get back the messages and do some search as well. So technically it should allow us to do the same checks.

@bukka
Copy link
Member

bukka commented Jan 18, 2024

Alternatively we could implement some debugging SMTP server which might not be that hard as it's pretty simple protocol and we could set some expectations there.

@SakiTakamachi
Copy link
Member

There are imap descriptions in these files, do I need to modify them?

"Extension: imap":

sudo cp ext/imap/tests/setup/dovecot.conf /etc/dovecot/dovecot.conf

EXTENSION: imap

https://github.com/php/php-src/blob/master/appveyor/setup_hmailserver.php

https://github.com/php/php-src/blob/master/php.ini-development
https://github.com/php/php-src/blob/master/php.ini-production

@derickr
Copy link
Member Author

derickr commented Jan 19, 2024

I'm leaving the dovecot and hmail server setup for now, as that can be used to do mail tests too. Removed the other occurences.

@alecpl
Copy link

alecpl commented Jan 19, 2024

I've been using https://greenmail-mail-test.github.io/greenmail/ for testing. Requires java, but does not need much setup.

@Uzume
Copy link

Uzume commented Dec 10, 2024

It should be noted the premise of the unbundling decision, that is the basis of this PR, is that the IMAP c-client library is unmaintained. This is erroneous and c-client is maintained as a part of Alpine and has been maintained there (as has all of imap that was once part of uw-imap) since Alpine integrated panda-imap (a fork of UW IMAP by Mark Crispin the original author of c-client) on 2013-11-02 (at Alpine 2.19.1; it is also mentioned in the release notes on their website for Alpine 2.20).

It was only ever unmaintained at older locations and @derickr, lead maintainer/developer of the pecl-mail-imap extension, failed to take into account Mark Crispin continuing to develop it after his time at University of Washington (as Panda IMAP) and Eduardo Chappa continuing its development as a part of Alpine since Alpine 2.19.1+ since 2013-11-02 after Crispin's death at the end of 2012.

Remember Crispin originally developed c-client at Stanford University (along with base64, etc.) and Crispin and his team developed pine and later alpine email clients at University of Washington (along with many other things like the their imapd server, etc.).

There is even a GH mirror at alpinemail/alpine and anyone looking for a maintained version of uw-map should probably look there or at the source repo on repo.or.cz. Feel free to compare:

@Girgias
Copy link
Member

Girgias commented Dec 10, 2024

No other major distribution uses Panda IMAP AFAIK, and it is impossible to build the IMAP extension on Fedora since a few years. It was already unmaintained for a long time within php-src and got the less than the bare minimum maintenance with no new features and basically no bug fixes.

Having an unmaintained extension in php-src is worse than having it in PECL.

It was unbundled for good reason, if someone else wants to maintain it they can do it, either here or by forking the repo.

@Uzume
Copy link

Uzume commented Dec 10, 2024

No other major distribution uses Panda IMAP AFAIK, and it is impossible to build the IMAP extension on Fedora since a few years. It was already unmaintained for a long time within php-src and got the less than the bare minimum maintenance with no new features and basically no bug fixes.

Yes, but there are existing alpine source packages (that include a maintained descendant of uw-imap) for many Unix distributions including Fedora and Debian (along with binary packages for the email client or course). It would not be terribly hard to build c-client, imapd, etc. packages from the same alpine source packages.

Having an unmaintained extension in php-src is worse than having it in PECL.

It was unbundled for good reason, if someone else wants to maintain it they can do it, either here or by forking the repo.

I am not arguing that it should not have been unbundled (this is the wrong forum for such an argument anyway)—only that the premise for it having been so was erroneous. The extension was certainly mostly unmaintained (it was just kept compiling with no changed to the actual imap/c-client code) for an extended period and usage and critical reasons to keep it bundled almost certainly evaporated.

I only wanted to call out to anyone that ended up at this PR, that there are potentially viable options for moving forward with a c-client binding extension to PHP should someone want to pursue such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants