Bug 206304 - net-mgmt/net-snmp [PATCH] Optionalize libpkg support to better support libressl
Summary: net-mgmt/net-snmp [PATCH] Optionalize libpkg support to better support libressl
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Ryan Steinmetz
Keywords: patch
Depends on:
Reported: 2016-01-16 04:43 UTC by Adam McDougall
Modified: 2020-09-12 12:58 UTC (History)
4 users (show)

See Also:
driesm.michiels: maintainer-feedback-

Patch to make libpkg support optional (3.03 KB, patch)
2016-01-16 04:43 UTC, Adam McDougall
no flags Details | Diff
Poudriere log with LIBPKG set (default) (541.36 KB, text/plain)
2016-01-16 04:44 UTC, Adam McDougall
no flags Details
Poudriere log with LIBPKG unset (non-default) (797.69 KB, text/plain)
2016-01-16 04:46 UTC, Adam McDougall
no flags Details
Patch to make libpkg.so optional (2.07 KB, patch)
2016-02-24 04:46 UTC, Ian Lepore
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam McDougall 2016-01-16 04:43:15 UTC
Created attachment 165650 [details]
Patch to make libpkg support optional

I am submitting this patch so the "pkg" support in net-snmp can be disabled using a compile option on the port.  Aside from the obvious effect, it can help avoid a SSL library conflict.

I build all my packages in a customized poudriere jail that has NO openssl or libressl support in it at all because I expect my compiled packages to pull in libressl.  I do this to avoid any chance of accidentally using openssl from base or having software link to both openssl and libressl.  net-snmp will link with both libressl and openssl because a currently non-optional patch adds -lpkg.  pkg is currently required to compile with openssl, at least without a lot of work.  

In a jail without base openssl like mine, trying to compile net-snmp fails because -lpkg is used unconditionally and libpkg cannot find the openssl library it had been linked against.  This example is represented by the net-snmp-5.7.3_12-LIBPKG.log poudriere log that I will attach showing the default behavior on my jail (but from my updated port).  Some site-specific make.conf customization has been removed to avoid revealing too much about our site.

net-snmp-5.7.3_12-noLIBPKG.log is an example where I've UNSET the new LIBPKG compile option which causes it to skip using the patch which adds -lpkg to the compile.  You can see in the log where it skips the patch extra-patch-agent_Makefile.in which I renamed from patch-agent_Makefile.in.  Again in this log I skipped some site-specific make.conf customization.

I tested that the patch applies cleanly to the port, I tested it in poudriere and attached logs, I bumped the PORTREVISION.  I attempted to use a newer "options helper" LIBPKG_EXTRA_PATCHES but I could not get it to work.  I resorted to a 3 line .if conditional which matches the style from the rest of the Makefile.

Please consider applying this to the port so I don't have to "rm /usr/ports/net-mgmt/net-snmp/files/patch-agent_Makefile.in" every time I svn update ports (or at minimum each time net-snmp updates, but easier to remember to always do it).   I am willing to make changes and discuss if needed.  Thanks.
Comment 1 Adam McDougall 2016-01-16 04:44:23 UTC
Created attachment 165651 [details]
Poudriere log with LIBPKG set (default)
Comment 2 Adam McDougall 2016-01-16 04:46:44 UTC
Created attachment 165652 [details]
Poudriere log with LIBPKG unset (non-default)
Comment 3 Adam McDougall 2016-02-24 03:19:14 UTC
This is not ready yet, looks like I need to make patch-agent__mibgroup__host__data_access__swinst_pkginfo.c conditional too.
Comment 4 Ian Lepore freebsd_committer 2016-02-24 04:46:01 UTC
Created attachment 167342 [details]
Patch to make libpkg.so optional

I tested the original patch and discovered net-snmp was still linking with libpkg due to the configure script detecting its presence and automatically adding it.  This patch pre-sets the configure variable so that it skips the test and uses the pre-set value.

Verified using ldd on the generated snmpd to see that the reference to libpkg is there or not depending on the LIBPKG option.
Comment 5 markus.gebert 2016-08-18 13:44:49 UTC
Any chance this patch will be committed? Ian's version works for me.

In my case, without this option to turn of pkg support for net-snmp, PHP's snmp module links libpkg and its base openssl, while other PHP modules correctly depend on openssl from ports which is bad and leads to hard to track down problems (i.e. memory corruptions) for PHP processes, especially if you build port openssl with ASM=on.

I guess the real solution would be to build libpkg against the ssl library that was actually requested by the user. I'm not sure why the pkg port insists on linking base ssl. As long as it does, it's probably best if dependencies on libpkg are optional.
Comment 6 Michael Osipov 2017-03-17 14:38:19 UTC
Guys, please see my analysis and tell me what you think:
Comment 7 Walter Schwarzenfeld freebsd_triage 2018-01-16 13:28:18 UTC
Any advance here?
Comment 8 Michael Osipov 2018-08-04 22:03:01 UTC
I keep disabling php-snmp.ini every time Í rebuild world with ports...
Comment 9 Dries Michiels 2018-08-09 15:36:23 UTC
This would in general be a nice addition to the port just for the flexibility of not wanting a dependency on pkg for net-snmp.
Comment 10 Ryan Steinmetz freebsd_committer freebsd_triage 2018-08-09 15:46:25 UTC
The right solution here is to produce a libpkg utility library that includes the local DB code, but lacks the linkage against openssl.  Then, link net-snmp against this new library.

A rough overview of what this change would look like for pkg would be to:

- Review libpkg/Makefile.am for the libpkg_la section.
- Split out the minimal db pieces into libpkgdb_la.
- Review libpkg/libpkg.ver and create a new libpkg/libpkgdb.ver as appropriate.
- Test/review the resulting automake changes.
- Commit.

This will cleanly/permanently resolve the openssl-related issues without impacting any of the native support for the hrSWRunTable table in HOST-RESOURCES-MIB.

If you have the ability to generate some patches for pkg to accomplish this, I will help push them into the tree.
Comment 11 Ian Lepore freebsd_committer 2018-08-09 15:51:21 UTC
(In reply to Ryan Steinmetz from comment #10)

I actuallly need this feature completely separately from any openssl issue.  I need net-snmp with no dependency on libpkg because that library will not exist on the system running net-snmp (an embedded device that doesn't have the need or the resources to run pkg(8)).  Thus I would still like to see my patch committed for that reason, regardless of how any openssl-related problems are resolved.

If you'd like me to open a different PR and attach the patch to that, I will.
Comment 12 Ryan Steinmetz freebsd_committer freebsd_triage 2018-08-10 02:30:55 UTC

If your patch works properly and doesn't break any existing functionality: then it is approved.
Comment 13 Dries Michiels 2018-11-17 14:28:12 UTC
Hi, could somebody take a look at this again please? Thanks.
Comment 14 Dries Michiels 2018-11-26 19:52:57 UTC
Disabling pkg linking with net-snmp is already a big step forward. Often I feel like its not needed/a requirement and for example in a jail net-snmp pulls in pkg when one possibly wants to manage all jails from the host. Atleast the latter is my use case for the OPTION PKG switch if it gets commited to head.
Comment 15 Ryan Steinmetz freebsd_committer freebsd_triage 2020-09-12 12:58:18 UTC
Committed as part of the 5.9 update