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

jgsogo · 2018-08-30T08:36:58Z

close Better information on mismatch in dependency tree #3316
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.
Add test

Current (1.7) ouput is:

Exporting package recipe
foo/1.0.0@issue/test: A new conanfile.py version was exported
foo/1.0.0@issue/test: Folder: /Users/jgsogo/.conan/data/foo/1.0.0/issue/test/export
carrot/1.0.0@issue/test requirement potato/1.0.0@issue/test overridden by foo/1.0.0@issue/test to potato/2.0.0@issue/test 
foo/1.0.0@issue/test: WARN: Forced build from source
foo/1.0.0@issue/test: Installing package
Requirements
    carrot/1.0.0@issue/test from local cache - Cache
    foo/1.0.0@issue/test from local cache - Cache
    potato/2.0.0@issue/test from local cache - Cache
Packages
    carrot/1.0.0@issue/test:88d205e84502b10878a230ecd0d4aeb5f0c15d5e - Missing
    foo/1.0.0@issue/test:bb034b7230b58e9c8c59372b69d68b06c5ffff5e - Build
    potato/2.0.0@issue/test:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 - Cache

potato/2.0.0@issue/test: Already installed!
carrot/1.0.0@issue/test: WARN: Can't find a 'carrot/1.0.0@issue/test' package for the specified options and settings:
- Settings: 
- Options: 
- Package ID: 88d205e84502b10878a230ecd0d4aeb5f0c15d5e

ERROR: Missing prebuilt package for 'carrot/1.0.0@issue/test'
Try to build it from sources with "--build carrot"
Or read "http://docs.conan.io/en/latest/faq/troubleshooting.html#error-missing-prebuilt-package"

we can easily improve the output listing the dependencies:

carrot/1.0.0@issue/test: WARN: Can't find a 'carrot/1.0.0@issue/test' package for the specified settings, options and dependencies:
- Settings: 
- Options: 
- Dependencies: potato/2.0.0@issue/test
- Package ID: 88d205e84502b10878a230ecd0d4aeb5f0c15d5e

Changelog: Feature: Improve the output of a conan install command printing dependencies when a binary is not found.

memsharded · 2018-08-30T09:34:04Z

Looks good, thanks. Please add a test for it.

memsharded · 2018-08-30T14:55:17Z

conans/test/integration/install_missing_dep_test.py

+    requires = "dep1/1.0@lasote/testing"
+        """
+
+        self.client.save({"conanfile.py": dep1_conanfile.format(version="1.0")}, clean_first=True)


Instead of parameterizing the template, name and version, remove them and do conan create . PkgName/version@user/channel

Didn't realize that this was possible, lesson learned!

memsharded · 2018-08-30T14:55:59Z

conans/test/integration/install_missing_dep_test.py

+from conans.paths import CONAN_MANIFEST
+
+
+class InstallMissingDependencie(unittest.TestCase):


InstallMissingDependency

memsharded · 2018-08-30T14:56:38Z

conans/test/integration/install_missing_dep_test.py

+
+class InstallMissingDependencie(unittest.TestCase):
+
+    def setUp(self):


Don't extract it to the setUp(), unless it is clear common code shared by several tests in the fixture. Just put the preparation code in the test itself.

memsharded · 2018-08-30T14:57:17Z

conans/test/integration/install_missing_dep_test.py

+
+        self.client.save({"conanfile.py": foo_conanfile.format(dep1_version="2.0")},
+                         clean_first=True)
+        self.client.run("create . lasote/testing", ignore_error=True)


This is exactly the place you have to check the returned error from run()

ok, I check for the return value. I thought that checking for the text itself in the following lines would be enough, but no, you are right, it is important to check that the command has failed

memsharded · 2018-08-30T14:57:55Z

conans/test/integration/install_missing_dep_test.py

+class FooPkg(ConanFile):
+    name = "foo"
+    version = "1.0"
+    requires = "dep1/{dep1_version}@lasote/testing", "dep2/1.0@lasote/testing"


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

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.

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.

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.

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

memsharded · 2018-08-30T16:22:05Z

conans/test/integration/install_missing_dep_test.py

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


remove this error

memsharded · 2018-08-30T16:22:45Z

conans/test/integration/install_missing_dep_test.py

+
+    def missing_dep_test(self):
+        test_server = TestServer()
+        self.servers = {"myremote": test_server}


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

memsharded · 2018-08-30T16:23:03Z

conans/test/integration/install_missing_dep_test.py

+    def missing_dep_test(self):
+        test_server = TestServer()
+        self.servers = {"myremote": test_server}
+        self.client = TestClient(servers=self.servers, users={"myremote": [("lasote", "mypass")]})


Do not use self.client if client is possible

memsharded · 2018-08-30T16:24:19Z

conans/test/integration/install_missing_dep_test.py

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


Name also unnecessary.

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?

Fair enough, you can leave the name.

memsharded · 2018-08-30T16:25:36Z

conans/test/integration/install_missing_dep_test.py

+class FooPkg(ConanFile):
+    name = "foo"
+    version = "1.0"
+    requires = "dep1/{dep1_version}@lasote/testing", "dep2/1.0@lasote/testing"


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

memsharded · 2018-08-30T22:12:58Z

conans/test/integration/install_missing_dep_test.py

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


Fair enough, you can leave the name.

memsharded · 2018-08-30T22:13:43Z

conans/test/integration/install_missing_dep_test.py

+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


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.

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

add dependencies to package output info when binary is not found

7be60f2

ghost assigned jgsogo Aug 30, 2018

ghost added the stage: review label Aug 30, 2018

jgsogo requested a review from memsharded August 30, 2018 08:37

memsharded added this to the 1.8 milestone Aug 30, 2018

jgsogo added 2 commits August 30, 2018 14:17

add test

13f5ee0

check that message contains expected messsages

038fc76

memsharded reviewed Aug 30, 2018

View reviewed changes

fixes according to @memsharded review: code simplification

5c091a9

memsharded reviewed Aug 30, 2018

View reviewed changes

remove server and 'self.' from places where it is not needed

3623f1a

memsharded approved these changes Aug 30, 2018

View reviewed changes

remove unused imports

2a3af44

memsharded merged commit 860fb81 into conan-io:develop Aug 31, 2018

ghost removed the stage: review label Aug 31, 2018

jgsogo deleted the issue/3316 branch September 19, 2018 12:21

danimtb mentioned this pull request Sep 26, 2018

"Missing prebuilt package" error message is missing essential information #3621

Closed

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018

Merge pull request conan-io#3438 from jgsogo/issue/3316

4a8cabf

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

fulara mentioned this pull request Jan 15, 2020

[feature] missing prebuilt package should have capability to provide more details #6364

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jgsogo commented Aug 30, 2018 •

edited by lasote

Loading

memsharded commented Aug 30, 2018

memsharded Aug 30, 2018

jgsogo Aug 30, 2018 •

edited

Loading

memsharded Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

jgsogo Aug 30, 2018

memsharded Aug 30, 2018

jgsogo Aug 30, 2018

memsharded Aug 30, 2018

jgsogo Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

jgsogo Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

memsharded Aug 30, 2018

		from conans.paths import CONAN_MANIFEST


		class InstallMissingDependencie(unittest.TestCase):


		class InstallMissingDependencie(unittest.TestCase):

		def setUp(self):

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

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

Conversation

jgsogo commented Aug 30, 2018 • edited by lasote Loading

memsharded commented Aug 30, 2018

Choose a reason for hiding this comment

jgsogo Aug 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgsogo commented Aug 30, 2018 •

edited by lasote

Loading

jgsogo Aug 30, 2018 •

edited

Loading