Injecting functions for Solaris 10 build

[SeanBurfordT]

I'm attempting to build PCP on Solaris 10.

Yes, I realize that this is potentially a complicated endeavor.

I have promising progress, however would like to add a local implementation of strcasestr(). Normally I would add a compat_strcasestr.c and compat_strcasestr.h file with a weak symbol for strcasestr(), however it's not obvious how to do this in the PCP build system.

bash ./Makepkgs CFLAGS='' CXXFLAGS='-std=gnu++11' AR='/usr/sfw/sparc-sun-solaris2.10/bin/ar'

== Cleaning build tree for packaging build
Wrote: /export/home/shared/src/pcp/build/tar/pcp-7.1.0.src.tar.gz

== Preparing fresh build tree for packaging build

== Configuring pcp, log is in /export/home/shared/src/pcp/Logs/pcp  (--sysconfdir=/etc --localstatedir=/var --prefix=/usr --libdir=/usr/lib/64  CFLAGS= CXXFLAGS=-std=gnu++11 AR=/usr/sfw/sparc-sun-solaris2.10/bin/ar)

== Building pcp, log is in /export/home/shared/src/pcp/Logs/pcp
Make default_pcp failed, see log in /export/home/shared/src/pcp/Logs/pcp
/opt/csw/bin/gcc -D_POSIX_PTHREAD_SEMANTICS -I../../src/include -I../../src/include/pcp -DPCP_VERSION=\"7.1.0\" -fPIC -fno-strict-aliasing -D_XPG4_2 -D__EXTENSIONS__ -m64 -Wshadow -Wno-array-bounds  -Wall -O2 -g     -c -o pmnsutil.o pmnsutil.c
/opt/csw/bin/gcc -D_POSIX_PTHREAD_SEMANTICS -I../../src/include -I../../src/include/pcp -DPCP_VERSION=\"7.1.0\" -fPIC -fno-strict-aliasing -D_XPG4_2 -D__EXTENSIONS__ -m64 -Wshadow -Wno-array-bounds  -Wall -O2 -g   -o pmnsmerge -m64 -fPIC -Wall -L../../src/libpcp/src  pmnsmerge.o pmnsutil.o -lpcp -lnsl -lsocket -lresolv -ldl -lposix4
Undefined                       first referenced
 symbol                             in file
strcasestr                          ../../src/libpcp/src/libpcp.so
ld: fatal: symbol referencing errors. No output written to pmnsmerge
make[2]: *** [GNUmakefile:41: pmnsmerge] Error 1
make[1]: *** [GNUmakefile:163: default_pcp] Error 2
make[1]: Leaving directory '/export/home/shared/src/pcp/pcp-7.1.0/src'
make: *** [GNUmakefile:50: default_pcp] Error 2
Reinstate VERSION.pcp ...

[kmcdonell]

@SeanBurfordT It may not be as complicated as you imagine ... I build PCP for OpenIndiana in the QA Farm.
For glibc symbols not in the local libc, the approach for foo() is something like ...

1. add a AC_CHECK_LIB() test in configure.ac to set have_foo to true or false
2. add a stanza of code in src/include/platform_defs.h.in guarded by #ifndef HAVE_FOO and then the function prototype for foo()
3. over in src/libpcp/src, pick a likely source file (strings.c would be a good one for you) and add the implementation guarded by #ifndef HAVE_FOO
4. but now there's a problem, as you need to get foo; into src/libpcp/src/exports.in in a conditional way, and the current recipe supports some conditionals, but not for #defines in platform_def.h

Not that there is already a strcasestr() implementation hiding in win32.c, so best plan would be to move that to strings.c.

Let me work in 4. if you want to tackle 1. to 3.

[kmcdonell]

OK, as of commit 9be1393 4. is done.

For 2. above, src/include/pcp/config.h.in is probably a better place for the #undef HAVE_STRCASESTR

[SeanBurfordT]

Thanks, I'll make and verify the changes and send a PR against this issue.

[kmcdonell]

👍

[SeanBurford]

Your advice has been useful. I'll have a set of small commits in my fork soon and will discuss how to bundle these for you.

The areas touched so far are:

