tests: Assert that byte-order is swapped on LE but not BE CPUs

[smcv]

Closes: #1392
Signed-off-by: Simon McVittie [email protected]

---

I'm still not sure I understand why the "obvious" code path through commit --add-metadata and show byteswaps the metadata - I would have expected that ostree would either define the metadata to be stored in a standardized byte order (in which case machines of the other byte order should byteswap during commit and byteswap back during show), or define the metadata to be stored in the host byte order of the machine that is expected to consume the tree, or store a byte-order indicator. But this does seem to work on both x86_64 (LE) and s390x (BE).

[cgwalters]

The byte order stuff is a mess; for core ostree we standardized on BE but that doesn't (necessarily) cover user-added metadata. We also screwed up on static deltas but that's mostly fixed now.

[cgwalters]

Nice, thank you very much! Keeping the test suite running is very much appreciated! I have a half-done WIP to run the ostree tests in the new Fedora test standard and it will greatly benefit from these fixes. Thanks again!

@rh-atomic-bot r+ 8e4a15d

[rh-atomic-bot]

⌛ Testing commit 8e4a15d with merge 994cd66...

[smcv]

for core ostree we standardized on BE but that doesn't (necessarily) cover user-added metadata

Sure, but if I put 42 in, shouldn't I get 42 out, without needing to know or care whether ostree stores it in BE, LE, JSON or hieroglyphs?

[cgwalters]

Sure, but if I put 42 in, shouldn't I get 42 out, without needing to know or care whether ostree stores it in BE, LE, JSON or hieroglyphs?

The problem is it's just one big variant...so we started doing byteswap-if-necessary for the core metadata like the timestamp, but that necessarily swaps the user metadata too.

We probably could teach show to byteswap-if-necessary just the core and display user metadata in a separate g_variant_print() operation but that would be a bit backwards incompatible.

[smcv]

Keeping the test suite running is very much appreciated

I wish we had more of it, but unfortunately Debian mostly provides build-time testing (make check) in a rather artificial environment, so I can't test anything that needs extended attributes, multiple users, root, etc. across all architectures.

The autopkgtest infrastructure runs GNOME-style installed-tests, but I can currently only do that on x86_64 (LXC on official infrastructure and qemu on my laptop) - there used to be arm64 LXC workers but those are currently unavailable.

[rh-atomic-bot]

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing 994cd66 to master...

[smcv]

We probably could teach show to byteswap-if-necessary just the core and display user metadata in a separate g_variant_print() operation but that would be a bit backwards incompatible.

It certainly feels as though something not right if ostree commit --add-metadata and show aren't inverses of each other - either both mapping host to network byte order if necessary, or neither doing so. I haven't looked into the underlying C APIs, but I feel as though each C API entry point ought to be able to document whether it expects network or host byte order (with the answer being host byte order 99% of the time). Is this a quirk of the CLI, or is the C API equally problematic?

(I actually think the existence of g_variant_byteswap() is a design flaw - I think g_variant_new_from_data(), g_variant_store() and similar functions should have taken an endianness parameter - but it's far too late for that now.)

Adding new, parallel APIs and deprecating the current ones might be a proportionate response. It's unfortunate that the serialized data structure on disk doesn't carry an indication of how it should be interpreted, but perhaps it isn't too late for rpm-ostree to start writing out metadata in a consistent endianness, with a flag to say it has done so?

[cgwalters]

Sorry, meant to follow up earlier. Yes...new API could make sense. And a consistent endianness marker.