Repository: conan-io/conan
Pull request: try to fix package_id with package_revision_mode and mixed modes
Changelog: Bugfix: Fix crash while computing the ``package_id`` of a package when different ``package_id_mode`` are mixed and include ``package_revision_mode``.
Docs: Omit

Continuation of https://github.com/conan-io/conan/pull/6947
Fix #6942

#tags: slow

cc/ @fulara 

Problem statement:
[bug] unable to build packages with package_revision_mode enabled.
Conan crashing after enabled package_revision_mode and tried to rebuild all of our projects.
stacktrace attached.

### Environment Details (include every applicable attribute)
  * Operating System+version: rh7
  * Compiler+version: gcc8
  * Conan version: 1.24.0
  * Python version: 3.6.10

### Steps to reproduce (Include if Applicable)
Have a complex graph tree.
I dont have mve at the moment, but i'll try to pinpoint something.

Global package_mode would be set to 'package_revision_mode'.
In some of the recipes we are using default versioning_schema.
In some of the recipes we are specifying use semver_mode for some of its dependencies.
In some of the recipes we are specifying use of full_package_mode for some of its dependencies.

### Logs stacktrace.
invoked: conan create . --build missing
```
30-Apr-2020 16:37:19 xyz/abc: Unknown binary for xyz/abc, computing updated ID
30-Apr-2020 16:37:19 Traceback (most recent call last):
30-Apr-2020 16:37:19 File "conan/conans/client/command.py", line 2002, in run
30-Apr-2020 16:37:19 File "conan/conans/client/command.py", line 369, in create
30-Apr-2020 16:37:19 File "conan/conans/client/conan_api.py", line 89, in wrapper
30-Apr-2020 16:37:19 File "conan/conans/client/conan_api.py", line 368, in create
30-Apr-2020 16:37:19 File "conan/conans/client/cmd/create.py", line 57, in create
30-Apr-2020 16:37:19 File "conan/conans/client/manager.py", line 75, in deps_install
30-Apr-2020 16:37:19 File "conan/conans/client/installer.py", line 309, in install
30-Apr-2020 16:37:19 File "conan/conans/client/installer.py", line 404, in _build
30-Apr-2020 16:37:19 File "conan/conans/client/graph/graph_binaries.py", line 347, in reevaluate_node
30-Apr-2020 16:37:19 File "conan/conans/client/graph/graph_binaries.py", line 319, in _compute_package_id
30-Apr-2020 16:37:19 File "conan/conans/model/info.py", line 540, in package_id
30-Apr-2020 16:37:19 File "conan/conans/model/info.py", line 216, in sha
30-Apr-2020 16:37:19 TypeError: '<' not supported between instances of 'NoneType' and 'str'
```

Hints from issue discussion:
Checking the trace, I cannot see how a None gets there. Please keep us tuned if you can reproduce, and if not, I will try to provide a branch with some traces so you can run in your environment.
@memsharded its 100% reproducible for me, i just have to wait for full build ( we only upload at the end of the build ) so it takes ~25 imnutes to build.

Just checkd - same stacktrace, if you want me to drop somethig - np.
Actually I was thinking that I will have to postpone this migration. 
Hi @fulara 

Quick question, do you have enabled ``full_transitive_package_id`` in your conan.conf configuration?
Also adding these two lines might help isolating the origin of the bug:

```patch
diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py 
index 16de60e89..e0729103e 100644                                                          
--- a/conans/client/graph/graph_binaries.py                                                
+++ b/conans/client/graph/graph_binaries.py                                                
@@ -294,6 +294,9 @@ class GraphBinariesAnalyzer(object):                                   
             direct_reqs = node.id_direct_prefs                                            
             indirect_reqs = node.id_indirect_prefs                                        
                                                                                           
+        assert None not in direct_reqs, "None found in direct_reqs"                       
+        assert None not in indirect_reqs, "None found in indirect_reqs"                   
+                                                                                          
         python_requires = getattr(conanfile, "python_requires", None)                     
         if python_requires:                                                               
             if isinstance(python_requires, dict):                                         
```
@memsharded  unless I made mistake ( I dont think I have ) this assert didnt trigger.
yes, of course i am using `full_transitive_package_id` after all it wouldnt be fair if i hadnt? :) 
https://github.com/fulara/conan/tree/package_revision_mode

