diff --git a/CHANGES.md b/CHANGES.md index ac1f3e8..b74bc41 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,11 @@ with a `# managed by mxdev` marker and prunes managed entries that are no longer configured, while leaving user-defined sources untouched. [jensens] +- Fix #78: Give a clear, actionable error when a configured `branch` does not exist on the + remote (e.g. it was deleted), naming the package, branch and URL and pointing at the + `mx.ini` setting, on both checkout and update. Expected VCS errors are no longer reported + with a full Python traceback (the traceback is kept at debug level). [jensens] + ## 5.3.2 (2026-05-30) diff --git a/src/mxdev/vcs/common.py b/src/mxdev/vcs/common.py index c409fe2..219b68c 100644 --- a/src/mxdev/vcs/common.py +++ b/src/mxdev/vcs/common.py @@ -426,11 +426,14 @@ def worker(working_copies: WorkingCopies, the_queue: queue.Queue) -> None: return try: output = action(**kwargs) - except WCError: + except WCError as e: with output_lock: for lvl, msg in wc._output: lvl(msg) - logger.exception("Can not execute action!") + # WCError is an expected operational failure: show a clean, + # actionable message and keep the full traceback for debug only. + logger.error("%s", e) + logger.debug("Traceback for the error above:", exc_info=True) working_copies.errors = True else: with output_lock: diff --git a/src/mxdev/vcs/git.py b/src/mxdev/vcs/git.py index 07ecb89..4c88c31 100644 --- a/src/mxdev/vcs/git.py +++ b/src/mxdev/vcs/git.py @@ -15,6 +15,14 @@ class GitError(common.WCError): pass +def _branch_not_found_message(name: str, branch: str, url: str) -> str: + """Build a clear, actionable error for a configured branch that is missing.""" + return ( + f"Branch '{branch}' for package '{name}' does not exist at '{url}'. " + f"Check the 'branch' setting for [{name}] in your mx.ini." + ) + + class GitWorkingCopy(common.BaseWorkingCopy): """The git working copy. @@ -117,8 +125,7 @@ def git_merge_rbranch(self, stdout_in: str, stderr_in: str, accept_missing: bool if accept_missing: logger.info("No such branch %r", branch) return (stdout_in, stderr_in) - logger.error("No such branch %r", branch) - sys.exit(1) + raise GitError(_branch_not_found_message(self.source["name"], branch, self.source["url"])) rbp = self._remote_branch_prefix cmd = self.run_git(["merge", f"{rbp}/{branch}"], cwd=path) @@ -151,6 +158,9 @@ def git_checkout(self, **kwargs) -> str | None: cmd = self.run_git(args) stdout, stderr = cmd.communicate() if cmd.returncode != 0: + branch = self.source.get("branch") + if branch and "not found in upstream" in stderr: + raise GitError(_branch_not_found_message(name, branch, url)) raise GitError(f"git cloning of '{name}' failed.\n{stderr}") if "rev" in self.source: stdout, stderr = self.git_switch_branch(stdout, stderr) @@ -206,8 +216,7 @@ def git_switch_branch(self, stdout_in: str, stderr_in: str, accept_missing: bool self.output((logger.info, f"No such branch {branch}")) return (stdout_in + stdout, stderr_in + stderr) else: - self.output((logger.error, f"No such branch {branch}")) - sys.exit(1) + raise GitError(_branch_not_found_message(self.source["name"], branch, self.source["url"])) # runs the checkout with predetermined arguments cmd = self.run_git(argv, cwd=path) stdout, stderr = cmd.communicate() diff --git a/tests/test_common.py b/tests/test_common.py index a6b957f..539b33d 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -525,7 +525,9 @@ def update(self, **kwargs): common.worker(working_copies, test_queue) assert working_copies.errors is True - assert "Can not execute action!" in caplog.text + # The actual error message is shown (no generic message / traceback noise). + assert "Test error" in caplog.text + assert "Can not execute action!" not in caplog.text def test_worker_with_bytes_output(mocker): diff --git a/tests/test_git.py b/tests/test_git.py index f33233c..d522d6c 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -304,3 +304,56 @@ def test_offline_prevents_vcs_operations(mkgitrepo, src): # After normal update, should have both files assert {x for x in path.iterdir()} == {path / ".git", path / "foo", path / "bar"} + + +def test_checkout_missing_branch_gives_clear_error(mkgitrepo, src, caplog): + """Cloning a configured branch that does not exist must yield a clear, + actionable error (issue #78) instead of a generic failure + traceback. + """ + import logging + + repository = mkgitrepo("repository") + create_default_content(repository) + path = src / "egg" + sources = { + "egg": dict( + vcs="git", + name="egg", + branch="does-not-exist", + url=str(repository.base), + path=str(path), + ) + } + + with caplog.at_level(logging.ERROR): + vcs_checkout(sources, ["egg"], False) + + assert "Branch 'does-not-exist' for package 'egg' does not exist" in caplog.text + assert "Check the 'branch' setting for [egg] in your mx.ini" in caplog.text + # No alarming generic message / traceback for an expected operational error. + assert "Can not execute action!" not in caplog.text + + +def test_update_missing_branch_gives_clear_error(mkgitrepo, src, caplog): + """Updating to a configured branch that no longer exists must yield the same + clear error (issue #78) instead of a terse 'No such branch' + sys.exit. + """ + import logging + + repository = mkgitrepo("repository") + create_default_content(repository) + path = src / "egg" + + sources_ok = {"egg": dict(vcs="git", name="egg", branch="master", url=str(repository.base), path=str(path))} + vcs_checkout(sources_ok, ["egg"], False) + + sources_bad = { + "egg": dict(vcs="git", name="egg", branch="does-not-exist", url=str(repository.base), path=str(path)) + } + caplog.clear() + with caplog.at_level(logging.ERROR): + vcs_update(sources_bad, ["egg"], False) + + assert "Branch 'does-not-exist' for package 'egg' does not exist" in caplog.text + assert "Check the 'branch' setting for [egg] in your mx.ini" in caplog.text + assert "Can not execute action!" not in caplog.text diff --git a/tests/test_git_additional.py b/tests/test_git_additional.py index 1bf514a..56d37e5 100644 --- a/tests/test_git_additional.py +++ b/tests/test_git_additional.py @@ -275,6 +275,7 @@ def test_git_merge_rbranch_missing_branch_accept(): def test_git_merge_rbranch_missing_branch_no_accept(): """Test git_merge_rbranch with missing branch and accept_missing=False.""" + from mxdev.vcs.git import GitError from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): @@ -293,9 +294,12 @@ def test_git_merge_rbranch_missing_branch_no_accept(): mock_process.communicate.return_value = ("* main\n develop\n", "") with patch.object(wc, "run_git", return_value=mock_process): - with pytest.raises(SystemExit): + with pytest.raises(GitError) as excinfo: wc.git_merge_rbranch("", "", accept_missing=False) + assert "Branch 'nonexistent' for package 'test-package' does not exist" in str(excinfo.value) + assert "Check the 'branch' setting for [test-package] in your mx.ini" in str(excinfo.value) + def test_git_merge_rbranch_merge_failure(): """Test git_merge_rbranch handles merge failure.""" @@ -888,6 +892,7 @@ def test_git_switch_branch_failure(): def test_git_switch_branch_missing_no_accept(): """Test git_switch_branch with missing branch and accept_missing=False.""" + from mxdev.vcs.git import GitError from mxdev.vcs.git import GitWorkingCopy with patch("mxdev.vcs.common.which", return_value="/usr/bin/git"): @@ -905,8 +910,12 @@ def test_git_switch_branch_missing_no_accept(): mock_process.communicate.return_value = ("* main\n", "") with patch.object(wc, "run_git", return_value=mock_process): - with pytest.raises(SystemExit): - wc.git_switch_branch("", "", accept_missing=False) + with patch.object(wc, "git_version", return_value=(2, 30, 0)): + with pytest.raises(GitError) as excinfo: + wc.git_switch_branch("", "", accept_missing=False) + + assert "Branch 'nonexistent' for package 'test-package' does not exist" in str(excinfo.value) + assert "Check the 'branch' setting for [test-package] in your mx.ini" in str(excinfo.value) def test_git_switch_branch_missing_accept():