Repository: conan-io/conan
Pull request: [fix] Respect order of declared directories when using components
Changelog: BugFix: Respect order of declared directories when using components.
Docs: omit

- [x] Refer to the issue that supports this Pull Request: close #8904
- [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](https://github.com/conan-io/conan/blob/develop/.github/CONTRIBUTING.md).
- [x] I've followed the PEP8 style guides for Python code.
- [x] I've opened another PR in the Conan docs repo to the ``develop`` branch, documenting this one. 

<sup>**Note:** By default this PR will skip the slower tests and will use a limited set of python versions. Check [here](https://github.com/conan-io/conan/blob/develop/.github/PR_INCREASE_TESTING.md) how to increase the testing level by writing some tags in the current PR body text.</sup>


Problem statement:
[question] How to specify 'includedirs'?
This refers to Windows and Visual Studio. 
I try to change default includedirs of a components on Windows, in package_info() method

I say
self.cpp_info.components["BASE"].includedirs = ['include/Release', 'include']
and get in CONAN_INCLUDE_DIRS: ['include', 'include/Release'] - reverse order!
in conanbuildinfo.cmake, I mean

I say
self.cpp_info.components["BASE"].includedirs = ['include', 'include/Release']
and get in CONAN_INCLUDE_DIRS: ['include', 'include/Release'] - correct, but not what I want

What I really want is
self.cpp_info.components["BASE"].includedirs = ['include/$(Configuration)', 'include']
but in this case I get CONAN_INCLUDE_DIRS: ['include'] - which is totally wrong

Hints from issue discussion:
Hi @gouriano,

What generator are you using to check the result of CONAN_INCLUDE_DIRS?
It is CMake
I am looking at the contents of conanbuildinfo.cmake,

So not sure what might be happening here but I guess is something related to repeated directories across the dependency graph. CONAN_INCLUDE_DIRS variable aggregates the include directories of all the dependencies in the graph you are installing. As "include" is the value by default, it might be first in the list because it is coming from an upstream dependency.

Another option could be that the generator is picking the value from ``cpp_info.includedirs`` and ``cpp_info.components["BASE"].includedirs``, but I think doing this is not allowed:

```
self.cpp_info.includedirs = []
self.cpp_info.components["BASE"].includedirs = ['include/Release', 'include']
```

Could you give it a go and let me know? thank you
What you suggest cannot work, because:
ConanException: CxxTest/0.1.6 package_info(): self.cpp_info.components cannot be used with self.cpp_info global values at the same time
(Yes, I have verified it)

I have tried few more variants:
When I specify, for example (note that all these directories exist in my package root)
self.cpp_info.components["BASE"].includedirs = ['cmake', 'include/common', 'include']
I get in CONAN_INCLUDE_DIRS = ['include', 'cmake', 'include/common']
'include' always goes first.

When I specify a directory which does not exits in package root:
self.cpp_info.components["BASE"].includedirs = ['notfound', 'include']
it is simply dropped. 
That is not good, I believe. Nonexistent directory is my problem (I write recipe), not yours.
I mean I still want to be able to specify
self.cpp_info.components["BASE"].includedirs = ['include/$(Configuration)', 'include']

Thanks


I have figured out how you can reproduce it.
Create new Conan package:
`conan new hello/0.1`
Change package_info into the following

    def package_info(self):
        self.cpp_info.libs = ["hello"]
        self.cpp_info.includedirs = ['lib', 'include']

run "conan create ."
Then, in a separate directory, create conanfile.txt:

    [requires]
    hello/0.1
    [generators]
    cmake

run "conan install ."
In the generated conanbuildinfo.cmake, CONAN_INCLUDE_DIRS in the correct order: ['lib', 'include']

Now change package_info

    def package_info(self):
        self.cpp_info.components["BASE"].defines.append("_UNICODE")
        self.cpp_info.components["BASE"].includedirs = ['lib', 'include']
        self.cpp_info.components["hellolib"].libs = ["hello"]
        self.cpp_info.components["hellolib"].requires = ["BASE"]

again, "conan create .", then "conan install .", then look into conanbuildinfo.cmake.
CONAN_INCLUDE_DIRS are now in wrong order: [include', 'lib']

So, is it a bug? or I do something wrong?
I use Conan v1.35.2


thanks for the quick response and the clear steps to reproduce the error.
I tried it and I think this is a bug in the way the directories are aggregated when components are used. I have created a tentative PR with the fix at #8927 to see if it does not brake other use cases that might be relying on this behavior.

Thanks a lot for reporting again!
Maybe, once we are at it, it is possible to allow "include/$(Configuration)" ?
that would be useful

Commit message: separate functions for aggregating lists and dicts

Patch to review:
diff --git a/conans/model/build_info.py b/conans/model/build_info.py
index 514f44c8b62..39f8cbaff68 100644
--- a/conans/model/build_info.py
+++ b/conans/model/build_info.py
@@ -556,14 +556,29 @@ def __getattr__(self, item):
             attr = self._cpp_info.__getattr__(item)
         return attr
 
-    def _aggregated_values(self, item, agg_func=merge_lists):
+    def _aggregated_dict_values(self, item):
         values = getattr(self, "_%s" % item)
         if values is not None:
             return values
-        values = getattr(self._cpp_info, item)
         if self._cpp_info.components:
+            values = {}
             for component in self._get_sorted_components().values():
-                values = agg_func(values, getattr(component, item))
+                values = merge_dicts(values, getattr(component, item))
+        else:
+            values = getattr(self._cpp_info, item)
+        setattr(self, "_%s" % item, values)
+        return values
+
+    def _aggregated_list_values(self, item):
+        values = getattr(self, "_%s" % item)
+        if values is not None:
+            return values
+        if self._cpp_info.components:
+            values = []
+            for component in self._get_sorted_components().values():
+                values = merge_lists(values, getattr(component, item))
+        else:
+            values = getattr(self._cpp_info, item)
         setattr(self, "_%s" % item, values)
         return values
 
@@ -614,71 +629,71 @@ def _get_sorted_components(self):
 
     @property
     def build_modules_paths(self):
-        return self._aggregated_values("build_modules_paths", agg_func=merge_dicts)
+        return self._aggregated_dict_values("build_modules_paths")
 
     @property
     def include_paths(self):
-        return self._aggregated_values("include_paths")
+        return self._aggregated_list_values("include_paths")
 
     @property
     def lib_paths(self):
-        return self._aggregated_values("lib_paths")
+        return self._aggregated_list_values("lib_paths")
 
     @property
     def src_paths(self):
-        return self._aggregated_values("src_paths")
+        return self._aggregated_list_values("src_paths")
 
     @property
     def bin_paths(self):
-        return self._aggregated_values("bin_paths")
+        return self._aggregated_list_values("bin_paths")
 
     @property
     def build_paths(self):
-        return self._aggregated_values("build_paths")
+        return self._aggregated_list_values("build_paths")
 
     @property
     def res_paths(self):
-        return self._aggregated_values("res_paths")
+        return self._aggregated_list_values("res_paths")
 
     @property
     def framework_paths(self):
-        return self._aggregated_values("framework_paths")
+        return self._aggregated_list_values("framework_paths")
 
     @property
     def libs(self):
-        return self._aggregated_values("libs")
+        return self._aggregated_list_values("libs")
 
     @property
     def system_libs(self):
-        return self._aggregated_values("system_libs")
+        return self._aggregated_list_values("system_libs")
 
     @property
     def frameworks(self):
-        return self._aggregated_values("frameworks")
+        return self._aggregated_list_values("frameworks")
 
     @property
     def defines(self):
-        return self._aggregated_values("defines")
+        return self._aggregated_list_values("defines")
 
     @property
     def cxxflags(self):
-        return self._aggregated_values("cxxflags")
+        return self._aggregated_list_values("cxxflags")
 
     @property
     def cflags(self):
-        return self._aggregated_values("cflags")
+        return self._aggregated_list_values("cflags")
 
     @property
     def sharedlinkflags(self):
-        return self._aggregated_values("sharedlinkflags")
+        return self._aggregated_list_values("sharedlinkflags")
 
     @property
     def exelinkflags(self):
-        return self._aggregated_values("exelinkflags")
+        return self._aggregated_list_values("exelinkflags")
 
     @property
     def requires(self):
-        return self._aggregated_values("requires")
+        return self._aggregated_list_values("requires")
 
 
 class DepsCppInfo(_BaseDepsCppInfo):
diff --git a/conans/test/unittests/model/build_info/components_test.py b/conans/test/unittests/model/build_info/components_test.py
index 01134759322..acb0e5d6c0b 100644
--- a/conans/test/unittests/model/build_info/components_test.py
+++ b/conans/test/unittests/model/build_info/components_test.py
@@ -414,3 +414,18 @@ def test_deps_cpp_info_components(self):
         self.assertListEqual([os.path.join(folder, "include")], list(deps_cpp_info.include_paths))
         self.assertListEqual([os.path.join(folder, "include")],
                              list(deps_cpp_info["my_lib"].components["Component"].include_paths))
+
+    def test_deps_cpp_info_components_includedirs(self):
+        folder = temp_folder()
+        info = CppInfo("my_lib", folder)
+        # Create files so paths are not cleared
+        save(os.path.join(folder, "lib", "kk.lib"), "")
+        save(os.path.join(folder, "include", "kk.h"), "")
+        info.components["component"].includedirs = ["lib", "include"]
+        dep_info = DepCppInfo(info)
+        expected = [os.path.join(folder, "lib"), os.path.join(folder, "include")]
+        self.assertListEqual(expected, list(dep_info.include_paths))
+        deps_cpp_info = DepsCppInfo()
+        deps_cpp_info.add("my_lib", dep_info)
+        expected = [os.path.join(folder, "lib"), os.path.join(folder, "include")]
+        self.assertListEqual(expected, list(deps_cpp_info.includedirs))


Code review comments:
diff_hunk: @@ -414,3 +414,18 @@ def test_deps_cpp_info_components(self):
         self.assertListEqual([os.path.join(folder, "include")], list(deps_cpp_info.include_paths))
         self.assertListEqual([os.path.join(folder, "include")],
                              list(deps_cpp_info["my_lib"].components["Component"].include_paths))
+
+    def test_deps_cpp_info_components_includedirs(self):
+        folder = temp_folder()
+        info = CppInfo("my_lib", folder)
+        # Create files so paths are not cleared
+        save(os.path.join(folder, "lib", "kk.lib"), "")
path: conans/test/unittests/model/build_info/components_test.py
text: @user1:
You can use `info.filter_empty=False`