EDIT:
i added one more silly print:
```
        print("PRINTING NOW STUFF! \n")
        for key, value in self._data.items():
            print("KEY IS" + str(key) + " value is: " + str(value) + + " dumps: " + value.dumps() + " \n ")
```
the result i got just before failing is:
```
PRINTING NOW STUFF! 

KEY ISboost/1.72.0:634ce480c172f753ce13327c4f9f6d3a5eabcc32 value is: <conans.model.info.RequirementInfo object at 0x7f5a07491f60> dumps: boost/1.72.0:634ce480c172f753ce13327c4f9f6d3a5eabcc32 
 
KEY ISrfa-server-lib/4.13:4e6ce351510b1e0711182fec674aed81b210cd32 value is: <conans.model.info.RequirementInfo object at 0x7f5a07491ba8> dumps: rfa-server-lib/4.Y.Z 
 
KEY ISrfa-convert-lib/2.5:d62d48a0c0e3e9eced23f5c6a0926139d0ff8478 value is: <conans.model.info.RequirementInfo object at 0x7f5a07d0ceb8> dumps: rfa-convert-lib/2.Y.Z 
 
KEY ISigcounters/1.3:ddb66222e853666802a54adae5ac3e9befedc54b value is: <conans.model.info.RequirementInfo object at 0x7f5a07d0c6a0> dumps: igcounters/1.Y.Z 
 
KEY ISpoco/1.9.4:ed4013c58aa4bd9377abede5e8e1db2513b6c7d0 value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8ae80> dumps: poco/1.9.4:ed4013c58aa4bd9377abede5e8e1db2513b6c7d0 
 
KEY ISdisruptor/2.4:55d8a52d22c8588a7455fc66d9546a9464da6adb value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8a550> dumps: disruptor/2.Y.Z 
 
KEY ISopenssl/1.0.2t:e54af4a8e0cd6901bb01dd9c8925a8859b6246b2 value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8aa90> dumps: openssl/1.0.2t:e54af4a8e0cd6901bb01dd9c8925a8859b6246b2 
 
KEY ISlibcli/1.9.7:f10ff948ad9a5779ed97e7f1c2f2e4c8cd675372 value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8aeb8> dumps: libcli/1.Y.Z 
 
KEY ISbzip2/1.0.8:0d28bbf593474851d2bc7d2ef0a546fcdc0233fe value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8af60> dumps: bzip2/1.0.8:0d28bbf593474851d2bc7d2ef0a546fcdc0233fe 
 
KEY ISrfa/8.0.1.E1:dc7e592c7c92901df7003480a162da6bd8500cbd value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8a048> dumps: rfa/8.0.1.E1:dc7e592c7c92901df7003480a162da6bd8500cbd 
 
KEY ISzlib/1.2.11:7d0f19a52d7be613ac3eb2f1ea1b8cc359e0bfe0 value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8a1d0> dumps: zlib/1.2.11:7d0f19a52d7be613ac3eb2f1ea1b8cc359e0bfe0 
 
KEY ISboost/1.72.0:634ce480c172f753ce13327c4f9f6d3a5eabcc32 value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8af28> dumps: boost/1.72.0#3c1a4170f35bcad9109cb8bc720d56d1:634ce480c172f753ce13327c4f9f6d3a5eabcc32#PREV unknown 
 
KEY ISfin-pricing-utils/2.3:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 value is: <conans.model.info.RequirementInfo object at 0x7f5a03b8a0b8> dumps: fin-pricing-utils/2.Y.Z 
```
So its some member variable of RequirementInfo thats null. 
@memsharded  looking at the stacktrace I think its because one of the 'boost' is depended on with package_recipe_mode and the other one is full_package_mode ( which is valid scenario ) and you are missing a none check somewhere where comparing the pprev or rrev.
> yes, of course i am using full_transitive_package_id after all it wouldnt be fair if i hadnt? :)

Well, I wouldn't blame you, sometimes it takes some time to migrate things :) 

I don't think the mode is involved here, because the key should be always the ``PackageReference``, the full one. It is more like some of those keys of type PackageReference (which is a namedtuple) contains a None in one of their fields. If you could print instead:

