<div><div>Hi everyone,</div><div><br></div><div>I am on the verge on committing the addition of a new check in checkpkg which is likely to fire some new warnings in a lot of builds, so I am posting the and requesting some comments before proceeding.</div>
<div><br></div><div>This new check will trigger an error each time a binary is linked against libraries it doesn't really use.</div><div><br></div><div>I worked on this check because I discovered that some programs were linked against libssl although they didn't really use them, which complicates even more the upgrade process.</div>
<div>But avoiding useless libraries linking has other advantages:</div><div> - it reduces the number of dependancies to install. For exemple, I was able to remove three dependencies from ssh client, it was dependencies on libraries required by the server and not by the client.</div>
<div> - it allows to detect some build errors. For exemple, imapproxy was compiled against libwrap but there was an error in the build system and the libwrap code was not enabled although imapproxy was linked against libwrap.</div>
<div> - it reduces the number of reverse dependencies impacted when a library is upgraded and also when it is removed. </div><div><br></div><div>This situation seems to happens a lot either because:</div><div><br></div><div>
1. There is a problem in a configure script which incorrectly add useless libraries to the linker flags.</div><div> For exemple, some programs tries to link with more base libraries that necessary to be on the safe side, or they tries to link with libraries that are in fact only needed by their dependencies.</div>
<div><br></div><div> 2. There is an error in the pkg-config configuration file of a dependency which incorrectly requests to link against a larger set of libraries that needed, sometimes becauses no difference is made between static case and dynamic case.</div>
<div> That was for exemple a problem with neon, the neon-config tool incorrectly says that every programs linked against neon should also be linked against libssl. </div><div><br></div><div> 3. There are a bunch of binaries provided in a package and the build system links all of them against the same set of libraries althought some of them need only to be linked to only a subset of these libraries.</div>
<div> </div><div>The proper way to fix cases 1 and 2 is a fix in the configure script or pkg-config script.</div><div>The case 3 is harder to fix as build system seldomly make distinction between the link options of binaries (and this is not a big problem for Opencsw except if the package is split).</div>
<div>
<br></div><div>But there is a also direct and easy way to solve all these cases: just add "-z ignore" to the linker flags and ld will automatically ignore "-llibrary" option when no symbol of the library is used.</div>
<div><br></div><div>As soon as I commit the patch, a lot of build will trigger this new checkpkg error so I am thinking about first trying to promote the "-z ignore" option as a default linker flags for all opencsw builds (following the same path I proposed for "-B direct" <a href="http://lists.opencsw.org/pipermail/maintainers/2012-August/017113.html">http://lists.opencsw.org/pipermail/maintainers/2012-August/017113.html</a> ).</div>
<div><br></div><div>I welcome any comment on that.</div><div><br></div><div>FYI, the new checkpkg error is documented in the wiki: <a href="http://wiki.opencsw.org/checkpkg-error-tags#toc50" target="_blank">http://wiki.opencsw.org/checkpkg-error-tags#toc50</a></div>
<div>the full patch is attached and you'll find below the patch content without the tests for comments. </div><div>Maciej already reviewed it and gave me useful feedback (thanks to him).</div><div><br></div><div>Thanks in advance for your feedbacks,</div>
<div><br></div></div><div>Yann</div><div><br></div>
<div><br></div><div><br></div><div><div>Index: lib/python/dependency_checks.py</div><div>===================================================================</div><div>--- lib/python/dependency_checks.py<span style="white-space:pre-wrap"> </span>(revision 18898)</div>
<div>+++ lib/python/dependency_checks.py<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -39,6 +39,12 @@</div><div> </div><div> PREFERRED_DIRECTORY_PROVIDERS = set([u"CSWcommon"])</div>
<div> </div><div>+BASE_SOLARIS_LIBRARIES = ( </div><div>+ "libsocket.so.1", "libnsl.so.1", "libdl.so.1", "librt.so.1", "libresolv.so.2", "libpthread.so.1",</div>
<div>+ # linked by default with C++, see "Default C++ Libraries" in Solaris Studio C++ User'sGuide</div><div>+ "libCstd.so.1", "libCrun.so.1", "libm.so.1", "libm.so.2", "libw.so.1", "libcx.so.1", "libc.so.1", "libC.so.3", "libC.so.5"</div>
<div>+)</div><div>+</div><div> def ProcessSoname(</div><div> ldd_emulator,</div><div> soname, path_and_pkg_by_basename, binary_info, isalist, binary_path, logger,</div><div>@@ -147,6 +153,20 @@</div><div> error_mgr,</div>
<div> pkgname, messenger)</div><div> orphan_sonames.extend(orphan_sonames_tmp)</div><div>+</div><div>+ ldd_info = pkg_data['ldd_info'][binary_info["path"]]</div><div>+ for ldd_response in ldd_info:</div>
<div>+ if ldd_response['state'] == 'soname-unused' and ldd_response['soname'] not in BASE_SOLARIS_LIBRARIES:</div><div>+ messenger.Message(</div><div>+ "Binary %s links to library %s but doesn't seem to use any of its symbols. "</div>
<div>+ "It usually happens because superfluous libraries were added to the linker options, "</div><div>+<span style="white-space:pre-wrap"> </span> "either because of the configure script itself or because of the \"pkg-config --libs\""</div>
<div>+<span style="white-space:pre-wrap"> </span> " output of one the dependency."</div><div>+ % ("/" + binary_info["path"], ldd_response['soname']))</div>
<div>+ error_mgr.ReportError(</div><div>+ pkgname, "soname-unused",</div><div>+ "%s is needed by %s but never used" % (ldd_response['soname'], "/" + binary_info["path"]))</div>
<div>+</div><div> orphan_sonames = set(orphan_sonames)</div><div> for soname, binary_path in orphan_sonames:</div><div> if soname not in ALLOWED_ORPHAN_SONAMES:</div><div>Index: lib/python/package_stats.py</div><div>
===================================================================</div><div>--- lib/python/package_stats.py<span style="white-space:pre-wrap"> </span>(revision 18898)</div><div>+++ lib/python/package_stats.py<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -208,6 +208,7 @@</div><div> "basic_stats": basic_stats,</div><div> "files_metadata": dir_pkg.GetFilesMetadata(),</div><div> "mtime": self.GetMtime(),</div><div>
+<span style="white-space:pre-wrap"> </span>"ldd_info": dir_pkg.GetLddMinusRlines(),</div><div> }</div><div> self.SaveStats(pkg_stats)</div><div> logging.debug("Statistics of %s have been collected.", repr(dir_pkg.pkgname))</div>
<div>Index: lib/python/inspective_package.py</div><div>===================================================================</div><div>--- lib/python/inspective_package.py<span style="white-space:pre-wrap"> </span>(revision 18898)</div>
<div>+++ lib/python/inspective_package.py<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -231,15 +231,14 @@</div><div> </div><div> def GetLddMinusRlines(self):</div><div> """Returns ldd -r output."""</div>
<div>- dir_pkg = self.GetInspectivePkg()</div><div>- binaries = dir_pkg.ListBinaries()</div><div>+ binaries = self.ListBinaries()</div><div> ldd_output = {}</div><div> for binary in binaries:</div><div>- binary_abspath = os.path.join(dir_pkg.directory, "root", binary)</div>
<div>+ binary_abspath = os.path.join(self.directory, "root", binary)</div><div> # this could be potentially moved into the DirectoryFormatPackage class.</div><div> # ldd needs the binary to be executable</div>
<div> os.chmod(binary_abspath, 0755)</div><div>- args = ["ldd", "-r", binary_abspath]</div><div>+ args = ["ldd", "-Ur", binary_abspath]</div><div> ldd_proc = subprocess.Popen(</div>
<div> args,</div><div> stdout=subprocess.PIPE,</div><div>@@ -250,7 +249,9 @@</div><div> logging.error("%s returned an error: %s", args, stderr)</div><div> ldd_info = []</div><div>
for line in stdout.splitlines():</div><div>- ldd_info.append(self._ParseLddDashRline(line))</div><div>+<span style="white-space:pre-wrap"> </span>result = self._ParseLddDashRline(line, binary_abspath) </div>
<div>+<span style="white-space:pre-wrap"> </span>if result:</div><div>+ ldd_info.append(result)</div><div> ldd_output[binary] = ldd_info</div><div> return ldd_output</div><div> </div><div>
@@ -263,7 +264,7 @@</div><div> sym = { 'address': fields[0], 'type': fields[1], 'name': fields[2] }</div><div> return sym</div><div> </div><div>- def _ParseLddDashRline(self, line):</div>
<div>
+ def _ParseLddDashRline(self, line, binary=None):</div><div> found_re = r"^\t(?P<soname>\S+)\s+=>\s+(?P<path_found>\S+)"</div><div> symbol_not_found_re = (r"^\tsymbol not found:\s(?P<symbol>\S+)\s+"</div>
<div> r"\((?P<path_not_found>\S+)\)")</div><div>@@ -280,12 +281,17 @@</div><div> r'file (?P<sizediff_file2>\S+) size=(?P<size2>0x\w+)\)$')</div>
<div> sizes_one_used = (r'^\t\t(?P<sizediffused_file>\S+) size used; '</div><div> r'possible insufficient data copied$')</div><div>- common_re = (r"(%s|%s|%s|%s|%s|%s|%s|%s)"</div>
<div>- % (found_re, symbol_not_found_re, only_so, version_so,</div><div>- stv_protected, sizes_differ, sizes_info, sizes_one_used))</div><div>+ unreferenced_object = (r'^\s*unreferenced object=(?P<object>.*); unused dependency of (?P<binary>.*)$')</div>
<div>+ unused_object = (r'^\s*unused object=.*$')</div><div>+ unused_search_path = (r'^\s*unused search path=.* \(RUNPATH/RPATH from file .*\)$')</div><div>+ blank_line = (r'^\s*$')</div>
<div>+ common_re = (r"(%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s)"</div><div>+ % (found_re, symbol_not_found_re, only_so, version_so, stv_protected, sizes_differ, sizes_info,</div><div>+<span style="white-space:pre-wrap"> </span> sizes_one_used, unreferenced_object, unused_object, unused_search_path, blank_line))</div>
<div> m = re.match(common_re, line)</div><div>- response = {}</div><div>+ response = None</div><div> if m:</div><div>+ response = {}</div><div> d = m.groupdict()</div><div> if "soname" in d and d["soname"]:</div>
<div> # it was found</div><div>@@ -298,6 +304,11 @@</div><div> response["soname"] = None</div><div> response["path"] = d["path_not_found"]</div><div> response["symbol"] = d["symbol"]</div>
<div>+ elif "binary" in d and d["binary"] and binary == d["binary"]:</div><div>+ response["state"] = "soname-unused"</div><div>+ response["soname"] = os.path.basename(d["object"])</div>
<div>+ response["path"] = None</div><div>+ response["symbol"] = None</div><div> elif d["path_only"]:</div><div> response["state"] = "OK"</div>
<div>
response["soname"] = None</div><div>@@ -328,12 +339,11 @@</div><div> response["soname"] = None</div><div> response["path"] = "%s" % (d["sizediffused_file"])</div>
<div> response["symbol"] = None</div><div>- else:</div><div>- raise StdoutSyntaxError("Could not parse %s with %s"</div><div>- % (repr(line), common_re))</div>
<div>+</div><div> else:</div><div>- raise StdoutSyntaxError("Could not parse %s with %s"</div><div>- % (repr(line), common_re))</div><div>+ raise package.StdoutSyntaxError("Could not parse %s with %s"</div>
<div>+ % (repr(line), common_re))</div><div>+</div><div> return response</div><div> </div><div> def GetDependencies(self):</div><div>Index: lib/python/package.py</div><div>===================================================================</div>
<div>--- lib/python/package.py<span style="white-space:pre-wrap"> </span>(revision 18898)</div><div>+++ lib/python/package.py<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -42,6 +42,8 @@</div><div> class PackageError(Error):</div><div> pass</div><div> </div><div>+class StdoutSyntaxError(Error):</div><div>+ pass</div><div> </div><div> class CswSrv4File(shell.ShellMixin, object):</div>
<div> """Represents a package in the srv4 format (pkg)."""</div></div><div><br></div><div><br></div>