New msbuild generator that creates 1 .props file per dependency (multi-configuration) - #7035
Conversation
jgsogo
left a comment
There was a problem hiding this comment.
I like creating one .props file per requirement, but I need more explanations about how this generator is supposed to work and if it works out of the box or the user needs to manually add these property sheets to the projects in the solution.
| return name.lower(), condition | ||
|
|
||
| def _multi(self, name_multi, name_conf, condition): | ||
| # read the existing mult_filename or use the template if it doesn't exist |
There was a problem hiding this comment.
I really don't like reading and modifying files, it is against the generator paradigm we have until now where all the information for the generator is provided by Conan and all files could be generated using a template
There was a problem hiding this comment.
This is not new. The visual_studio_multi generator has been doing this for a very long time. It is the only way to avoid load errors, or you need to conan install all the configurations before you can open the IDE, and that was very annoying. Furthermore, the use case here was more dynamic, in cmake_multi only the build_type is involved, but here there were requested changes based on the toolset too.
There was a problem hiding this comment.
OMG, VS multi generators are evil then
There was a problem hiding this comment.
Yes, a bit, but on the good side, it works surprisingly well, because it is XML, so we haven't had big issues. I'd say that it works more smoothly than the cmake_multi one.
jgsogo
left a comment
There was a problem hiding this comment.
Many minor suggestions.
Need someone using Windows to test it.
Not needed for this PR, but I want to comment it here. In the CMake world we are planning to generate a file with the variables and then the file for the cmake_find_package generator or the cmake one that includes that one. I don't know if we ever will need that same flexibility here, but implementation can be almost the same:
- File
conan_zlib_release_x64_v141.propscontains only the<PropertyGroup Label="ConanVariables">section with all the<Conanzlib....variables. - File
conan_zlib.propsimports the properconan_zlib_xxxxx.propsfile and then populates the differentPropertyGroupandItemDefinitionGroup.
Totally agree. When we come up with a pattern for cmake (1 file with all vars, 1 file per dependency), we will replicate the same pattern here |
jgsogo
left a comment
There was a problem hiding this comment.
Looking good! ...but I need some Windows to actually test it works without flaws.
I really think that moving everything but the ConanVariables from the conan_zlib_xxxxxx.props file to the conan_zlib.props one will reduce code duplication and will make generated files easier to follow (even if those files are not intended to be read by the user). I think this doesn't need to wait for the CMake refactor.
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
Done. Please review again @jgsogo |
danimtb
left a comment
There was a problem hiding this comment.
Nothing relevant. It works as expected with Visual Studio 2019 even with debug/release configurations
Changelog: Feature: New experimental
msvcgenerator that generates a .props file per dependency and is also multi-configuration.Docs: conan-io/docs#1732
Close #7017
Many thanks to @birsoyo!!!
#tags: slow
A bit of context/vision:
conan_zlib.props, not each individual configurationconan_zlib_release_v141.props), as the generator accounts for the conditional logic to differentiate between Platforms, Toolsets and build_typeMSBuildhelper to inject specific .props file for subprojects, it seems that the only way is let the user define their dependencies.toolchain()to automate the injection of the globalconan_deps.propswhich adds all the direct dependencies, it could be doable, but not sure we want to do it.