MsgPack.hpp include in omsgp.hpp.j2

[strilov]

The template omsgp.hpp.j2 includes a MsgPack.hpp file when an externally defined schema is used, https://github.com/brettviren/moo/blob/master/moo/templates/omsgp.hpp.j2#L8. Should the header include be msgp.hpp?

[brettviren]

Hi @strilov

The tcname jinja macro should be set to the name of the "record schema" for which the MessagePack serialization code is being generated. It is used to to locate the Struct.hpp and Nljs.hpp headers which are also generated from this record schema.

But, perhaps you are not asking about line 8 and instead:

https://github.com/brettviren/moo/blob/master/moo/templates/omsgp.hpp.j2#L39

??

If so, I don't know what msgp.hpp would refer to. Maybe a different MessagePack library?

[strilov]

Hi @brettviren,

Thanks for the reply.

The way I was reading it was that line 8 sets tcname to MsgPack and then this is used in line 24 to build the path to the external schema msgp header file.

For example if I'm building a msgp.hpp from a schema dunedaq.timing.timingrccmd, which relies on an external schema dunedaq.timing.definitions, defined in a separate file, I end up with the following snippet:

/*
 * This file is 100% generated.  Any manual edits will likely be lost.
 *
 * This contains functions struct and other type definitions for schema in
 * namespace dunedaq::timing::timingrccmd to be serialized via MsgPack.
 */
#ifndef DUNEDAQ_TIMING_TIMINGRCCMD_MSGPACK_HPP
#define DUNEDAQ_TIMING_TIMINGRCCMD_MSGPACK_HPP

// My structs
#include "timing/timingrccmd/Structs.hpp"

// MsgPack for externally referenced schema
#include "timing/definitions/MsgPack.hpp"

// We have ANY types so need to include NLJS serialization
#include "timing/timingrccmd/Nljs.hpp"

#include <msgpack.hpp>

However timing/definitions/MsgPack.hpp doesn't exist, because the omsgp.hpp.j2 template produces msgp.hpp files, rather than MsgPack.hpp files.

[brettviren]

Ah, okay. Thanks I don't have all this context loaded in my head. Now I half remember there was some hook related to "intrusive instrumenting" or messagepack?

@philiprodrigues maybe you remember the story here?

[philiprodrigues]

I don't remember the details, but I wouldn't be surprised if I never tested this particular case (with a dependent schema) when I wrote the code.

It looks like the fix is to make the omsgp.hpp.j2 filename compatible with the tcname, either by changing tcname so that lowercase(tcname) == "msgp" or by changing the filename of omsgp.hpp.j2 to omsgpack.hpp.j2. I think I prefer the latter, although only marginally. I'm not quite sure whether the need to do this comes from moo or from daq_cmake, which has this stanza:

https://github.com/DUNE-DAQ/daq-cmake/blob/928537656e544540cf0bcd6c6f1bca8a6067d6ef/cmake/DAQ.cmake#L234

But it's also true that lowercase(tcname) == template filename basename in onljs.hpp.j2 and ostructs.hpp.j2, so the requirement might be from moo?

Anyway, @strilov, could you try:

1. check out moo into your working area
2. rename moo/moo/templates/omsgp.hpp.j2 to moo/moo/templates/omsgpack.hpp.j2
3. install moo into your virtualenv with pip install -e . (in the moo top directory)
4. update your daq_codegen line in CMakeLists.txt to change the template name to MsgPack.hpp.j2

and let us know how it goes?

[brettviren]

Ah, thanks Phil. I guess I see the issue now.

Yes, matching file patterns in both the template and any external thing that decides file names (eg cmake) is something that requires coordination. In principle, this coordination must be done just once as cmake and template parts need not change frequently. But it is easy to make decisions in either context that messes up that coordination and easier if these two contexts are kept "distant" in terms of where their source lives.

Perhaps it is best we fork any *.j2 template files needed by DAQ and which are currently provided in moo into a DAQ-controlled repo to allow easier coordination.

[strilov]

Hi @philiprodrigues,

The recipe above works OK, the code compiles at least.

The implication is that the any code expecting the generated header to be called msgp.hpp has to be updated. On the other hand, this way, the template names are aligned. Things look good otherwise!

[alessandrothea]

Hi, I second @brettviren suggestion to maintain the templates required by the DAQ in DUNE-DAQ packages.
This is already happening for opmonlib schemas:

https://github.com/DUNE-DAQ/opmonlib/tree/develop/schema/opmonlib

And example of usage in
https://github.com/DUNE-DAQ/readoutlibs/blob/develop/CMakeLists.txt#L19

[philiprodrigues]

Stoyan tells me that the code which needs this functionality won't be going into dunedaq release 2.10, so I think that means we have time to work on moving the templates to a suitable DUNE-DAQ package.

So I suggest we close this issue and open a new issue in some DUNE-DAQ package (@alessandrothea any suggestion which package?) to copy the relevant templates there.

[alessandrothea]

Hi, what package includes msgpack directly? serialization? That would seem a good place where to put the templates.

[strilov]

Hi. The resulting msgpack headers would be included by the packages which have the N2Q/Q2N adapters, e.g. https://github.com/DUNE-DAQ/timinglibs/blob/develop/plugins/TimingHwCmdNQ.cpp. I guess those packages would also have a dependency on the package hosting the templates.

[strilov]

@alessandrothea @philiprodrigues In any case, I think serialization is probably the right package to host the templates, regardless of which packages directly include the resulting headers. I can go ahead and make the change if that sounds good?

[alessandrothea]

Sounds good to me.

[strilov]

Thanks. PR below if someone could take a look:

DUNE-DAQ/serialization#14