glib-mkenums and glib-genmarshal support

[QuLogic]

This is a WIP; it's almost complete except for the problem noted below.

It adds gnome.mkenums and gnome.genmarshal (this namespace is a bit overloaded) which can convert to the enumeration and marshaller code from various sources.

As noted on IRC, when compiling with build within the source it compiles fine, but when compiling outside the source, it fails. It seems like the working directory of compilation for the generated file is different. Unfortunately, the latter is the form used by the tests so it's not quite there yet.

This is based on #712.

[QuLogic]

OK, it looks like glib-mkenums can use @basename@ instead of @filename@, but it requires glib 2.22; is it okay for the framework test to require glib 2.22? It's from 2009, so I'd guess so...

[QuLogic]

Rebased to get CI refreshed.

[tp-m]

Some quick drive-by bikeshedding: looks ok at first glance, seems to map the tool arguments pretty much 1:1 to the functions, so should work fine.

It might be possible to do it nicer though .. somehow (waves hands).

It might be possible to combine the header + body creation function calls into one single call for example? That might require returning an array with two targets, or perhaps we can still return a single one and it just depends on the two? That might be nicer usability-wise, but not sure if there are cases where it would not work.

Alternatively, for gnome.genmarshal() one could argue for separate gnome.genmarshal_{header,body}() instead of using gnome.genmarshal(..., body : true) and gnome.genmarshal(..., body : true).

[jpakkane]

Maybe something like the following:

marshallers = gnome.genmarshal('foobar',
  sources : 'foobar.list',
  install_header : true,
  install_dir : 'include/baz')

And then you would have:

marsh_c = marshallers[0]
marsh_h = marshallers[1]

[QuLogic]

The other annoyance is that if you pass body : true, you need to pass header : true, or else you get warnings about missing prototypes. This can be automated, of course.

If generating both, it'd be nice if the foobar.c file could #include <foobar.h>, but that would require an extra wrapper and I'm a bit against that.

[jpakkane]

Why would it require an extra wrapper? We just have to make sure the .h file is generated first and we can do that behind the scenes with depends.

[QuLogic]

How would you get the #include <foobar.h> at the beginning of the file without a wrapper?

It's not strictly necessary though, since the --header option places the prototypes in the right place. It's just a matter of duplicate information. Since it's automatically generated and relatively small, DRY is probably not as big of a concern.

[jpakkane]

I thought that include would be done automatically by the generator. Well never mind then.

[tp-m]

I'm not sure I understand why an #include <foo-enumtypes.h> would require an extra wrapper. Can't we just prepend that to the rest of the stuff passed to --fhead ? Or does this apply only to the template variant?

[QuLogic]

I was talking about glib-genmarshal; it doesn't have a --template or --fhead argument.

I didn't think you'd generate both header and source file for glib-mkenums from the same meson command since they might use different templates.

[QuLogic]

I've modified gnome.genmarshal to generate both header and body at the same time. This is a nice idea that provides some consistency with the gnome.compile_resources method.

[QuLogic]

How are these for some defaults?

        if is_header:
            defaults = {
                'fhead': '''#ifndef __{output}__
#define __{output}__

#include <glib-object.h>

G_BEGIN_DECLS
'''.format(output=output_filename.to_upper().underscorify()),
                'fprod': '''
/* enumerations from \"@filename@\" */
''',
                'vhead': '''GType @enum_name@_get_type (void);
#define @ENUMPREFIX@_TYPE_@ENUMSHORT@ (@enum_name@_get_type())
''',
                'ftail': '''G_END_DECLS

#endif /* __{output}__ */'''.format(output=output_filename.to_upper().underscorify())
            }
        else:
            defaults = {
                'fhead': '''{headers}
'''.format(headers),
                'fprod': '''
/* enumerations from "@filename@" */''',
                'vhead': '''GType
@enum_name@_get_type (void)
{
  static gsize id = 0;
  static const G@Type@Value values[] = {''',
                'vprod': '''    { @VALUENAME@, "@VALUENAME@", "@valuenick@" },''',
                'vtail': '''    { 0, NULL, NULL }
  };

  if (g_once_init_enter (&id)) {
    GType tmp = g_@type@_register_static ("@EnumName@", values);
    g_once_init_leave (&id, tmp);
  }

  return (GType) id;
}'''
            }

where is_header is some flag or other to determine the type of output (should we just make both?), output is just the output filename to get a reasonable guard, and headers are any headers needed to compile.

I'm not sure whether that casting thing that GStreamer does is needed.

[QuLogic]

Any one of them could be overridden and if template is specified, then it's just used directly.

[jpakkane]

I'm not very familiar with gir but these defaults start to look a bit too magical. Is it ok to leave them empty for now? We can add stuff later once people start using it and complaining.

[QuLogic]

From IRC, it's @thiblahute's contention that everyone writes the "same" template for mkenums (this isn't quite gir, btw.)

[jpakkane]

Sorry for the delay. It's looking good, but just to be sure: if this was merged as-is, would it be good enough to fulfill all requirements for GStreamer so they could drop their custom hack code? @tp-m?

[thiblahute]

Sorry for the delay. It's looking good, but just to be sure: if this was merged as-is, would it be good enough to fulfill all requirements for GStreamer so they could drop their custom hack code? @tp-m?

Yes, it should be good enough for us, could be enhanced but... :)

[tp-m]

Is there a reason we're not going for generating both header and body at the same time with mkenums as well?

[QuLogic]

Mostly @jpakkane isn't convinced?

[nirbheek]

I thought the contention was on 'everyone uses the same template'. I think we can and should generate both mkenum body and header at the same time.

[jpakkane]

If we merge the two then it would look something like this:

enum_arr = gnome.mkenums('enums.h',
  sources : 'meson-sample.h',
  header_template : 'enums.h.in',
  source_template: 'enums.c.in',
  install_header : true,
  install_dir : get_option('includedir'))

Seems reasonable to me and consistent with genmarshal. How do others feel?

Unrelated, install_dir should default to get_option('includedir'), right? Or should we check for it in this code and error out if it is not set?

Also, just to be sure, there is never need to install the source file? (and if there is one person in the world who needs to do that, they can use a custom install script)

[nirbheek]

First two arguments should be the header and the source code. I think mkenums should deduce which is the header filename and which is the source filename from the filename suffix so that people can put it in any order.

Also install_dir should indeed default to includedir, and install_header should default to false. No one ever installs the source file, but they do generate it in make dist. However, we don't support that yet so it doesn't matter.

[jpakkane]

First two arguments should be the header and the source code. I think mkenums should deduce which is the header filename and which is the source filename

Or, alternatively the first argument should be just the basename foo. We would then generate from that the filenames foo.c and foo.h, which would be simpler for users.

[nirbheek]

I was thinking maybe someone wants to not have the same basename for the header and body, but I can't think of a good reason for that, so w/e. Let's have it be the basename. :+1:

[QuLogic]

So we're going to require a template, then? Not using the argument form? Or add twice the number of arguments for header vs. source?

[tp-m]

Ah, right. That was the issue. Not sure, undecided :)

On a side note, I found a glib-mkenums use case I haven't seen before. Bit exotic imho, still haven't seen that in the wild, but might be interesting to check if we can cover it or not:

• https://git.gnome.org/browse/glib/tree/gio/tests/gsettings.c#n2605
• https://git.gnome.org/browse/glib/tree/gio/tests/enums.xml.template

[jpakkane]

To make things faster I created #817 which is this branch but mkenum converted into a single invocation as discussed. Please add your comments there. Thanks.

[QuLogic]

I pulled in the changes from #817 and then made it possible to support both forms. Basically, just don't require both templates to be specified. That way, we don't need to duplicate all of the optional arguments for each type; just specify one template at a time (cf. the second executable in the test.) If you don't specify any templates, then that's the same as generating everything from the arguments (cf. the third executable in the test.) The only "bug" there is that install_header applies to any type, but I'm not sure how to avoid that.

[nirbheek]

I think it's ok to rename that to install, and just let people do whatever they want with it. People likely won't set it accidentally anyway since you need to provide install_dir.

[nirbheek]

(for instance, people use it to generate gsettings enum xml schema files that are installed, and that's not a header)

[tp-m]

Sorry, what is the implication of the comment about the install parameter? That by default the .c files are installed as well?

[QuLogic]

install_header made sense when both c_template and h_template were both required. Now, if the user doesn't supply either template, install_header will apply even if the resulting file is a source file.

So I guess we generally should just allow install but ignore it for c_template.

[jpakkane]

Or we could keep install_header and ignore it for c_template. It is more self-documenting.

[TingPing]

Personally I think the install bits are not important, if you are installing headers you already have a list of headers.. just throw the output of these into it.

[jpakkane]

Ignoring the install_header this looks ok to me. Could someone with Gnome experience comment on whether this does everything that is needed to support real world Gnome stuff such as GStreamer?

[QuLogic]

You cannot just use install_headers (and possibly any of the other install* commands) on the result because it only accepts strings and without using the actual object, the dependency will be missing.

I only think it should not be named install_header because there was some mention of *.xml files being generated by this same program which I assume also need to be installed. Also keep in mind that neither type of template is required.

[TingPing]

You cannot just use install_headers (and possibly any of the other install* commands) on the result because it only accepts strings and without using the actual object, the dependency will be missing

That's a shame.

there was some mention of *.xml files being generated by this same program which I assume also need to be installed

Should only be installing a header...

Could someone with Gnome experience comment on whether this does everything that is needed to support real world Gnome stuff

I've used both mkenums and genmarhsal in 3 projects (which all just used templates) I've ported so far with success.

EDIT: Though I remember if you didn't specify the template it just threw an empty file out which was useless and should error or something?

[nirbheek]

You cannot just use install_headers (and possibly any of the other install* commands) on the result because it only accepts strings and without using the actual object, the dependency will be missing

That's a shame.

That's on purpose. install_headers only accepts static files. Everything else that generates files has its own install and install_dir kwargs for installing the generated files.

[TingPing]

Slightly off topic but what is the proper way to pass these around then?

foo_enums = gnome.mkenums(...)

# This fails, unresolved types
gnome.generate_gir(sources: foo_enums, ...)

# This works but has a missing dependency?
gnome.generate_gir(sources: ['foo-enum.h', 'foo-enum.c'], ...)

In general I think Meson needs to be stricter and more verbose about types.

[nirbheek]

gnome.generate_gir needs to accept CustomTarget as a valid source file type. It should also accept Generator types.

[jpakkane]

I'd say it is time to finally merge this. Whatever issues still remain can be fixed afterwards. Thank you to all involved, this has most likely been the longest reviewed commit in the entire project.

[QuLogic]

Note that I never changed the name from install_header in the end...

[nirbheek]

We should definitely change that I think. Can we not do a release till that is fixed?

[nirbheek]

Okay, crap, we already released... Can we do a brown-paper-bag release with that changed please.

[TingPing]

As I said earlier, nothing but a header should ever be installed, why is this a problem?