1634963 - [RNP] Allow for using system libbz2, json-c, botan libraries when building

Status: RESOLVED FIXED

(Thunderbird :: Build Config, enhancement, P2)

Description

[Rob Lemley [:rjl]]

--with-system-zlib is honored when building so that Linux distros can compile librnp against the system libraries. Ideally the same would be supported for bz2 and json-c. These libraries are quite stable.

For now Botan and RNP will only be supported with the in-tree copies. RNP is new enough that it's not really in many distributions. Botan may be, but it's unknown whether those builds will work with RNP.

Comment 1

[Landry Breuil (:gaston)]

botan has regular releases, on OpenBSD we have 1.10.17 and 2.10 available in packages, upstream is at 2.14, and according to https://hg.mozilla.org/releases/comm-beta/file/tip/third_party/README.botan the botan copy is 2.13. I will have no issue updating our systemwide botan to 2.14 (and other downstreams too probably) so it would be nice to have --with-system-botan available with a version check. After all, it's done for nss/nspr and many other external dependencies.

i get it that for rnp patches are being actively pushed upstream so it makes sense to only allow the bundled version for now.

from my understanding of https://hg.mozilla.org/releases/comm-beta/file/tip/third_party/moz.build the 5 subdirs are used, so im not sure --with-system-zlib is honored for librnp - will have to test. i'm pretty sure there used to be a --with-system-bz2 option too but i cant find it anymore in the m-c tree..

a bit unrelated, but i see that in bug #1518166 libotr, libgcrypt and libgpg-error were imported - im not 100% sure those are built/distributed (from my own testing, if libotr is installed systemwide, i see that it's dlopened at runtime, otherwise there's an error msg in the debug console) but if so, are there plans to allow similar --with-system-{otr,gcrypt,gpg-error} switches? from my understanding of https://bugzilla.mozilla.org/show_bug.cgi?id=1518164#c16 and following comments the 'unix distributor' problem was discussed, and those bundled libs arent built on unix.

Comment 2

[Rob Lemley [:rjl]]

Thanks for the input. I don't work with non-Linux UNIXes much so the perspective is very helpful.

There was a --with-system-bz2 in M-C at some point, but it has been removed.

If you look at the moz.build file in the zlib directory, --with-system-zlib is checked there and if set OS_LIBS gets set accordingly to link to the system version. I have my local builds set up this way currently, so it should be working.

I initially left Botan off the list based on quickly looking at a few Linux distributions and the version they currently have. The other consideration with Botan is that RNP requires specific modules to enabled.

libotr was added mostly for our Windows builds done in Taskcluster. We do include it in the macOS and Linux builds as well, but it's poorly integrated. I would like to improve that at some point.

Comment 3

[Landry Breuil (:gaston)]

(In reply to Rob Lemley [:rjl] from comment #2)

Thanks for the input. I don't work with non-Linux UNIXes much so the perspective is very helpful.

Always happy to provide more input from downstreams, especially exotic ones :)

There was a --with-system-bz2 in M-C at some point, but it has been removed.

If you look at the moz.build file in the zlib directory, --with-system-zlib is checked there and if set OS_LIBS gets set accordingly to link to the system version. I have my local builds set up this way currently, so it should be working.

i dont have an objdir around, but i know that biulding 77.0b3 using --with-system-zlib, i had a third-party/zlib dir in my objdir, with a Makefile and a backend.mk, but no actual obj files, so i suppose that yes it works. I'll recheck at my next build :)

I initially left Botan off the list based on quickly looking at a few Linux distributions and the version they currently have. The other consideration with Botan is that RNP requires specific modules to enabled.

If that turns into the situation we had w/ building using system-sqlite that required SECURE_DELETE (which would have impacted all other sqlite consumers), then yes it makes sense to not allow building with system botan.

I suppose the list of modules required is https://searchfox.org/comm-central/source/third_party/botan/botan_configure.py#18 - i know that our systemwide botan2 doesnt pass --enable-modules so the default upstream list is probably used. What are the downsides/upsides of requiring those modules for the user ? will there be runtime breakage if some modules arent found in the systemwide botan ?

libotr was added mostly for our Windows builds done in Taskcluster. We do include it in the macOS and Linux builds as well, but it's poorly integrated. I would like to improve that at some point.

I saw that libotr isnt built on OpenBSD, nor its dependencies - if that has to change at some point, then it would be nice to be also able to build with system/external versions :)

Comment 4

[Rob Lemley [:rjl]]

(In reply to Landry Breuil (:gaston) from comment #3)

I suppose the list of modules required is https://searchfox.org/comm-central/source/third_party/botan/botan_configure.py#18 - i know that our systemwide botan2 doesnt pass --enable-modules so the default upstream list is probably used. What are the downsides/upsides of requiring those modules for the user ? will there be runtime breakage if some modules arent found in the systemwide botan ?

That list in botan_configure.py is based on what :kaie had given me from his development work. It's a minimal list based on what RNP requires. I expect that the default for Botan will include everything that's necessary. I just need to confirm that.

I saw that libotr isnt built on OpenBSD, nor its dependencies - if that has to change at some point, then it would be nice to be also able to build with system/external versions :)

You can do that already. There's no build-time requirement for libotr since it's loaded with js-ctypes. If you add it as a package dependency it should be enough. The loading code at https://searchfox.org/comm-central/rev/1ae9ab95d0267bcefbe2d3598bfdb50ca934a11d/chat/modules/OTRLib.jsm#20-87 looks in /usr/lib and a few other places for files like "libotr.so.5" and such. If the system library cannot be found, that's the place to make changes.

As I said, the code was added to the tree so mostly we could include libotr in our Windows builds. Open source OS's tend to have system packages already which mostly have just worked.

Comment 5

[Rob Lemley [:rjl]]

Attachment: Bug 1634963 - Configure flag for building librnp with system JSON-C. r?darktrojan

New mach configure flag --with-system-jsonc is available if --enable-openpgp
is set (the default).

Comment 6

[Rob Lemley [:rjl]]

Attachment: Bug 1634963 - Configure flag for building librnp with system Bzip2. r?darktrojan

New mach configure flag --with-system-bz2 is available if --enable-openpgp
is set (the default).
Firefox had this flag previously, but it has been removed.

Depends on D84612

Comment 7

[Rob Lemley [:rjl]]

With the patches from comment 5 and comment 6, compiling against a system zlib, bzip2, and json-c is possible. I have so far only tested on my Arch Linux system so feedback from others will be appreciated.

./mach configure --enable-application=comm/mail --enable-openpgp --with-system-jsonc --with-system-zlib --with-system-bz2

json-c should have a pkgconfig file installed in /usr/lib/pkgconfig. I did not implement a way to specify a location for this one.

bzip2 does not include a pkgconfig file in its source distribution so a path is accepted (not tested) --with-system-bz2=/usr/local (where headers are in /usr/local/include and libs in /usr/local/lib). Some distros like Fedora add their own pkgconfig file for bzip2, so if a path is not provided that's checked before falling back to relying on the builder to have configured their system right or setting CFLAGS and LDFLAGS would probably work as well. If a path is provided and pkgconfig file is present, the given path should be preferred.

I implemented json-c before doing bz2, so if all that heuristic and path checking works for bz2 i could make json-c work the same way.

With the above mach configure command, I got a successful build and librnp.so is linked to the system libraries:

➜ ldd librnp.so
	linux-vdso.so.1 (0x00007fff297ef000)
	libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f2eafcc1000)
	libdl.so.2 => /usr/lib/libdl.so.2 (0x00007f2eafcbb000)
	libbz2.so.1.0 => /usr/lib/libbz2.so.1.0 (0x00007f2eafca8000)
	libjson-c.so.5 => /usr/lib/libjson-c.so.5 (0x00007f2eafc96000)
	libz.so.1 => /usr/lib/libz.so.1 (0x00007f2eafc7c000)
	libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x00007f2eafa9f000)
	libm.so.6 => /usr/lib/libm.so.6 (0x00007f2eaf958000)
	libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f2eaf93e000)
	libc.so.6 => /usr/lib/libc.so.6 (0x00007f2eaf777000)
	/usr/lib64/ld-linux-x86-64.so.2 (0x00007f2eaff9f000)

Kai and Alex, if you have time would you mind trying these patches out on your systems?

Comment 8

[Alessandro Castellani [:aleca]]

Setting NI for myself so I remember to check this next week.

Comment 9

[Rob Lemley [:rjl]]

Attachment: Bug 1634963 - Do not add vendored zlib to include path when building with system zlib. r?darktrojan

Depends on D84613

Comment 10

[Rob Lemley [:rjl]]

