Allow "backend_startup_project" option when not using the VS backend.

[lukester1975]

Hello

backend_startup_project can't be added to a meson.build and used without the VS backend:

project('startup_test', ['cpp'], default_options : ['backend_startup_project=startup_test'])
executable('startup_test', 'main.cpp')

meson setup builddir gives:

meson.build:1:0: ERROR: Unknown options: "backend_startup_project"

The documentation shows it being set in a meson.build rather than via command line and it would obviously be nice if this didn't break ninja.

I think allowing the default active VS project to be set in a meson.build is entirely reasonable: there may be only one executable and multiple libraries, in which case the executable should be made active by default as it's the one that will be run.

I notice the same appears to be true of the ninja backend_max_links option but I haven't touched that, as that seems more build machine specific and probably not appropriate for meson.build.

Regards

[dcbaker]

this won't actually do what you want (it's a bit surprising I know), you need OptionKey.from_string('backend_startup_project'), or OptionKey('startup_project', _type=OptionType.BACKEND)

[lukester1975]

Oh! How so? It seems to do what I was after:

project('startup_test', ['cpp'], default_options : ['backend_startup_project=a'])
executable('a', 'main.cpp')
executable('b', 'main.cpp')
executable('c', 'main.cpp')
executable('d', 'main.cpp')

Change to d:

Now what is weird but unrelated to the change (does it with master) is b or c lose d!

I might investigate that tomorrow. I've tripped over it before and just reordered things to make it work, but can't be intentional...

[lukester1975]

OK I stepped through it, looks like

meson/mesonbuild/mesonlib/universal.py

Lines 2071 to 2072 in 5163a02

	if _type is None:
	_type = _classify_argument(self)

meson/mesonbuild/mesonlib/universal.py

Lines 2028 to 2030 in 5163a02

	elif key.name.startswith('backend_'):
	assert key.machine is MachineChoice.HOST, str(key)
	return OptionType.BACKEND

is setting the type. If being explicit is still preferred I can make that change!

Cheers

[dcbaker]

The problem is the key should be stored as OptionKey('startup_project', _type=OptionType.BACKEND), which from_string handles correctly, but if you just pass the string straight to the initializer it gets stored as OptionKey('backup_startup_project', _type=OptionType.BACKUP).

This is one of those cases where I wish that python had private constructors, using the initializer directly is fragile.

[lukester1975]

Well I'm more of a Typescript programmer than a Python programmer, and I'm not much of a Typescript programmer (as I may have mentioned)...

class OptionKey:
...
    def __init__(self, name: str, subproject: str = '',
                 machine: MachineChoice = MachineChoice.HOST,
                 lang: T.Optional[str] = None,
                 module: T.Optional[str] = None,
                 _type: T.Optional[OptionType] = None):
        # the _type option to the constructor is kinda private. We want to be
        # able tos ave the state and avoid the lookup function when
        # pickling/unpickling, but we need to be able to calculate it when
        # constructing a new OptionKeyhkj
        object.__setattr__(self, 'name', name)
        object.__setattr__(self, 'subproject', subproject)
        object.__setattr__(self, 'machine', machine)
        object.__setattr__(self, 'lang', lang)
        object.__setattr__(self, 'module', module)
        object.__setattr__(self, '_hash', hash((name, subproject, machine, lang, module)))
        if _type is None:
            _type = _classify_argument(self)
        object.__setattr__(self, 'type', _type)

Stepping through with (OptionKey('backend_startup_project'), BuiltinOption(UserStringOption, 'The default Visual Studio active project', '')) the initializer is called with:

name == 'backend_startup_project'
subproject == ''
machine == MachineChoice.HOST
lang == None
module == None
_type == None

Then _classify_argument fixes up type to OptionType.BACKEND.

With (OptionKey.from_string('backend_startup_project'), BuiltinOption(UserStringOption, 'The default Visual Studio active project', '')) it ends up called the same:

name == 'backend_startup_project'
subproject == ''
machine == MachineChoice.HOST
lang == None
module == None
_type == None

When you said "or OptionKey('startup_project', _type=OptionType.BACKEND)" did you mean backend_startup_project? It doesn't seem to prefix with backend_ if _type is BACKEND so doesn't solve the problem (option is still unrecognised - in fact worse as not recognised with backend=vs due to being removed from init_backend_options!).

OptionKey('backend_startup_project', _type=OptionType.BACKEND) works, and saves the call to _classify_argument but other than that I can't see the difference :(

Sorry if I'm being dense.

[dcbaker]

You have found a bug! from_string should be splitting backend off of the string in the same way it would split b_ from a builtin option, which it doesn't either. It only seems to be properly splitting language... Sigh

[dcbaker]

I guess then for the moment it doesn't matter which way you do it, they'll both work the same. I'll have to sort out the bug another time.

[lukester1975]

Ah I see, makes sense now!

In that case, is it actually best to use OptionKey.from_string as it'll work now and still be correct when fixed? And obviously change

meson/mesonbuild/backend/vs2010backend.py

Line 449 in bd9d981

startup_project = self.environment.coredata.options[OptionKey('backend_startup_project')].value

too.

