mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.33k stars 1.53k forks source link

Set project and meson options in cross/native files #6597

Closed dcbaker closed 3 years ago

dcbaker commented 4 years ago

This is a partial implementation of #3001, but not everything.

This series allows meson builtins and project options to be specified in the cross and/or native files. This should make it easier to set up subprojects in a reliable way, as all of the options can be kept in a native/cross file. With file layering a projects can even ship preconfigured options, that the end user can overlay with their own cross/native files.

For mesa we'd like to couple this with the ability to set some builtin options (namely default_library) on a per-subproject basis to control wraps in a predictable way.

dcbaker commented 4 years ago

@marc-h38, I think this was what you were wanting.

xclaesse commented 4 years ago

I haven't read the whole thing yet, but one thing I'm wondering is why do you split options in different sections?

I was expecting a simpler format, like the one we already use in <builddir>/meson-private/cmd_line.txt. For example:

[options]
prefix = '/opt/gnome'
default_library = 'both'
ffmpeg:default_library = 'static'
glib:gtk_doc = 'disabled'

That way you can load that into a single dict that would match exactly as if the user passed them with -D.

I think one common use-case is when you have only a cross file, and I think you should be able to set all options from there. But then what happens if you have both a cross file and a native file with options defined in both? I guess we should abort with an error if the same option is set in both files. That would be similar to what we currently do when passing both --default-library and -Ddefault_library. Of course build. options must only be in cross file.

dcbaker commented 4 years ago

@xclaesse I really want to get rid of the one off format in cmd_line.txt and replace it with a native/cross file.

I haven't read the whole thing yet, but one thing I'm wondering is why do you split options in different sections?

Composition, for one. cross and native files can be composed by passing the argument more than once. Since each component is it's own section you can re-use a cross file in more than one build. Say you need zlib in two different projects, and you want to statically link it in both cases, now you can have a single zlib file.

For meson builtin versus project arguments it makes sense to split them as well, because they're conceptually different. Project arguments by definition apply only to one project, meson builtins (at least generally) apply to all projects, so being able to specify them separately makes sense to me.

But then what happens if you have both a cross file and a native file with options defined in both?

This is clearly defined. If you have both a cross and native file project options from the native file are ignored, meson options that have a per-machine value like pkg_config_path are read from the native-file and applied to the build machine. values from the cross file are applied to the host arguments.

jpakkane commented 4 years ago

I'm so glad you posted this, because I was going to start working on it myself today. :)

It would be great to get all functionality needed for configuration so we can merge #6595 to get a general solution for all options rather than a one-off feature for one of them. One possible syntax change comes to mind. Rather than doing this:

[meson options]
c_std = 'c99'
[zlib:meson options]
default_library = 'static'

could we instead do this:

[meson options]
c_std = 'c99'
zlib:default_library = 'static'
dcbaker commented 4 years ago

I thought about that, but I really dislike it because it means you have to repeat yourself a lot if you have more than one options:

[meson options]
zlib:foo = 'bar'
zlib:bar = 'foo'
zlib:thing = false

vs

[zlib:meson options]
foo = 'bar'
bar = 'foo'
thing = false

And on the implementation side this makes things easier because we know what subproject we're in when we're applying defaults, so we load the right options from the start and don't have to loop over them asking "are you a zlib option?"

It would be great to get all functionality needed for configuration so we can merge #6595 to get a general solution for all options rather than a one-off feature for one of them

I've based this on @xclaesse's series, I'm not sure exactly what parts you don't like, but all of the machinery he put in place except in the interpreter changes is needed to make per-subproject builtins work. The language options are more complicated because they're generated dynamically. I have a branch for that, but there are still bugs and no tests. It's more complicated because, again, language options are added when a language is added to meson, we don't by default have c_std and friends. It also exposes all of the language args, and I'm not sure that's what we really want.

jpakkane commented 4 years ago

I'm not sure exactly what parts you don't like, but all of the machinery he put in place except in the interpreter changes is needed to make per-subproject builtins work

Per-project values of options should only be settable via the native/cross file. There are usually more than one of these, so concentrating them all in one place makes it easier to review.

dcbaker commented 4 years ago

Per-project values of options should only be settable via the native/cross file. There are usually more than one of these, so concentrating them all in one place makes it easier to review.

I think we should be able to set them via the command line. We allow setting project options on the command line, and we're going to get no end of complaints about -Dsub:foo=c works but -Dsub:c_std=c99 doesn't.

nirbheek commented 4 years ago

We allow setting project options on the command line, and we're going to get no end of complaints about -Dsub:foo=c works but -Dsub:c_std=c99 doesn't.

It's also going to be very inconsistent to not be able to set arguments on the command-line like that. We already have a bunch of unavoidable inconsistency in our argument handling, let's not increase it.

Setting arguments on the command-line vs a file is also much clearer for CI purposes since it's right there in the yaml file, and it shows up in the CI logs for debugging.

scivision commented 4 years ago

Would this make it possible to set the Ninja binary location as well in a native file, to avoid use of NINJA or PATH environment variable?

dcbaker commented 4 years ago

@scivision That would be a separate change, because I don't think we need any infrastructure changes for that, we just need to find ninja using the find_program implementation.

xclaesse commented 4 years ago
[meson options]
c_std = 'c99'
zlib:default_library = 'static'

I still strongly prefer that syntax over splitting into different sections. We use the subproject:opt syntax everywhere, it's consistent with command line options and with output of meson configure. It also can trivially be parsed into a dict used as default values to options.cmd_line_options, we already have that code when we parse cmd_line.txt.

I also do not agree this supersede cmd_line.txt, we still have to remember options set from command line and from meson configure -Dfoo=bar. Both concepts are complementary, and IMHO should share as much code and file format as possible.

Ultimately what I would like to see happening is projects shipping a set of cross files with their source code, that contains default values to build for specific targets (e.g. Android NDK, Linux distro mingw, etc). That often means specific values for a set of options, but still allowing them to be overriden from command line by the user.

xclaesse commented 4 years ago

Also, nitpicking, but I don't think we need "meson" in [meson options], we know it's a meson file. We don't have [meson binaries], etc.

marc-h38 commented 4 years ago

Also, nitpicking, but I don't think we need "meson" in [meson options], we know it's a meson file.

While pretty much everything around here is meson-related and the term "meson" is almost as vague and overloaded as "build", I suspect [meson options] was a relatively specific hence useful reference to meson_options.txt, was it not?

xclaesse commented 4 years ago

I suspect [meson options] was a relatively specific hence useful reference to meson_options.txt, was it not?

meson_options.txt only contains project options, so that would be a misleading reference since in the current proposal [meson options] actually does NOT contain project options, afaiu.

marc-h38 commented 4 years ago

meson_options.txt only contains project options,

Mmmm... so this PR wants to add [project options] and [meson options] and out of these two the one that is more closely related to meson_options.txt is... [project options]?!

meson_options.txt only contains project options,

... yet meson_options.txt is not mentioned in https://mesonbuild.com/Reference-manual.html#project , only in https://mesonbuild.com/Build-options.html ?!

Am I really the only one worried about the concepts and terminology? As a sanity check, do you guys try from time to time to discuss these terms and concepts with developers not familiar with meson?

xclaesse commented 4 years ago

Am I really the only one worried about the concepts and terminology? As a sanity check, do you guys try from time to time to discuss these terms and concepts with developers not familiar with meson?

I'm glad you volunteer to improve our documentation, there is certainly a lot to do :D

marc-h38 commented 4 years ago

@xclaesse to a reasonable extent I do volunteer to improve the documentation, in fact I already started that a little bit in other places. However no one can improve the documentation if developers use the terms "meson options" and "project options" in one place but vice-versa in another (really?), in that case it's a lost cause. Before writing any documentation, the design must have clearly defined concepts and language first. Super vague words like "build" and "meson" are not good names to start with. I also volunteer to find and suggest names (if not too late) but the high-level concepts must be summarized in a few sentences first, otherwise the concepts are flawed anyway.

xclaesse commented 4 years ago

