WayfireWM / wf-config

A library for managing configuration files, written for wayfire
https://wayfire.org/
MIT License
18 stars 19 forks source link

test(types_test.cpp.o): Build error in Fedora 35 #45

Closed tim77 closed 3 years ago

tim77 commented 3 years ago

In Fedora 34 compiles fine, but build error in f34. GCC version basically identical - 11.0.1.

FAILED: test/types_test.p/types_test.cpp.o 
g++ -Itest/types_test.p -Itest -I../test -Iinclude -I../include -I/usr/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -std=c++17 -Wno-deprecated-declarations -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -MD -MQ test/types_test.p/types_test.cpp.o -MF test/types_test.p/types_test.cpp.o.d -o test/types_test.p/types_test.cpp.o -c ../test/types_test.cpp
In file included from /usr/include/signal.h:315,
                 from /usr/include/c++/11/csignal:42,
                 from ../test/doctest.h:3241,
                 from ../test/types_test.cpp:2:
../test/doctest.h:4403:45: error: size of array ‘altStackMem’ is not an integral constant-expression
 4403 |         static char             altStackMem[SIGSTKSZ];
      |                                             ^~~~~~~~
../test/doctest.h:4453:48: error: size of array ‘altStackMem’ is not an integral constant-expression
 4453 |     char    FatalConditionHandler::altStackMem[SIGSTKSZ] = {};
      |                                                ^~~~~~~~
alebastr commented 3 years ago

Already fixed by using system doctest in #43.

wf-config.spec diff ```spec --- a/wf-config.spec +++ b/wf-config.spec @@ -6,14 +6,17 @@ Summary: Library for managing configuration files, written for wayfire License: MIT URL: https://github.com/WayfireWM/wf-config Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz +# Fix build caused by glibc 2.34 SIGSTKSZ changes +# Fedora doctest package is already patched, but wf-config was using bundled copy +Patch0: %{url}/pull/43.patch#/wf-config-0.7.0-use-system-doctest.patch BuildRequires: gcc-c++ BuildRequires: meson BuildRequires: cmake +BuildRequires: cmake(doctest) BuildRequires: cmake(glm) BuildRequires: pkgconfig(libevdev) BuildRequires: pkgconfig(libxml-2.0) -BuildRequires: pkgconfig(wlroots) >= 0.12.0 %description %{summary}. @@ -40,6 +43,10 @@ Development files for %{name}. %meson_install +%check +%meson_test + + %files %license LICENSE %{_libdir}/lib%{name}.so.0* ```

Similar issue with wayfire build should be fixed by updating bundled wf-touch to https://github.com/WayfireWM/wf-touch/commit/8974eb0f6a65464b63dd03b842795cb441fb6403

tim77 commented 3 years ago

@alebastr thank you! Updated package.

tim77 commented 3 years ago

If you don't mind i'd reopen issue since we still have a problem with tests, but now on 32-bit arches like i686 and armv7hl.

+ /usr/bin/meson test -C i686-redhat-linux-gnu --num-processes 48 --print-errorlogs
ninja: Entering directory `/builddir/build/BUILD/wf-config-0.7.0/i686-redhat-linux-gnu'
ninja: no work to do.
 1/10 Types test          OK              0.03s
 2/10 OptionBase test     OK              0.03s
 3/10 Option test         OK              0.02s
 4/10 Option wrapper test OK              0.02s
 5/10 Section test        OK              0.02s
 6/10 XML test            OK              0.02s
 7/10 ConfigManager test  OK              0.02s
 8/10 Log test            OK              0.01s
 9/10 File parsing test   OK              0.31s
10/10 Duration test       FAIL            0.37s   exit status 1
>>> MALLOC_PERTURB_=147 /builddir/build/BUILD/wf-config-0.7.0/i686-redhat-linux-gnu/test/duration_test
――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
[doctest] doctest version is "2.4.5"
[doctest] run with "--help" for options
===============================================================================
../test/duration_test.cpp:12:
TEST CASE:  wf::animation::duration_t
../test/duration_test.cpp:19: ERROR: CHECK( duration.running() == false ) is NOT correct!
  values: CHECK( true == false )
../test/duration_test.cpp:20: ERROR: CHECK( duration.progress() == doctest::Approx{1.0} ) is NOT correct!
  values: CHECK( -9392801.5 == Approx( 1.0 ) )
===============================================================================
[doctest] test cases:  3 |  2 passed | 1 failed | 0 skipped
[doctest] assertions: 56 | 54 passed | 2 failed |
[doctest] Status: FAILURE!
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Summary of Failures:
10/10 Duration test       FAIL            0.37s   exit status 1
Ok:                 9   
Expected Fail:      0   
Fail:               1   
Unexpected Pass:    0   
Skipped:            0   
Timeout:            0   

Full log.

alebastr commented 3 years ago

Ah, that's a funny one: duration_t::impl::is_ready assumes that get_elapsed on freshly initialized object will return sufficiently large positive number. That works on 64-bit architectures, as now() - 0 fits into 64-bit long. That also fails on all 32-bit architectures as the value overflows 32-bit long type and becomes negative.

https://github.com/WayfireWM/wf-config/blob/4a699350a094e05754daa9d58a35287bbf7e03ad/src/duration.cpp#L33-L38

I guess get_elapsed should be returning std::chrono::milliseconds::rep.

ammen99 commented 3 years ago

Ah, that's a funny one: duration_t::impl::is_ready assumes that get_elapsed on freshly initialized object will return sufficiently large positive number. That works on 64-bit architectures, as now() - 0 fits into 64-bit long. That also fails on all 32-bit architectures as the value overflows 32-bit long type and becomes negative.

https://github.com/WayfireWM/wf-config/blob/4a699350a094e05754daa9d58a35287bbf7e03ad/src/duration.cpp#L33-L38

I guess get_elapsed should be returning std::chrono::milliseconds::rep.

Nice find, we could also just return int64_t, I am not a fan of such long types :)

tim77 commented 3 years ago

Thank you. Built for f35 and all tests passed.