Attachment: Bug 1634963 - Configure flag for building librnp with system Botan. r?darktrojan

New mach configure flag --with-system-botan is available if --enable-openpgp
is set (the default).

Depends on D85226

Comment 11

[Rob Lemley [:rjl]]

Attachment: Bug 1634963 - Update IQuote mozbuild function so it can be called multiple times. r?darktrojan

The include path for building librnp needs to be built incrementally depending
on the flags passed to configure. The COMPILE_FLAGS dictionary cannot be updated in a
template function so just work with CXXFLAGS directly.

Comment 12

[Rob Lemley [:rjl]]

With these patches, the following should work:
--with-system-jsonc
--with-system-botan
Both jsonc and botan use pkgconfig to determine compile and linker flags.

Bzip2 also works, --with-system-bz2. Bzip2 doesn't come with a pkgconfig file though. Some distributions like Fedora include one, while others like Debian and Arch do not. So for bzip2, a path can be passed like --with-system-bz2=/usr/local. CFLAGS will be set "-I/usr/local/include" and LDFLAGS to "-L/usr/local/lib -lbz2". If a path is given and a pkgconfig file is present, the provided path should take priority. And there is a fallback that assumes the headers and libraries are in the compiler's search path already and things just work.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/dd5379237d8f
Update IQuote mozbuild function so it can be called multiple times. r=darktrojan
https://hg.mozilla.org/comm-central/rev/a38edfe637a6
Configure flag for building librnp with system JSON-C. r=darktrojan
https://hg.mozilla.org/comm-central/rev/72a232021e2d
Configure flag for building librnp with system Bzip2. r=darktrojan,kaie
https://hg.mozilla.org/comm-central/rev/4af8b744316e
Do not add vendored zlib to include path when building with system zlib. r=darktrojan
https://hg.mozilla.org/comm-central/rev/d573a15d33c3
Configure flag for building librnp with system Botan. r=darktrojan

Comment 14

[Rob Lemley [:rjl]]

Comment on attachment 9165551 [details]
Bug 1634963 - Configure flag for building librnp with system JSON-C. r?darktrojan

[Approval Request Comment]
Request for all patches:
https://hg.mozilla.org/comm-central/rev/dd5379237d8f
https://hg.mozilla.org/comm-central/rev/a38edfe637a6
https://hg.mozilla.org/comm-central/rev/72a232021e2d
https://hg.mozilla.org/comm-central/rev/4af8b744316e
https://hg.mozilla.org/comm-central/rev/d573a15d33c3

This is desirable functionality for Linux distributions so they can link librnp.so against system libraries.

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9165551 [details]
Bug 1634963 - Configure flag for building librnp with system JSON-C. r?darktrojan

Approved for beta

Comment 16

[Rob Lemley [:rjl]]

Thunderbird 80.0b1 (build4):
https://hg.mozilla.org/releases/comm-beta/rev/706236f247d6
https://hg.mozilla.org/releases/comm-beta/rev/f07d4133a708
https://hg.mozilla.org/releases/comm-beta/rev/ea9f6e8258f9
https://hg.mozilla.org/releases/comm-beta/rev/ebef4a5e6599
https://hg.mozilla.org/releases/comm-beta/rev/b6b955975e8d

Comment 17

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1634963 follow-up - Restore #ifdef COMPILE_ENVIRONMENT to fix artifact builds. r?rjl

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8cb44fa1b2be
follow-up - Restore #ifdef COMPILE_ENVIRONMENT to fix artifact builds. r=rjl DONTBUILD

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 9165551 [details]
Bug 1634963 - Configure flag for building librnp with system JSON-C. r?darktrojan

[Triage Comment]
Approved for esr78

Comment 20

[Rob Lemley [:rjl]]

Thunderbird 78.1.1:
https://hg.mozilla.org/releases/comm-esr78/rev/c88f3d87ab6b
https://hg.mozilla.org/releases/comm-esr78/rev/132fe17802d1
https://hg.mozilla.org/releases/comm-esr78/rev/c1f4bdade5b5
https://hg.mozilla.org/releases/comm-esr78/rev/d177856d1be6
https://hg.mozilla.org/releases/comm-esr78/rev/de2e7e7f8242
https://hg.mozilla.org/releases/comm-esr78/rev/545e5f416b6f

Comment 21

[Rob Lemley [:rjl]]

Thunderbird 80.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/12e9a7dd397b