Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contents broken for Go in latest released ksc v0.8 #528

Closed
lannybroo opened this issue Mar 8, 2019 · 7 comments
Closed

contents broken for Go in latest released ksc v0.8 #528

lannybroo opened this issue Mar 8, 2019 · 7 comments

Comments

@lannybroo
Copy link

lannybroo commented Mar 8, 2019

Trying to compile a .ksy that has a fixed contents field using ksc --target all or ksc --target go fails due to some issue with the go compiler.

Example broken_go.ksy:

meta:
  id: broken_go
seq:
  - id: magic
    contents: [0x44, 0x42, 0x2F]

Compiler output:

> ksc --version
kaitai-struct-compiler 0.8
> ksc --target go broken_go.ksy --verbose file
parsing broken_go.ksy...
reading broken_go.ksy...
... compiling it for go... 
List(ArraySeq(IntNum(68), IntNum(66), IntNum(32))) (of class io.kaitai.struct.exprlang.Ast$expr$List)
@KOLANICH
Copy link

KOLANICH commented Mar 9, 2019

kaitai-struct-compiler 0.8

Have you tried the current unstable version of the compiler?

@GreyCat
Copy link
Member

GreyCat commented Mar 9, 2019

golang support is still in relatively stages, even as of now in unstable version. Latest CI report shows that it passes only 41% of tests, which is actually pretty bad, although your particular use case now passes (see FixedContents test).

Unfortunately, there's been pretty quiet in golang compiler's land lately: last few updates to #146 were done by me, and I'm probably the person least interested in golang, as I don't use it in my everyday's life, I don't have any experience with it, etc. @lannybroo, if you feeling like you could contribute something to golang support, you'd be most welcome.

@lannybroo
Copy link
Author

lannybroo commented Mar 11, 2019

Thanks for the link to the CI report -- I had not realized that go is essentially not ready yet. I'd love to contribute but also don't have a good background in go (more interest in Construct, however). Perhaps the right approach for golang is to not include go in the ksc --target all command until it passes more useful percentage of the tests?

@GreyCat
Copy link
Member

GreyCat commented Mar 12, 2019

There is no ksc --all command. There is ksc -t all, which is 99.9% of time used for exactly for the purpose of these CI test runs — if we'll omit go from there, how would we test it?

Also, note that, unfortunately, @arekbulski stopped working on Construct: #377 (comment), so most likely Construct target will be phased out in some time :(

@lannybroo
Copy link
Author

Ah, I interpreted ksc --target all as the library's typical way of emitting trusted parsers for all of the supported languages once I have tested my .ksy in the language I happen to be developing with. As an end user of the library, I did not have the CI test run in mind.

@lannybroo
Copy link
Author

Closing this issue. I don't know that there are actual actions to take (other than perhaps not including unsupported/incomplete languages when someone runs ksc --target all).

@GreyCat
Copy link
Member

GreyCat commented Mar 14, 2019

KS is still an project under development, and there's no clear distinction between "unsupported/incomplete" vs "supported/complete". We can probably mark up some languages as "officially good" if they pass 100% of the tests, but then probably only Java & Ruby will be passing up to that mark — all other languages will have some percentages lower than that. This might seem terrible, but in reality if test pass rate is 85-90%, it will be still useful for some (or even many) people. Even at 41%, I know that some people have reported some success with go target.

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

No branches or pull requests

4 participants