Skip to content

Conversation

samysweb
Copy link

This pull request intends to make ProtoBuf.jl and its CI compatible to 32-bit architectures (x86).

For CI I tried to run ProtoBuf.jl on a 32-bit system and noticed a bug due to the way LengthDelimitedProtoDecoder implements endpos:
Int is platform-dependent in Julia, and hence this corresponds to Int32 in 32-bit systems.
Conversely, IOBuffer (what my test uses) uniformly returns Int64 for position independently of the platform.

This issue also becomes visible when activating the CI of ProtoBuf.jl for x86 although for a different reason (UInt32 not converted into Int32).
I propose to uniformly use Int64 as an endmarker and to perform explicit conversion (see suggested changes to Codecs.jl and decode.jl.
I've also updated the tests in test_protojl.jl to make the choice of Integer type explicit (which is lateron checked) and to account for the fewer allocations on x86 systems in test_perf.jl.

With these changes, the CI now runs flawlessly everywhere except for x86 Julia nightly.
Where a Dict seems to be encoded in the opposite order for Julia Nightly on x86 (?).
Note that two tests fail and that expected/observed outputs are swapped between the two tests...
I am open to fixing this remaining bug, but would require your guidance on whether this is an implementation bug or a limitation of the current test?

samysweb added 9 commits July 15, 2025 09:21
32-bit system file buffers still have a 64 bit position;
PipeBuffer positions are 32 bit (together with UInt32 this yields UInt32), but can be silently lifted to 64 bits.

This only affects the message_done check in Codecs.jl in line 20, but comparison between (U)Int32 and Int64 still works fine...
Copy link

codecov bot commented Jul 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.99%. Comparing base (79df484) to head (6370c6e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #275   +/-   ##
=======================================
  Coverage   93.98%   93.99%           
=======================================
  Files          25       25           
  Lines        3011     3012    +1     
=======================================
+ Hits         2830     2831    +1     
  Misses        181      181           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samysweb
Copy link
Author

As expected, the two failing tests concern nightly for x86, note that the two failing tests have swapped expectations:

encoded payload 1: Test Failed at D:\a\ProtoBuf.jl\ProtoBuf.jl\test\test_encode.jl:90
  Expression: elbytes == elexpected
   Evaluated: UInt8[0x0a, 0x01, 0x62, 0x12, 0x01, 0x61] == UInt8[0x0a, 0x01, 0x63, 0x12, 0x01, 0x64]
[...]
encoded payload 2: Test Failed at D:\a\ProtoBuf.jl\ProtoBuf.jl\test\test_encode.jl:90
  Expression: elbytes == elexpected
   Evaluated: UInt8[0x0a, 0x01, 0x63, 0x12, 0x01, 0x64] == UInt8[0x0a, 0x01, 0x62, 0x12, 0x01, 0x61]

I.e. elexpected of encoded payload 1 corresponds to elbytes of encoded payload 2 and conversely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant