I think that -Werror makes sense for upstream development CI with
controlled dependencies and toolchains, but it is too strict and brittle
for downstream packaging, because insignificant warnings from new
toolchain or dependency versions tend to cause frequent unnecessary
breakage. The more complex the package and its dependencies, the truer
this is.
I suggest patching these to remove -Werror:
$ rg Werror
libphonenumber-8.13.52-build/libphonenumber-8.13.52/cpp/CMakeLists.txt
445: add_definitions ("-Wall -Werror")
libphonenumber-8.13.52-build/libphonenumber-8.13.52/tools/cpp/CMakeLists.txt
28: add_definitions ("-Wall -Werror")
You can try to fix the root causes of the warnings too, if you like, but
I don’t think a simple deprecation warning should be preventing you from
building libphonenumber. It’s good to look at warnings, but being
warning-free all the time across all branches is a lot to ask.
– Ben Beasley (FAS: music)
On 1/28/25 9:38 PM, Sérgio Basto via devel wrote:
On Tue, 2025-01-28 at 10:06 +0100, Jakub Jelinek wrote:
On Mon, Jan 27, 2025 at 07:31:35PM +0000, Sérgio Basto via devel
wrote:
I just want check, if I'm thinking correctly before submitting a
fix in
gtest package
The problem is on Rawhide I have this warning that make other
packages
fail to build [1]
gtest source [2] source get __cplusplus value and include <version>
or
#include <ciso646> depending on __cplusplus value .
On rawhide I got the warning on Fedora 41 don't but __cplusplus of
GCC
compiler is the same (201703)
My proposal is to change the comparison from 202002L to 201703L
-#if GTEST_INTERNAL_CPLUSPLUS_LANG >= 202002L && \
+#if GTEST_INTERNAL_CPLUSPLUS_LANG >= 201703L && \
That is not correct. <version> is actually not guaranteed to be
there for
C++17, it was only added in https://wg21.link/p0754r2 in 2018 (so
e.g. for
GCC since GCC 9.1). __has_include is supported by GCC since 2014 (so
GCC
5.1). So your change would break building with GCC 5.1 to 8.5.
I think if it did e.g.
#if (GTEST_INTERNAL_CPLUSPLUS_LANG >= 202002L && \
!defined(__has_include)) || \
(GTEST_INTERNAL_CPLUSPLUS_LANG >= 201703L && \
GTEST_INTERNAL_HAS_INCLUDE(<version>))
#include <version>
#endif
it would be better, C++20 should guarantee there is <version> (but
also
that __has_include is there, but this stuff attempts to cover also
the cases
of the incremental development of the standard features).
I don't really see the point of including <ciso646>, it is a useless
header
which doesn't contain anything since its introduction and has been
removed
without deprecation in C++20.
I think the rationale some people give is that it is the smallest C++
header
(contains nothing) which still includes some basic header with some
macros
(in the libstdc++ case it is <bits/c++config.h>, in libcxx case it is
<__config>). But e.g. in the libstdc++ case, that header doesn't
really
define any feature test macros, the __cpp_* macros are predefined by
the
compiler and __cpp_lib_* are defined by the individual headers which
provide
that functionality or (when it exists) in <version>.
And this header then has
#include <ctype.h> // for isspace, etc
#include <stddef.h> // for ptrdiff_t
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <cerrno>
// #include <condition_variable> // Guarded by GTEST_IS_THREADSAFE
below
#include <cstdint>
#include <iostream>
#include <limits>
#include <locale>
#include <memory>
#include <ostream>
#include <string>
// #include <mutex> // Guarded by GTEST_IS_THREADSAFE below
#include <tuple>
#include <type_traits>
#include <vector>
so I really don't see the point, including cerrno will include
everything
what ciso646 provided, cstdint as well, etc.
Or go with
// Detect C++ feature test macros as gracefully as possible.
// MSVC >= 19.15, Clang >= 3.4.1, and GCC >= 4.1.2 support feature
test macros.
#if GTEST_INTERNAL_HAS_INCLUDE(<version>) || \
(GTEST_INTERNAL_CPLUSPLUS_LANG >= 202002L &&
!defined(__has_include))
#include <version> // C++20 and later
#else
#include <cerrno> // Pre-C++20
#endif
and leave the #include <cerrno> later on.
Applied , thank you .
libphonenumber [1] doesn't build on rawhide because treats this warning
as errors
abseil-cpp-20240722.1 also have one header that emit this king of
warning.
in absl/hash/internal/hash.h line 33
// For feature testing and determining which headers can be included.
#if ABSL_INTERNAL_CPLUSPLUS_LANG >= 202002L
#include <version>
#else
#include <ciso646>
#endif
so , what you suggest ?
Thank you
[1]
https://koji.fedoraproject.org/koji/taskinfo?taskID=128578010
--
_______________________________________________
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue
