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

cyrus-imapd: depends on obsolete pcre library #3861

Open
guimard opened this issue Jan 10, 2022 · 10 comments · May be fixed by #4545
Open

cyrus-imapd: depends on obsolete pcre library #3861

guimard opened this issue Jan 10, 2022 · 10 comments · May be fixed by #4545

Comments

@guimard
Copy link
Contributor

guimard commented Jan 10, 2022

Ref: https://bugs.debian.org/1000034

Hi,

when using pcre2, all test passed except this one:

  TESTCASE("re: no re: foobar", "nore:foobar");

which gives:

  Test: subject_normalise ...FAILED
    1. ./cunit/conversations.testc:1353  -
       CU_ASSERT_EQUAL(b.len=14,sizeof(_exp)-1=11)
    2. ./cunit/conversations.testc:1353  -
       CU_ASSERT_STRING_EQUAL(b.s="re:nore:foobar",_exp="nore:foobar")

Cheers,
Yadd

@elliefm
Copy link
Contributor

elliefm commented Jan 11, 2022

This is deeply confusing but I think I have finally understood...

  • The OLD "pcre" is called "pcre3" for some reason
  • The NEW "pcre" is called "pcre2"
  • Cyrus works fine with the old pcre
  • Debian want to remove the old one, and only package the new one, but this will require Cyrus to start supporting the new one
  • Cyrus builds fine with the new one, but there is at least one test failure when built like this -- and maybe other problems we've yet to discover for lack of a test...

Is that a correct summation of the situation?

There's more detail in the linked Debian bug, but I want to make sure I'm clear on the basics before I try reading any deeper

@guimard
Copy link
Contributor Author

guimard commented Jan 12, 2022

Hi @elliefm, thanks for the translation and sorry for this : I'm not fluent in English. You are totally right here

@elliefm
Copy link
Contributor

elliefm commented Jan 13, 2022

Your English is fine, it was the version number going backwards that was the confusing part! :)

@guimard
Copy link
Contributor Author

guimard commented Jan 19, 2022

Sure, sorry. Is this just a test issue or a core issue ?

@elliefm
Copy link
Contributor

elliefm commented Jan 20, 2022

No idea yet. But I think it's reasonable to assume that if our tests are doing something the new library doesn't like, then our real code is probably also doing it and will also break.

@guimard guimard changed the title cyrus-imapd: depends on obsolete pcre3 library cyrus-imapd: depends on obsolete pcre library Feb 4, 2022
@elliefm
Copy link
Contributor

elliefm commented Mar 15, 2022

@brong does this sound like the same issue you mentioned on the Cyrus conf call the other day?

@robert-scheck
Copy link

robert-scheck commented Sep 22, 2022

Just as a "heads up": Fedora also tries to get rid of the old pcre, which is meanwhile even end-of-life at upstream, in favor of pcre2; see also: https://bugzilla.redhat.com/show_bug.cgi?id=2128286

@ArchangeGabriel
Copy link

Same for ArchLinux: https://bugs.archlinux.org/task/77058#comment214212
Building with the pcre2 patch in this comment leads to the same test failure.

@loqs
Copy link

loqs commented Jan 11, 2023

diff --git a/configure.ac b/configure.ac
index 8fe7d78..ac601a3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -666,7 +666,7 @@ AM_CONDITIONAL([SQUATTER],
 AC_ARG_ENABLE(sieve,
         [AS_HELP_STRING([--disable-sieve], [disable Sieve support])],,[enable_sieve="yes";])
 AC_ARG_ENABLE(pcre,
-        [AS_HELP_STRING([--disable-pcre], [disable PCRE library])],[cyrus_cv_pcre_utf8="$enableval"])
+        [AS_HELP_STRING([--disable-pcre2], [disable PCRE library])],[cyrus_cv_pcre_utf8="$enableval"])
 
 if test "$enable_sieve" != "no"; then
         AC_DEFINE(USE_SIEVE,[],[Build in Sieve support?])