Also, it struck me this is actually a regression in 0.60. Previously it was only rewrite that complained (#8174) but as of 0.60 it's setup (so explains recent #9442). Not sure if that warrants a fix getting out sooner rather than later and whether the doc snippet is correct?

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -1.95% ⚠️

Comparison is base (84466b7) 70.11% compared to head (33e832a) 68.16%.
Report is 2 commits behind head on master.

❗ Current head 33e832a differs from pull request most recent head b6a23ad. Consider uploading reports for the commit b6a23ad to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9611      +/-   ##
==========================================
- Coverage   70.11%   68.16%   -1.95%     
==========================================
  Files         218      414     +196     
  Lines       47886    90279   +42393     
  Branches    11404    20707    +9303     
==========================================
+ Hits        33576    61542   +27966     
- Misses      11821    23991   +12170     
- Partials     2489     4746    +2257     

Files Changed	Coverage Δ
mesonbuild/coredata.py	84.21% <ø> (+1.34%)	⬆️

... and 391 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jpakkane]

It would probably be better to make Meson ignore all option names that start with backend_ that do not exist.

[lukester1975]

As a user, I'd probably expect meson.build to error on any unknown (across all) backend options and command line to error on any unknown backend options for the in-use backend.

As the author of the PR, I don't think I can really spare much time to get to know the codebase enough to do much more than trivial few-line fixes right now, interesting and/or educational as it might be :)

It would just be nice to fix the regression sooner than later if nobody else is going to expand on it, though!

Thanks

[eli-schwartz]

@anarazel you asked about this recently

[lukester1975]

Go on, you know you want to merge this. Belated xmas present for VS users 😀

[tristan957]

Put this on the 1.3.0 milestone. Coming up on two years with an approval, but just seems to have been forgotten.

[jpakkane]

That seems to add an option for vs startup project even on backends that do not have anything to do with VS. Basically it would mean that every backend would need to define all the options of every other backend as well to get the same behaviour. That is not great. Making the code ignore unknown backend options is better in this regard.

[xclaesse]

That seems to add an option for vs startup project even on backends that do not have anything to do with VS. Basically it would mean that every backend would need to define all the options of every other backend as well to get the same behaviour. That is not great. Making the code ignore unknown backend options is better in this regard.

I agree and I would add that we have a precedent for this already. Depending on the actual compiler, not all base options are added to the project. But you can still do get_option() for all of them. Not sure you can also set them in default_option, but if not I would consider that being a bug.

[xclaesse]

I'm not exactly sure what does the startup project with vs backend, but if it's a way to tell the IDE which executable to run when pressing the play button, maybe that should not be a backend option. That's something we want with ninja backend too, and vscode-meson extension could use.

[eli-schwartz]

What would vscode-meson do to retrieve its value?

[lukester1975]

• Yes, backend_startup_project sets the active project in a VS solution. So it's the one that will be run/debugged by default on F5/play. (Also the one that will get built unless a source file from another project has focus, IIRC. Maybe some other stuff)
• That in itself isn't great; see Set vcproj's default workdir and args based on a matching test. #11343 (like this mod, that is just a pragmatic "this is a bit better" suggestion. It may be possible to essentially create pseudo projects for every defined test of the same target, or something. Dunno what cmake does).
• To me, it only makes sense for backend_startup_project to be supported the meson.build. There's no point accepting it on the command line, as if generating VS projects surely it's to use the IDE. Saving one click to set the active project at the cost of a bunch of typing doesn't add up (not suggesting no command line support, it just doesn't seem useful).
• Given there's only two backend options (this and backend_max_links AFAICT), I'd argue "it would mean that every backend would need to define all the options of every other backend as well to get the same behaviour" isn't a big deal for now. Quick hack fix and add a longer term task to do a better generic job later? Given the comment from @dcbaker above I'm thinking it's not something I could realistically get involved with.
• I'm happy to make the same type of change for backend_max_links if that is more palattable, though the converse is true there: it shouldn't be in the meson.build IMHO, so not really a concern...
• Is it a problem that meson setup debug --backend=vs --backend-startup-project=... and meson setup debug --backend=vs -Dbackend_startup_project=... are both accepted with this mod? I can to figure out how to make only the latter work, assuming it's not too complicated (given on that same comment from above).
• Beyond that I'd have to leave it to the experts 😀 Ultimately I'm just trying to be able to use meson without any local patches!

Thanks

[xclaesse]

What would vscode-meson do to retrieve its value?

Could be part of introspection data.

More generally, I think there is nothing specific to visual studio here. A default run target is something most IDE has, would thus be nice to have a generic solution. I'm also not entirely sure it should be a user option, but rather a project decision? I've been wondering for a while if we could have e.g. exe = executable(); meson.project_target(exe) that IDE can find in introspection data and vs backend could find as well.

[lukester1975]

I'm also not entirely sure it should be a user option, but rather a project decision?

That's what I want to do and what doesn't work sanely right now...

I've been wondering for a while if we could have e.g. exe = executable(); meson.project_target(exe) that IDE can find in introspection data and vs backend could find as well.

I would like this; I tried suggesting something in #9645 but it was rejected.