Regression?

25 views
Skip to first unread message

Koen De Keyser

unread,
Feb 24, 2014, 8:26:11 AM2/24/14
to pi...@googlegroups.com
Hi all,

I've been moving to the most recent piqi / piqi-ocaml revisions (trying to rebase the changes I did on an older revision, before the split), and I now run into an issue that I did not have before. I've tried to nail it down to a simple case:

I have 2 .proto files. First one, called "id.proto" contains:

package test;
message Id
{
  required int64 id = 1; // Unique ID
}

Second one, called "example.proto" contains:

package test;
import "id.proto";

message example
{
  required Id id = 1;
}

I use piqi of-proto --normalize to generate the piqi files, and then piqic-ocaml --pp --multi-format the generate the _piqi.ml and _piqi_ext.ml files. This works fine.

Next, I've created a simple Ocaml program:

let x =  Example_piqi.default_example () in
Example_piqi_ext.print_example x

Compiling this works fine, however when running it I get this error: Fatal error: exception Piqi_common.Error(_, "imported piqi module is not found: "id"")

I've tried going through the Piqi code, and I noticed there is a global cache of modules which gets used when the _piqi_ext.ml file is evaluated (at start-up) during the parsing of the piqi value from the _piqi.ml file. I have the impression that it cannot find the "id" module at that time, and then will try to look (on disk) in some paths for the .piqi file, which is something that I did not expect (I would have though the .piqi files only need to be there at code-generation-time, not at runtime)

Any ideas on what is causing this?

thanks,

Koen

Anton Lavrik

unread,
Feb 25, 2014, 5:26:13 AM2/25/14
to pi...@googlegroups.com
Hi Koen,

Yes, this was a bug in piqilib that was introduced in one of the previous changes. The runtime shouldn't have tried to load the module from the filesystem as all necessary information is already compiled in as a part of _piqi.ml

I also made some other fixes and improvements in piqi and piqi-ocaml.

Thank you for bringing this up,

Anton



--
You received this message because you are subscribed to the Google Groups "piqi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to piqi+uns...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Koen De Keyser

unread,
Mar 27, 2014, 5:30:51 AM3/27/14
to pi...@googlegroups.com
Hi Anton,

I updated to the most recent piqi / piqi-ocaml and still ran into the same issue. Upon further investigation, I figured out that the issue is only triggered when I compiled the aforementioned example with ocamlbuild, and not when using a Makefile, because the makefile causes all object files to be linked in the executable, while ocamlbuild does an analysis of the source files and determines that "id_piqi_ext.ml" is never referenced. This causes the link step to omit the corresponding object file. The call to Piqirun_ext.init is thus never executed for "id". This triggers the exception when trying to use the extended functionality of modules that depend on it.

As a quick-and-dirty workaround, I modified piqi-ocaml to create a module alias in to the _piqi_ext module as well, i.e. "module Id_ext = Id_piqi_ext" in example_piqi.ml. This is sufficient for ocamlbuild to also link in these modules.

I've committed these changes on a branch in my forked repo:
- the example code: https://github.com/amplidata/piqi-ocaml/commit/09413b9d0b297817eac201ca3765bdf4398c1f89
- the workaround: https://github.com/amplidata/piqi-ocaml/commit/80b38d63c22ffd5b47f2c7003fd7c6e6d7014ac5

Koen

Anton Lavrik

unread,
Mar 28, 2014, 2:46:01 AM3/28/14
to pi...@googlegroups.com
Hi Koen,

Thank you for the detailed description. This bug creeped in when I added some optimizations what require linking all _piqi_ext.ml dependencies. This is because they initialize their Piqi modules that are now defined exclusively in respective _piqi.piqi () binaries instead of being duplicated as before.

I really like your solution. Only thing -- I added this import directly to _piqi_ext.ml instead of _piqi.ml. Just pushed this to master.

Let me know if you run into any other problems.

Anton


For more options, visit https://groups.google.com/d/optout.

Reply all
Reply to author
Forward
0 new messages