From 32b591a44f1e335c43cf96b692140adfddd9085f Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 8 Feb 2024 08:37:14 -0300 Subject: [PATCH] Return failure code to the system when commands fail Currently mu will always return 0 to the system even when the git commands have failed. While it is definitely good to always try the operations in all repositories (instead of stopping at the first error), always returning 0 to the system makes using 'mu' in scripts difficult, as we cannot easily be sure that the operations were successful. In addition, this will now print (in red) the repositories where the operation has failed: a very common beginner mistake when using `mu` is to not notice that the command failed for one of the repositories, as it may end up at the top of the output. --- mu | 6 +++++- mu.py | 6 +++++- mu_repo/__init__.py | 10 +++++++++ mu_repo/action_checkout.py | 2 ++ mu_repo/action_default.py | 11 +++++++--- mu_repo/execute_git_command_in_thread.py | 3 +++ mu_repo/execute_parallel_command.py | 1 + mu_repo/tests/test_action_default.py | 26 ++++++++++++++++++++++++ mu_repo/tests/test_checkout.py | 10 ++++++--- 9 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 mu_repo/tests/test_action_default.py diff --git a/mu b/mu index 6e0ecd4..dd45c5e 100755 --- a/mu +++ b/mu @@ -1,4 +1,8 @@ #!/usr/bin/env python if __name__ == '__main__': + import sys import mu_repo - mu_repo.main() + + status = mu_repo.main() + success = status is None or status.succeeded + sys.exit(0 if success else 1) diff --git a/mu.py b/mu.py index 16fd582..5ac4715 100644 --- a/mu.py +++ b/mu.py @@ -1,4 +1,8 @@ #!/usr/bin/env python if __name__ == '__main__': + import sys import mu_repo - mu_repo.main() + + status = mu_repo.main() + success = status is None or status.succeeded + sys.exit(0 if success else 1) diff --git a/mu_repo/__init__.py b/mu_repo/__init__.py index 38f35ef..e45d244 100644 --- a/mu_repo/__init__.py +++ b/mu_repo/__init__.py @@ -26,6 +26,16 @@ def __init__(self, status_message, succeeded, config=None): self.succeeded = succeeded self.config = config + def __repr__(self): + return f"Status(status_message={self.status_message!r}, succeeded={self.succeeded!r})" + + def __eq__(self, other): + return ( + self.status_message == other.status_message + and self.succeeded == other.succeeded + and self.config == other.config + ) + #=================================================================================================== # Params #=================================================================================================== diff --git a/mu_repo/action_checkout.py b/mu_repo/action_checkout.py index dcc1723..980654b 100644 --- a/mu_repo/action_checkout.py +++ b/mu_repo/action_checkout.py @@ -2,6 +2,7 @@ from mu_repo.backwards import iteritems from mu_repo.get_repos_and_local_branches import GetReposAndLocalBranches from mu_repo.print_ import Print +from mu_repo import Status #=================================================================================================== # Run @@ -33,4 +34,5 @@ def Run(params): Print('Found more than one branch that matches ${START_COLOR}%s${RESET_COLOR}:\n' % params.args[1]) PrintBranchToRepos(branch_to_repos, params) Print('\n${START_COLOR}ERROR${RESET_COLOR}: unable to decide branch to work on.', __color__='RED') + return Status("ERROR", succeeded=False) diff --git a/mu_repo/action_default.py b/mu_repo/action_default.py index 9f425ec..b020a07 100644 --- a/mu_repo/action_default.py +++ b/mu_repo/action_default.py @@ -49,7 +49,12 @@ def Run(params, on_output=None): else: commands.append(ParallelCmd(repo, [config.git] + args)) - ExecuteInParallel(commands, on_output, serial=config.serial) - - return Status('Finished', True) + cmd_return_codes = ExecuteInParallel(commands, on_output, serial=config.serial) + failed_cmds = [cmd for (cmd, returncode) in cmd_return_codes if returncode != 0] + if failed_cmds: + message = "Failed:\n" + "\n".join(f' {x.repo}' for x in failed_cmds) + Print('\n${START_COLOR}' + message + '${RESET_COLOR}', __color__="RED") + return Status(message, False) + else: + return Status('Finished', True) diff --git a/mu_repo/execute_git_command_in_thread.py b/mu_repo/execute_git_command_in_thread.py index 9dee8e9..b7c952a 100644 --- a/mu_repo/execute_git_command_in_thread.py +++ b/mu_repo/execute_git_command_in_thread.py @@ -42,6 +42,7 @@ def __init__(self, repo, cmd, output_queue): self.repo = repo self.cmd = cmd self.output_queue = output_queue + self.returncode = None class ReaderThread(threading.Thread): @@ -97,6 +98,7 @@ def run(self, serial=False): PrintError('Error executing: ' + ' '.join(cmd) + ' on: ' + repo) if p is not None: p.wait() + self.returncode = p.returncode else: try: @@ -123,6 +125,7 @@ def run(self, serial=False): stderr = AsStr(self.stderr_thread.GetFullOutput()) self._HandleOutput(msg, stdout, stderr) + self.returncode = p.returncode def GetPartialStderrOutput(self): stderr_thread = getattr(self, 'stderr_thread', None) diff --git a/mu_repo/execute_parallel_command.py b/mu_repo/execute_parallel_command.py index a203bc6..58b5bab 100644 --- a/mu_repo/execute_parallel_command.py +++ b/mu_repo/execute_parallel_command.py @@ -82,6 +82,7 @@ def ExecuteInParallel(commands, on_output=None, serial=False): else: from mu_repo.execute_git_command_in_thread import ExecuteThreadsHandlingOutputQueue ExecuteThreadsHandlingOutputQueue(threads, output_queue, on_output=on_output) + return [(commands[i], t.returncode) for i, t in enumerate(threads)] diff --git a/mu_repo/tests/test_action_default.py b/mu_repo/tests/test_action_default.py new file mode 100644 index 0000000..f62b007 --- /dev/null +++ b/mu_repo/tests/test_action_default.py @@ -0,0 +1,26 @@ +import subprocess +from pathlib import Path + + +import mu_repo + + +def test_action_default(workdir, monkeypatch): + workdir = Path(workdir) + monkeypatch.chdir(workdir) + subprocess.check_call(f'git init core') + subprocess.check_call(f'git init app') + (workdir/ 'app/.mu_repo').write_text("repo=.\nrepo=../core\n", encoding="utf-8") + + monkeypatch.chdir(workdir/ 'app') + subprocess.check_call(f'git config --add foo.bar foo-value') + + subprocess.check_call(f'git config --get foo.bar') + + status = mu_repo.main(args=['config', '--get', 'core.bare'], config_file='.mu_repo') + assert status == mu_repo.Status("Finished", succeeded=True) + + status = mu_repo.main(args=['config', '--get', 'foo.bar'], config_file='.mu_repo') + assert status == mu_repo.Status("Failed:\n ../core", succeeded=False) + + diff --git a/mu_repo/tests/test_checkout.py b/mu_repo/tests/test_checkout.py index 616ceb5..e79bb76 100644 --- a/mu_repo/tests/test_checkout.py +++ b/mu_repo/tests/test_checkout.py @@ -36,17 +36,21 @@ def test_checkout_partial_names(workdir): mu_repo.main(config_file='.mu_repo', args=['branch', 'rb-scissors']) # Checkout fb-rock on both projects - mu_repo.main(config_file='.mu_repo', args=['checkout', 'fb-rock']) + status = mu_repo.main(config_file='.mu_repo', args=['checkout', 'fb-rock']) + assert status == mu_repo.Status("Finished", True) assert 'fb-rock' == get_current_branch('app') assert 'fb-rock' == get_current_branch('lib') # Only one possible mathc, switch to rb-scissors - mu_repo.main(config_file='.mu_repo', args=['checkout', 'rb-']) + status = mu_repo.main(config_file='.mu_repo', args=['checkout', 'rb-']) + assert status == mu_repo.Status("Finished", True) assert 'rb-scissors' == get_current_branch('app') assert 'rb-scissors' == get_current_branch('lib') # Couldn't guess branch name, do not checkout - mu_repo.main(config_file='.mu_repo', args=['checkout', 'fb-']) + status = mu_repo.main(config_file='.mu_repo', args=['checkout', 'fb-']) + assert status == mu_repo.Status("ERROR", False) + assert 'rb-scissors' == get_current_branch('app') assert 'rb-scissors' == get_current_branch('lib')