Skip to content

Draft: Turn liboct_parser into a Library

Alex Buccheri requested to merge liboct_parser/cmake_build into main

Building liboct_parser as a Library

This MR is primarily to gain feedback and help before proceeding further.

Octopus cmake currently uses the source files from the liboct_parser directory, directly. See src/basic/CMakeLists.txt

liboct_parser cannot be built as a standalone library because one function that it uses, varinfo_variable_exists, is defined in varinfo_low.c (Octopus source), and depends upon opt_type, var_type and vars. vars is another stupid global data structure.

liboct_parser also depends upon config.h header, which is generated by cmake or autotools, respectively.

DONE/Explored

  • Because varinfo_variable_exists is used by the parser, I've moved it to the parser, else there's a circular dependency
  • Because varinfo_variable_exists depends on opt_type, var_type and vars, I've moved the definitions of opt_type and var_type to a header file in liboct_parser. vars is forward-declared in the same header, but initialised (and presumably populated?) on the Octopus side. One needs to clarify if this works as intended.
  • Added CMakeLists.txt to build liboct_parser as a library

TODOs

  • Have cmake produce a config.h file as the Octopus build does. If liboct_parser is to be used as a standalone library, then its build system should generate it. I currently hack this by copy-pasting the generated config.h from my build (for testing purposes).
  • I am not passing GSL flags, as done in the autotools build. Maybe this is fine?
  • Correctly link to the library. I need some feedback on this. I have:
    • Included the additional .c source in basic/CMakeLists.txt, along with target_include_directories the liboct_parser dir such that cmake knows where the header file is. (Probably the wrong thing to do, or at least the wrong place for it). This was not sufficient for the linking, so I:
    • Added the add_subdirectory(liboct_parser) to top-level cmake
    • Linked all the utility binaries to liboct_parser - this is bad because I'm now treating oct parser as both part of the source (which I thought would just work) and a library, which is not built by Octopus's cmake.
    • Linking still fails on Octopus or Octopus lib.
  • Need to make the library installable, and export the correct headers
  • Have Octopus cmake build liboct_parser

Additional TODOs

  • Add a README to liboct_parser w.r.t. building
  • Add some basic unit tests - i.e. parse a file from a string, retrieve the values from the symbol table and assert on them

The main questions are:

  • a) How can I get the linking to work, by just including occ_parser source in Octopus source, as was previously done?
  • b) Then how best to build liboct_parser, and do the linking to the lib object.

@LecrisUT Is it easier to address a) first, and confirm the changes to the source (I am not a C programmer) are valid, or move straight to building as an external lib and linking Octopus to this?

@hmenke If you have any insight or are able to assist on this, it would also be very welcome.

Edited by Alex Buccheri

Merge request reports