-
Notifications
You must be signed in to change notification settings - Fork 592
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.
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
Makefiles cleanup #3410
Makefiles cleanup #3410
Changes from 6 commits
5582c89
91f06a4
ce5736b
a050e11
98a9e83
45a6b92
a2b4855
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,8 +45,10 @@ mcpp_ldflags := $(MCPP_HOME)/lib/libmcpp.a | |
cppflags = -fvisibility=hidden -Wall -Wextra -Wshadow -Wshadow-all -Wredundant-decls -Wno-shadow-field \ | ||
-Wdeprecated -Wstrict-prototypes -Werror -Wconversion -Wdocumentation -Wno-shadow-uncaptured-local \ | ||
-Wreorder-init-list -pthread \ | ||
$(if $(filter yes,$(OPTIMIZE)),-O2 -DNDEBUG,-g) | ||
$(if $(filter yes,$(OPTIMIZE)),-O2 -DNDEBUG,-g) \ | ||
-std=c++20 | ||
|
||
# TODO review this setting | ||
ifeq ($(MAXWARN),yes) | ||
cppflags += -Wweak-vtables | ||
endif | ||
|
@@ -55,15 +57,6 @@ endif | |
static_objdir = obj | ||
shared_objdir = obj | ||
|
||
clang_version = $(shell clang -dumpversion 2>&1 | cut -f1 -d\.) | ||
|
||
# We use -std=c++20 with clang 15.0 and later. | ||
ifeq ($(shell test $(clang_version) -ge 15 && echo yes),yes) | ||
cppflags += -std=c++20 | ||
else | ||
cppflags += -std=c++17 | ||
endif | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need to care about these older clang versions. |
||
nodeprecatedwarnings-cppflags := -Wno-deprecated-declarations | ||
nounusedparameter-cppflags := -Wno-unused-parameter | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,31 +1,17 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
known_distributions := centos rhel fedora debian ubuntu amzn sles | ||
linux_ids := $(if $(wildcard /etc/os-release),$(shell . /etc/os-release && echo $${ID} $${ID_LIKE}),) | ||
linux_id ?= $(word 1,$(or $(filter $(known_distributions),$(linux_ids)),$(linux_ids))) | ||
is-bin-program = $(and $(filter $(bindir)%,$($4_targetdir)),$(filter $($1_target),program)) | ||
|
||
ifneq ($(OECORE_SDK_VERSION),) | ||
linux_id = yocto | ||
endif | ||
|
||
ifneq ($(filter yocto poky,$(linux_id)),) | ||
known_distributions := centos rhel fedora debian ubuntu | ||
|
||
build-platform = default | ||
supported-platforms = $(build-platform) | ||
# Assign the linux_ids from the ID and ID_LIKE variables in /etc/os-release if available. | ||
linux_ids := $(if $(wildcard /etc/os-release),$(shell . /etc/os-release && echo $${ID} $${ID_LIKE}),) | ||
|
||
$(build-platform)_cc = $(CC) | ||
$(build-platform)_cxx = $(CXX) | ||
$(build-platform)_cppflags = -Wno-psabi | ||
$(build-platform)_libtool = $(AR) | ||
$(build-platform)_targetdir = $(if $(filter %/build,$5),/$(build-platform)) | ||
$(build-platform)_installdir = | ||
$(build-platform)_objdir = | ||
# Assign the linux_id to the first known distribution in the linux_ids list, or for unknown distributions to the first | ||
# element in the linux_ids list. | ||
linux_id ?= $(firstword $(or $(filter $(known_distributions),$(linux_ids)),$(linux_ids))) | ||
|
||
# system install directory | ||
system-install-dir := $(OECORE_TARGET_SYSROOT)/usr | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these no longer needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was for Yocto, if we ever want to add back support for it we can bring it back. But so far we are not testing/supporting this with 3.8. |
||
is-bin-program = $(and $(filter $(bindir)%,$($4_targetdir)),$(filter $($1_target),program)) | ||
|
||
else ifneq ($(and $(filter centos rhel fedora,$(linux_id)),$(filter x86_64 i%86,$(shell uname -m))),) | ||
ifneq ($(and $(filter centos rhel fedora,$(linux_id)),$(filter x86_64 i%86,$(shell uname -m))),) | ||
|
||
# | ||
# MultiLib Linux (x64 libraries go in the lib64 directory, x86 executable names are suffixed with 32) | ||
|
@@ -64,10 +50,8 @@ x86_excludes = slice2% | |
|
||
else ifneq ($(filter debian ubuntu,$(linux_id)),) | ||
|
||
# | ||
# MultiArch Linux (libraries are installed in lib/<arch>, executables are installed in bin/<arch> | ||
# except for the build architecture where executables are installed in bin/). | ||
# | ||
# MultiArch Linux (libraries are installed in lib/<arch>, executables are installed in bin/<arch> except for the build | ||
# architecture where executables are installed in bin/). | ||
build-platform ?= $(shell dpkg --print-architecture) | ||
foreign-platforms ?= $(shell dpkg --print-foreign-architectures) | ||
|
||
|
@@ -113,12 +97,8 @@ else | |
# | ||
ifeq ($(OPTIMIZE),yes) | ||
# Use default system packaging flags if building with OPTIMIZE and CXXFLAGS/LDFLAGS aren't defined. | ||
ifneq ($(filter amzn sles,$(linux_id)),) | ||
opt-cppflags = $(if $(CXXFLAGS),,$(shell setarch $1 rpm --eval %optflags)) | ||
opt-ldflags = $(if $(LDFLAGS),,$(shell setarch $1 rpm --eval %?__global_ldflags)) | ||
else | ||
opt-cppflags = $(if $(CXXFLAGS),,-O2) | ||
endif | ||
opt-cppflags = $(if $(CXXFLAGS),,$(shell setarch $1 rpm --eval %optflags)) | ||
opt-ldflags = $(if $(LDFLAGS),,$(shell setarch $1 rpm --eval %?__global_ldflags)) | ||
endif | ||
|
||
build-platform = $(if $(filter arm%,$(shell uname -m)),arm,$(shell uname -m)) | ||
|
@@ -136,7 +116,7 @@ rpath-link-ldflag = -Wl,-rpath-link,$1 | |
make-rpath-link-ldflags = $(foreach d,$(filter-out $2,$(call get-all-deps,$1)),$(call rpath-link-ldflag,$($d_targetdir))) | ||
|
||
# If building objects for a shared library build, add -fPIC or -fPIE. | ||
# gcc in most Linux distribution is configued with --enable-default-pie, which is equivalent to auto-adding -fPIE -pie: | ||
# gcc in most Linux distribution is configured with --enable-default-pie, which is equivalent to auto-adding -fPIE -pie: | ||
# https://gcc.gnu.org/install/configure.html | ||
# We pass these options explicitly in case the gcc we use is not configured with --enable-default-pie. | ||
shared_cppflags = $(if $(filter-out program,$($1_target)),-fPIC,-fPIE) | ||
|
@@ -160,8 +140,6 @@ cppflags = -fvisibility=hidden -Wall -Wextra -Wredundant-decls -Wshadow - | |
$(if $(filter yes,$(OPTIMIZE)),-DNDEBUG,-g) | ||
ldflags = -pthread | ||
|
||
gcc_version = $(shell $(CXX) -dumpversion 2>&1 | cut -f1 -d\.) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. never used. |
||
|
||
# As of GCC 13.4, https://gcc.gnu.org/projects/cxx-status.html#cxx20 still describes C++20 support as "experimental". | ||
cppflags += -std=c++17 | ||
|
||
|
@@ -186,35 +164,15 @@ mkshlibname = lib$(1).so | |
# Clear the iconv ldflags, iconv is part of libc on Linux | ||
iconv_ldflags := | ||
|
||
# | ||
# With GCC < 6.1, libbacktrace crash with a SEGV for -pie exes linked with debug-stripped libs | ||
# See https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00132.html | ||
# Here we assume only release builds occasionally get -g and later get stripped | ||
# TODO: enable libbacktrace for release builds with GCC >= 6.1 | ||
ifneq ($(OPTIMIZE),yes) | ||
libbacktrace_fullpath := $(shell $(CXX) --print-file-name=libbacktrace.a) | ||
ifneq ($(libbacktrace_fullpath),libbacktrace.a) | ||
libbacktrace = yes | ||
endif | ||
endif | ||
|
||
IceUtil_system_libs = -lrt $(if $(filter yes,$(libbacktrace)),-lbacktrace) | ||
Ice_system_libs = -ldl -lssl -lcrypto $(IceUtil_system_libs) | ||
ifeq ($(shell pkg-config --exists libsystemd 2> /dev/null && echo yes),yes) | ||
Ice_system_libs += $(shell pkg-config --libs libsystemd) | ||
endif | ||
IceUtil_system_libs = -lrt -lbacktrace | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lib backtrace and lib systemd should be available in all supported platforms |
||
Ice_system_libs = -ldl -lssl -lcrypto $(IceUtil_system_libs) $(shell pkg-config --libs libsystemd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens if there's no systemd, does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think we should keep the check for libsystemd installed, or just emit an error if missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the check. For instance it's not going to almost ever be available in slim Docker builds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed as suggested There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! |
||
Glacier2CryptPermissionsVerifier_system_libs = -lcrypt | ||
|
||
icegridadmin_system_libs = -ledit | ||
icestormadmin_system_libs = -ledit | ||
|
||
# | ||
# On supported platforms and if Bluez and DBus are installed, we set the IceBT | ||
# system libraries. The build system checks for this variable to build or not | ||
# the IceBT plugin, demos, ... | ||
# | ||
ifneq ($(filter debian ubuntu yocto poky,$(linux_id)),) | ||
# If BlueZ and DBus are installed, set the IceBT system libraries. | ||
# The build system uses this variable to determine whether or not to build the IceBT targets. | ||
ifeq ($(shell pkg-config --exists bluez dbus-1 2> /dev/null && echo yes),yes) | ||
IceBT_system_libs = $(shell pkg-config --libs dbus-1) | ||
endif | ||
IceBT_system_libs = $(shell pkg-config --libs dbus-1) | ||
endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,5 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
# | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just the same as project macro, don't to have both. |
||
# $(call test,[$1]) | ||
# | ||
# Returns the test project name (./test/Ice/operations -> test/Ice/operations) | ||
# | ||
test = $(patsubst $(lang_srcdir)/%,%,$(if $1,$1,$(currentdir))) | ||
|
||
# | ||
# $(call test-sources,$1=sources,$2=dir,$3=main-src extra-srcs) | ||
# | ||
|
@@ -83,7 +76,7 @@ endef | |
# Returns the tests which don't have a Makefile.mk fragment specified | ||
# | ||
tests-without-project-makefile = $(foreach d,$(patsubst %/Client.$1,%,$(shell find $(lang_srcdir)/test -name Client.$1)),\ | ||
$(if $(wildcard $d/Makefile.mk),,$(call test,$d))) | ||
$(if $(wildcard $d/Makefile.mk),,$(call project,$d))) | ||
|
||
# | ||
# The tests variable is used to load tests in Makefile.mk fragments | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_writer_sources = Writer.cpp DuplicateSymbols.cpp Test.ice | ||
$(project)_writer_sources = Writer.cpp DuplicateSymbols.cpp Test.ice | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = reader writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = reader writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_reader_sources = Reader.cpp | ||
$(test)_writer_sources = Writer.cpp | ||
$(project)_reader_sources = Reader.cpp | ||
$(project)_writer_sources = Writer.cpp | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = reader writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = reader writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_reader_sources = Reader.cpp | ||
$(test)_writer_sources = Writer.cpp | ||
$(project)_reader_sources = Reader.cpp | ||
$(project)_writer_sources = Writer.cpp | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = reader writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = reader writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_reader_sources = Reader.cpp Test.ice | ||
$(test)_writer_sources = Writer.cpp Test.ice | ||
$(project)_reader_sources = Reader.cpp Test.ice | ||
$(project)_writer_sources = Writer.cpp Test.ice | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = reader writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = reader writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_reader_sources = Reader.cpp Test.ice | ||
$(test)_writer_sources = Writer.cpp Test.ice | ||
$(project)_reader_sources = Reader.cpp Test.ice | ||
$(project)_writer_sources = Writer.cpp Test.ice | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = reader writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = reader writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_reader_sources = Reader.cpp | ||
$(test)_writer_sources = Writer.cpp | ||
$(project)_reader_sources = Reader.cpp | ||
$(project)_writer_sources = Writer.cpp | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_programs = reader writer | ||
$(test)_dependencies = DataStorm Ice TestCommon | ||
$(project)_programs = reader writer | ||
$(project)_dependencies = DataStorm Ice TestCommon | ||
|
||
$(test)_reader_sources = Reader.cpp Test.ice | ||
$(test)_writer_sources = Writer.cpp Test.ice | ||
$(project)_reader_sources = Reader.cpp Test.ice | ||
$(project)_writer_sources = Writer.cpp Test.ice | ||
|
||
tests += $(test) | ||
tests += $(project) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
# Copyright (c) ZeroC, Inc. | ||
|
||
$(test)_client_sources = Client.cpp BackendI.cpp Backend.ice | ||
$(test)_client_dependencies = Glacier2 | ||
$(project)_client_sources = Client.cpp BackendI.cpp Backend.ice | ||
$(project)_client_dependencies = Glacier2 | ||
|
||
$(test)_server_sources = Server.cpp BackendI.cpp Backend.ice | ||
$(project)_server_sources = Server.cpp BackendI.cpp Backend.ice | ||
|
||
tests += $(test) | ||
tests += $(project) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments to these macros to make clear what is the intend.