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

Conversation

Projects
None yet
2 participants
@jgsogo
Copy link
Member

commented Aug 30, 2018

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 Aug 30, 2018

@memsharded

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2018

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)

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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

This comment has been minimized.

Copy link
@jgsogo

jgsogo Aug 30, 2018

Author Member

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

from conans.paths import CONAN_MANIFEST


class InstallMissingDependencie(unittest.TestCase):

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

InstallMissingDependency


class InstallMissingDependencie(unittest.TestCase):

def setUp(self):

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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)

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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

This comment has been minimized.

Copy link
@jgsogo

jgsogo Aug 30, 2018

Author Member

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"

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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

This comment has been minimized.

Copy link
@jgsogo

jgsogo Aug 30, 2018

Author Member

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.

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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.

This comment has been minimized.

Copy link
@jgsogo

jgsogo Aug 30, 2018

Author Member

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.

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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")

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

remove this error


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

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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")]})

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

Do not use self.client if client is possible

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

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

Name also unnecessary.

This comment has been minimized.

Copy link
@jgsogo

jgsogo Aug 30, 2018

Author Member

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?

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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"

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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"

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 30, 2018

Contributor

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

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Aug 31, 2018

@jgsogo jgsogo deleted the jgsogo:issue/3316 branch Sep 19, 2018

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
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
You can’t perform that action at this time.