Re: new features with no documentation
Ken, Partha, Thanks for the explanation. > chardet is used by the JMAP module of httpd to detect the character set of untagged 8-bit headers. Does that mean that these libs are required? If not, what would happen if not included? How Cyrus would detect the charset? Or put in other words, if they are not required, what's the benefit to use them? On the other hand, may I suggest to please add these details to the Compiling [1] page? There's already a lot of useful information, but some of the libs lack any details on what's their purpose in Cyrus and why they are beneficial/required (and if they are not used yet, maybe they shouldn't be mentioned there at all?). This could help porters and maintainers to make more educated decisions on how to package Cyrus. Thanks, Anatoli [1] https://www.cyrusimap.org/imap/developer/compiling.html On 3/6/20 19:01, Ken Murchison wrote: > > On 6/3/20 5:44 PM, Partha Susarla wrote: >> Hello Anatoli, >> On Wed, 3 Jun 2020, at 6:24 PM, Anatoli wrote: >>> Cyrus developers, >>> >>> What is the purpose/benefit of zeroskip? >>> >> Zeroskip is an append-only key-value DB, currently in experimental stage. >> >>> What is the purpose of chardet and cld2 in cyrus-imapd? >>> >> Both the character detection and language detection libraries although are >> listed in the dependencies, i don't think are being used. But I will let >> ellie/Ken respond to this. > > > chardet is used by the JMAP module of httpd to detect the character set of > untagged 8-bit headers. > > cld2 is also used by JMAP for language detection but it doesn't look like its > being used by any code on the master branch yet. I believe that Robert S. > has a dev branch that uses it. We can't ask him right now because he's on > holiday the rest of June. > >
Re: run-time dependencies
Thanks, Ken, it's clear now. I wasn't intending to make it adaptable at run-time, just wasn't sure all of them were used as dynamic libraries. I'm accommodating cyrus-imapd package to 3.2 + http, just wanted to confirm this to specify the RUN_DEPENDS list correctly. On 3/6/20 18:52, Ken Murchison wrote: > Ahh, I didn't read your first email well enough. If you configure the build > so that those libraries are linked, then they will be needed at runtime. I'm > not aware of a way to disable functionality in binary based on what runtime > libs are available, but would be willing to make that work with Cyrus is > possible (pull requests welcome). > > > On 6/3/20 5:41 PM, Anatoli wrote: >> Hi Ken, >> >> Thanks for the explanation, though my question was a bit of another >> scope. Please, let me reformulate it. >> >> If we use all these elements during build time, i.e. cyrus-imapd becomes >> compiled with all of them (when using --enable-http), then... will all >> of them be also required (as OS packages) during runtime or we can avoid >> installing some of them on the systems where the cyrus-imapd package >> would be running? >> >> In other words which of them do not constitute runtime dependencies once >> they were used as build-time dependencies? >> >> If all of them are used as shared libraries or some executables invoked >> by some component of cyrus-imapd, then I guess all of them would be >> required during runtime. >> >> If some of them are only used for building the executables or linked as >> static libs, then they should not be needed during runtime, I guess, but >> I'm not sure if there are any. >> >> Thanks, >> Anatoli >> >> On 3/6/20 07:39, Ken Murchison wrote: >>> On 6/3/20 3:48 AM, Anatoli wrote: Cyrus developers, I couldn't find in the documentation the *run-time* dependencies for cyrus-imapd. In particular, are any of these not required to be present for the correct execution of cyrus-imapd (with --enable-http)? pcre e2fsprogs/uuid jansson sqlite3 icu4c libical nghttpd2 brotli wslay >>> >>> pcre is NOT required for httpd at all as far as I recall >>> >>> nghttpd2, brotli, wslay are entirely OPTIONAL >>> >>> sqlite3 is required if you want to use any DAV functionality >>> >>> libical is required if you want to use CalDAV >>> >>> icu4c is required if libical was compiled with support for non-Gregorian >>> calendars >>> >>> jansson is definitely required for JMAP functionality, and may be required >>> for other modules >>> >>> uuid may be prevasive enough to be required for multiple modules >>>
Re: configure: wslay v1.1.1 required but the latest one is 1.1.0
Ken, Why do you believe it could be an issue with Cyrus? It appears the fix was commited 4 years ago and that part was revorked later, so it should not be a problem anymore in 1.1.0. On 3/6/20 07:44, Ken Murchison wrote: > Yes, 1.1.0 is probably sufficient, unless this bug is an issue with Cyrus: > https://github.com/tatsuhiro-t/wslay/pull/47 > > > On 6/3/20 1:19 AM, ellie timoney wrote: >> Cool, thanks for confirming that. So far it's sounding like 1.1.0 is >> probably adequate, but I'll wait a little bit to see if Ken has any input >> once he's been online >> >> On Wed, Jun 3, 2020, at 2:58 PM, Anatoli wrote: >>> Hi Ellie, >>> When you configure it with your patch applied and 1.1.0 installed, >>> does Cyrus build okay? >>> >>> Yes, it builds without errors. >>> >>> Configure prints: >>> >>> checking for WSLAY... yes >>> wslay: yes >>> >>> And -lwslay is passed as arg numerous times during build process. >>> >>> And effectively httpd binary includes references to wslay_event_xxx in >>> its symbols table. >>> >>> Regards, >>> Anatoli >>> >>> On 3/6/20 01:35, ellie timoney wrote: In our "cyruslibs" package, the wslay submodule is at this commit: commit 4a937cd (HEAD, origin/master, origin/HEAD, master) Author: Tatsuhiro Tsujikawa AuthorDate: Fri Jun 8 23:19:03 2018 +0900 Commit: Tatsuhiro Tsujikawa CommitDate: Fri Jun 8 23:19:03 2018 +0900 Bump up version number to 1.1.1-DEV Which is the commit immediately following the release-1.1.0 tag. So, presumably, we're not dependent on any feature/fix that's only in the unreleased version, because otherwise we would've bumped the cyruslibs submodule to include those commits? So, "1.1.1" might be a typo, or an anticipatory thing that didn't go anywhere yet, I'm not sure. When you configure it with your patch applied and 1.1.0 installed, does Cyrus build okay? Cheers, ellie On Wed, Jun 3, 2020, at 2:13 PM, Anatoli wrote: > Cyrus developers, > > The configure script checks for wslay lib version 1.1.1, but the latest > version released is 1.1.0. So when it is installed, it reports: > > checking for WSLAY... no > configure: httpd will not have support for WebSockets. Consider > installing libwslay > > The wslay's github repo has a mention of a 1.1.1-DEV version. Not sure > if cyrus-imapd httpd requires something from it or if it was just a > typo and 1.1.0 is ok. > > For the later case below is a patch. > > Regards, > Anatoli > > > diff --git a/configure.ac b/configure.ac > index dc0e0fff2..30e925c60 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1631,7 +1631,7 @@ dnl AC_MSG_WARN([Your version of > OpenDKIM can not support iSchedu > > AC_ARG_WITH(wslay, [AS_HELP_STRING([--without-wslay], [disable > WebSockets support (check)])],,[with_wslay="check"]) > if test "x$with_wslay" = "xyes" -o "x$with_wslay" = "xcheck"; > then > - PKG_CHECK_MODULES([WSLAY], [libwslay >= 1.1.1], [ > + PKG_CHECK_MODULES([WSLAY], [libwslay >= 1.1.0], [ > AC_DEFINE(HAVE_WSLAY,[], > [Build WebSockets support into httpd?]) > with_wslay=yes >
Re: new features with no documentation
On 6/3/20 5:44 PM, Partha Susarla wrote: Hello Anatoli, On Wed, 3 Jun 2020, at 6:24 PM, Anatoli wrote: Cyrus developers, What is the purpose/benefit of zeroskip? Zeroskip is an append-only key-value DB, currently in experimental stage. What is the purpose of chardet and cld2 in cyrus-imapd? Both the character detection and language detection libraries although are listed in the dependencies, i don't think are being used. But I will let ellie/Ken respond to this. chardet is used by the JMAP module of httpd to detect the character set of untagged 8-bit headers. cld2 is also used by JMAP for language detection but it doesn't look like its being used by any code on the master branch yet. I believe that Robert S. has a dev branch that uses it. We can't ask him right now because he's on holiday the rest of June. -- Kenneth Murchison Senior Software Developer Fastmail US LLC
Re: run-time dependencies
Ahh, I didn't read your first email well enough. If you configure the build so that those libraries are linked, then they will be needed at runtime. I'm not aware of a way to disable functionality in binary based on what runtime libs are available, but would be willing to make that work with Cyrus is possible (pull requests welcome). On 6/3/20 5:41 PM, Anatoli wrote: Hi Ken, Thanks for the explanation, though my question was a bit of another scope. Please, let me reformulate it. If we use all these elements during build time, i.e. cyrus-imapd becomes compiled with all of them (when using --enable-http), then... will all of them be also required (as OS packages) during runtime or we can avoid installing some of them on the systems where the cyrus-imapd package would be running? In other words which of them do not constitute runtime dependencies once they were used as build-time dependencies? If all of them are used as shared libraries or some executables invoked by some component of cyrus-imapd, then I guess all of them would be required during runtime. If some of them are only used for building the executables or linked as static libs, then they should not be needed during runtime, I guess, but I'm not sure if there are any. Thanks, Anatoli On 3/6/20 07:39, Ken Murchison wrote: On 6/3/20 3:48 AM, Anatoli wrote: Cyrus developers, I couldn't find in the documentation the *run-time* dependencies for cyrus-imapd. In particular, are any of these not required to be present for the correct execution of cyrus-imapd (with --enable-http)? pcre e2fsprogs/uuid jansson sqlite3 icu4c libical nghttpd2 brotli wslay pcre is NOT required for httpd at all as far as I recall nghttpd2, brotli, wslay are entirely OPTIONAL sqlite3 is required if you want to use any DAV functionality libical is required if you want to use CalDAV icu4c is required if libical was compiled with support for non-Gregorian calendars jansson is definitely required for JMAP functionality, and may be required for other modules uuid may be prevasive enough to be required for multiple modules -- Kenneth Murchison Senior Software Developer Fastmail US LLC
Re: new features with no documentation
Hello Anatoli, On Wed, 3 Jun 2020, at 6:24 PM, Anatoli wrote: > Cyrus developers, > > What is the purpose/benefit of zeroskip? > Zeroskip is an append-only key-value DB, currently in experimental stage. > What is the purpose of chardet and cld2 in cyrus-imapd? > Both the character detection and language detection libraries although are listed in the dependencies, i don't think are being used. But I will let ellie/Ken respond to this. > Should any be considered in production environments running 3.2 branch? > You can skip installing Zeroskip, as it is not being used in Cyrus-imapd currently. ..Partha
Re: run-time dependencies
Hi Ken, Thanks for the explanation, though my question was a bit of another scope. Please, let me reformulate it. If we use all these elements during build time, i.e. cyrus-imapd becomes compiled with all of them (when using --enable-http), then... will all of them be also required (as OS packages) during runtime or we can avoid installing some of them on the systems where the cyrus-imapd package would be running? In other words which of them do not constitute runtime dependencies once they were used as build-time dependencies? If all of them are used as shared libraries or some executables invoked by some component of cyrus-imapd, then I guess all of them would be required during runtime. If some of them are only used for building the executables or linked as static libs, then they should not be needed during runtime, I guess, but I'm not sure if there are any. Thanks, Anatoli On 3/6/20 07:39, Ken Murchison wrote: > > On 6/3/20 3:48 AM, Anatoli wrote: >> Cyrus developers, >> >> I couldn't find in the documentation the *run-time* dependencies for >> cyrus-imapd. >> >> In particular, are any of these not required to be present for the >> correct execution of cyrus-imapd (with --enable-http)? >> >> pcre e2fsprogs/uuid jansson sqlite3 icu4c libical nghttpd2 brotli wslay > > > pcre is NOT required for httpd at all as far as I recall > > nghttpd2, brotli, wslay are entirely OPTIONAL > > sqlite3 is required if you want to use any DAV functionality > > libical is required if you want to use CalDAV > > icu4c is required if libical was compiled with support for non-Gregorian > calendars > > jansson is definitely required for JMAP functionality, and may be required > for other modules > > uuid may be prevasive enough to be required for multiple modules >
Re: deflatePending not available in zlib on OpenBSD (undefined symbol)
This is my latest proposed fix:
https://github.com/cyrusimap/cyrus-imapd/pull/3061
On 6/2/20 7:34 PM, Anatoli wrote:
Looks good to me and compiles correctly on OpenBSD.
Could it be included in the next 3.2 release (3.2.2)?
On 2/6/20 19:31, Ken Murchison wrote:
Yes, you're correct. We have a comment in prot.c that quotes the same
passage ion the docs.
I think this might do the trick:
diff --git a/imap/httpd.c b/imap/httpd.c
index fc430d935..b5014b97e 100644
--- a/imap/httpd.c
+++ b/imap/httpd.c
@@ -149,24 +149,12 @@ HIDDEN int zlib_compress(struct transaction_t
*txn, unsigned flags,
zstrm->next_in = (Bytef *) buf;
zstrm->avail_in = len;
- buf_ensure(&txn->zbuf, deflateBound(zstrm, zstrm->avail_in));
buf_reset(&txn->zbuf);
do {
int zr;
- if (!zstrm->avail_out) {
- unsigned pending;
-
- zr = deflatePending(zstrm, &pending, Z_NULL);
- if (zr != Z_OK) {
- /* something went wrong */
- syslog(LOG_ERR, "zlib deflate error: %d %s", zr,
zstrm->msg);
- return -1;
- }
-
- buf_ensure(&txn->zbuf, pending);
- }
+ buf_ensure(&txn->zbuf, deflateBound(zstrm, len));
zstrm->next_out = (Bytef *) txn->zbuf.s + txn->zbuf.len;
zstrm->avail_out = txn->zbuf.alloc - txn->zbuf.len;
On 6/2/20 6:19 PM, Anatoli wrote:
Hi Ken,
According to the docs [1]: If deflate returns Z_OK and with zero
avail_out, it must be called again after making room in the buffer
because there might be more output pending.
On the other hand it says: Z_FINISH can be used in the first deflate
call after deflateInit if all the compression is to be done in a single
step. In order to complete in one call, avail_out must be at least the
value returned by deflateBound. Then deflate is guaranteed to return
Z_STREAM_END.
But: Note that it is possible for the compressed size to be larger than
the value returned by deflateBound() if flush options other than
Z_FINISH or Z_NO_FLUSH are used.
At the beginning of zlib_compress() there's code that decides with which
flush option to call deflate().
/* Only flush for static content or on last (zero-length) chunk */
If it's possible to make flush to be always Z_FINISH, then I guess the
entire
do { ... } while (!zstrm->avail_out);
loop could be simplified to just:
zstrm->next_out = (Bytef *) txn->zbuf.s + txn->zbuf.len;
zstrm->avail_out = txn->zbuf.alloc - txn->zbuf.len;
zr = deflate(zstrm, flush);
if (zr != Z_STREAM_END) {
/* something went wrong */
syslog(LOG_ERR, "zlib deflate error: %d %s", zr, zstrm->msg);
return -1;
}
I changed the "if (zr)" condition as the only good value would be
Z_STREAM_END.
But if the flush option can't be always Z_FINISH, then I believe the
loop should be kept with the checks for !zstrm->avail_out as it's
possible that there would be not enough buffer for deflate() to complete
in one call.
Regards,
Anatoli
[1] https://www.zlib.net/manual.html
On 2/6/20 17:36, Ken Murchison wrote:
Hi Anatoli,
Thanks for the report. I'm not sure that we even need the
deflatePending() call, since we use deflateBound() to create an
appropriately-sized buffer to hold the entire compressed response body.
Let me do some testing.
On 6/2/20 3:48 AM, Anatoli wrote:
Cyrus developers,
Is it possible to somehow rework the code in imap/httpd.c lines 158-169
in order to NOT use deflatePending as this func is not available on
OpenBSD? The zlib version there is 1.2.3 and deflatePending appeared in
1.2.5 so the code doesn't compile with --enable-http (undefined symbol:
deflatePending). The packaged version disables http for now.
Is it safe to reduce these lines:
158 if (!zstrm->avail_out) {
159 unsigned pending;
160
161 zr = deflatePending(zstrm, &pending, Z_NULL);
162 if (zr != Z_OK) {
163 /* something went wrong */
164 syslog(LOG_ERR, "zlib deflate error: %d %s", zr,
zstrm->msg);
165 return -1;
166 }
167
168 buf_ensure(&txn->zbuf, pending);
169 }
to something like:
158 if (!zstrm->avail_out) {
159 buf_ensure(&txn->zbuf, 256 * 1024);
160 }
If I understand it correctly, deflatePending in this case is only used
to get the needed buffer size and we could replace it with a hardcoded
buffer size (like in my example of 256K).
Thanks,
Anatoli
--
Kenneth Murchison
Senior Software Developer
Fastmail US LLC
Re: configure: wslay v1.1.1 required but the latest one is 1.1.0
Yes, 1.1.0 is probably sufficient, unless this bug is an issue with Cyrus: https://github.com/tatsuhiro-t/wslay/pull/47 On 6/3/20 1:19 AM, ellie timoney wrote: Cool, thanks for confirming that. So far it's sounding like 1.1.0 is probably adequate, but I'll wait a little bit to see if Ken has any input once he's been online On Wed, Jun 3, 2020, at 2:58 PM, Anatoli wrote: Hi Ellie, When you configure it with your patch applied and 1.1.0 installed, does Cyrus build okay? Yes, it builds without errors. Configure prints: checking for WSLAY... yes wslay: yes And -lwslay is passed as arg numerous times during build process. And effectively httpd binary includes references to wslay_event_xxx in its symbols table. Regards, Anatoli On 3/6/20 01:35, ellie timoney wrote: In our "cyruslibs" package, the wslay submodule is at this commit: commit 4a937cd (HEAD, origin/master, origin/HEAD, master) Author: Tatsuhiro Tsujikawa AuthorDate: Fri Jun 8 23:19:03 2018 +0900 Commit: Tatsuhiro Tsujikawa CommitDate: Fri Jun 8 23:19:03 2018 +0900 Bump up version number to 1.1.1-DEV Which is the commit immediately following the release-1.1.0 tag. So, presumably, we're not dependent on any feature/fix that's only in the unreleased version, because otherwise we would've bumped the cyruslibs submodule to include those commits? So, "1.1.1" might be a typo, or an anticipatory thing that didn't go anywhere yet, I'm not sure. When you configure it with your patch applied and 1.1.0 installed, does Cyrus build okay? Cheers, ellie On Wed, Jun 3, 2020, at 2:13 PM, Anatoli wrote: Cyrus developers, The configure script checks for wslay lib version 1.1.1, but the latest version released is 1.1.0. So when it is installed, it reports: checking for WSLAY... no configure: httpd will not have support for WebSockets. Consider installing libwslay The wslay's github repo has a mention of a 1.1.1-DEV version. Not sure if cyrus-imapd httpd requires something from it or if it was just a typo and 1.1.0 is ok. For the later case below is a patch. Regards, Anatoli diff --git a/configure.ac b/configure.ac index dc0e0fff2..30e925c60 100644 --- a/configure.ac +++ b/configure.ac @@ -1631,7 +1631,7 @@ dnlAC_MSG_WARN([Your version of OpenDKIM can not support iSchedu AC_ARG_WITH(wslay, [AS_HELP_STRING([--without-wslay], [disable WebSockets support (check)])],,[with_wslay="check"]) if test "x$with_wslay" = "xyes" -o "x$with_wslay" = "xcheck"; then -PKG_CHECK_MODULES([WSLAY], [libwslay >= 1.1.1], [ +PKG_CHECK_MODULES([WSLAY], [libwslay >= 1.1.0], [ AC_DEFINE(HAVE_WSLAY,[], [Build WebSockets support into httpd?]) with_wslay=yes -- Kenneth Murchison Senior Software Developer Fastmail US LLC
Re: run-time dependencies
On 6/3/20 3:48 AM, Anatoli wrote: Cyrus developers, I couldn't find in the documentation the *run-time* dependencies for cyrus-imapd. In particular, are any of these not required to be present for the correct execution of cyrus-imapd (with --enable-http)? pcre e2fsprogs/uuid jansson sqlite3 icu4c libical nghttpd2 brotli wslay pcre is NOT required for httpd at all as far as I recall nghttpd2, brotli, wslay are entirely OPTIONAL sqlite3 is required if you want to use any DAV functionality libical is required if you want to use CalDAV icu4c is required if libical was compiled with support for non-Gregorian calendars jansson is definitely required for JMAP functionality, and may be required for other modules uuid may be prevasive enough to be required for multiple modules -- Kenneth Murchison Senior Software Developer Fastmail US LLC
new features with no documentation
Cyrus developers, What is the purpose/benefit of zeroskip? What is the purpose of chardet and cld2 in cyrus-imapd? Should any be considered in production environments running 3.2 branch? Thanks, Anatoli
run-time dependencies
Cyrus developers, I couldn't find in the documentation the *run-time* dependencies for cyrus-imapd. In particular, are any of these not required to be present for the correct execution of cyrus-imapd (with --enable-http)? pcre e2fsprogs/uuid jansson sqlite3 icu4c libical nghttpd2 brotli wslay Thanks, Anatoli
