Pb_parsing_util was adding "packed" option for float repeated fields in proto3 syntax unconditionally. In previous implementation there was a single Pb_option module with last-write-wins semantics, after refactoring of options there was Pb_raw_option with compilation phase to Pb_option and different semantics: adding more "packed" field resulted in it being compiled to "packed": [true,true], as repetion of options in Protobuf means it's a destructured list. That unconditional overriding of option stopped working as expected, and as integration tests were not run during dune runtest, this went under the radars unfortunately when I was introducing options refactoring. I've added an explicit test which catches this problem, and separate commit which fixes this behavior (via new Pb_raw_option.add_or_replace which removes previous values if any).
@c-cube please take a look. I've verified that this fixes the test run and build on your branch with resurrection of integration tests.
c-cube
commented
5 months ago
Pb_parsing_utilwas adding "packed" option for float repeated fields in proto3 syntax unconditionally. In previous implementation there was a singlePb_optionmodule with last-write-wins semantics, after refactoring of options there wasPb_raw_optionwith compilation phase toPb_optionand different semantics: adding more "packed" field resulted in it being compiled to"packed": [true,true], as repetion of options in Protobuf means it's a destructured list. That unconditional overriding of option stopped working as expected, and as integration tests were not run duringdune runtest, this went under the radars unfortunately when I was introducing options refactoring. I've added an explicit test which catches this problem, and separate commit which fixes this behavior (via newPb_raw_option.add_or_replacewhich removes previous values if any).@c-cube please take a look. I've verified that this fixes the test run and build on your branch with resurrection of integration tests.