diff --git a/odev/__main__.py b/odev/__main__.py index 5df45d48..78a1afbd 100644 --- a/odev/__main__.py +++ b/odev/__main__.py @@ -35,7 +35,7 @@ def main(): odev = init_framework() odev.start(start_time) logger.debug(f"Framework started in {monotonic() - start_time:.3f} seconds") - odev.dispatch() + sys.exit(0 if odev.dispatch() else 1) except OdevError as error: logger.error(error) diff --git a/odev/_version.py b/odev/_version.py index 4226fa7f..5626e70a 100644 --- a/odev/_version.py +++ b/odev/_version.py @@ -22,4 +22,4 @@ # or merged change. # ------------------------------------------------------------------------------ -__version__ = "4.26.0" +__version__ = "4.27.0" diff --git a/odev/commands/database/cloc.py b/odev/commands/database/cloc.py index 1c4d4bba..7facb90b 100644 --- a/odev/commands/database/cloc.py +++ b/odev/commands/database/cloc.py @@ -40,7 +40,7 @@ def run(self): process = self.odoobin.run(args=self.args.odoo_args, subcommand=self._name, stream=False) - if process is None: + if process is None or process.returncode: raise self.error("Failed to fetch cloc result.") headers = [ diff --git a/odev/commands/database/create.py b/odev/commands/database/create.py index 344c92eb..413fe74f 100644 --- a/odev/commands/database/create.py +++ b/odev/commands/database/create.py @@ -39,6 +39,7 @@ class CreateCommand(OdoobinTemplateCommand): ) version_argument = args.String( name="version", + aliases=["-V", "--version"], description="""The Odoo version to use for the new database. If not specified and a template is provided, the version of the template database will be used. Otherwise, the version will default to "master". diff --git a/odev/commands/database/delete.py b/odev/commands/database/delete.py index e021042a..548ec6a4 100644 --- a/odev/commands/database/delete.py +++ b/odev/commands/database/delete.py @@ -76,7 +76,7 @@ def run(self): databases_list: str = string.join_and([f"{db!r}" for db in databases]) logger.warning(f"You are about to delete the following databases: {databases_list}") - if not self.console.confirm("Are you sure?", default=False): + if not self.console.confirm("Are you sure?", default=self.args.bypass_prompt): raise self.error("Command aborted") tracker = progress.Progress() diff --git a/odev/commands/database/run.py b/odev/commands/database/run.py index 764a3348..3ae8a81d 100644 --- a/odev/commands/database/run.py +++ b/odev/commands/database/run.py @@ -76,4 +76,7 @@ def run(self): if self.odoobin.is_running: raise self.error(f"Database {self._database.name!r} is already running") - self.odoobin.run(args=self.args.odoo_args, progress=self.odoobin_progress) + process = self.odoobin.run(args=self.args.odoo_args, progress=self.odoobin_progress) + + if process and process.returncode: + raise self.error("Odoo process failed") diff --git a/odev/commands/database/test.py b/odev/commands/database/test.py index 4edae165..cc96bc61 100644 --- a/odev/commands/database/test.py +++ b/odev/commands/database/test.py @@ -38,6 +38,11 @@ class TestCommand(OdoobinCommand): description="Comma-separated list of modules to install for testing. If not set, install the base module.", ) + @property + def _database_exists_required(self) -> bool: + """Return True if a database has to exist for the command to work.""" + return not bool(self.args.version) + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.test_files: list[str] = [] @@ -84,8 +89,8 @@ def create_test_database(self): """Return the arguments to pass to the create command.""" args = ["--bare"] - if self._database.version is not None: - args.extend(["--version", str(self._database.version)]) + if self.version is not None: + args.extend(["--version", str(self.version)]) args.append(self.test_database.name) self.odev.run_command("create", *args) @@ -109,8 +114,14 @@ def run_test_database(self): self.create_test_database() odoobin = self.test_database.process or OdoobinProcess(self.test_database) - odoobin.with_version(self._database.version) - odoobin.with_edition(self._database.edition) + odoobin.with_version(self.version) + + edition = ( + "enterprise" + if self.args.enterprise or (self._database.exists and self._database.edition == "enterprise") + else "community" + ) + odoobin.with_edition(edition) odoobin.with_venv(self.venv) odoobin.with_worktree(self.worktree) @@ -126,16 +137,20 @@ def run_test_database(self): def odoobin_progress(self, line: str): """Handle odoo-bin output and fetch information real-time.""" - if re.match(r"^(i?pu?db)?>+", line): + if re.match(r"^(?:ipdb|pudb|pdb)>+|^\(Pdb\)|(?:^>\s+.*\.(?:py|js)\(\d+\))", line): raise self.error("Debugger detected in odoo-bin output, remove breakpoints and try again") problematic_test_levels = ("warning", "error", "critical") match = self._parse_progress_log_line(line) - if match is None: - if self.last_level in problematic_test_levels: + if match is None or not self.args.pretty: + if match is None and self.last_level in problematic_test_levels: self.test_buffer.append(line) + if not self.args.pretty: + self.print(line, highlight=False, soft_wrap=False) + return + color = f"logging.level.{self.last_level}" if self.last_level in problematic_test_levels else "color.black" self.print(string.stylize(line, color), highlight=False, soft_wrap=False) return diff --git a/odev/commands/git/worktree.py b/odev/commands/git/worktree.py index 9711c68b..1fda8292 100644 --- a/odev/commands/git/worktree.py +++ b/odev/commands/git/worktree.py @@ -90,7 +90,8 @@ def create_worktree(self): self.__check_name() if self.args.name in self.grouped_worktrees: - raise self.error(f"Worktree with name '{self.args.name}' already exists") + logger.info(f"Worktree with name '{self.args.name}' already exists") + return with progress.spinner(f"Creating worktree {self.args.name}"): for repository in self.repositories: diff --git a/odev/common/bash.py b/odev/common/bash.py index 1546fa7f..06888bec 100644 --- a/odev/common/bash.py +++ b/odev/common/bash.py @@ -36,7 +36,9 @@ # --- Helpers ------------------------------------------------------------------ -def __run_command(command: str, capture: bool = True, sudo_password: str | None = None) -> CompletedProcess[bytes]: +def __run_command( + command: str, capture: bool = True, sudo_password: str | None = None, env: dict[str, str] | None = None +) -> CompletedProcess[bytes]: """Execute a command as a subprocess. If `sudo_password` is provided and not `None`, the command will be executed with elevated privileges. @@ -45,6 +47,7 @@ def __run_command(command: str, capture: bool = True, sudo_password: str | None :param bool capture: Whether to capture the output of the command. :param str sudo_password: The password to use when executing the command with elevated privileges. + :param dict env: The environment variables to use when executing the command. :return: The result of the command execution. :rtype: CompletedProcess """ @@ -58,6 +61,7 @@ def __run_command(command: str, capture: bool = True, sudo_password: str | None check=True, capture_output=capture, input=sudo_password.encode() if sudo_password is not None else None, + env=env, ) @@ -80,7 +84,9 @@ def __raise_or_log(exception: CalledProcessError, do_raise: bool) -> None: # --- Public API --------------------------------------------------------------- -def execute(command: str, sudo: bool = False, raise_on_error: bool = True) -> CompletedProcess[bytes] | None: +def execute( + command: str, sudo: bool = False, raise_on_error: bool = True, env: dict[str, str] | None = None +) -> CompletedProcess[bytes] | None: """Execute a command in the operating system and wait for it to complete. Output of the command will be captured and returned after the execution completes. @@ -97,7 +103,7 @@ def execute(command: str, sudo: bool = False, raise_on_error: bool = True) -> Co """ try: logger.debug(f"Running process: {shlex.quote(command)}") - process_result = __run_command(command) + process_result = __run_command(command, env=env) except CalledProcessError as exception: # If already running as root, sudo will not work if not sudo or not os.geteuid(): @@ -112,7 +118,7 @@ def execute(command: str, sudo: bool = False, raise_on_error: bool = True) -> Co return None try: - process_result = __run_command(command, sudo_password=sudo_password) + process_result = __run_command(command, sudo_password=sudo_password, env=env) except CalledProcessError as exception: sudo_password = None __raise_or_log(exception, raise_on_error) @@ -121,15 +127,16 @@ def execute(command: str, sudo: bool = False, raise_on_error: bool = True) -> Co return process_result -def run(command: str) -> CompletedProcess: +def run(command: str, env: dict[str, str] | None = None) -> CompletedProcess: """Execute a command in the operating system and wait for it to complete. Output of the command will not be captured and will be printed to the console in real-time. :param str command: The command to execute. + :param dict env: The environment variables to use when executing the command. """ logger.debug(f"Running process: {shlex.quote(command)}") - return __run_command(command, capture=False) + return __run_command(command, capture=False, env=env) def detached(command: str) -> Popen[bytes]: @@ -141,15 +148,16 @@ def detached(command: str) -> Popen[bytes]: return Popen(command, shell=True, start_new_session=True, stdout=DEVNULL, stderr=DEVNULL) # noqa: S602 - intentional use of shell=True -def stream(command: str) -> Generator[str, None, None]: # noqa: PLR0912 +def stream(command: str, env: dict[str, str] | None = None) -> Generator[str, None, None]: # noqa: PLR0912 """Execute a command in the operating system and stream its output line by line. :param str command: The command to execute. + :param dict env: The environment variables to use when executing the command. """ logger.debug(f"Streaming process: {shlex.quote(command)}") if not sys.stdin.isatty(): logger.warning("STDIN is not a TTY, running command in non-interactive mode") - exec_process = execute(command) + exec_process = execute(command, env=env) if not exec_process: yield "" @@ -164,13 +172,14 @@ def stream(command: str) -> Generator[str, None, None]: # noqa: PLR0912 master, slave = pty.openpty() try: - process = Popen( # noqa: S603 - shlex.split(command), + process = Popen( # noqa: S602 + command, stdout=slave, stderr=slave, stdin=slave, start_new_session=True, - universal_newlines=True, + shell=True, + env=env, ) received_buffer: bytes = b"" diff --git a/odev/common/commands/odoobin.py b/odev/common/commands/odoobin.py index b3f88405..51409df1 100644 --- a/odev/common/commands/odoobin.py +++ b/odev/common/commands/odoobin.py @@ -142,6 +142,12 @@ def version(self) -> OdooVersion: if self._database.version: return self._database.version + if self._database.exists and not self._database.is_odoo: + logger.warning( + f"Database {self._database.name!r} is not an Odoo database. Defaulting to 'master'. " + f"Consider using 'odev create -V {self._database.name}' to initialize it properly." + ) + return OdooVersion("master") @property diff --git a/odev/common/config.py b/odev/common/config.py index 806cbf23..b17bdc37 100644 --- a/odev/common/config.py +++ b/odev/common/config.py @@ -83,6 +83,17 @@ def dumps(self) -> Path: def dumps(self, value: str | Path): self.set("dumps", value.as_posix() if isinstance(value, Path) else value) + @property + def upgrade(self) -> Path: + """Path to the directory where Odoo Enterprise migration scripts are stored. + Defaults to ~/odoo/repositories/odoo/upgrade. + """ + return Path(cast(str, self.get("upgrade", "~/odoo/repositories/odoo/upgrade"))).expanduser() + + @upgrade.setter + def upgrade(self, value: str | Path): + self.set("upgrade", value.as_posix() if isinstance(value, Path) else value) + class UpdateSection(Section): """Configuration for odev auto-updates.""" diff --git a/odev/common/connectors/git.py b/odev/common/connectors/git.py index eba5afe1..3c805672 100644 --- a/odev/common/connectors/git.py +++ b/odev/common/connectors/git.py @@ -252,10 +252,13 @@ def __init__(self, repo: str, path: Path | None = None): """ super().__init__() + # If repo looks like an absolute path, use it as the path + if not path and repo and Path(repo).is_absolute(): + path = Path(repo) + self._path: Path | None = path - """Forced path to the git repository on the local system.""" - if path: + if path and path.joinpath(".git").exists(): repo_url = Repo(path).remote().url self._organization, self._repository = repo_url.removesuffix(".git").split("/")[-2:] @@ -371,6 +374,16 @@ def branch(self) -> str | None: return self.repository.active_branch.name.split("/")[-1] + @property + def is_dirty(self) -> bool: + """Whether the repository has uncommitted changes.""" + return self.repository.is_dirty(untracked_files=True) if self.exists and self.repository else False + + @property + def is_protected_branch(self) -> bool: + """Whether the current branch is a protected branch (main or master).""" + return self.branch in ("main", "master") + @property def requirements_path(self) -> Path: """Path to the requirements.txt path of the repo, if present.""" diff --git a/odev/common/connectors/rest.py b/odev/common/connectors/rest.py index 75c63e06..db22fb03 100644 --- a/odev/common/connectors/rest.py +++ b/odev/common/connectors/rest.py @@ -133,6 +133,19 @@ def _load_cookies(self): if cookie: self._connection.cookies.set(key, cookie, domain=domain) + def _get_cookie_header(self) -> str: + """Return the cookies as a string suitable for the 'Cookie' header.""" + if self._connection is None: + return "" + + cookies = [] + for domain in self.session_domains: + for key in self.session_cookies: + value = self._connection.cookies.get(key, domain=domain) + if value: + cookies.append(f"{key}={value}") + return "; ".join(cookies) + def _save_cookies(self): """Save session cookies to the secrets vault.""" if self._connection is None: @@ -269,7 +282,7 @@ def _request( logger_message + f" -> [{response.status_code}] {response.reason} ({response.elapsed.total_seconds():.3f} seconds)" ) - except RequestsConnectionError as error: + except (RequestsConnectionError, ConnectionResetError) as error: if retry_on_error: logger.debug(error) return self._request( @@ -283,11 +296,12 @@ def _request( raise ConnectorError(f"Could not connect to {self.name}", self) from error + self.cache(cache_key, response) + self._save_cookies() + if raise_for_status: response.raise_for_status() - self.cache(cache_key, response) - self._save_cookies() return response @abstractmethod diff --git a/odev/common/odev.py b/odev/common/odev.py index 3bad2497..4af66e5f 100644 --- a/odev/common/odev.py +++ b/odev/common/odev.py @@ -875,9 +875,10 @@ def run_command( return not command_errored - def dispatch(self, argv: list[str] | None = None) -> None: + def dispatch(self, argv: list[str] | None = None) -> bool: """Handle commands and arguments as received from the terminal. :param argv: Optional list of command-line arguments used to override arguments received from the CLI. + :return: True if the command were executed successfully, False otherwise. """ argv = (argv or sys.argv)[1:] @@ -895,7 +896,7 @@ def dispatch(self, argv: list[str] | None = None) -> None: logger.debug("Help argument or no command provided, falling back to help command") argv.insert(0, "help") - self.run_command(argv[0], *argv[1:], history=True) + return self.run_command(argv[0], *argv[1:], history=True) def check_release(self) -> None: """Check if a new release is available.""" diff --git a/odev/common/odoobin.py b/odev/common/odoobin.py index 591b8dfb..a8461bcc 100644 --- a/odev/common/odoobin.py +++ b/odev/common/odoobin.py @@ -49,6 +49,9 @@ ODOO_ENTERPRISE_REPOSITORIES: list[str] = ["odoo/enterprise"] +ODOO_UPGRADE_REPOSITORY: str = "odoo/upgrade" + + ODOO_PYTHON_VERSIONS: Mapping[int, str] = { 19: "3.12", 16: "3.10", @@ -531,7 +534,7 @@ def deploy( except CalledProcessError as error: error_message: str = error.stderr.strip().decode().rstrip(".").replace("ERROR: ", "") logger.error(f"Odoo exited with an error: {error_message}") - return None + return CompletedProcess(error.cmd, error.returncode, error.stdout, error.stderr) else: return process @@ -600,7 +603,7 @@ def run( # noqa: PLR0913 logger.error(f"STDERR: {error.stderr.decode()}") logger.error("Odoo exited with an error, check the output above for more information") - return None + return CompletedProcess(error.cmd, error.returncode, error.stdout, error.stderr) else: return process diff --git a/odev/common/progress.py b/odev/common/progress.py index 6bcab3a1..899ea7f0 100644 --- a/odev/common/progress.py +++ b/odev/common/progress.py @@ -176,7 +176,7 @@ def spinner(message: str) -> StackedStatus: :param message: The message to display. :type message: str """ - if DEBUG_MODE: + if DEBUG_MODE or not console.is_interactive: logger.info(message) status = StackedStatus(console.render_str(message), console=console, spinner="arc") diff --git a/odev/common/python.py b/odev/common/python.py index ae3024ee..5ca3b42e 100644 --- a/odev/common/python.py +++ b/odev/common/python.py @@ -551,7 +551,7 @@ def run_script( command = f"{self.python} {script_path} {' '.join(args)}" if script_input is not None: - command = f"{script_input} | {command}" + command = f"printf '%s' {shlex.quote(script_input)} | {command}" if not stream: return bash.execute(command) @@ -559,10 +559,17 @@ def run_script( if progress is None: return bash.run(command) - for line in bash.stream(command): - progress(line) - - return CompletedProcess(command, 0) + output = [] + returncode = 0 + try: + for line in bash.stream(command): + output.append(line) + progress(line) + except CalledProcessError as error: + returncode = error.returncode + + # Note: bash.stream mixes stdout and stderr via PTY, so we return the combined output as stdout. + return CompletedProcess(command, returncode, stdout="\n".join(output).encode()) def run(self, command: str) -> CompletedProcess | None: """Run a python command. diff --git a/odev/common/store/tables/secrets.py b/odev/common/store/tables/secrets.py index e794adc4..614aa5d1 100644 --- a/odev/common/store/tables/secrets.py +++ b/odev/common/store/tables/secrets.py @@ -70,11 +70,7 @@ class SecretStore(PostgresTable): @classmethod def _list_ssh_keys(cls) -> list[AgentKey]: """List all SSH keys available in the ssh-agent.""" - try: - keys = list(SSHAgent().get_keys()) - except (SSHException, ConnectionError) as e: - logger.warning(f"Failed to communicate with ssh-agent: {e}") - keys = [] + keys = list(SSHAgent().get_keys()) if not keys and not os.environ.get("ODEV_NO_SSH_AGENT"): raise OdevError("No SSH keys found in ssh-agent, or ssh-agent is not running.") @@ -98,8 +94,12 @@ def encrypt(cls, plaintext: str) -> str: :rtype: str """ ciphered: str | None = None + keys = cls._list_ssh_keys() + + if not keys: + return plaintext - for key in cls._list_ssh_keys(): + for key in keys: try: ciphered = str(b64encode(ssh_encrypt(plaintext, ssh_key=key)).decode()) if plaintext else "" except SSHException as e: @@ -122,8 +122,12 @@ def decrypt(cls, ciphertext: str) -> str: :rtype: str """ deciphered: str | None = None + keys = cls._list_ssh_keys() + + if not keys: + return ciphertext - for key in cls._list_ssh_keys(): + for key in keys: key_desc = f"{key.fingerprint} ({key.name}, {key.comment})" try: diff --git a/tests/fixtures/capture.py b/tests/fixtures/capture.py index fd103633..37297bd2 100644 --- a/tests/fixtures/capture.py +++ b/tests/fixtures/capture.py @@ -74,5 +74,5 @@ def stderr(self): if self._stderr and not self._stderr.closed: self._stderr_value = self._stderr.getvalue() - self._stderr_value = re.sub(r"\x1b[^m]*m", "", self._stdout_value) + self._stderr_value = re.sub(r"\x1b[^m]*m", "", self._stderr_value) return self._stderr_value diff --git a/tests/tests/commands/test_database.py b/tests/tests/commands/test_database.py index 75fa4ca2..caa13c75 100644 --- a/tests/tests/commands/test_database.py +++ b/tests/tests/commands/test_database.py @@ -403,6 +403,40 @@ def test_13_run_with_version(self): self.assertDatabaseIsOdoo(self.database_name) self.assertDatabaseVersionEqual(self.database_name, ODOO_DB_VERSION) + def test_14_run_empty_db_warning(self): + """Command `odev run` should warn the user when running on an empty database without a version.""" + empty_db = f"{self.database_name}-empty" + LocalDatabase(empty_db).create() + try: + with ( + self.patch("odev.common.bash", "stream", return_value=iter([])), + self.patch("odev.common.bash", "run", return_value=None), + ): + # odoo-bin will fail on an empty DB, but we've mocked bash to avoid the error + stdout, stderr = self.dispatch_command("run", empty_db, "--stop-after-init") + + # Logging goes to stdout in these tests + combined_output = stdout + stderr + self.assertIn(f"Database {empty_db!r} is not an Odoo database. Defaulting to 'master'.", combined_output) + self.assertIn( + f"Consider using 'odev create -V {empty_db}' to initialize it properly.", combined_output + ) + finally: + LocalDatabase(empty_db).drop() + + def test_15_test_non_existent_db(self): + """Command `odev test` should work even if the target database does not exist, provided a version is given.""" + non_existent_db = f"{self.database_name}-non-existent" + self.assertDatabaseNotExist(non_existent_db) + + # Mock run_command to avoid actually running 'create' or 'test' + with ( + self.patch("odev.common.bash", "stream", return_value=iter([])), + self.patch("odev.common.odev.Odev", "run_command"), + ): + # This should not raise SystemExit or any exception + self.dispatch_command("test", "-V", ODOO_DB_VERSION, "--tags", ":base", non_existent_db) + # -------------------------------------------------------------------------- # Test cases - delete # Keep at the end to avoid interference with other tests and to cleanup diff --git a/tests/tests/common/test_odev.py b/tests/tests/common/test_odev.py index 09e68ab8..8b42d6b8 100644 --- a/tests/tests/common/test_odev.py +++ b/tests/tests/common/test_odev.py @@ -132,3 +132,14 @@ def test_12_dispatch_command_error(self): self.odev.dispatch() mock_error.assert_called_once_with("Cannot display help for inexistent command 'invalid-command'") + + def test_13_dispatch_version(self): + """Odev should display its version when called with 'version' command.""" + sys.argv = ["odev", "version"] + + with CaptureOutput() as output: + self.odev.dispatch() + + # VersionCommand output includes name, version, and release channel info + self.assertIn(self.odev.version, output.stdout) + self.assertIn(self.odev.name.capitalize(), output.stdout) diff --git a/tests/tests/common/test_python_env.py b/tests/tests/common/test_python_env.py new file mode 100644 index 00000000..03062cdf --- /dev/null +++ b/tests/tests/common/test_python_env.py @@ -0,0 +1,57 @@ +from pathlib import Path +from subprocess import CalledProcessError, CompletedProcess +from unittest.mock import MagicMock, patch + +from odev.common import bash +from odev.common.python import PythonEnv + +from tests.fixtures import OdevTestCase + + +class TestPythonEnv(OdevTestCase): + """Test the PythonEnv class.""" + + @classmethod + def setUpClass(cls): + with patch("odev.common.odev.Odev.start", return_value=None): + super().setUpClass() + + def test_run_script_streaming_success(self): + """Test run_script with a streaming process that succeeds.""" + env = PythonEnv(path="/tmp/fake_venv", version="3.10") # noqa: S108 + + with ( + self.patch(Path, "exists", return_value=True), + self.patch(PythonEnv, "exists", return_value=True), + self.patch(PythonEnv, "python", Path("/tmp/fake_venv/bin/python")), # noqa: S108 + self.patch(bash, "stream", return_value=iter(["line 1", "line 2"])), + ): + progress_mock = MagicMock() + result = env.run_script("fake_script.py", stream=True, progress=progress_mock) + + self.assertIsInstance(result, CompletedProcess) + self.assertEqual(result.returncode, 0) + self.assertEqual(progress_mock.call_count, 2) + progress_mock.assert_any_call("line 1") + progress_mock.assert_any_call("line 2") + + def test_run_script_streaming_failure(self): + """Test run_script with a streaming process that fails.""" + env = PythonEnv(path="/tmp/fake_venv", version="3.10") # noqa: S108 + + def streaming_failure(command): + yield "line 1" + raise CalledProcessError(1, command) + + with ( + self.patch(Path, "exists", return_value=True), + self.patch(PythonEnv, "exists", return_value=True), + self.patch(PythonEnv, "python", Path("/tmp/fake_venv/bin/python")), # noqa: S108 + self.patch(bash, "stream", side_effect=streaming_failure), + ): + progress_mock = MagicMock() + result = env.run_script("fake_script.py", stream=True, progress=progress_mock) + + self.assertIsInstance(result, CompletedProcess) + self.assertEqual(result.returncode, 1) + progress_mock.assert_called_once_with("line 1")