On Tue, 22 Jun 2021 13:29:36 +0300 Tzvetomir Stoyanov <tz.stoyanov@xxxxxxxxx> wrote: > On Tue, Jun 22, 2021 at 4:26 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > > > On Mon, 14 Jun 2021 10:50:01 +0300 > > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote: > > > > > If libz is available, use that library to provide trace file compression > > > support. The library is detected runtime. > > > > Why have the library detected at runtime? > > > > If it is detected, we can then have the library flags include -lz. > > > > Why use dlopen to load zlib? And not just include it? > > > > This seems rather fragile to try to get right. > > The idea of this design is not to bring additional mandatory > dependencies. I do not know if libz is available by default, but even > if we assume it is - each additional compression library that is added > will be a mandatory dependency. The compression is not a mandatory > functionality, trace-cmd can work without it. Why do you think it is > fragile, I think dlopen() uses the same core linker logic to find and > load the library. The only difference I see is that using "-lz" leads > to a mandatory dependency, trace-cmd will not run without it. > The dependency is determined at compile time, just like we do for the python libraries, and dwarf, and anything else that might add a new feature. Let me explain a scenario of why I called it fragile. Let's say that trace-cmd has no package dependency on zlib, but the system has some other package that does have a dependency. Thus the package manager pulls in zlib to satisfy this other package. The user does a recording, and trace-cmd detects zlib, and compresses the data. Later, the user decides they do not need this other package and uninstalls it. The package manager sees there's nothing that depends on zlib anymore, and uninstalls zlib as well. Then the user goes to read their trace.dat file, and suddenly trace-cmd can't read it! THAT is what I call fragile. And if something like that ever happened to me, I would stop using whatever did that to me. Manually pulling in system dynamic libraries with dlopen is something I never heard of. The way to do this is make it a build time dependency. If zlib exists, then define HAVE_ZLIB and allow compressions and everything else. If it does not, then we don't support compression. Simple as that. Making it a runtime dependency has a lot of issues, *especially* since one run of trace-cmd (the reporting) depends on a previous run of trace-cmd (the recording), and if the environment changes between the two, the user will rightfully say WTF! -- Steve
|