```python
        print("PRINTING NOW STUFF! \n")
        for key, value in self._data.items():
            print("KEY IS", key.ref.name, key.ref.version, key.ref.user, key.ref.channel, key.ref.revision, 
                     key.id, key.revision, value.dumps() )
```
That should give us all the fields and we could identify the offending None (I still cannot figure out why there is None coming there)



here you go:
code is:
```
        for key, value in self._data.items():
            print("ref.name: ", key.ref.name, " ref.version ", key.ref.version, " ref.user ", key.ref.user, " ref.channel ", key.ref.channel, " ref.revision ", key.ref.revision, " id ", 
                     key.id, " revision ", key.revision, " dumps ", value.dumps(), "\n" )
```
```
PRINTING NOW STUFF! 

ref.name:  rfa-convert-lib  ref.version  2.5  ref.user  None  ref.channel  None  ref.revision  58f27ceb315f99e273c5ecadf75fcad3  id  3fea224fa96ea0aee3fd04b66788247769ce5a88  revision  bee3e3c900c64414ab91cb812fb9a8f7  dumps  rfa-convert-lib/2.Y.Z 

ref.name:  poco  ref.version  1.9.4  ref.user  None  ref.channel  None  ref.revision  42c1907520edbda9833bec9ee7b22c5f  id  e8c0afb6bd27a5c10d28c2b8222e300f4f94381d  revision  bd15d0f9475a3207ca1f4611a110ef56  dumps  poco/1.9.4:e8c0afb6bd27a5c10d28c2b8222e300f4f94381d 

ref.name:  rfa-server-lib  ref.version  4.13  ref.user  None  ref.channel  None  ref.revision  04fe1a0bb78e44568b6052b7323168a9  id  49323f7cb1e8c34a8222d4bb4b82d19fa2bafdaa  revision  ec125f5a77a6bd3a8e628d276691608d  dumps  rfa-server-lib/4.Y.Z 

ref.name:  boost  ref.version  1.72.0  ref.user  None  ref.channel  None  ref.revision  3c1a4170f35bcad9109cb8bc720d56d1  id  3f890e8db573d1fea921ff792e1d7a3e17718ab8  revision  33089a35430b60e799c76bb6b7d1a043  dumps  boost/1.72.0:3f890e8db573d1fea921ff792e1d7a3e17718ab8 

ref.name:  igcounters  ref.version  1.3  ref.user  None  ref.channel  None  ref.revision  eee56f3742689bd079b9b246073cb700  id  75e3e5f7724fc192123090cbe892c40afa841ea3  revision  adcafc3c3df160389b226cb3380d5c1c  dumps  igcounters/1.Y.Z 

ref.name:  fin-pricing-utils  ref.version  2.3  ref.user  None  ref.channel  None  ref.revision  f6f35138752d691fd1aac8b0114479ad  id  5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9  revision  262873877900a3059e1d60de48b37d1c  dumps  fin-pricing-utils/2.Y.Z 

ref.name:  openssl  ref.version  1.0.2t  ref.user  None  ref.channel  None  ref.revision  98b72be1284eb54893f45b1f043ecead  id  135f0560b485ffdf5b9f993d63f2aaecc8fb281e  revision  df8c72fcd905d9c2885adefdfc0cc2d9  dumps  openssl/1.0.2t:135f0560b485ffdf5b9f993d63f2aaecc8fb281e 

ref.name:  libcli  ref.version  1.9.7  ref.user  None  ref.channel  None  ref.revision  990f469c93b05f229ff6d711cca6cda5  id  4a09d987d91684aecef7218066b1c8edb559d34e  revision  72640db512697f14b8cc1f13423ca80b  dumps  libcli/1.Y.Z 

ref.name:  boost  ref.version  1.72.0  ref.user  None  ref.channel  None  ref.revision  3c1a4170f35bcad9109cb8bc720d56d1  id  3f890e8db573d1fea921ff792e1d7a3e17718ab8  revision  None  dumps  boost/1.72.0#3c1a4170f35bcad9109cb8bc720d56d1:3f890e8db573d1fea921ff792e1d7a3e17718ab8#PREV unknown 

ref.name:  bzip2  ref.version  1.0.8  ref.user  None  ref.channel  None  ref.revision  ad6efb7d25adcbde4984125a43434af2  id  76a4a7324a2083cb6964ea8321da67a1ceb31b50  revision  7be9a190f5446bbb47422a8558050bdd  dumps  bzip2/1.0.8:76a4a7324a2083cb6964ea8321da67a1ceb31b50 

ref.name:  rfa  ref.version  8.0.1.E1  ref.user  None  ref.channel  None  ref.revision  a7f8a61e064b5d8ee8d46fa5b5219a1d  id  2f48c782b34dfcb01aab706713eda393eed4a638  revision  45463dd5c2c58f243ae6a1f4f2c94986  dumps  rfa/8.0.1.E1:2f48c782b34dfcb01aab706713eda393eed4a638 

ref.name:  disruptor  ref.version  2.4  ref.user  None  ref.channel  None  ref.revision  c11f9a2b83faf9ea36d5a829237cb247  id  d4eab5eaba08a2639f6f809e50b7e7a7606d8829  revision  1fd0fade0d84bb48d3e62c384b9e3d34  dumps  disruptor/2.Y.Z 

ref.name:  zlib  ref.version  1.2.11  ref.user  None  ref.channel  None  ref.revision  ddccdddea098293f5202c5e8eb29967b  id  9fdb4217a0bb5bac441d7e17705c9172eeeb6cfe  revision  0bf6e52e8a2a2bfaeca54f6c72adcc51  dumps  zlib/1.2.11:9fdb4217a0bb5bac441d7e17705c9172eeeb6cfe 
```
Ok, I start to see where it comes from. Working on a fix.

