[Bug]: hardcoded _FORTIFY_SOURCE=2 conflicts with distribution presets

[bnavigator]

⚠️ Before submitting, please verify the following: ⚠️

• This is a bug, not a question or a configuration issue.
• This issue is not already reported on Github (I've searched it).
• Nextcloud Server and Desktop Client are up to date. See Server Maintenance and Release Schedule and Desktop Releases for supported versions.
• I agree to follow Nextcloud's Code of Conduct

Bug description

Uncovered by #4690, the compilation of the desktop client fails, because there are two (!) places in the cmake files where _FORTIFY_SOURCE=2 is hardcoded, but the next level _FORTIFY_SOURCE=3 is in the CFLAGS presets from the distribution.

See also https://bugzilla.opensuse.org/show_bug.cgi?id=1201070

Steps to reproduce

1. have -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 in the CFLAGS
2. try to build nextcloud desktop via CMake

Expected behavior

• Successful build with _FORTIFY_SOURCE=3
• The build system should not impose conflicts with custom environements

Which files are affected by this bug

src/CMakeLists.txt, make/modules/DefineCompilerFlags.cmake

Operating system

Linux

Which version of the operating system you are running.

openSUSE Tumbleweed 20220702

Package

Distro package manager

Nextcloud Server version

N/A

Nextcloud Desktop Client version

3.5.1

Is this bug present after an update or on a fresh install?

Fresh desktop client install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

Are you using an external user-backend?

• Default internal user-backend
• LDAP/ Active Directory
• SSO - SAML
• Other

Nextcloud Server logs

N/A

Additional info

Failing build log:

[    9s] [  6%] Building CXX object src/csync/CMakeFiles/nextcloud_csync.dir/vio/csync_vio_local_unix.cpp.o
[    9s] cd /home/abuild/rpmbuild/BUILD/desktop-3.5.1/build/src/csync && /usr/bin/c++ -DQT_CONCURRENT_LIB -DQT_CORE_LIB -DQT_DISABLE_DEPRECATED_BEFORE=0x000000 -DQT_MESSAGELOGCONTEXT -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_URL_CAST_FROM_STRING -DQT_USE_QSTRINGBUILDER -DUNICODE -DWITH_WEBENGINE=1 -D_UNICODE -Dnextcloud_csync_EXPORTS -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/build/src/csync -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/build/src/csync/nextcloud_csync_autogen/include -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/build -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/src -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/build/src -I/home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/std -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtCore -isystem /usr/lib64/qt5/mkspecs/linux-g++ -isystem /usr/include/qt5/QtConcurrent -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 -Wpedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -fPIC -fdiagnostics-color=always -fPIC -std=gnu++14 -MD -MT src/csync/CMakeFiles/nextcloud_csync.dir/vio/csync_vio_local_unix.cpp.o -MF CMakeFiles/nextcloud_csync.dir/vio/csync_vio_local_unix.cpp.o.d -o CMakeFiles/nextcloud_csync.dir/vio/csync_vio_local_unix.cpp.o -c /home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/vio/csync_vio_local_unix.cpp
[    9s] <command-line>: warning: "_FORTIFY_SOURCE" redefined
[    9s] <command-line>: note: this is the location of the previous definition
[    9s] In file included from /home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/vio/csync_vio_local_unix.cpp:31:
[    9s] /home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/vio/csync_vio_local_unix.cpp: In function 'int _csync_vio_local_stat_mb(const mbchar_t*, csync_file_stat_t*)':
[    9s] /home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/std/c_private.h:88:15: error: '_stat' was not declared in this scope; did you mean 'stat'?
[    9s]    88 | #define lstat _stat
[    9s]       |               ^~~~~
[    9s] /home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/std/c_private.h:128:24: note: in expansion of macro 'lstat'
[    9s]   128 | #define _tstat         lstat
[    9s]       |                        ^~~~~
[    9s] /home/abuild/rpmbuild/BUILD/desktop-3.5.1/src/csync/vio/csync_vio_local_unix.cpp:144:9: note: in expansion of macro '_tstat'
[    9s]   144 |     if (_tstat(wuri, &sb) < 0) {
[    9s]       |         ^~~~~~
[    9s] make[2]: *** [src/csync/CMakeFiles/nextcloud_csync.dir/build.make:303: src/csync/CMakeFiles/nextcloud_csync.dir/vio/csync_vio_local_unix.cpp.o] Error 1
[    9s] make[2]: Leaving directory '/home/abuild/rpmbuild/BUILD/desktop-3.5.1/build'
[    9s] make[2]: *** Waiting for unfinished jobs....
[    9s] make[2]: Entering directory '/home/abuild/rpmbuild/BUILD/desktop-3.5.1/build'

Workaround:

Index: desktop-3.5.1/cmake/modules/DefineCompilerFlags.cmake
===================================================================
--- desktop-3.5.1.orig/cmake/modules/DefineCompilerFlags.cmake
+++ desktop-3.5.1/cmake/modules/DefineCompilerFlags.cmake
@@ -45,15 +45,6 @@ if (${CMAKE_C_COMPILER_ID} MATCHES "(GNU
         set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fstack-protector")
     endif (WITH_STACK_PROTECTOR AND NOT WIN32)
 
-    if (CMAKE_BUILD_TYPE)
-        string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER)
-        if (CMAKE_BUILD_TYPE_LOWER MATCHES "(release|relwithdebinfo|minsizerel)")
-            check_c_compiler_flag("-Wp,-D_FORTIFY_SOURCE=2" WITH_FORTIFY_SOURCE)
-            if (WITH_FORTIFY_SOURCE)
-                set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wp,-D_FORTIFY_SOURCE=2")
-            endif (WITH_FORTIFY_SOURCE)
-        endif()
-    endif()
 endif (${CMAKE_C_COMPILER_ID} MATCHES "(GNU|Clang)")
 
 if (UNIX AND NOT WIN32)
Index: desktop-3.5.1/src/CMakeLists.txt
===================================================================
--- desktop-3.5.1.orig/src/CMakeLists.txt
+++ desktop-3.5.1/src/CMakeLists.txt
@@ -32,8 +32,6 @@ if(NOT MSVC)
 
   string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER)
   if(CMAKE_BUILD_TYPE_LOWER MATCHES "(release|relwithdebinfo|minsizerel)")
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -D_FORTIFY_SOURCE=2")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D_FORTIFY_SOURCE=2")
   endif()
 
   if (CMAKE_CXX_COMPILER MATCHES "Clang")

[mgallien]

@bnavigator I am curious why is this needed now ?
what is the rationale for the switch to _FORTIFY_SOURCE=3 ?

[mgallien]

we may still need to ensure that other community binary builds of desktop client are not done without a safe and proper use of _FORTIFY_SOURCE

[bnavigator]

Yes, openSUSE Tumbleweed changed their presets from _FORTIFY_SOURCE=2 to _FORTIFY_SOURCE=3:

https://lists.opensuse.org/archives/list/factory@lists.opensuse.org/message/55RQGTWGCLN4GSIDEDIWMQTVZ7UNITYX/

[bnavigator]

we may still need to ensure that other community binary builds of desktop client are not done without a safe and proper use of _FORTIFY_SOURCE

That's why I only mention the patch as work around. We can use it in the distribution, because we know that we have even safer flags already specified. It's obviously not suited for builds where you would end up without any flag.