[feature] MSBuildDeps | Configuration and Platform keys for property sheets

[vermz99]

Changelog: Feature: Add Configuration and Platform keys for MSBuildDeps property sheets.
Docs: conan-io/docs#3888

Description

Context

This PR is an attempt to add a bit more customization to the MSBuildDeps class for large monorepos visual studio solutions. The current Visual Studio extension for conan is not practical for solutions with hundreds of projects. The ideal and more user friendly workflow in this case is to have a single conanfile that is maintain in the solution. To work effectively, this requires the user to create a new visual studio project (e.g., conanproject.vcxproj) and add it to its solution. With a bit of custom build step and build ordering magic in the conanproject.vcxproj file, the user can ensure that conan is ran at each build of its solution.

The problem arise when your configuration in your conanproject.vcxproj differ from the configurations selected of some other projects in the same solution. For CI purpose, you often find yourself in the position of having to do multiple configurations for different projects and having to mix them in the final build of the solution. To be able to correctly import the property sheets generated by conan in your other projects, you need to be able to set a new "Configuration" variable in your project .vcxproj to match to the one from conan.

This is hard to explain, please ask questions or clarifications if this is not clear enough!

Solution

What worked very well for us was to overload the _condition method in the MSBuildDeps class and provide a custom Configuration variable name. The thing is, overloading a "private" method is not great and could be regarded as undefined behavior, so i'm proposing to make this into an additional customization point for everyone. I added customization point for platform as well as filename to extend the possibilities. In the issue described above, having only the self.configuration_condition_key variable solve the problem.

TODO

• Review

• Add documentation - Documentation on this might confuse basic user, need feedback if needed.

• Refer to the issue that supports this Pull Request. N/A

• If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.

• I've read the Contributing guide.

• I've followed the PEP8 style guides for Python code.

• I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

[CLAassistant]

All committers have signed the CLA.

[vermz99]

Not sure, but this might partially help #16276. @sykhro, any opinion?

[memsharded]

Thanks very much for your contribution @vermz99

I like it that as-is, it is quite safe, in the sense that it won't break any existing usage.

But I'd like to understand a bit better what is the use case, if we could start with a (as simple as possible) test for the test suite, that illustrate the expected UX and usage, and does some basic checking of the output (like checking the generated .props to see they are what are expected), also to guarantee that things do not break in the future. (this could go to an test/integration/toolchains/microsoft.

[sykhro]

Not sure, but this might partially help #16276. @sykhro, any opinion?

I will think about it a bit - need to check what's the situation with different vendors. At the moment I'm using CMake everywhere and bypassing the problem entirely

[memsharded]

Hi @vermz99

Did you check my above comment about adding some tests?
This will really help to move this forward, please ask for advice or help with providing the tests, we can help with that, if you need.

[AlexVermette-Eaton]

@memsharded Hey James, sorry for the delay, I got half of it done and should be able to complete soon. I was thinking of validating that the generated props file contains the expected values for the key of Configuration and Platform as well as the filename.

I'm not sure how to build a more extensive test case without actually using Visual Studio here.

[memsharded]

Hey James, sorry for the delay, I got half of it done and should be able to complete soon. I was thinking of validating that the generated props file contains the expected values for the key of Configuration and Platform as well as the filename.

Great, sure, no hurries, I just wanted to follow up in case you needed some help with that.

This is enough, no need for a test that really uses VS.
The main idea would be to illustrate and capture in the test the UX of the conanfile.py, how users define and use this, if it is intended to have a connection with custom settings.yml, etc. Something that captures the use case without necessarily doing a full compilation.

[vermz99]

@memsharded Just added integration tests. I rolled back the changes related to filename since i could not find a use case. I also clarified the function related to filename to avoid confusion in the future.

🤔 Let me know what you think. I started with pretty heavy combinatory testings, but realized it might be too much for such a simple feature.

Also, not sure what is wrong with CI since I cannot access it.

[memsharded]

🤔 Let me know what you think. I started with pretty heavy combinatory testings, but realized it might be too much for such a simple feature.

Indeed, not necessary heavy combinatorics over this, just something simple, one success and one fail scenario to cover the basics use cases and ensure the feature is not broken in some refactor or other changes

Also, not sure what is wrong with CI since I cannot access it.

Yes, we had to make it private for security reasons, but we are in the process to move it to public again, hopefully soon. Don't worry at the moment about failed builds, we will check them.

[memsharded]

Thanks for your contribution.
I think it can be merged, for next Conan 2.9 release

[memsharded]

The None, None case could have been simplified, as this would already be covered by other tests.
But no prob, once it is done, we can keep it!