[bug] Compatible cached packages not resolved correctly

[burgerkiller79]

Environment Details (include every applicable attribute)

• Operating System+version: Windows 10 64
• Compiler+version: VS 2019 16
• Conan version: 1.41.0
• Python version: 3.8.8
• Revisions enabled: True

Steps to reproduce (Include if Applicable)

• Assume packages A, B, C, D and two build settings Settings1 and Settings2 resulting in different package ids.
• B requires A
• C requires A
• D requires both B and C
• A, B and C have been built with Settings1 and each override package_id to provide a compatible package for Settings2.
• D needs to be built with Settings2

If you call conan install on D with Settings2 we observer the following:

• A, B and C are reported as missing but a compatible package has been found:
  A/1.0@user/channel: Main binary package 'f278b6f5a018dfdaf3f1aaf111c91b35b8dbeec5' missing. Using compatible package 'f85619e3f4a4d53fb5b48d3c40437fc2f08c69e6'
B/1.0@user/channel: Main binary package 'f278b6f5a018dfdaf3f1aaf111c91b35b8dbeec5' missing. Using compatible package 'f85619e3f4a4d53fb5b48d3c40437fc2f08c69e6'
C/1.0@user/channel: Main binary package 'f278b6f5a018dfdaf3f1aaf111c91b35b8dbeec5' missing. Using compatible package 'f85619e3f4a4d53fb5b48d3c40437fc2f08c69e6'
• A might end up multiple times within the self.info.requires dict used in the package_id methods of B and C due to different package ids which leads to wrong hashes being created.
• If we workaround the previous issue by throwing out the multiple occurrances of the same package requires within package_id, the wrong package binary is finally used for A:
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\installer.py", line 560, in _handle_node_cache
assert os.path.isdir(package_folder), ("Package '%s' folder must exist: %s\n"
AssertionError: Package 'A/1.0@user/channel:f278b6f5a018dfdaf3f1aaf111c91b35b8dbeec5' folder must exist: C:\.conan\3785e5\1

Perhaps this might be related to #9446

Possible Fix ?

• From debugging we found that there exist two nodes in the conan graph for package A, which originally point to the same package id.
• For the first node A _evalute_node from file graph_binaries.py is called and compatible packages are computed. Then, the _package_id of that node is set to the compatible package id f85619... and the node.binary is set to cached mode.
• For the second node A _evaluate_is_cached in the same file encounters the first node A (previous_nodes) and sets its binary also cached as well. However, the _package_id is not modified.

If we set the _package_id to the previous_node everything works fine but we are not sure if this has any unwanted side effects:
node._package_id = previous_node._package_id

I hope that this helps in resolving the bug.

[memsharded]

Hi @burgerkiller79

As a first, quick feedback, I would like to comment that we are thinking about deprecating the compatible_packages feature. It is a very complicated and problematic one, and the value is limited. Mostly because the selection of the settings to be applied for each package can easily be defined in profiles (like D:setting1=value1 settings1=value), and also for many packages, a profile can easily maintain a list with jinja to keep it very short. We will submit this to discussion with the Tribe for planning for Conan 2.0 soon.

We will have a look at this bug, thanks very much for your detailed report and investigation.

[burgerkiller79]

Hi @memsharded and thank you for the quick response.

We are currently using compatible_packages a lot with the use case of creating packages with multiple builds (debug and release) and providing compatible package ids for all of the involved build types. This is mostly due to us having a complete build system based on CMake and its config scripts, which requires having all build types within the packages for switching between these build types in the IDE.

But some users of our packages apply the standard single build_type approach and we didn't want to create additional packages for those cases, so we use the compatible package feature.

With your changes for Conan 2.0, do you think this approach ist still possible?

[memsharded]

Do you know that CMakeDeps generator allows to switch build_type from the IDE, without doing that? It works well even using editable packages, if you want to see the updated section in: https://docs.conan.io/en/latest/developing_packages/editable_packages.html (and this was the base for a demo with Visual Studio done in the last meetup, if you want to check the live video in https://www.youtube.com/watch?v=VzUJQw89U7o

[burgerkiller79]

Thank you for the links and nice talk - good to see some hands on live footage :).

We make heavy use of the workspace install feature with lots of dynamic libraries involved for use in the IDE until the point we are creating the packages. So far, the reason that kept us from using single builds for our packages was that CMake find_package does not support adding build type target scripts from different folders. And recreating all the CMake find info within the conanfile package_info for the conan imported targets is actually a problem with more complicated libraries (like Qt, ITK, VTK, etc), that's what we use the cmake config scripts for.

But CMakeDeps is new to me. If this enables us to somehow use the standard config scripts for finding the dependencies and it will take care of handling build type specific target infos, this would be amazing.

I will look into this, thank you very much!

[memsharded]

I am trying to reproduce this, this is the test I have so far:

    def test_compatible_diamond(self):
        # https://github.com/conan-io/conan/issues/9880
        client = TestClient()
        conanfile = textwrap.dedent("""
            from conan import ConanFile
            class Pkg(ConanFile):
                {}
                settings = "build_type"
                def package_id(self):
                    if self.settings.build_type == "Debug":
                        compatible_pkg = self.info.clone()
                        compatible_pkg.settings.build_type = "Release"
                        self.compatible_packages.append(compatible_pkg)
                """)

        client.save({"pkga/conanfile.py": conanfile.format(""),
                     "pkgb/conanfile.py": conanfile.format('requires = "pkga/0.1"'),
                     "pkgc/conanfile.py": conanfile.format('requires = "pkga/0.1"'),
                     "pkgd/conanfile.py": conanfile.format('requires = "pkgb/0.1", "pkgc/0.1"')
                     })
        client.run("create pkga pkga/0.1@ -s build_type=Release")
        client.run("create pkgb pkgb/0.1@ -s build_type=Release")
        client.run("create pkgc pkgc/0.1@ -s build_type=Release")

        client.run("install pkgd -s build_type=Debug")
        print(client.out)

Unfortunately, I don't see duplicated items in the self.info.requires of the packages. Could you please check this and elaborate a bit more how you are seeing them? Any help to try to reproduce this? Thanks!

[burgerkiller79]

Thanks for looking into this. I will try to reproduce the error with your test code and modify it accordingly.

[burgerkiller79]

I guess the reason for multiple entries in the graph is that one package declares a requirement as private, while another package uses it as normal dependency.

It can be reproduced with 3 packages, where pkga is used as private requirement by pkgb and both pkga and pkgb are requirements of pkgc.

I've changed your code to the following test case:

def test_compatible_diamond():
    # https://github.com/conan-io/conan/issues/9880
    client = TestClient()
    conanfile = '''
from conans import ConanFile
class Pkg(ConanFile):
    settings = "build_type"

    def package_id(self):
        compatible_pkg = self.info.clone()
        if self.settings.build_type == "Debug":
            compatible_pkg.settings.build_type = "Release"
            self.compatible_packages.append(compatible_pkg)

    {}
    '''

    client.save({"pkga/conanfile.py": conanfile.format(""),
                 "pkgb/conanfile.py": conanfile.format('''
    def requirements(self):
        self.requires("pkga/0.1", "private")'''),
                 "pkgc/conanfile.py": conanfile.format('''
    def requirements(self):
        self.requires("pkga/0.1")
        self.requires("pkgb/0.1")''')
                 })
    client.run("create pkga pkga/0.1@ -s build_type=Release")
    client.run("create pkgb pkgb/0.1@ -s build_type=Release")
    client.run("create pkgc pkgc/0.1@ -s build_type=Debug")
    print(client.out)

But it turns out that this bug a rather tricky one, as its occurrence is non-deterministic and I wasn't able to reproduce it from the pytest call, at all. However, if I make the call to create pkgc from the command line, it will fail roughly 3 out of 10 times.

I ran this with python 3.8 and conan 1.41.0 .

This is the output in case it fails:

C:\Projects\conan\pkgid-debug>conan create pkgc pkgc/0.1@ -s build_type=Debug
Exporting package recipe
pkgc/0.1: The stored package has not changed
pkgc/0.1: Using the exported files summary hash as the recipe revision: 6fe7e1268fe335f082c6fa3b3271b4f2
pkgc/0.1: Exported revision: 6fe7e1268fe335f082c6fa3b3271b4f2
Configuration:
[settings]
arch=x86_64
arch_build=x86_64
build_type=Debug
compiler=Visual Studio
compiler.runtime=MDd
compiler.version=16
os=Windows
os_build=Windows
[options]
[build_requires]
[env]