Trying to reproduce with a test first. I guess you are using ``private`` dependencies somewhere in the graph, aren't you? 
Nope @memsharded  we dont use that kind of magic.

Commit message: moved inside propagate_info()

Patch to review:
diff --git a/conans/client/graph/graph.py b/conans/client/graph/graph.py
index f4f8ccaf391..9e2afdfb46c 100644
--- a/conans/client/graph/graph.py
+++ b/conans/client/graph/graph.py
@@ -86,6 +86,24 @@ def __init__(self, ref, conanfile, context, recipe=None, path=None):
         self._ancestors = _NodeOrderedDict()  # set{ref.name}
         self._id = None  # Unique ID (uuid at the moment) of a node in the graph
         self.graph_lock_node = None  # the locking information can be None
+        self.id_direct_prefs = None
+        self.id_indirect_prefs = None
+
+    def package_id_transitive_reqs(self):
+        """
+        accumulate the direct and transitive requirements prefs necessary to compute the
+        package_id
+        :return: set(prefs) of direct deps, set(prefs) of transitive deps
+        """
+        self.id_direct_prefs = set()  # of PackageReference
+        self.id_indirect_prefs = set()  # of PackageReference, avoid duplicates
+        for neighbor in self.neighbors():
+            self.id_direct_prefs.add(neighbor.pref)
+            self.id_indirect_prefs.update(neighbor.id_direct_prefs)
+            self.id_indirect_prefs.update(neighbor.id_indirect_prefs)
+        # Make sure not duplicated, totally necessary
+        self.id_indirect_prefs.difference_update(self.id_direct_prefs)
+        return self.id_direct_prefs, self.id_indirect_prefs
 
     @property
     def id(self):
