Test 150_tag_config_invalid_tags.txt on s390x architecture

[carnil]

Hi

In Debian we see a test case failure, strangely only on s390x:

Running test case: 150_tag_config_invalid_tags.txt
    Invoking: boxes -f 150_tag_config_invalid_tags.cfg -l
1,2c1,5
< boxes: 150_tag_config_invalid_tags.cfg: line 5: invalid tag -- tag4    , tag5
< boxes: 150_tag_config_invalid_tags.cfg: line 32: invalid tag -- Invalid1, 2invalid, invalid3-, -invalid4, in--valid5
---
> boxes: 150_tag_config_invalid_tags.cfg: line 32: invalid tag -- Invalid1
> boxes: 150_tag_config_invalid_tags.cfg: line 32: invalid tag -- 2invalid
> boxes: 150_tag_config_invalid_tags.cfg: line 32: invalid tag -- invalid3-
> boxes: 150_tag_config_invalid_tags.cfg: line 32: invalid tag -- -invalid4
> boxes: 150_tag_config_invalid_tags.cfg: line 32: invalid tag -- in--valid5
29c32
< tag1 (1) | tag2 (1) | tag3 (1) | tag6 (1)
---
> tag1 (1) | tag2 (1) | tag3 (1) | tag4 (1) | tag5 (1) | tag6 (1)
Error in test case: 150_tag_config_invalid_tags.txt (top: actual; bottom: expected)

https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=s390x&ver=2.3.1-1%7Eexp2&stamp=1732398167&raw=0
but not seen on other release architectures: https://buildd.debian.org/status/package.php?p=boxes&suite=experimental

Does this by chance ring a bell?

[tsjensen]

Aww, that's another one of these isolated cases where all other test cases are miraculously fine. 🤔

The only thing I could guess is that because it's on s390x, which I really don't know, maybe it has to do with endianness or something? The code in question uses the to_utf32() function to convert an ASCII character to UTF-32 in a way that assumes some things about byte order. And it's obviously the finding of the comma (,) which is faulty in this case.

On the other hand, all the other test cases are green ...

[pkern]

Aww, that's another one of these isolated cases where all other test cases are miraculously fine. 🤔

The only thing I could guess is that because it's on s390x, which I really don't know, maybe it has to do with endianness or something? The code in question uses the to_utf32() function to convert an ASCII character to UTF-32 in a way that assumes some things about byte order. And it's obviously the finding of the comma (,) which is faulty in this case.

On the other hand, all the other test cases are green ...

I'd try to put it into bytes[3] on big endian machines. But ultimately the question is how it is used. If it's used in a numerical context at all then yes, the code is purely little endian right now (stuffing the lowest bits into the first byte of a four byte sequence, instead of the last).

[carnil]

So specifically the issue is been introduced with 8a7bb80 ("Enable lexer and parser to handle UTF-8 config file #72") AFAIU and for cross-reference.

[carnil]

@tsjensen sorry for following up without code contribution: Do you have a suggestion on what do do here best? Excluding the test seems wrong to me as we have a general(?) issue with the UTF-8 support for big-endian architectures, correct? One option for Debian trixie would be that I otherwise just declare boxes as unsupported on big-endian architectures and let ftp-master team remove the builds on those and do not build it anymore. But I'm unsure TBH. boxes would be useful as well on those architectures even if we might have only little set of users.

Any opinions?

[tsjensen]

🐌 I'm a little slow in responding due to the holiday period. Thanks for being patient with me!

As far as I understand, the endianness thing is an as yet unverified hypothesis. Or do we already know the problem exists on all big-endian architectures? (I'm really just asking openly!)

Assuming that it's really endianness: I have no access to a big-endian system and anyway wouldn't know what it takes to properly be "endian-agnostic". So I would need some help there.

If you include the sunny-day tests and the unit tests, we have over 550 tests in total. One of them is failing. Since it's just one, I would say that excluding it is a tolerable risk, and certainly preferable to not having boxes at all on those architectures.

[carnil]

Hi @tsjensen

🐌 I'm a little slow in responding due to the holiday period. Thanks for being patient with me!

No problem at all, sorry if that came the wrong way to you. If so I apologise. My aim was just to check if you know already something on the matter.

As far as I understand, the endianness thing is an as yet unverified hypothesis. Or do we already know the problem exists on all big-endian architectures? (I'm really just asking openly!)

We have build failures in Debian on the big-endian architectures (only one is in official release, the others are in ports):
s390x, hppa, powerpc, ppc64, sparc64

https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=s390x&ver=2.3.1-1%7Eexp2&stamp=1732398167&raw=0
https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=hppa&ver=2.3.1-1%7Eexp2&stamp=1732405663&raw=0
https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=powerpc&ver=2.3.1-1%7Eexp2&stamp=1732398561&raw=0
https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=ppc64&ver=2.3.1-1%7Eexp2&stamp=1732398594&raw=0
https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=sparc64&ver=2.3.1-1%7Eexp2&stamp=1732398960&raw=0

Assuming that it's really endianness: I have no access to a big-endian system and anyway wouldn't know what it takes to properly be "endian-agnostic". So I would need some help there.

If you include the sunny-day tests and the unit tests, we have over 550 tests in total. One of them is failing. Since it's just one, I would say that excluding it is a tolerable risk, and certainly preferable to not having boxes at all on those architectures.

Actually we do not run all tests. Only the normal testsuite. Maybe I should runn all, and include tests for sunny-day tests and unit tests? :)

[tsjensen]

I see, so it appears to be endianness after all. Like I said, I have no access to a big-endian system and don't know what it takes to properly be "endian-agnostic". So I would need some help here.

About the tests: Yes, it certainly makes sense to run them all. It adds only a few seconds to the job runtime (5 seconds in our GitHub Action on Linux). Then we'd also be better equipped to judge whether excluding a test is reasonable.

[Gayathri-Berli]

Hi @tsjensen / @carnil ,

I investigated this issue and reproduced it in the Debian source.
Because it is endianness-specific, I used c = (ucs4_t)ascii; in the ucs4_t to_utf32(char ascii) function to ensure compatibility with both little- and big-endian systems. The test case 150_tag_config_invalid_tags.txt now passes, and everything appears to be in order. If you're okay, I can create a relevant PR.

[tsjensen]

@Gayathri-Berli That would be fantastic! 👍

[Gayathri-Berli]

Hi @tsjensen ,
Could you pls review the PR? I have tested it from my end. The build is passing.
#131

[tsjensen]

@carnil I've merged @Gayathri-Berli's fix. Can you retry? Keeping my fingers crossed! 🤞

[carnil]

@carnil I've merged @Gayathri-Berli's fix. Can you retry? Keeping my fingers crossed! 🤞

@tsjensen Confirmed! Cf. https://buildd.debian.org/status/package.php?p=boxes&suite=experimental an in particular https://buildd.debian.org/status/fetch.php?pkg=boxes&arch=s390x&ver=2.3.1-1%7Eexp3&stamp=1737598174&raw=0

@Gayathri-Berli thanks!

[Gayathri-Berli]

Thank you @tsjensen / @carnil 😊.

[tsjensen]

@carnil Can we close this issue, or is there anything else you need about this?

[carnil]

@tsjensen not it's fixed, we can close the issue.

Thanks a lot!

(FYI, it's now in Debian testing, cf https://tracker.debian.org/pkg/boxes)

[tsjensen]

Alright - let us know if we can help with anything.