Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bisected] Can no longer override subproject defaults in superproject's project() statement #7573

Closed
Akaricchi opened this issue Aug 8, 2020 · 5 comments

Comments

@Akaricchi
Copy link
Contributor

Akaricchi commented Aug 8, 2020

Describe the bug
In Meson 0.55.0 and below, it was possible to override a subproject's default options inside the superproject's project() statement, like so:

project('superproject', 'c',
  default_options : [
      'subproject:test_option=false'])

In current git master, this is broken. Configuring such a project for the first time results in the following (irrelevant output omitted):

$ grep test_option subprojects/subproject/meson_options.txt 
option('test_option', type : 'boolean', value : true, description : 'Test option. Superproject overrides default to "false"')

$ meson setup build

... omitted ...

WARNING: Unknown options: "subproject:test_option"
The value of new options can be set with:
meson setup <builddir> --reconfigure -Dnew_option=new_value ...

... omitted ...

|Executing subproject subproject method meson 
|
|WARNING: In subproject subproject: Unknown options: "subproject:subproject:test_option"
|The value of new options can be set with:
|meson setup <builddir> --reconfigure -Dnew_option=new_value ...

... omitted ...

Option subproject:test_option is: true [default: false]

$ meson configure build | grep test_option
  subproject:test_option     true           [true, false]       Test option. Superproject overrides default to "false"

It seems to recognize false as the new "default", however it still applies the old default value to a fresh build directory, as if -Dtest_option=true was passed explicitly. If an old build directory is --wiped, any such options are also reverted to their subproject defaults, unless they have been explicitly overriden at any point during the build dir's lifetime.

There is also a new spurious warning

|WARNING: In subproject subproject: Unknown options: "subproject:subproject:test_option"

This seems to happen for every option overriden via the project() statement.

To Reproduce
Here is a minimal test case:
meson-test-subproject-option-override-from-superproject.zip

Expected behavior
After configuring a build directory for the attached test case, subproject:test_option should be false. There should not be any warnings about missing options.

system parameters

@Akaricchi
Copy link
Contributor Author

Akaricchi commented Aug 8, 2020

Bisected to 591e6e9, part of #6597. CC @dcbaker

@Akaricchi Akaricchi changed the title Can no longer override subproject defaults in superproject's project() statement [bisected] Can no longer override subproject defaults in superproject's project() statement Aug 9, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 9, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 9, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
@xclaesse
Copy link
Member

xclaesse commented Aug 9, 2020

Thanks for testing master! I didn't know we could have subprojects:foo in project(). I fixed this regression in #7574 and added a unit test for that.

xclaesse added a commit to xclaesse/meson that referenced this issue Aug 9, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
@Akaricchi
Copy link
Contributor Author

I didn't know we could have subprojects:foo in project()

I believe this isn't actually documented, and might've even been allowed by accident (correct me if I'm wrong). But this is useful, has worked for a while, and some projects depend on it, so I'd say it's worth promoting to an official feature. Pretty sure this used to be the only working way to override subproject defaults at some point; not sure whether it still is.

@dcbaker
Copy link
Member

dcbaker commented Aug 11, 2020

I'm not sure we meant to enable this. Since we have support for setting default arguments to subprojects in the subproject() call I'd prefer to deprecate this and keep the one in the subproject call.

@Akaricchi
Copy link
Contributor Author

But what if you don't call subproject()? It's common and encouraged to just use dependency() with a fallback instead, and in 0.55.0 the fallback can even be implicit. I don't think you can specify subproject options in that way, can you? Besides, having it all in one place is just more readable and maintainable. I'd rather deprecate subproject(default_options:), especially considering that it used to be broken for a while.

dcbaker added a commit to dcbaker/meson that referenced this issue Aug 14, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Aug 14, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Aug 14, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Aug 14, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 16, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 16, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Aug 16, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Aug 16, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Aug 18, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 18, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 18, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 18, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Aug 18, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
dcbaker added a commit to dcbaker/meson that referenced this issue Oct 5, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 5, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 5, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 5, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 5, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
Ericson2314 pushed a commit to Ericson2314/meson that referenced this issue Oct 6, 2020
the superproject

This only allows the super project to do this, and not for subprojects
to affect each other, as that would create race conditions and just be
really difficult to make work without a two pass interpreter. This
should probably be deprecated in favor of the default_options argument
to subproject() anyway.

Fixes mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 14, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 14, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 16, 2020
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 16, 2020
This is consistent with c_args in machine file overriding CFLAGS from
env. This also spotted an issue where in a native build this resulted
in pkg_config_path being /bar instead of /foo:
`-Dpkg_config_path=/foo -Dbuild.pkg_config_path=/bar`

Fixes: mesonbuild#7573
xclaesse added a commit that referenced this issue Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants