Skip to content

Makefile: Use pkg-config if available. #618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sangjinhan merged 4 commits into from
Aug 30, 2017
Merged

Makefile: Use pkg-config if available. #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24e4e5f
Makefile: Use pkg-config if available.
ddiproietto Aug 24, 2017
4067b84
Makefile: list `-ldl` also after DPDK.
ddiproietto Aug 25, 2017
c3c0292
Makefile: Fix build on container.
ddiproietto Aug 25, 2017
bbb11c3
Makefile: Use `:=` instead of `=` when shelling out.
ddiproietto Aug 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 39 additions & 6 deletions core/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ MAKEFLAGS += --no-builtin-rules

CXX ?= g++
PROTOC ?= protoc
PKG_CONFIG ?= pkg-config

VERBOSE ?= 0

Expand All @@ -57,6 +58,8 @@ $(error g++ 5 or higher is required. Use container_build.py if newer g++ is not
endif
endif

HAS_PKG_CONFIG := $(shell command -v $(PKG_CONFIG) 2>&1 >/dev/null && echo yes || echo no)

RTE_SDK ?= $(abspath ../deps/dpdk-17.05)
RTE_TARGET ?= $(shell uname -m)-native-linuxapp-gcc
DPDK_LIB ?= dpdk
Expand All @@ -75,6 +78,28 @@ else ifeq ($(words $(MAKECMDGOALS)),1)
endif
endif

# We always want these libraries to be dynamically linked even when the
# user requests a static build. These needs to be listed first, so even
# if they're listed again in the static section, they will be dynamically
# linked.
ALWAYS_DYN_LIBS := -lpthread -ldl
Copy link
Copy Markdown
Member

@sangjinhan sangjinhan Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to add -lpthread here? The -pthread option already implies this...

Copy link
Copy Markdown
Contributor Author

@ddiproietto ddiproietto Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's necessary.

grpc includes -lpthread in its statically required libraries. If we don't specify -lpthread before (in the dynamic section), -lpthread will be linked statically, and that's not well supported apparently

# These libraries are not supported by pkg-config.
ALWAYS_LIBS := -lpcap -lgflags
# If pkg-config is available, we just need a list of the dependecies.
PKG_CONFIG_DEPS := libglog protobuf grpc++ libunwind zlib
# If pkg-config is not available, we need to list the libs we depend on.
NO_PKG_CONFIG_LIBS := -lglog -lgflags -lprotobuf -lgrpc++ -lunwind -lz
# If pkg-config is not available and we're static linking, we also need
# the indirect dependecies. This is annoying, because they may change
# in future versions.
NO_PKG_CONFIG_LIBS_INDIRECT := -lgrpc -lssl -lcrypto -llzma

ifeq ($(HAS_PKG_CONFIG), yes)
PKG_CFLAGS := $(shell $(PKG_CONFIG) --cflags $(PKG_CONFIG_DEPS))
else
PKG_CFLAGS :=
endif

# Plugins get to look in $COREDIR through -I $(COREDIR). Let them also
# see their own top level directory, e.g., /some/path/to/modules/foo.cc
# gets /some/path/to/modules/.. so that it can read its own
Expand All @@ -93,7 +118,7 @@ CXXFLAGS += -std=c++11 -g3 -ggdb3 $(CXXARCHFLAGS) \
-isystem $(dir $<).. -isystem $(COREDIR)/modules \
-D_GNU_SOURCE \
-Werror \
-Wall -Wextra -Wcast-align
-Wall -Wextra -Wcast-align $(PKG_CFLAGS)

PERMISSIVE := -Wno-unused-parameter -Wno-missing-field-initializers \
-Wno-unused-private-field
Expand All @@ -112,20 +137,28 @@ LDFLAGS += -rdynamic -L$(DPDK_LIB_DIR) -Wl,-rpath=$(DPDK_LIB_DIR) -pthread
ifdef BESS_LINK_DYNAMIC
LIBS_ALL_SHARED = -Wl,-call_shared
LIBS_DL_SHARED =
ifeq ($(HAS_PKG_CONFIG), yes)
PKG_LIBS := $(shell $(PKG_CONFIG) --libs $(PKG_CONFIG_DEPS))
else
PKG_LIBS := $(NO_PKG_CONFIG_LIBS)
endif
else # Used static libraries
LIBS_ALL_SHARED =
LIBS_DL_SHARED = -Wl,-call_shared
LIBS_LZMA = -llzma
LDFLAGS += -static-libstdc++
ifeq ($(HAS_PKG_CONFIG), yes)
PKG_LIBS := $(shell $(PKG_CONFIG) --static --libs $(PKG_CONFIG_DEPS))
else
PKG_LIBS := $(NO_PKG_CONFIG_LIBS) $(NO_PKG_CONFIG_LIBS_INDIRECT)
endif
endif

LIBS += -Wl,-non_shared \
LIBS += $(ALWAYS_DYN_LIBS) -Wl,-non_shared \
-Wl,--whole-archive -l$(DPDK_LIB) -Wl,--no-whole-archive \
$(LIBS_ALL_SHARED) \
-lglog -lgflags -lprotobuf -lgrpc++ -lgrpc \
-lssl -lcrypto -lunwind $(LIBS_LZMA) -lpcap -lz \
$(PKG_LIBS) $(ALWAYS_LIBS) \
$(LIBS_DL_SHARED) \
-ldl
$(ALWAYS_DYN_LIBS)
Copy link
Copy Markdown
Member

@sangjinhan sangjinhan Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking: Adding $(ALWAYS_DYN_LIBS) twice was intended?

Copy link
Copy Markdown
Contributor Author

@ddiproietto ddiproietto Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was intended.

DPDK depends on -ldl, so -ldl must be specified after DPDK.

But then again, we want to dynamically link with -ldl. If one of the other libs depend on -ldl it will be listed in the static section. We must make sure that we include it at the beginning of the cmdline, in the dynamic section


ifdef SANITIZE
CXXFLAGS += -fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer
Expand Down
1 change: 1 addition & 0 deletions env/packages.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- libtool
- make
- cmake
- pkg-config
- libpthread-stubs0-dev
- libunwind8-dev
- liblzma-dev
Expand Down