Request 1105335: Submit libphonenumber

This request is superseded by request 1105413 (Show diff)

Overview

Request 1105335 superseded

- Force-add -DNDEBUG to CFLAGS; libphonenumber relies on it.

new cmake exposed this https://build.opensuse.org/request/show/1104274

Created by jengelh over 1 year ago
In state superseded
Package maintainer: Vogtinator
Superseded by 1105413

Submit libphonenumber

Comments for request 1105335 10

Fabian Vogt (Vogtinator) - over 1 year ago

target maintainer

This is supposed to be set by CMake itself. If this no longer works with the linked cmake submission, it's probably wrong.

> grep NDEBUG /usr/share/cmake/Modules/Compiler/GNU.cmake
  string(APPEND CMAKE_${lang}_FLAGS_MINSIZEREL_INIT " -Os -DNDEBUG")
  string(APPEND CMAKE_${lang}_FLAGS_RELEASE_INIT " -O3 -DNDEBUG")
  string(APPEND CMAKE_${lang}_FLAGS_RELWITHDEBINFO_INIT " -O2 -g -DNDEBUG")

Jan Engelhardt (jengelh) - over 1 year ago

author target maintainer

If your code does not compile without -DNDEBUG, that's wrong. NDEBUG should only control assert(), cf. https://en.cppreference.com/w/c/error/assert

Fabian Vogt (Vogtinator) - over 1 year ago

target maintainer

I agree with that, but it's expected that in RelWithDbgInfo builds, -DNDEBUG is set.

Whatever the cmake SR does breaks this and needs to be fixed.

Jan Engelhardt (jengelh) - over 1 year ago

author target maintainer

You can edit d:l:c/libphonenumber's "working" .spec file and use the "working" cmake and replace the %cmake_build line with

%cmake -DBUILD_STATIC_LIB=OFF -DBUILD_SHARED_LIB=ON -DBUILD_TESTING=ON -DBUILD_GEOCODER=OFF -DREGENERATE_METADATA=OFF \
'-DCMAKE_C_FLAGS_RELWITHDEBINFO=-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g' \
'-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g'

Setting CFLAGS from the command line is a supported use-case of cmake, and libphonenumber threw it down the gutter. This bug is on libphonenumber and libphonenumber itself.

Fabian Vogt (favogt) - about 1 year ago

The CMakeLists sets CMAKE_${lang}_FLAGS_RELWITHDEBINFO_INIT which is correct. The issue is that this sets CMAKE_C_FLAGS_RELWITHDEBINFO instead of CFLAGS, so it overrides CMAKE_${lang}_FLAGS_RELWITHDEBINFO_INIT: https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS_INIT.html#variable:CMAKE_%3CLANG%3E_FLAGS_INIT

Jan Engelhardt (jengelh) - about 1 year ago

author target maintainer

When I said “Setting CFLAGS from the command line” I meant “Setting (the cmake's unwieldly long name for what most people know as CFLAGS) from the command line”.

Fabian Vogt (favogt) - about 1 year ago

Yeah, and that works here.

What the spec file currently does results in C_FLAGS = -O2 -g -m64 -fmessage-length=0 -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -O2 -g -DNDEBUG -fPIC -Wall -Werror in the Makefiles.

Jan Engelhardt (jengelh) - about 1 year ago

author target maintainer

Huh?!

See home:jengelh:cmakecrap/libphonenumber the moment I'm asking for full debug, which is totally within the right of a user to do, the thing fails to build.

Fabian Vogt (favogt) - about 1 year ago

-DCMAKE_BUILD_TYPE=Debug should be enough. Let me check what happens here.

Fabian Vogt (favogt) - about 1 year ago

Issue reproduced, it's a bug in abseil. absl_log is missing a dependency on absl_log_internal_check_op.