build: enhance pcre and protobuf-c dependencies #613
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Have top level Makefile use user provided path to libprotobuf-c.a when configuring the agent. Update doc string in agent's config.m4.
Don't depend on pcre-config return information how to include libpcre in the build because this always results in linking to a shared object which becomes are a runtime dependency that is hard to satisfy because different Linux distributions use different name of the shared object (libpcre.so or libpcre3.so). To hedge against this nuisance, agent needs to statically link to pcre library. Therefore a new configuration option is added to agent's configure script: --with-pcre. It allows to provide path prefix to a lib directory that contains libpcre.a. The default location is /opt/nr/pcre/8.40 which is where build scripts install this library.
At the time the 'agent' target is built, log information about the origin of the build dependencies: 'pcre' and 'protobuf-c'.
Use information provided by pkg-config to set defaults for the origin of the build dependencies: 'pcre' and 'protobuf-c'.
Improve build dependencies checks, and abort build fast if user provided install location of 'pcre' or 'protobuf-c' does not satisfy agent's build requirements.
Improve the name of a variable that defines 'protobuf-c' install location - VENDOR_PREFIX is too broad.
Agent's build system makes an assumption that build dependencies are installed in standard locations in relation to dependency's prefix. I.e. header files are in %prefix%/include, libraries are in %prefix%/lib and binaries are in %prefix%/bin. Agent's build system does not support custom values for any of these pkg-config variables: bindir, exec_prefix, includedir, libdir.
Handle scenario when pkg-config can't find libpcre or libprotobuf-c packags by leaving _PREFIX variable empty.
Fix sytax and typo in `ifneq` comparison.
At the time PROTOBUF_C_PREFIX is set by the 'libprotobuf-c' dependency fallback strategy (i.e. build and use vendored source code), the 'local' subdirectory of 'vendor' may not exist yet which will cause `realpath` function to fail and leave variable PROTOBUF_C_PREFIX empty. Fix this by providing a known and existing path to realpath and then appending fixed string - '/local'.
Remove extra 'o' from 'libprotobuf-c' 🤦 (leftover from testing)
Rename VENDOR_PREFIX to PROTOBUF_C_PREFIX in all places.
Rename VENDOR_PREFIX to PROTOBUF_C_PREFIX in all places.
lavarou
force-pushed
the
build/user-provided-protobuf
branch
from
08b6076 to
42ddf38
Compare
lavarou
force-pushed
the
build/user-provided-protobuf
branch
from
42ddf38 to
4077e75
Compare
ZNeumann
approved these changes
Feb 23, 2023
zsistla
approved these changes
Feb 23, 2023
mfulb
reviewed
Feb 24, 2023
| @@ -305,7 +316,7 @@ daemon-protobuf: src/newrelic/infinite_tracing/com_newrelic_trace_v1/v1.pb.go | |||
| src/newrelic/infinite_tracing/com_newrelic_trace_v1/v1.pb.go: protocol/infinite_tracing/v1.proto | |||
| $(MAKE) vendor # Only build vendor stuff if v1.proto has changed. Otherwise | |||
| # this rule will be triggered every time the daemon is built. | |||
| $(VENDOR_BASE)/local/bin/protoc \ | |||
| $(VENDOR_PREFIX)/bin/protoc \ | |||
There was a problem hiding this comment.
Should this be PROTOBUF_C_PREFIX?
There was a problem hiding this comment.
I was thinking about it but decided this will require a separate PR for the following reasons:
protocis notprotoc-c.protocis protobuf compiler with support for many languages but C.protocis provided byprotobuf. protoc-c is built on top ofprotocand adds support for C language. In this particular targetprotocis needed, notprotoc-cand this PR only makes updates to provider ofprotoc-c.- src/newrelic/infinite_tracing/com_newrelic_trace_v1/v1.pb.go is stored in the repository and this target is not usually triggered in day to day builds.
src/newrelic/infinite_tracing/com_newrelic_trace_v1/v1.pb.gohas to be rebuild manually afterprotocol/infinite_tracing/v1.protogets an update and the result of the manual build has to be checked into the repository. If this target needs to be rebuilt, not onlyprotocneeds to be available, but also Go protocol buffer plugin needs to be manually installed - see Makefile#L295-L298.
Given 1. and especially 2., which requires additional manual effort to properly configure build environment, I decided for this PR not to touch this area of the build system and have it use protoc from vendor subdirectory, which will be built by the command on line 317.
mfulb
approved these changes
Feb 24, 2023
Include php-build-scripts's pcre installation in results provided by pkg-config when it sets default for the origin of the pcre build dependency.
bduranleau-nr
approved these changes
Feb 27, 2023
|
ok jenkins |
lavarou
added a commit
that referenced
this pull request
Mar 3, 2023
By default let the agent link to whatever libpcre is available on the system (most likely a shared object). However for release builds the agent needs to link with a static version of libpcre to ensure it runs on all Linux distros. This is needed because different Linux distributions use different names for the shared object and its impossible to link to a shared object that works universally on all Linux distributions. If PCRE_STATIC is set to yes, agent's build system will enforce linking to static version of libpcre and if it is not available, the build will abort. Supplements #613
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Give the user an option to use
protobuf-cfrom location alternative tovendorsubdirectory viaPROTOBUF_C_PREFIXenvironmental variable.$PROTOBUF_C_PREFIX/binneeds to haveprotoc-cbinary,$PROTOBUF_C_PREFIX/includeneeds to haveprotobuf-c/protobuf-c.hheader file and
$PROTOBUF_C_PREFIX/libneeds to havelibprotobuf-c.a.By default agent's build system will try to look for
protobuf-cdependencyin location returned by
pkg-config libprotobuf-c --variable=prefix, alocation where
protobuf-cis installed in the system via packagemanagers. If that location does not have
protobuf-c, agent's buildsystem will fallback to using
protobuf-cfromvendordirectory. Thisenhancement speeds up agent build because neither
vendor/protobufnorvendor/protobuf-cneed to be build ifprotobuf-cis installed in thesystem by the user.
If user or system provided
protobuf-cdoes not meet agent's build systemrequirements, the build will abort with an error message.
Force the agent to use static version of pcre library. This avoids the
issue with runtime dependency on libpcre shared object that is hard to
satisfy universally because different Linux distributions use different
name of the shared object: libpcre.so or libpcre3.so. Additionally the
user has an option to point to pcre library installation via
PCRE_PREFIXenvironmental variable. By default agent's build systemwill try to look for pcre dependency using
pkg-config.If user or system provided
pcredoes not meet agent's build systemrequirements, the build will abort with an error message.
To troubleshoot build issues caused by dependencies, agent will log which
dependencies it is using.