Galera fails to build because it uses obsolete functions from check library

[hhorak]

The issue was reported in Fedora 33, where check component was upgraded to 0.15.2 already.

An example of the compilation issue:

galera/tests/data_set_check.cpp:139:14: error: too many arguments for format [-Werror=format-extra-args] 139 | "expected: %zu, got %zu", offset, dset_out.size()); | ^~~~~~~~

The relevant code is:

fail_if (dset_out.size() != offset, "expected: %zu, got %zu", offset, dset_out.size());

And it looks correct. However, further investigation revealed these issues: https://github.com/libcheck/check/issues/293 that led to this change in check-0.15.1 and further in check-0.15.2: https://github.com/libcheck/check/commit/7ac1fcbcefe8813e2a75388ec61e20a184ddc8c8 https://github.com/libcheck/check/pull/298/commits/82540c5428d3818b64d6a8aefb601e722520651f

where we can read: """ A fix proposed by heftig in github.com//issues/293 is to add a new NULL to the end of every fail call in the macro itself. For users of these APIs who do pass a message there will be a new warning about too many arguments. As the fail APIs are deprecated, this new warning is a reasonable trade-off, and can be avoided by switching fail calls to ck_assert* calls. """

So, I believe the correct thing to do is to fix all fail_if and fail_unless calls with respective ck_assert calls as suggested.

[hhorak]

An example of a partial patch that seems to be working:

diff --git a/galera/tests/data_set_check.cpp b/galera/tests/data_set_check.cpp
index 8ab8dd8..5452b5c 100644
--- a/galera/tests/data_set_check.cpp
+++ b/galera/tests/data_set_check.cpp
@@ -135,30 +135,30 @@ static void test_ver(gu::RecordSet::Version const rsv)

     // this should be allocated inside current page
     offset += dset_out.append (rout0.buf(), rout0.serial_size(), true);
-    fail_if (dset_out.size() != offset,
+    ck_assert_msg (dset_out.size() != offset,
              "expected: %zu, got %zu", offset, dset_out.size());

     // this should trigger new page since not stored
     offset += dset_out.append (rout1.buf(), rout1.serial_size(), false);
-    fail_if (dset_out.size() != offset);
+    ck_assert (dset_out.size() != offset);

     // this should trigger new page since previous one was not stored
     offset += dset_out.append (rout2.buf(), rout2.serial_size(), true);
-    fail_if (dset_out.size() != offset);
+    ck_assert (dset_out.size() != offset);

     // this should trigger a new page, since not stored
     offset += dset_out.append (rout3.buf(), rout3.serial_size(), false);
-    fail_if (dset_out.size() != offset);
+    ck_assert (dset_out.size() != offset);

     // this should trigger new page, because won't fit in the current page
     offset += dset_out.append (rout4.buf(), rout4.serial_size(), true);
-    fail_if (dset_out.size() != offset);
+    ck_assert (dset_out.size() != offset);

     // this should trigger new page, because 4MB RAM limit exceeded
     offset += dset_out.append (rout5.buf(), rout5.serial_size(), false);
-    fail_if (dset_out.size() != offset);
+    ck_assert (dset_out.size() != offset);

-    fail_if (1 != size_t(dset_out.count()));
+    ck_assert (1 != size_t(dset_out.count()));

     DataSetOut::GatherVector out_bufs;
     out_bufs().reserve (dset_out.page_count());

However, there are too many fail_if and fail_unless calls for me to fix this today. Hope somebody can pick this and create a complete patch :)

[ljavorsk]

This looks relevant for me, as I'm working on fixing this to galera too. Unfortunately, there is so many fail*() calls that this is going to be a really big patch.

So is there some progress from upstream in this, or is it new for you and you didn't know about this.

However, this is quite urgent in the Fedora release schedule, so it would be nice of you to let us know, how you want to deal with this issue. If we should help you somehow, or you can provide us a new release with this one fixed.

Thank you for any updates.

[temeo]

Hi,

We are aware of this issue. The fix is in progress and should be included in the next release.

[ljavorsk]

Thank you for letting us know.

One more thing, due to time pressure on our side, is there any way you can tell us when approximately we can expect the next release to be released?

Thanks

[temeo]

@ljavorsk Both 3.x and 4.x branches should be now up to date with the next release. The release tags have not been placed though, but there should not be any major changes anymore.

We have verified that the code builds on Fedora 33, but we'd appreciate any feedback if that is not so.

[temeo]

The fix was pushed in release_25.3.31 and release_26.4.6.