Potential regression 1.1.4 -> 1.1.5: Test fails on big-endian architecture (Debian s390x)

[hosiet]

The build of libxlsxwriter previously succeeded on Debian s390x architecture, but it started to fail with version 1.1.5:

For an overview of build history, please visit https://buildd.debian.org/status/logs.php?pkg=libxlsxwriter&arch=s390x .

Example of failed build log: https://buildd.debian.org/status/fetch.php?pkg=libxlsxwriter&arch=s390x&ver=1.1.5-1%7Eexp2&stamp=1691020110&raw=0

For other architetures with failed build, please visit https://buildd.debian.org/status/package.php?p=libxlsxwriter&suite=experimental . Obviously 1.1.5 fails on all big-endian architectures (powerpc, hp-ux, sparc).

Since this looks like a regression, bisecting the code base may help.

[jmcnamara]

For a big endian system you will need to pass the USE_BIG_ENDIAN flag:

http://libxlsxwriter.github.io/getting_started.html#gsg_endian

I didn’t see that in the Cmake flags in the failed build (but I may have missed it because I’m traveling and reading this on my phone).

Could you try add that if it isn’t being used.

That doesn’t explain why it passed earlier but try that as a starting point.

Obviously 1.1.5 fails on all big-endian architectures (powerpc, hp-ux, sparc).

Since this looks like a regression, bisecting the code base may help.

Do l need a big endian system to reproduce this? As far as I remember you can’t use a big-endian VM guest on a little endian host. Is that correct or can you cross compile and run the tests on a little-endian system?

When I developed the USE_BIG_ENDIAN feature I used a ~20 year old Power PC mac Mini that I brought down from the attic. :-)

[hosiet]

Thanks. However, USE_BIG_ENDIAN is only present in hand-written Makefile, and was never placed in the logic of CMake:

-> % git rev-parse --short HEAD
4aaca54c
-> % cat CMakeLists.txt | grep ENDIAN | wc -l
0

My thought is that the control of using big-endian or not should not be configurable, but rather depends on the target build platform. With my personal preference, I would like to suggest the following patch to completely get rid of manual defining a macro (I release this code snippet under the same license as libxlsxwriter, BSD-2-Clause):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index fb5a2f5..8996c31 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -243,6 +243,13 @@ if(BUILD_TESTS)
     list(APPEND LXW_PRIVATE_COMPILE_DEFINITIONS TESTING)
 endif()

+# Define "LXW_BIG_ENDIAN" macro on big-endian architectures
+include(TestBigEndian)
+TEST_BIG_ENDIAN(LXW_TARGET_BIG_ENDIAN)
+if(LXW_TARGET_BIG_ENDIAN)
+    list(APPEND LXW_PRIVATE_COMPILE_DEFINITIONS LXW_BIG_ENDIAN)
+endif()
+
 file(GLOB LXW_SOURCES src/*.c)
 file(GLOB_RECURSE LXW_HEADERS RELATIVE include *.h)

Let me know whether that looks good to you. I am applying this patch for a Debian's trial build 1.1.5-1~exp3, and you will be able to see the build results within the next 24 hours at https://buildd.debian.org/status/package.php?p=libxlsxwriter&suite=experimental . If the patch is working, we should see that all big-endian builds pass again.

[jmcnamara]

Thanks for the patch. That is a better solution.

I have applied it to main.