test/functional/test_image.py::TestCompareXLSXFiles::test_image58 failure on 32bit architectures

[AdrianBunk]

https://buildd.debian.org/status/fetch.php?pkg=libxlsxwriter&arch=armhf&ver=1.1.7-1%7Eexp1&stamp=1714910384&raw=0

=================================== FAILURES ===================================
______________________ TestCompareXLSXFiles.test_image58 _______________________

self = <test_image.TestCompareXLSXFiles testMethod=test_image58>

    def test_image58(self):
>       self.run_exe_test('test_image58')

test/functional/test_image.py:165: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/functional/base_test_class.py:60: in run_exe_test
    self.assertEqual(exp, got)
E   AssertionError: Lists differ: ['xl/[979 chars]" y="199752966000"/>', '<a:ext cx="304800" cy=[168 chars]Dr>'] != ['xl/[979 chars]" y="-2110496912"/>', '<a:ext cx="304800" cy="[167 chars]Dr>']
E   
E   First differing element 31:
E   '<a:off x="0" y="199752966000"/>'
E   '<a:off x="0" y="-2110496912"/>'
E   
E     ['xl/drawings/drawing1.xml',
E      '<?xml version="1.0" encoding="UTF-8" standalone="yes"?>',
E      '<xdr:wsDr '
E      'xmlns:xdr="http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing" '
E      'xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main">',
E      '<xdr:twoCellAnchor editAs="oneCell">',
E      '<xdr:from>',
E      '<xdr:col>0</xdr:col>',
E      '<xdr:colOff>0</xdr:colOff>',
E      '<xdr:row>1048572</xdr:row>',
E      '<xdr:rowOff>0</xdr:rowOff>',
E      '</xdr:from>',
E      '<xdr:to>',
E      '<xdr:col>0</xdr:col>',
E      '<xdr:colOff>304800</xdr:colOff>',
E      '<xdr:row>1048573</xdr:row>',
E      '<xdr:rowOff>114300</xdr:rowOff>',
E      '</xdr:to>',
E      '<xdr:pic>',
E      '<xdr:nvPicPr>',
E      '<xdr:cNvPr id="2" name="Picture 1" descr="red.png"/>',
E      '<xdr:cNvPicPr>',
E      '<a:picLocks noChangeAspect="1"/>',
E      '</xdr:cNvPicPr>',
E      '</xdr:nvPicPr>',
E      '<xdr:blipFill>',
E      '<a:blip '
E      'xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" '
E      'r:embed="rId1"/>',
E      '<a:stretch>',
E      '<a:fillRect/>',
E      '</a:stretch>',
E      '</xdr:blipFill>',
E      '<xdr:spPr>',
E      '<a:xfrm>',
E   -  '<a:off x="0" y="199752966000"/>',
E   ?                    ^^^^^  ^^^^
E   
E   +  '<a:off x="0" y="-2110496912"/>',
E   ?                   ++ ^^^  ^^^
E   
E      '<a:ext cx="304800" cy="304800"/>',
E      '</a:xfrm>',
E      '<a:prstGeom prst="rect">',
E      '<a:avLst/>',
E      '</a:prstGeom>',
E      '</xdr:spPr>',
E      '</xdr:pic>',
E      '<xdr:clientData/>',
E      '</xdr:twoCellAnchor>',
E      '</xdr:wsDr>']
=========================== short test summary info ============================
FAILED test/functional/test_image.py::TestCompareXLSXFiles::test_image58 - As...
======================== 1 failed, 759 passed in 14.15s ========================


50% tests passed, 1 tests failed out of 2

Total Test time (real) =  14.48 sec

The following tests FAILED:
	  2 - functional (Failed)
Errors while running CTest
make[1]: *** [Makefile:74: test] Error 8

The problem is the following code commit 31b3314 added in src/xmlwriter.c:

    lxw_snprintf(attribute->value, LXW_MAX_ATTRIBUTE_LENGTH, "%ld",
                 (long) value);

value is now a 64bit variable, but it is printed as long which is 32bit.

@hosiet FYI

[jmcnamara]

Thanks for the report. I'll look into it.

[hosiet]

I see changes in 42700ea , which has failed CI tests.

If the variable to be printed has fixed length (e.g., in64_t), the printf format string should use functionalities in <inttype.h> with the PRId64 macro. See https://stackoverflow.com/questions/9225567/how-to-portably-print-a-int64-t-type-in-c for more explanation. Using %ld or %lld are not portable across different platforms, and will cause build failure on one platform or another. Of course, we need to pay attention to whether lxw_snprintf() is compatible with such format strings, though.

In the meanwhile, the dirty hack of Ignore %lld warning with gcc in xmlwriter.c. in src/Makefile shall be removed.

[jmcnamara]

I see changes in 42700ea , which has failed CI tests.

That was just a WIP attempt before I went to bed. :-)

See https://stackoverflow.com/questions/9225567/how-to-portably-print-a-int64-t-type-in-c for more explanation.

I don't think any of those suggestions work with ANSI-C (whether I should still try to support that is a separate question).

Anyway, I went with a fix based on a double conversion/format. That will work in the integer range required for those large uint64_t offsets. The test case that was failing is more or less the maximum value for that attribute.

I tested it on a 32bit ubuntu system. Could you try it when you get a chance.

[hosiet]

Thanks, I will try it and the rebuild result for all architectures will be available in 24 hours.

[jmcnamara]

Thanks, I will try it and the rebuild result for all architectures will be available in 24 hours.

Ok. Let me know if you need it in a new release.

[jmcnamara]

I have added a 32bit github action to hopefully catch issues like this in future.

[hosiet]

Thanks, the new patch seems to have fixed the bug.

I don't mind having a new release; if you feel like postponing the release, I can always backport the patch only.

[jmcnamara]

I don't mind having a new release; if you feel like postponing the release, I can always backport the patch only.

I'm still accumulating fixes for a new release so it is probably best to work with the patch/fix for now. I'll let you know when there is an update.