• Address missing strcasestr() and cfmakeraw().
• Rename printdelta() to printtimedelta() in AF.c to resolve conflict with a Solaris 10 streams macro in the system headers.
• Fix a bug in printdelta() (the FILE * argument was ignored and output always went to stderr, which was OK since all callers provide f=stderr, but still a bug).
• pcp.env assumes /bin/sh is Bash, which is not true on Solaris 10. I have changed the one instance of export A=B to A=B; export A in pcp.env. This Bash assumption affects lots of scripts under qa/admin as well, but I'm leaving them alone.

makecfraw is used by pmdas/roomtemp and pmieconf. Rather than exporting it from libpcp you might prefer that I add it to pmieconf (it is already conditionally defined in roomtemp)?

You can view the diff at main...SeanBurford:pcp:main

[kmcdonell]

G'day @SeanBurford.
Suggest you push first 3 in one PR ... no issues that I can see there, modulo a review (see below)
The pcp.env one is a bigger problem ... not in terms of the change (it is an auto-rewrite that our mate sed(1) will eat for lunch), but the scope (it will touch some 350 files) ... suggest I tackle this one across the board outside the PR above.

Review of diff:

• I think cfmakeraw() can live in libpcp/src/util.c with a #ifndef HAVE_CFMAKERAW guard, like all the other "not in everyone's libc" baggage
• otherwise LGTM

[SeanBurford]

Thanks, there is still at least one build failure to work through (strncmp in pmcd), I'll prepare a PR once it is addressed.

[kmcdonell]

@SeanBurfordT OK, as of commit 78339f1 all the export FOO=bar fragments have been rewritten as FOO=bar; export FOO.

[SeanBurford]

I have successfully built PCP on Solaris 10, which has uncovered another bug (the no*.c files that disable functionality when libuv is not available were missing a couple of functions).

The exports workaround in the previous PR was incorrect. It works with the GNU linker but fails with a duplicate symbol on the Solaris linker. I have a better fix that does not disrupt symbol export history in the exports file (remove the global: keyword from the new version section, so that the new section is empty on systems that have all necessary libc functions).

More importantly, I feel like adding libc function names to exports is the wrong long term approach. While it is convenient in order to get a new platform working, it commits to having libpcp export these functions. Most of the exports support a handful of utilities:

1. cfmakeraw is used by libpcp, libpcp_*, PMDAS/roomtemp, PMIEConf.
2. strcasestr is used by libpcp, libpcp_*, htop.
3. strndup is used by libpcp, libpcp3, libpcp_*, htop, PMCD, PMDAS, PMIE, pmlogconf, pmlogger.
4. strsep is used by QA, atop, PMDAS, htop. QA built against libpcp3 requires strcasestr, meaning that libpcp3 also needs to export strcasestr.

I suggest a new issue to track that creation of a new library (e.g. called it libport or libcompat). str* and libc functions from utils.c, strings.c and win32.c would move into libport. libpcp and PCP utilities would then import libport, creating a ring fence around the portability functions so that they don't pollute libpcp's exports.

An alternative would be to make the utilities conditionally compile when the necessary libc functions are available. I don't like this approach, because it redefines HAVE_METHOD to either mean that an alternative implementation has been provided (e.g. by libport) or that the utility should not compile. Adding an alternative implementation of a missing function will mean hunting through the code base for utilities that can now compile.

PCP contains a wealth of compatibility functions that I've also implemented elsewhere, I would love to borrow PCP libport when building other tools.

[kmcdonell]

@SeanBurford thanks for these suggestions.
My first reaction is that another library is not needed:

• there is significant development and testing cost for the additional packaging overhead associated with adding a new libpcp_compat ... I really don't want to do that if there is an option
• 3 of the 4 cases you site involve these routines being called from libpcp, so the symbols could comfortably live in libpcp for platforms that don't provide 'em via libc, and as you note in the 4th case the apps already link with libpcp
• we should treat the Windows ones as old history, AFAIK no one has built PCP for Windows for decades, and if that was ever an interest again there would be a lot of rework required
• the symbols pollution in libpcp is completely contained already ... if configure finds you have foo, then HAVE_FOO is false and we can not compile our version of foo() and not include foo in the exports (via the exports.in conditionals we use in the build)
• I do think pulling all of the implementations out of the various source files into a new compat.c would tidy things up in the source code, and you could then "borrow" compat.c for your other projects
• the problem you encountered with exports.in will go away when there is another unconditional symbol in the next version or we tack 'em onto the current last version pro tem