pkga/0.1: Main binary package '5a67a79dbc25fd0fa149a0eb7a20715189a0d988' missing. Using compatible package '4024617540c4f240a6a5e8911b0de9ef38a11a72'
pkgb/0.1: Main binary package '7a8b95543b4e3738be01d064adc3f6c0449c9df5' missing. Using compatible package '96a5c7bd97e984b4bd30026de5908eea95ec48bd'
pkgc/0.1: Forced build from source
Installing package: pkgc/0.1
Requirements
    pkga/0.1 from local cache - Cache
    pkga/0.1 from local cache - Cache
    pkgb/0.1 from local cache - Cache
    pkgc/0.1 from local cache - Cache
Packages
    pkga/0.1:4024617540c4f240a6a5e8911b0de9ef38a11a72 - Skip
    pkga/0.1:5a67a79dbc25fd0fa149a0eb7a20715189a0d988 - Cache
    pkgb/0.1:96a5c7bd97e984b4bd30026de5908eea95ec48bd - Cache
    pkgc/0.1:3c5ae3b9400202e3edbf274032f2ca1f3b8c14c5 - Build

Installing (downloading, building) binaries...
pkga/0.1: Already installed!
Traceback (most recent call last):
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\command.py", line 2232, in run
    method(args[0][1:])
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\command.py", line 382, in create
    info = self._conan.create(args.path, name=name, version=version, user=user,
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\conan_api.py", line 93, in wrapper
    return f(api, *args, **kwargs)
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\conan_api.py", line 388, in create
    create(self.app, ref, graph_info, remotes, update, build_modes,
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\cmd\create.py", line 84, in create
    deps_install(app=app,
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\manager.py", line 83, in deps_install
    installer.install(deps_graph, remotes, build_modes, update, profile_host, profile_build,
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\installer.py", line 316, in install
    self._build(nodes_by_level, keep_build, root_node, profile_host, profile_build,
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\installer.py", line 451, in _build
    self._handle_node_cache(node, keep_build, processed_package_refs, remotes)
  File "c:\programdata\anaconda3\lib\site-packages\conans\client\installer.py", line 560, in _handle_node_cache
    assert os.path.isdir(package_folder), ("Package '%s' folder must exist: %s\n"
AssertionError: Package 'pkga/0.1:5a67a79dbc25fd0fa149a0eb7a20715189a0d988' folder must exist: C:\conan\data\pkga\0.1\_\_\package\5a67a79dbc25fd0fa149a0eb7a20715189a0d988

As you can see, pkga is listed twice as requirement and the correct package id is chosen initially, however while resolving the packages from cache, it skips the correct one.

Hope this helps.

[memsharded]

Thanks very much for putting the effort to reproduce.

Indeed using private dependencies could have more issues. This feature in Conan 1.X is barely documented, because it has some rough edges and it is fragile. I will have a look, but it is possible that this cannot be fixed properly in 1.X. Conan 2.0 has implemented a new graph model, with a much better built-in management of "visibility" of dependencies that we hope it will improve over this use case.

[burgerkiller79]

That is an impressive response time, thank you. I really need to dig into conan 2.0 .

Just as a remark: we have ran our patched conan version with the "possible fix" I've mentioned in my initial post for quite some time now with also complex graph structures. We never had any issues with compatible packages so far.

This is the change in the according method in the client/graph/graph_binaries.py file:

    def _evaluate_is_cached(self, node, pref):
        previous_nodes = self._evaluated.get(pref)
        if previous_nodes:
            previous_nodes.append(node)
            previous_node = previous_nodes[0]
            # The previous node might have been skipped, but current one not necessarily
            # keep the original node.binary value (before being skipped), and if it will be
            # defined as SKIP again by self._handle_private(node) if it is really private
            if previous_node.binary == BINARY_SKIP:
                node.binary = previous_node.binary_non_skip
            else:
                node.binary = previous_node.binary
            node.binary_remote = previous_node.binary_remote
            node.prev = previous_node.prev

            # this line fixed it for us:
            node._package_id = previous_node._package_id

            return True
        self._evaluated[pref] = [node]

[memsharded]

Great, I have been able to reproduce with your test changes too.

Also reviewed your fix, it is indeed looking good, exactly in the spot, great job. Thanks for all the work. I have submitted PR #10266 for 1.45, but if you want to submit it yourself to become official contributor, you totally deserve it, just let me know and I will close my PR.

This will be naturally merged to develop2 branch too, lets try to make sure that this continue to work in 2.0 (except a new way to define visibility will be introduced).

[burgerkiller79]

I'm glad I was able to contribute something. I don't necessarily have to submit the PR, especially since you already did it, but thanks for the offer.