kaitai-io / kaitai_struct_visualizer

Kaitai Struct: visualizer and hex viewer tool
https://rubygems.org/gems/kaitai-struct-visualizer
GNU General Public License v3.0
284 stars 25 forks source link

Remove legacy (pre-imports) feature of accepting multiple .ksy files #59

Open generalmimon opened 1 year ago

generalmimon commented 1 year ago

Currently, ksv's usage help looks like this:

$ ksv --help
Usage: ksv [options] <file_to_parse.bin> <format.ksy>...|<format.rb>

Note the ellipsis in <format.ksy>..., indicating that multiple .ksy files are accepted. Indeed, ksv does accept multiple .ksy specs - the first one is treated as the main one:

https://github.com/kaitai-io/kaitai_struct_visualizer/blob/3ec1dceaa8a275f97a45870a8caabf2f1db479a7/lib/kaitai/struct/visualizer/ksy_compiler.rb#L166-L170

I wondered what this is good for: personally, I've always only specified the root .ksy spec, which contained a /meta/imports section with all the dependencies. So I searched some old commits and discussions on this topic.

This commit from March 2016 introduced ksv's support for accepting multiple .ksy files:

https://github.com/kaitai-io/kaitai_struct_visualizer/commit/c5871c964cd2a1f22630a60756702b7ef58d7de7

Changed CLI arguments from "<format> <binary>" to "<binary>
 <formats>..." and implemented support for compilation and loading of multiple
 files

And these @GreyCat's comments from November 14, 2016 explain the purpose of this feature:

https://github.com/kaitai-io/kaitai_struct/issues/49#issuecomment-260286637

As for "having 50s of them in one file vs having 50 different files" - it's generally a matter of preference. Kaitai Struct allows you to have several different files for different structures and allows you to reference types in different files. If you'll be using ksv, then you can load up several ksy files at once into it like that: ksv your-binary.dat first-file.ksy second-file.ksy third-file.ksy, etc.

https://github.com/kaitai-io/kaitai_struct/issues/49#issuecomment-260289310

In the end all of those hacks would remain in one KSY file, right? At least as long as I want your compiler to do the reading work. Or is there any include/module approach I'm missing?

I guess I'm kind of answered that question, but let me elaborate some more. Right now, it's perfectly fine to have stuff like these:

meta:
  id: top_level
seq:
  - id: child
    type: child

and

meta:
  id: child
seq:
  - id: something
    type: u1 # or anything else

in 2 distinct files. It's not possible to separate a single type in 2 different types (at least now). If you feel like that we need to add some sort of include mechanism - let's discuss that - it shouldn't be that hard to implement. Right now we've implemented somewhat working IP networking stack using this approach alone (i.e. some files referencing types in other files).


If we combine this knowledge with 0.7 release notes, it all starts making sense:

2017-03-22: Kaitai Struct v0.7 released

  • New ksy features:

    • Type importing system: meta/imports can be used to import other types as first-class citizens in current compilation unit; "opaque types" are now disabled by default (see below)
  • Command-line compiler options:

    • --opaque-types=true to enable opaque types (disabled by default, i.e. using unknown type would be treated as error)

In short, the ksv's feature of accepting multiple .ksy files pre-dates version 0.7, when imports were added to Kaitai Struct. Before 0.7, the compiler always worked in the "opaque types" mode - if you referenced a type that wasn't defined in the current .ksy spec, it was assumed to be defined somewhere else (either in another .ksy spec, or directly implemented in the target language as an "opaque type" as we understand it today).

So when using a .ksy spec with other .ksy specs as dependencies, you were expected to know what these dependencies are and provide them all to ksv after the main spec. This approach would still work with current ksv, provided you opt into the opaque types mode (which is disabled by default):

opaque_external_type.ksy

meta:
  id: opaque_external_type
  ks-opaque-types: true
seq:
  - id: hw
    type: hello_world

hello_world.ksy

meta:
  id: hello_world
seq:
  - id: one
    type: u1
pp@DESKTOP-89OPGF3 MINGW64 /c/temp/kaitai_struct/tests/formats (serialization)
$ /c/temp/kaitai_struct/visualizer/bin/ksdump -f json ../src/term_strz.bin opaque_external_type.ksy hello_world.ksy
{
  "hw": {
    "one": 102
  }
}

However, I don't see any point in this feature anymore - to me, it's a legacy of pre-0.7 versions which had no imports. I think only one .ksy spec should be accepted (i.e. ksv [options] <file_to_parse.bin> <format.ksy>) and all dependencies on other .ksy files should be specified via /meta/imports - providing more than one .ksy file would cause an error.

What do you think?