Skip to content
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

testing/install removes -Werror blindly, leaving any arguments hanging (test fails) #3775

Open
drew-parsons opened this issue Aug 20, 2023 · 1 comment
Labels
area: build Build issues

Comments

@drew-parsons
Copy link

The configuration file for testing/install removes -Werror (and -Wall) blindly from CFLAGS (CXXFLAGS),

# Remove -Wall -Werror in this scope

# Remove -Wall -Werror in this scope
if(CMAKE_C_FLAGS)
  string(REPLACE "-Wall"    " " CMAKE_C_FLAGS   ${CMAKE_C_FLAGS})
  string(REPLACE "-Werror"  " " CMAKE_C_FLAGS   ${CMAKE_C_FLAGS})
endif()

if(CMAKE_CXX_FLAGS)
  string(REPLACE "-Wall"    " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
  string(REPLACE "-Werror"  " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
endif()

The problem is that these flags may have arguments (values). Since the replacement is simple, any such value is left hanging.

For instance debian packages are built with standard CFLAGS intended to improve security (build hardening), determined by dpkg-buildflags, for instance

CFLAGS=-g -O2 -ffile-prefix-map=/home/drew=. -fstack-protector-strong -Wformat -Werror=format-security

In cmake builds this is passed to CMAKE_C_FLAGS (and CMAKE_CXX_FLAGS etc). So adios2 gets built with -Werror=format-security. Then, once testing/install/CMakeLists.txt has had its way, the compilation line gets rendered with c++ -Wformat =format-security, which obviously fails. The error message is

cc: warning: =format-security: linker input file unused because linking not done
cc: error: =format-security: linker input file not found: No such file or directory

from trying to interpret =format-security on its own as a valid linker flag.

There are a handful of options for fixing it, depending on what the intention for removing -Werror was in the first place.
If it's just -Werror itself, add a tailing space "-Werror " (and likewise "-Wall "). If any -Werror is meant to be removed then use a wildcard "-Werror*" (or .*, whatever the wildcard syntax needs to be in this context)

A more robust solution might be to parse the flags individually, e.g. using add_definitions and remove_definitions.

Another option is to append -Wno-error rather than removing -Werror (likewise -Wno-all)

To Reproduce

  1. export CMAKE_CXX_FLAGS="-Werror=format-security"
  2. cmake configure, build
  3. ctest
  4. See error

Expected behavior
tests should pass. The names of CFLAGS should not be removed leaving their values hanging in place.

Desktop (please complete the following information):

  • OS/Platform: Debian unstable (linux)
  • Build gcc 13.2.0, cmake 3.27.2, shared library build
@vicentebolea
Copy link
Collaborator

@drew-parsons many thanks for reporting this. This indeed has been a nightmare which has probably caused issues with the generation of the adios2-config script needed for non-cmake applications to find adios2 includes and libraries.

Another option is to append -Wno-error rather than removing -Werror (likewise -Wno-all)

I think this is the best at this stage

@vicentebolea vicentebolea added the area: build Build issues label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build issues
Projects
None yet
Development

No branches or pull requests

2 participants