Fix require_overrides for virtual packages

[fredizzimo]

Changelog: Bugfix: Workspaces with multiple root nodes did not respect require_overrides, but now they are passed correctly

• Refer to the issue that supports this Pull Request.
• 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.

When testing my custom workspace functionality, I noticed that the require_overrides were not passed along, which seems like a mistake, since it's passed with all other types of root packages. So this fixes that.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[memsharded]

Thanks for your contribution.

Could you please try to provide a test that is fixed by this? Much better if it doesn't use the workspace feature, as it has been removed from Conan 2.0 (to be replaced by user scripts until we resume work on workspaces in 2.X). Ask for guidance if you need it, thanks!

[fredizzimo]

Yes, of course. I will try to find a way to do that.

[fredizzimo]

Hi @memsharded,

I think I need some help with the testing. I found similar tests to what I need in install_test.py. The problem is that they all test the public cli interface, and as far as I can see there's no way to create a "virtual package" through that currently.

I'm able to to call the api install_reference (passing an array of references) to test the functionality of this pull request. However, the TestClient does not have any functionality for capturing the output when doing that. I also don't see any other tests that are testing the API directly for an example.

I also don't see any similar tests at the unit test, or functional level, and I'm not even sure if testing this is possible at that level, at least not without heavy mocking.

So, I see a couple of alternatives,

1. Add a run_api function to the TestClient, which works exactly as testing the command line.
2. Add the possibility of specifying multiple references to the install command. But that's a bit bigger change, which at least need additional tests, and additionally might not be something you want to expose.

What do you think? I'm not familiar with the code so you probably have some better suggestions.

[memsharded]

I see what you mean. I was possibly recalling the Conan 2.0 codebase, where you can do conan install --requires=pkga/1.0 --requires=pkgb/2.0, so it is easier to fire this behavior without workspaces. You are probably right, and it is not possible to test this without workspaces, the only public cli using this api is the workspaces.

Maybe we can just merge it as-is. If you have tested the fix locally, and nothing else breaks, we might be good, workspaces are removed in 2.0 anyway (so I'd recommend to try to move out of workspaces to a custom script and then feed that feedback when we resume work on 2.0 new workspaces)

[fredizzimo]

Yes, this fix works for me locally, and the existing tests passes here in this pull request, so it should be fairly safe to merge. But the call is up to you.

Note, that we have some completely custom workspace functionality locally. It uses cmake add_subdirectory, and has a custom CMakeDeps generator, like discussed here #11224. This works very well for our use cases. It also uses a custom API function by deriving from ConanAPI, so that I'm able to use temporary editable packages, and I'm able to use the public released conan version.

Unfortunately I'm not able to share the code right now, but at some point, when it has been proven a bit more on our side, I would want to work on enabling that functionality entirely through the public conan API. I think that an install command that can take a list of packages to put into editable mode (but don't actually install or compile those), and a list of root package references would be enough, in addition to the toolchain changes of course. The root CMakeLists.txt file with all the add_subdirectory calls can then be done from the outside, without using conan.

If this kind of functionality was provided then we could also add tests for these virtual packages.

[memsharded]

Hi @fredizzimo

I am very sorry, it seems this PR fell through the cracks (It sometimes happen due to high load, and also during the summer that many of the team take some vacation...)

I guess that you found a workaround around this, so maybe this is no longer needed? Also, this was for 1.X, and we are avoiding doing changes to 1.X unless very strictly necessary. Please let me know if we can close this PR, or you'd still want to move it forward.

[fredizzimo]

@memsharded

Yes, indeed we have a workaround. We used a very dirty hack of patching the function directly from our code.

Now when we are in progress of updating to Conan 2.0, that hack does not seem to be necessary, api.graph.load_graph_requires works as it should.

Closing this.

[memsharded]

For workspace feature in 2.0 it would be great to count with your feedback, this is the ticket that you should follow to track progress when we resume this for 2.X roadmap: #12466. Thanks again!