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

Test for fake.ksy #8

Closed
wants to merge 3 commits into from
Closed

Test for fake.ksy #8

wants to merge 3 commits into from

Conversation

ManasviGoyal
Copy link
Collaborator

@ManasviGoyal ManasviGoyal commented Oct 20, 2023

I have added a test for the fake.ksy which generates the required awkward_array.

These commands reproduce the generated array:

g++ -fPIC src-fake-test/fake.cpp -o libfake.so ./build/cp310-cp310-manylinux_2_35_x86_64/kaitai_struct_cpp_stl_runtime/libkaitai_struct_cpp_stl_runtime.so -shared
 python src-fake-test/fake.py

The plan after this is as follows -

  • Make a PR to awkward to add the required functions in the LayoutBuilder.h
  • generate the C code (as we need the LayoutBuilder*) from AwkwardCompiler.scala (maybe in a separate file?)
  • modify the CMakeLists.txt to generate .so files for kaitai_struct_cpp_runtime and lib[ksy_name].so using the source and header files generated by the user after running kaitai-struct-compiler
  • There are still some fixes need in the scala code but that can be done separately from the above.

@ManasviGoyal ManasviGoyal changed the title Working test for fake.ksy Test for fake.ksy Oct 20, 2023
@ManasviGoyal
Copy link
Collaborator Author

It would be great if you all (@jpivarski @agoose77, @ianna) can have a look and give your feedback and suggestions on how to move forward with this.

@jpivarski
Copy link

Just a comment: code copied from dependencies like Awkward's header-only code and Kaitai's C++ runtime shouldn't be version-controlled. The generation process can make copies of these in a build directory, but that's transient. Eventually, the user might even want an option that cleans up (deletes) the working directory when it's done. (Not deleting it would be for debugging purposes.)

@ManasviGoyal
Copy link
Collaborator Author

ManasviGoyal commented Oct 20, 2023

Just a comment: code copied from dependencies like Awkward's header-only code and Kaitai's C++ runtime shouldn't be version-controlled. The generation process can make copies of these in a build directory, but that's transient. Eventually, the user might even want an option that cleans up (deletes) the working directory when it's done. (Not deleting it would be for debugging purposes.)

You mean the header files, right? I have added that so that I could test it independently (sincw I have made modifications to the LayoutBuilder. h. That would be generated with the CMake only and won't be added to the runtime.

I just added it in a separate branch so that I can get suggestions on how to structure the repository and what should go in the generated code and what not.

@agoose77
Copy link
Contributor

@ManasviGoyal my understanding of this PR is that it shows a hand-written Kaitai → Awkward example that will eventually be generated by kaitai?

At this stage I don't exactly know what you'd like for me to review. I'm totally happy to do a review, though! I can imagine that it might feel like there are lots of things to do here. Are you looking for suggestions on how to follow through your plan outlined above?

@ManasviGoyal
Copy link
Collaborator Author

@ManasviGoyal my understanding of this PR is that it shows a hand-written Kaitai → Awkward example that will eventually be generated by kaitai?

At this stage I don't exactly know what you'd like for me to review. I'm totally happy to do a review, though! I can imagine that it might feel like there are lots of things to do here. Are you looking for suggestions on how to follow through your plan outlined above?

Hi, I wanted to get some suggestion on the CMakeLists.txt and how the module will be generated based on the new plan.

@ManasviGoyal ManasviGoyal deleted the ManasviGoyal/test-fake.ksy branch October 27, 2023 08:52
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.

3 participants