conan
conan copied to clipboard
Issue with 'automatic' compatible packages and custom package ids
ISSUE-16785: Issue with 'automatic' compatible packages and custom package ids
In case the recipe uses its own package_id() and compatible packages are generated automatically (e.g. Visual Studio vs msvc), then the compatible packages will use wrong 'base' info set for the compatible package id generation.
In case the recipe's own package_id() erases a few options / settings, the compatible packages will not know about it, as those are generated prior the recipe's package_id() is being called.
The change makes sure that first the recipe's package_id() is called, then the compatible packages are generated, so that they pick up the updated info set.
Changelog: (Bugfix): Automatically generated compatible packages are wrong in case of custom package_id() Docs: Omit
- [X] Refer to the issue that supports this Pull Request.
- [x] If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
- [x] I've read the Contributing guide.
- [x] I've followed the PEP8 style guides for Python code.
- [x] I've opened another PR in the Conan docs repo to the
developbranch, documenting this one.
@memsharded I've ran into a ''somewhat'' complex issue with regards to the msvc / Visual Studio compatibility :(
Currently "automatic" compatible_packages are generated prior calling the package_id(). This has a drawback though that if the package_id() erases this-and-that, then the compatible packages will look for wrong packages (as those still contain the "this-and-that" part of the options/settings).
I have tested locally and fixes my use-case, though I have no clue how intrusive the change is :( Please advise, Thanks,
Hi @memsharded,
Thanks for the reply. I see the following use cases:
- There is no package_id() in the recipe or the package_id() doesn't alter self.info (which is kinda strange on its own, as why would you have a package_id() if it doesn't do actually anything with self.info). In this case the changes are irrelevant, as the compatibility layer will work on the exact same self.info as before
- There is a package_id() and it alters self.info but the recipe doesn't use 'msvc' by any means. In this case the changes are also irrelevant (as the msvc compatible package logic is not triggered).
- There is a package_id() and it alters self.info and the recipe uses 'msvc'. In this case the PR is mandatory as it fixes a currently buggy behavior
- There is a package_id() and it uses for some strange reason self.compatible_packages to do something with self.info. In this case the PR will break this recipe.
Sadly I have zero idea how realistic the last use case is :)
I am afraid that it will not be possible to move this forward. I have added to your PR a simple test:
def test_crash():
client = TestClient()
conanfile = textwrap.dedent("""
from conans import ConanFile
class Pkg(ConanFile):
settings = "os", "compiler"
def package_id(self):
del self.info.settings.compiler.version
""")
profile = textwrap.dedent("""
[settings]
os = Windows
compiler=msvc
compiler.version=192
compiler.runtime=dynamic
compiler.cppstd=14
build_type=Release
""")
client.save({"conanfile.py": conanfile,
"myprofile": profile})
client.run("create . pkg/0.1@ -pr=myprofile")
It will crash because of your change (it works fine in develop branch).
It is very likely that users will have a package_id removing something like del self.info.settings.compiler.version that will make it fail.
@memsharded Let me argue with that conclusion of yours a bit :)
Afaik the test should be doing this:
- you create a package with "msvc" "compiler.version=193"
- a compatibility package with "Visual Studio" will be created including the corresponding "compiler.version=17"
- you erase "compiler.version" from your package indicating that it should be compatible with "compiler.version=192" (and compiler.version=16) as well
- you try to consume that package with a "Visual Studio" profile that has "compiler.version=16" set, it will fail as within the compatible package automatically added with msvc_compatible you'll still have "compiler.version=17" explicitly set)
- surely you can consume it with "msvc" and with "compiler.version=192" (because of the package_id() behavior)
What do I miss?
[Anyhow added a commit, that checks if a certain compiler.* variable is present at all]
Understood, thanks a lot! Just a quick recap, what would be the release date (roughly) for 1.66? (Our company is blocked from transitioning to Conan 2 without this - obviously I can hack it around via using custom package_id() functions that add another msvc_compatible compatible package with the proper info)
I have absolutely no idea how /why Github has closed the PR :(, reopening it
@memsharded, it seems this is the last issue to be resolved before 1.66.0 could be released. Due to #16930 it would enable the hopefully last compiler update before migrating to Conan 2 for us. @harsszegi any update from your side, do you still need this PR?
Hi @petermbauer, obviously I’d like to see it merged to Conan 1, as I still think the fix is the correct approach to handle Visual Studio / msvc migration, but I also understand if you guys think it’s risky and don’t want it to cause any issue. I have implemented a workaround at our end, but surely that has some limitation (e.g it requires the consumer to use a python requires class). Anyhow the choice is yours :)
thanks a lot @memsharded !
Sorry this took some time, thanks @harsszegi for the contribution and to you for the feedback!
Thanks @memsharded for merging this! I owe you one :)
No, of course not, thanks to you for contributing, and sorry this took some time to release! Cheers!