diff --git a/conans/client/graph/graph_binaries.py b/conans/client/graph/graph_binaries.py
index 16de60e89af..ef33c8d918d 100644
--- a/conans/client/graph/graph_binaries.py
+++ b/conans/client/graph/graph_binaries.py
@@ -283,16 +283,7 @@ def _compute_package_id(self, node, default_package_id_mode, default_python_requ
             # Make sure not duplicated
             indirect_reqs.difference_update(direct_reqs)
         else:
-            node.id_direct_prefs = set()  # of PackageReference
-            node.id_indirect_prefs = set()  # of PackageReference, avoid duplicates
-            for neighbor in neighbors:
-                node.id_direct_prefs.add(neighbor.pref)
-                node.id_indirect_prefs.update(neighbor.id_direct_prefs)
-                node.id_indirect_prefs.update(neighbor.id_indirect_prefs)
-            # Make sure not duplicated, totally necessary
-            node.id_indirect_prefs.difference_update(node.id_direct_prefs)
-            direct_reqs = node.id_direct_prefs
-            indirect_reqs = node.id_indirect_prefs
+            direct_reqs, indirect_reqs = node.package_id_transitive_reqs()
 
         python_requires = getattr(conanfile, "python_requires", None)
         if python_requires:
diff --git a/conans/client/installer.py b/conans/client/installer.py
index 4772b5a79ed..a1ae3f6386a 100644
--- a/conans/client/installer.py
+++ b/conans/client/installer.py
@@ -395,13 +395,14 @@ def _build(self, nodes_by_level, keep_build, root_node, graph_info, remotes, bui
         self._raise_missing(missing)
         processed_package_refs = set()
         self._download(downloads, processed_package_refs)
+        fix_package_id = self._cache.config.full_transitive_package_id
 
         for level in nodes_by_level:
             for node in level:
                 ref, conan_file = node.ref, node.conanfile
                 output = conan_file.output
 
-                self._propagate_info(node, using_build_profile)
+                self._propagate_info(node, using_build_profile, fix_package_id)
                 if node.binary == BINARY_EDITABLE:
                     self._handle_node_editable(node, graph_info)
                     # Need a temporary package revision for package_revision_mode
@@ -417,7 +418,7 @@ def _build(self, nodes_by_level, keep_build, root_node, graph_info, remotes, bui
                     self._handle_node_cache(node, keep_build, processed_package_refs, remotes)
 
         # Finally, propagate information to root node (ref=None)
-        self._propagate_info(root_node, using_build_profile)
+        self._propagate_info(root_node, using_build_profile, fix_package_id)
 
     def _handle_node_editable(self, node, graph_info):
         # Get source of information
@@ -507,7 +508,13 @@ def _build_package(self, node, output, keep_build, remotes):
         return pref
 
     @staticmethod
-    def _propagate_info(node, using_build_profile):
+    def _propagate_info(node, using_build_profile, fixed_package_id):
+        if fixed_package_id:
+            # if using config.full_transitive_package_id, it is necessary to recompute
+            # the node transitive information necessary to compute the package_id
+            # as it will be used by reevaluate_node() when package_revision_mode is used and
+            # PACKAGE_ID_UNKNOWN happens due to unknown revisions
+            node.package_id_transitive_reqs()
         # Get deps_cpp_info from upstream nodes
         node_order = [n for n in node.public_closure if n.binary != BINARY_SKIP]
         # List sort is stable, will keep the original order of the closure, but prioritize levels
diff --git a/conans/test/functional/package_id/package_id_requires_modes_test.py b/conans/test/functional/package_id/package_id_requires_modes_test.py
index b4fd4db2b00..e722086e75c 100644
--- a/conans/test/functional/package_id/package_id_requires_modes_test.py
+++ b/conans/test/functional/package_id/package_id_requires_modes_test.py
@@ -513,6 +513,25 @@ def transitive_multi_mode_package_id_test(self):
         client.run('create . consumer/1.0@user/testing --build')
         self.assertIn("consumer/1.0@user/testing: Created", client.out)
 
+    def transitive_multi_mode2_package_id_test(self):
+        # https://github.com/conan-io/conan/issues/6942
+        client = TestClient()
+        client.run("config set general.default_package_id_mode=package_revision_mode")
+        # This is mandatory, otherwise it doesn't work
+        client.run("config set general.full_transitive_package_id=True")
+
+        client.save({"conanfile.py": GenConanfile()})
+        client.run("export . dep1/1.0@user/testing")
+
+        pkg_revision_mode = "self.info.requires.full_version_mode()"
+        client.save({"conanfile.py": GenConanfile().with_require_plain("dep1/1.0@user/testing")
+                    .with_package_id(pkg_revision_mode)})
+        client.run("export . dep2/1.0@user/testing")
+
+        client.save({"conanfile.py": GenConanfile().with_require_plain("dep2/1.0@user/testing")})
+        client.run('create . consumer/1.0@user/testing --build')
+        self.assertIn("consumer/1.0@user/testing: Created", client.out)
+
     def package_revision_mode_editable_test(self):
         # Package revision mode crash when using editables
         client = TestClient()


Code review comments:
diff_hunk: @@ -507,7 +508,13 @@ def _build_package(self, node, output, keep_build, remotes):
         return pref
 
     @staticmethod
-    def _propagate_info(node, using_build_profile):
+    def _propagate_info(node, using_build_profile, fixed_package_id):
+        if fixed_package_id:
+            # if using config.full_transitive_package_id, it is necessary to recompute
+            # the node transitive information necessary to compute the package_id
+            # as it will be used by reevaluate_node() when package_revision_mode is used and
+            # PACKAGE_ID_UNKNOWN happens due to unknown revisions
+            node.package_id_transitive_reqs()
path: conans/client/installer.py
text: @user1:
Is there a reason to do this only when `full_transitive_package_id` is activated?

@user1:
`reevaluate_node` will run `_compute_package_id` that calls this same function. Why is it needed to update this information before doing those calls?

@author:
Because ``reevaluate_node`` only runs for those nodes that has PACKAGE_ID_UNKNOWN, but the information that needs to be re-computed after being built are the one of the dependencies of the node with PACKAGE_ID_UNKNOWN.

Only ``full_transitive_package_id`` is causing the issue in this case, because if not defined, a different computation is done, based on ``conanfile.requires`` (which is updated).

I think the root cause of this is the immutability of the reference objects, that produces that there are several copies of ConanFileReference and PackageReference. I think for the future we should aim for a central definition of these for each node, in a way that an update to them automatically is seen by everyone else, because they contain a reference to this object.

@user1:
> Because `reevaluate_node` only runs for those nodes that has PACKAGE_ID_UNKNOWN, but the information that needs to be re-computed after being built are the one of the dependencies of the node with PACKAGE_ID_UNKNOWN.

Here you mean, the CONSUMERS of the node with `PACKAGE_ID_UNKNOWN`, right?

----

> Only full_transitive_package_id is causing the issue in this case, because if not defined, a different computation is done, based on conanfile.requires (which is updated).

Having a quick look to `conanfile.requires`, it is built using the _direct_ and the _indirect_ prefs too, so maybe we can simplify all of this a lot moving logic into ConanInfo (not this PR). 

@author:
PkgA->PkgB->PkgC

PkgA has PACKAGE_ID_UNKNOWN, because we still need to build PkgC or PkgB and awe are in ``package_revision_mode``

``reevaluate_node`` will only be executed for ``PkgA``. But the computation of transitive updated ``PackageReferences`` need to be computed for both ``PkgB`` and ``PkgC``.

Does this clarify the issue a bit?

@user1:
Ok, we are running them to populate `Node::id_direct_prefs` and `Node::id_indirect_prefs` with the new values (are we doing this twice for `pkgC`?)

@author:
Yes, it is being called twice. That is the key of the issue, we need to run this 2 times:
- One to compute the first package_id, while analyzing the graph binaries
- A second one, while installing the binaries, to update for possible changes upstream that will be used in reevaluate_node to compute the package_id of consumers with PACKAGE_ID_UNKNOWN.

It seems it would be very challenging to skip running this for leaf nodes that wouldn't need to run this.

diff_hunk: @@ -283,16 +283,7 @@ def _compute_package_id(self, node, default_package_id_mode, default_python_requ
             # Make sure not duplicated
             indirect_reqs.difference_update(direct_reqs)
         else:
-            node.id_direct_prefs = set()  # of PackageReference
-            node.id_indirect_prefs = set()  # of PackageReference, avoid duplicates
-            for neighbor in neighbors:
-                node.id_direct_prefs.add(neighbor.pref)
-                node.id_indirect_prefs.update(neighbor.id_direct_prefs)
-                node.id_indirect_prefs.update(neighbor.id_indirect_prefs)
-            # Make sure not duplicated, totally necessary
-            node.id_indirect_prefs.difference_update(node.id_direct_prefs)
-            direct_reqs = node.id_direct_prefs
-            indirect_reqs = node.id_indirect_prefs
+            direct_reqs, indirect_reqs = node.package_id_transitive_reqs()
path: conans/client/graph/graph_binaries.py
text: @user1:
Probably if we are moving only one branch this function doesn't belong to `node`, but to `self`, or we should move both branches.

@author:
I put it in the ``node`` because now it needs to be called from 2 different locations: ``graph_binaries.py`` and ``installer.py``. I could leave it here in the ``graph_binaries.py`` as a public static method that receives a single ``node`` as argument, no problem.