Adding a generic build order rule

[Jehan]

Describe the bug

Currently meson has specific build order logics depending on the type of rule. On some rule, it may depend on input, or depends or the main source files for executable(), and so on. Unfortunately as soon as we get into a case not planned by meson developers, we are either stuck or forced to do ugly or bug-prone build logics.

I'd like to get a syntax to block some target build rule until some other target got built first. Maybe it could be something like:

if (wait(some_target))
  some_rule(options)
endif

To Reproduce

The use case we had today (in GIMP build):

1. We generate a file git-version.h with this rule:

  gitversion_h = vcs_tag(
    input : gitversion_h2,
    output: 'git-version.h',
    command: [ 'git', 'log', '-n1', '--date=format:%Y', '--pretty=%cd', ],
    replace_string: '@GIMP_GIT_LAST_COMMIT_YEAR@',
    fallback: 'unknown (unsupported)',
  )

2. On another meson.build, we generate gimp-plug-ins.rc from some template:

gimp_plugins_rc = configure_file(
  input : 'gimp-plug-ins.rc.in',
  output: 'gimp-plug-ins.rc',
  configuration: versionconfig,
)

3. Then we use gimp_plugins_rc in a few dozen meson.build like this:

  plugin_sources += windows.compile_resources(
    gimp_plugins_rc,
    args: [
      '--define', 'ORIGINALFILENAME_STR="@0@"'.format(plugin_name+'.exe'),
      '--define', 'INTERNALNAME_STR="@0@"'    .format(plugin_name),
      '--define', 'TOP_SRCDIR="@0@"'          .format(meson.source_root()),
    ],
    include_directories: [
      rootInclude, appInclude,
    ],
  )

What happens

The problem is that gimp-plug-ins.rc actually has an #include "git-version.h". So we don't need git-version.h to generate the .rc file, but the later is simply useless without the .h. Or to be more accurate: the build will fail if the .rc was built first. As meson kind of starts everything in the same time, it sometimes work, but sometimes doesn't. It's a race condition. We just had a CI build on a tag which failed whereas it was succeeding on the commit the tag is on (i.e. exact same code), just because of this race condition.

Expected behavior

I looked up documentation and I see that we could add depends: [ gitversion_h ], on each and every windows.compile_resources() calls. We have more than 30 of these. And we have another rc file which has the same issue, and finally we have other rc files generated from the first one. Basically we have a lot of these calls. We could all update them, but this is bug prone as hell. This should be taken care of on the rc generation level. Basically gimp_plugins_rc target should not be created before gitversion_h target. This is basic build order which makes sense (even though gitversion_h is not a dependency to build gimp_plugins_rc, it is definitely a dependency to use it, so it makes no sense to make gimp_plugins_rc available before gitversion_h).

This is why I am not asking to add a depends: option on configure_file() or or any other target type. Instead I propose a generic call to wait for another target. My proposed naming idea was a wait() call, though it could be anything else as long as it allows to create call orders when needed:

if (wait(gitversion_h))
  gimp_plugins_rc = configure_file(
  input : 'gimp-plug-ins.rc.in',
  output: 'gimp-plug-ins.rc',
  configuration: versionconfig,
)
endif

The fact is also that we can never take every special case into account. There will always exist some special build case where we need to wait for another target (for reason X or Y which matters to the project). So a generic wait() rule is really worth it as it would take care of all the remaining needs.

system parameters

• Is this a cross build or just a plain native build (for the same computer)?
  This was a cross-build but it doesn't matter for the requested feature.
• what operating system (e.g. MacOS Catalina, Windows 10, CentOS 8.0, Ubuntu 18.04, etc.)
  Build on Linux Fedora 33 for Windows, but once again, this is a generic feature so OS don't matter.
• what Python version are you using e.g. 3.8.0
  Python 3.9.0
• what meson --version
  0.55.3
• what ninja --version if it's a Ninja build
  1.10.1

[eli-schwartz]

What do you expect build.ninja to look like? configure_file output is generated during meson setup builddir/ and there are NO build rules to create it.

[Jehan]

What do you expect build.ninja to look like?

I don't actually know much (or anything) about ninja syntax itself, so it's hard to answer your question. Yet while I was looking for a possible way to make such order dependency with meson, I found this ninja docs: https://ninja-build.org/manual.html#ref_dependencies

Clearly the "Order-only dependencies" is what we want. In my example, the git-version.h would not trigger a rebuild of the .rc file, but no output based on these .rc should be rebuilt before the order-only dependency (git-version.h) is rebuilt when needed.
So yeah, I guess something based of the || dep1 dep2 syntax of ninja.

configure_file output is generated during meson setup builddir/ and there are NO build rules to create it.

Oh I see. Well it does make sense as here configure_file() does not actually need git-version.h to exist at build time. Then I guess it would be nice to have a concept of "virtual target". I.e. that the actual file might be generated at configure time but then the "target" should not be considered done until all its order dependencies are actually built first during the ninja step.

[jpakkane]

Sorry fatfingered a reply away, but basically there should be a depends kwarg on the resource compilation instead, so you only need to define the depend in the one place that actually needs it. It is typically better to be explicit as it leads to better parallelization. "Wait for random things to happen here" kind of things is problematic in that it scales very badly. One delay is ok, but ten is not. And, sadly, people have a tendency to try to fix their parallelism issues by adding random delays and pauses hoping for the best rather than fixing the core issue.

[Jehan]

basically there should be a depends kwarg on the resource compilation instead

What, no. This is like 30 years of good practice for dependency handling down the drain if you do this. A target should basically bring its own dependency and meson/ninja is therefore able to handle them perfectly and without any error. This is exactly for this reason for instance that libtool then pkg-config or similar tools were created on library-level: so that dependencies bring their own dependencies. So when you depend on libxyz, you don't have to know anymore libxyz dependencies, check and maintain a easily broken list by hand (which indeed breaks every time an update of libxyz changes its deps). Instead libxyz brings its own dependency list through pkg-config. Well here it's the same thing except it's not for external libs and it's at build-system level. We have an internal target and we know its dependencies, so we can just bring them over and have them always up-to-date. What you are proposing would be the same as if someone told me to stop using pkg-config and just maintain dependencies of dependencies lists!

Typically here I grepped and counted over 30 occurrences where we used this rc file. If we were to do it by hand, then what happens the day when we changed the dependencies? We redo each and every rule. Of course, this is something common in build system (maintaining source files dependant of each others), but when we have the opportunity to simplify this like here because the dependency is itself a rule target, why not take it?
Worse as it's a race condition bug, it doesn't break every time, so we may miss some, and we would randomly fix one here, one there (possibly for months, here for instance, it was the first time we realized this bug after months of usage because of this). Relying on "chance" because a race condition bug is rare is not a good thing in my book (I want all my code to rely on logics).

"Wait for random things to happen here" kind of things is problematic in that it scales very badly.

There is really nothing random here, not sure why you say this. We have a target which depends on another. So we should not use the dependent one before its dependency is built. This is like the base of software building. 🙂

And, sadly, people have a tendency to try to fix their parallelism issues by adding random delays and pauses hoping for the best rather than fixing the core issue.

You will always have people making crap build rules, you can't change this. But a lot of crap rules are made because the build system is lacking on the needed feature allowing to make the proper rule. Typically here, this is one such case. If we don't find a way to make our target depend on its actual dependency, then we will end up making very crappy build rules to work around this meson issue (I would even call this a bug).

I mean, just look at the case. It's not a "random delay or pause". We are not doing something random "hoping for the best". I looked in our build (when it failed), then saw the problem which is that one target depends on another. And currently there is absolutely no way in meson to create this dependency relationship. This is not random. It's build investigation, accurate diagnosis and search for an actual logics (which is: a dependency should not be used before its own dependencies are created).

So yeah, I guess something based of the || dep1 dep2 syntax of ninja.

Anyway I had a thought the other day regarding this doc I found about ninja's "Order-only dependencies". From what I seem to have understood what meson does at configure time, I think it should absolutely be able to create these order-only dependencies at configure time with such a syntax on every rule where such a wait dependency is used. It should be good and semantically correct.
But I would need to look a bit closer and won't have time before at the very least several days at least (or more, we'll see).

[jpakkane]

So we don't need git-version.h to generate the .rc file, but the later is simply useless without the .h

Ok, my bad, I missed this. In this particular case you should be able to do this:

rc_sources += windows.compile_resources(
  gimp_plugins_rc,
  args: [
    '--define', 'ORIGINALFILENAME_STR="@0@"'.format(plugin_name+'.exe'),
    '--define', 'INTERNALNAME_STR="@0@"'    .format(plugin_name),
    '--define', 'TOP_SRCDIR="@0@"'          .format(meson.source_root()),
  ],
  include_directories: [
    rootInclude, appInclude,
  ],
)
plugin_sources += [rc_sources, gitversion_h]

and it should do the right thing. This means "gitversion_h must exist when the sources generated by the resource compiler are compiled" whereas I was thinking of "gitversion_h must exist when the resource file is being generated".

[eli-schwartz]

The correct solution here, in general, is the existing declare_dependency() which can be used to create a dependency object that a) provides sources: for the .rc file, b) depends on the header file (also via sources, in this case, but can use dependencies or whatever floats your boat, if needed).

The dependency object will guarantee that any target depending on the .rc source as input, also depends on the header having been generated.

[jpakkane]

These two approaches are pretty much the same. You can choose either one depending on which one suits your project setup better.

[Jehan]

So we don't need git-version.h to generate the .rc file, but the later is simply useless without the .h

Ok, my bad, I missed this. In this particular case you should be able to do this:

rc_sources += windows.compile_resources(
  gimp_plugins_rc,
  args: [
    '--define', 'ORIGINALFILENAME_STR="@0@"'.format(plugin_name+'.exe'),
    '--define', 'INTERNALNAME_STR="@0@"'    .format(plugin_name),
    '--define', 'TOP_SRCDIR="@0@"'          .format(meson.source_root()),
  ],
  include_directories: [
    rootInclude, appInclude,
  ],
)
plugin_sources += [rc_sources, gitversion_h]

and it should do the right thing. This means "gitversion_h must exist when the sources generated by the resource compiler are compiled" whereas I was thinking of "gitversion_h must exist when the resource file is being generated".

I don't think this would work because git-version.h is not needed when the sources generated by the resource compiler are compiled, but indeed when the resource files themselves are generated (as you initially thought). At least that's how I interpret the error (which we still have regularly, randomly):

[60/2051] Generating gimp-plug-ins.rc with a custom command
FAILED: plug-ins/file-jpeg/build_windows_gimp-plug-ins.rc_gimp-plug-ins.o 
/usr/bin/x86_64-w64-mingw32-windres --define 'ORIGINALFILENAME_STR="file-jpeg.exe"' --define 'INTERNALNAME_STR="file-jpeg"' --define 'TOP_SRCDIR="/builds/GNOME/gimp"' -I./. -I../. -I./app -I../app build/windows/gimp-plug-ins.rc plug-ins/file-jpeg/build_windows_gimp-plug-ins.rc_gimp-plug-ins.o --preprocessor-arg=-MD --preprocessor-arg=-MQplug-ins/file-jpeg/build_windows_gimp-plug-ins.rc_gimp-plug-ins.o --preprocessor-arg=-MFplug-ins/file-jpeg/build_windows_gimp-plug-ins.rc_gimp-plug-ins.o.d
build/windows/gimp-plug-ins.rc:2:10: fatal error: git-version.h: No such file or directory
    2 | #include "git-version.h"
      |          ^~~~~~~~~~~~~~~
compilation terminated.
/usr/bin/x86_64-w64-mingw32-windres: preprocessing failed.

So unless mistaken, it looks like the good meson code would be:

  plugin_sources += windows.compile_resources(
    gimp_plugins_rc,
    args: [
      '--define', 'ORIGINALFILENAME_STR="@0@"'.format(plugin_name+'.exe'),
      '--define', 'INTERNALNAME_STR="@0@"'    .format(plugin_name),
      '--define', 'TOP_SRCDIR="@0@"'          .format(meson.source_root()),
    ],
    depends: [ gitversion_h ],
    include_directories: [
      rootInclude, appInclude,
    ],
  )

As I said, I really don't like this approach which feels like a huge stepback from advanced build system logics, because it means we must maintain a list of dependencies and always update it in 35 places (I wc -l/counted it!) , whereas a generated target by itself should have its own list of dependencies.

The correct solution here, in general, is the existing declare_dependency() which can be used to create a dependency object that a) provides sources: for the .rc file, b) depends on the header file (also via sources, in this case, but can use dependencies or whatever floats your boat, if needed).

The dependency object will guarantee that any target depending on the .rc source as input, also depends on the header having been generated.

This seemed a lot better because it let me imagine I could just do this and be done with it (because gimp_plugins_rc would flow back with its dependencies into all places where it's used):

gimp_plugins_rc_dep = configure_file(
  input : 'gimp-plug-ins.rc.in',
  output: 'gimp-plug-ins.rc',
  configuration: versionconfig,
)   
gimp_plugins_rc = declare_dependency(sources: [ gimp_plugins_rc_dep, gitversion_h ])

Then anything depending on gimp_plugins_rc would just depend on both the actual .rc file and the header being generated. Except it would indeed work for a target needing only a dependency, but in windows.compile_resources(), the dependency is used as a file:

  plugin_sources += windows.compile_resources(
    gimp_plugins_rc,
    args: [
      '--define', 'ORIGINALFILENAME_STR="@0@"'.format(plugin_name+'.exe'),
      '--define', 'INTERNALNAME_STR="@0@"'    .format(plugin_name),
      '--define', 'TOP_SRCDIR="@0@"'          .format(meson.source_root()),
    ],
    include_directories: [
      rootInclude, appInclude,
    ],
  )

So with my change, I now get this error:

../extensions/goat-exercises/meson.build:11:2: ERROR: Unexpected source type <InternalDependency null: True>. windows.compile_resources accepts only strings, files, custom targets, and lists thereof.

Therefore I would still need to use the actual dependency returned by configure_file() anyway (possibly both).

So I still think some command is needed to actually create a specific dependency order as needed.
Note that in my specific use case here, one may consider that the chances of dependency change are low, but I am not advocating something in relation to this specific case, but really as a good practice regarding dependency management for custom targets. Moreover it's not true that chances of change are low here, because these .rc files are dependencies in dozens of plug-ins in GIMP and could easily be used in more places as the software evolve. Current situation just leaves place for future bugs.

[eli-schwartz]

That's a pity, but, I wonder if windows.compile_resources() could be improved to accept declare_dependency() objects?

[Jehan]

That's a pity, but, I wonder if windows.compile_resources() could be improved to accept declare_dependency() objects?

Note that it's not the only one you'd have to change. While testing other variants, I saw among the 35 uses, we have a bunch like this one:

    plugin_rc = configure_file(
      input : gimp_plugins_rc,
      output: plugin_name + '.rc',
      copy: true,
    )

Which gets as error:

../../../../../../../dev/src/gimp/plug-ins/common/meson.build:159:4: ERROR: Inputs can only be strings or file objects

This means that you'd configure_file() to accept more types of objects. I know, I am annoying to repeat this 😛, but I really think a more generic "this depends on that" (whatever this or that are) would be a lot more useful as generic, rather than trying to cater to every possible case which will come up.

[Jehan]

Today I again had a similar issue. This time, I needed to change a XML file through xsltproc (using custom_target) then I wanted to simply do some basic search and replace on the result so I used configure_file. Unfortunately this didn't work because configure_file() a file or string on input, whereas custom_target returns CustomTarget:

ERROR: configure_file keyword argument 'input' was of type array[CustomTarget] but should have been array[File | str]

If instead I try to just hardcode the output name of the custom_target file result, or even just use result.full_path() (where result is the result of the custom_target) as input of configure_file, meson tells me:

File menus/dockable-menu.ui.in does not exist.

Sure! It is supposed to be built by the first step, but since I can't use the CustomTarget as input of configure_file, it looks like meson is unable to tell it's just a basic dependency rule and simply refuses to configure.

Anyway I could work around the issue by reversing the order of operations, i.e. doing a configure_file first, then a custom_target (since configure_file returns a file and custom_target accepts file as input). Then I encountered issues with xsltproc which was adding some property when an included XML file and the including XML file were not in the same directory, and GTK didn't like this property and crashed (yet I couldn't find an option to disable this behavior of xsltproc). So I ended up doing 2 configure_file file followed by one custom_target so that both XML files are in the same (build) directory.

Of course meson is not responsible for xsltproc limitations, but my point is that it could have been extra easy from the start if meson allowed me to do my first expected working dependency order. Instead I had to work around this by reworking the order and I spent more than 30 mins for something which should have taken 2 mins. Really I wish we could just have a rule to say "this depends on that, don't bother about types or whatever"! 😢

Simplicity first in other words.

[eli-schwartz]

I don't think this is a good example, because the problem here isn't about dependency ordering.

The problem is that configure_file runs during meson setup, analogous to ./configure, whereas custom targets run during ninja / make.

You cannot depend on build rules before the build finishes configuring. This is precisely why configure_file detects that you aren't allowed to do it and tells you that you can't do that -- you can only use source files or other files that exist during configuration (the output of another configure_file is a valid input, for example).

You tried to hack around this by casting CustomTarget to string but the underlying problem remained the same, so it still failed.

[Jehan]

Ah right. I had forgotten about this. It's a bit confusing. This being said, a build-time configure_file-style call would be nice.

[eli-schwartz]

It's possible to use xsltproc as a configure_file command, and it will then run during every configure step. Also force reconfigure if the source file is modified.

[eli-schwartz]

Yeah, that would be interesting. I've hacked around this in the past by using configure_file to produce a shell script with replaced parameters to sed. That sed script then runs at build time.

Implementing it in meson and guaranteeing it works on all platforms (internally running a python implementation) could be pretty useful and I'd be amenable to reviewing and merging such a thing. The main use case would be for files that need to be replaced with configured values, but which frequently change -- it's very awkward to reconfigure the entire project every time a data file is modified, worse when it's the main installable artifact that just needs the install prefix inserted at the top. :D