I never checked what the doc says exactly. From meson configure there are those terms: Core options, Backend options, Base options, Compiler options, Directories, Testing options, and project options. core+directories+testing are often referred as builtin options (that's how they are named in the code) because those options are always present.

meson_options.txt are project options because they are the options defined by the project, and not defined by meson itself.

dcbaker commented 4 years ago

I still strongly prefer that syntax over splitting into different sections. We use the subproject:opt syntax everywhere, it's consistent with command line options and with output of meson configure. It also can trivially be parsed into a dict used as default values to options.cmd_line_options, we already have that code when we parse cmd_line.txt.

And I'm strongly in favor of having separate sections for each project. I think it makes sense because of composition (ie, mesa will probably ship a config file for just zlib for windows, but unless you're doing a windows->windows build you'll need to compose it with another cross file) and it makes it clear, at a glance, exactly what projects are being modified by the file.

Also, nitpicking, but I don't think we need "meson" in [meson options], we know it's a meson file. We don't have [meson binaries], etc.

How about "builtin options" then? I'd like to make it clear that one section is for meson level arguments, and the other is for options defined in your meson_options.txt, which I guess should be called "build options" as it is in the docs?

xclaesse commented 4 years ago

And I'm strongly in favor of having separate sections for each project. I think it makes sense because of composition (ie, mesa will probably ship a config file for just zlib for windows, but unless you're doing a windows->windows build you'll need to compose it with another cross file) and it makes it clear, at a glance, exactly what projects are being modified by the file.

Composition works just fine with a single section, they will be merged.

Also, nitpicking, but I don't think we need "meson" in [meson options], we know it's a meson file. We don't have [meson binaries], etc.

How about "builtin options" then? I'd like to make it clear that one section is for meson level arguments, and the other is for options defined in your meson_options.txt, which I guess should be called "build options" as it is in the docs?

The simple fact that it's hard to name is already a proof we should not split options, because whatever name we pick it will not be understood by the user without reading long documentation. If we insist on splitting options, it should at least follow the naming we have in meson configure, so I would call them [core options] and [project options]. For subproject options maybe e.g. [zlib options].

dcbaker commented 4 years ago

Composition works just fine with a single section, they will be merged.

Certainly, but if you have three files

[zlib:builtin options]
c_std = 'c99'
c_args = ['-someopt']
[expat:builtin options]
c_std = 'gnu99'
[meson options]
c_std = 'c11'
cpp_std = 'c++11'

It's super clear what is being done in each one, the first only only affects zlib, I can see that just by reading the section header, the second one only affects expat, and the thrid only affects the primary project. compare that to:

[builtin options]
zlib:c_std = 'c99'
zlib:c_args = ['-someopt']
[builtin options]
expat:c_std = 'gnu99'

Where I can't tell at a glance that a file affects more than one project, but lets have a bigger more complicated example:

[zlib:builtin options]
c_std = 'c99'
c_args = ['-foo', '-bar', 'something else']
b_ndebug = true
b_lto = false

[expat:builtin options]
c_std = 'gnu99'
c_args = ['-foo']
c_link_args = ['-Wl,--arg']
b_ndebug = true

[builtin options]
b_ndebug = true
c_std = 'c11'

vs

```ini
zlib:c_std = 'c99'
zlib:c_args = ['-foo', '-bar', 'something else']
zlib:b_ndebug = true
zlib:b_lto = false
expat:c_std = 'gnu99'
expat:c_args = ['-foo']
expat:c_link_args = ['-Wl,--arg']
b_ndebug = true
c_std = 'c11'

The first way is much more readable to me, and it forces you to keep per-project options sorted, you would end up with

[builtin options]
c_std = 'c99'
zlib:b_ndeug = false
c_args = ['foo', 'bar']
c_link_args = ['-Wl,-something']
expat:c_std = 'c11'
zlib:c_args = ['-foo']
expat:b_ndebug=false

The simple fact that it's hard to name is already a proof we should not split options, because whatever name we pick it will not be understood by the user without reading long documentation. If we insist on splitting options, it should at least follow the naming we have in meson configure, so I would call them [core options] and [project options]. For subproject options maybe e.g. [zlib options].

That's a terrible argument. Naming things is one of the hardest things in computer science, by that logic no software should be written at all!

But I don't think we're going to solve this by arguing about it anymore, clearly I like blue bikesheds and you like yellow ones. Maybe some other people have opinions?

xclaesse commented 4 years ago
zlib:c_std = 'c99'
zlib:c_args = ['-foo', '-bar', 'something else']
zlib:b_ndebug = true
zlib:b_lto = false
expat:c_std = 'gnu99'
expat:c_args = ['-foo']
expat:c_link_args = ['-Wl,--arg']
b_ndebug = true
c_std = 'c11'

Exactly what I have in mind, pretty readable to me, it's a matter of coding style, like in any language. It's also exactly what I already have in script that calls meson with all options set.

But I don't think we're going to solve this by arguing about it anymore, clearly I like blue bikesheds and you like yellow ones. Maybe some other people have opinions?

Indeed, I think it's pretty clear we'll never agree, so we need others to vote :-)

marc-h38 commented 4 years ago

I'm glad you volunteer to improve our documentation, there is certainly a lot to do :D

Monday morning, fresh mind let's give it a go.

I found the name "User" in the first sentence of that page, didn't take me much effort. "User" is IMHO a relatively good negation of "built-in" but please suggest better ones.

Nothing gets called "build options" because in a meson context, it's hard to come with a name more vague, less "searchable" and generally less useful than "build".

In the same meson context, the name... "meson" (!) options is slightly less vague but barely. The name "meson BUILTINS" is tolerated in the rare case of potential confusion with some other sort of builtins.

BTW "build" is already used in the build/host/target autoconf triplet which is very relevant to... cross files! Another, huge reason not to overload it with anything else in the same area. It's a bit too late to change autoconf names, like 30 years late. There's also a "build step", a "build directory" and I bet we could find other "build"/nameless things (most of them also much older than meson).

In case anyone doubts "build" is an empty placeholder, see how "build options" sometimes refers to not-builtins-options, example here: https://mesonbuild.com/Builtin-options.html

two kinds of options: build options provided by the build files and built-in options that..

In some other places "build options" refers to ALL option kinds! Example: https://mesonbuild.com/Configuring-a-build-directory.html

Since 0.50.0, it is also possible to get a list of all build options by invoking meson configure

https://mesonbuild.com/Subprojects.html#build-options-in-subproject

The names "Project and "Global" are forbidden outside subproject-related topics to avoid overloading and intolerable confusion.

I really, really hope I didn't misunderstand the concepts here. If I have please correct the concepts and do not discuss the names I just suggested as gold standards, that would be pointless. I may have misunderstood the concepts because... they don't have good, standard names yet :-)

https://martinfowler.com/bliki/TwoHardThings.html

dcbaker commented 4 years ago

@nirbheek, @jpakkane, do either of you have strong opinions regarding the "one section per project" or "one sections for all projects" debate that @xclaesse and I are having?

dcbaker commented 4 years ago

Once we have the big bikesheding debate worked out I'll also rename the sections to match our documentation, builtin-in options and user options.

marc-h38 commented 4 years ago

A bikeshed is in the backyard. This is the user interface = the front porch.

jpakkane commented 4 years ago

I agree that meson_options.txt is not a good name, because the file specifies options for the project. In my defense I made that choice back when there were zero other users (or team members) to ask comments for. Oh well, you can't win them all, I guess.

I personally prefer the format where each subproject has its own top level section, for pretty much the same reasons as listed above: it makes it easy to see options affecting each subproject at a glance.

I agree that [meson options] is a bit of a misnomer and if we had a word for "options common to all projects including subdir, language standards etc" we should probably start using that consistently. The problem being what should it be called? The only ones I could come up with were common options and shared options but neither of those feels like a good overall term.

dcbaker commented 4 years ago

I've rebased this on master, and changed the meson options section to built-in options, as that seems a little clearer to me.

xclaesse commented 4 years ago

What about not inventing words, and simply use [options] for all options that does not have a subproject: prefix, and [subproject options] for all options that have subproject: prefix?

[options]
default_library=shared
some_main_project_option=enabled

[foo options]
# Maybe drop `foo:` prefix here, and automatically prefix them all with first word of section title.
# But IMHO it's better to keep the prefix because that's how they are shown in `meson configure`
foo:default_library=static
foo:some_subproject_option=enabled
dcbaker commented 4 years ago

I don't think we can safely drop the prefix:. We guarantee that a project wont have a : in the name but we don't do that for any other character. you could name your project zlib options if you wanted. I'm not totally opposed to merging the two sections, but I didn't "make up" built-in, that's literally the name of the section in the manual for options that meson provides https://mesonbuild.com/Builtin-options.html.

@jpakkane, do you have a strong preference on one section or two? If you don't have a preference for two I think I'm going to merge them into one section.

xclaesse commented 4 years ago

I didn't "make up" built-in, that's literally the name of the section in the manual for options that meson provides https://mesonbuild.com/Builtin-options.html.

Ah, good point. In the doc built-in options contains core, base, compiler options.

dcbaker commented 4 years ago

Right, the intention was that all of the core, base, and compiler options would go in the built-in options sections, all of the options defined in meson_options.txt would be in the project options sections. Does that seem good, or would you still prefer one options section?

jpakkane commented 4 years ago

Fine by me.

dcbaker commented 4 years ago

I'd like to target this for 0.55 or later, I think there's enough in 0.54 as is, and this is probably going to be the source of a lot of bugs.

jpakkane commented 4 years ago

This being the case we'd probably need to get the part that allows you to set options in build files from #6595 and merge it for this release.

Ericson2314 commented 4 years ago

This would benefit I believe benefit from a better separation of parsing files and business logic, especially if we are fine holding off until 0.55. I got started on that separation #5489, after which I want to improve Properties and get rid of Environment.cmd_line_optionsby merging in the command line intoProperties(perhaps using logic for unioningProperties` from #6815).

I think a good order of things is:

  1. 5489

  2. Improve Properties and compiler options with a Language enum and PerLanguage a la MachineChoice and PerMachine. (#7003)
  3. This PR

Also concurrently after step 2:

  1. 6815

  2. Get rid of Environment.cmd_line_options

And finally after both threads

  1. More structure for subprojects, thinking about default options, etc.
dcbaker commented 4 years ago

I think @xclaesse and I both have different, unfinished branches to make compiler options per subproject as well, although both of those make the env.comand_line_options even grosser IIRC (I know mine does, maybe the other didn't?)

Ericson2314 commented 4 years ago

@dcbaker Well, then I better hurry up getting rid of Environment.cmd_line_options! :)

dcbaker commented 4 years ago

Okay, I've rebased this, and done some small documentation cleanups, but otherwise this is pretty much the same as before. I'd like to move toward landing this sooner rather than later if possible. @Ericson2314, @xclaesse, @jpakkane

TheQwertiest commented 4 years ago

How would be the collision between native and cross resolved? For example, what would be the result of combining the following two files? native:

[built-in options]
backend=ninja

cross:

[built-in options]
backend=vs2019

[Edit]: Nevermind, found the corresponding section in the docs: (if doing a cross build the options from the native file will be ignored)

TheQwertiest commented 4 years ago

Regarding (if doing a cross build the options from the native file will be ignored): why is this the case? This would somewhat limit the abilities of this feature. For example it won't be possible to use a common file for specifying options only: meson --native-file=options.ini --native-file=native_toolchain.ini meson --native-file=options.ini --native-file=native_toolchain.ini --cross-file=cross_toolchain.ini one would have to move options.ini from --native-file to --cross-file depending on the build type.

nirbheek commented 4 years ago

How would be the collision between native and cross resolved

I think this should be an error.

dcbaker commented 3 years ago

The problem with it being an error is that some options are allowed to be different for build (native) and host (cross) targets, so we need to be able to set at least some options in the native file.

dcbaker commented 3 years ago

@xclaesse, sorry for the late response on this. That wont work because we need the options from both the native and the cross file for options that are allowed to be set on a per machine basis, like pkg-config path. I've added a test for this in the latest version.

xclaesse commented 3 years ago

@xclaesse, sorry for the late response on this. That wont work because we need the options from both the native and the cross file for options that are allowed to be set on a per machine basis, like pkg-config path. I've added a test for this in the latest version.

Hm, you would have to indeed prefix them with build., good point.

dcbaker commented 3 years ago

sigh, time to break on the windows laptop

dcbaker commented 3 years ago

Okay, I think I've addressed everything except whether we should deprecated the [paths] section (which I'd rather do in a separate MR if possible).

@xclaesse, @jpakkane, @Ericson2314, @nirbheek, what else needs to be done for this to land?

dcbaker commented 3 years ago

@xclaesse, @jpakkane, @Ericson2314, @nirbheek, I feel like we're ready to land this, anyone object?

xclaesse commented 3 years ago

I strongly dislike that syntax:

[sub:built-in options]
default_library=static

And I strongly dislike the way too complex implementation that I still do not really understand, compared to the trivial solution I proposed.

That being said, I ran the unit tests I wrote for https://github.com/mesonbuild/meson/pull/7293 and they passed, and it seems I'm alone in disliking this PR, so... lgtm ;-)

xclaesse commented 3 years ago

Would be nice in a future PR to deprecate [paths] section since they are built-in options after all, and also deprecate compiler options in [properties] section.