Skip to content

Commit

Permalink
Fixing private dependencies in CMakeDeps and MSBuildDeps (#9517)
Browse files Browse the repository at this point in the history
* new test to reproduce

* fixing private

* fixing tests
  • Loading branch information
memsharded committed Sep 2, 2021
1 parent e869976 commit 2c94b9c
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 14 deletions.
8 changes: 5 additions & 3 deletions conan/tools/cmake/cmakedeps/templates/target_configuration.py
Expand Up @@ -160,19 +160,21 @@ def get_deps_targets_names(self):

# Get a list of dependencies target names
# Declared cppinfo.requires or .components[].requires
visible_host = self.conanfile.dependencies.filter({"build": False, "visible": True})
visible_host_direct = visible_host.filter({"direct": True})
if self.conanfile.new_cpp_info.required_components:
for dep_name, component_name in self.conanfile.new_cpp_info.required_components:
if not dep_name:
# Internal dep (no another component)
req = self.conanfile
else:
req = self.conanfile.dependencies.host[dep_name]
req = visible_host[dep_name]

dep_name = self.get_target_namespace(req)
component_name = self.get_component_alias(req, component_name)
ret.append("{}::{}".format(dep_name, component_name))
elif self.conanfile.dependencies.direct_host:
elif visible_host_direct:
# Regular external "conanfile.requires" declared, not cpp_info requires
ret = ["{p}::{n}".format(p=self.get_target_namespace(r), n=self.get_global_target_name(r))
for r in self.conanfile.dependencies.direct_host.values()]
for r in visible_host_direct.values()]
return ret
7 changes: 5 additions & 2 deletions conan/tools/cmake/cmakedeps/templates/target_data.py
Expand Up @@ -114,14 +114,16 @@ def get_required_components_cpp(self):
ret = []
sorted_comps = self.conanfile.new_cpp_info.get_sorted_components()

direct_visible_host = self.conanfile.dependencies.filter({"build": False, "visible": True,
"direct": True})
for comp_name, comp in sorted_comps.items():
pfolder_var_name = "{}_PACKAGE_FOLDER{}".format(self.pkg_name, self.config_suffix)
deps_cpp_cmake = DepsCppCmake(comp, pfolder_var_name)
public_comp_deps = []
for require in comp.requires:
if "::" in require: # Points to a component of a different package
pkg, cmp_name = require.split("::")
req = self.conanfile.dependencies.direct_host[pkg]
req = direct_visible_host[pkg]
public_comp_deps.append("{}::{}".format(self.get_target_namespace(req),
self.get_component_alias(req, cmp_name)))
else: # Points to a component of same package
Expand All @@ -138,7 +140,8 @@ def _get_dependency_filenames(self):
if self.conanfile.is_build_context:
return []
ret = []
direct_host = self.conanfile.dependencies.direct_host
direct_host = self.conanfile.dependencies.filter({"build": False, "visible": True,
"direct": True})
if self.conanfile.new_cpp_info.required_components:
for dep_name, _ in self.conanfile.new_cpp_info.required_components:
if dep_name and dep_name not in ret: # External dep
Expand Down
2 changes: 1 addition & 1 deletion conan/tools/microsoft/msbuilddeps.py
Expand Up @@ -272,7 +272,7 @@ def _content(self):
dep_name = dep_name.replace(".", "_")
cpp_info = DepCppInfo(dep.cpp_info) # To account for automatic component aggregation
public_deps = [d.ref.name.replace(".", "_")
for d in dep.dependencies.direct_host.values()]
for r, d in dep.dependencies.direct_host.items() if r.visible]
# One file per configuration, with just the variables
vars_props_name = "conan_%s_vars%s.props" % (dep_name, conf_name)
result[vars_props_name] = self._vars_props_file(dep_name, cpp_info, public_deps)
Expand Down
39 changes: 31 additions & 8 deletions conans/model/dependencies.py
Expand Up @@ -6,12 +6,13 @@

class Requirement(object):

def __init__(self, ref, build=False, direct=True, test=False):
def __init__(self, ref, build=False, direct=True, test=False, visible=True):
# By default this is a generic library requirement
self.ref = ref
self.build = build # This dependent node is a build tool that is executed at build time only
self.direct = direct
self.test = test
self.visible = visible

def __repr__(self):
return repr(self.__dict__)
Expand All @@ -25,6 +26,13 @@ def __eq__(self, other):
def __ne__(self, other):
return not self.__eq__(other)

def aggregate(self, other):
""" when closing loop and finding the same dependency on a node, the information needs
to be aggregated
"""
assert self.build == other.build
self.visible |= other.visible


class UserRequirementsDict(object):
""" user facing dict to allow access of dependencies by name
Expand Down Expand Up @@ -83,23 +91,34 @@ class ConanFileDependencies(UserRequirementsDict):
@staticmethod
def from_node(node):
# TODO: This construction will be easier in 2.0
build, test, host = [], [], []
build, test, host, private = [], [], [], []
for edge in node.dependencies:
if edge.build_require:
if not edge.require.force_host_context:
build.append(edge.dst)
else:
test.append(edge.dst)
elif edge.private:
private.append(edge.dst)
else:
host.append(edge.dst)

d = OrderedDict()

def expand(nodes, is_build, is_test):
def update_existing(req, conanfile):
existing = d.get(req)
if existing is not None:
_, existing_req = existing
existing_req.aggregate(req)
req = existing_req
d[req] = conanfile, req

def expand(nodes, is_build, is_test, is_visible):
all_nodes = set(nodes)
for n in nodes:
conanfile = ConanFileInterface(n.conanfile)
d[Requirement(n.ref, build=is_build, test=is_test)] = conanfile
req = Requirement(n.ref, build=is_build, test=is_test, visible=is_visible)
update_existing(req, conanfile)

next_nodes = nodes
while next_nodes:
Expand All @@ -112,12 +131,16 @@ def expand(nodes, is_build, is_test):
next_nodes = new_nodes
for n in next_nodes:
conanfile = ConanFileInterface(n.conanfile)
d[Requirement(n.ref, build=is_build, test=is_test, direct=False)] = conanfile
req = Requirement(n.ref, build=is_build, test=is_test, direct=False,
visible=is_visible)
update_existing(req, conanfile)

expand(host, is_build=False, is_test=False)
expand(build, is_build=True, is_test=False)
expand(test, is_build=False, is_test=True)
expand(host, is_build=False, is_test=False, is_visible=True)
expand(private, is_build=False, is_test=False, is_visible=False)
expand(build, is_build=True, is_test=False, is_visible=False)
expand(test, is_build=False, is_test=True, is_visible=False)

d = OrderedDict([(k, v[0])for k, v in d.items()])
return ConanFileDependencies(d)

def filter(self, require_filter):
Expand Down
Expand Up @@ -276,3 +276,18 @@ def package_info(self):
c.save({"conanfile.py": consumer_conanfile, "CMakeLists.txt": cmake})
c.run("create .")
assert "MYVAR=>Like a Rolling Stone" in c.out


def test_private_transitive():
# https://github.com/conan-io/conan/issues/9514
client = TestClient()
client.save({"dep/conanfile.py": GenConanfile(),
"pkg/conanfile.py": GenConanfile().with_require("dep/0.1", private=True),
"consumer/conanfile.py": GenConanfile().with_requires("pkg/0.1")
.with_settings("os", "build_type", "arch")})
client.run("create dep dep/0.1@")
client.run("create pkg pkg/0.1@")
client.run("install consumer -g CMakeDeps -s arch=x86_64 -s build_type=Release")
assert "dep/0.1:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Skip" in client.out
data_cmake = client.load("pkg-release-x86_64-data.cmake")
assert "set(pkg_FIND_DEPENDENCY_NAMES ${pkg_FIND_DEPENDENCY_NAMES} )" in data_cmake
25 changes: 25 additions & 0 deletions conans/test/functional/toolchains/microsoft/test_msbuilddeps.py
Expand Up @@ -403,6 +403,7 @@
if "17" in tools_locations['visual_studio'] and not tools_locations['visual_studio']['17'].get('disabled', False):
vs_versions.append({"vs_version": "17", "msvc_version": "19.3", "ide_year": "2022", "toolset": "v143"})


@parameterized_class(vs_versions)
@pytest.mark.tool_visual_studio
@pytest.mark.skipif(platform.system() != "Windows", reason="Requires MSBuild")
Expand Down Expand Up @@ -704,12 +705,14 @@ def build(self):
def test_build_vs_project_with_a_vs2017():
check_build_vs_project_with_a("15")


@pytest.mark.tool_visual_studio(version="17")
@pytest.mark.tool_cmake
@pytest.mark.skipif(platform.system() != "Windows", reason="Requires MSBuild")
def test_build_vs_project_with_a_vs2022():
check_build_vs_project_with_a("17")


def check_build_vs_project_with_a(vs_version):
client = TestClient()
client.save({"conanfile.py": GenConanfile()})
Expand Down Expand Up @@ -788,12 +791,14 @@ def build(self):
def test_build_vs_project_with_test_requires_vs2017():
check_build_vs_project_with_test_requires("15")


@pytest.mark.tool_visual_studio(version="17")
@pytest.mark.tool_cmake
@pytest.mark.skipif(platform.system() != "Windows", reason="Requires MSBuild")
def test_build_vs_project_with_test_requires_vs2022():
check_build_vs_project_with_test_requires("17")


def check_build_vs_project_with_test_requires(vs_version):
client = TestClient()
client.save(pkg_cmake("updep.pkg.team", "0.1"))
Expand Down Expand Up @@ -832,3 +837,23 @@ def build(self):
client.run_command(r"x64\Release\MyProject.exe")
assert "mydep_pkg_team: Release!" in client.out
assert "updep_pkg_team: Release!" in client.out


@pytest.mark.skipif(platform.system() != "Windows", reason="Requires MSBuild")
def test_private_transitive():
# https://github.com/conan-io/conan/issues/9514
client = TestClient()
client.save({"dep/conanfile.py": GenConanfile(),
"pkg/conanfile.py": GenConanfile().with_require("dep/0.1", private=True),
"consumer/conanfile.py": GenConanfile().with_requires("pkg/0.1")
.with_settings("os", "build_type", "arch")})
client.run("create dep dep/0.1@")
client.run("create pkg pkg/0.1@")
client.run("install consumer -g MSBuildDeps -s arch=x86_64 -s build_type=Release")
assert "dep/0.1:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Skip" in client.out
deps_props = client.load("conandeps.props")
assert "conan_pkg.props" in deps_props
assert "dep" not in deps_props

pkg_data_props = client.load("conan_pkg_release_x64.props")
assert "conan_dep.props" not in pkg_data_props
Empty file added origin)
Empty file.

2 comments on commit 2c94b9c

@aberegovoy
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt it worth a dedicated issue, but the "oriigin)" seeems odd. But smiley, though. )

@memsharded
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt it worth a dedicated issue, but the "oriigin)" seeems odd. But smiley, though. )

Thanks for pointing it out! Yes, I had this pending, I'll remove it in one of my already approved PRs.

Please sign in to comment.