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

Save/restore ConanOutput global state when using Conan API's command #17095

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
16 changes: 15 additions & 1 deletion conan/api/subapi/command.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from conans.errors import ConanException
from conan.api.output import ConanOutput


class CommandAPI:
Expand All @@ -20,5 +21,18 @@ def run(self, cmd):
command = commands[current_cmd]
except KeyError:
raise ConanException(f"Command {current_cmd} does not exist")
# Conan has some global state in the ConanOutput class that
# get redefined when running a command and leak to the calling scope
# if running from a custom command.
# Store the old one and restore it after the command execution as a workaround.
_conan_output_level = ConanOutput._conan_output_level
Copy link
Member Author

Choose a reason for hiding this comment

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

This pattern is spread thru the code in a few places, we could have something like this maybe?

@contenxtmanager
def restore_after(entity, *attributes):
    old_values = [getvalue(entity, attribute) for attribute in attributes)
    try:
        yield
    finally:
        for attribute, old_value in zip(attributes, old_values):
            setattr(entity, attribute, old_value)

which would be used in 3-4 places from a quick look. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I have reviewed all finally in our code, I am not sure it is worth the abstraction at this point, but we can discussed, I am not opposed.

_silent_warn_tags = ConanOutput._silent_warn_tags
_warnings_as_errors = ConanOutput._warnings_as_errors

return command.run_cli(self.conan_api, args)
try:
result = command.run_cli(self.conan_api, args)
finally:
ConanOutput._conan_output_level = _conan_output_level
ConanOutput._silent_warn_tags = _silent_warn_tags
ConanOutput._warnings_as_errors = _warnings_as_errors
return result
26 changes: 26 additions & 0 deletions test/integration/command_v2/custom_commands_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,32 @@ def mycmd2(conan_api, parser, *args, **kwargs):
assert "MYCMD1!!!!!" in c.out
assert "MYCMD2!!!!!" in c.out

def test_command_verbosity_leak(self):
mycommand = textwrap.dedent(f"""
import json
from conan.cli.command import conan_command
from conan.api.output import ConanOutput

@conan_command(group="custom commands")
def mycommand(conan_api, parser, *args, **kwargs):
\""" mycommand help \"""
parser.add_argument("foo", help="foo")
args = parser.parse_args(*args)

out = ConanOutput()
out.title("This is my first title")
conan_api.command.run("config home")
out.title("This is my second title")
""")

c = TestClient()
command_file_path = os.path.join(c.cache_folder, 'extensions',
'commands', 'cmd_mycommand.py')
c.save({f"{command_file_path}": mycommand})
c.run("mycommand foo -vquiet")
assert "This is my first title" not in c.out
assert "This is my second title" not in c.out

def test_command_reuse_interface_create(self):
mycommand = textwrap.dedent("""
import json
Expand Down