search

kaitai-io / kaitai_struct

Kaitai Struct: declarative language to generate binary data parsers in C++ / C# / Go / Java / JavaScript / Lua / Nim / Perl / PHP / Python / Ruby
https://kaitai.io
3.95k stars 192 forks source link

Construct: the order of `Computed` fields matter #1106

Open Mingun opened 4 months ago

Mingun commented 4 months ago

Currently some tests on https://ci.kaitai.io/ are failed because of that fact that instance fields are sorted alphabetically (due to SortedMap used for storage) and generated in the same order.

Sorting of instances are good thing which will ensure stable output friendly for diffs, but Construct currently cannot calculate dependents inside of Computed fields and raises an error instead.

Although problem seems to be resolved on Construct side, it also could be useful to generate Computed properties in a topological order of dependencies.

generalmimon commented 1 month ago

it also could be useful to generate Computed properties in a topological order of dependencies.

No, the real cause of the problem is that Kaitai Struct instances are supposed to be lazy (see https://doc.kaitai.io/user_guide.html#_instances_data_beyond_the_sequence), but they are eager in the Construct target because KSC implements them using Computed, which is an eager construct.

This problem has been already discussed in https://github.com/kaitai-io/kaitai_struct/issues/377 - apparently, @arekbulski added Lazy to Construct more or less to support translating .ksy files to Construct, as far as I understand. In https://github.com/kaitai-io/kaitai_struct/issues/377#issuecomment-376446919, @GreyCat tried to use it, but it wasn't implemented yet.

Nowadays, I hope it has been implemented since then, because https://construct.readthedocs.io/en/latest/lazy.html claims "This feature is fully implemented".

GreyCat commented 1 month ago

Folks, I can also point to the elephant in the room — do we have capacity/interest to even support Construct target?

Looks like @arekbulski unfortunately paused his participation in the project, so maybe we should follow the suit and disable/remove Construct support?

generalmimon commented 1 month ago

@GreyCat:

Looks like @arekbulski unfortunately paused his participation in the project

In what project, Construct or KS? He seems to be still somewhat active in Construct: https://github.com/construct/construct/issues/1080#issuecomment-2155884855

I assume Construct is not dead, i.e. it is probably still used by people. https://pypistats.org/packages/construct can give some very rough idea (although you never know how much of that is bot traffic).

The development of Construct might be stopped, but according to https://github.com/kaitai-io/kaitai_struct/issues/377#issuecomment-772739764, that's supposedly because the project is "finished", which isn't inherently bad.

so maybe we should follow the suit and disable/remove Construct support?

I don't see any good reason to do that. In my view, "supporting" the Construct target doesn't cost us anything we don't want. We can keep it as it is now and some people may still find it useful. And if someone wants, they can improve it. Not that long ago, there were some some PRs by @Mimickal (https://github.com/kaitai-io/kaitai_struct_compiler/pulls?q=is%3Apr+is%3Aopen+construct+sort%3Aupdated-desc), which we haven't gotten to yet, but they show interest, and also @wbarnha seemed interested at some point (https://github.com/kaitai-io/kaitai_struct_compiler/pull/242#issuecomment-1518087780). Also https://github.com/kaitai-io/kaitai_struct_compiler/pull/256 by @MatrixEditor (which I assume was closed for the lack of our response).

(Speaking of hot candidates for removal, I think HtmlClassCompiler (-t html) should go first. The idea is OK, but the implementation is so preliminary that it arguably can't be useful to anyone.)

Mimickal commented 1 month ago

For some context, I was contributing to Construct support because at the time it was the only way to achieve serialization with Kaitai Struct. I've been away from the project that needed that for some time, but I've been keeping an eye on the space. As far as I can tell, there still isn't a great solution for data-driven struct generation like KSC anywhere.

Aside from the dev time needed to support it, I don't think there's a good case for removing Construct support from Kaitai until Kaitai's own Python generation supports serialization. Then at least there would be an alternative.