@@ -692,25 +692,25 @@ fi
 
 AM_CONDITIONAL([SIEVE], [test "${enable_sieve}" != "no"])
 
-if test "$enable_pcre" != "no"; then
-        AC_CHECK_HEADER(pcreposix.h)
-        if test "$ac_cv_header_pcreposix_h" = "yes"; then
-            AC_MSG_CHECKING(for utf8 enabled pcre)
-            AC_CACHE_VAL(cyrus_cv_pcre_utf8, AC_TRY_CPP([#include <pcreposix.h>
-#ifndef REG_UTF8
+if test "$enable_pcre2" != "no"; then
+        AC_CHECK_HEADER(pcre2posix.h)
+        if test "$ac_cv_header_pcre2posix_h" = "yes"; then
+            AC_MSG_CHECKING(for utf8 enabled pcre2)
+            AC_CACHE_VAL(cyrus_cv_pcre_utf8, AC_TRY_CPP([#include <pcre2posix.h>
+#ifndef REG_UTF
 #include </nonexistent>
-#endif],cyrus_cv_pcre_utf8=yes,cyrus_cv_pcre_utf8=no))
-            AC_MSG_RESULT($cyrus_cv_pcre_utf8)
+#endif],cyrus_cv_pcre2_utf8=yes,cyrus_cv_pcre2_utf8=no))
+            AC_MSG_RESULT($cyrus_cv_pcre2_utf8)
         else
-            cyrus_cv_pcre_utf8="no"
+            cyrus_cv_pcre2_utf8="no"
         fi
 fi
 
 LIB_REGEX=
-if test "$cyrus_cv_pcre_utf8" = "yes"; then
-        LIB_REGEX="-lpcre -lpcreposix";
+if test "$cyrus_cv_pcre2_utf8" = "yes"; then
+        LIB_REGEX="-lpcre2-posix";
         AC_DEFINE(ENABLE_REGEX, [], [Do we have a regex library?])
-        AC_DEFINE(HAVE_PCREPOSIX_H, [], [Do we have usable pcre library?])
+        AC_DEFINE(HAVE_PCRE2POSIX_H, [], [Do we have usable pcre2 library?])
 else
         AC_CHECK_HEADERS(rxposix.h)
         if test "$ac_cv_header_rxposix_h" = "yes"; then
diff --git a/imap/cyr_buildinfo.c b/imap/cyr_buildinfo.c
index d8c4a96..c2db3b0 100644
--- a/imap/cyr_buildinfo.c
+++ b/imap/cyr_buildinfo.c
@@ -181,7 +181,7 @@ static json_t *buildinfo()
 #else
     json_object_set_new(dependency, "jansson", json_false());
 #endif
-#if defined(ENABLE_REGEX) && defined(HAVE_PCREPOSIX_H)
+#if defined(ENABLE_REGEX) && defined(HAVE_PCRE2POSIX_H)
     json_object_set_new(dependency, "pcre", json_true());
 #else
     json_object_set_new(dependency, "pcre", json_false());
diff --git a/lib/util.h b/lib/util.h
index 6ac085c..dbeef25 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -71,16 +71,17 @@
 extern const char CYRUS_VERSION[];
 
 #ifdef ENABLE_REGEX
-# ifdef HAVE_PCREPOSIX_H
-#  include <pcre.h>
-#  include <pcreposix.h>
-# else /* !HAVE_PCREPOSIX_H */
+# if defined(HAVE_PCRE2POSIX_H) && !defined(PCRE2_CODE_UNIT_WIDTH)
+#  define PCRE2_CODE_UNIT_WIDTH 8
+#  include <pcre2.h>
+#  include <pcre2posix.h>
+# elif !defined(HAVE_PCRE2POSIX_H) /* !HAVE_PCRE2POSIX_H */
 #  ifdef HAVE_RXPOSIX_H
 #   include <rxposix.h>
 #  else /* !HAVE_RXPOSIX_H */
 #   include <regex.h>
 #  endif /* HAVE_RXPOSIX_H */
-# endif /* HAVE_PCREPOSIX_H */
+# endif /* HAVE_PCRE2POSIX_H */
 #endif /* ENABLE_REGEX */
 
 #ifdef HAVE_LIBUUID
diff --git a/sieve/bc_eval.c b/sieve/bc_eval.c
index f14a5d7..9c3d67c 100644
--- a/sieve/bc_eval.c
+++ b/sieve/bc_eval.c
@@ -316,9 +316,9 @@ static int regcomp_flags(int comparator, int requires)
 {
     int cflags = REG_EXTENDED;
 
-#ifdef HAVE_PCREPOSIX_H
+#ifdef HAVE_PCRE2POSIX_H
     /* support UTF8 comparisons */
-    cflags |= REG_UTF8;
+    cflags |= REG_UTF;
 #endif
 
     if (comparator == B_ASCIICASEMAP) {
diff --git a/sieve/comparator.h b/sieve/comparator.h
index b043bc2..8ff05b0 100644
--- a/sieve/comparator.h
+++ b/sieve/comparator.h
@@ -47,16 +47,17 @@
 #include <sys/types.h>
 
 #ifdef ENABLE_REGEX
-# ifdef HAVE_PCREPOSIX_H
-#  include <pcre.h>
-#  include <pcreposix.h>
-# else /* !HAVE_PCREPOSIX_H */
+# if defined(HAVE_PCRE2POSIX_H) && !defined(PCRE2_CODE_UNIT_WIDTH)
+#  define PCRE2_CODE_UNIT_WIDTH 8
+#  include <pcre2.h>
+#  include <pcre2posix.h>
+# elif !defined(HAVE_PCRE2POSIX_H) /* !HAVE_PCRE2POSIX_H */
 #  ifdef HAVE_RXPOSIX_H
 #   include <rxposix.h>
 #  else /* !HAVE_RXPOSIX_H */
 #   include <regex.h>
 #  endif /* HAVE_RXPOSIX_H */
-# endif /* HAVE_PCREPOSIX_H */
+# endif /* HAVE_PCRE2POSIX_H */
 #endif /* ENABLE_REGEX */
 
 #include "sieve_interface.h"
diff --git a/sieve/sieve.y b/sieve/sieve.y
index b080c26..8692fc1 100644
--- a/sieve/sieve.y
+++ b/sieve/sieve.y
@@ -2215,9 +2215,9 @@ static int verify_regexlist(sieve_script_t *sscript,
     regex_t reg;
     int cflags = REG_EXTENDED | REG_NOSUB;
 
-#ifdef HAVE_PCREPOSIX_H
+#ifdef HAVE_PCRE2POSIX_H
     /* support UTF8 comparisons */
-    cflags |= REG_UTF8;
+    cflags |= REG_UTF;
 #endif
 
     if (collation == B_ASCIICASEMAP) {

@ArchangeGabriel
Copy link

I’m not sure whether @loqs intended to post the patch without comments, but if you read the Arch ticket linked above, you will see they figured out that this one (plus removing sieve/sieve.c in the release tarball and running autoreconf -fi to regenerate it) makes cyrus-imapd compile with pcre2 and all tests passing. And this solves #1848 / #2629.

yselkowitz added a commit to yselkowitz/cyrus-imapd that referenced this issue Jul 11, 2023
Classic pcre is unmaintained and being dropped from distributions.

Fixes: cyrusimap#3861
Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
@yselkowitz yselkowitz linked a pull request Jul 11, 2023 that will close this issue
yselkowitz added a commit to yselkowitz/cyrus-imapd that referenced this issue Jul 11, 2023
Classic pcre is unmaintained and being dropped from distributions.
pcre2posix.h is missing idempotent guards, so handle that ourselves.

Fixes: cyrusimap#3861
Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants