Meson: Silently ignoring changes to default_options in project() is a bad user experience

[nirbheek]

This has come up repeatedly, most recently when @pwithnall saw CI failures that didn't get fixed when he added c_std=c99 to default_options because we silently ignore changes to it on reconfigure.

Our options are:

1. Change the outputted build.ninja to contain changes to default_options (tricky; f.ex what if default_options changes a value that the user had previously set on the command-line?)
2. Warn that default options were changed, and users should run ninja wipe-reconfigure (which is a target that we've talked about which would wipe all meson configuration and re-configure from scratch)

The current situation is somewhat bad, IMO.

[pwithnall]

Warn that default options were changed

That’s not useful for the CI use case, where warnings mean nothing to the CI machine. For the interactive-developer use case, a warning at the top of the output is unlikely to be noticed.

What’s the use case for not reconfiguring when default_options is changed?

[nirbheek]

That’s not useful for the CI use case, where warnings mean nothing to the CI machine

We can add CI-specific behaviour, which is useful in other cases too. See: https://github.com/mesonbuild/meson/pull/1662

What’s the use case for not reconfiguring when default_options is changed?

I can't think of one; the biggest obstacle is the code itself. We currently don't track whether an option is currently at its 'meson-default' value, its 'build-file-default' value, or has been overridden on the command-line, so if the 'build-file-default' changes, we can't know whether to change the value and regenerate build.ninja with it.

The best solution would probably be to fix this.

[jpakkane]

Because that is what it is, a _default value_, not _current value_. A design principle in Meson has been that values of options can _never_ be changed from within the build files. The reason for this is that CMake does allow for it and it is maddeningly confusing.

In Meson the process of building must be functional. That is, given the same inputs (source files and option values) it must _always_ produce the same output. Allowing existing options to change values breaks this, and at worst leads to a non-converging build setup where running the build again will always produce different outputs on two consecutive runs.

[nirbheek]

Because that is what it is, a default value, not current value.

But the default values affect the current value, by definition, if you don't override that option from the command line.

A design principle in Meson has been that values of options can never be changed from within the build files

I think the project() invocation is an exception because we parse it before we start parsing the build files themselves.

That is, given the same inputs (source files and option values) it must always produce the same output. Allowing existing options to change values breaks this, and at worst leads to a non-converging build setup where running the build again will always produce different outputs on two consecutive runs.

I don't understand this. Changing the build files leads to a reconfigure, and produces the wrong output compared to if you configure with a fresh build directory. Doesn't that lead to a non-converging build setup?

[jpakkane]

But the default values affect the current value, by definition, if you don't override that option from the command line.

They do not define the current value. What they mean is "in case this was not already defined, then use this value".

Changing the build files leads to a reconfigure, and produces the wrong output compared to if you configure with a fresh build directory.

It's not "wrong" as such, just different. The two builds have a different value for an option and thus the output is different (usually).

Doesn't that lead to a non-converging build setup?

It leads to a different setup, but both of them are stable. What I meant here is if you can change option values (with a hypothetical set_option) then someone can do this:

if get_option('some_bool')
  # do something
else
  #do something else
endif
set_option(not get_option('some_bool'))

This will never converge. Whenever you run the build it will change things and trigger a full reconfigure the next time you run ninja. Or if it doesn't then you have a mismatch between what the value of some_bool actually is and what it should be. (This is slightly tangent to the issue here but the same point still stands.)

Default values were added to address exactly this. They provide a way to set up a custom default value but which are ignored on all consecutive runs. This is also in the documentation but sadly people don't read through all of it and if they do then they don't remember all the niggles.

I admit that the way the current thing behaves is a bit confusing the first time you hit it but the alternatives are worse and no-one has yet come up with a way to make this clear and understandable in all the corner cases.

[nirbheek]

They do not define the current value. What they mean is "in case this was not already defined, then use this value".

The value was indeed not set, so why wouldn't it use the new default value now?

It's not "wrong" as such, just different. The two builds have a different value for an option and thus the output is different (usually).

If the same build files with the same command-line and the same environment lead to two different outputs, that's a bug. It violates user's expectations and hence the principle of least surprise.

It leads to a different setup, but both of them are stable. What I meant here is if you can change option values (with a hypothetical set_option) then someone can do this:

That's fine, but we don't have set_option, and we never will, and this is about project() which cannot have conditional default options.

[jpakkane]

The value was indeed not set, so why wouldn't it use the new default value now?

Here "set" means "had some value before Meson started executing any code".

If the same build files with the same command-line and the same environment lead to two different outputs, that's a bug.

Yes. But that is not the case here. A fresh run has a different environment than a rerun one: it does not have values for options. They don't exist at all in the former but do have pre-existing values in the latter.

this is about project() which cannot have conditional default options.

Well yes and no. There are all sorts of nasty things one could do such as:

if get_option()
  subproject('foo', default_options : 'baz=bob')
else
  subproject('foo', default_options : 'baz=bar')
endif

Or possibly even:

project('foo', default_options : get_option('foo') ? 'foo=false' : 'foo=true')

Or if you have changed the value of foo from its default value to bar and then you have consecutive commits that change the default value first to bar and then to baz. If you pull both changes in one step the value remains bar but if you pull them one at a time, the final result will have value baz.

[jpakkane]

Storing the changes and warning (#2 in the post) is workable, we can do that.

[pwithnall]

Storing the changes and warning (#2 in the post) is workable, we can do that.

Warning is no use for CI unless there’s an option to turn it into a failure. Even better, rather than failing and requiring human intervention to fix (sad times), it would automatically reconfigure.

I don’t really care about the semantics of how options are handled. I pushed a change to git, and it wasn’t reflected in the next CI run. This should be possible to fix without the fix requiring human intervention. :-)

[gauteh]

I agree, options set through project() should trigger a reconfig - if they've been explicitly set by the user, e.g. a different test-suite run is one of the few times that will happen, then print a warning - otherwise update the option. If you have to use a value|default-value setup that is cumbersome, but as far as I can understand from the comments here project() is one of the few places where options can be set both by the build file and the user - perhaps then the project options should be limited to constants and not any evaluations?

[NickeZ]

Couldn't we always print the default values during reconfigure? That way you would notice that "these are not the default values i expected". If we implement option 2 in OP, you have to see it in the output of the first reconfiguration. Always printing the default values would let you see the inconstancy every time.

[jpakkane]

We already print a lot of stuff, I fear this would just add noise that people would almost always ignore.

[NickeZ]

What about printing all options that are not the default values every time? (edit: and their default values of course)

[nioncode]

I agree that for a CI environment (or even most default developer machines) where options are never set manually with meson configure and just the defaults are used, changing the default_options in meson.build has to trigger a reconfigure, just like it does when changing the option with meson configure. Otherwise, somebody has to clean the build directory on the CI or manually update the options in the build directory.

For this to work, it should be enough to store a flag for each option that denotes if it has been changed manually by the user through meson configure or if it is the meson / project default. Then, when the options are changed in meson.build, this flag can be checked to see if the value should be updated from default_options.

[jpakkane]

Otherwise, somebody has to clean the build directory on the CI or manually update the options in the build directory.

CI should always run from a clean slate. That's what it's _for_ and also why CCache was invented.

Then, when the options are changed in meson.build, this flag can be checked to see if the value should be updated from default_options.

Which means that whether something changes value or not is based on hidden state. Which is also an awful user experience.

[nioncode]

meson's approach has always been that you configure your build dir once and only have to call ninja afterwards. However, this is not true if you modify the default_options of your project. Then you either have to wipe your build dir or also change the option through meson configure.

This is bad user experience. If I never touched the default values, but then change them in my meson.build, I'd expect meson to pick this up and reconfigure accordingly.

[xclaesse]

For this to work, it should be enough to store a flag for each option that denotes if it has been changed manually by the user through meson configure or if it is the meson / project default.

FWIW, that's already stored in the cmd_line.txt file, only options that have been modified by the user are stored there.

If you reconfigure with --wipe it will update to new default_options unless you have set a value to that option. But it has the downside that you'll have to rebuild everything from scratch.

I'm wondering if we should have something like a --soft-wipe that would nuke only coredata.dat to make a reconfigure from scratch (loading new default_options) but won't delete all built objects to avoid a full rebuild.

[nioncode]

The thing is, as a user I don't want to call any other command manually after updating my build files. I simply want to call ninja and everything should work and not use old information that I never touched myself.