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.

Sign up for GitHub

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 ConanInvalidConfiguration exception and handle error codes. #3517

Merged
merged 3 commits into from
Sep 24, 2018
Merged

Add ConanInvalidConfiguration exception and handle error codes. #3517

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36b210c
add exception
jgsogo Sep 10, 2018
369ca34
test for command line
jgsogo Sep 10, 2018
28aeab5
move exit codes to global scope
jgsogo Sep 11, 2018
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
23 changes: 16 additions & 7 deletions conans/client/command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from conans.client.conan_command_output import CommandOutputer
from conans.client.output import Color

from conans.errors import ConanException, NoRemoteAvailable
from conans.errors import ConanException, NoRemoteAvailable, ConanInvalidConfiguration
from conans.model.ref import ConanFileReference
from conans.util.config_parser import get_bool_from_text
from conans.util.log import logger
Expand Down Expand Up @@ -1267,7 +1267,11 @@ def run(self, *args):
"""HIDDEN: entry point for executing commands, dispatcher to class
methods
"""
errors = False
jgsogo marked this conversation as resolved.
Show resolved Hide resolved
SUCCESS = 0
ERROR_GENERAL = 1
ERROR_INVALID_CONFIGURATION = 5

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 think that we need a single place to put this hardcoded values, as they are tightly coupled with those documented in the main method:

def main(args):
    """ main entry point of the conan application, using a Command to
    parse parameters

    Exit codes for conan command:

        0: Success (done)
        1: General ConanException error (done)
        2: Migration error
        3: Ctrl+C
        4: Ctrl+Break
        5: Invalid configuration (done)
    """

Copy link
Member

Choose a reason for hiding this comment

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

Yes, feel free to move them to global scope, I guess here, in the command.py file? Other file?

ret_code = SUCCESS
try:
try:
command = args[0][0]
Expand All @@ -1287,24 +1291,28 @@ def run(self, *args):
method(args[0][1:])
except KeyboardInterrupt as exc:
logger.error(exc)
errors = True
ret_code = SUCCESS
except SystemExit as exc:
if exc.code != 0:
logger.error(exc)
self._user_io.out.error("Exiting with code: %d" % exc.code)
errors = exc.code
ret_code = exc.code
except ConanInvalidConfiguration as exc:
ret_code = ERROR_INVALID_CONFIGURATION
msg = exception_message_safe(exc)
self._user_io.out.error(msg)
except ConanException as exc:
errors = True
ret_code = ERROR_GENERAL
msg = exception_message_safe(exc)
self._user_io.out.error(msg)
except Exception as exc:
import traceback
print(traceback.format_exc())
errors = True
ret_code = ERROR_GENERAL
msg = exception_message_safe(exc)
self._user_io.out.error(msg)

return errors
return ret_code


def get_reference_fields(arg_reference):
Expand Down Expand Up @@ -1394,6 +1402,7 @@ def main(args):
2: Migration error
3: Ctrl+C
4: Ctrl+Break
5: Invalid configuration (done)
"""
try:
conan_api, client_cache, user_io = Conan.factory()
Expand Down
7 changes: 7 additions & 0 deletions conans/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ def conanfile_exception_formatter(conanfile_name, func_name):
"""
try:
yield
except ConanInvalidConfiguration as exc:
msg = "{}: Invalid configuration: {}".format(conanfile_name, exc) # TODO: Move from here?
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 don't like having this message formatted here... any ideas?

Copy link
Member

Choose a reason for hiding this comment

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

I think it is consistent to the way exceptions from conanfile.py are managed. Check _format_conanfile_exception(), it might be worth to print the line number, etc. in the error message too.

Otherwise, if you wanted something specific to configure(), you could try-except the call to conanfile.configure() in graph_builder.py, but wouldn't be much cleaner either.

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 prefer to handle the exception message here, better not pollute the graph code.

This is not an error, is it? so I don't think that adding information about file (it is always conanfile.py), or line (always inside configure) adds any information to the caller. It contains the conan ref and the message defined in the recipe, so it should be enough.

Copy link
Member

@memsharded memsharded Sep 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking about lazy raise ConanInvalidConfiguration("Unsupported platform"), and which some context, probably seeing the line above could help. But yes, fair, leave it as is, it is good.

raise ConanInvalidConfiguration(msg)
except Exception as exc:
msg = _format_conanfile_exception(conanfile_name, func_name, exc)
raise ConanExceptionInUserConanfileMethod(msg)
Expand Down Expand Up @@ -95,6 +98,10 @@ class ConanExceptionInUserConanfileMethod(ConanException):
pass


class ConanInvalidConfiguration(ConanExceptionInUserConanfileMethod):
pass


# Remote exceptions #
class InternalErrorException(ConanException):
"""
Expand Down
68 changes: 68 additions & 0 deletions conans/test/command/invalid_configuration_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import unittest
import platform
import os

from conans.test.utils.tools import TestClient, TestServer
from conans.model.ref import ConanFileReference, PackageReference
from conans.paths import CONANFILE, CONANINFO
from conans.model.info import ConanInfo
from conans.test.utils.cpp_test_files import cpp_hello_conan_files
from conans.paths import CONANFILE_TXT
from conans.client.conf.detect import detected_os
from conans.util.files import load, mkdir, rmdir


class InvalidConfigurationTest(unittest.TestCase):

def setUp(self):
self.client = TestClient()
self.client.save({"conanfile.py": """
from conans import ConanFile
from conans.errors import ConanInvalidConfiguration
jgsogo marked this conversation as resolved.
Show resolved Hide resolved

class MyPkg(ConanFile):
settings = "os", "compiler", "build_type", "arch"

def configure(self):
if self.settings.compiler.version == "12":
raise ConanInvalidConfiguration("user says that compiler.version=12 is invalid")

"""})
settings = "-s os=Windows -s compiler='Visual Studio' -s compiler.version={ver}"
self.settings_msvc15 = settings.format(ver="15")
self.settings_msvc12 = settings.format(ver="12")

def test_install_method(self):
self.client.run("install . %s" % self.settings_msvc15)

error = self.client.run("install . %s" % self.settings_msvc12, ignore_error=True)
self.assertEqual(error, 5) # TODO: hardcoded!!
self.assertIn("Invalid configuration: user says that compiler.version=12 is invalid",
self.client.out)

def test_create_method(self):
self.client.run("create . name/ver@jgsogo/test %s" % self.settings_msvc15)

error = self.client.run("create . name/ver@jgsogo/test %s" % self.settings_msvc12,
ignore_error=True)
self.assertEqual(error, 5) # TODO: hardcoded!!
jgsogo marked this conversation as resolved.
Show resolved Hide resolved
self.assertIn("name/ver@jgsogo/test: Invalid configuration: user says that "
"compiler.version=12 is invalid", self.client.out)

def test_as_requirement(self):
self.client.run("create . name/ver@jgsogo/test %s" % self.settings_msvc15)
self.client.save({"other/conanfile.py": """
from conans import ConanFile
from conans.errors import ConanInvalidConfiguration

class MyPkg(ConanFile):
requires = "name/ver@jgsogo/test"
settings = "os", "compiler", "build_type", "arch"
"""})
self.client.run("create other/ other/ver@jgsogo/test %s" % self.settings_msvc15)

error = self.client.run("create other/ other/ver@jgsogo/test %s" % self.settings_msvc12,
ignore_error=True)
self.assertEqual(error, 5) # TODO: hardcoded!!
jgsogo marked this conversation as resolved.
Show resolved Hide resolved
self.assertIn("name/ver@jgsogo/test: Invalid configuration: user says that "
"compiler.version=12 is invalid", self.client.out)