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

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

Closed
tim77 opened this issue Apr 12, 2021 · 6 comments
Closed

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

tim77 opened this issue Apr 12, 2021 · 6 comments

Comments

@tim77
Copy link

tim77 commented Apr 12, 2021

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
Copy link
Contributor

Already fixed by using system doctest in #43.

wf-config.spec diff
--- 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 WayfireWM/wf-touch@8974eb0

@tim77
Copy link
Author

tim77 commented Apr 12, 2021

@alebastr thank you! Updated package.

@tim77 tim77 closed this as completed Apr 12, 2021
@tim77
Copy link
Author

tim77 commented Apr 12, 2021

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.

@tim77 tim77 reopened this Apr 12, 2021
@alebastr
Copy link
Contributor

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.

long get_elapsed() const
{
using namespace std::chrono;
auto now = system_clock::now();
return duration_cast<milliseconds>(now - start_point).count();
}

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

@ammen99
Copy link
Member

ammen99 commented Apr 15, 2021

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.

long get_elapsed() const
{
using namespace std::chrono;
auto now = system_clock::now();
return duration_cast<milliseconds>(now - start_point).count();
}

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
Copy link
Author

tim77 commented Apr 21, 2021

Thank you. Built for f35 and all tests passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants