Re: commits 7f452687 and cedf154ea6
On Wed, Feb 20, 2013, at 09:13 AM, Дилян Палаузов wrote: Hallo Greg, I checked very fast your commit from yesterday and ... Thanks Dilyan, I greatly appreciate all reviews :) I've cc'd the development list to help show that review is good and should be public, hope you don't mind. commit cedf154ea6dc3e0350023b8cf74e4b1ac8bb4484 Author: Greg Banks g...@fastmail.fm Date: Mon Jul 2 20:59:32 2012 +1000 configure checks for MySQL sensibly ... removes the variable $use_mysql from configure.ac, but $use_mysql is used on the third last line in configure.ac to show if mysql was found; Ah, this is the danger of merging in months-old commits. That code at the end of configure.ac did not exist when I was first writing my commit. So I think the end summary is really kind of pointless. For example, under External dependencies only the PCRE library is listed, but Cyrus has *heaps* of external libraries it can use in various situations to provide extra features, like krb5, openssl, uuid, gssapi and even zephyr (whatever that is). But I don't see any good reason to break it if other people find it useful, so please see commit ea0c572 Fix configure summary for MySQL commit 7f452687a563aff45da070647a6ad32936f13785 Author: Greg Banks g...@fastmail.fm Date: Mon Jul 9 15:05:46 2012 +1000 util: fix subtle bug in buf_replace_*() when given a buf which has been initialised with buf_init_ro_cstr(), buf_replace_*() will ensure that the target buf is a C string (for searching) but not that it's writable (for replacing) which are sometimes but not always the same thing. ... adds #include assert.c to lib/util.c, but util.c already contains #include assert.c by the time the commit was pushed at git.cyrusimap.org . Another subtle merge issue, thanks for finding that. I've removed the assert.h, as we want to be obviously and explicitly including lib/assert.h not /usr/include/assert.h, in a way which will break at compile time should someone later break the -I compile options. I also went around and fixed some other #includes of a similar nature. See commit cad2b6e Normalise use of #include assert.h -- Greg.
Re: Re-designing cyrus.cache format
On Tue, Feb 12, 2013, at 08:44 PM, Bron Gondwana wrote: As you can see, there are some normalised things from some headers. The same information normalised in a DIFFERENT way in the ENVELOPE and then a BODYSTRUCTURE and a BODY response. We have already changed the normalisation rules here a couple of times. There are two benefits to doing this. 1: reduced CPU usage re-parsing the fields for fast responses. Maybe this was true once, but current fastmail code has to reparse everything from cache, and it's a crawling horror of bizarre special cases. See for example message2_parse_cbodystructure() at https://github.com/gnb/cyrus-imapd/blob/fastmail/imap/message.c#L4150 So - I would propose this: 1) keep the BODYSTRUCTURE, it's the result of parsing the entire message, and can't be calculated cheaply again 2) keep the SECTION data (possibly along with the bodystructure) - it's the offsets for the various parts of the message, same issue These two are inconsistent with each other, full of bizarre edge cases, and don't map well onto MIME structures or what the modern and flexible fastmail message2 code needs for its internal representation. They need to be a) unified into one b) redesigned to be more easily parseable into data structures and not to be compatible with IMAP wire format c) redesigned to be more MIME-friendly 3) add a list of SUPPRESSED HEADERS. This would list any header which is present in the file, but NOT in the cache. Easily discovered from the field_desc_t structures in the mesage2 code. 4) cache every other header, including all the To:, From:, Subject:, etc - in as close to raw form as possible. This could be trivially achieved as a binary dump of the part_t tree in the message2 code. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #1069
On Mon, Jan 7, 2013, at 09:13 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/1069/ -- [...truncated 4455 lines...] + '[' -d .git ']' ++ git ls-files ++ wc -l + nfiles=99 + '[' 99 -gt 0 ']' + git ls-files -o broken.log cass.errs cassandane.ini coverage.xml e.fpl errs errs2 find-process-leak.sh reports.old/TEST-Cassandane.Cyrus.Annotator.xml reports.old/TEST-Cassandane.Cyrus.Bug3072.xml reports.old/TEST-Cassandane.Cyrus.Bug3463.xml reports.old/TEST-Cassandane.Cyrus.Bug3470.xml reports.old/TEST-Cassandane.Cyrus.Bug3649.xml reports.old/TEST-Cassandane.Cyrus.Delete.xml reports.old/TEST-Cassandane.Cyrus.Delivery.xml reports.old/TEST-Cassandane.Cyrus.Fetch.xml reports.old/TEST-Cassandane.Cyrus.Flags.xml reports.old/TEST-Cassandane.Cyrus.Idle.xml reports.old/TEST-Cassandane.Cyrus.Info.xml reports.old/TEST-Cassandane.Cyrus.Lsub.xml reports.old/TEST-Cassandane.Cyrus.Master.xml reports.old/TEST-Cassandane.Cyrus.Metadata.xml reports.old/TEST-Cassandane.Cyrus.Nntp.xml reports.old/TEST-Cassandane.Cyrus.Pop3.xml reports.old/TEST-Cassandane.Cyrus.Quota.xml reports.old/TEST-Cassandane.Cyrus.Rename.xml reports.old/TEST-Cassandane.Cyrus.Replication.xml reports.old/TEST-Cassandane.Cyrus.Search.xml reports.old/TEST-Cassandane.Cyrus.Sieve.xml reports.old/TEST-Cassandane.Cyrus.Simple.xml reports.old/TEST-Cassandane.Test.Address.xml reports.old/TEST-Cassandane.Test.Cassini.xml reports.old/TEST-Cassandane.Test.Clone.xml reports.old/TEST-Cassandane.Test.Config.xml reports.old/TEST-Cassandane.Test.DateTime.xml reports.old/TEST-Cassandane.Test.Message.xml reports.old/TEST-Cassandane.Test.MessageStoreFactory.xml reports.old/TEST-Cassandane.Test.Metronome.xml reports.old/TEST-Cassandane.Test.Sample.xml reports/TEST-Cassandane.Cyrus.Annotator.xml reports/TEST-Cassandane.Cyrus.Bug3072.xml reports/TEST-Cassandane.Cyrus.Bug3463.xml reports/TEST-Cassandane.Cyrus.Bug3470.xml reports/TEST-Cassandane.Cyrus.Bug3649.xml reports/TEST-Cassandane.Cyrus.Delete.xml reports/TEST-Cassandane.Cyrus.Delivery.xml reports/TEST-Cassandane.Cyrus.Fetch.xml reports/TEST-Cassandane.Cyrus.Flags.xml reports/TEST-Cassandane.Cyrus.Idle.xml reports/TEST-Cassandane.Cyrus.Info.xml reports/TEST-Cassandane.Cyrus.Lsub.xml reports/TEST-Cassandane.Cyrus.Master.xml reports/TEST-Cassandane.Cyrus.Metadata.xml reports/TEST-Cassandane.Cyrus.Nntp.xml reports/TEST-Cassandane.Cyrus.Pop3.xml reports/TEST-Cassandane.Cyrus.Quota.xml reports/TEST-Cassandane.Cyrus.Rename.xml reports/TEST-Cassandane.Cyrus.Replication.xml reports/TEST-Cassandane.Cyrus.Search.xml reports/TEST-Cassandane.Cyrus.Sieve.xml reports/TEST-Cassandane.Cyrus.Simple.xml reports/TEST-Cassandane.Test.Address.xml reports/TEST-Cassandane.Test.Cassini.xml reports/TEST-Cassandane.Test.Clone.xml reports/TEST-Cassandane.Test.Config.xml reports/TEST-Cassandane.Test.DateTime.xml reports/TEST-Cassandane.Test.Message.xml reports/TEST-Cassandane.Test.MessageStoreFactory.xml reports/TEST-Cassandane.Test.Metronome.xml reports/TEST-Cassandane.Test.Sample.xml utils/gdbtramp utils/gdbtramp.o utils/lemming utils/lemming.o working.log + git status # On branch master # Untracked files: # (use git add file... to include in what will be committed) # # broken.log # coverage.xml # e.fpl # errs # errs2 # find-process-leak.sh # working.log nothing added to commit but untracked files present (use git add to track) + make make[1]: Entering directory `http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/cassandane/utils' make[1]: Nothing to be done for `all'. make[1]: Leaving directory `http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/cassandane/utils' testrunner.pl syntax OK Cassandane/ThreadedGenerator.pm syntax OK Cassandane/Config.pm syntax OK Cassandane/MasterEntry.pm syntax OK Cassandane/Cyrus/Quota.pm syntax OK Cassandane/Cyrus/Delivery.pm syntax OK Cassandane/Cyrus/Conversations.pm syntax OK Cassandane/Cyrus/Rename.pm syntax OK Cassandane/Cyrus/Bug3463.pm syntax OK Cassandane/Cyrus/Fetch.pm syntax OK Cassandane/Cyrus/Bug3470.pm syntax OK Cassandane/Cyrus/Delete.pm syntax OK Cassandane/Cyrus/Pop3.pm syntax OK Cassandane/Cyrus/Metadata.pm syntax OK Cassandane/Cyrus/Info.pm syntax OK Cassandane/Cyrus/Nntp.pm syntax OK Cassandane/Cyrus/Bug3649.pm syntax OK Cassandane/Cyrus/TestCase.pm syntax OK Cassandane/Cyrus/Sieve.pm syntax OK Cassandane/Cyrus/Replication.pm syntax OK Cassandane/Cyrus/Master.pm syntax OK Cassandane/Cyrus/Flags.pm syntax OK Cassandane/Cyrus/Lsub.pm syntax OK Cassandane/Cyrus/Idle.pm syntax OK Cassandane/Cyrus/Bug3072.pm syntax OK Cassandane/Cyrus/Search.pm syntax OK Cassandane/Cyrus/Annotator.pm syntax OK Cassandane/Cyrus/Simple.pm syntax OK Cassandane/Test/Config.pm syntax OK Cassandane/Test/Sample.pm syntax OK Cassandane/Test/Message.pm syntax OK Cassandane/Test/DateTime.pm syntax
Re: Build failed in Jenkins: cyrus-imapd-master #1126
On Mon, Feb 4, 2013, at 09:12 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/1126/ Test failures and errors summary Cassandane::Cyrus::Metadata.msg_replication_exp_mas http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_msg_replication_exp_mas/ Cassandane::Cyrus::Metadata.msg_replication_mod_bot_msl http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_msg_replication_mod_bot_msl/ Cassandane::Cyrus::Metadata.permessage_getset http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_permessage_getset/ Cassandane::Cyrus::Metadata.permessage_unknown_allowed http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_permessage_unknown_allowed/ Cassandane::Cyrus::Metadata.msg_replication_new_bot_mse_gul http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_msg_replication_new_bot_mse_gul/ Cassandane::Cyrus::Metadata.msg_replication_new_mas http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_msg_replication_new_mas/ Cassandane::Cyrus::Metadata.permessage_unknown http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_permessage_unknown/ Cassandane::Cyrus::Metadata.folder_delete_msg_dmimm http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_folder_delete_msg_dmimm/ Cassandane::Cyrus::Metadata.nonexistant_mailbox http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_nonexistant_mailbox/ Cassandane::Cyrus::Metadata.msg_replication_mod_mas http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_msg_replication_mod_mas/ Cassandane::Cyrus::Nntp.cve_2011_3208_newnews http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Nntp/test_cve_2011_3208_newnews/ Cassandane::Cyrus::Nntp.cve_2011_3208_list_active http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Nntp/test_cve_2011_3208_list_active/ Cassandane::Cyrus::Nntp.cve_2011_3208_list_newsgroups http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Nntp/test_cve_2011_3208_list_newsgroups/ Cassandane::Cyrus::Pop3.subfolder_login http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Pop3/test_subfolder_login/ Cassandane::Cyrus::Quota.quota_f_prefix http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_quota_f_prefix/ Cassandane::Cyrus::Quota.using_message_late http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_using_message_late/ Cassandane::Cyrus::Quota.using_annotstorage_msg_copy_deimm http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_using_annotstorage_msg_copy_deimm/ Cassandane::Cyrus::Quota.using_annotstorage_msg_copy_dedel http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_using_annotstorage_msg_copy_dedel/ Cassandane::Cyrus::Quota.upgrade_v2_4 http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_upgrade_v2_4/ Cassandane::Cyrus::Quota.replication_annotstorage http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_replication_annotstorage/ Cassandane::Cyrus::Quota.quota_f_nested_qr http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_quota_f_nested_qr/ Cassandane::Cyrus::Quota.using_message http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_using_message/ Cassandane::Cyrus::Quota.replication_storage http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_replication_storage/ Cassandane::Cyrus::Quota.bz3529 http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_bz3529/ Cassandane::Cyrus::Quota.exceeding_message http://ci.cyrusimap.org/job/cyrus-imapd-master/1126//testReport/%28root%29/Cassandane__Cyrus__Quota/test_exceeding_message/ Cassandane::Cyrus::Quota.quota_f_vs_update
Re: Build failed in Jenkins: cyrus-imapd-master #1059
On Tue, Jan 8, 2013, at 10:02 PM, Sébastien Michel wrote: 2013/1/3 Bron Gondwana br...@fastmail.fm: I vote for using the technically best library. It's probably small enough to that we can ship it inside the Cyrus tarballs if we need. It's only 132k in the src directory of the latest jannson, and it's MIT licensed. 2013/1/4 Greg Banks g...@fastmail.fm: As long as the feature gracefully disables itself if the library is not installed at configure time, I'm really not fussed which library we use. For the CI server we'll probably need to build it manually, like a whole bunch of other things. If we choose to include the latest jansson sources in the Cyrus tarbal, It should be only if the jansson library is missing on the host server. In this case event notification may be always compiled and it will be possible to make some cleanup : - removing test for event-notification in configure.ac - removing ENABLE_MBOXEVENT C macro in some source files (imapd.c, pop3d.c, etc.) Those can be made cleaner anyway, by moving them into mboxevent.c. Then the mailbox.c code would always call mboxevent_* functions but those functions would, depending on the result of ./configure, either be a trivial stub that does do nothing or would actually do the work of pushing the event out. - removing MBOXEVENT macro in Makefile.am In the scenario above you would always compile mboxevent.c, so this automake conditional would not be needed. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #1059
On Wed, Jan 2, 2013, at 09:31 PM, Sébastien Michel wrote: 2013/1/2 Bron Gondwana br...@fastmail.fm: Sounds good to me. I had a brief glance through today. It looks nice at a first reading. I have a couple of questions: Thank you. We can also warmly thank Greg who has taken some time to proofread and give advices on the first draft. No worries :) -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #1059
On Thu, Jan 3, 2013, at 01:20 AM, Jeroen van Meeuwen (Kolab Systems) wrote: On 2013-01-02 15:07, Sébastien Michel wrote: I used initially json-c that is the most common. However, only the trunk offered support to 64bit integers. This is why I switched to jansson that is also valuable. Unfortunately, it is less common than json-c, and for which the version 0.10 is now available in EPEL. Should I change back the JSON library? there is still time before releasing Cyrus 2.5 As long as the feature gracefully disables itself if the library is not installed at configure time, I'm really not fussed which library we use. For the CI server we'll probably need to build it manually, like a whole bunch of other things. - We should consider upgrading our CI system to Enterprise Linux 6, I think we should *add* a RHEL6 CI slave. And a Solaris slave and a Ubuntu slave. And build across them all. -- Greg.
Re: commit mboxevent: Rewrite JSON formatting
Sent from my iPhone On 16/08/2012, at 0:46, Bron Gondwana br...@fastmail.fm wrote: On Wed, Aug 15, 2012, at 01:41 PM, Leena Heino wrote: On 14.8.2012 20:07, Sébastien Michel wrote: I would say the minimal version of pkg-config. Indeed, version 0.24 or greater is required in order to avoid the duplicate AC_SUBST macro calls for PKG_CHECK_MODULES substitutions. This version is available since May 2010. That requirement would rule out systems like RHEL 5, RHEL 6. Is this a requirement for build from git only, or a requirement for build from theoretical distribution tarballs? I want the tarballs to build on old machines for sure. I wouldn't describe RHEL 5 and 6 as old yet, their end of production dates are in 2017 and 2020 respectively. https://access.redhat.com/support/policy/updates/errata/ Running such systems out to a large fraction of their vendors' nominal production lifespan is a perfectly sensible approach for a site to take, and I don't see a valid reason to exclude such folks from being able to contribute to the project via git as well as just use it. Just because their CIO has a desire for stability does not exclude them from Freedom #1 http://www.gnu.org/philosophy/free-sw.html Sébastien, it might be easiest to copy a recent working version of the PKG_CHECK_MODULES macro into cmulocal/ so it works on all current platforms. Assuming of course licencing allows. You may need to rename it to ensure the correct version is used, I'm not sure what the include priority is. Also, thanks for coming back with the revisions, I really appreciate your efforts. Unfortunately until today I've been up to my ears in fixing Cyrus search and now I'm going on holiday, so I won't be able to give your work the attention it deserves for some time. Greg.
Re: commit mboxevent: Rewrite JSON formatting
Sent from my iPhone On 15/08/2012, at 2:40, Дилян Палаузовdilyan.palau...@aegee.org wrote: Hello Michel, according to my understanding, there are no strict rules what goes in libcyrus, libcyrus_min and libcyrus_imap . I am not aware of a guidelines, describing in which library to include what kind of file. While working on getting Automake/libtool in cyrus-imapd, I moved some files between libcyrus and libcyrus_min according to how I felt the things. I think we should definitely come up with a written definition of these and encode it in the Cyrus docs. The current situation is way too confused. Here's my idea of what the splits should be. The split between libcyrus_imap and the others is that it contains higher level model code, i.e. mailboxes, messages, and the like. The others contain utility code, strings and the like. There are some grey areas and some arguably misplaced code, eg parseaddr.c. The split between libcyrus and libcyrus_min is more easily defined: if the master process needs it to link, it must be in libcyrus_min, otherwise it must be in libcyrus. This strongly minimizes the amount of code in the master process, which is necessary because the master process cannot be upgraded gracefully in place (most of the other processes such as imapd will detect when their binary has been overwritten and exit so that master can restart them, but master itself cannot do this). By this definition we have a major problem where lib/util.c drags in a lot of unnecessary stuff into master. That needs fixing, presumably by splitting up util.c. We also have several other daemons that cannot be gracefully upgraded in place, notably idled. That also needs fixing. lib/parseaddr.c is used within libcyrus_sieve and imap/, so having it in the common library libcyrus seems logical. I do not know, why iostat.c is part of libcyrus. I think parseaddr should be part of libcyrus_imap; it's clearly a domain model object. I also think libcyrus_sieve should just use more things from libcyrus_imap instead of reimplementing them badly or delegating them to poorly implemented callbacks. My work on the search branch will help move us in that direction by providing a useful message abstraction. If nobody else expresses opinion, whether to put xjson in libcyrus or libcyrus_imap, it is up to you. I just told you my opinion. My 2c: JSON code is a utility and should go in libcyrus. I have no opinion about the minimal required version for json. The comment in the Makefile state, that LD_BASIC_ADD is used to link with programs, linking with libcyrus and libcyrus_min, while LD_UTILITY_ADD is used to link with all command line tools (that are not invoked as services from master). -lcrypto is always provided, even if the linked program does not use libcrypto. Със здраве Дилян On 14.08.2012 16:21, Sébastien Michel wrote: please delete imap/Makefile.in : all the build rules are in /Makefile.am . It's already done. Sorry, I just forgot to mention it. As libjson supports the .pc format, you can detect libjson in configure.ac with PKG_CHECK_MODULES ([libjson], [json = 0.10], [check_libjson=yes], [check_libjson=no]) and remove the configure.ac:AC_CHECK_LIB handling. Possibly add an AM_CONDITIONAL([LIBJSON], [$check_libjson = yes]). I know currently cyrus-imapd does not use the PKG_CHECK_MODULES-routine, but on the other side there are no other external modules, which can be detected by it. I do not have recipe to how to trivially #define HAVE_LIBJSON when using libjson, but this shall not be hard to handle. I've used PKG_CHECK_MODULES and it looks fine. However I have noticed some issues : - By default, the macro will set up two variables, joining the given prefix with the suffixes _CFLAGS and _LIBS. The macro will call AC_SUBST on these variables only with pkg-config 0.24 and later What do you prefer between always calling AC_SUBST or testing the pkg-config version ? - Using PKG_CHECK_MODULES in conditional block would raise a failure. Calling PKG_PROG_PKG_CONFIG seems to be one possible solution : http://www.flameeyes.eu/autotools-mythbuster/pkgconfig/pkg_check_modules.html#idp1027760 - some people discourage the use of such macro : http://stackoverflow.com/questions/10220946/pkg-check-modules-considered-harmful I am in favour of PKG_(PROG_)PKG_CONFIG, but if you prefer not to use it, then you can leave configure.ac as it is. OK. Can we consider that we support only version 0.24 minimum ? Then shift the lib/xjson.c and lib/xjson.h to imap/, xjson.[ch] doesn't depends on any structure/function in imap folder. As a basic utility I think it should be located in lib. Why do you prefer move it here ? Provided that the json code is integrated and used within libcyrus_imap (and not libcyrus(_min), then the files have to go under imap/ -- there are all libcyrus_imap sources. While for the
Re: commit mboxevent: Rewrite JSON formatting
Sent from my iPhone On 24/08/2012, at 19:49, Sébastien Michel sebastien.mic...@atos.net wrote: 2012/8/24 Greg Banks g...@fastmail.fm: If nobody else expresses opinion, whether to put xjson in libcyrus or libcyrus_imap, it is up to you. I just told you my opinion. My 2c: JSON code is a utility and should go in libcyrus. Indeed. As discussed on IRC, we decided to change the library to format to JSON, from libjson (that will support 64bit integer only in the next release) to jansson that is also a mature library, available on major Linux distro and already support 64bit. Cool. The last debate is on the bugzilla ticket #3605. It is about removing the internal xjson.[ch] json formatter and add a default option --disable-event-notification or --enable-event-notification and force requirement on jansson library if enabled. That involves decorating the code with a C macro in all source files that refer to mboxevent.h. Personally I'd be happier with only one set of code to test. Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #770
On Tue, Aug 14, 2012, at 07:11 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/770/ [...] Cassandane::Test::Metronome.basic http://ci.cyrusimap.org/job/cyrus-imapd-master/770//testReport/%28root%29/Cassandane__Test__Metronome/test_basic/ Ok, enough of this. http://git.cyrusimap.org/cassandane/commit/?id=74d79fc1c6689dc153d786dba8bc71268e7450eb -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #756
On Tue, Aug 7, 2012, at 07:12 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/756/changes Changes: [gnb] Bug #3726 Add a portability stub for posix_fadvise [gnb] Update KR code in portability stubs. [gnb] Bug #3726 Add a portability stub for strsep [gnb] Bug #3726 Add a portability stub for memmem [...] Cassandane::Test::Metronome.basic http://ci.cyrusimap.org/job/cyrus-imapd-master/756//testReport/%28root%29/Cassandane__Test__Metronome/test_basic/ Cassandane::Test::Metronome.test_basic (from Cassandane__Test__Metronome) Failing for the past 1 build (Since Failed#756 ) Took 1 sec. Error Message Standard deviation 1.43380587135031 is too high Hmm, this failure had nothing to do with the recent commits - that test is not even running C code. The test is just time sensitive. This is the second time it's happened recently; perhaps the physical hardware is more heavily loaded these days. If it happens again I'll bump up the threshold in the test. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #756
On Tue, Aug 7, 2012, at 08:58 PM, Bron Gondwana wrote: On Tue, Aug 7, 2012, at 11:56 AM, Greg Banks wrote: On Tue, Aug 7, 2012, at 07:12 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/756/changes Changes: [gnb] Bug #3726 Add a portability stub for posix_fadvise [gnb] Update KR code in portability stubs. [gnb] Bug #3726 Add a portability stub for strsep [gnb] Bug #3726 Add a portability stub for memmem [...] Cassandane::Test::Metronome.basic http://ci.cyrusimap.org/job/cyrus-imapd-master/756//testReport/%28root%29/Cassandane__Test__Metronome/test_basic/ Cassandane::Test::Metronome.test_basic (from Cassandane__Test__Metronome) Failing for the past 1 build (Since Failed#756 ) Took 1 sec. Error Message Standard deviation 1.43380587135031 is too high Hmm, this failure had nothing to do with the recent commits - that test is not even running C code. The test is just time sensitive. This is the second time it's happened recently; perhaps the physical hardware is more heavily loaded these days. If it happens again I'll bump up the threshold in the test. For sure. Why are we caring about relative speed of the virtual machine across time? Master has a fork rate limiting feature. It was very broken. Now it's fixed and has a test. That test needs to measure the actual fork rate that master allows when fed a rate of connection attempts which is above the configured fork rate limit. To generate that load, the test uses the Metronome class, which uses the POSIX realtime API to generate a controlled average rate of events. The test that is failing, measures the rate stability of the Metronome class itself. It doesn't need to be super accurate, but we do need to know if it's like 50% wrong. Spurious test failures just lead to being ignored :( Sure. -- Greg.
Re: cyrus 2.4.16, duplicate suppression = false and sieve redirects
Sent from my iPhone
On 03/08/2012, at 2:30, Chris Stromsoe c...@ucla.edu wrote:
Setting duplicate suppression to false does not disable duplicate suppression
for sieve redirects in 2.4.16.
From lmtp_sieve.c, starting line 378:
/* if we have a msgid, we can track our redirects */
if (m-id) {
snprintf(buf, sizeof(buf), %s-%s, m-id, rc-addr);
sievedb = make_sieve_db(sd-username);
dkey.id = buf;
dkey.to = sievedb;
dkey.date = ((deliver_data_t *) mc)-m-date;
/* ok, let's see if we've redirected this message before */
if (duplicate_check(dkey)) {
duplicate_log(dkey, redirect);
return SIEVE_OK;
}
}
Is the missing check a bug or feature?
Feature, surprisingly. Cyrus uses its duplicate db for three separate purposes,
only one of which is to prevent duplicate delivery and is properly controlled
by the config option. This is one of the others. You can tell by what goes in
the key.id field.
BTW, now that you've read the code, it would be great to see a patch adding
some comments for the next guy, or even (gasp) some tests.
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #730
On Wed, Jul 25, 2012, at 07:12 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/730/ Test failures and errors summary Cassandane::Cyrus::Bug3470.list_2011 http://ci.cyrusimap.org/job/cyrus-imapd-master/730//testReport/%28root%29/Cassandane__Cyrus__Bug3470/test_list_2011/ Cassandane::Cyrus::Delivery.duplicate_suppression_off http://ci.cyrusimap.org/job/cyrus-imapd-master/730//testReport/%28root%29/Cassandane__Cyrus__Delivery/test_duplicate_suppression_off/ Cassandane::Cyrus::Flags.seen http://ci.cyrusimap.org/job/cyrus-imapd-master/730//testReport/%28root%29/Cassandane__Cyrus__Flags/test_seen/ Cassandane::Cyrus::Idle.disabled http://ci.cyrusimap.org/job/cyrus-imapd-master/730//testReport/%28root%29/Cassandane__Cyrus__Idle/test_disabled/ Iiinteresting. I've raised bz3722 for this, see the ticket for preliminary analysis. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #728
On Tue, Jul 24, 2012, at 07:11 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/728/ [...] Test failures and errors summary Cassandane::Test::Metronome.basic http://ci.cyrusimap.org/job/cyrus-imapd-master/728//testReport/%28root%29/Cassandane__Test__Metronome/test_basic/ Standard deviation 1.00591923328965 is too high This test is used to calibrate the variability of the Metronome code, which is in turn used to test the fork rate limiting code in the master process. The threshold at which the std is considered too high is 1.0, so this only just barely failed. Considering this has been running for months on a VM, I'm pretty happy with that failure rate and I'm going to just ignore this test failure. -- Greg.
Re: -fvisibility=hidden
On Fri, Jul 20, 2012, at 12:13 AM, Дилян Палаузов wrote:
Hello,
On 19.07.2012 03:11, Greg Banks wrote:
On Wed, Jul 18, 2012, at 10:46 PM, Дилян Палаузов wrote:
On 16.07.2012 02:08, Greg Banks wrote:
That's a lot of exported symbols for such a small library :(
But you could fix that without advanced non-portable linker trickery, by
(for example) moving chunks of sieve/tree.c into sieve/sieve.y, or
#include'ing one into the other or vice versa.
The most T-symbols are generated by flex/bison and this cannot be
tricked. Flex can be instructed not to generate unnecessary functions
with %option noyyset_in or alike, however flex on ci.cyrus-imapd.org
does not support these options. So the functions are there.
You could always post-process the generated C code at build time with a
two-line Perl script which inserts the word static in front of
function definitions and/or declarations.
Moving functions from tree.c to sieve.y is marginal compared to the
amount of functions created by bison/flex.
The non-portable trickery does not harm. It is supported by GCC and
Clang (according to
http://clang-developers.42468.n3.nabble.com/Does-clang-support-attribute-visibility-quot-default-quot-td3944043.html
).
There are still some other compilers left in the world. The Sun, sorry
Oracle, compiler, for example, will parse and ignore both
__attribute__(visibility()) in the code and -fvisibility=hidden on the
commandline, without failing.
-bash-3.00$ cat foo.c
#include stdio.h
#include stdlib.h
#include unistd.h
int foo(int x)
{
return x+42;
}
__attribute__((visibility(default))) int bar(int x)
{
return x-42;
}
-bash-3.00$ ld -V
ld: Software Generation Utilities - Solaris Link Editors: 5.10-1.493
-bash-3.00$ cc -V
cc: Sun C 5.10 SunOS_i386 2009/06/03
usage: cc [ options] files. Use 'cc -flags' for details
-bash-3.00$ cc -Kpic -fvisibility=hidden -c -o foo.o foo.c
cc: Warning: Option -fvisibility=hidden passed to ld, if ld is invoked,
ignored otherwise
-bash-3.00$ cc -Kpic -G -o libfoo.so foo.o
-bash-3.00$ nm foo.o | egrep '(foo|bar)'
foo.o:
[17]|32| 26|FUNC |GLOB |0|2 |bar
[16]| 0| 26|FUNC |GLOB |0|2 |foo
[1] | 0| 0|FILE |LOCL |0|ABS|foo.c
-bash-3.00$ nm libfoo.so | egrep '(foo|bar)'
libfoo.so:
[42]| 592| 26|FUNC |GLOB |0|4 |bar
[43]| 560| 26|FUNC |GLOB |0|4 |foo
[31]| 0| 0|FILE |LOCL |0|ABS|foo.c
[1] | 0| 0|FILE |LOCL |0|ABS|libfoo.so
-bash-3.00$ nm -D libfoo.so | egrep '(foo|bar)'
libfoo.so:
[6] | 592| 26|FUNC |GLOB |0|4 |bar
[7] | 560| 26|FUNC |GLOB |0|4 |foo
nm -D does the same thing as the GNU nm, i.e. prints the symbols visible
to the dynamic linker.
Here, you're lucky that -fvisibility=hidden is effectively ignored by
the compile stage, and is harmlessly misinterpreted by the linker as
-f name
Useful only when building a shared object. Specifies
that the symbol table of the shared object is used as an
auxiliary filter on the symbol table of the shared
object specified by name. Multiple instances of this
option are allowed. This option can not be combined with
the -F option.
I think that you can solve this problem in a more portable way, that
actually does the useful thing you want to achieve on all platforms,
without breaking our link semantics on any platform.
--
Greg.
Re: -fvisibility=hidden
On Wed, Jul 18, 2012, at 10:46 PM, Дилян Палаузов wrote:
Hello,
On 16.07.2012 02:08, Greg Banks wrote:
So, remind me again what actual value we're getting from this
-fvisibility=hidden stuff again?
Initially, libcyrus_sieve had a lot of exported symbols generated by
bison and flex. When libcyrus_sieve was loaded by the dynamic linker
the list of exported symbols (section .dynsyms) was long, so finding the
needed symbol by the dynamic linker was supposed to take long time.
Well, maybe. On Linux at least, the linker puts a hashtable index to
the exported symbols into a section called .gnu.hash and the runtime
linker uses that to speed up searching for symbols. So lookup is going
to be something closer to O(1) than O(N). So I'm unconvinced by an
argument from runtime link performance.
There is an argument to be made from untidiness.
gnb@gnb-desktop 2069 nm -D --defined-only
./sieve/.libs/libcyrus_sieve.so | awk '{h[$2]++}END{for (i in h) print
h[i],i}'
3 A
10 B
3 D
115 T
That's a lot of exported symbols for such a small library :(
But you could fix that without advanced non-portable linker trickery, by
(for example) moving chunks of sieve/tree.c into sieve/sieve.y, or
#include'ing one into the other or vice versa.
Annotating in libcyrus_sieve (and the other libraries) the
functions/symbols with EXPORTED, that are needed outside the library,
and compiling with -fvisibility=hidden, keeps the list of exported
symbols short, so the dynamic linker can load the library faster. On
the other side the resulting library is smaller. In addition, I think
this makes things easier to maintain, as it is clear, if a function is
used outside the library (EXPORTED), only within the library (HIDDEN),
or only within the source file, it is defined (static)
I think -fvisibility=hidden is an excellent mechanism for folks
maintaining real actual libraries with defined and documented interfaces
at both the source and ABI levels, to enforce that users of their
libraries use only the supported interfaces. Cyrus really isn't one of
those. We have no documentation, no ABI guarantee, no versioning
mechanism. Worse, we have a lot of truly horrible tricks which rely on
traditional linktime semantics, fatal() being the obvious example.
Those can and should be fixed, but right now they're pretty broken.
Compiling different Automake targets with different CPPFLAGS/CFLAGS
creates .o files with very long names, when the non standarf AM_CFLAGS
are used. The reason for the long name is, that Automake assumes, that
a file can be compiled once with the non-standard (not Makefile.am-wide)
CFLAGS, and once with AM_CFLAGS, so Automake reserves its right to
create different .o files from the same .c file. this could be avoided,
if -fvisibility=hidden is used not only for libraries but also for
executables and is put in AM_CFLAGS.
Sure, if it's one place it should be in as many places as possible.
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #717
On Wed, Jul 18, 2012, at 11:42 PM, Bron Gondwana wrote:
On Wed, Jul 18, 2012, at 05:03 PM, Jenkins wrote:
Running unit tests
./cunit-to-junit.pl: ran 278 tests, 2 failed
make[3]: *** [check-local] Error 1
make[3]: Leaving directory
`http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory
`http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory
`http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'
make: *** [check] Error 2
+ fatal 'Can'\''t make check'
+ echo 'imapd/tools/jenkins-build.sh: Can'\''t make check'
imapd/tools/jenkins-build.sh: Can't make check
Of course it bloody runs clean on my machine. Short of logging in and...
./cunit/byteorder64.testc:12: CU_ASSERT_EQUAL(ntohll(*((uint64_t
*)(b64)))=72057594037927936,1=1)
Oh shit.
So what's different from my machine? The architecture certainly isn't.
This means that cmu/master is super-dangerous right now for anyone not
building on the same platform as me.
Greg - any ideas?? It should be using be64toh from endian.h on Linux
according to the code.
Hmm.
static void test_byteorder(void)
{
const char b64[8] = { 0, 0, 0, 0, 0, 0, 0, 1 };
const char b32[4] = { 0, 0, 0, 1 };
char i64[8];
char i32[4];
Here's your first problem, the alignment of all these variables is
uncontrolled. The code is only working by accident because the C
runtime environment gives you an aligned stack, and also because x86
doesn't care so much about alignment.
/* test 64 bit */
CU_ASSERT_EQUAL(ntohll(*((uint64_t *)(b64))), 1);
*((uint64_t *)i64) = htonll(1);
CU_ASSERT_EQUAL(memcmp((char *)i64, b64, 8), 0);
/* test 32 bit */
CU_ASSERT_EQUAL(ntohl(*((uint32_t *)(b32))), 1);
*((uint32_t *)i32) = htonl(1);
CU_ASSERT_EQUAL(memcmp((char *)i32, b32, 4), 0);
}
In the header file lib/byteorder64.h
/* http://stackoverflow.com/a/4410728/94253 */
#if defined(__linux__)
# include endian.h
#elif defined(__FreeBSD__) || defined(__NetBSD__)
# include sys/endian.h
#elif defined(__OpenBSD__)
# include sys/types.h
# define be16toh(x) betoh16(x)
# define be32toh(x) betoh32(x)
# define be64toh(x) betoh64(x)
#elif defined(WORDS_BIGENDIAN)
#define CYRUS_BYTESWAP
#endif
That could do with some improvement.
Solaris also has an optimised ntohll() in libc which uses the 64b bswap
instruction, although it's not inlined.
http://cvs.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/amd64/gen/byteorder.s
and gcc has a builtin which uses the instruction on platforms which have
it
http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-g_t_005f_005fbuiltin_005fbswap64-
which presumably will work on some platforms where Linux's endian.h
(which re-implements the same thing differently) is not available.
Disassembling cunit/byteorder64.o, I don't see any bswap instructions in
there, but nm shows undefined symbols for libc's htonl and ntohl.
Writing a function that calls ntohll() and nothing else, it seems that
ntohll() expands to a no-op. Running gcc -E confirms this.
Aha..
#ifdef be64toh
#define htonll(x) htobe64(x)
#define ntohll(x) be64toh(x)
#elif defined (CYRUS_BYTESWAP)
/* little-endian 64-bit host/network byte-order swap functions */
extern unsigned long long _htonll(unsigned long long);
extern unsigned long long _ntohll(unsigned long long);
#define htonll(x) _htonll(x)
#define ntohll(x) _ntohll(x)
#else
#define htonll(x) (x)
#define ntohll(x) (x)
#endif
The endian.h on my desktop defines a be64toh(). The same header on
ci.cyrusimap.org doesn't.
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #717
On Thu, Jul 19, 2012, at 06:46 AM, Bron Gondwana wrote: On Thu, Jul 19, 2012, at 12:02 PM, Greg Banks wrote: /* http://stackoverflow.com/a/4410728/94253 */ [...] That could do with some improvement. Looks like stackoverflow might indeed be full of it. I guess we should really test for each of those in configure.in and then #define the right inline instruction. I guess - except that it's hard in configure to tell the difference between a slow and a fast ntohl() provided by libc. Configure is really good at telling you if something is available, not whether it's fast. Perhaps you could test for the availability, in order, of 1. __builtin_bswap32() from gcc, or 2. ntohl() from libc, or 3. be32toh() from libc and then do the same sequence again for the 64b versions, but add 4. fallback to Cyrus' own code The endian.h on my desktop defines a be64toh(). The same header on ci.cyrusimap.org doesn't. Yeah, damn. Is it really ancient, or 32 bit, or something? Ancient. It doesn't have be32toh() either. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #710
On Sun, Jul 15, 2012, at 05:03 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/710/ [...] /bin/sh ./libtool --tag=CC --mode=link gcc -fPIC --coverage -fvisibility=hidden -g -O0-o ptclient/ptloader ptclient/ptloader.o imap/mutex_fake.o master/service-thread.o ptclient/ldap.o imap/libcyrus_imap.la -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -ldb-4.3 -lz -lwrap -lnsl -lldap -llber -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -ldb-4.3 -lz libtool: link: gcc -fPIC --coverage -fvisibility=hidden -g -O0 -o ptclient/.libs/ptloader ptclient/ptloader.o imap/mutex_fake.o master/service-thread.o ptclient/ldap.o imap/.libs/libcyrus_imap.so -luuid http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/lib/.libs/libcyrus_min.so http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/lib/.libs/libcyrus.so -lsasl2 -lssl -lcrypto -lwrap -lnsl -lldap -llber -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support /usr/lib64/libdb-4.3.so -lpthread -lz -Wl,-rpath -Wl,/usr/cyrus/lib -Wl,-rpath -Wl,/usr/lib64 /usr/bin/ld: ptclient/.libs/ptloader: hidden symbol `fatal' in ptclient/ptloader.o is referenced by DSO /usr/bin/ld: final link failed: Nonrepresentable section on output collect2: ld returned 1 exit status So, remind me again what actual value we're getting from this -fvisibility=hidden stuff again? -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #711
G'day Dilyan,
On Sun, Jul 15, 2012, at 05:03 PM, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/711/changes
[...]
libtool: link: gcc -fPIC --coverage -fvisibility=hidden -g -O0 --coverage
-o cunit/.libs/unit cunit/unit.o cunit/syslog.o cunit/timeout.o
cunit/annotate.o cunit/backend.o cunit/binhex.o cunit/buf.o
cunit/charset.o cunit/crc32.o cunit/db.o cunit/dlist.o cunit/duplicate.o
cunit/getxstring.o cunit/glob.o cunit/guid.o cunit/hash.o cunit/imapurl.o
cunit/mboxname.o cunit/md5.o cunit/message.o cunit/msgid.o
cunit/parseaddr.o cunit/parse.o cunit/prot.o cunit/ptrarray.o
cunit/quota.o cunit/sieve.o cunit/spool.o cunit/squat.o cunit/strarray.o
cunit/strconcat.o cunit/times.o cunit/tok.o imap/mutex_fake.o
imap/spool.o sieve/.libs/libcyrus_sieve.so imap/.libs/libcyrus_imap.so
-luuid
http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/lib/.libs/libcyrus_min.so
http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/lib/.libs/libcyrus.so
-lsasl2 -lssl -lcrypto -lcunit -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err
-lkrb5support /usr/lib64/libdb-4.3.so -lpthread -lz -Wl,-rpath
-Wl,/usr/cyrus/lib -Wl,-rpath -Wl,/usr/lib64
make[3]: Leaving directory
`http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'
make check-local
make[3]: Entering directory
`http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'
Running unit tests
./cunit-to-junit.pl: ran 277 tests, 13 failed
make[3]: *** [check-local] Error 1
Some of the tests in cunit/annotate.testc depend on catching the output
of syslog(), like this
1267 static void test_missing_definitions_file(void)
1268 {
1269 set_annotation_definitions(NULL);
1270 CU_SYSLOG_MATCH(annotations\\.def: could not open.*No such
file);
1271
1272 annotate_init(NULL, NULL);
1273 /* if we got here, we didn't fatal() */
1274
1275 /* but we did complain to syslog */
1276 CU_ASSERT_SYSLOG(/*all*/0, 1);
1277 }
The CU_SYSLOG_MATCH() macro tells the test framework to remember a
regexp to be matched against later messages passed to syslog() by the
Code Under Test. The CU_ASSERT_SYSLOG() macro asserts that the count of
matches for (0 = all) previously registered regexps is equal to the 2nd
argument = 1. That assert is failing because no emitted syslog messages
matched the regexp. The most probable explanation is that your
visibility changes have prevented code in libcyrus_imap.so from linking
against the syslog() symbol defined in cunit/syslog.c and instead
they're getting the syslog in libc.
--
Greg.
Re: linking with libtool and
On Sun, Jul 15, 2012, at 12:26 PM, Дилян Палаузов wrote: Hello, I reverted the changes causing the warnings with static in the header files. The linking problem, Bron mentioned on 12.07., was is caused by (./libtool --config)'s link_all_deplibs=no . Does anybody have an idea, when is link_all_deplibs set to no, and when to yes/unknown? On my machine it's set in configure 8957 link_all_deplibs=unknown ... 8980 case $host_os in ... 8996 linux* | k*bsd*-gnu) 8997 link_all_deplibs=no 8998 ;; 8999 esac This is with autoconf 2.67, automake 1.11.1 libtool 2.2.6b. Looking at the configure code, it's set to 'yes' on those platforms where the dynamic linker is incapable of, or due to bugs intermittently capable of, transitively following library dependencies on other libraries at program link time. In other words, the correct value for all Linux variants is 'no'. -- Greg.
Re: static and HIDDEN
On Thu, Jul 12, 2012, at 03:05 PM, Дилян Палаузов wrote: Hello, Dilyan, this warning is caused by some of your recent commits, like libcyrus: mark all local symbols declared in .h with static which made some of the function declarations in header files 'static'. That's just never right. Also, you've marked a number of cyrusdb_*() functions as 'static'. These are actual external APIs used by code, albeit in the conversations branch, not master. This is just not helpful. If you care that much about external symbols, remove the functions entirely and we'll put them back when we merge the conversations code. Or just leave them alone. Yes, attaching static to all variables and functions, that are not used out of the library, was sometimes inappropriate. I do not have problems with either of the approaches to remove the functions entirely, or to mark them as part of the external API, which will be used by future versions. On the other side, there shall be some track of functions, which are really unnecessary (like lib/util.c:kv_bsearch()), and marking them with HIDDEN or static eases their finding. That particular one doesn't seem to have ever been used. It was introduced by commit 307c689e3e099f437a732bcc659be8f90549a5f2 Author: John Gardiner Myers j...@andrew.cmu.edu Date: Mon Mar 14 22:47:51 1994 + Initial revision and was not used then, nor has any other code used it since. It appears to have copied in from some other code at CMU. I do not know which of the remaining HIDDEN symbols (in libcyrus and libcyrus_imap; in cyrus_min and cyrus_sieve internal symbols are not annotated), belong to future external API, and which will be strictly internal for the library but are included in a header file Agreed, it's very hard to tell. It doesn't help that the FastMail practice is to put changes that affect common code, e.g. by adding new functions, into separate commits, which after a few days or weeks end up in the master branch without the code that needs them. Nor does it help that in this particular case the new functions were added but no unit tests were added for them (in any branch). Nor does it help that lots of development is occurring off the master branch, and not even on git.cyrusimap.org. Bron's latest stuff is usually at https://github.com/brong/cyrus-imapd/tree/fastmail and here's the branch I'm working on https://github.com/gnb/cyrus-imapd/commits/search Other than that, there's not much help I can offer -- Cyrus is just old and dusty and untidy and you have to dig around to figure anything out and every now and again you discover a landmine. (e.g. why is fuzzy_match() part of lmtpd.h, when it is defined and used entirely in lmtpd.c and thus shall be static?). That's probably historical -- a lot of Cyrus code was written decades ago when compilers had fewer warnings. In KR C. On CVS. Probably before CVS too. Using git-blame -s I see the declaration in the header was added in commit 51791d1e012695e15ff375f42dcfe121630f1a4d Author: Ken Murchison mu...@andrew.cmu.edu Date: Thu Oct 20 15:29:01 2005 + Added support for Sieve scripts on shared mailboxes via the /vendor/cmu/cyrus-imapd/sieve annotation but the definition was added in commit eed0f295b090a3cbee5e2298001c7692fb62435f Author: Ken Murchison mu...@andrew.cmu.edu Date: Thu Nov 30 17:11:16 2006 + moved 2.3 code to trunk which was before the CVS - git migration so the CVS branch merge basically lost all useful development history. But it looks like the function was never used outside lmtpd.c, so probably it should always have been static and never should have been added to the header. I think your best bet is to use git-blame -s and git-show to work out when the function was first added (as opposed to moved from another file, or had it's declaration changed to ANSI C or some such change). If it was added in the last three years, your first guess should be that it was made extern deliberately, otherwise that it was an accident. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #689
G'day, Bron, you shouldn't need to specify -fPIC when you override CFLAGS anymore, as automake will add that into AM_CFLAGS which is defined separately and used in addition to CFLAGS. This is why -fPIC appears twice on each compile command in your make log. On Thu, Jul 12, 2012, at 12:48 AM, Bron Gondwana wrote: On Thu, Jul 12, 2012, at 12:17 AM, Дилян Палаузов wrote: Hello, can you please send the last lines of make before the crash, your versions of automake and libtool, and the content of libcyrus_imap.la ? My output of make is attached. From it can be seen, on the lines for cyr_synclog, that libtool is invoked to link with libcyrus_imap.la, and it expands to libcyrus_imap.so, libcyrus.so and libcyrus_min.so . Trying it without the -j: libtool: compile: gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -I/usr/include -fPIC -g -fPIC -W -Wall -Wextra -MT lib/hashu64.lo -MD -MP -MF lib/.deps/hashu64.Tpo -c lib/hashu64.c -fPIC -DPIC -o lib/.libs/hashu64.o /bin/bash ./libtool --tag=CC --mode=link gcc -fPIC -g -fPIC -W -Wall -Wextra -o perl/libcyrus_min.la lib/assert.lo lib/hash.lo lib/imapopts.lo lib/libconfig.lo lib/lock_fcntl.lo lib/map_shared.lo lib/mpool.lo lib/retry.lo lib/strarray.lo lib/strhash.lo lib/util.lo lib/xmalloc.lo lib/xstrlcat.lo lib/xstrlcpy.lo lib/hashu64.lo -ldb-4.8 -lpcre -lpcreposix -lz libtool: link: ar cru perl/.libs/libcyrus_min.a lib/.libs/assert.o lib/.libs/hash.o lib/.libs/imapopts.o lib/.libs/libconfig.o lib/.libs/lock_fcntl.o lib/.libs/map_shared.o lib/.libs/mpool.o lib/.libs/retry.o lib/.libs/strarray.o lib/.libs/strhash.o lib/.libs/util.o lib/.libs/xmalloc.o lib/.libs/xstrlcat.o lib/.libs/xstrlcpy.o lib/.libs/hashu64.o libtool: link: ranlib perl/.libs/libcyrus_min.a libtool: link: ( cd perl/.libs rm -f libcyrus_min.la ln -s ../libcyrus_min.la libcyrus_min.la ) gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -I/usr/include -fPIC -fvisibility=hidden -g -fPIC -W -Wall -Wextra -MT imtest/imtest_imtest-imtest.o -MD -MP -MF imtest/.deps/imtest_imtest-imtest.Tpo -c -o imtest/imtest_imtest-imtest.o `test -f 'imtest/imtest.c' || echo './'`imtest/imtest.c ./lib/prot.h:156:12: warning: ‘prot_flush_internal’ declared ‘static’ but never defined [-Wunused-function] Dilyan, this warning is caused by some of your recent commits, like libcyrus: mark all local symbols declared in .h with static which made some of the function declarations in header files 'static'. That's just never right. Also, you've marked a number of cyrusdb_*() functions as 'static'. These are actual external APIs used by code, albeit in the conversations branch, not master. This is just not helpful. If you care that much about external symbols, remove the functions entirely and we'll put them back when we merge the conversations code. Or just leave them alone. /bin/bash ./libtool --tag=CC --mode=link gcc -fPIC -g -fPIC -W -Wall -Wextra -o imap/arbitron imap/arbitron.o imap/cli_fatal.o imap/mutex_fake.o imap/libcyrus_imap.la -ldb-4.8 -lpcre -lpcreposix -lz -ldb-4.8 -lpcre -lpcreposix -lz libtool: link: gcc -fPIC -g -fPIC -W -Wall -Wextra -o imap/.libs/arbitron imap/arbitron.o imap/cli_fatal.o imap/mutex_fake.o imap/.libs/libcyrus_imap.so -ldb-4.8 -lpcre -lpcreposix -lz -Wl,-rpath -Wl,/usr/cyrus/lib /usr/bin/ld: imap/arbitron.o: undefined reference to symbol 'xstrdup' /usr/bin/ld: note: 'xstrdup' is defined in DSO /usr/cyrus/lib/libcyrus_min.so.0 so try adding it to the linker command line /usr/cyrus/lib/libcyrus_min.so.0: could not read symbols: Invalid operation ??? -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #689
On Wed, Jul 4, 2012, at 05:03 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/689/changes Changes: [git-dpa] update .gitignore to include config.(sub,guess) and install-sh [git-dpa] update .gitignore [git-dpa] config.h: add #define EXPORTED and HIDDEN [git-dpa] libcyrus_sieve: hide all internal symbols [git-dpa] imap/libcyrus_imap: hide symbol search_prefilter_messages() [git-dpa] fixup: libcyrus_sieve: hide all internal symbols [git-dpa] mark local variable as static [git-dpa] imap/libcyrus_imap: mark some internal variables as static [git-dpa] lib/libcyrus_min: hide function beautify_code [git-dpa] imap/libcyrus_imap: make even more variables static and hidden [...] cunit/sieve.o: In function `test_comparator': http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/./cunit/sieve.testc:85: undefined reference to `lookup_comp' http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/./cunit/sieve.testc:86: undefined reference to `lookup_comp' http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/./cunit/sieve.testc:87: undefined reference to `lookup_comp' http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/./cunit/sieve.testc:88: undefined reference to `lookup_comp' http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/./cunit/sieve.testc:89: undefined reference to `lookup_comp' cunit/sieve.o:http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/./cunit/sieve.testc:91: more undefined references to `lookup_comp' follow collect2: ld returned 1 exit status This broke because Dilyan's linkage changes hid one too many symbols in libsieve. He's fixed it in commit http://git.cyrusimap.org/cyrus-imapd/commit/?id=7a94a84ff546ebb8d2ed4ef4255e32396b0c5afb It's great to see people actually responding to Jenkins build failures :) -- Greg.
Re: minimal required version of (optional) tools
On Sun, Jul 1, 2012, at 12:07 AM, Дилян Палаузов wrote: Hello, is it possible to define in doc/install-compile.html the minimum versions of the tools, a developer needs in order to re-generate all source files need for the compilation of cyrus-imapd? The needed tools are autoconf, automake, bison, (f)lex, gperf, libtool and possibly compile_et . with the version requirements autoconf = 2.67 automake = 1.10 libtool = 2.2.6 What about the minimum required version of (f)lex, bison and gperf? gperf is checked for, only when ./configure --maintainer-mode . If gperf is not found, ./configure aborts (I think). However, ./configure always checks for lex and bison, and does not fail, if they are not found (I think). Can we stick to flex instead of lex? In any case, flex 2.5.35 supports %option reentrant noyyget_FUNCTIO and noyyset_FUNCTION, which options are not supported by lex on ci.cyrusimap.org . noyyget_ and noyyset_ permit to exclude some unneeded functions from sieve/addr-lex.c and sieve/sieve-lex.c, which results smaller .c files and smaller tables of exported symbols (which in theory means less time for the dynamic linker to find an exported symbol). I think some good rules of thumb for any OSS project would be: 1) if we have any doubts about the version of tools needed to generate parts of the source, then ship the generated source in the dist tarball and hide the make rules in maintainer mode. The code should be buildable from a dist tarball on the widest possible range of platforms. 2) For maintainer mode, the ideal situation is that versions of tools available in the current Long Term Support releases of common target platforms should work without difficulty. 3) For maintainer mode, if a tool is needed to build then check for it in configure and fail configure if the tool is either not present or is missing a necessary feature. Ideally we would be enforcing 2) by having CI machines for all supported target platforms. Currently we have only one CI machine and it's running an old CentOS. We really should add some more. Also, as a project we really should have some kind of policy about which platforms we officially support. I'm guessing the list would be something like RHEL, SLES, Ubuntu Server, and Solaris. Re the specific question of flex, I believe both bison and flex are supported and easily installable on all those target platforms, including Solaris (in SFW or whatever Oracle call it now). -- Greg.
Re: minimal required version of (optional) tools
On Mon, Jul 2, 2012, at 11:15 AM, Bron Gondwana wrote: And the makefiles we ship with release tarballs better bloody work everywhere or the panda will be sad. Good point. Once we get the dist: target sorted out in the new build system, we should set up a Jenkins job which builds a dist tarball from git and then builds from the tarball. Speaking of which...Dilyan, what is the status of the dist: target? -- Greg.
Re: Libtool and Support for Shared Libraries (3)
On Fri, Jun 29, 2012, at 02:20 AM, Florian Pflug wrote: On Jun28, 2012, at 01:53 , Greg Banks wrote: On Wed, Jun 27, 2012, at 04:36 PM, Dilyan Palauzov wrote: Moreover, does it make any difference, if imapd links with libcyrus_sieve and libcyrus_sieve links with libcom_err, or if imapd links explicitly with both libcyrus_sieve and libcom_err ? Very little. It really only matters for shared libraries which are going to be dlopen()ed, of which we currently have none Hm, I think this matters on OSX. On that OS, you only see the symbols exported by libraries you link to directly, not the ones pulled in indirectly. Interesting. The OSX ld(1) manage says Indirect dynamic libraries If the command line specifies to link against dylib A, and when dylib A was built it linked against dylib B, then B is considered an indirect dylib. When linking for two-level namespace, ld does not look at indi- rect dylibs, except when re-exported by a direct dylibs. On the other hand when linking for flat namespace, ld does load all indirect dylibs and uses them to resolve references. Even though indirect dylibs are specified via a full path, ld first uses the specified search paths to locate each indirect dylib. If one cannot be found using the search paths, the full path is used. So it seems to make a difference to symbol visibility - symbols in directly linked libs have more visibility. In that case, we actually *want* to change to linking -lcom_err directly so that the main executables can call error_message(). -- Greg.
Re: Libtool and Support for Shared Libraries (3)
On Wed, Jun 27, 2012, at 07:58 AM, Bron Gondwana wrote:
On Wed, Jun 27, 2012, at 08:54 AM, Greg Banks wrote:
On Wed, Jun 27, 2012, at 12:27 AM, Bron Gondwana wrote:
/usr/bin/ld.bfd.real: /usr/lib/libcom_err.a(error_message.o): relocation
R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared
object; recompile with -fPIC
/usr/lib/libcom_err.a: could not read symbols: Bad value
So what am I doing wrong,
Nothing.
or in the alternative what is the build system
doing wrong?
There is a path through configure.ac which sets
COM_ERR_LIBS=${with_com_err}/lib/libcom_err.a
and COM_ERR_LIBS gets used to link against shared libraries in
Makefile.am
imap_libcyrus_imap_la_LIBADD = $(COM_ERR_LIBS) $(LIB_UUID)
...
sieve_libcyrus_sieve_la_LIBADD = $(COM_ERR_LIBS)
...
I would guess you did something like
./configure --with-com_err=/usr
but you should be able to get a better result by just dropping the
option entirely.
Ahh, yep - so I was! Looking better now :)
It's a bug; --with_com_err=/usr should be the same as no option. Raise
a bugzilla ticket.
libtool: install: warning: `sieve/libcyrus_sieve.la' has not been
installed in `/usr/cyrus-future/lib'
libtool: install: warning: `lib/libcyrus.la' has not been installed in
`/usr/cyrus-future/lib'
libtool: install: warning: `lib/libcyrus_min.la' has not been installed
in `/usr/cyrus-future/lib'
libtool: install: /usr/bin/install -c sieve/.libs/sievec
/usr/src/cyrus-future-build/cyrus/debian/cyrus-future/usr/cyrus-future/bin/sievec
but that's only a warning :)
Yeah, libtool warnings...grumble mumble...
--
Greg.
Re: Libtool and Support for Shared Libraries (3)
On Wed, Jun 27, 2012, at 04:36 PM, Dilyan Palauzov wrote: Hello, as far as I understand, the systemwide libcom_err.a is non PIC, the libcyrus_imap and libcyrus_sieve are PIC and it is not portable to dymically link non-PIC libcom_err.a with PIC libcyrus_sieve . Yes, but how to proceed in this case? Currently, Makefile.am states, that libcyrus_imap depends on libcom_err, and when some application links with libcyrus_imap, it automatically links with the system wide libcom_err.a (implicit linking with shared libraries vs. explicit linking when everything is statically built). The trouble is twofold: First, there are two paths through configure which are intended to link against the system com_err library, but one results in passing -lcom_err to the libcyrus_imap link phase (which works because it finds the system libcom_err.so) and the other passes /usr/lib/libcom_err.a (which doesn't work for the reasons you've pointed out). They should both do -lcom_err. Second, if we have any doubts about whether a library is shared or static we should pass it as -lfoo to the link line for the executable, not for the Cyrus shared library. Either works in the former case, only shared library work in the latter. Does somebody know if it is portable to link the PIC executable (e.g. imapd) The executable shouldn't be PIC, that's only needed for shared libraries or things that will eventually be linked into shared libraries. with PIC libcyrus_sieve, and with non-PIC libcyrus_imap ? Moreover, does it make any difference, if imapd links with libcyrus_sieve and libcyrus_sieve links with libcom_err, or if imapd links explicitly with both libcyrus_sieve and libcom_err ? Very little. It really only matters for shared libraries which are going to be dlopen()ed, of which we currently have none -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #674
On Wed, Jun 27, 2012, at 05:08 PM, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/674/
--build-url=http://ci.cyrusimap.org/job/cyrus-imapd-master/674/
Test failures and errors summary
Cassandane::Cyrus::Metadata.specialuse
http://ci.cyrusimap.org/job/cyrus-imapd-master/674//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_specialuse/
Lots of tests were failing with Some process is already listening on
127.0.0.1:9100, the highly annoying Cassandane cascading failure mode.
Cassandane::Cyrus::Metadata.msg_replication_mod_bot_msl
http://ci.cyrusimap.org/job/cyrus-imapd-master/674//testReport/%28root%29/Cassandane__Cyrus__Metadata/test_msg_replication_mod_bot_msl/
This was an actual bug
Error Message
Perl exception: Core files found in /var/tmp/cass/21032384/conf/cores
Core was generated by
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/sync_clie'.
Program terminated with signal 6, Aborted.
#0 0x003665830265 in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x003665830265 in raise () from /lib64/libc.so.6
#1 0x003665831d10 in abort () from /lib64/libc.so.6
#2 0x00366586a84b in __libc_message () from /lib64/libc.so.6
#3 0x00366587230f in _int_free () from /lib64/libc.so.6
#4 0x00366587276b in free () from /lib64/libc.so.6
#5 0x2abf9a6df754 in dlist_free (dlp=0x7fff951d71d8) at
imap/dlist.c:650
#6 0x0040a20c in mailbox_full_update (mboxname=0x18d164c0
user.cassandane) at imap/sync_client.c:1366
#7 0x0040ac77 in update_mailbox (local=0x18d15f10,
remote=0x18d16b50, reserve_guids=0x18d16760)
at imap/sync_client.c:1502
#8 0x0040c139 in do_folders (mboxname_list=0x18d14ab0,
replica_folders=0x18d14a90, delete_remote=1)
at imap/sync_client.c:1825
#9 0x0040cc70 in do_user_main (user=0x7fff951d9b0f
cassandane, replica_folders=0x18d14a90,
replica_quota=0x18d15a70) at imap/sync_client.c:2000
#10 0x0040d9bf in do_user (userid=0x7fff951d9b0f cassandane)
at imap/sync_client.c:2204
#11 0x00412588 in main (argc=9, argv=0x7fff951d8768) at
imap/sync_client.c:3246
Hmm
646 void dlist_free(struct dlist **dlp)
647 {
648 if (!*dlp) return;
649 _dlist_clean(*dlp);
650 free((*dlp)-name);
651 free(*dlp);
652 *dlp = NULL;
653 }
1361mailbox_close(mailbox);
1362
1363dlist_free(kin);
1364dlist_free(kaction);
1365dlist_free(kexpunge);
1366dlist_free(kuids);
1367
1368return r;
1369}
Ah, that line was added recently by Bron on the basis of an analysis I
made on an internal Fastmail mailing list. Let's look at the code a bit
harder.
1217 static int mailbox_full_update(const char *mboxname)
1218 {
…
1225 struct dlist *kuids = NULL;
…
1321 kexpunge = dlist_newkvlist(NULL, EXPUNGE);
1322 dlist_setatom(kexpunge, MBOXNAME, mailbox-name);
1323 dlist_setatom(kexpunge, UNIQUEID, mailbox-uniqueid); /* just
for safety */
1324 kuids = dlist_newlist(kexpunge, UID);
1325 for (ka = kaction-head; ka; ka = ka-next) {
1326 if (!strcmp(ka-name, EXPUNGE)) {
1327 dlist_setnum32(kuids, UID, dlist_num(ka));
1328 }
1329 else if (!strcmp(ka-name, COPYBACK)) {
1330 r = copy_remote(mailbox, dlist_num(ka), kr);
1331 if (r) goto cleanup;
1332 dlist_setnum32(kuids, UID, dlist_num(ka));
1333 }
1334 else if (!strcmp(ka-name, RENUMBER)) {
1335 r = copy_local(mailbox, dlist_num(ka));
1336 if (r) goto cleanup;
1337 }
1338 }
…
1365 dlist_free(kexpunge);
1366 dlist_free(kuids);
Whoops, I was completely wrong about leaking kuids. Now we are
double-freeing it. The 'kuids' pointer actually points into the dlist
tree whose root is pointed to by 'kexpunge'. So we're freeing the kuids
dlists at line 1365, then once more at the new line 1366. My bad :(
Fixed in commit
http://git.cyrusimap.org/cyrus-imapd/commit/?id=6f10cc844e297d31d2c3ed8ec83a818533cdbf90
--
Greg.
Re: Libtool and Support for Shared Libraries (3)
On Wed, Jun 27, 2012, at 12:27 AM, Bron Gondwana wrote:
/usr/bin/ld.bfd.real: /usr/lib/libcom_err.a(error_message.o): relocation
R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared
object; recompile with -fPIC
/usr/lib/libcom_err.a: could not read symbols: Bad value
So what am I doing wrong,
Nothing.
or in the alternative what is the build system
doing wrong?
There is a path through configure.ac which sets
COM_ERR_LIBS=${with_com_err}/lib/libcom_err.a
and COM_ERR_LIBS gets used to link against shared libraries in
Makefile.am
imap_libcyrus_imap_la_LIBADD = $(COM_ERR_LIBS) $(LIB_UUID)
...
sieve_libcyrus_sieve_la_LIBADD = $(COM_ERR_LIBS)
...
I would guess you did something like
./configure --with-com_err=/usr
but you should be able to get a better result by just dropping the
option entirely.
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #652
On Tue, Jun 19, 2012, at 11:17 PM, Дилян Палаузов wrote: Hello, I merged today the dev/libtool branch in master. Ok, so the first problem is that you did a merge rather than rebase the devllibtool branch on current master and then push that result. Merging makes the commit graph really really messy and hard to follow, and it's harder to debug problems resulting after the merge, like this one. In particular it makes bisecting much harder. I know rebasing is a pain - believe me I know - but merging instead is just wantonly invoking technical debt, i.e. pushing the pain into the future. I have no idea, why autoreconf does not run cleanly on ci.cyrusimap.org and complains about missing RANLIB and LT_INIT. On my system there are no problems, libtool recommended to remove AC_PROG_RANLIB from configure.ac and LT_INIT is there. Can somebody with local access check why autoreconf does not run? A manual deletion of aclocal.m4 or autom4te.cache might solve the problem (or not). I removed those files but autoreconf fails in exactly the same way - see build #653. The offending line in Makefile.am is 70 check_LIBRARIES = and it should be noted that check_LIBRARIES is not used *anywhere* anymore. Looking in git-blame I can see that this line was added by you, in this commit commit 18798292db946f9814a015120defab681a716a45 Author: Дилян Палаузов git-...@aegee.org Date: Thu Apr 12 10:03:02 2012 + Makefile.am: merge installsieve/Makefile.in It is unclear why this directory exist, since there is no place, that installs installsieve. but most of that commit has been removed again in subsequent commits. So we don't seem to need that line anymore. It doesn't cause a warning on my system either, go figure :( I removed that line, but build #654 is still broken. The next problem is that autoreconf seems to be confused about libtool bash-3.2$ autoreconf -v -i -f -Icmulocal ... autoreconf: configure.ac: tracing autoreconf: configure.ac: not using Libtool ... Makefile.am:70: Libtool library used but `LIBTOOL' is undefined Makefile.am:70: The usual way to define `LIBTOOL' is to add `LT_INIT' Makefile.am:70: to `configure.ac' and run `aclocal' and `autoconf' again. Makefile.am:70: If `LT_INIT' is in `configure.ac', make sure Makefile.am:70: its definition is in aclocal's search path. It turns out that the libtool on this machine is sufficiently old that it doesn't define LT_INIT but instead has AC_PROG_LIBTOOL. Autoreconf is happy to detect either, but advises you use LT_INIT. So I changed configure.ac to use the older deprecated AC_PROG_LIBTOOL. This was a terrible idea. Then I remembered that we're not running with system autotools at all but with ones I built, so I built and installed a more modern version of libtool as well. This worked slightly better: build #656. We get lots of these warning configure.ac:121: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call detected in body autoconf/lang.m4:197: AC_LANG_CONFTEST is expanded from... autoconf/general.m4:2670: _AC_LINK_IFELSE is expanded from... autoconf/general.m4:2680: AC_LINK_IFELSE is expanded from... cmulocal/libtool.m4:5331: _LT_LINKER_SHLIBS is expanded from... cmulocal/libtool.m4:5416: _LT_LANG_C_CONFIG is expanded from... cmulocal/libtool.m4:240: _LT_SETUP is expanded from... cmulocal/libtool.m4:104: LT_INIT is expanded from... configure.ac:121: the top level but then the build fails with /bin/sh ./libtool --tag=CC --mode=link gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o lib/libcyrus_min.la -rpath /usr/cyrus/lib lib/assert.lo lib/hash.lo lib/imapopts.lo lib/libconfig.lo lib/lock_fcntl.lo lib/mappedfile.lo lib/map_shared.lo lib/mpool.lo lib/retry.lo lib/strarray.lo lib/strhash.lo lib/util.lo lib/xmalloc.lo lib/xstrlcat.lo lib/xstrlcpy.lo -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -ldl -ldb-4.3 -lz ./libtool: line 5310: cd: 4.8/lib: No such file or directory libtool: link: cannot determine absolute directory name of `4.8/lib' This seems to be a bug in the tools/jenkins-build.sh script which has gone un-noticed for a long time, but which finally broke when libtool was added. The script is running configure --with-bdb=4.8, which seems to have once worked as figure out how to compile and link against version 4.8 of the Bekerley DB library but now means the Berkeley DB library can be found in the directory named '4.8/' . Before libtool, this resulted in broken -I and -L options which were silently ignored. With libtool, the same broken option gives a build failure. So I removed that configure argument. At this point we the build goes all the way through to running tests, and we get a single test failure Cassandane::Cyrus::Sieve.test_badscript_timsieved (from Cassandane__Cyrus__Sieve) Failing for the past 1 build (Since Failed#657 ) Took 0.48 sec. add description Error Message Boolean assertion failed Stacktrace
Re: Libtool and Support for Shared Libraries (3)
On Sun, Jun 10, 2012, at 08:28 PM, Carson Gaspar wrote: On 6/10/12 7:32 PM, Greg Banks wrote: On Sat, Jun 9, 2012, at 04:42 PM, Carson Gaspar wrote: ./configure --prefix=/Tools/SunOS_5.11_i86pc_amd64/cyrus-imapd-2.5.autofoo --sysconfdir=/etc --with-sasl=/Tools/SunOS_5.11_i86pc_amd64/cyrus-sasl-git-20120609 --with-cyrus-group=cyrus --with-cyrus-user=cyrus --with-openssl=/Tools/SunOS_5.11_i86pc_amd64/openssl-1.0.1c --with-cyrus-prefix=/Tools/SunOS_5.11_i86pc_amd64/cyrus-imapd-2.5.autofoo --with-dbdir=/Tools/SunOS_5.11_i86pc_amd64/db-5.3.21 CPPFLAGS=-I/usr/include/kerberosv5 LIBS=-lkrb5 Does --with-krb or --with-krb=/usr not work for you? That would be a bug! Ummm... --with-krb is all about KRB4. It has nothing whatsoever to do with KRB5. I would argue that any time you need to specify CPPFLAGS and LIBS to tell configure to use a particular library, then configure is broken. --enable-gssapi from cmulocal/sasl2.m4 would seem to be what is desired, except that none of the code uses gssapi, it just uses raw krb5. So it links in useless libraries, and fails to find the proper include files... Frankly, it all needs to be ripped out and replaced with something sane. Sadly cmulocal/kerberos_v5.m4 does not qualify as sane either :-( Hmm, that seems to do some things right but some things wrong. The way it does the platform specific linker flags fu to add the runtime linker path is downright silly - it should be using CMU_ADD_LIBPATH_TO, and most probably that macro should be doing the switch on $build_host and doing guesswork only as a last resort. That could use fixing. I don't know enough about Kerberos to tell whether any of the rest of it is sane, but it sounds like you know enough to fix it? Patches are always welcome. -- Greg.
Re: Libtool and Support for Shared Libraries (3)
Sent from my iPhone On 09/06/2012, at 7:22, Carson Gaspar car...@taltos.org wrote: [ Much perl tsuris elided ] ;) Folks keep talking about linking a static libfoo.a into a dynamic libbar.so. This MUST NOT HAPPEN. Linking non-PIC code into a shared object will fail in unpredictable ways. Understood - this is why the old build system makes an archive library full of PIC objects, to be linked into the DSO for Perl to dlopen(). It may _appear_ to work, but it really won't. The Solaris linker will usually scream bloody murder and error out. Linux/gnu ld tend to fail to enforce this, and leave the errors for runtime. The libtool non-static convenience lib stuff sounds like it does the right thing, but I haven't looked at it to be sure. Yes it does - I checked the log to see that the objects that go into it are built with -fPIC, then extracted an object from it with 'ar xv' and used 'objdump -r' to make sure the object contained relocs against the PLT and GOT, which only happens in PIC objects. If I have some time this weekend, I'll try building the dev/libtool branch and provide more feedback, as I install every package into it's own tree, so I will trigger any missing RPATH issues. Excellent, I'm looking forward to hearing how that goes. If anyone out there builds on MacOS or BSD, it would be great if you could build the dev/libtool branch and report your experiences. -- Carson
Re: Libtool and Support for Shared Libraries (3)
Sent from my iPhone On 07/06/2012, at 23:24, Bron Gondwana br...@fastmail.fm wrote: On Tue, Jun 5, 2012, at 12:49 PM, Дилян Палаузов wrote: Hello, now libcyrus and libcyrus_min are compiled once as shared libraries, once under perl/ as non-static convenience libraries, perl/imap/IMAP.so and perl/sieve/managesieve/managesieve.so link statically with the non-static convenience libraries, so that IMAP.so and managesieve.so do need neither libcyrus and libcyrus_min at run time nor the RPATH with those libraries at tun time. Talking about dependencies across random shit - I've just started trying to backport the autocreate and autosieve libraries - and it's a right pain. They need sieve/libsieve.a linked in to EVERYWHERE that has mboxlist linked it by the looks of things, which is painful: https://github.com/brong/cyrus-imapd/tree/autofoo gcc -fPIC -g -W -Wall -Wextra -o imap/arbitron imap/arbitron.o imap/cli_fatal.o imap/mutex_fake.o sieve/libsieve.a imap/libimap.a lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -lssl -lcrypto -ldl - ldb-4.8 -lpcre -lpcreposix -lz -lcom_err -luuid -ldl -ldb-4.8 - lpcre -lpcreposix -lz imap/libimap.a(autosieve.o): In function `autoadd_sieve': /home/brong/src/cyrus-imapd/imap/autosieve.c:233: undefined reference to `sieve_generate_bytecode' /home/brong/src/cyrus-imapd/imap/autosieve.c:237: undefined reference to `sieve_script_free' /home/brong/src/cyrus-imapd/imap/autosieve.c:243: undefined reference to `sieve_emit_bytecode' /home/brong/src/cyrus-imapd/imap/autosieve.c:247: undefined reference to `sieve_free_bytecode' /home/brong/src/cyrus-imapd/imap/autosieve.c:248: undefined reference to `sieve_script_free' /home/brong/src/cyrus-imapd/imap/autosieve.c:255: undefined reference to `sieve_free_bytecode' /home/brong/src/cyrus-imapd/imap/autosieve.c:256: undefined reference to `sieve_script_free' imap/libimap.a(autosieve.o): In function `is_script_parsable': /home/brong/src/cyrus-imapd/imap/autosieve.c:437: undefined reference to `sieve_interp_alloc' /home/brong/src/cyrus-imapd/imap/autosieve.c:443: undefined reference to `sieve_register_redirect' /home/brong/src/cyrus-imapd/imap/autosieve.c:448: undefined reference to `sieve_register_discard' /home/brong/src/cyrus-imapd/imap/autosieve.c:453: undefined reference to `sieve_register_reject' /home/brong/src/cyrus-imapd/imap/autosieve.c:458: undefined reference to `sieve_register_fileinto' /home/brong/src/cyrus-imapd/imap/autosieve.c:463: undefined reference to `sieve_register_keep' /home/brong/src/cyrus-imapd/imap/autosieve.c:469: undefined reference to `sieve_register_imapflags' /home/brong/src/cyrus-imapd/imap/autosieve.c:475: undefined reference to `sieve_register_size' /home/brong/src/cyrus-imapd/imap/autosieve.c:481: undefined reference to `sieve_register_header' /home/brong/src/cyrus-imapd/imap/autosieve.c:487: undefined reference to `sieve_register_envelope' /home/brong/src/cyrus-imapd/imap/autosieve.c:493: undefined reference to `sieve_register_vacation' /home/brong/src/cyrus-imapd/imap/autosieve.c:499: undefined reference to `sieve_register_notify' /home/brong/src/cyrus-imapd/imap/autosieve.c:505: undefined reference to `sieve_register_parse_error' /home/brong/src/cyrus-imapd/imap/autosieve.c:516: undefined reference to `sieve_script_parse' /home/brong/src/cyrus-imapd/imap/autosieve.c:522: undefined reference to `sieve_script_free' /home/brong/src/cyrus-imapd/imap/autosieve.c:529: undefined reference to `sieve_interp_free' collect2: ld returned 1 exit status make[2]: *** [imap/arbitron] Error 1 I'm thinking maybe what's actually needed here is moving all the automagic out into a separate library which only gets included by the three daemons which actually need it (lmtp which already has sieve, imapd and pop3d) Any other ideas? Hook in the new code at runtime via callbacks? Bron. -- Bron Gondwana br...@fastmail.fm
Re: Libtool and Support for Shared Libraries (3)
On Mon, Jun 4, 2012, at 04:22 PM, Дилян Палаузов wrote: Hello, then I suggest to build both static and shared libraries, to link all services and user programmes with the shared libraries, to link the IMAP.so and manageiseve.so files with the static libraries, and resolve by this way the need to insert RPATH in IMAP.so and managesieve.so . Using noinst_LTLIBRARIES to create a non-static convenience library (in libtool's terminology) whose only purpose is to get linked into IMAP.so and managesieve.so ? This might work. I guess libtool has to be run for every step, and this makes the things very messy. Maybe not. According to the libtool documentation, you can convince libtool --mode=link to build a native .a file using the obvious options -o foo.a. So you might be able to convince the main Makefile.am to build a native .a and link against that in the MakeMaker makefile. Running libtool with --quiet during make install DESTDIR= (achieved by make install DESTDIR=... LIBTOOLOPTIONS=--quiet) steal leads to warnings that the shared libraries are not installed on their final destination. Ah, libtool... :( From the documention it looks like using -R $(libdir) instead of -rpath $(libdir) might be useful. -- Greg.
Re: Libtool and Support for Shared Libraries (3)
On Tue, Jun 5, 2012, at 12:49 PM, Дилян Палаузов wrote: Hello, now libcyrus and libcyrus_min are compiled once as shared libraries, once under perl/ as non-static convenience libraries, perl/imap/IMAP.so and perl/sieve/managesieve/managesieve.so link statically with the non-static convenience libraries, so that IMAP.so and managesieve.so do need neither libcyrus and libcyrus_min at run time nor the RPATH with those libraries at tun time. Sounds promising. I hope this is the end with the shared libraries, before they can be merged. [...] After merging the support for sieve-seconds, jenkins did not rebuild sieve/sieve.c from sieve/sieve.y (it considered probably sieve.c newer as sieve.y), I fixed tools/jenkins-build.sh to run make maintainer-clean (instead of make distclean) before each run, so that sieve/sieve.c is deleted. This caused deleting also imap/rfc822_headers.[ch], which were not rebuild, as ./configure was not run with --enable-maintainer-mode. Well, that's a bug with maintainer-clean. The definition of maintainer-clean in the make documentation says `maintainer-clean' Delete almost everything that can be reconstructed with this Makefile. This typically includes everything deleted by `distclean', plus more: C source files produced by Bison, tags tables, Info files, and so on. So if maintainer-clean deletes something that can't be rebuilt with the current configure settings, it's broken. If gperf is installed on jenkins.cyrusimapd.org the next build shall run normally. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #623
On Tue, Jun 5, 2012, at 05:01 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/623/changes ./tools/config2header CC=gcc ./lib/imapopts.c ./lib/imapopts.h ./lib/imapoptions cd imap /usr/bin/compile_et .././imap/imap_err.et cd imap /usr/bin/compile_et .././imap/mupdate_err.et cd imap /usr/bin/compile_et .././imap/nntp_err.et cd imap .././snmp/snmpgen .././imap/lmtpstats.snmp basename lmtpstats cd imap .././snmp/snmpgen .././imap/pushstats.snmp basename pushstats make: *** No rule to make target `imap/rfc822_header.c', needed by `all'. Stop. + fatal 'Can'\''t make all' Dilyan - you need to fix tools/jenkins-build.sh either a) to not make maintainer-clean, or b) to ./configure --enable-maintainer-mode Either should work. I cleaned up the workspace and installed gperf. -- Greg.
Re: Libtool and Support for Shared Libraries (3)
On Tue, Jun 5, 2012, at 12:49 PM, Дилян Палаузов wrote: Hello, now libcyrus and libcyrus_min are compiled once as shared libraries, once under perl/ as non-static convenience libraries, perl/imap/IMAP.so and perl/sieve/managesieve/managesieve.so link statically with the non-static convenience libraries, so that IMAP.so and managesieve.so do need neither libcyrus and libcyrus_min at run time nor the RPATH with those libraries at tun time. I hope this is the end with the shared libraries, before they can be merged. Sigh...this still doesn't work. I built your latest dev/libtool branch, and got these errors during the config step config.status: executing perl/annotator/Makefile commands Writing Makefile for Cyrus::Annotator::Daemon config.status: executing perl/imap/Makefile commands Unrecognized argument in LIBS ignored: '../../perl/.libs/libcyrus.a' --- Unrecognized argument in LIBS ignored: '../../perl/.libs/libcyrus_min.a' --- Writing Makefile for Cyrus::IMAP config.status: executing perl/sieve/managesieve/Makefile commands Unrecognized argument in LIBS ignored: '../../../perl/.libs/libcyrus.a' --- Unrecognized argument in LIBS ignored: '../../../perl/.libs/libcyrus_min.a' --- Writing Makefile for Cyrus::SIEVE::managesieve Make then went on to build, link, and install an IMAP.so and a managesieve.so. However the link step was rm -f blib/arch/auto/Cyrus/IMAP/IMAP.so cc -shared -O2 -g -L/usr/local/lib -fstack-protector IMAP.o -o blib/arch/auto/Cyrus/IMAP/IMAP.so -ldb-4.8 -lsasl2 -lssl -lcrypto -luuid -lz \ note the complete lack of libcyrus.a. So of course the resulting .so's are missing all the symbols they need from libcyrus: gnb@gnb-desktop 2818 nm usr/cyrus/lib/perl/5.10.1/auto/Cyrus/IMAP/IMAP.so ... U imapurl_fromURL U imapurl_toURL U imclient_addcallback U imclient_authenticate U imclient_clearflags U imclient_close U imclient_connect U imclient_getselectinfo U imclient_processoneevent U imclient_send U imclient_servername U imclient_setflags U imclient_starttls ... gnb@gnb-desktop 2820 nm usr/cyrus/lib/perl/5.10.1/auto/Cyrus/SIEVE/managesieve/managesieve.so ... U prot_flush U prot_free U prot_getc U prot_new U prot_printf U prot_setsasl U prot_ungetc U prot_write ... U ucase ... U xmalloc U xstrdup U xstrdupnull This builds but will fail at Perl module load time. I tried using this syntax instead 'LIBS' = [ -L@top_builddir@/perl/.libs -lcyrus -lcyrus_min ... which changes the warning at configure time to config.status: executing perl/annotator/Makefile commands Checking if your kit is complete... Looks good Writing Makefile for Cyrus::Annotator::Daemon config.status: executing perl/imap/Makefile commands Checking if your kit is complete... Looks good Note (probably harmless): No library found for -lcyrus Note (probably harmless): No library found for -lcyrus_min Writing Makefile for Cyrus::IMAP config.status: executing perl/sieve/managesieve/Makefile commands Checking if your kit is complete... Looks good Note (probably harmless): No library found for -lcyrus Note (probably harmless): No library found for -lcyrus_min but the same end effect. After some experimentation I found that using the LDFROM variable does the trick, like this 'LDFROM' = \$(OBJECT) @top_builddir@/perl/.libs/libcyrus.a @top_builddir@/perl/.libs/libcyrus_min.a and all is well. I've pushed this fix to the dev/libtool branch. If it works on your environment, I see no further obstacles to merging. -- Greg.
Re: Libtool and Support for Shared Libraries (3)
Sent from my iPhone On 03/06/2012, at 20:50, Дилян Палаузов dilyan.palau...@aegee.org wrote: Hello, concerning the rpath for the perl shared objects IMAP.so and managesieve.so, after all I found that MakeMaker uses as linker by default the value returned by $Config(ld) (visible by running perl - V under Linker and Libraries), which is sometimes ld='cc' and sometimes ld='ld'. If I do not overwrite the default LD-variable (= $config(ld)), then it might be cc, which could be gcc or not. I have no idea how to pass linker flags, if the linker is cc, but not gcc (with gcc the parameters are passed with -Wl,param). So on the one side, I do not know how to instruct $config(ld)/the linker in a portable way by passing -rpath parameters to include rpath in the .so file. This is very platform specific, which is why libtool exists. Perhaps you could arrange for MakeMaker to run libtool for the link step? This might result in needing to run libtool for *every* step, which may be awkward. Relying on LD_RUN_PATH is just as platform specific as using -rpath. Greg.
Re: IRC
G'day, (00:06:17) aosowski: gnb1, I'm running 3.3.4 and cassandane is complaining E.netstat: no support for `AF INET (sctp)' on this system. can I ignore that or how can I fix it? (no sctp module available, just various nf_*_sctp which are all loaded) Cassandane doesn't care about SCTP pe se. It does have some tests to verify that the Cyrus master process can handle binding to IPv4, IPv6 and UNIX domain sockets, which are all supported features of the master process. As part of these tests, and as part of the normal procedure of starting up a Cyrus instance, Cassandane runs the netstat program to verify that a process is actually bound to the socket that it's supposed to be. It uses commands of the form netstat -l -n -Ainet netstat -l -n -Ainet6 netstat -l -n -Aunix Most probably you're seeing some message which is a side effect of netstat being confused on your system. If they're just warnings, and netstat still runs and emits correct output, you should be fine to just ignore the messages. If Cassandane runs any tests to completion, you probably don't need to worry. -- Greg.
Re: Libtool and Support for Shared Libraries (3)
On Fri, Jun 1, 2012, at 11:15 AM, Jeroen van Meeuwen (Kolab Systems) wrote: On 2012-05-31 18:25, Дилян Палаузов wrote: - make install DISTDIR= causes warnings from libtool, that state, that the libraries are not installed on the place they are intended to be installed (as configured) and the executables are not going to work (as they will not be able to find the libraries under the DESTDIR= root). This warning gives right information, on the other side Greg says, that the make install DISTDIR= is a normal process that shall not lead to warnings. I asked at libtool-...@gnu.org , but I if they do not answer, I can't say anything more about this. Is the typo DISTDIR/DESTDIR deliberate or accidental? My world uses DESTDIR, for the record. Everybody here means DESTDIR, the other one is a typo (it doesn't even appear in the Makefile so it would have no effect at all). -- Greg.
Re: Libtool and Support for Shared Libraries (3)
On Thu, May 31, 2012, at 06:25 PM, Дилян Палаузов wrote: Hello, to sum up the issues with Libtool: - the perl shared objects IMAP.so and managesieve.so do not contain Library rpath on Greg's computer, but have it on mine. I have not checked the details, but I think on my computer -rpath comes from the fact, that to build LDFLAGS the file libdb-4.8.la is considered (when I configure with BDB support), the file contains libdir=/usr/lib64 and this leads to -Wl,-rpath,/usr/lib64 in LDFLAGS. At the same time, Automake includes automatically _rpath = -rpath $(libdir) variables (at least with --enable-sieve for libsieve, and with --enable-server for libimap, unconditionally for libcyrus_min and libcyrus, and when the bundled libcom_err is used, it is also linked with -rpath $(libdir)). However the cyrus-libraries are installed in $(libdir), and not /usr/cyrus/lib, because we want them to be on a standard location, which is considered by the dynamic loader, and are in ldconfig's path. So what is the problem, when the perl shared objects do not contain -rpath? I think the other executables shall also not contain it. So this problem might not affect you, but it will break FastMail completely. We build and install *on the same machine* multiple Cyrus .deb packages built with different --prefix options. For this to work, we expect that * When built with --prefix=/foo where /foo != /usr, Cyrus will install everything under /foo and nothing at all under /usr (this ensures that multiple packages don't try to own the same files, so they can be removed and installed independently without concern) * If an installed executable e.g. /foo/bin/master is run, it knows how to find all the runtime resources it needs to find from subdirectories of /foo without further intervention (this ensures we don't need to futz with environment variables, nor use full pathnames to service executable in cyrus.conf) I don't think either of these conditions are onerous, and neither affect normal vendor Cyrus packaging. With the current build system, these conditions are both true. You are proposing to break both of them. - make install DISTDIR= causes warnings from libtool, that state, that the libraries are not installed on the place they are intended to be installed (as configured) and the executables are not going to work (as they will not be able to find the libraries under the DESTDIR= root). This warning gives right information, on the other side Greg says, that the make install DISTDIR= is a normal process that shall not lead to warnings. I asked at libtool-...@gnu.org , but I if they do not answer, I can't say anything more about this. These warnings I find annoying but not fatal. If they're hard to fix I can filter them out. I would have thought that there would be some easy libtool option to avoid them. Apart from the suboptimal building of the perl parl, which was suboptimal before, and is even more suboptimal now, I think that are all points with libtool/shared libraries. As the shared libraries were not discussed, is somebody against using shared libraries with cyrus-imapd 2.5 / merging the dev/libtool branch? -- Greg.
Re: Libtool and Support for Shared Libraries (2)
On Mon, May 28, 2012, at 09:33 PM, Dilyan Palauzov wrote: On my system managesieve.so and IMAP.so contain Library rpath. Curious. Presumably it's a bug in my libtool or automake. I'm tempted to put in a post-install check for Linux build hosts. [...]probably using make install DESTDIR= with libtool is either wrong or implemented/handled wrong in Automake/libtool. Well, that sucks. Why do not you use ./configure --prefix=$(DESTDIR), so that make install DESTDIR=somewhere is not necessary? To my understanding installing in DESTDIR is used to create packages, So we now generate dozens of warnings when doing a straightforward, entirely normal, and unavoidable step in the packaging process? I don't see how that's acceptable. -- Greg.
Re: libzephyr and notifications
On Mon, May 28, 2012, at 03:22 PM, Jeroen van Meeuwen (Kolab Systems) wrote: On 2012-05-22 9:40, Bron Gondwana wrote: We're having issues building zephyr with the new automake stuff, and before we spend too much time fixing it - there's a question worth asking... Does anyone actually use zephyr? I (we) don't. If not, I'd prefer to remove it and integrate worldline's notification bus work rather than having multiple competing notification systems. No objections from me (us). Andreas Osowski has successfully merged in / rebased the notification bus work from worldline's github, BTW, so if it can be given a glance-over before it's finally merged into origin/master, that'd be great. Ok, I'll review that, starting today or tomorrow (flu permitting). I saw Andreas' message go past but I can't seem to find it now that I need it :( so what's the git URL? -- Greg.
Re: Libtool and Support for Shared Libraries (2)
On Tue, May 22, 2012, at 01:47 PM, Дилян Палаузов wrote: Hello, commit Makefile.am: move the defintion of COM_ERR_LIB here I don't think there's any point having COM_ERR_LIB and COM_ERR_LIBS, just one should do, but we can tweak that later. libimap and libsieve have to depend on libcom_err.a whenever the bundled libcom_err.a is used, so that the bundled libcom_err.a is build before libimap and libsieve are linked. Sure, I see why you did it. Any ideas how to achieve this, that are different from what I did? Use a single variable, which is set by in the configure script and appended to in one branch of the conditional (the other branch being empty). commit convert lib/libcyrus_min.a to libtool archive commit convert lib/libcyrus.a to libtool archive Looks good, except that the Perl build is broken. I get this: make[2]: Entering directory `/home/gnb/software/cyrus/imapd/perl/imap' make[2]: *** No rule to make target `-lcyrus', needed by `subdirs'. Stop. make[2]: *** Waiting for unfinished jobsinfo I've been wrestling with this most of the day but libtool + MakeMaker = one enormous headache :( I moved -lcyrus -lcyrus_min in Makefile.PL.in from MYEXTLIB to OTHERLDFLAGS, so that the target subdirs does not depend anymore on -lcyrus -lcyrus_min. Please try with the uploaded fix. (I can understand why you got this error, but I do not get it.) Ok, that builds and installs now :) One more problem: the installed IMAP.so and managesieve.so depend on the Cyrus libraries but do not have a runtime library path which will find them in the case where --prefix != /usr. The libtool-linked executables installed into bindir do have that. gnb@gnb-desktop 2010 readelf -d ../inst/usr/cyrus/bin/imapd Dynamic section at offset 0x36be0 contains 35 entries: TagType Name/Value 0x0001 (NEEDED) Shared library: [libcyrus_imap.so.0] --- 0x0001 (NEEDED) Shared library: [libcyrus.so.0] 0x0001 (NEEDED) Shared library: [libcyrus_min.so.0] 0x0001 (NEEDED) Shared library: [libwrap.so.0] 0x0001 (NEEDED) Shared library: [libnsl.so.1] 0x0001 (NEEDED) Shared library: [libdl.so.2] 0x0001 (NEEDED) Shared library: [libdb-4.8.so] 0x0001 (NEEDED) Shared library: [libpcre.so.3] 0x0001 (NEEDED) Shared library: [libpcreposix.so.3] 0x0001 (NEEDED) Shared library: [libz.so.1] 0x0001 (NEEDED) Shared library: [libc.so.6] 0x0001 (NEEDED) Shared library: [libcom_err.so.2] 0x0001 (NEEDED) Shared library: [libsasl2.so.2] 0x0001 (NEEDED) Shared library: [libssl.so.0.9.8] 0x0001 (NEEDED) Shared library: [libcrypto.so.0.9.8] 0x000f (RPATH) Library rpath: [/usr/cyrus/lib] 0x000c (INIT) 0x406d30 0x000d (FINI) 0x42fc68 but not gnb@gnb-desktop 2009 readelf -d ../inst/./usr/cyrus/lib/perl/5.10.1/auto/Cyrus/IMAP/IMAP.so Dynamic section at offset 0x8da0 contains 28 entries: TagType Name/Value 0x0001 (NEEDED) Shared library: [libcyrus.so.0] 0x0001 (NEEDED) Shared library: [libcyrus_min.so.0] 0x0001 (NEEDED) Shared library: [libdb-4.8.so] 0x0001 (NEEDED) Shared library: [libsasl2.so.2] 0x0001 (NEEDED) Shared library: [libssl.so.0.9.8] 0x0001 (NEEDED) Shared library: [libcrypto.so.0.9.8] 0x0001 (NEEDED) Shared library: [libuuid.so.1] 0x0001 (NEEDED) Shared library: [libz.so.1] 0x0001 (NEEDED) Shared library: [libc.so.6] 0x000c (INIT) 0x1b88 0x000d (FINI) 0x8098 Also, is there a way of getting rid of these warnings: /bin/bash ./libtool --mode=install /usr/bin/install -c imtest/imtest '/home/gnb/software/cyrus/inst/usr/cyrus/bin' libtool: install: warning: `lib/libcyrus.la' has not been installed in `/usr/cyrus/lib' libtool: install: warning: `lib/libcyrus_min.la' has not been installed in `/usr/cyrus/lib' because there's quite a lot of them and they're confusing the script I use to find compile time warnings. Pass --prefix to ./configure . I am. + ./configure --prefix=/usr/cyrus --with-cyrus-prefix=/usr/cyrus --enable-maintainer-mode --enable-idled --enable-nntp --enable-murder --enable-replication --enable-unit-tests --without-snmp The resulting binaries have to be aware where the shared libraries are (when the shared libraries are not on standard
Re: Libtool and Support for Shared Libraries (2)
On Thu, May 17, 2012, at 01:21 AM, Дилян Палаузов wrote:
Hello,
I have integrated the comments about libtool/shared libraries and at the
end the changes are:
-- in libcyrus_min:libconfig:config_read one parameter is added for
config_need_data, so that this variable does not have to be global and
presented in programs, not using config_read, update all invocations of
the function to use the new API
-- in libimap:global:cyrus_init add one parameter (config_need_data),
so that its value can be passed to config_read; update all invocations
of cyrus_init;
-- create by default only shared libraries, this can be further tuned
with ./configure --enable-static / --disable-shared
-- rename libsieve and libimap to libcyrus_sieve and libcyrus_sieve
and install them in $(libdir) (e.g. /usr/lib)
-- make the perl/{imap,sieve/managesieve} modules use the shared
libraries libcyrys_min and libcyrus. This might lead to a problem, when
the tests in perl/imap/t/ are run, before the shared libraries are
installed, as the tests depend on the shared libraries, but the tests
are not instructed to where to find thw libraries.
-- when there is no system-wide libcom_err, compile the bundled one
as libcyrus_com_err and eventually install it in $(libdir)
commit Makefile.am: remove LD_(BASIC,SERVER,SIEVE,UTILITY)_FLAGS
commit Makefile.am: remove LD_SIEVE_FLAGS
commit timsieved/scripttest.c: mark internal functions with static
commit Makefile.am: build doc/text/htmlstrip only on make dist
This all looks good
commit Makefile.am: move the defintion of COM_ERR_LIB here
I don't think there's any point having COM_ERR_LIB and COM_ERR_LIBS,
just one should do, but we can tweak that later.
commit libcyrus_min:libconfig:config_read: add parameter
config_need_data
commit libimap:global:cyrus_init add parameter config_need_data
I think this could be done more cleanly, but it does solve the immediate
problem.
commit notifyd/notifytest: add a function fatal
commit configure.ac, Makefile.am: Add libtool support
commit /.gitignore: update to ingore .la, .lo and .libs/ files
commit convert lib/libcyrus_min.a to libtool archive
commit convert lib/libcyrus.a to libtool archive
commit convert com_err/et/libcom_err.a to a libtool archive
commit convert sieve/libsieve.a to a libtool archive
commit convert imap/libimap.a to a libtool archivedev
Looks good, except that the Perl build is broken. I get this:
make[2]: Entering directory `/home/gnb/software/cyrus/imapd/perl/imap'
make[2]: *** No rule to make target `-lcyrus', needed by `subdirs'.
Stop.
make[2]: *** Waiting for unfinished jobsinfo
I've been wrestling with this most of the day but libtool + MakeMaker =
one enormous headache :(
Also, is there a way of getting rid of these warnings:
/bin/bash ./libtool --mode=install /usr/bin/install -c imtest/imtest
'/home/gnb/software/cyrus/inst/usr/cyrus/bin'
libtool: install: warning: `lib/libcyrus.la' has not been installed in
`/usr/cyrus/lib'
libtool: install: warning: `lib/libcyrus_min.la' has not been installed
in `/usr/cyrus/lib'
because there's quite a lot of them and they're confusing the script I
use to find compile time warnings.
I hope with uploading now the separate branch dev/libtool on
git.cyrusimapd.org I will not mess up the things again.
I think you should go ahead and merge it in once we can get the Perl
code building . There's a few problems remaining after that but nothing
we can't handle once we know about it. In other news I've tweaked
Cassandane to set up $LD_LIBRARY_PATH so it will be able to run Cyrus
binaries built for shared libraries.
--
Greg.
Re: Libtool and Support for Shared Libraries
On Thu, May 10, 2012, at 09:42 PM, Jeroen van Meeuwen (Kolab Systems) wrote: I agree. This tidying up is going to majorly invasive - don't forget we want to merge in caldav before this happens as well. I strongly recommend any major really tidying things up is postponed to 2.6. Agreed. I think we've changed enough for 2.5. It might be worth declaring 2.5 to be in feature freeze after the caldav merge, and starting a 2.6 branch. Otherwise we'll never get it released. -- Greg.
Re: [PATCH 1/4] autobuild: Fix directory handling
Sent from my iPhone On 08/05/2012, at 18:44, Philipp Hahn h...@univention.de wrote: If ../inst/ or ../cassandane/ don't exists, running autobuild.sh fails because it tries to rename the current working directory containing the source tree. Autobuild.sh isn't for you to use, it's for the Jenkins CI server. In that context the directory structure autobuild.sh expects is carefully arranged to exist, and a missing Cassandane directory must be a fatal error. There should have been a comment at the top making this clear. Greg.
Re: Add VACATION :seconds support
On Tue, May 8, 2012, at 01:55 PM, Bron Gondwana wrote: On Tue, May 8, 2012, at 01:52 PM, Philipp Hahn wrote: autobuild had just the right name and did what I was looking for - until it renamed $PWD to $PWD.orig :-( gnb - looks like an attractive nuisance. Yeah - naming is hard :( Can it be renamed something more obvious without annoying Jenkins? You bet, assuming the corresponding tweak in Jenkins. When it originally lived in /usr/local/bin it was called jenkins-build-and-test.sh, but a filename that long just confuses the ls output :( I'll go child-proof it a bit. -- Greg.
Re: [PATCH 0/4] Add VACATION :seconds support
On Tue, May 8, 2012, at 01:52 PM, Bron Gondwana wrote: On Tue, May 8, 2012, at 01:50 PM, Philipp Hahn wrote: Hello, Am Dienstag 08 Mai 2012 13:07:04 schrieb Bron Gondwana: On Tue, May 8, 2012, at 11:54 AM, Philipp Hahn wrote: For converting days to seconds and the inverse I always explicitly wrote 24 * 60 * 60. Is there a good location for defining this as a macro? lib/util.h ? Will take a look. Would the following be fine: #define DAY2SEC(x) ((x) * 24 * 60 * 60) Or is there a more preferrable name? That's clear. I'm fine with it. There's a lib/times.h which has a significant part of the other time handling declarations in it. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #557
On Thu, May 3, 2012, at 05:07 PM, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/557/
[...]
Test failures and errors summary
Cassandane::Cyrus::Sieve.badscript_timsieved
http://ci.cyrusimap.org/job/cyrus-imapd-master/557//testReport/%28root%29/Cassandane__Cyrus__Sieve/test_badscript_timsieved/
Hmm.
Error Message
Boolean assertion failed
Stacktrace
test_badscript_timsieved(Cassandane::Cyrus::Sieve)
Boolean assertion failed at
/usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/Exception.pm line 13
Test::Unit::Exception::throw_new('Test::Unit::Failure=HASH(0x1cdab900)',
'-package', 'Cassandane::Cyrus::Sieve', '-file',
'Cassandane/Cyrus/Sieve.pm', '-line', 290, '-object',
'Cassandane::Cyrus::Sieve=HASH(0x1c6457a0)', ...) called at
/usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/Assert.pm line 85
Test::Unit::Assert::do_assertion('Cassandane::Cyrus::Sieve=HASH(0x1c6457a0)',
'Test::Unit::Assertion::Boolean=SCALAR(0x1d1157e0)',
'Cassandane::Cyrus::Sieve', 'Cassandane/Cyrus/Sieve.pm', 290)
called at /usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/Assert.pm
line 19
Test::Unit::Assert::assert('Cassandane::Cyrus::Sieve=HASH(0x1c6457a0)')
called at Cassandane/Cyrus/Sieve.pm line 290
Cassandane::Cyrus::Sieve::badscript_common('Cassandane::Cyrus::Sieve=HASH(0x1c6457a0)')
called at Cassandane/Cyrus/Sieve.pm line 365
Cassandane::Cyrus::Sieve::test_badscript_timsieved('Cassandane::Cyrus::Sieve=HASH(0x1c6457a0)')
called at /usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/TestCase.pm
line 75
[...framework calls elided...]
http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/cassandane/cass.errs/*view*/
= Cyrus::Sieve[363] Testing sieve script compile failures, via
timsieved
= Cyrus::Sieve[185] Checking preconditions for compiling sieve
script badrequire
= Cyrus::Sieve[203] Running installsieve on script badrequire
= Instance[1077] Running:
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/installsieve
-i /var/tmp/cass/210243151/badrequire.script -u cassandane
127.0.0.1:9100
= Cyrus::Sieve[118] errors:
= Cyrus::Sieve[119] Cyrus::SIEVE::managesieve object version 2.5
does not match bootstrap parameter 0.01 at
/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/DynaLoader.pm line 253.
= Cyrus::Sieve[119] Compilation failed in require at
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/installsieve
line 66.
= Cyrus::Sieve[119] BEGIN failed--compilation aborted at
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/installsieve
line 66.
Looks like my commit Makefile.PL's use @VERSION@ from configure broke
something deep in the guts of the Cyrus Perl modules. At first glance
it seems like there are strings like $VERSION = '1.0' riddled throughout
the modules, and the one in managesieve.pm is actually checked at
runtime against a version string built into managesieve.so, which since
the commit is 2.5.
This was too high a price to fix a compile time warning, so I've
reverted the commit.
http://git.cyrusimap.org/cyrus-imapd/commit/?id=775a512d604171a2cbd7acf8e118366ffb5d0036
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #553
On Tue, May 1, 2012, at 05:02 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/553/changes [...] cd sieve /usr/bin/compile_et .././sieve/sieve_err.et gcc -fPIC --coverage -g -O0 -L4.8/lib -Wl,-rpath,4.8/lib -o sieve/test sieve/test.o imap/mutex_fake.o imap/libimap.a sieve/libsieve.a lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -lssl -lcrypto -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -ldl -ldb-4.3 -lz -lcom_err -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -ldl -ldb-4.3 -lz imap/libimap.a(mailbox.o): In function `mailbox_make_uniqueid': http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/imap/mailbox.c:745: undefined reference to `uuid_clear' http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/imap/mailbox.c:746: undefined reference to `uuid_generate' http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/imap/mailbox.c:750: undefined reference to `uuid_unparse_lower' collect2: ld returned 1 exit status The _LDADD line for the sieve/test executable was missing -luuid. Dilyan has already fixed this in commit Makefile.am: sieve_test_LDADD: add $(LIB_UUID) -- Greg.
Re: In preparation of Cyrus IMAP 2.5: autoconf and automake
G'day, On Sat, Apr 28, 2012, at 11:39 AM, Carson Gaspar wrote: Thanks for the bug report! The below is on Solaris 11 using the studio compiler, using the snapshot from the URL below. On 4/28/12 9:15 AM, Jeroen van Meeuwen (Kolab Systems) wrote: http://git.cyrusimap.org/cyrus-imapd/snapshot/cyrus-imapd-2.5-snapshot-autoconf-and-automake.tar.gz The canonical build process we think applies, generally speaking, is: $ autoreconf -v This should have been $ autoreconf -vi I also see: OPENSSL_LIB = -L/Tools/SunOS_5.11_i86pc_amd64/openssl-1.0.0f-aesni/lib -L/Tools/SunOS_5.11_i86pc_amd64/openssl-1.0.0f-aesni/lib -R/Tools/SunOS_5.11_i86pc_amd64/openssl-1.0.0f-aesni/lib Something is adding the openssl -L flag twice. LDFLAGS only has it once. It seems we have yet more historically broken logic in the implementation of various --with-foo arguments to configure. I'm guessing your configure arguments included --with-openssl=/Tools/SunOS_5.11_i86pc_amd64/openssl-1.0.0f-aesni ? On our test systems, the OpenSSL headers are in /usr/include/openssl/ and libssl is in /usr/lib, so we're taking a different path through the configure code. I've fixed this in commit 'automake: rejig OpenSSL linker flags and libraries' lib/auth_krb5.c fails to build, because autofoo does absolutely nothing to find krb5.h: lib/auth_krb5.c, line 60: cannot find include file: krb5.h The only configure options I see are for krb4. If I manually fix the krb5 problem, I get to: We'll take a look at this later. Adding '-Xc' to the sun studio compiler to keep it from polluting the namespace would fix that, If the description of -Xc here http://docs.oracle.com/cd/E24457_01/html/E22003/cc.1.html is correct then we definitely want to use it. The compiler default behaviour is suitable for transitioning from KR C to ANSI C, a process that we've recently completed in Cyrus (only two decades late). But for the time being we have bigger problems. but would also undefine __FUNCTION__ causing the build to fail on the snmp bits. I'm confused by this statement. We don't use __FUNCTION__ anywhere exception in a unit test, and that's just a leftover debugging thing that could be removed. Can you post the error messages from your build failure? So I renamed all instances of sun to mysun in imap/append.c and imap/idlemsg.c. Sure, if you like. Undefined first referenced symbol in file krb5_free_principal lib/libcyrus.a(auth_krb5.o) krb5_realm_compare lib/libcyrus.a(auth_krb5.o) krb5_build_principallib/libcyrus.a(auth_krb5.o) uuid_unparse_lower imap/libimap.a(mailbox.o) krb5_get_default_realm lib/libcyrus.a(auth_krb5.o) krb5_parse_name lib/libcyrus.a(auth_krb5.o) krb5_init_context lib/libcyrus.a(auth_krb5.o) krb5_free_context lib/libcyrus.a(auth_krb5.o) krb5_unparse_name lib/libcyrus.a(auth_krb5.o) So not only is autofoo not bothering to find the header file, it isn't adding the krb5 libs, either! How on earth does this link for anyone?! Manually adding LIBS='-lkrb5' to configure fixes all but the uuid problem. I'm not sure what went wrong here... Solaris 11 has libuuid, but libuuid only has libuuid_unparse, not libuuid_unparse_lower. configure only checks for uuid_generate (which is present). I forced it to fail the uuid check by passing ac_cv_lib_uuid_uuid_generate=no to configure. Ok, the lack of uuid_unparse_lower() should be easy to work around using lcase(). With the above changes, it correctly compiles and links, and spot checking it appears RPATH is correctly set everywhere. Ok. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #542
On Fri, Apr 27, 2012, at 01:52 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/542/ -- [...truncated 1952 lines...] mv -f $depbase.Tpo $depbase.Po depbase=`echo imap/mutex_fake.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/mutex_fake.o -MD -MP -MF $depbase.Tpo -c -o imap/mutex_fake.o imap/mutex_fake.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/arbitron imap/arbitron.o imap/cli_fatal.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz depbase=`echo imap/chk_cyrus.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/chk_cyrus.o -MD -MP -MF $depbase.Tpo -c -o imap/chk_cyrus.o imap/chk_cyrus.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/chk_cyrus imap/chk_cyrus.o imap/cli_fatal.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz depbase=`echo imap/ctl_cyrusdb.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/ctl_cyrusdb.o -MD -MP -MF $depbase.Tpo -c -o imap/ctl_cyrusdb.o imap/ctl_cyrusdb.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/ctl_cyrusdb imap/cli_fatal.o imap/ctl_cyrusdb.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz depbase=`echo imap/ctl_deliver.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/ctl_deliver.o -MD -MP -MF $depbase.Tpo -c -o imap/ctl_deliver.o imap/ctl_deliver.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/ctl_deliver imap/cli_fatal.o imap/ctl_deliver.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz depbase=`echo imap/ctl_mboxlist.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/ctl_mboxlist.o -MD -MP -MF $depbase.Tpo -c -o imap/ctl_mboxlist.o imap/ctl_mboxlist.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/ctl_mboxlist imap/cli_fatal.o imap/ctl_mboxlist.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz depbase=`echo imap/cvt_cyrusdb.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/cvt_cyrusdb.o -MD -MP -MF $depbase.Tpo -c -o imap/cvt_cyrusdb.o imap/cvt_cyrusdb.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/cvt_cyrusdb imap/cli_fatal.o imap/cvt_cyrusdb.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz depbase=`echo imap/cyr_df.o | sed 's|[^/]*$|.deps/|;s|\.o$||'`;\ gcc -DHAVE_CONFIG_H -I. -I. -I./lib -I. -I./lib -DHAVE_CONFIG_H -fPIC --coverage -g -O0 -MT imap/cyr_df.o -MD -MP -MF $depbase.Tpo -c -o imap/cyr_df.o imap/cyr_df.c \ mv -f $depbase.Tpo $depbase.Po gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/cyr_df imap/cli_fatal.o imap/cyr_df.o imap/mutex_fake.o imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto
Re: Build failed in Jenkins: cyrus-imapd-master #541
On Fri, Apr 27, 2012, at 12:23 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/541/changes [...] gcc -fPIC --coverage -g -O0 --coverage -L4.8/lib -Wl,-rpath,4.8/lib -o imap/lmtpd imap/lmtpd.o imap/lmtpengine.o imap/lmtpstats.o imap/mutex_fake.o imap/proxy.o imap/spool.o master/service.o imap/lmtp_sieve.o imap/smtpclient.o sieve/libsieve.a imap/libimap.a -lcom_err lib/libcyrus.a lib/libcyrus_min.a -lsasl2 -luuid -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz -lwrap -lnsl -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lssl -lcrypto -ldb-4.3 -lz sieve/libsieve.a(script.o): In function `sieve_script_parse': http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd/sieve/script.c:196: undefined reference to `sievelineno' collect2: ld returned 1 exit status make[2]: *** [imap/lmtpd] Error 1 An old sieve-lex.c was left over from yesterday because the make distclean at the start of the build failed. This should be a once-off around the automake branch merge, so I've manually removed those files and started another build. -- Greg.
Re: make dist and cunit
On Tue, Apr 24, 2012, at 10:46 AM, Jeroen van Meeuwen wrote:
On Monday, April 23, 2012 03:05:24 PM Greg Banks wrote:
On Fri, Apr 20, 2012, at 12:15 PM, Дилян Палаузов wrote:
Hello,
Can SMakefile be deleted?
I hope so. Most of what's in there is useless and the rest is wrong.
It would be nice to have an autogen.sh which does all the necessary
autotools futzery, from aclocal to configure.
See attached (minor edits for the use of ~/tmp/ = ${TMPDIR:-/tmp}
notwithstanding).
Do we need make snapshot in addition to make dist?
The current dist: target seems to be broken; it fails on the master
branch because it's expecting a git tag which doesn't exist yet. I
would expect it to always build a tarball but in this case indicate (in
both the xversion.h and the version number of the tarball) that the
tarball does not represent a tagged release.
The purpose of requiring a tag with the dist target was to prevent
multiple
versions of cyrus-imapd-$x.$y.$z.tag.gz files from getting out there.
Agreed, a tarball that has a name like a release should be a release,
and not something else.
It
was
intended so that /make dist/ would fail, and someone would realize a
/make
snapshot/ was what they wanted.
That said, if they are under the same target and end up having done the
correct thing in each situation, we're fine.
I'd prefer one target that does what you meant.
+ autogen-cyrus-imapd.sh
3k (application/x-shellscript)
Yes, like that, except it's purpose is not to record your particular
build - I have a script like that too :) - but to provide a quick way
for a person compiling Cyrus for the first time to get through the maze
of twisty little autotools. Something more like:
#!/bin/sh
autoreconf -i -f -v -Icmulocal
./configure $@
So then a user can build from source by
- downloading the tarball
- autogen.sh
- make
- make check
- make install
This script looks like it was extracted from something a packaging
system ran, which is doing too much, like setting up C++ and Fortran
flags and forcing --with-perl and --bindir. But it does raise some
interesting points:
* If there are actual C flags that need setting (and I can see good
arguments for using -D_FORTIFY_SOURCE=2 and -fstack-protector, on
systems where gcc is the compiler and those aren't already the system
compiler's default), then the configure script is the right place to do
that so everyone gets the benefit of them.
* Ditto warning flags - I would love to see at least -Wall -Wextra by
default if we're using gcc.
--
Greg.
Re: make dist and cunit
On Wed, Apr 25, 2012, at 08:56 PM, Carson Gaspar wrote: On 4/25/12 6:07 PM, Greg Banks wrote: * Ditto warning flags - I would love to see at least -Wall -Wextra by default if we're using gcc. 1) Please actually detect gcc properly if you're going to do this. Fair enough. I'm so tired of removing gcc-isms from Makefiles (and from C and especially C++ code written by folks who wouldn't know a language standard if it bit them). Don't get me started... 2) Why? I can understand why a _developer_ wants all the warnings, but what value do they add to the poor schmuck who's just trying to build an IMAP server? It's just noise, and provides no benefit that I've ever been able to figure out. Because those schmucks have combinations of compilers, operating systems, and third party libraries which are not available to developers, and the developers natural process of code change inevitably introduces problems with those combinations, problems which better warnings might help find at compile time instead of months later then mis-compiled code is exercised. -- Greg.
Re: Automake Support for cyrus-imapd 2.5
G'day, On Wed, Apr 18, 2012, at 09:13 AM, Дилян Палаузов wrote: Hello Greg, I've done a bunch more testing on this branch and fixed a few issues which I found. Remaining issues are: * The cunit/ directory is proving resistant to automakification, but successfully builds and runs unit tests. * We're now building everything with -fPIC because the Perl extensions need functions from five .c files in lib/, which is silly. Neither of these are blockers for the merge, and could be cleaned up afterward. I say it's good to go. Thanks Dilyan for an excellent job :) -- Greg.
Re: Automake Support for cyrus-imapd 2.5
On Tue, Apr 17, 2012, at 03:09 PM, Dilyan Palauzov wrote:
commit Prepend the path in #include to .et - .h files
[...] I don't see why you would change
-#include imap_err.h
+#include imap/imap_err.h
without changing any of the other #include lines around it.
Generally, I think it would be a good thing to have a clear consistent
and documented policy for how we #include headers. We don't really have
that now, and I don't think this commit gets us any closer.
These files
are prefixed with the path, as in #include imap/imap_err.h, so that
the build works (this was my conclusion, when I was trying to build with
VPATH).
Why make the way in which files are #include'd different depending on
whether they are built sources or not? That seems very confusing. I
would very much prefer to have every #include line use the same format,
either
#include imap/imap_err.h /* relative path down from
top_{build|src}dir */
OR
#include imap_err.h /* filename only, rely on lots of -I options */
but not confusing admixtures of both. I don't care which one, just that
it's consistent. It would also be nice if the convention was documented
in doc/internal/hacking.
If you were experiencing problems with build order due to built headers
not being available when a .c file was being compiled, then that's what
BUILT_SOURCES is for.
I prefer to keep (AM_)CPPFLAGS minimalistic and have everthing
relative to the top_dirs. Automake includes automatically -I.
-I$(srcdir) and -I(the directory with config.h = top_builddir), so it
is not necessary to add those explicitly (even if it is done for
top?builddir).
Yes, I agree so far.
Specifying the path to generated and included *.h
files is the minimal effort required to keep AM_CPPFLAGS as small as
possible (top_dir and top_dir/lib).
I have two problems with this statement.
Firstly, I don't see why generated .h files need to be treated any
differently. If they're mentioned in BUILT_SOURCES then they get built
before everything else in the 'all' target, so by the time any other .c
file is compiled or the dependencies are extracted the generated .h
already exists and is found correctly.
Secondly, lib/ is not the only directory from which .h files are
included. I did a quick count based on gcc-generated .dep files and
found
3578 headers included into .c files, comprising
1456 included from .
174 include from top_dir into files in other directories (e.g. config.h)
1872 included from top_dir/lib into files in other directories
61 included from top_dir/imap into files in other directories
14 included from top_dir/sieve into files in other directories
1 included from top_dir/master into files in other directories
commit lib/Makefile.in: Reorder LIBCYR(M)_(OBJS, HDRS) alphabetically
Looks good. While you're doing that, how about splitting them up one
per line? We have all sorts of headaches doing git rebases of commits
which add or remove .c files because of this.
Okay, we can split them one per line, if you want. But you can also put
them all on the same line. Having them all on the same line, it is
quite easy to find where the change is.
Ah, but *finding* is not the problem; my text editor has a working
search function.
The problem is when you're rebasing a commit that adds a single source
file, and the upstream changes you're rebasing around add or remove
another source file. With multiple source files per line, you are far
more likely to get spurious context damage which prevents the git merge
from succeeding, which then requires a manual merge conflict fix, which
is annoying slow and error-prone. When Bron and I were actively working
on the conversations branch we would suffer through dozens of these
spurious merge conflicts per week. See commit
3aa2792f402a27687cf0ecdcd38654716436ec0c
The problem would probably not have appeared, if all the files were on
a single, very long line.
On the contrary, such an arrangement is the worst possible case because
every change to the list of files is a merge conflict waiting to happen.
It would have resulted in conflicts every single time we had to rebase
around a commit which added a source file, rather than about half of
them.
+master: master.o masterconf.o cyrusMasterMIB.o
the master program needs to depend on libcyrus.a in some way. For
example, in imapd/Makefile.in we do
In Makefile.am the dependencies are right.
Ok.
--
Greg.
Re: Automake Support for cyrus-imapd 2.5
G'day, On Sun, Apr 15, 2012, at 01:31 AM, Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . Some reviews follow. commit rename configure.in to configure.ac Looks good, but there's little point updating the $Id$ lines in configure.ac, in fact they should be removed. commit Update requirements for Autoconf from version 2.54 to 2.68 Looks good. I guess I'm going to be upgrading soon, I'm on 2.67 :( commit Add AS_HELP_STRING to AC_ARG_WITH and AC_ARG_ENABLE Looks good. +[AS_HELP_STRING([--enable-replication], [enable replication support (experimental)])], Hmm, I don't think this still qualifies as experimental commit .gitignore: remove sieve/, add perl/sieve/managesieve/ Looks good. commit */Makefile.in: .c.o.: removed newline before $ for consistency Looks good, commit imap/Makefile.in: add .c.snmp: recipe The old makefile will rebuild pushstats.c if the snmpgen script changes. The new one won't. commit lib/mkchartable.pl move output from stderr to stdout In mkchartable.pl, you need to handle the open() failing. Also, mkchartable.pl can fail partway through, e.g. if one of the charset/*.t files is not readable. In the old makefile, such a failure would trigger this code -|| (rm -f chartable.c exit 1) which would break the build and avoid half-creating a chartable.c so that the next attempt to build was also (correctly) fail. In the new makefile there is no such safeguard: a partly finished chartable.c would be created and the next attempt to build would (incorrectly) not fail at this point. commit Prepend the path in #include to .et - .h files I am deeply confused about why this is necessary, can you explain? Getting rid of these is obviously useful: -#include ../lib/imapurl.h -#include ../lib/util.h but I don't see why you would change -#include imap_err.h +#include imap/imap_err.h without changing any of the other #include lines around it. Also, here -CPPFLAGS = -I.. -I$(srcdir)/../sieve -I$(srcdir)/../imap -I$(srcdir)/../lib @COM_ERR_CPPFLAGS@ @CPPFLAGS@ @SASLFLAGS@ +CPPFLAGS = -I.. -I$(srcdir)/../lib @COM_ERR_CPPFLAGS@ @CPPFLAGS@ @SASLFLAGS@ it might be a good idea to use paths down from $(top_srcdir) instead of up-and-around from $(srcdir), like this CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib @COM_ERR_CPPFLAGS@ @CPPFLAGS@ @SASLFLAGS@ Generally, I think it would be a good thing to have a clear consistent and documented policy for how we #include headers. We don't really have that now, and I don't think this commit gets us any closer. commit */Makefile.in replace IMAP_COM_ERR_LIBS with COM_ERR_LIBS Looks good. commit Move def. Makefile.in:(PACKAGE/VERSION) to configure.ac:AC_INIT Looks good, but here +AC_INIT([cyrus-imapd], [2.5], [http://bugzilla.cyrusimap.org],,[http://www.cyrusimap.org]) it might be nice to have the version on it's own line, as we know that the version string will be modified once for every release. commit configure.ac: replace AC_CONFIG_HEADER with AC_CONFIG_HEADERS Looks good. commit lib/Makefile.in: Reorder LIBCYR(M)_(OBJS, HDRS) alphabetically Looks good. While you're doing that, how about splitting them up one per line? We have all sorts of headaches doing git rebases of commits which add or remove .c files because of this. commit lib/Makefile.in: Reorder LIBCYR(M)_(OBJS, HDRS) alphabetically + $(srcdir)/brearch.h s/brearch/bsearch/ +LIBCYRM_OBJS = assert.o hash.o imapopts.o libconfig.o mappedfile.o mpool.o \ + retry.o signals.o strhash.o util.o xmalloc.o xstrlcat.o xstrlcpy.o \ + @IPV6_OBJS@ map_@WITH_MAP@.o map_@WITH_MAP@.o is out of alphabetical order. commit lib/Makefile.in: expand ACL and AUTH +LIBCYR_OBJS = acl.o acl_afs.o auth.o auth_krb.o auth_unix.o auth_krb5.o \ auth_krb5.o and auth_pts.o are out of alphabetical order. commit lib/prot.h: Add #include config.h, remove from LIBCYR_HRDS I'm a little confused by your approach here. You say Since lib/prot.h #include config.h it was removed from LIBCYR_HDRS, but you also remove the #include config.h from lib/prot.h? Given that lib/prot.h will not compile correctly unless HAVE_SSL and HAVE_ZLIB are correctly defined, this seems unwise. commit remove inserting of files in both libcyrus and libcyrus_min I find this change confusing but I can't see anything wrong with it. However as long as it's a static library I think you could definitely go one step further and eliminate libcyrus_min entirely. Oh, I see you fixed the ordering issue with map_@WITH_LOCK@.o. commit lib/Makefile.am: move strarray from libcyrus.a to libcyrus_min.a Looks good, but here +master: master.o masterconf.o cyrusMasterMIB.o the master program needs to depend on libcyrus.a in some way. For example, in imapd/Makefile.in we do DEPLIBS = ../lib/libcyrus.a ../lib/libcyrus_min.a @DEPLIBS@ imapd: $(IMAPDOBJS) mutex_fake.o libimap.a
Re: Automake Support for cyrus-imapd 2.5
On Mon, Apr 16, 2012, at 04:44 PM, Greg Banks wrote:
Have to go now, more later.
commit */Makefile.in: add top_builddir
Looks good.
commit */Makefile.in: insert $(top_builddir) in all DEPLIBS
Looks good
commit */Makefile.in: add top_(builddir,srcdir) to CPPFLAGS
+CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib -I$(top_builddir)
-I$(top_builddir)/lib @COM_ERR_CPPFLAGS@ @CPPFLAGS@ @SASLFLAGS@
Surely the correct order is
-I$(top_builddir)
-I$(top_srcdir)
-I$(top_builddir)/lib
-I$(top_srcdir)/lib
?
Also, why do we have -I. -I$(srcdir) in some CPPFLAGS but not others?
commit (timsieved,sieve)/Makefile.in: OBJS order alphabetically
Looks good.
commit (master,perl/sieve/lib)/Makefile.in: add $(top_builddir) to
LIBS
Looks good
commit perl/Makefile*: enable building perl modules out of srcdir
+ if [ -f ../${srcdir}/$$d/Makefile.PL -a ! -f
Makefile ]; then \
This breaks if $srcdir is an absolute rather than a relative path. In
fact so does this
+ srcdir=../${srcdir}/$$d \
+ top_srcdir=../$(top_srcdir) \
It might be a whole lot simpler to generate Makefile.PL from
Makefile.PL.in at configure time. Then, you could run the Makefile.PL
from configure using AC_CONFIG_COMMANDS.
+INSTALLDIRS = 'vendor',
Please don't do that! I just had to do a whole bunch of futzing to
avoid it :(
commit perl/imap/IMAP.xs: remove #include config.h
Looks good.
commit imap/Makefile.in: remove linking with tls.o, when libimap.a
linked
Looks good.
commit imap/Makefile.in: remove linking with mupdate-client.o, when
libimap.a
Looks good
commit imap/Makefile.in: remove backend.o and imapparse.o, when -limap
Looks good
commit imap/Makefile.in: idled: do not link explicitly with idlemsg.o
Looks good
commit imap/Makefile.in: order everything alphabetically
My comments from lib/Makefile.in about splitting these up 1 per line
also apply here.
In LOBJS, imapparse,o, message.o, saslclient.o, saslserver.o are out of
order.
In IMAPDOBJS, proxy.o is out of order.
In PROGS, cyr_dbtool and cyrdump are out of order
There's a reason why $SERVICE ought to be linked first, or at the very
least before any libraries from outside the project: it contains main()
and must have the first definition of main() seen by the linker even if
some library also defines main().
cyr_info links masterconf.o but does not depend on it.
commit imap/Makefile.in: abolish SIEVE_CPPFLAGS
Looks good.
commit rename $service_path to $servicedir
+AC_DEFINE_UNQUOTED(SERVICE_PATH,$servicedir,[Directory to use for
service binaries])
Don't you want to rename SERVICE_PATH too?
Also, the difference between $servicedir and $bindir is slight; in the
default arrangement one will be /usr/cyrus/bin and the other /usr/bin.
I would hope that eventually we could eliminate $cyrus_prefix and
replace $servicedir with $bindir.
commit remove cyrus_prefix from every Makefile.in, as it is unused
Since three days ago, it is used, in the perl/ directories to expand
$PERL_PREINSTALL. Otherwise, looks good.
commit configure.ac: remove check for libfl
Looks good,
commit configure.ac: remove AC_DEFINE(WITH_PTS,[],[Build in PTS
support?])
Looks good
commit configure.ac: remove SNMP_SUBDIRS
Sure. I hope to resurrect the SNMP code in some future version, maybe
2.6, but for now we can remove this.
commit configure.ac: remove ${SQL_LIBS} from IMAP_LIBS
Looks good
commit Initial Automake support
Do we need the 'depend' target? Surely automake handles that for us.
+ SIEVE_LIBS=../sieve/libsieve.a
$(top_builddir)
commit Makefile.am: merge man/Makefile.in
Looks good.
This time I really have to go, more tomorrow.
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #514
On Sat, Apr 14, 2012, at 05:07 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/514/ -- [...truncated 2479 lines...] Cassandane/MasterStart.pm syntax OK + sed -e 's|^##destdir =.*$|destdir = http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/inst|' -e 's|^##pwcheck = .*$|pwcheck = sasldb|' + rm -rf reports.old + mv -f reports reports.old + mkdir -m 0777 reports + ./testrunner.pl --cleanup -f xml -v + exitcode=1 + '[' -x jenkins-xml-summary.pl ']' + ./jenkins-xml-summary.pl --build-url=http://ci.cyrusimap.org/job/cyrus-imapd-master/514/ Test failures and errors summary Cassandane::Cyrus::Annotator.add_annot_deliver http://ci.cyrusimap.org/job/cyrus-imapd-master/514//testReport/%28root%29/Cassandane__Cyrus__Annotator/test_add_annot_deliver/ Woops. My fix for Bug 3652 changed the installation location of the Cyrus Annotator perl module back again to it's earlier location, without adjusting the way that Cassandane sets up $PERL5LIB to find it. Fixed in The following changes since commit cb4a4480ec1a90e40e08482130432062469e558d: Lsub tests create a user with multi-level folders (2012-04-11 09:15:46 +1000) are available in the git repository at: ssh://git.cyrusimap.org/git/cassandane master Greg Banks (1): Correct Cyrus perl module installation directories Cassandane/Instance.pm | 21 +++-- 1 files changed, 15 insertions(+), 6 deletions(-) -- Greg.
Re: Automake Support for cyrus-imapd 2.5
On Sun, Apr 15, 2012, at 08:27 PM, Bron Gondwana wrote: On Sun, Apr 15, 2012, at 01:31 AM, Дилян Палаузов wrote: Hello, at git://demo.aegee.org:143/cyrus-imapd.git , branch dpa/automake I have patched cyrus-imapd/master to support Automake . With the exception of perl and CUnit, I it works fine and permits building in a directory, different from the source directory. All dependencies are moved to a single Makefile.am (except CUnit and Perl). The point with CUnit is, that I did not manage to get it running on my system, so I left it as is. No worries, I can fix it. It's an awful mess of hacks, for which I blame the suboptimal nature of the CUnit library (and my insistence on escaping those limitations). The Makefile's generated from Makefile.PL do not permit compiling perl files in a directory, different from the source directory. I wonder if we can change from Makefile.PL to one of the more modern Perl build systems... I don't use srcdir builds myself, but proper autotools projects are supposed to support them, so it would be nice to have something better than Makefile.PLs. I hope you will like the result. Let me know, if you experience any (compilation) problems, after merging the changes. I will respond promptly. By the way, when approximately will be v2.5 released? We don't know for sure. I had a good chat with Greg in Melbourne last week about what still needs to be done. My plan is to build a set of A bugs that need to be resolved, and make sure everything we're thinking about is on that list! Then we'll have a clearer idea. I think we agree that it needs to be soonish. The largest remaining issue is working out how to make upgrades from 2.3 and 2.4 as painless as possible, and how far we're willing to go to achieve that. Other that that, I have on my agenda for 2.5: - some pending cleanups - improving the way 'quota -f' works (again) - fix a lot of documentation Regardless, I think we should merge your automake code now, so we have a chance to test it for a while! Agreed, I think we should merge it as soon as we can get it to build. Thanks for all your work on this, Seconded, heartily! -- Greg.
Re: Let's map tickets to milestones
On Sun, Apr 15, 2012, at 03:43 PM, Jeroen van Meeuwen (Kolab Systems) wrote: A conversation has been started on when Cyrus IMAP 2.5 would be released. [...] If we were to give people a week or so to do all of this, then I reckon next week we could have a meeting to review everything we want to do for 2.5. Ok, works for me. I've added Blocks: 3674 to my remaining 2.5 tickets. So, I've created a 2.5.0 release blocker ticket; https://bugzilla.cyrusimap.org/show_bug.cgi?id=3674 [...] Another example enhancement, #3343[3] (conversations support), currently has no dependencies... but I'm sure these will pop up as the work on the feature moves back to origin and closer and closer to master. I was under the impression from reading http://cyrusimap.org/mediawiki/index.php/RoadMap that conversations support was due in 2.6. Otherwise we would have merged it by now! -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #509
On Wed, Apr 11, 2012, at 05:08 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/509/ [...] --build-url=http://ci.cyrusimap.org/job/cyrus-imapd-master/509/ Test failures and errors summary Cassandane::Cyrus::Sieve.deliver_fileinto_dot http://ci.cyrusimap.org/job/cyrus-imapd-master/509//testReport/%28root%29/Cassandane__Cyrus__Sieve/test_deliver_fileinto_dot/ This is correctly detecting Bug 3664 https://bugzilla.cyrusimap.org/show_bug.cgi?id=3664 which is still broken in the master branch. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #507
On Tue, Apr 10, 2012, at 05:09 PM, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/507/
Test failures and errors summary
Cassandane::Cyrus::Lsub.lsub_toplevel
http://ci.cyrusimap.org/job/cyrus-imapd-master/507//testReport/%28root%29/Cassandane__Cyrus__Lsub/test_lsub_toplevel/
The Lsub tests were using this syntax to create a user
# sub folders of another user - one is subscribable
$self-{instance}-create_user(other,
subdirs = [ qw(sub sub.folder) ]);
A recent change to support running tests with unixhierarchysep=on made
the checking in create_user() stricter and it now rejects 'sub.folder'
because it contains the default hierarchy separator. Creating a 2-level
folder is a perfectly reasonable thing to want to do here, so I fixed
this by adding a new syntax to support that without the ambiguity:
# sub folders of another user - one is subscribable
$self-{instance}-create_user(other,
subdirs = [ 'sub', ['sub', 'folder']
]);
Fixed in
http://git.cyrusimap.org/cassandane/commit/?id=cb4a4480ec1a90e40e08482130432062469e558d
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #505
G'day, On Mon, Apr 9, 2012, at 05:03 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/505/ [...] + make [] Can't locate File/chdir.pm in @INC (@INC contains: /usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi /usr/lib/perl5/site_perl/5.8.8 /usr/lib/perl5/site_perl /usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.8 /usr/lib/perl5/vendor_perl /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi /usr/lib/perl5/5.8.8 .) at Cassandane/Test/Cassini.pm line 46. BEGIN failed--compilation aborted at Cassandane/Test/Cassini.pm line 46. Sorry, this build was repeatedly failing every 12 hours while I was away for the long weekend, because the File::chdir package wasn't installed on the build machine. Fixed using sudo yum install perl-File-chdir. Also documented in http://git.cyrusimap.org/cassandane/commit/?id=55649d206dcf2fbd178409d93243125067b7ccd7 -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #453
On Wed, Mar 14, 2012, at 05:07 PM, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/453/
--
[...truncated 2028 lines...]
Test failures and errors summary
Cassandane::Cyrus::Sieve.badscript_timsieved
http://ci.cyrusimap.org/job/cyrus-imapd-master/453//testReport/%28root%29/Cassandane__Cyrus__Sieve/test_badscript_timsieved/
Error Message
Boolean assertion failed
Stacktrace
test_badscript_timsieved(Cassandane::Cyrus::Sieve)
Boolean assertion failed at
/usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/Exception.pm line 13
Test::Unit::Exception::throw_new('Test::Unit::Failure=HASH(0x3b24e60)',
'-package', 'Cassandane::Cyrus::Sieve', '-file',
'Cassandane/Cyrus/Sieve.pm', '-line', 289, '-object',
'Cassandane::Cyrus::Sieve=HASH(0x346ea80)', ...) called at
/usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/Assert.pm line 85
Test::Unit::Assert::do_assertion('Cassandane::Cyrus::Sieve=HASH(0x346ea80)',
'Test::Unit::Assertion::Boolean=SCALAR(0x3b45050)',
'Cassandane::Cyrus::Sieve', 'Cassandane/Cyrus/Sieve.pm', 289)
called at /usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/Assert.pm
line 19
Test::Unit::Assert::assert('Cassandane::Cyrus::Sieve=HASH(0x346ea80)')
called at Cassandane/Cyrus/Sieve.pm line 289
Cassandane::Cyrus::Sieve::badscript_common('Cassandane::Cyrus::Sieve=HASH(0x346ea80)')
called at Cassandane/Cyrus/Sieve.pm line 364
Cassandane::Cyrus::Sieve::test_badscript_timsieved('Cassandane::Cyrus::Sieve=HASH(0x346ea80)')
called at /usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/TestCase.pm
line 75
[...framework calls elided...]
286 ($res, @errs) = $self-compile_sieve_script('badrequire',
287 require [\nonesuch\];\n);
288 $self-assert_str_equals('failure', $res);
289 $self-assert(grep m/Unsupported feature.*nonesuch/, @errs);
290
So the test code ran the installsieve script and it failed to produce
the expected output.
What it did produce was
= Cyrus::Sieve[362] Testing sieve script compile failures, via
timsieved
= Cyrus::Sieve[185] Checking preconditions for compiling sieve
script badrequire
= Cyrus::Sieve[203] Running installsieve on script badrequire
= Instance[1063] Running:
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/installsieve
-i /var/tmp/cass/210257136/badrequire.script -u cassandane
127.0.0.1:9102
= Cyrus::Sieve[118] errors:
= Cyrus::Sieve[119] Can't locate Cyrus/SIEVE/managesieve.pm in @INC
(@INC contains: /usr/cyrus/lib/perl /usr/cyrus/share/perl
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/cassandane
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/share/perl
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/lib/perl
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/lib/perl5/site_perl/5.8.8
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/lib/perl5/site_perl
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib/perl5/site_perl/5.8.8
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib/perl5/site_perl
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib/perl5/vendor_perl/5.8.8
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib/perl5/vendor_perl
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/lib/perl5/5.8.8
/usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.8 /usr/lib/perl5/site_perl
/usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.8 /usr/lib/perl5/vendor_perl
/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi /usr/lib/perl5/5.8.8 .)
at
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/installsieve
line 45.
= Cyrus::Sieve[119] BEGIN failed--compilation aborted at
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/installsieve
line 45.
Rats, more Perl module path problems. I've raised
https://bugzilla.cyrusimap.org/show_bug.cgi?id=3652
--
Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #449
On Mon, Mar 12, 2012, at 10:10 PM, Bron Gondwana wrote:
On Mon, Mar 12, 2012 at 05:03:06PM -0400, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/449/changes
No idea what actually went wrong there - I don't know how to read
these stack dumps. It's uglier than Java I tell you!
Cassandane is run in -v (verbose) mode, which generates enormous amounts
of output, which is shoved aside into the file cass.errs, which you can
view from
the web interface thusly:
http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/cassandane/cass.errs/*view*/
The last few lines are
...
= Instance[688] start main instance for test
test_cve_2011_3208_list_active: basedir /var/tmp/cass/21021782
= Instance[1063] Running: /usr/sbin/saslpasswd2 -f
/var/tmp/cass/21021782/conf/sasldb2 -c -p admin
= Instance[526] _start_master: logging to
/var/tmp/cass/21021782/conf/master.log
= Instance[1063] Running:
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/master
-C /var/tmp/cass/21021782/conf/imapd.conf -l 255 -p
/var/tmp/cass/21021782/run/cyrus.pid -d -M
/var/tmp/cass/21021782/conf/cyrus.conf -L
/var/tmp/cass/21021782/conf/master.log
= Instance[535] _start_master: waiting for PID file
= Instance[538] _start_master: PID file present and correct
= Instance[544] _start_master: PID waiting for services
= Service[269] is_listening: service nntp is listening on
127.0.0.1:9101
= Instance[555] _start_master: all services listening
= Instance[652] create user cassandane
= Instance[1063] Running: /usr/sbin/saslpasswd2 -f
/var/tmp/cass/21021782/conf/sasldb2 -c -p cassandane
News::NNTPClient::SOCK1 connecting to localhost.localdomain:9101
News::NNTPClient unexpected EOF on News::NNTPClient::SOCK1
at Cassandane/Cyrus/Nntp.pm line 66
News::NNTPClient::SOCK1 command: AUTHINFO USER cassandane
That looks like the NNTPClient Perl code failed because the nntpd it was
talking
to dropped the connection, probably a result of terminating prematurely.
Also, every other test seems to follow a pattern like
= Instance[688] start main instance for test
test_set_system_flag_deliver: basedir /var/tmp/cass/2102173
= Instance[1063] Running: /usr/sbin/saslpasswd2 -f
/var/tmp/cass/2102173/conf/sasldb2 -c -p admin
= Instance[526] _start_master: logging to
/var/tmp/cass/2102173/conf/master.log
= Instance[1063] Running:
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/master
-C /var/tmp/cass/2102173/conf/imapd.conf -l 255 -p
/var/tmp/cass/2102173/run/cyrus.pid -d -M
/var/tmp/cass/2102173/conf/cyrus.conf -L
/var/tmp/cass/2102173/conf/master.log
= Instance[535] _start_master: waiting for PID file
= Instance[538] _start_master: PID file present and correct
= Instance[544] _start_master: PID waiting for services
WARNING: Port 9102 never freed. Allocated at Cassandane/Service.pm
line 100
Cassandane::Service::port('Cassandane::IMAPService=HASH(0xa7b84c0)')
called at Cassandane/Service.pm line 142
[...Perl stack trace...]
This is missing the bulk of the test xlog() calls, and seems to have
failed early in instance startup.
Let's have a look to see if there's any cores. Normally, tests will be
failed if we get
to the end of them and Cassandane notices there are cores in the
instance directories,
but in this case because of the Port 9102 never freed warning I
suspect instances
are not being shut down properly.
root@hudson-01 2004 cd /var/tmp/cass
root@hudson-01 2004 find * -name core\* -type f | while read f ; do echo
-n $f: ; gdb -q --batch -c $f | sed -ne 's|Core was generated by ||p'
; done
2102171/conf/cores/core.20475:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021710/conf/cores/core.20536:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021711/conf/cores/core.20543:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021712/conf/cores/core.20550:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021713/conf/cores/core.20557:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021714/conf/cores/core.20564:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021715/conf/cores/core.20570:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021716/conf/cores/core.20577:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021717/conf/cores/core.20583:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021718/conf/cores/core.20589:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
21021719/conf/cores/core.20594:
`/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/ctl_cyrus'.
2102172/conf/cores/core.20483:
Re: Build failed in Jenkins: cyrus-imapd-master #410
Sent from my iPhone On 25/02/2012, at 21:33, Jeroen van Meeuwen (Kolab Systems) vanmeeu...@kolabsys.com wrote: On 2012-02-24 3:40, Greg Banks wrote: I logged in earlier this morning and grabbed a copy of that directory before the next build ran (each build removes the leftovers from the previous one and I'm now wondering whether that's a good thing). If you want, a utility called tmpwatch can clean up files and directories that are older than $x weeks or $y days in this directory, allowing us some time to get to the root of a problem using the original data, while not filling up the filesystem. Sadly without the cleanup code Cassandane will fill the whole vmdk before it finishes a single run. Each instance directory takes 20 to 40 MB, which is almost entirely Berkeley DB environment files. The 135 test cases use at least one instance directory each. Perhaps it might be better to delete only the BDB junk instead of the whole directory. Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #410
On Thu, Feb 23, 2012, at 05:07 AM, Jenkins wrote:
See http://ci.cyrusimap.org/job/cyrus-imapd-master/410/
--
[...]
+ exitcode=1
+ '[' -x jenkins-xml-summary.pl ']'
+ ./jenkins-xml-summary.pl
--build-url=http://ci.cyrusimap.org/job/cyrus-imapd-master/410/
Test failures and errors summary
Ok, so the good news first.
* these test failure report URLs are now correct
* the stray process killer in autobuild.sh is working
Now for the test failures. Thirty-four of these thirty-five appear to be of
the nature
test_dup_keep_keep(Cassandane::Cyrus::Sieve)
Perl exception: Some process is already listening on 127.0.0.1:9101
which AFAICS are all cascading failures due to Cyrus failing to shut down
correctly here:
Cassandane::Cyrus::Bug3072.copy_longset
http://ci.cyrusimap.org/job/cyrus-imapd-master/410//testReport/%28root%29/Cassandane__Cyrus__Bug3072/test_copy_longset/
which is thorny one. The error message is
Error Message
Perl exception: Cannot shut down master pid 14360
Stacktrace
test_copy_longset(Cassandane::Cyrus::Bug3072)
Perl exception: Cannot shut down master pid 14360
at Cassandane/Instance.pm line 831
Cassandane::Instance::stop('Cassandane::Instance=HASH(0x1db7eb80)')
called at Cassandane/Cyrus/TestCase.pm line 255
Cassandane::Cyrus::TestCase::tear_down('Cassandane::Cyrus::Bug3072=HASH(0x1c2642d0)')
called at Cassandane/Cyrus/Bug3072.pm line 64
Cassandane::Cyrus::Bug3072::tear_down('Cassandane::Cyrus::Bug3072=HASH(0x1c2642d0)')
called at /usr/lib/perl5/vendor_perl/5.8.8/Test/Unit/TestCase.pm line 65
So Cassandane::Instance::_stop_pid() returned false. This happens because it
fails to shut
down the master process gracefully, where graceful means the master
terminates within about
20 seconds of receiving SIGQUIT.
We can see the sequence in syslog
Feb 23 05:04:55 hudson-01 cassandane: = Instance[826] stop
Feb 23 05:04:55 hudson-01 cassandane: = Instance[804] _stop_pid: sending
signal 3 to 14360
Feb 23 05:05:24 hudson-01 cassandane: = Instance[811] _stop_pid: failed to
shut down pid 14360 with signal 3
Feb 23 05:05:24 hudson-01 cassandane: = Instance[804] _stop_pid: sending
signal 4 to 14360
Feb 23 05:05:24 hudson-01 cassandane: = Util::Wait[75] Waited 0.011758 sec
for unknown condition
At 05:04:55, _stop_pid() sent SIGQUIT (signal 3) to the master process.
Twenty-nine seconds
later, it gave up waiting and sent SIGILL (signal 4) in the hope of getting a
core file. Twelve
milliseconds later that hope was fulfilled.
There's nothing else interesting in the log. In particular, no unusual
messages from master.
The previous message which mentions instance starting is
Feb 23 05:03:17 hudson-01 cassandane: = Instance[688] start main instance
for test test_copy_longset: basedir /var/tmp/cass/1003145
I logged in earlier this morning and grabbed a copy of that directory before
the next
build ran (each build removes the leftovers from the previous one and I'm now
wondering
whether that's a good thing).
gnb@hudson-01 2002 ls -l 1003145/conf/cores/
total 840
-rw--- 1 gnb gnb 851968 Feb 23 05:05 core.14360
gnb@hudson-01 2004 file 1003145/conf/cores/core.14360
1003145/conf/cores/core.14360: ELF 64-bit LSB core file AMD x86-64, version 1
(SYSV), SVR4-style, from 'master'
gnb@hudson-01 2005 gdb
/var/lib/jenkins/jobs/cyrus-imapd-master/workspace/inst/usr/cyrus/bin/master
1003145/conf/cores/core.14360
...
Program terminated with signal 4, Illegal instruction.
#0 0x00406903 in child_janitor (now=...) at master.c:1060
1060janitor_position = janitor_position % child_table_size;
(gdb) bt
#0 0x00406903 in child_janitor (now=...) at master.c:1060
#1 0x0040c916 in main (argc=12, argv=0x7fffade2ce88) at master.c:2272
At this point I poked around and eventually worked out that master was busy
looping into and out of the main select loop and the janitor code, but the
janitor
code itself wasn't stuck in a loop.
The child table contains exactly one entry.
(gdb) p ctable
$1 = {0x0 repeats 4366 times, 0x13be10a0, 0x0 repeats 5633 times}
(gdb) p *(struct centry *)0x13be10a0
$2 = {pid = 14366, service_state = SERVICE_STATE_READY, janitor_deadline = 0,
si = 0, next = 0x0}
According to syslog, pid 14366 was an imapd
Feb 23 05:03:17 hudson-01 1003145/imap[14366]: login: localhost.localdomain
[127.0.0.1] admin plaintext User logged in
SESSIONID=1003145-14366-1329991397-1
This explains why master didn't die, it's waiting for this child imapd to die.
Indeed,
we see the struct service has the correct nactive and ready_workers counts
(gdb) p Services[0]
$5 = {name = 0x13be12c0 imap, listen = 0x13be1120 127.0.0.1:9104, proto =
0x13be1280 tcp, exec = 0x13be1360, babysit = 0,
associate = 0, family = 2, socket = 5, stat = {7, 9}, desired_workers = 0,
max_workers = 2147483647,
Re: Build failed in Jenkins: cyrus-imapd-master #402
On Sun, Feb 19, 2012, at 05:03 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/402/ -- [...truncated 1246 lines...] for file in ./imtest.1 ./pop3test.1 ./nntptest.1 ./lmtptest.1 ./smtptest.1 ./sivtest.1 ./mupdatetest.1 ./installsieve.1 ./sieveshell.1; \ There are at least four separate problems happening here, all at once :( 1) the Master.service_ipv6 test is failing because the older netstat program on ci.cyrusimap.org reports connect TCP/IPv6 sockets slightly differently than Cassandane was expecting. I fixed that here http://git.cyrusimap.org/cassandane/commit/?id=a76ec6226f0bad1bd81238a8b80360d58c894080 2) The Master.maxforkrate test was accidentally triggering an old and obscure race condition bug in the master process. I made the test not trigger that bug anymore http://git.cyrusimap.org/cassandane/commit/?id=10999db507143bdc668d49f59349f23ba61fc8e1 and added a new test just for that bug http://git.cyrusimap.org/cassandane/commit/?id=ad9bf14faddef02f070f5b82fb8f408e37826b1c and fixed the bug http://git.cyrusimap.org/cyrus-imapd/commit/?id=d288a9ad66636ab406e537b1fb57f05f016e1f38 3) The Master.maxforkrate test has detected that the maxforkrate parameter is not being enforced correctly, which is strange because that code did in fact work fine in that test when last modified. I suspect some kind of environmental problem is breaking the fork rate calculation, will investigate further. 4) Cassandane sometimes leaves master and lemming processes lying around. I haven't been able to reproduce that problem, although I have solved it several times before. Those leaked processes are never cleaned up and hog the TCP ports that Cassandane expects to be able to use, causing subsequent Cassandane runs to fail spuriously. I'm not entirely sure of the best way to address this, but I'm thinking of something like a sledgehammer which kills all processes running as the cyrus userid. So sadly it will be another day or so before the build gets back to stable. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #402
On Mon, Feb 20, 2012, at 04:14 PM, Jeroen van Meeuwen (Kolab Systems) wrote: On 2012-02-20 11:44, Greg Banks wrote: Sent from my iPhone On 20/02/2012, at 19:13, Bron Gondwana br...@fastmail.fm wrote: On Mon, Feb 20, 2012 at 07:00:52PM +1100, Greg Banks wrote: 4) Cassandane sometimes leaves master and lemming processes lying around. I haven't been able to reproduce that problem, although I have solved it several times before. Those leaked processes are never cleaned up and hog the TCP ports that Cassandane expects to be able to use, causing subsequent Cassandane runs to fail spuriously. I'm not entirely sure of the best way to address this, but I'm thinking of something like a sledgehammer which kills all processes running as the cyrus userid. My 'cyrus-devtools' scripts use a sledgehammer that looks for the -C option that I set on all my processes. Yeah, I could identify master processes started by Cassandane, and service processes started by those masters, by looking for -C options. I might make --cleanup do that. There's also the opportunity to have Jenkins execute a 'sudo pkill -9 -u cyrus' at the start of a run, perhaps? This is what I do manually on my box when I see that happen. It's easy and it deals with the case of a test forking off processes which don't have Cyrus -C arguments in their commandlines, but it will shut down any running production servers on the same machine, which is a nasty little landmine to be leaving other people to trip over. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #402
On Mon, Feb 20, 2012, at 07:00 PM, Greg Banks wrote: On Sun, Feb 19, 2012, at 05:03 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/402/ 3) The Master.maxforkrate test has detected that the maxforkrate parameter is not being enforced correctly, which is strange because that code did in fact work fine in that test when last modified. I suspect some kind of environmental problem is breaking the fork rate calculation, will investigate further. I fixed this here http://git.cyrusimap.org/cyrus-imapd/commit/?id=8afe4017853079290cf8f0dca5b1d6244756d209 and another nasty bug that I discovered while debugging that. maxforkrate was very broken :( Hopefully the next Jenkins build will be clean, sorry for the noise. 4) Cassandane sometimes leaves master and lemming processes lying around. Will address this tomorrow. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #394
G'day On Thu, Feb 16, 2012, at 12:45 AM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/394/ -- [...truncated 1809 lines...] for file in ./imclient.3; \ This seems to be due to the new Cassandane tests for the master program failing in interesting new ways that I'd not seen before, and leaving dead master processes holding on to ports, which breaks subsequent runs. Still investigating. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #387
Sent from my iPhone On 13/02/2012, at 21:03, Jenkins do-not-re...@cyrusimap.org wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/387/ -- [...truncated 1803 lines...] make[1]: Entering directory ` This was due to my commits to Cassandane earlier today, which added a dependancy on a Perl module which appears not to be installed: Can't locate FreezeThaw.pm in @INC (@INC contains: /usr/lib64/perl5/ site_perl/5.8.8/x86_64-linux-thread-multi /usr/lib/perl5/site_perl/ 5.8.8 /usr/lib/perl5/site_perl /usr/lib64/perl5/vendor_perl/5.8.8/ x86_64-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.8 /usr/lib/ perl5/vendor_perl /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi / usr/lib/perl5/5.8.8 .) at Cassandane/Unit/TestPlan.pm line 97. BEGIN failed--compilation aborted at Cassandane/Unit/TestPlan.pm line 97. Compilation failed in require at ./testrunner.pl line 46. BEGIN failed--compilation aborted at ./testrunner.pl line 46. I'll fix this tomorrow. Greg.
Re: FOSDEM Report - Saturday
On Thu, Feb 9, 2012, at 09:58 AM, Jeroen van Meeuwen wrote: I think the important thing is trackability from a user point of view. Let's say I'm a Cyrus sysadmin who has discovered a bug in their installation, which is described in a Bugzilla ticket and already fixed in some newer release. The key things I want to know are: - is there a fix? - if so, is that fix in a release yet? - if so, which release(s) is it? - if not, where can I get a patch that I can apply to my install? - and/or, for the last point, where/how can I request the patch be made available for a current release series (i.e. 2.4.$z) Good point. Special-Use: Technically, it's been outlined what Kolab Systems is seeking to do here, and as it is not so much on the roadmap for other parties involved, we're therefore seeking ways to allow us to also actually do the work (instead of asking other parties to do the work for us). I'm looking forward to it, as currently I may have seemed to ill- / ambiguously define what it is we're trying to do exactly. What we're specifically looking for, paraphrasing a 30.000 ft high-level birds view, is xCal and xCard stored in IMAP folders that are also made available through CalDAV / CardDAV, Ok. where such folders are annotated through SPECIAL-USE, Sounds good. so that any IMAP client is automatically compatible. I'm not entirely sure what you mean by compatible. If a client understood Content-Type: application/calendar+xml and did something useful with it, like display the calendar, or start up a separate calendar tool, then it's compatible. If a client didn't understand that, a user opening that folder would see a bunch of XML gibberish, which is very nearly the moral equivalent of not showing anything at all, except more confusing. SPECIAL-USE already requires ENABLE SPECIAL-USE by the client, No it doesn't. RFC6154 says no such thing. and the server can therefore (to a client not enabling SPECIAL-USE) just hide the \Calendar and/or \Contacts folders. Hmm, this is an ugly area. RFC6154 defines a new RETURN (SPECIAL-USE) option for the LIST command, which allows a client to specifically request a listing of mailboxes including the SPECIAL-USE information. Cyrus recognises this and adds in the SPECIAL-USE information. However, RFC6154 allows the server to emit that information even when not specifically requested to. This is legal thanks to a loophole in RFC5258, and the example in section 5.1 of RFC6154 shows this happening. Apparently the iOS 5 client expects that to happen: [1]http://bugzilla.cyrusimap.org/bugzilla3/show_bug.cgi?id=3642 Such-and-such requires an extension to the current SPECIAL-USE RFC though, as currently it only ever involves messages, does not set any standards regarding the format of folder's contents (as it only involves mail). There need to be some clarifications on what is a private annotation within SPECIAL-USE, Visibility semantics of the new attributes are extremely fuzzy and poorly thought out in RFC6154. and what could arguably be a shared annotation, all of which justify an extension / replacement of the current RFC to be drafted. Cyrus does an ugly thing here - it exports an annotation /private/specialuse but the annotation's behaviour is almost exactly the normal /shared semantics. In fact the mapping between the semantics of annotation visibility and special-use attribute visibility are not at all obvious. I explained this problem in an internal post the other day The special-use data is already defined in the RFC as being tied to a specific annotation. Unfortunately the RFC authors didn't quite get the visibility semantics right. The annotation is defined in the final RFC to be /private/specialuse, which sounds right at first glance but is a poor approximation to the actual underlying semantics of some of the keywords. It is also a pain to implement because it means storing multiple values on the mailbox, one for each user who has set a value. An earlier draft had /shared/specialuse, which has different problems. Cyrus compromises by calling the annotation /private/specialuse, enforcing /private access controls, but storing it in a way which is actually more like /shared. The *real* visibility semantics of special-use are fuzzy and not well addressed in the RFC. They probably vary from keyword to keyword, and the most sensible set of semantics for most of those keywords are probably not either of the semantics defined for /private or /shared but a third thing. Those might be something like this is a flag stored on a mailbox, which any user can see, but which has a special effect only when accessed by the owner of the mailbox, but that's not the only possibility. Another possibility might be this is a flag stored on a mailbox, which only a specific user can see, which has a special effect if seen. Or even
Re: FOSDEM Report - Saturday
On Wed, Feb 8, 2012, at 05:32 PM, Jeroen van Meeuwen (Kolab Systems) wrote: On 2012-02-08 5:08, Greg Banks wrote: On Tue, Feb 7, 2012, at 02:27 PM, Bron Gondwana wrote: Have they announced the URL for the videos? I think the links are on every talk page on the FOSDEM schedule on fosdem.org. Neat. - Gerrit? Some sort of code review process to make it easier to keep track of the work from drive-by contributers. I see two benefits to Gerrit or something like it a) casual contributions don't get lost in the noise b) regular contributors get regular code reviews There's a couple more; - Jenkins can be put in charge of giving the first thumbs-up for the commit(-series) but letting a build using the commit(s) succeed, possibly along with Cassandane executing successful tests. I went to a talk about the OpenStack development process at LCA. They use both Gerrit and Jenkins but put Gerrit first in the pipeline so that all patches get a human review in Gerrit before they undergo integration testing in Jenkins. It seems that the other order is equivalent to allowing random unknown people to execute unreviewed code on your CI machine, in an environment which may have write access to the git repo, which is something of a security problem. - New contributors can be trusted to commit to what goes through Gerrit, lowering the entry barrier. Suppose Kolab Systems, for example, were to find a C programmer with some C experience, that person will obviously need quite some time to catch up with everything that happens within Cyrus IMAP. One can imagine how automating (a large part of) as well as requiring such a person's contributions be reviewed serves both parties quite well. Agree entirely. - Bugzilla - use it for everything. If it doesn't have a bug number where discussion took place, it doesn't get accepted. It's a transparency thing, as well as a planning thing. Then afterwards, its a reporting thing. Sure. Sometimes though transparency is not good, e.g. when dealing with a reported but not yet fixed vulnerability. Those exceptions need to be handled gracefully and in a defined manner. This might be create a bug number with full details after the software release, or have a magic Security flag on the ticket which restricts its visibility or have a separate Bugzilla instance. Another thing I'd like to see is a push hook on git.cyrusimap.org's master and 2.4 branches, so that if a git commit has the text Bug in the subject then the commit message is copied to Bugzilla and the bug marked RESOLVED - FIXED. What this does is create properly hard dependencies between GIT branches and Bugzilla versions/milestones. Yes. This isn't a bad thing, but we are currently winging it between the two. It means every time a thing happens on the one, the thing needs to happen on the other as well, and properly so that the match between the two things can be made by such program. Yes. I think the important thing is trackability from a user point of view. Let's say I'm a Cyrus sysadmin who has discovered a bug in their installation, which is described in a Bugzilla ticket and already fixed in some newer release. The key things I want to know are: - is there a fix? - if so, is that fix in a release yet? - if so, which release(s) is it? - if not, where can I get a patch that I can apply to my install? Having that information right there in the Bugzilla entry, and guaranteed by automation to be present and correct, would a signficant win. I've worked in an environment where the BTS and VCS did this dance, and it made the support job *much* easier. I wonder if DIstZilla or something like it could be used to automate the release process? I'm gonna go out on a limb and say Not Your Problem(TM) - it's why non-coders like me exist and can have some value to the project ;-) Sweet! It's not like I have insufficient problems. Special-Use: (...snip...) Yes, our RFC compliance is basic here. Technically, it's been outlined what Kolab Systems is seeking to do here, and as it is not so much on the roadmap for other parties involved, we're therefore seeking ways to allow us to also actually do the work (instead of asking other parties to do the work for us). I'm looking forward to it, as currently I may have seemed to ill- / ambiguously define what it is we're trying to do exactly. Would that be http://wiki.kolab.org/KEP:9 ? I really need to read and comment on that... * Undo. Wait, what!? I think this comes from the realm of the anti-Are you sure? movement. Asking whether or not a user is sure the user wants to perform an action the user performed is absolutely intrusive to one's productivity, and simply annoying. Instead of asking for confirmation, it is argued, with which I agree, one should offer undo to recover from mistakes. Ah I see. I
Re: FOSDEM Report - Saturday
G'day, Sounds like you're having fun at FOSDEM :) On Tue, Feb 7, 2012, at 02:27 PM, Bron Gondwana wrote: (I'm copying this to the cyrus-devel list because it's of interest to Cyrus people too...) I'm replying here on the same principle. http://fosdem.org/2012/schedule/event/keynotes_welcome [...] There are 420 talks, 273 hours of scheduled content. You can't see it all! As much as possible will be videoed. Have they announced the URL for the videos? Saturday 11:00 - 2:20 - Mail Track = [...] Cyrus process: - Gerrit? Some sort of code review process to make it easier to keep track of the work from drive-by contributers. I see two benefits to Gerrit or something like it a) casual contributions don't get lost in the noise b) regular contributors get regular code reviews - Bugzilla - use it for everything. If it doesn't have a bug number where discussion took place, it doesn't get accepted. This is a major workflow change, and is probably more of a challenge for me than anyone else. Sounds good. Another thing I'd like to see is a push hook on git.cyrusimap.org's master and 2.4 branches, so that if a git commit has the text Bug in the subject then the commit message is copied to Bugzilla and the bug marked RESOLVED - FIXED. - Websites in git. Yes! - Release process - simplify to save the repeated typing involved. I wind up writing the changelog, the website release note and the email release note, plus manually changing a bunch of things in the website PHP every time I do a release. I wonder if DIstZilla or something like it could be used to automate the release process? New Features - Conversations, XMOVE, etc: - Alexey is willing to help us with the standardisation process if we want to push for conversations to become more standard. Cool! Special-Use: - Long discussion here. Kolab currently store a custom usage annotation, but no other client knows to look for it. There are two axes: - 1) this is MY \Sent folder - 2) this is a \Calendar folder - it contains calendar entries as encoded attachments. If I share it, other users should see that - so we probably have to extend special use for private and shared, and make sure there's a defined priority order if both exist. Also, there may be more than one special use on the same folder - both \Calendar and a personal \DefaultCalendar (or something) Yes, our RFC compliance is basic here. E-Discovery, deletion controls: - Kolab are planning to use the msg bus stuff from Worldline to have a listener that collects data for e-discovery. Kind of a cyrus watcher. I'd love to have a more generic infrastructure for listening to changes in, and interacting with, the cyrus data model, on top of which we could implement the Annotator daemon, mupdate, and NOTIFY. ActiveSync: - MetaWays have a very good open-source ActiveSync stack: http://www.h-online.com/open/news/item/Tine-2-0-supports-ActiveSync-740315.html That seems to be another PHP implementation like Z-Push. Spam Reporting via IMAP: - Alexey mentioned that there's talk of adding a command to IMAP to report a message as spam/non-spam rather than setting flags. This would be used to actually take action based on the report. - Google and Yahoo are both involved in this effort. Sounds interesting, any more information? Community: - there is at least a community of IMAP Server implementors. There isn't really one for IMAP Client implementors. They just do their own thing, often just by looking at protocol traces, certainly not bothering to understand the entire RFC stack. And can you blame them? It's a jungle. http://fosdem.org/2012/schedule/event/thunderbird [...] I grabbed Ludovic for a few minutes afterwards and outlined our plans for a new mail protocol. He will raise it at their team meeting next week, and start a discussion about it on their mailing lists (this is already done, I have joined the tb-planning mailing list) Cool. Mail Protocol - initial notes: (Timo Bron) == - Issues - folders vs tags. - if a tag can be added/removed, need to change the UID to be compatible with IMAP semantics. - 1/1 relationship Tags/Folders - stay compatible? - GUIDs for Folders as well, detect renames. Dovecot and Cyrus both keep a GUID for each folder internally. Yes! - MSGNO/UID/Uidvalidity/etc? What do we need to keep? Ordering properties are nice. MSGNO is a completely useless historical artifact that should be buried in lye and forgotten. We need a stable identifier for each message, preferably one which is truly stable, i.e. doesn't change on reconstruct or replication. UID+UIDVALIDITY is a sad approximation to this. RFC822 Message-Id is a better approximation, except for Drafts and other Message-Id-less messages.
Re: Do cyrus devs plan to implement PUSH-IMAP protocol?
On Tue, Jan 31, 2012, at 09:42 AM, Sébastien Michel wrote: Here's a tip:) it supports IMAP SPECIAL-USE, but *only* in response to the non-extended IMAP LIST command. The workaround is easy to code in Cyrus Cool! Have you coded it yet? I don't see a Bugzilla ticket. Yes [...] Excellent, I've raised https://bugzilla.cyrusimap.org/show_bug.cgi?id=3642 to track it. Bron, I'd appreciate your input on the ticket. -- Greg.
Re: Do cyrus devs plan to implement PUSH-IMAP protocol?
On 30/01/2012, at 19:59, Georg C. F. Greve gr...@kolabsys.com wrote: On Monday 30 January 2012 12.12:52 crocket wrote: Is there any plan to implement PUSH-IMAP in cyrus IMAP? Generally I'd be very interested in everything that improves performance for mobile devices with IMAP, but in this case I notice that the drafts went on a little longer, but then ultimately expired with draft 12, it seems: https://tools.ietf.org/id/draft-maes-lemonade-p-imap-12.txt That draft seems to have expired in 2006 with no RFC as its result, perhaps because in 2005 Nokia claimed to hold patents that apply to it: https://datatracker.ietf.org/ipr/604/ Whether these are among patents that was provided to the patent trolls in the recent deals I do not know and did not check. [1] In any case, as the draft expired almost 6 years ago and never turned into RFC, one wonders what Apple plans to do with this, which expired draft version it plans to support, whether there will be proprietary extensions and so on and so forth. The state of push email on the iPhone seems quite confusing (in particular it seems the Apple documentation is contradictory or missing), but after some reading it seems that the iPhone supports three push email protocols: - for Yahoo accounts, Yahoo's non-standard push protocol based on UDP - for MobileMe accounts, Apple's non-standard push protocol based on XMPP - for the Microsoft Exchange account (just the one per iPhone apparently), Microsoft's ActiveSync protocol. Google also uses this. This information is more or less rumour. Perhaps somebody from Apple could clarify what the actual situation is? But, on current information it seems the only way for a service which isn't Yahoo or Apple to achieve actual email push to a iPhone is to talk ActiveSync. This is presumably why the Google folks added ActiveSync support. The original poster mentioned the Z-Sync project, which a PHP implementation of the ActiveSync serverside which can talk IMAP to a backend. Presumably that involves the Z-Sync server polling IMAP, which would be better than the client polling but still less than wonderful. Given that the CMU guys have been adding http and XML support to Cyrus for CalDAV, some of the pieces that ActiveSync needs are coming anyway, so it's not entirely impossible to imagine some future Cyrus supporting ActiveSync directly. Georg, I notice there's a page about Z-Sync on the Kolab wiki. Do you have any idea of the legal issues with the protocol? Personally I'd be strongly in favour of such technologies. Me too; I work with an iOS device and a Cyrus server daily. Lots of our customers use iOS devices. Greg.
Re: Do cyrus devs plan to implement PUSH-IMAP protocol?
On Mon, Jan 30, 2012, at 10:40 PM, Sébastien Michel wrote: Georg, I notice there's a page about Z-Sync on the Kolab wiki. Do you have any idea of the legal issues with the protocol? Hard to find an answer, but below some useful links : Thanks Sébastien! http://www.microsoft.com/about/legal/en/us/IntellectualProperty/IPLicensing/Programs/ExchangeActiveSyncProtocol.aspx That page is less than entirely useful. In summary: ActiveSync exists, we own patents on it, talk to us. http://notes.kateva.org/2009/02/googles-activesync-license-interesting.html http://z-push.sourceforge.net/phpbb/viewtopic.php?f=4t=113 These pages are both more or less entirely rumour and speculation. However, one contained a link to the Microsoft site which eventually proved interesting. At this time I should point out that I'm not a lawyer. It seems Microsoft have several different licence programs under which they licence patent-encumbered protocols and file formats: Microsoft Interoperability Program, Microsoft Communications Protocol Program, Workgroup Server Protocol Program, and Interoperability Principles. Some specifications are covered by multiple of these. The different programs have different reasons for their existence. Some exist as a result of an antitrust decision against Microsoft, i.e. the company was compelled by a court to open the protocols. These smell pretty safe for us to use. The protocols that Samba uses are covered by such a licence program, and I've heard Tridge say very complimentary things to Microsoft about their co-operation (once compelled by the court). The ActiveSync protocols however appear to be covered only by the weakest of the licencing programs, which is described here http://www.microsoft.com/openspecifications/en/us/programs/other/interoperability-principles-patent-pledges/default.aspx On a careful but not lawyerly reading I note that a) It's just a promise - there's no legal ruling enforcing it and nothing stopping Microsoft from going back on it at any time. It's not even a licence. b) It's a promise to the developer of open source software, either companies or individuals, and not to any users of that software. c) There is some worrying wiggle room around what open source licence means. I'd guess that any licence listed at the Open Source Initiative is safe, as Microsoft have submitted some of their licences to the OSI, The Cyrus licence isn't there, but it's very very similar to one of the BSD licences that is there. Z-Push is GPL and should be fine. Personally I'd be strongly in favour of such technologies. Me too; I work with an iOS device and a Cyrus server daily. Lots of our customers use iOS devices. pity it is a poorly written IMAP client. It doesn't support well IMAP standards. Possibly, but it is ubiquitous. Here's a tip:) it supports IMAP SPECIAL-USE, but *only* in response to the non-extended IMAP LIST command. The workaround is easy to code in Cyrus Cool! Have you coded it yet? I don't see a Bugzilla ticket. -- Greg.
Re: Build failed in Jenkins: cyrus-imapd-master #360
On Mon, Jan 30, 2012, at 05:05 PM, Jenkins wrote: See http://ci.cyrusimap.org/job/cyrus-imapd-master/360/changes Changes: [brong] strarray: add _uniq and pass a strcmp to _sort. -- This was a spurious test failure caused by unsolved permission problems with the .gcda files used by gcov profiling. Fixed (well, worked around) in http://git.cyrusimap.org/cassandane/commit/?id=bc26deb6ebc37bc3074819fd027efc8d5f2a6b99 -- Greg.
Re: Jenkins emails
On 26/01/2012, at 4:57, Dave McMurtrie dav...@andrew.cmu.edu wrote: I added do-not-re...@cyrusimap.org to accept_these_nonmembers for the cyrus-devel list. This should work. If not, please let me know. Thanks, will go test now. Greg.
Re: Jenkins emails
On Thu, Jan 26, 2012, at 08:56 AM, Greg Banks wrote: On 26/01/2012, at 4:57, Dave McMurtrie dav...@andrew.cmu.edu wrote: I added do-not-re...@cyrusimap.org to accept_these_nonmembers for the cyrus-devel list. This should work. If not, please let me know. Thanks, will go test now. (sigh) I manually started a build and forgot that Jenkins is configured not to send emails if the build succeeds and has no changes. So I used the send a test email button like I should have in the first place, and it works fine. Now I have to solve the git polling bug in the Multiple SCMs plugin, or maybe just configure a regular build. Then we'll all start seeing Jenkins emails regularly. Apologies to Bron and Olivier who've been receiving lots of spam from Jenkins as I trigger builds every few minutes :) Thanks, Dave! -- Greg.
Re: Cyrus reviews
On Tue, Jan 24, 2012, at 08:16 AM, Ken Murchison wrote: Greg Banks wrote: This code + newdest = buf_release(buf); will leak the string, as newdest is never free()d (and indeed in some other branches of the logic, cannot be). I thought the same thing when I looked as what was already in master when I began my forward port: [...] Was this also incorrect? No, it was correct (if confusing) and is still correct. The memory pointed to by 'newdest' is taken over by either spool_cache_header() or spool_replace_header(), not copied, which weirdness I had forgotten. The fact that 'newdest' sometimes points to memory already owned by the function and sometimes not is fine as all we do with it is fprintf(). Sorry, false alarm :( -- Greg.
Re: Cyrus reviews
On Wed, Jan 25, 2012, at 11:16 AM, Greg Banks wrote: On Tue, Jan 24, 2012, at 08:16 AM, Ken Murchison wrote: While we're here... commit nntpd: better commenting of add_header() Looks good. commit imapoptions: more complete description of newsaddheaders behavior Looks good. -- Greg.
Re: Cyrus review
G'day,
Bron said we should have this discussion in public, which is a good idea, so...
On Mon, Jan 23, 2012, at 08:22 AM, Bron Gondwana wrote:
On Mon, Jan 23, 2012 at 01:52:54PM +1100, Greg Banks wrote:
commit dlist: unlink before writing temporary file
commit always mkdir actually
I don't get this - are you trying to protect against a race with some other
code? Or does the filename get accidentally reused somehow?
Heh... so consider a buggy sync_client which uploads the same file
multiple times. The filename gets reused, because it's a direct
product of pid and sha1. That's pretty bad.
What is worse: RESERVE
user/foo/5. has sha1 X
RESERVE (user.foo) (X)
$pid/X is now a hard link to user/foo/5.
along comes
user.foo.sub UID 2 SHA1 X - but the sync_client chooses to upload.
It actually overwrites user/foo/5. with the new content. Now, this
is always the same file - so nothing is lost at the end. But if
ANOTHER process also hardlinks user/foo/5. into its own space and
starts reading it, it can get a short read.
Ah, right. Ouch.
Doing an open(O_RDWR|O_CREAT|O_EXCL) followed by an fdopen() might be a better
solution.
The O_EXCL should close the race.
My suspicion is that the Linux kernel changed some behaviour which
exposed this mistake, where previously the clients were always seeing
the old cached version, because we parse the file just before linking
the first time.
It could just be a CPU scheduler change.
commit twoskip: don't crash during checkpoint on full disk
+ int r2 = mycheckpoint(db);
+ if (r2) {
+ syslog(LOG_NOTICE, twoskip: failed to checkpoint %s: %m,
+ _fname(db));
+ }
Hmm, the error message uses %m when an error is returned from
mycheckpoint(),
but errno is not reliable in mycheckpoint() as there is all sorts of
potentially errno-trashing
system call activity in between the proximate error and the return.
Otherwise, looks good.
So should probably be error_message(r2) - except we don't have
error_message in here.
Yeah, because of those wacky CYRUSDB_* errors.
Damn I hate the error handling dual-framework.
We should just #define CYRUSDB_* to some new codes in imap/imap_err.et
commit add Xmove command
I like the idea! This one is actually useful for loads of people. But,
it's existence should
be announced in CAPABILITY.
It does:
{ XLIST, 2 },
+{ XMOVE, 2 }, /* not standard */
{ SPECIAL-USE, 2 },
At least in the copy in my repository.
Woops, missed that :(
r = append_setup(appendstate, name, state-userid,
-state-authstate, ACL_INSERT, qdiffs,
+state-authstate, ACL_INSERT,
+ismove ? NULL : qdiffs,
namespace, isadmin);
This is only correct if both the source and destination mailboxes have the
same
quotaroot, which isn't always the case. Contrary cases include a) moving
to another
user's mailbox, b) moving to a mailbox in the shared space, c) moving
between the
same user's mailboxes across the boundary of a nested quotaroot.
True - we need to test for that case. Damn. It's not easy with all the
layers either :(
Yeah, hairy in the extreme.
commit mailbox: fixup delete
I'm deeply confused about this.
That probably means it's not well described in the code. I'll see if I
can
clear it up a bit.
The core bug was: mailbox_lock_index() now returns a doesn't exist
response
if the mailbox has an OPT_DELETED flag set in the index header. Which is
correct in general. It means any client re-locking the mailbox after it
has been deleted will get a this mailbox is deleted, go away rather
than
having to check the mailbox itself.
The problem is - this codepath is during unlock, and it's to see if we
need
to do various cleanups under a mboxname exclusive lock. In this case
we're
basically setting the index_locktype field because the various assert
checks elsewhere in the code aren't smart enough to know that we have an
exclusive namelock, which gives the same protections.
Gah. That is definitely unobvious.
commit Bug #3381 - make rehash tool 64-bit safe
Hmm. There's some really ugly Perl in there.
I know. I didn't like it much, but Leena has a point - it was broken
as it was. The whole of tools/rehash is awful. I did rewrite it once,
a few years ago, to support a new format which we never went ahead with.
It was a nice format which allowed a much better fast rename, but it
was yet another format.
These days I'd be tempted to do directory names based on UNIQUEID,
Now that they're actually unique, sure.
and
never bloody rename files for folder renames. But it's a big change.
We could provide symlinks...
But really, I'm not sure it's worth optimising folder delete and rename, they're
relatively infrequent operations
Cyrus reviews
G'day, I've been told I should do reviews more openly. Ok, here goes. commit rename: ensure user owns both source and dest for Bug #3586 workaround Ok, but why? commit nntpd: use defaultdomain in conjunction with newspostuser to create Reply-To header addresses Looks good commit nntpd: when adding newsgroup post addresses to Reply-To, check for poster commit nntpd: fix handling of Followup-To:poster when using From: header I find the add_header() function really confusing, and hard to work out what it's supposed to do. Is there any chance of a comment and/or some tests? In add_header(), the variable sep could be a const char *. This code + newdest = buf_release(buf); will leak the string, as newdest is never free()d (and indeed in some other branches of the logic, cannot be). A better solution would be const char *newdest = NULL; ... newdest = buf_cstring(buf); ... buf_free(buf); } Otherwise, looks good. I'm a little surprised we didn't explicitly handle Followup-To: poster, it's been around since RFC1036. commit nntpd: added 'newsaddheaders' option... The documentation doesn't describe what happens if the incoming message already contains the To: or Reply-To: header fields. From reading the code, I think they're passed through untouched, perhaps it would be nice to document the intended semantics. Otherwise, looks good. -- Greg.
Re: Cyrus reviews
On Tue, Jan 24, 2012, at 07:25 AM, Bron Gondwana wrote: On Tue, Jan 24, 2012 at 01:49:52PM +1100, Greg Banks wrote: I've been told I should do reviews more openly. Ok, here goes. commit rename: ensure user owns both source and dest for Bug #3586 workaround Ok, but why? CMU had somebody issue rename $sharedroot INBOX.Trash. Since they had no permissions on $sharedroot, the lower level returns IMAP_MAILBOX_NONEXISTENT. Since submailboxes are done as admin, there were no ACL checks. It was only the quota which stopped their entire shared heirarchy being renamed under INBOX.Trash of one user. Gah! Still, checking for the same user is a rather ugly hack when what we actually want is to do an ACL check. -- Greg.
Jenkins
G'day, I spent the last week at Linux.conf.au in Ballarat, and one thing I noticed is that most open source projects which run a Jenkins instance are running it at ci.theirproject.org. So is there are chance we can get hudson.cyrusimap.org renamed or aliased to ci.cyrusimap.org ? -- Greg.
Re: Cassandane for 2.4
On Wed, Jan 18, 2012, at 09:57 PM, Bron Gondwana wrote: So I'm working on patches for the next 2.4 as well, and I find that Cassandane isn't so happy with 2.4! Jan 18 21:55:59 launde 2055591/master[26844]: unable to initialize groups for user cyrus: Operation not permitted Jan 18 21:55:59 launde 2055591/master[26844]: can't change to the cyrus user: Operation not permitted Jan 18 21:55:59 launde 2055591/master[26842]: process 26844 exited, status 1 Jan 18 21:55:59 launde 2055591/master[26842]: unable to initialize groups for user cyrus: Operation not permitted Jan 18 21:55:59 launde 2055591/master[26842]: can't change to the cyrus user: Operation not permitted It would be great to keep a branch that can test cyrus-imapd-2.4 as well, while we're still doing stable releases :) I agree, and it's my intention to set up such a branch once I've massaged the hudson.cyrusimap.org environment to be able to run on master. However the problem in this case is probably not Cassandane but Cyrus, in particular the 2.4 branch is missing commit a5caf503c7060b4f1ec546e4dc6fe75e5b9c4029 gnb hack: allow running as the cyrus user See section 1 in cassandane/doc/setting_up.txt -- Greg.
Re: Cassandane Troubles
On Sun, Jan 8, 2012, at 01:00 PM, Jeroen van Meeuwen (Kolab Systems) wrote:
Hello there,
I'm trying to run Cassandane on my laptop, because I'm interested in
writing tests and automating the execution thereof.
Cool, always good to have more feedback.
Having followed the instructions in doc/setting_up.txt -liberally, I
must admit-, I notice that;
- While Cyrus IMAP is installed with a proverbial './configure; make;
make install DESTDIR=/var/tmp/cyrus-imapd-2.4/', and the binaries
therefore end up in /var/tmp/cyrus-imapd-2.4/usr/cyrus/bin/, the
configuration that Cassandane writes out refers to binaries in
/var/tmp/cyrus-imapd-2.4/bin/, a directory that does not exist.
It seems that this feature is currently broken, sorry :(
However if you follow section 4 in that document precisely, it should work.
$ cat /var/tmp/cass/cassandane/conf/cyrus.conf
START {
# integrity check and setup of databases
recover cmd=ctl_cyrusdb -C /var/tmp/cass/cassandane/conf/imapd.conf
-r
This is a bug, it should have been a full path to ctl_cyrusdb. Today I
pushed some changes from a development branch which should fix that.
- When /var/tmp/cyrus-imapd-2.4/bin/ is created a symbolic link for, to
/var/tmp/cyrus-imapd-2.4/usr/cyrus/bin/, Cyrus IMAP / Cassandane
ultimately fails authenticating. I recon this is a Cyrus SASL thing, but
I was wondering whether Cassandane requires the system to have a valid
SASL configuration with an 'admin' user, whether any additional users
would be required, and whether it could be made so that no such
system-wide configuration is required (by starting an SASL auth daemon
with a different user database then the system database?).
Cassandane relies on the no-password hack in libsasl which is enabled using
sasl_pwcheck_method: alwaystrue
which might not be available on your build of libsasl?
--
Greg.
Re: Fix for cyrus bug 3269
G'day Philip, On 22/11/11 11:14, Philip Prindeville wrote: Hi Greg, Can you please have a look at my fix for bug 3269? https://bugzilla.cyrusimap.org/show_bug.cgi?id=3269#c2 Thanks, -Philip The bug says: Cyrus logs a lot of crap (which is not useful 99% of the time, but must be there for the remaining 1%), I agree it's not useful 99% of the time, but I'm not convinced it's useful the remaining 1% :) So yes, something needs to be done to reduce pointless load on the syslog daemon, and this patch certainly looks like something. The patch itself looks fine, although I would feel the same way about a patch that just removed almost every single syslog(LOG_DEBUG) call. Pushed to CMU master branch, thanks for your contribution :) -- Greg.
Re: prefork
G'day,
On 30/09/11 19:38, Zbierski Christophe wrote:
Hi, I continued to analyze the problem :
In master.c (main() function)
masterconf_getsection(START, add_start, NULL);
masterconf_getsection(SERVICES, add_service, NULL);
masterconf_getsection(EVENTS, add_event, NULL);
/* set signal handlers */
sighandler_setup();
/* initialize services */
for (i = 0; i nservices; i++) {
service_create(Services[i]);
if (verbose 2)
syslog(LOG_DEBUG, init: service %s socket %d pipe %d %d,
Services[i].name, Services[i].socket,
Services[i].stat[0], Services[i].stat[1]);
}
There is 2 actions, one to read the configuration (add_service()), and
another to create services (service_create()).
nservices contains the number of services, it's updated in add_service(), but
service_create() function also updates this variable :
if (s == service) {
if (nservices == allocservices) {
if (allocservices SERVICE_MAX - 5)
fatal(out of service structures, please restart,
EX_UNAVAILABLE);
Services = xrealloc(Services,
(allocservices+=5) * sizeof(struct
service));
if (!Services) fatal(out of memory, EX_UNAVAILABLE);
}
memcpy(Services[nservices++], s, sizeof(struct service));
}
I do not understand this part of code.
Christophe, did you get further in your analysis? If not, can you
please create a Bugzilla ticket? I may have some time soon to poke
around in the master process.
--
Greg.
Cassandane fixes
G'day, Yesterday on IRC (18:47:12) suiryc: gnb1: are you here ? (18:51:28) suiryc: I've got commits that may interest you in the cassandane branch 'extra/misc' on our github repo git://github.com/worldline-messaging/cassandane.git (18:53:14) suiryc: there are three commits in that branch, two being features that we use to test some of our cyrus-imapd things, one that is a minor bugfix I've pushed those commits (plus a couple of tweaks of my own) to cmu master. Thanks! -- Greg.
Re: MESSAGE quota resource implemention
On 17/09/11 01:37, Julien Coloos wrote: As discussed here and on IRC, I rebased my commits with all the changes: - the 'utility' methods have been promoted to 'command' ones, which are now generic and may (options hash) concern cyrus binaries - the 'start_command_bg' method is now replaced by a 'background' option in 'run_command' - dropped the 'mode' option, leaving only the 'redirects' one - moved 'unpack' method from Cassandane::Unit::TestCase to Cassandane::Instance, leaving the tar utility determine itself the kind of the tar file - by default destination is the current cassandane instance base directory; alternatively one can specify a relative path - 'path/to/file' - to this directory, or an absolute path - '/path/to/file' - use the pre-existing 'user.cassandane' mailbox for quota upgrade tests - fixed a few minor things (like avoiding short-life zombies by harvesting some cassandane dead children) - refactored a few more stuff - removed stuff that became unnecessary Hoping this will work out this time :) Excellent work, thanks a lot. I particularly like the refactoring in Quota.pm that allows the tests to express checks of reported quota numbers in a way that's concise but not sensitive to the order reported by the server. I've pulled your changes and pushed them out to github and cmu. -- Greg.
