You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by jablko <gi...@git.apache.org> on 2017/02/01 22:37:56 UTC
[GitHub] trafficserver pull request #1408: Prune some unused library dependencies
GitHub user jablko opened a pull request:
https://github.com/apache/trafficserver/pull/1408
Prune some unused library dependencies
and add a script to check for unused dependencies going forward.
This PR depends on #1392 because otherwise the script fails -- traffic_server is linked with zlib and/or liblzma but never uses either. The first commit belongs to that PR.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jablko/trafficserver unused
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1408.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1408
----
commit c902e82c50eeff10bdeef976e4c48d02d59cda2c
Author: Jack Bates <ja...@nottheoilrig.com>
Date: 2017-01-27T19:06:17Z
TS-2095: TS_HAS_LIBZ and TS_HAS_LZMA are always false
Since replacing TS_FLAG_HEADERS, zlibh and lzmah are always false.
Just use HAVE_ZLIB_H and HAVE_LZMA_H directly.
commit ac4d50b873dcb863070c893b7e999b6350fd5c10
Author: Jack Bates <ja...@nottheoilrig.com>
Date: 2017-02-01T21:29:16Z
Prune some unused library dependencies
and add a script to check for unused dependencies going forward.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1466/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
clang-analyzer build *successful*! See https://ci.trafficserver.apache.org/job/Github-Clang/22/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1451/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1457/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/1350/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
clang-analyzer build *successful*! See https://ci.trafficserver.apache.org/job/Github-Clang/30/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
clang-analyzer build *successful*! See https://ci.trafficserver.apache.org/job/clang-analyzer-github/50/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
clang-analyzer build *successful*! See https://ci.trafficserver.apache.org/job/Github-Clang/16/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
Linux build *successful*! See https://ci.trafficserver.apache.org/job/linux-github/1379/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/freebsd-github/1486/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by jablko <gi...@git.apache.org>.
Github user jablko commented on the issue:
https://github.com/apache/trafficserver/pull/1408
I rebased this onto #1416. My theory is that traffic_top (etc.) was getting hwloc (etc.) because of Libtool's link_all_deplibs [1]. As I understand it, when Libtool links to a library (libfoo), link_all_deplibs calls ld with -llibrary for all of libfoo's dependencies (inter-library dependencies). link_all_deplibs is turned off on Debian and Ubuntu (maybe other platforms) -- that's why I didn't see it on my machine -- but the default is "unknown", which is a synonym for "yes".
We can set link_all_deplibs ourselves, but calling the linker with --as-needed (#1416) is even more powerful because it will skip unused -llibraries that we add to the command line (because we add them to LIBS) as well as those that Libtool adds.
[1] https://www.gnu.org/software/libtool/manual/libtool.html#index-link_005fall_005fdeplibs
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1408: Prune some unused library dependencies
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1408#discussion_r99240250
--- Diff: iocore/cache/Cache.cc ---
@@ -1041,12 +1041,12 @@ CacheProcessor::cacheInitialized()
case CACHE_COMPRESSION_FASTLZ:
break;
case CACHE_COMPRESSION_LIBZ:
-#if !TS_HAS_LIBZ
+#ifndef HAVE_ZLIB_H
--- End diff --
Agreed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1408: Prune some unused library dependencies
Posted by jablko <gi...@git.apache.org>.
Github user jablko commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1408#discussion_r99212271
--- Diff: iocore/cache/Cache.cc ---
@@ -1041,12 +1041,12 @@ CacheProcessor::cacheInitialized()
case CACHE_COMPRESSION_FASTLZ:
break;
case CACHE_COMPRESSION_LIBZ:
-#if !TS_HAS_LIBZ
+#ifndef HAVE_ZLIB_H
--- End diff --
Yes to everything -- except I think we should be clear about which macros are either defined or undefined and which are either 1 or 0. Long term, I think we should only use one type or the other. For better or worse, Autoconf consistently uses defined/undefined.
I think we should consistently use #ifdef with defined/undefined macros because it's an indication of their type. I take seriously your point that #if works with either type, I just wonder if that's a good thing -- I wonder if it might obscure things. My feeling is that it's better to avoid mixing them -- avoid cases where a macro is possibly 0 and possibly undefined. I dunno -- someone might be tempted to refactor the following:
```C++
#if HAVE_ZLIB_H
print_feature("TS_HAS_LIBZ", 1, json);
#else
print_feature("TS_HAS_LIBZ", 0, json);
#endif
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1408: Prune some unused library dependencies
Posted by jablko <gi...@git.apache.org>.
Github user jablko closed the pull request at:
https://github.com/apache/trafficserver/pull/1408
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/1344/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1465/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1408: Prune some unused library dependencies
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1408
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/1358/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1408: Prune some unused library dependencies
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1408#discussion_r99174413
--- Diff: iocore/cache/Cache.cc ---
@@ -1041,12 +1041,12 @@ CacheProcessor::cacheInitialized()
case CACHE_COMPRESSION_FASTLZ:
break;
case CACHE_COMPRESSION_LIBZ:
-#if !TS_HAS_LIBZ
+#ifndef HAVE_ZLIB_H
--- End diff --
So, while I'm generally OK with this (I think), I'm slightly concerned that we don't have a universal pattern for the #if vs #ifdef. In general, I think we ought to prefer #if over $ifdef's, assuming it's defined to "1". This helps avoid the problem where #define foo 0 is actually "defined", but is off.
I know that we have plenty of cases where we do #ifndef / #ifdef, but unless there are other reasons to use that here, I think we should stick to #if when possible for these boolean types. Ideally, we'd later change this to be consistent as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---