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

Prevent changing backend during reconfigure. #11571

Closed
wants to merge 1 commit into from
Closed

Conversation

jpakkane
Copy link
Member

@jpakkane jpakkane commented Mar 23, 2023

The fact that this is possible currently is a bug. It was never supposed to be possible to change backends once set.

def run(original_args, mainfile):
prevent_backend_change(original_args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with fixing this bug, but why are we doing it in mesonmain? We should be doing this inside msetup, rather than walking the command line to check this during minstall, mtest, mcompile, mdevenv...

for arg in args:
if arg.startswith('--reconfigure'):
has_reconf = True
if arg.startswith('--backend'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if we do it during msetup then instead of arg.startswith we can use actual argparse properties.

Also we can load the old coredata and check that if a backend is specified, it's not different from the previous one. Because I think people have (not unreasonably) decided to specify the backend in a shell variable and pass a big list of options to meson setup, but if a build directory already exists then they additionally insert --reconfigure. They're not trying to change the backend though, because it's just re-specifying the same one as last time (because it's an easier if/else).

Also we can properly handle -Dbackend=... and -D "backend=...".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that you can't tell it just with properties. --backend has a default value so you can't tell whether the actual command line argument was there or not.

@xclaesse
Copy link
Member

Backend could also be changed from "meson configure". I think we should have a generic way of having read-only options, like I did in 762e5b2#diff-fcc9a2d88754c3db52e10dbc83e0410ae7cae4d77d47637eb8ef8046cd1a8cc5R690-R693.

@xclaesse
Copy link
Member

@jpakkane made an alternative PR based on the code I already had: #11572.

@jpakkane
Copy link
Member Author

Ok, closing this then.

@jpakkane jpakkane closed this Mar 23, 2023
@jpakkane jpakkane deleted the keepbackend branch March 23, 2023 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants