Skip to content

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

Merged
memsharded merged 6 commits into
conan-io:developfrom
jgsogo:issue/3316
Aug 31, 2018
Merged

memsharded merged 6 commits into
conan-io:developfrom
jgsogo:issue/3316

Conversation

@jgsogo

@jgsogo jgsogo commented Aug 30, 2018

Copy link
Copy Markdown
Contributor

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.

@ghost ghost assigned jgsogo Aug 30, 2018
@ghost ghost added the stage: review label Aug 30, 2018
@jgsogo
jgsogo requested a review from memsharded August 30, 2018 08:37
@memsharded

Copy link
Copy Markdown
Member

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

@memsharded memsharded added this to the 1.8 milestone Aug 30, 2018
requires = "dep1/1.0@lasote/testing"
"""

self.client.save({"conanfile.py": dep1_conanfile.format(version="1.0")}, clean_first=True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@jgsogo jgsogo Aug 30, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

from conans.paths import CONAN_MANIFEST


class InstallMissingDependencie(unittest.TestCase):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

InstallMissingDependency


class InstallMissingDependencie(unittest.TestCase):

def setUp(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.


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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove this error


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

Copy link
Copy Markdown
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.

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

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

Copy link
Copy Markdown
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.

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

Copy link
Copy Markdown
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.

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
Copy Markdown
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.

@memsharded
memsharded merged commit 860fb81 into conan-io:develop Aug 31, 2018
@ghost ghost removed the stage: review label Aug 31, 2018
@jgsogo
jgsogo deleted the issue/3316 branch September 19, 2018 12:21
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
Add dependencies to package output info when binary is not found (close conan-io#3316)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better information on mismatch in dependency tree

2 participants