Loading binary schemas to generate code files not supported?

[herrfrei]

I'd like to use binary schemas created from schema files for later re-generation of source files. My aim is to store binary data together with the bfbs files to reconstruct not only the data, but also the classes to deal with this data (e.g. in Python). Calling flatc with the bfbs file as argument does not work out of the box.

My test message looks like this:

namespace test;

table message {
  key: string;
  timestamp: float64;
  data: [ubyte];
}

root_type message;  

Then I generate the binary schema with flatc-orig.exe -b --schema test.fbs. This works and results in the test.bfbs file.

Running flatc.exe -c test.bfbs results in the following error message:

error: d:\consys_ws\cs_flatbuffers\src\test_flatbuffers\test.bfbs(1, 1): error: illegal character: code: 24

I looked at the source code and found the following lines in file flatc.cpp, function `FlatCompiler::Compile``:

      ...
      if (is_binary_schema) {
        LoadBinarySchema(*parser.get(), filename, contents); // <-- this works, the contents is parsed
      }
      if (opts.use_flexbuffers) {
        if (opts.lang_to_generate == IDLOptions::kJson) {
          parser->flex_root_ = flexbuffers::GetRoot(
              reinterpret_cast<const uint8_t *>(contents.c_str()),
              contents.size());
        } else {
          parser->flex_builder_.Clear();
          ParseFile(*parser.get(), filename, contents, include_directories);
        }
      } else {
        ParseFile(*parser.get(), filename, contents, include_directories);  // <-- here the contens is parsed again as text resulting in the error
        if (!is_schema && !parser->builder_.GetSize()) {
          // If a file doesn't end in .fbs, it must be json/binary. Ensure we
          // didn't just parse a schema with a different extension.
          Error("input file is neither json nor a .fbs (schema) file: " +
                    filename,
                true);
        }
      }

If I change the code to not parse the binary schema (which has already been successfully parsed), the generators work again. Is this change acceptable to create a PR? Or does it possibly affect the flatc compiler for other use cases?

Here are the changes I applied:

      ...
      if (is_binary_schema) {
        LoadBinarySchema(*parser.get(), filename, contents); 
      }
      if (opts.use_flexbuffers) {
        if (opts.lang_to_generate == IDLOptions::kJson) {
          parser->flex_root_ = flexbuffers::GetRoot(
              reinterpret_cast<const uint8_t *>(contents.c_str()),
              contents.size());
        } else {
          parser->flex_builder_.Clear();
          ParseFile(*parser.get(), filename, contents, include_directories);
        }
      } else {
        if (!is_binary_schema) {                                                         // added
          ParseFile(*parser.get(), filename, contents, include_directories);
          if (!is_schema && !parser->builder_.GetSize()) {
            // If a file doesn't end in .fbs, it must be json/binary. Ensure we
            // didn't just parse a schema with a different extension.
            Error("input file is neither json nor a .fbs (schema) file: " +
                      filename,
                  true);
          }
        }                                                                               // added
      }

[CasperN]

Its not currently supported but I'd like to make that happen. See #6428 for a roadmap that I'm hoping to push flatc towards. The TLDR is to decouple code generators from the parser by using the reflection/bfbs as an intermediate representation.

I'd be happy to review a PR though it probably should also be looked over by @aardappel too

[CasperN]

Note: You need to provide the flags you want as IdlOptions are not serialized into the bfbs.

[aardappel]

@CasperN we do support reading binary schemas like this.

Yes, that code looks buggy, it is parsing the binary file as text. An even simpler fix may be simply to add else after the if (is_binary_schema) block, as binary schemas and FlexBuffers have no reason to mix either.

While we're at it, I spot another potential bug, the parser.reset just above that code should probably also happen for binary schemas.

PRs welcome :)

[herrfrei]

Thank you both for this feedback. I had also thought at first of just inserting an else. But wasn't sure what effect that would have on reading flexbuffers for example.

@aardappel, your proposed changes work as desired for me, I'll prepare a PR.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.