Experimental 'genvslite' WIP. #11049
Conversation
|
This pull request introduces 1 alert when merging 318133c into 8dfa550 - view on LGTM.com new alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
|
It would be useful to know if this is being overlooked or perhaps just deferred for various reasons (and what those reasons might be). Without feedback on at least the above rationale and description of this addition, I'm left a bit baffled as to why others might not consider the existing generated visual studio files quite as cumbersome for multi-config switching as I do and how it can be fixed/improved. |
|
I've thought a bit about this problem, as I've been writing another implementation of Meson in c++. For meson++, creating rules to build multiple buildtypes at the same time should be doable because of architectural decisions I've made. For meson it's a bit more complicated, because meson is a just-in-time interpreter, and you'd effectively have to run the same interpreter multiple times to get all of the information you need to generate the graph for each buildtype, then you'd need to de-duplicate any targets that aren't altered between buildtypes (custom_target, generator, etc). I know that you've side stepped a lot of that by generating multiple vs-solutions, then have a single vs-solution that invokes |
|
I don't think I understand the underlying topic well enough to be the best reviewer for the feature, I certainly cannot test it. So I'm not sure how much I have to say on the topic. One thing that stands out to me is that any check for raise MesonException('You are not allowed to use the genvslite backend and also check the value'
'of options that the genvslite backend generates multiple profiles for')CMake IIRC instead says you "should" only check the value of such options via generator expressions, and for obvious reasons Meson is not going to implement generator expression, which means we need to instead detect the builds that would be mishandled by this and refuse to configure at all. |
|
I think (If I understand correctly) the solution proposed is that the end user generates separate build directories for each target type, then this code generates a solution that invokes those solutions when called for a specific build type. So that entire issue is side-stepped. |
|
"the end user generates separate build directories for each target type, then this code generates a solution that invokes those solutions when called for a specific build type. So that entire issue is side-stepped.": That is essentially correct, except it doesn't generate multiple visual studio 'solutions'; only multiple ninja-back-end-configured build directories (one for each of the multiple build types) and then finally one visual-studio-back-end build directory, but where the .sln and .vcxprojs are correctly set up to support switching between the multiple build types as visual studio configurations... where the build/clean/rebuild hooks just redirect to invoking the appropraite 'meson compile ...' command for the appropriate ninja-backend-configured build directory. In short, it works as if the user had manually run - and then finally creates a .sln and .vcxproj that supports 'debug', 'debugoptimized', and 'release' configurations and hooks the build/rebuild/clean build events up to running the appropriate - so I don't see that there would be any problems from any meson script checks of The only think I'm aware of as potentially problematic with this change is that I've configured solution's intellisense to use the per-configuration (per 'buildtype') defines and compiler options fields in each .vcxproj, where the intellisense fields of each source file in a VS project merely reference the shared intellisense includes, defines, and compiler options set in the projects intellisense fields. To put it another way, if I were to set up a meson project that mixed multiple different source types (e.g. .cpp, .c, and .cs) that meson might allow to have totally different includes, defines, and compiler options even under the same target (.vcxproj), then what I've done here will just use a single set (but still per config/buildtype) of intellisense defines, include directories, and options for all source files in each .vcxproj. That's not to say that it has regressed any existing functionality (only the use of this new '--genvslite' mode does this). I don't think it would be too much trouble to change it to embed the intellisense includes/dirs/options fields alongside every source file reference in a VS project. It's just that would have an awful lot of intellisense field redundancy in the generated .vcxproj files (per source and header file fields instead of per project fields). |
|
Perhaps those such as @CrendKing, @anarazel, or @nioncode, who have authored some of the more recent 'vs20[xx]backend.py' changes are in a position to review/critique these changes. It feels like it'd be a shame to miss out on these changes for which I hope I've given a good enough rationale as to why I think a single, multi-configuration-switching solution, but which still hooks in to use the meson build system (rather than the native MSBuild system), is a good addition to meson's visual studio support and is actually a more usual/useful use-case of visual studio development (at least regarding the use of multiple 'configurations' in solutions and projects). Failing that, rather than let any of this go to waste for want of a reviewer, what are the chances that these changes (i.e. the use of the '--genvslite') could be adopted, merely as an experimental option, since the existing '--backend vs[xxxx]' functionality is unchanged? |
|
Sorry for not getting to this earlier but there has been a ton of stuff to do w.r.t. 1.0 and other things. The basic questions regarding this would be:
This is just to gauge the overall feel of this feature. The above reasons were the main reason I chose to write "native" solution files. Note that you can change an existing VS solution from debug to release with |
|
Thanks for the reply, @jpakkane . Regarding "all the usability integration stuff that VS users are used to": It provides -
"Do we know if this is something people would actually use": This usage (specifically having VS just as an editor/debugger/IDE and an entirely separate, stand-alone build system, with simple hooks driven from VS) was ubiquitous across many projects, over many years, where I've worked. I was surprised that this more simple (just the clean/build/rebuild hooks) and flexible approach wasn't available in meson. I'm repeating much of what I've already said above that motivated these changes but having a 'meson build' to generate a VS native build seems very odd to me -
[As a side-note/feature request, on the last point above, I'd argue that meson is already facing a little difficulty through its questionable choice of what I call 'find, guess, and flail by default' behaviours (things like trying to implicitly guess at toolchains, platform sdk headers/libs), which can easily, quietly change from one user's machine to the next, which I have found out the hard way and have to fight against, rather than having simpler, explicit behaviour of just stopping with an error if the user hasn't explicitly specified a toolchain or a path to a needed platform header/lib. A default of no searching/guessing would likely marginally speed up a few steps, too. Regarding "you can change an existing VS solution from debug to release with meson configure": Unless I've misunderstood, this is really quite a cumbersome procedure (bring up command prompt in the root meson script dir, run 'meson configure --buildtype [debug/opt/release] ...), wait a little for processing and project generation to complete, switch back to VS and reload the re-generated solution and projects. Comparing that to the more usual approach (by which I really mean the only way I've ever seen or known visual studio to used with multiple build configurations) of having the multiple build configurations as an integral part of the solution and projects and users quickly and simply switch via the configuration selector, and is something generally done several times a day, there's no contest in which is the quicker, simpler user workflow, surely. |
Do you mean stuff like how cmake adds directories from PATH as package prefixes, then searches for headers and libraries in lib/ and include/ children of all those PATH elements? Meson doesn't do that. :D Meson does attempt to guess the names of compilers, e.g. it will search for cl.exe, gcc, clang, cc, and various other names. It drives the platform SDK headers/libs detection solely from the detected compiler toolchain though (including environment variables that the compiler is documented to inherently respect -- e.g. that influences what gcc --print-search-dirs will print out).
Sounds like this is just machine files? Those have a binaries section that forces program lookup, including compilers, to always use the explicit paths. You can also specify include/library directories in that machine file, among other things. |
Tbh, considering that the vs backend is a lot of work and the ninja backend has the best quality (this is a polite way of saying that various less common things unexpectedly turn out to not yet be implemented for the vs backend, such as languages less common than C/C++) I don't think that's an unreasonable perspective to take, and for that reason alone I find your arguments to be persuasive. |
|
I was trying to test this but ended up with some errors. Here is what I did:
snip
snip
snip |
|
Thanks for the detailed steps @CrendKing . Now that it seems the general concept, as described in all my blurb above, hasn't been dismissed outright (and perhaps even be given a tentative 'OK'), I feel happier that I won't be clearly wasting my time cleaning up the rough edges and issues (like the obviously hard-coded 'cpp'-centric-ness, which is what I needed to test and suit my purposes). |
I'm using meson on windows for a project where I often need to switch between debug/release builds and I can confirm that it's absolutely annoying to have multiple solutions for this (running |
I'm afraid I've lost exact details of most of the specific examples of investigating and fighting against this quiet guess-like fallback behaviour, but I do recall some struggle trying to find out why certain compilers and sdk header/lib paths were coming from when they weren't what I wanted/expected. For example, I notice I ended up adding a 'auto_features = disabled' line in my toolchain ini, which I presume I eventually found after some frustration investigating why something or some paths were quietly automatically found or used which I didn't want. However, I've not noted exactly what problem it gave me that I needed to fix by disabling some auto/guess behaviour. One example I do remember is when I added a ".c" source file to a local meson test project that previously was exclusively .cpp only and which I had intended to be used in a fully explicit toolchain ini to specify all aspects of the build that would otherwise be prone to the undesireable 'find and guess' behaviour. However, I had forgotten to add the required C compiler settings/args/flags to the toolchain ini file, but I only realised by chance inspection of some internal state; I wasn't made aware of this in a simple, unavoidable way (printing out a log info/warning message, for example, doesn't require my attention and action, whereas an error/failure prompt does). What appears to have happened is that meson has gone off and found a C compiler somewhere on my machine and ended up using that. What I want is for meson to skip any/all search and guess behaviour (I'm suggesting via some kind of --fullyexplicit option) and, if a part of the build needs to use a binary (or a compiler/linker flag or even anything that's involved in find-n-guess behaviour) that hasn't been explicitly provided by me, then I don't want it to guess or use a default but to stop and give me an error, saying something like, "Hey. Your project has a .c file and you've not told me what C compiler to use!". That's then very easy for anyone to see, understand, and correct in a toolchain .ini file, and now I know, because I've been explicit, that there's no longer the chance of weird problems occurring when building on different machines that may have different sets and versions of toolchains that meson would previously have automatically found and tried to use. Same goes for platform header/lib paths: I'd much prefer the compiler/linker fail, saying it can't find a 'cstdio' header or report unresolved symbols, which I can easly understand is due to the fact that I haven't explicitly specified a required include or lib path and which I can easily add (in a way that ensures the toolchain or sdk is going to be stand-alone and portable), rather than trying to track down and counteract the quiet, automatic use of undesireable include/lib paths. Again, I realise some people actually like that kind of quiet, automatic functionality, but in general, I think this is verging on an anti-pattern, where there's only a short-term saving of one or two lines of explicitly specifying something that meson has deemed to be needed, but is trivial for everyone to resolve explicitly, and doing reduces portability problems (as well as informing the user about exactly what is needed for their build, which helps them plan and structure it better than just leaving it up to whatever can be found on whatever local machine it ends up being run). |
This setting controls whether project-defined configure options (i.e. the ones in meson_options.txt) of type "tristate feature defaulting to auto" are bulk toggled to disabled. That means that optional dependencies are by default per upstream configuration checked for and enable new parts of the build only if successfully found, but you've specified to disable them altogether instead. This is definitely a useful option -- many people don't like "automagic" dependencies, e.g. https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies and Meson provides the tools necessary to make this a choice in the hands of the building user.
If you add a C source file to a meson project that was exclusively C++, then you get an error message stating unknown compiler for that source filetype. If you don't get that error, then the project isn't exclusively C++ because either the project('test project name', 'list', 'of', 'languages')included both In both cases, the toolchain ini was consulted, the C compiler was detected, and any platform header/lib paths were handled, at the time of the language addition -- NOT when the source file was added to a target. Which also means that the C compiler was mandatory and not having one would cause your build to fail. (Note also that you can over-specify your toolchain ini, and it will not add those languages to the build -- it will only be used if the build files also specify to use those languages.) Assuming you don't use the dangerous |
|
Note that I'm not necessarily averse to the idea of an --ignore-env-and-only-use-machine-files option, but in this case it would simply have caused the build to fail all along (and therefore I suppose alerted you to the fact that you had an unused C language floating around in your build files the entire time). |
Maybe I didn't explain my scenario well enough: After some struggles fighting meson's 'quietly find/guess at toolchain binaries (and perhaps header/lib dirs)' behaviours, I eventually managed to get what I thought was a fully explicit cpp toolchain ini where I am finally able to control all these previously searched/guessed elements of the build. Later, I tried adding a .c file to what was previously a cpp-only project by adding to the project languages (e.g. 'project('someproject',['cpp','c'], ...)') as well as adding a .c source file to the 'executable(...)', all while still using the same fully-explicit-but-cpp-only toolchain ini. My mistake being simply that I'd forgotten to add the desired C tool settings in my existing toolchain ini file. Meson quietly (i.e. not forcing me) proceeds to search and guess at a C compiler, which I later find, by accident, is not the compiler I want or would choose if I were alerted to this situation by meson. If meson had this '--fullyexplicit' mode (which, on second thoughts, might even seem to fit in nicely with simply being implied through the use of a toolchain file) then I could be immediately told, "Hey! You haven't specified a C compiler that's required to compile 'whatever.c'". I'd immediately understand and learn of my negligence in adding the necessary C compiler fields to my toolchain ini and would easily fix it. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report
@@ Coverage Diff @@
## master #11049 +/- ##
==========================================
+ Coverage 65.41% 68.55% +3.14%
==========================================
Files 420 209 -211
Lines 91320 45756 -45564
Branches 20281 9477 -10804
==========================================
- Hits 59734 31369 -28365
+ Misses 26968 11958 -15010
+ Partials 4618 2429 -2189 see 248 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@CrendKing: This has been updated to fix the issues you mentioned above. Please feel free to have another look and give feedback on the changes. The same goes for anyone else looking at this. |
|
Trying the new version on zstd and getting this error: Output of |
|
Thanks for the zstd example @CrendKing . I've checked and fixed that issue now. |
|
Can anyone see (or failing that, help investigate) whether my changes have caused the failing 'msys2 / gccx86ninja' automation check, which seems to suggest that the unit test 'AllPlatformTests.test_long_output' is failing (timing out)? I'm not familiar with most things going on in these tests (e.g. the full set of tools and environments in use) but I at least can run - |
|
That test sometimes flakes out although I am not positive I recall it doing so on msys2 specifically. I've restarted that one job anyway, so we should soon see if it was just a flaky failure. |
|
So, it passes now. :) |
|
I was expecting someone to have a look over the changes and offer some suggested changes or other points for discussion but I'm starting to wonder if that's now unlikely to happen. If that's that case, should I now just concern myself with getting all tests green and ensure I've covered the 'new feature' requirements, namely -
Can someone suggest what tests might be reasonable to add in this case, where to start, and what existing tests can be referenced in going about trying to achieve all (or some) of the above? |
docs/markdown/Builtin-options.md
Outdated
| @@ -76,6 +76,7 @@ machine](#specifying-options-per-machine) section for details. | |||
| | -------------------------------------- | ------------- | ----------- | -------------- | ----------------- | | |||
| | auto_features {enabled, disabled, auto} | auto | Override value of all 'auto' features | no | no | | |||
| | backend {ninja, vs,<br>vs2010, vs2012, vs2013, vs2015, vs2017, vs2019, vs2022, xcode} | ninja | Backend to use | no | no | | |||
| | genvslite {vs2022} | vs2022 | Setup multiple buildtype-suffixed ninja-backend build directories (e.g. [builddir]_[debug/release/etc.]) and generate [builddir]_vs containing a Visual Studio solution with multiple configurations that invoke a meson compile of the newly setup build directories, as appropriate for the current configuration (builtype) | no | no | | |||
| | buildtype {plain, debug,<br>debugoptimized, release, minsize, custom} | debug | Build type to use | no | no | | |||
There was a problem hiding this comment.
We typically want to avoid adding new options whenever possible. In this case could this instead be implemented by adding a new value for backend? Something like vslite?
There was a problem hiding this comment.
@jpakkane I think I may have thought about that as a first attempt but decided against it because I'd argue that this new functionality doesn't behave in the same way as the other 'backend' options; all other backends create only one build directory that configures the build for a single, specific buildtype. I'd argue most reasonable users would expect just another 'backend' option would lead them to expect the same pattern of setup behaviour and build directory creation... Not that it instead goes and creates a set of (currently) 4 suffixed build directories, for different buildtypes, and one for the generated solution and project files to tie it all together.
There was a problem hiding this comment.
That may be so, but the backend is currently not an "option". It can not be changed after the build dir is set up. This however is an option so people would expect to be able to change between the two types by changing the option. That is not to be accepted, the "kindness" of a build dir needs to be immutable.
Since we probably need to make some backwards compatibility breaking changes to this before it is complete, maybe the type should be "exp-vslite" instead of "vslite"? (I considered "unstable-vslite", but it's a bit too much to type IMHO.).
There was a problem hiding this comment.
Sorry but I don't follow most of what you're trying to say. Can you be more be more fully qualified or throw in examples of what you think would be a problem and what you think a better solution would be?
backend is currently not an option
Do you mean "the --backend option would currently have no effect in conjunction with the --genvslite option"? If so, that's correct; as stated by the usage help/description -
Setup multiple buildtype-suffixed ninja-backend build directories (e.g. builddir_[debug/release/etc.]) and generate [builddir]_vs containing a Visual Studio solution ...
people would expect to be able to change between the two types by changing the option.
What two types are you referring to here?
Are you suggesting someone would do meson setup --genvslite vs2022 somebuilddir somesrcdir to set up somebuilddir_debug/debugoptimized/release/vs and then want to reconfigure some of the setup settings in one or more of these multiple buildtype-suffixed build dirs? If so, how is that a problem? I think it's clear to the users that the --genvslite option acts as a kind of 'setup macro' for generating multiple normal, ninja-backended build directories, so if they then really want to go and manually tweak/reconfigure what has been set up in their 'somebuilddir_release' directory, for example, then that's up to them and they should (and I think can) understand what they'll get.
we probably need to make some backwards compatibility breaking changes to this before it is complete
Can you explain further, please? In this change's current state, nothing of this new option breaks any of the previous behaviours that I'm aware of. Are you suggesting making changes that would cause back compat. breaking changes? If so, can you go into more detail, please?
There was a problem hiding this comment.
Are you suggesting someone would do meson setup --genvslite vs2022 somebuilddir somesrcdir and then want to reconfigure some of the setup settings in one or more of these multiple buildtype-suffixed build dirs?
What I'm saying is that if people set up a vs backend with genvslite set to true, then they expect that if you change that to false later and reconfigure, their build dir changes from "lite style" to "classical style" VS project. And that is something we don't want to support because mixing different styles of project styles within a single directory seems extremely fragile.
So what I'm suggesting is that this would be set up with something like meson setup --exp-vslite somebuilddir somesrcdir. Or maybe it should be called exp-vsninja to make it more clear what it does. But the point is that once the build dir is set up to contain a "lite" vs project, it can not change. Because of that the setting can't really be a Meson option.
Can you explain further, please? In this change's current state, nothing of this new option breaks any of the previous behaviours that I'm aware of
What we have discovered through experience is that whenever new functionality like this is added, some part of it goes wrong. Not because of the commit would be bad, but because in real usage we find some unforeseen problems or quirks that would require breaking changes. As this is a totally new kind of a backend, I suspect we will find run into these. This is why it would be nice to keep this MR as isolated as possible and also mark it explicitly as unstable so we can change it later (and if people complain, then that is their problem). We do the same for new modules, they are typically first marked as unstable-modname.
There was a problem hiding this comment.
That may be so, but the backend is currently not an "option". It can not be changed after the build dir is set up. This however is an option so people would expect to be able to change between the two types by changing the option. That is not to be accepted, the "kindness" of a build dir needs to be immutable.
I regret to inform you both that it's currently an option, and that you can currently change it during --reconfigure.
Now granted, if you do it will fully configure the entire thing, then traceback at the last second, see #10014 -- but that's only because of backend-specific options, you cannot move between vs and ninja, and I think you can move from ninja to xcode.
There was a problem hiding this comment.
What I'm saying is that if people set up a vs backend with genvslite ...
This is still a little confusing and I'm wondering if there's a fundamental misunderstanding or assumption or terminiology that means different things to us. Perhaps others could shed some light if you can see the possible source of miscommunication. The above quoted sentence does not make any sense to me; use of genvslite cannot set up a vs backend; it effectively forces the --backend ninja option for multiple different (suffixed) build directories. What will happen, if someone does meson setup --genvslite vs2022 somebuilddir somesrcdir, is it will (internally) go off and run the following setup steps -
meson setup --backend ninja --buildtype debug somebuilddir_debug somesrcdirmeson setup --backend ninja --buildtype debugoptimized somebuilddir_debugoptimized somesrcdirmeson setup --backend ninja --buildtype release somebuilddir_release somesrcdir- And also generate a directory called 'somebuilddir_vs' containing the .sln and .vcxproj files that glues these all together in a multi-configuration (a.k.a 'buildtype') solution.
It is very much just like a meson setup macro and is pretty unambiguous about what it's doing, what build directories you'll end up with, and how they're configured. Is the fact that you end up with multiple buldtype-suffixed build directories (which is unique to this setup option) really at the heart of your concerns here?
... if you change that to false later and reconfigure, their build dir changes from "lite style" to "classical style" VS project ... that is something we don't want to support because mixing different styles of project styles within a single directory seems extremely fragile.
To me, this seems like quite a strange example that looks like a misunderstanding of what the --genvslite setup option will do. If, after someone does meson setup --genvslite vs2022 somebuilddir somesrcdir, then they want to change back to a 'classical style VS project', then that means they want to go back to a single build directory, containing a single buildtype-configured .sln and .vcxprojs (that will build with the msbuild machinery). To do so would mean they almost certainly want to set that up in a brand new 'somebuilddir', not the recently generated 'somebuilddir_[debug/debugoptimized/release/vs]'. However, if they really wanted (and though I can't think of a realistic scenario in which someone would want to do this), there's nothing stopping them from piggybacking and reconfiguring one of the previously setup buildtype-suffixed build directories. E.g. meson setup --reconfigure -Dsomeoption=somenewval --backend vs --buildtype debug somebuilddir_debug somesrcdir so that 'somebuilddir_debug' is then reconfigured from its previous ninja-backend to the msbuild stuff.
If they were to try meson setup --reconfigure -Dsomeoption=somenewval --backend vs --buildtype debug somebuilddir somesrcdir (note the build directory name difference), then that wouldn't work or (in my opinion) be expected to work because 'somebuilddir' doesn't exist.
So what I'm suggesting is that this would be set up with something like
meson setup --exp-vslite somebuilddir somesrcdir
I think this just confirms to me that either I or you (or both) misunderstand something, since your suggestion appears to be essentially the same, only changing the name of the setup option from 'genvslite' to 'exp-vslite', as well as getting rid of the set of supported visual studio versions for which the user wants to generate the .sln/.vcxproj files. Regarding the enumeration of 'vsxxxx' versions: Sure, currently, I only had the option to generate for vs2022 but there's nothing stopping the addition of supporting generation of older or newer versions, which will need some way of specifiying the generate .sln/.vcxproj versions. What am I missing here?
While writing this, I've been wondering if perhaps you're expectation is that meson setup --genvslite vs2022 somebuilddir somesrcdir will (or should) still build a SINGLE build directory called 'somebuilddir', and then within that single 'somebuilddir' will be further hidden, internal build directories (e.g. 'somebuilddir/debug', 'somebuilddir/debugoptimized', 'somebuilddir/release') that are the conventional meson ninja build dirs. I think doing something like that would make things a bit more complicated and perhaps less intuitive. The previous "multi-config-suffixed build dirs" behaviour (i.e. the current genvslite behaviour in this PR) makes it about as clear and obvious as I think is possible that you're getting multiple buildtype configured ninja build dirs. Any attempt to shoehorn everything in to a single build dir now introduces a bit of ambiguity as to what kind of buildtype (debug/opt/release/etc) the one builddir is configured as well as tries to bend over backwards to make everything continue to look like the conventional single builddir, single configuration approach when that's no longer the case.. And since the whole point of the option is to support a multi-config/buildtype VS solution and project files that still build through meson's ninja back-end, what does it mean for someone to try to setup or '--reconfigure' their one builddir for a different buildtype? With this PR's proposed approach, I'd argue it's just simple in that it ignores any '--builtype X' in the setup command line and just plops out '..._debug', '..._debugoptimized', and '..._release' configured ninja-backended build directories, and that an approach to somehow squeeze this multi-buildtype setup functionality into a single build directory feels like too much of a contortion and raises more questions and increases complexity over this current 'multiple suffixed build dir setup macro'-like functionality.
There was a problem hiding this comment.
I regret to inform you both that it's currently an option, and that you can currently change it during --reconfigure.
There was a problem hiding this comment.
argue it's just simple in that it ignores any '--builtype X' in the setup command line and just plops out '..._debug', '..._debugoptimized', and '..._release' configured ninja-backended build directories
What you could do is something like:
build
build\top_level_vs_solution.sln
build\debug
build\debugoptimized
...
In this way everything is still contained in a single build dir for easy deletion but the separate build types have their own dirs.
There was a problem hiding this comment.
What you could do is something like ... In this way everything is still contained in a single build dir for easy deletion...
This is just what I said a couple of posts above 😕 -
I've been wondering if perhaps you're expectation is that meson setup --genvslite vs2022 somebuilddir somesrcdir will (or should) still build a SINGLE build directory ... and then within that single 'somebuilddir' will be further hidden, internal build directories ...
And I go on to explain why I think doing that is a bit more complicated and problematic than this solution.
I think having multiple build dirs concurrently is fine and identifying or deleting them, just as for deleting any single build dir, is something I can't imagine is troublesome for anyone.
At least for my part, yes... I can offer very little critique of VS-related code in general so my plan is basically to ask that the tests work, see if anyone else has critique, and if there are no other discussion points then I'd be okay with merging it.
The There are other tests that check whether the current backend being tested is the VS backend rather than the ninja backend -- you can raise The alternative would be to set up a new azure pipelines job that tells run_project_tests.py how to generically run all test cases using the new backend, and then run the full test suite on that CI runner but with the new backend. |
GertyP
force-pushed
the
experimentalgensln
branch
from
f1cf662
to
c1e488d
Compare
GertyP
force-pushed
the
experimentalgensln
branch
from
9b8e41a
to
fc54def
Compare
| if isinstance(target, build.BuildTarget): | ||
| buildtype = target.get_option(OptionKey('buildtype')) | ||
| captured_compile_args_per_buildtype_and_target[buildtype][target.get_id()] = self.generate_common_compile_args_per_src_type(target) | ||
|
|
There was a problem hiding this comment.
FWICT captured_compile_args_per_buildtype_and_target is only written meaning it is actually an out parameter. If that is the case then that needs to be changed as those are not allowed. The function should store this in a local variable and return that as a return value instead.
There was a problem hiding this comment.
Thanks Jussi. I think this is now addressed in e8ff3fa.
I can totally understand and if it's a rule then it's a rule and I'm fine with this suggestion. I just wanted to add though, that the ability to use an 'out' param can arguably make for cleaner or more concise code. E.g. Passing in a valid arg acts as both an implicit bool argument to the called function that you want to fill the dict with captured compile values (or whatever) as well as providing the container to be filled in.
Additionally, in this particular case, we have the behaviour of -
- only ninja backend 'generate's are expected to potentially produce captured compile options, if asked.
- and only vs backend 'generate's are expected to make use of previously gathered captured compile options.
so that behaviour could be conveyed entirely through passing None or a valid dict to the ninja backend's 'generate' to fill in, and then passing a valid, filled in 'capture dict' to the vs backend's 'generate', through the same arg, to read from... which is arguably a bit simpler and/or cleaner; the alternative now being one bool arg to request a capture (which only needs to be heeded in the ninja backend) through returned value (only used in ninja backends) as well as an additional 'in'-only arg that's only used in vs backends.
Anyway, like I said, I'm happy to make the change, but just wanted to mention that I think I actually prefer the original, multi-functional arg style 😄
There was a problem hiding this comment.
I see at the bottom of this whole PR, it currently says, '1 change requested'. I'm just wondering if there's some automatic github PR workflow gating that I'm not aware of, that means I need to do or tick something, in addition to my reply immediately above, in order to notify reviewers to get issues and requested changes resolved? I can obviously see the 'resolve conversation' at the bottom of this issue/thread but that's surely not for me to do; I shouldn't be able to decide whether I have resolved an issue raised by someone else.
I'm not impatient (I know a good code review on changes this size is usually time consuming); just wondering if I've missed something that others might be waiting on.
| 'Setup multiple buildtype-suffixed ninja-backend build directories (e.g. builddir_[debug/release/etc.])' | ||
| 'and generate [builddir]_vs containing a Visual Studio solution with multiple configurations that invoke a meson compile of the newly' | ||
| 'setup build directories, as appropriate for the current build configuration (buildtype)', | ||
| 'vs2022', |
There was a problem hiding this comment.
The way Python string concatenation works is that these strings are joined without a space between them, so instead of newly setup you get newlysetup instead.
There was a problem hiding this comment.
Thanks. Fixed in f7b86b0
mesonbuild/msetup.py
Outdated
| options.builddir = f'{builddir_prefix}_vs' | ||
| options.cmd_line_options[mesonlib.OptionKey('genvslite')] = genvsliteval | ||
| app = MesonApp(options) | ||
| app.generate(captured_compile_args_per_buildtype_and_target = captured_compile_args_per_buildtype_and_target) |
There was a problem hiding this comment.
This if/else should be isolated into its own function instead for readability. Something like:
if <bulding vslite>
options = set_up_actual_options(...)
else:
options = ...
app.generate(options)
There was a problem hiding this comment.
Thanks very much for taking the time to look through all this. This issue should be resolved in e8ff3fa
GertyP
force-pushed
the
experimentalgensln
branch
from
1988526
to
f7b86b0
Compare
|
⚔️ conflicts. |
…tion for all languages used in building the targets so that these args, defines, and include paths can be applied to the .vcxproj's intellisense fields for all buildtypes/configurations.
Solution generation is now set up for mutiple build configurations (buildtypes) when using '--genvslite'.
All generated vcxprojs invoke the same high-level meson compile to build all targets; there's no selective target building (could add this later). Related to this, we skip pointlessly generating vcxprojs for targets that aren't buildable (BuildTarget-derived), which aren't of interest to the user anyway.
When using --genvslite, no longer inject '<ProjectReference ...>' dependencies on which a generated .vcxproj depends because that imposes a forced visual studio build dependency, which we don't want, since we're essentially bypassing VS's build in favour of running 'meson compile ...'.
When populating the vcxproj's shared intellisense defines, include paths, and compiler options fields, we choose the most frequent src file language, since this means more project src files can simply reference the project shared fields and fewer files of non-primary language types need to populate their full set of intellisense fields. This makes for smaller .vcxproj files.
Paths for generated source/header/etc files, left alone, would be added to solution projects relative to the '..._vs' build directory, where they're never generated; they're generated under the respective '..._[debug/opt/release]' ninja build directories that correspond to the solution build configuration. Although VS doesn't allow conditional src/header listings in vcxprojs (at least not in a simple way that I'm aware of), we can ensure these generated sources get adjusted to at least reference locations under one of the concrete build directories (I've chosen '..._debug') under which they will be generated.
Testing with --genvslite has revealed that, in some cases, the presence of 'c:\windows\system32;c:\windows' on the 'Path' environment variable (via the make-style project's ExecutablePath element) is critical to getting the 'meson compile ...' build to succeed. Not sure whether this is some 'find and guess' implicit defaults behaviour within meson or within the MSVC compiler that some projects may rely on. Feels weird but not sure of a better solution than forcibly adding these to the Path environment variable (the Executable Path property of the project).
Added a new windows-only test to windowstests.py ('test_genvslite') to exercise the --genvslite option along with checking that the 'msbuild' command invokes the 'meson compile ...' of the build-type-appropriate-suffixed temporary build dir and checks expected program output.
Check and report error if user specifies a non-ninja backend with a 'genvslite' setup, since that conflicts with the stated behaviour of genvslite. Also added this test case to 'WindowsTests.test_genvslite'
I had problems tracking down some problematic environment variable behaviour, which appears to need a work-around. See further notes on VSINSTALLDIR, in windowstests.py, test_genvslite.
'meson setup --help' clearly states that positional arguments are ... [builddir] [sourcedir]. However, BasePlatformTests.init(...) was passing these in the order [sourcedir] [builddir]. This was producing failures, saying, "ERROR: Neither directory contains a build file meson.build." but when using the correct ordering, setup now succeeds.
Changed regen, run_tests, and run_install utility projects to be simpler makefile projects instead, with commands to invoke the appropriate '...meson.py --internal regencheck ...' (or install/test) on the '[builddir]_[buildtype]' as appropriate for the curent VS configuration. Also, since the 'regen.vcxproj' utility didn't work correctly with '--genvslite' setup build dirs, and getting it to fully work would require more non-trivial intrusion into new parts of meson (i.e. '--internal regencheck', '--internal regenerate', and perhaps also 'setup --reconfigure'), for now, the REGEN project is replaced with a simpler, lighter-weight RECONFIGURE utility proj, which is unlinked from any solution build dependencies and which simply runs 'meson setup --reconfigure [builddir]_[buildtype] [srcdir]' on each of the ninja-backend build dirs for each buildtype.
Yes, although this will enable the building/compiling to be correctly configured, it can leave the solution/vcxprojs stale and out-of-date, it's simple for the user to 'meson setup --genvslite ...' to fully regenerate an updated, correct solution again. However, I've noted this down as a 'fixme' to consider implementing the full regen behaviour for the genvslite case.
- Avoid use of 'captured_compile_args_per_buildtype_and_target' as an 'out' param. - Factored a little msetup.py, 'run(...)' macro/looping setup steps, for genvslite, out into a 'run_genvslite_setup' func.
mesonbuild/backend/ninjabackend.py
Outdated
| def generate(self): | ||
| def generate( | ||
| self, | ||
| capture: bool = False, | ||
| captured_compile_args_per_buildtype_and_target: dict = None | ||
| ) -> T.Optional[dict]: |
There was a problem hiding this comment.
Meson does not use black-style formatting, in particular, please do NOT include the closing parenthesis for a function definition at the same indentation level as the preceding "def".
Also this parameter name seems a bit on the long side... I suspect that is the only reason you broke it onto multiple lines to begin with.
There was a problem hiding this comment.
On your point about closed parenthesese indentation: I find this -
def somefunc(
self,
something,
another,
yet_more,
and_finally) -> None
self.start.doing_some_stuff_with(something)
...
makes the first line of the method's body get lost with the with the params. While this -
def somefunc(
self,
something,
another,
yet_more,
and_finally
) -> None
self.start.doing_some_stuff_with(something)
...
gives a much clearest distinction between multiple params and function body.
Perhaps you prefer this -
def somefunc(
self,
something,
another,
yet_more,
and_finally
) -> None
self.start.doing_some_stuff_with(something)
...
which I find a little clearer than the first snippet above, but not quite as good as the second, which you're not keen on, it seems.
What's it to be?
There was a problem hiding this comment.
Also, regarding the long name: Whenever it's a choice between clarity and conciseness for naming, I always favour clarity.
For want of a better example -
world_records_by_sex_and_sport
makes it clearer (when compared to a variable named just 'world_records') that you could expect to use it like -
record = world_records_by_sex_and_event['male']['100m']
which I think marginally accelerates comprehension for the reader, especially if someone mistakenly misused it, like -
record = world_records_by_sex_and_event['100m']['male']
... Anyone reading this can, I think, more more easily see the mistake in order of applying keys here, while this would be much easier to misuse or overlook the mistake if we were only using world_records.
If you insist, I'll can change all uses of the long name to just captured_compile_args, but I just wanted to put forward the case for my preference in this case.
Let me know what you think and I won't make any more fuss on this point 😄
There was a problem hiding this comment.
The style that is used everywhere else is to have the closing parenthesis and return type on the same line as the last argument. Whether we should change it is a separate discussion but for now this MR should follow that style.
There was a problem hiding this comment.
On your point about closed parenthesese indentation: I find this -
def somefunc( self, something, another, yet_more, and_finally) -> None self.start.doing_some_stuff_with(something) ...makes the first line of the method's body get lost with the with the params. While this -
But really it should be this:
def somefunc(self,
something,
another,
yet_more,
and_finally) -> None:
self.start.doing_some_stuff_with(something)
...
[...]
which I find a little clearer than the first snippet above, but not quite as good as the second, which you're not keen on, it seems.
There's quite a bit of your third suggestion used in the codebase, which is to say, having the closing paren on a new line but still indented more than four spaces to line up with the opening paren.
Also somewhat common:
def somefunc(self, something, another, yet_more,
will_it_end, and_finally) -> None:
self.start.doing_some_stuff_with(something)There was a problem hiding this comment.
In terms of what I would call style bugs that must be fixed, functions currently use the style that arguments are aligned with the opening paren, and the close paren is either inline with the last argument or on its own line matching the indent of arguments.
This is how I believe most or all current code is styled.
How many arguments to use per line is typically left to the taste of the code author.
Whether to give the close paren its own line is also typically left to the taste of the code author with a bias towards doing so specifically when the return annotation takes up a large width.
There was a problem hiding this comment.
Apart from shortening the long variable name (I wasn't sure whether this was a show-stopper for you, in light of my arguments on this point above), this is done in 51b0700.
If you really do prefer, let me know and I'll also shorten captured_compile_args_per_buildtype_and_target to captured_compile_args under protest 😄
There was a problem hiding this comment.
Although it's way too late, I'd just like to clarify that if I had had a chance to reply to you in between 4 AM and when this PR was merged, I would have said that yes, I still believe a shorter (non-Java-style) name is better for readability and code clarity.
If I was writing this code myself I'd probably use something like vslite_ctx since it's semantically and conceptually a dictionary of context for vslite, and the fact that it contains buildtype arguments is not semantically relevant to the API.
mesonbuild/backend/nonebackend.py
Outdated
| from mesonbuild.utils.core import MesonException | ||
|
|
||
| from .backends import Backend | ||
| from .. import mlog | ||
| from ..mesonlib import MesonBugException |
There was a problem hiding this comment.
You should import MesonException from mesonlib, not utils.core (the latter is primarily to allow having only parts of mesonlib imported during extremely speed-critical subprocesses).
There was a problem hiding this comment.
Although probably these could all be mesonbugexception, since it indicates a coding error in meson itself if these arguments are ever used for the wrong backend.
mesonbuild/backend/vs2010backend.py
Outdated
| # (pre-processor defines, include paths, additional compiler options) | ||
| # fields to use to fill in the respective intellisense fields of sources that can't simply | ||
| # reference and re-use the shared 'primary' language intellisense fields of the vcxproj. | ||
| def get_non_primary_lang_intellisense_fields( |
There was a problem hiding this comment.
Any particular reason these are freestanding functions as opposed to methods on the VS backend?
There was a problem hiding this comment.
I can suggest a few reasons why it might be preferable as a free function (over a method) -
- Making it a class method can make a reader incorrectly assume there's the possibility for affecting or interacting with intermal member state.
- Conversely, a free function, particularly one that has no need for any implicit member data/functionality (which is the case with this function) just makes it crystal clear to the reader what the inputs are and that there's no hidden, implicit inputs to muddy the reader's understanding of what's going on in the func.
- Following on from the point above, free funcs are very often easier (and at least never harder) to refactor (split, merge, relocate) than member funcs, and to reason about while refactoring.
- The function, in this case, doesn't need to use any members of self/Vs2010Backend.
- And really scraping the bottom of the barrel 😄 ... We avoid both needlessly typing 'self', that goes unused, as well as infinitesimally helping the python compiler/run-time out through avoiding it having to do its tinty bit of work to pass self to a function that doesn't use it.
There was a problem hiding this comment.
Having them as freestanding functions is fine. There is a Python code checker (I forget which) that actually complains if you have a method that does not use its self argument at all.
| backend = self.coredata.get_option(OptionKey('backend')) | ||
| backend_name = self.coredata.get_option(OptionKey('backend')) | ||
| from ..backend import backends | ||
| self.backend = backends.get_backend_from_name(backend, self.build, self) | ||
|
|
||
| if OptionKey('genvslite') in self.user_defined_options.cmd_line_options.keys(): | ||
| # Use of the '--genvslite vsxxxx' option ultimately overrides any '--backend xxx' | ||
| # option the user may specify. | ||
| backend_name = self.coredata.get_option(OptionKey('genvslite')) | ||
| self.backend = backends.get_genvslite_backend(backend_name, self.build, self) | ||
| else: | ||
| self.backend = backends.get_backend_from_name(backend_name, self.build, self) |
There was a problem hiding this comment.
Why don't we move the other (original) backend_name assignment to the else branch here?
There was a problem hiding this comment.
Yep. Well spotted. Done: 28ea078
GertyP
force-pushed
the
experimentalgensln
branch
from
f7b86b0
to
dcc9888
Compare
… case so moved it into else/non-genvslite block.
|
There is already a test case If need be, this can be merged after rc1 but hopefully these changes are the final ones. |
…lided with a newly added test dir again.
Fixed: 390d92c |
…e MesonBugException instead of MesonException.
|
I would just like to state for the record that this PR was not ready to be merged and was still under active review. Merging it early didn't fall under the category of "the perfect is the enemy of the good", either. Really not a fan of merging things "because deadline" when they are in the middle of receiving active review and the most recent activity was a request from the PR author for clarification of a review that was likewise offered that exact day. I would have responded except that the request occurred at 4 AM my time. |
| @@ -246,7 +253,10 @@ def _generate(self, env: environment.Environment) -> None: | |||
| fname = os.path.join(self.build_dir, 'meson-logs', fname) | |||
| profile.runctx('intr.backend.generate()', globals(), locals(), filename=fname) | |||
| else: | |||
| intr.backend.generate() | |||
| captured_compile_args = intr.backend.generate( | |||
There was a problem hiding this comment.
This is broken if profile-self is used, because the cProfile instrumented version of this code doesn't handle arguments or return values to generate().
@jpakkane are you sure you reviewed this PR?
There was a problem hiding this comment.
I noticed this while looking at my other thought for reducing the effect on the function signature for generate():
The shower thought that in theory one could save the vslite context as an instance variable and retrieve it from intr.backend after invoking intr.backend.generate. I haven't taken a close enough look to see which API is better, especially since it's a moot point at the moment... I'm not positive it's better because it might require paired functions at a minimum, one to set it and one to retrieve it. How elegant is that? 🤔
There was a problem hiding this comment.
I did review but I mostly go by tests. It is just plain impossible to consider all the ways things can fail. At the very least this has shown that there is no test for profile-self.
Since this MR seems to have some issues unless they get fixed in time for the release I'll revert this out. Sorry.
There was a problem hiding this comment.
This is broken if profile-self is used...
I'm not familiar with this profile path. I guess this isn't covered by any of the build check automation. Are there instructions for what we need to do to test this? Anything beyond just adding --profile-self in a test and ensuring succeeds (and in this case, fixing up the 'statement' arg to profile.runctx(...) to now match the new signature)?
The shower thought that in theory one could save the vslite context as an instance variable and retrieve it from intr.backend after invoking intr.backend.generate.
I also considered this kind of approach but wasn't keen on it (at least just yet) for a few reasons -
- If in doubt, I'll lean towards clarity, and strict, explicit code (params/state), over hiding params/state/dependencies/functionality/etc.
- Although it would make the generate() interface a little cleaner, I think the total amount of additional code surface area would be more overall.
- I think, if
intr.backendnow has to carry around some extra member state (flags to indicate we want it to provide the captured compile args when we call 'generate' and then extra member state to hold on to those results, post-generate, for when we might then request them), then the lifetime of that data is now much more sloppy and spread out than it is when we have a nice, tight, and strict window in which all this generate context/state is in play. - and less strict aspect of an interface would need a bit more validation to sanity check no one's trying to get hold of these results before they've been generated, and things like that.
- This would appear to be the first feature of meson (that I'm aware of) that needs to pull together results from multiple setup stages, so, at this time, it feels like it may be prematurely constructing some abstraction machinery, based on just one use case, perhaps in anticipation of some unknown future feature that might want other context data, on top of the captured compile args. Maybe someone will want to add something similar that needs slightly different data from multiple different backend 'generations' (e.g. some other IDE generator that simply glues together the ninja backends but also with access to more details on the individual targets... for whatever reason), at which point perhaps it might look better to hide away a growing list of generate flags/args and resultant data, accessed through a retrieval func... or it might still be OK to simply pass in to generate a structure that aggregates more options for returning captured state from 'generate' which returns a union type of different results depending on the options.
There was a problem hiding this comment.
At the very least this has shown that there is no test for profile-self.
We do test profile-self in general in AllPlatformTests.test_command_line, due to the default None value the effect of running the profile-self codepath is that no context is passed around, which is also what running non-vslite backends do.
So my assumption is that running meson setup --genvslite --profile-self builddir is broken because it passes around a null context, but needs it to contain something -- and everything inside Vs2010Backend(gen_lite=True) that accesses that dict is going to crash with various AttributeErrors when trying to treat a NoneType as a list[str]. We don't specifically check the combination of both, because genvslite is only tested by running WindowsTests.test_genvslite.
Anything beyond just adding
--profile-selfin a test and ensuring succeeds (and in this case, fixing up the 'statement' arg toprofile.runctx(...)to now match the new signature)?
Probably this is sufficient to fix that issue.
* Capture all compile args from the first round of ninja backend generation for all languages used in building the targets so that these args, defines, and include paths can be applied to the .vcxproj's intellisense fields for all buildtypes/configurations.
Solution generation is now set up for mutiple build configurations (buildtypes) when using '--genvslite'.
All generated vcxprojs invoke the same high-level meson compile to build all targets; there's no selective target building (could add this later). Related to this, we skip pointlessly generating vcxprojs for targets that aren't buildable (BuildTarget-derived), which aren't of interest to the user anyway.
When using --genvslite, no longer inject '<ProjectReference ...>' dependencies on which a generated .vcxproj depends because that imposes a forced visual studio build dependency, which we don't want, since we're essentially bypassing VS's build in favour of running 'meson compile ...'.
When populating the vcxproj's shared intellisense defines, include paths, and compiler options fields, we choose the most frequent src file language, since this means more project src files can simply reference the project shared fields and fewer files of non-primary language types need to populate their full set of intellisense fields. This makes for smaller .vcxproj files.
Paths for generated source/header/etc files, left alone, would be added to solution projects relative to the '..._vs' build directory, where they're never generated; they're generated under the respective '..._[debug/opt/release]' ninja build directories that correspond to the solution build configuration. Although VS doesn't allow conditional src/header listings in vcxprojs (at least not in a simple way that I'm aware of), we can ensure these generated sources get adjusted to at least reference locations under one of the concrete build directories (I've chosen '..._debug') under which they will be generated.
Testing with --genvslite has revealed that, in some cases, the presence of 'c:\windows\system32;c:\windows' on the 'Path' environment variable (via the make-style project's ExecutablePath element) is critical to getting the 'meson compile ...' build to succeed. Not sure whether this is some 'find and guess' implicit defaults behaviour within meson or within the MSVC compiler that some projects may rely on. Feels weird but not sure of a better solution than forcibly adding these to the Path environment variable (the Executable Path property of the project).
Added a new windows-only test to windowstests.py ('test_genvslite') to exercise the --genvslite option along with checking that the 'msbuild' command invokes the 'meson compile ...' of the build-type-appropriate-suffixed temporary build dir and checks expected program output.
Check and report error if user specifies a non-ninja backend with a 'genvslite' setup, since that conflicts with the stated behaviour of genvslite. Also added this test case to 'WindowsTests.test_genvslite'
I had problems tracking down some problematic environment variable behaviour, which appears to need a work-around. See further notes on VSINSTALLDIR, in windowstests.py, test_genvslite.
'meson setup --help' clearly states that positional arguments are ... [builddir] [sourcedir]. However, BasePlatformTests.init(...) was passing these in the order [sourcedir] [builddir]. This was producing failures, saying, "ERROR: Neither directory contains a build file meson.build." but when using the correct ordering, setup now succeeds.
Changed regen, run_tests, and run_install utility projects to be simpler makefile projects instead, with commands to invoke the appropriate '...meson.py --internal regencheck ...' (or install/test) on the '[builddir]_[buildtype]' as appropriate for the curent VS configuration. Also, since the 'regen.vcxproj' utility didn't work correctly with '--genvslite' setup build dirs, and getting it to fully work would require more non-trivial intrusion into new parts of meson (i.e. '--internal regencheck', '--internal regenerate', and perhaps also 'setup --reconfigure'), for now, the REGEN project is replaced with a simpler, lighter-weight RECONFIGURE utility proj, which is unlinked from any solution build dependencies and which simply runs 'meson setup --reconfigure [builddir]_[buildtype] [srcdir]' on each of the ninja-backend build dirs for each buildtype.
Yes, although this will enable the building/compiling to be correctly configured, it can leave the solution/vcxprojs stale and out-of-date, it's simple for the user to 'meson setup --genvslite ...' to fully regenerate an updated, correct solution again. However, I've noted this down as a 'fixme' to consider implementing the full regen behaviour for the genvslite case.
* Review feedback changes -
- Avoid use of 'captured_compile_args_per_buildtype_and_target' as an 'out' param.
- Factored a little msetup.py, 'run(...)' macro/looping setup steps, for genvslite, out into a 'run_genvslite_setup' func.
* Review feedback: Fixed missing spaces between multi-line strings.
* 'backend_name' assignment gets immediately overwritten in 'genvslite' case so moved it into else/non-genvslite block.
* Had to bump up 'test cases/unit/113 genvslites/...' up to 114; it collided with a newly added test dir again.
* Changed validation of 'capture' and 'captured_compile_args_...' to use MesonBugException instead of MesonException.
* Changed some function param and closing brace indentation.
|
Hi, nice work.
Could you explain why this feature is only available for VS 2022 but not for older, still supported VS versions? |
|
VS2022 was the only version I had to hand at the time so that was the only version I used and tested. I probably could have gone to the trouble of finding older versions to see if they'd work with the same set of xml fields used for these new makefile style vcxprojs, but it didn't seem necessary at the time. E.g. how far back should we check for VS support of the new project types/fields this uses and when we find a version that chokes, should we spend time digging in to seeing what project types or work-arounds can be supported by that version to have the same clean/build/rebuild hooks in that older VS version? So, as seems reasonable for these kind of things, it makes for less work and a clearer, simpler scenario to do just what's useful for the known users of this area at the time (i.e. me 😄 ) and leave additional features/niceties for later as the need/desire becomes apparent from use cases. |
Since VS 2017, there is alternative to using a xml project as 'Open Folder' which use a set of json files. I don't know if this feature has any advantage over the xml syntax. https://learn.microsoft.com/en-us/cpp/build/open-folder-projects-cpp?view=msvc-170 |
|
I don't know if it has any advantages either. |
Hello.
I don't claim this is fit for integrating; it's more a casual request for feedback and discussion on an experimental change to either learn why what I've added may be dumb or, if it useful, what other cases and issues need considering for it to progress to a point where it can be pulled.
I was a little puzzled at how meson generates a visual studio solution and project files for only a single build configuration ('buildtype' in meson terminology) and also at how the generated solution/projects are configured to use the native visual studio build system.
Both of these decisions seem quite strange to me because, although I like and appreciate the generation of V.S. solution and project files from meson build scripts, I (and I would have thought most others) would prefer to keep target builds/compilations within the domain of meson's expertise (not msbuild/Vis.studio) for various reasons -
Because of these, I've made an experimental local addition to meson to allow me to do -
> meson setup ... --genvslite vs2022 somebuilddirwhich will generate the standard ninja-backend build directories, 'somebuilddir_debug', 'somebuilddir_debugoptimized', and 'somebuilddir_release', each of which are equivalent to doing -
Additionally, it also generates another directory, 'somebuilddir_vs', which is similar to if I had done -
> meson setup ... --backend vs2022 somebuilddir_vsexcept the generated visual studio solution and projects are set up with the debug, debugoptimised, and release build configurations as well as having their build, rebuild, and clean steps set to simply invoke the relevant 'meson compile ...' command for the appropriate, previously configured 'somebuilddir_[...]'.
(* I think an important feature of meson is an ability to configure fully portable and absolutely explicit compiler/linker toolchains and portable standard library and sdk headers/libs, allowing the user total and explicit control to set up the entire build tools and infrastructure as a fully portable, self-contained set of directories so that the whole build system can be transplanted as a loose set of files/directories onto any other machine. This is something I think all build systems should have as a primary feature because implicit behaviour (hidden searching around and guessing for what tools, libraries, and settings should be used in a build, based on what can be found installed on the local machine) is a recipe for problems across different machines.
However, I would like to mention that fully explicit toolchains with meson is not particularly well introduced or publicised in the docs and its something I've found requires some care to try to fight against meson's innate tendency towards implicitly, automagically inspecting the local machine's environment to guess and flail for certain tools and settings. I've been thinking something like an '-explicit' option would be ideal in enforcing everything the build needs (toolchains, sdks, standard libraries, build options) is explicitly specified, while skipping all implicit and searching behaviours, and therefore guaranteeing reproducible, portable builds)