new module "sourceset" to match source file lists against configuration data

[bonzini]

In QEMU a single set of source files is built against many different configurations in order to generate many executable. Each executable includes a different but overlapping subset of the source files; some of the files are compiled separately for each output, others are compiled just once.

Using Makefiles, this is achieved with a complicated mechanism involving a combination of non-recursive and recursive make; Meson can do better. However, because there are hundreds of such conditional rules, it's important to keep meson.build files brief and easy to follow. Therefore, this commit adds a new module to satisfy this use case while preserving Meson's declarative nature.

Configurations are mapped to a configuration_data object, and a new "source set" object is used to store all the rules, and then retrieve the desired set of sources together with their dependencies.

The test case shows how the source set object can be used either directly or via extract_objects, in order to satisfy both cases—that is, either share object files across targets or keeping them separate. In the real-world case of QEMU we would use two or more source set objects, and then do executable(..., sources: ... , objects: ...).

[bonzini]

Based on @dcbaker's comments on #1617, this should be changed to accept a dictionary instead of a configuration_data.

[bonzini]

@jpakkane I have the impression that you're not convinced. :) If you'd like, I can try and put together an almost-realistic example.

You can certainly achieve the same in pure Meson using arrays of dictionaries, not unlike the Foreach with a dictionary example. In this case you'd write something like

sourceset += [{'symbols': ['ECC'], 'sources': files('ecc.c')}]

as a replacement for

sourceset.add('CONFIG_ECC', files('ecc.c'))

and then use a foreach loop to extract the files and dependencies. However, from least to most severe:

• at 20-ish extra characters per line, the syntax would be more unwieldy. QEMU currently has about 1300 rules with 600 different conditions, which makes it important to find the right balance between terseness and clarity (I put this as least important only because clarity is a bit in the eye of the beholder);

• it would suffer from quadratic behavior. I tried timing 1300 += vs. 1300 sourceset.add(), and the cost was 300 ms vs 10 ms; it would be repeated many times during the build;

• I would have to copy over and over the add_all() and apply() loops since they are performed on various sourcesets at different stages. The loops would also be complicated by the need to validate free-form dictionaries, instead of relying on permittedKwargs.

[bonzini]

Now that #5119 is in, I cleaned up 218 source set custom target a bit.

[bonzini]

@jpakkane Could you please review at least the concept? It's the only remaining blocker for even considering switching QEMU to Meson.

[jpakkane]

Sorry, there has been a lot of stuff happening. I'll try to get something done about this tomorrow.

[jpakkane]

On a general level having a better ("declarative-y") way of getting sources based on external factors like here is something we definitely want. I have gone through this twice now and there are things that need changing but the thing has not yet "clicked" for me to know exactly what it should look like.

I'll need to think about this more. In the mean time here are some things that came to mind:

• Why does it return a Python dependency since this has nothing to do with Python. Should it not be a regular dependency object instead?

• This should not use flattening preventer. Having more than one type of arguments is confusing. We only permit it for build targets and even then only because it has exactly one string for the name, not several. Those should all be kwargs instead. It also makes it more readable and self-documenting.

• Is Source files object really needed? Would it be enough to return an array with the elements? (OTOH doing that broke in the Gnome module just recently)

[bonzini]

Why does it return a Python dependency since this has nothing to do with Python. Should it not be a regular dependency object instead?

It's just a cut-and-paste error in the documentation. all_sources returns simply a list of file objects

This should not use flattening preventer. Having more than one type of arguments is confusing. We only permit it for build targets and even then only because it has exactly one string for the name, not several. Those should all be kwargs instead. It also makes it more readable and self-documenting.

Fair enough. My reasoning is that add is going to be used tens or hundreds times in meson.build, it is going to be pretty self-explanatory what it does and there is some advantage in keeping the length of the lines to the minimum. However, I guess adding sources: to each line is not the end of the world and you're the boss anyway. ;)

Is Source files object really needed? Would it be enough to return an array with the elements?

It's needed because the configuration data affects both the source files and the dependencies. I'll rename the source files object to "source configuration".

[jpakkane]

I could not think of things that would obviously break so no point in keeping this blocked. In addition since it is a module, we can just create a new one. There are a few fixes though:

• the arguments should be kwarged, having two different types is confusing, thus this method should be marked as @noPosArgs
• the documentation should have a simple practical example along the lines of "we add these files with these settings and then when do X, only this subset of things comes out"

[bonzini]

Since I force pushed, you can use this link to compare the previous version with this one.

[jpakkane]

The thing that is still sticking out to my eye a bit is the cond keyword. It is abbreviated a bit too much. On the other hand condition seems a bit vague. Maybe something like based_on? Other suggestions for the name are gladly accepted. 😃

[bonzini]

I'll change it to when. Short but not abbreviated. Thanks for forcing me to think about it once more. ;-)

[jpakkane]

Sounds good to me.