Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dependencies to package output info when binary is not found (close #3316) #3438

Merged
merged 6 commits into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions conans/client/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,17 @@ def call_system_requirements(conanfile, output):
raise ConanException("Error in system requirements")


def raise_package_not_found_error(conan_file, conan_ref, package_id, out, recorder):
def raise_package_not_found_error(conan_file, conan_ref, package_id, dependencies, out, recorder):
settings_text = ", ".join(conan_file.info.full_settings.dumps().splitlines())
options_text = ", ".join(conan_file.info.full_options.dumps().splitlines())
dependencies_text = ', '.join(dependencies)

msg = '''Can't find a '%s' package for the specified options and settings:
msg = '''Can't find a '%s' package for the specified settings, options and dependencies:
- Settings: %s
- Options: %s
- Dependencies: %s
- Package ID: %s
''' % (conan_ref, settings_text, options_text, package_id)
''' % (conan_ref, settings_text, options_text, dependencies_text, package_id)
out.warn(msg)
recorder.package_install_error(PackageReference(conan_ref, package_id),
INSTALL_ERROR_MISSING, msg)
Expand Down Expand Up @@ -268,7 +270,9 @@ def _build(self, nodes_by_level, deps_graph, keep_build, root_node):
output = ScopedOutput(str(conan_ref), self._out)
package_id = conan_file.info.package_id()
if node.binary == BINARY_MISSING:
raise_package_not_found_error(conan_file, conan_ref, package_id, output, self._recorder)
dependencies = [str(dep.dst) for dep in node.dependencies]
raise_package_not_found_error(conan_file, conan_ref, package_id, dependencies,
out=output, recorder=self._recorder)

self._propagate_info(node, inverse_levels, deps_graph, output)
if node.binary == BINARY_SKIP: # Privates not necessary
Expand Down
56 changes: 56 additions & 0 deletions conans/test/integration/install_missing_dep_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import unittest
from conans.test.utils.tools import TestClient, TestServer
from conans.model.ref import ConanFileReference, PackageReference
import os
from conans.test.utils.cpp_test_files import cpp_hello_conan_files
from conans.util.files import load, save
from time import sleep
Copy link
Member

Choose a reason for hiding this comment

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

Unused imports. Try to enable a linter (like pep8, pycodestyle) in your IDE, it typically warns about unused imports (and unused variables, etc).

Also, try to follow the import order: first python ones, then dependencies, then conan imports.

import time
from conans.paths import CONAN_MANIFEST


class InstallMissingDependency(unittest.TestCase):

def missing_dep_test(self):
test_server = TestServer()
self.servers = {"myremote": test_server}
Copy link
Member

Choose a reason for hiding this comment

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

Finally, there is no upload, so you can remove the server.

self.client = TestClient(servers=self.servers, users={"myremote": [("lasote", "mypass")]})
Copy link
Member

Choose a reason for hiding this comment

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

Do not use self.client if client is possible


# Create deps packages
dep1_conanfile = """from conans import ConanFile
class Dep1Pkg(ConanFile):
name = "dep1"
Copy link
Member

Choose a reason for hiding this comment

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

Name also unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, but I prefer to keep the name in the recipe... at least to know but that conanfile.py is about. Is there any reason (internal to the machinery of Conan) to leave it out?

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, you can leave the name.

"""

dep2_conanfile = """from conans import ConanFile
class Dep2Pkg(ConanFile):
name = "dep2"
version = "1.0"
requires = "dep1/1.0@lasote/testing"
"""

self.client.save({"conanfile.py": dep1_conanfile}, clean_first=True)
self.client.run("create . dep1/1.0@lasote/testing")
self.client.run("create . dep1/2.0@lasote/testing")

self.client.save({"conanfile.py": dep2_conanfile}, clean_first=True)
self.client.run("create . lasote/testing")

foo_conanfile = """from conans import ConanFile
class FooPkg(ConanFile):
name = "foo"
version = "1.0"
requires = "dep1/{dep1_version}@lasote/testing", "dep2/1.0@lasote/testing"
Copy link
Member

Choose a reason for hiding this comment

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

why 2 dependencies? The test can be perfectly done with 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue we are testing is that dep2/1.0 (which depends on dep1/1.0) is not available when we bump dep1/1.0 to dep1/2.0 inside foo package.

Copy link
Member

Choose a reason for hiding this comment

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

It could be tested such output, even if the binary is missing for another reason, so not really necessary to have such complex dependency graph. But I agree, it is nicer to test the full case, just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you are right, we can remove dep1/1.0 from the repo and it should provide the same kind of information in the output message. It wouldn't fail for the same reason (override of transitive dependencies) but that behavior should have been tested at some other place.

I can leave here the minimum test and maybe create a new test module to store those tests that are 1-to-1 related to an specific issue.

Copy link
Member

Choose a reason for hiding this comment

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

No need to do 2 tests, I think this one is ok here, just with some improvements.

"""
self.client.save({"conanfile.py": foo_conanfile.format(dep1_version="1.0")},
clean_first=True)
error = self.client.run("create . lasote/testing")
Copy link
Member

Choose a reason for hiding this comment

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

remove this error

self.assertFalse(error)

self.client.save({"conanfile.py": foo_conanfile.format(dep1_version="2.0")},
clean_first=True)
error = self.client.run("create . lasote/testing", ignore_error=True)
self.assertTrue(error)
self.assertIn("Can't find a 'dep2/1.0@lasote/testing' package", self.client.user_io.out)
self.assertIn("- Dependencies: dep1/2.0@lasote/testing", self.client.user_io.out)