Fix libcyrus_imap not being linked to dependency libcyrus_sieve

[J5lx]

This PR fixes another issue I noticed while building the 3.10 RC, although this one could also be older. I found that libcyrus_imap depends on libcyrus_sieve, but doesn’t actually link to it. (This messes with -Wl,--as-needed, which is how I noticed it.) It kind of works currently because with the way the build system is set up, all of libcyrus_imap’s dependents are necessarily also linked to libcyrus_sieve directly. However, I do believe that the right way is to link libcyrus_sieve into libcyrus_imap itself rather than its dependents. To achieve that, I also had to move libcyrus_sieve bits a little further up in the Makefile because, at least at install-time, libtool is sensitive to ordering.

[elliefm]

What are you using that's causing problems? -Wl,--as-needed doesn't cause me any problems for libcyrus_sieve. Specifically, I do all my local builds with:

    CFLAGS="-g -O0 -Wall -Wextra -Wformat-security -Werror -Wl,--as-needed -fstack-protector-strong -fstack-clash-protection $CFLAGS" \
    CXXFLAGS="-g -O0 -Wall -Wextra -Wformat-security -Werror -fstack-protector-strong -fstack-clash-protection $CXXFLAGS" \

among the configure arguments. It's interesting that I set that flag in CFLAGS but not in CXXFLAGS—I wonder if that's the difference?

I believe libcyrus_sieve is maybe supposed to be able to stand on its own (whether it does or not is a separate question), and perhaps that's why even though libcyrus_imap depends on it, it doesn't embed or link to it. Otherwise other programs that link both libcyrus_imap and libcyrus_sieve might get two copies of the latter. @ksmurchison any thoughts?

This stuff is difficult to casually reason about, so I'm pretty cautious about making changes here. I'd be more confident if I could reproduce your problem locally in order to prove the fix.

[J5lx]

Ah sorry, I should have explained myself better. The issue that I’m trying to address is that libcyrus_imap is not linked to libcyrus_sieve but still uses functions from it. As I said this does not currently seem to cause any issues for Cyrus itself because both libraries are simply added to LD_UTILITY_ADD, along with several others. LD_UTILITY_ADD is applied to all the utilities and figuring out which libraries are actually needed is left to -Wl,--as-needed. Now, libcyrus_sieve isn’t actually used in all of them, but because it is needed for libcyrus_imap, -Wl,--as-needed will still leave it in whereas ldd -u will report it as unused (which to me seems kind of like a case of overlinking, after a fashion). Conversely, this also means that trying to link a binary that does not itself use libcyrus_sieve only to libcyrus_imap (i.e. specifying libcyrus_imap on the linker command line but not libcyrus_sieve, which the Cyrus build system currently does not) will result in errors due to underlinking:

/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_script_load'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `bc_header_parse'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_generate_bytecode'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_emit_bytecode'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_script_free'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_script_parse_string'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_free_bytecode'
/usr/bin/ld: imap/.libs/libcyrus_imap.so: undefined reference to `sieve_script_unload'

As I said (or tried to say, anyway), to me linking libcyrus_imap to libcyrus_sieve seems like the “correct” thing to do because of this, even if it has not caused any serious issues so far. Though admittedly, I’m also not entirely sure I fully understand your remark about libcyrus_sieve standing on its own.

Anyway, I understand why you’re cautious about this sort of change, and I hope I was able to convey my motivation at least a little better than in the OP.

[elliefm]

Thanks for clarifying, I'll think about it a bit more. There's conceptual overlap here with #4798, which I haven't thought about for several months, and what I thought I knew back then is no longer fresh in mind. Looks like I didn't get as far as looking at libcyrus_imap yet, but it's likely that if I had I would have reached the same conclusion you have here.

I probably wouldn't backport this for 3.10 even if we do accept it for master, because it isn't fixing anything that wasn't already broken for a long time, and I'd like to get 3.10.0 actually released instead of stuck in endless release candidates and tinkering.