scripts: west_commands: fix default build directory fallback

fixed fallback to default build directory (build) in case that no other
build directory could be determined.

Signed-off-by: Thorsten Klein <[email protected]>

Author: thorsten-klein

scripts/west_commands/build_helpers.py

@@ -84,35 +84,33 @@ def _resolve_build_dir(fmt, guess, cwd, **kwargs):
 
 def find_build_dir(dir, guess=False, **kwargs):
     '''Heuristic for finding a build directory.
-
-    The default build directory is computed by reading the build.dir-fmt
-    configuration option, defaulting to DEFAULT_BUILD_DIR if not set. It might
-    be None if the build.dir-fmt configuration option is set but cannot be
-    resolved.
-    If the given argument is truthy, it is returned. Otherwise, if
-    the default build folder is a build directory, it is returned.
-    Next, if the current working directory is a build directory, it is
-    returned. Finally, the default build directory is returned (may be None).
+    If `dir` is specified, this directory is returned as the build directory.
+    Otherwise, the default build directory is determined according to the
+    following priorities:
+    1. Resolved `build.dir-fmt` configuration option (all {args} are resolvable).
+       Return this directory, if it is an already existing build directory.
+    2. The current working directory, if it is an existing build directory.
+    3. Resolved `build.dir-fmt` configuration option, no matter if it is an
+       already existing build directory.
+    4. DEFAULT_BUILD_DIR
     '''
 
-    if dir:
-        build_dir = dir
-    else:
-        cwd = os.getcwd()
-        default = config.get('build', 'dir-fmt', fallback=DEFAULT_BUILD_DIR)
-        default = _resolve_build_dir(default, guess, cwd, **kwargs)
-        log.dbg(f'config dir-fmt: {default}', level=log.VERBOSE_EXTREME)
-        if default and is_zephyr_build(default):
-            build_dir = default
-        elif is_zephyr_build(cwd):
-            build_dir = cwd
-        else:
-            build_dir = default
+    build_dir = dir
+    cwd = os.getcwd()
+    dir_fmt = config.get('build', 'dir-fmt', fallback=None)
+    if dir_fmt:
+        log.dbg(f'config dir-fmt: {dir_fmt}', level=log.VERBOSE_EXTREME)
+        dir_fmt = _resolve_build_dir(dir_fmt, guess, cwd, **kwargs)
+    if not build_dir and is_zephyr_build(dir_fmt):
+        build_dir = dir_fmt
+    if not build_dir and is_zephyr_build(cwd):
+        build_dir = cwd
+    if not build_dir and dir_fmt:
+        build_dir = dir_fmt
+    if not build_dir:
+        build_dir = DEFAULT_BUILD_DIR
     log.dbg(f'build dir: {build_dir}', level=log.VERBOSE_EXTREME)
-    if build_dir:
-        return os.path.abspath(build_dir)
-    else:
-        return None
+    return os.path.abspath(build_dir)
 
 def is_zephyr_build(path):
     '''Return true if and only if `path` appears to be a valid Zephyr
@@ -128,6 +126,9 @@ def is_zephyr_build(path):
     The cached ZEPHYR_BASE was added in
     https://github.com/zephyrproject-rtos/zephyr/pull/23054.)
     '''
+    if not path:
+        return False
+
     try:
         cache = zcmake.CMakeCache.from_build_dir(path)
     except FileNotFoundError:

scripts/west_commands/tests/test_build.py

@@ -5,6 +5,10 @@
 from argparse import Namespace
 
 from build import Build
+from build_helpers import DEFAULT_BUILD_DIR
+from pathlib import Path
+import configparser
+import os
 import pytest
 
 TEST_CASES = [
@@ -55,3 +59,151 @@ def test_parse_remainder(test_case):
     b._parse_remainder(test_case['r'])
     assert b.args.source_dir == test_case['s']
     assert b.args.cmake_opts == test_case['c']
+
+
+cwd = Path(os.getcwd())
+
+DEFAULT_ARGS = Namespace(
+    build_dir=None,
+    board='native_sim',
+    source_dir=os.path.join("any", "project", "app")
+)
+
+BUILD_DIR_FMT_VALID = [
+    # dir_fmt, expected
+    ('resolvable-{board}', cwd / f'resolvable-{DEFAULT_ARGS.board}'),
+    ('my-dir-fmt-build-folder', cwd / 'my-dir-fmt-build-folder'),
+]
+
+BUILD_DIR_FMT_INVALID = [
+    'unresolvable-because-of-unknown-{board}',
+    '{invalid}'
+]
+
+
+def mock_dir_fmt_config(monkeypatch, dir_fmt):
+    config = configparser.ConfigParser()
+    config.add_section("build")
+    config.set("build", "dir-fmt", dir_fmt)
+    monkeypatch.setattr("build_helpers.config", config)
+    monkeypatch.setattr("build.config", config)
+
+def mock_is_zephyr_build(monkeypatch, func):
+    monkeypatch.setattr("build_helpers.is_zephyr_build", func)
+
+
+@pytest.fixture
+def build_instance(monkeypatch):
+    """Create a Build instance with default args and patched helpers."""
+    b = Build()
+    b.args = DEFAULT_ARGS
+    b.config_board = None
+
+    # mock configparser read method to bypass configs during test
+    monkeypatch.setattr(configparser.ConfigParser, "read", lambda self, filenames: None)
+    # mock os.makedirs to avoid filesystem operations during test
+    monkeypatch.setattr("os.makedirs", lambda *a, **kw: None)
+    # mock is_zephyr_build to always return False
+    monkeypatch.setattr("build_helpers.is_zephyr_build", lambda path: False)
+    # mock os.environ to make tests independent from environment variables
+    monkeypatch.setattr("os.environ", {})
+
+    return b
+
+def test_build_dir_fallback(monkeypatch, build_instance):
+    """Test: Fallback to DEFAULT_BUILD_DIR if no build dirs exist"""
+    b = build_instance
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd / DEFAULT_BUILD_DIR
+
+
+@pytest.mark.parametrize('test_case', BUILD_DIR_FMT_VALID)
+def test_build_dir_fmt_is_used(monkeypatch, build_instance, test_case):
+    """Test: build.dir-fmt is used if it is resolvable (no matter if it exists)"""
+    dir_fmt, expected = test_case
+    b = build_instance
+
+    # mock the config
+    mock_dir_fmt_config(monkeypatch, dir_fmt)
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == expected
+
+
+@pytest.mark.parametrize('dir_fmt', BUILD_DIR_FMT_INVALID)
+def test_build_dir_fmt_is_not_resolvable(monkeypatch, build_instance, dir_fmt):
+    """Test: Fallback to DEFAULT_BUILD_DIR if build.dir-fmt is not resolvable."""
+    b = build_instance
+    b.args.board = None
+
+    # mock the config
+    mock_dir_fmt_config(monkeypatch, dir_fmt)
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd / DEFAULT_BUILD_DIR
+
+
+def test_dir_fmt_preferred_over_others(monkeypatch, build_instance):
+    """Test: build.dir-fmt is preferred over cwd and DEFAULT_BUILD_DIR (if all exist)"""
+    b = build_instance
+    dir_fmt = "my-dir-fmt-build-folder"
+
+    # mock the config and is_zephyr_build
+    mock_dir_fmt_config(monkeypatch, dir_fmt)
+    mock_is_zephyr_build(monkeypatch,
+        lambda path: path in [dir_fmt, cwd, DEFAULT_BUILD_DIR])
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd / dir_fmt
+
+def test_non_existent_dir_fmt_preferred_over_others(monkeypatch, build_instance):
+    """Test: build.dir-fmt is preferred over cwd and DEFAULT_BUILD_DIR (if it not exists)"""
+    b = build_instance
+    dir_fmt = "my-dir-fmt-build-folder"
+
+    # mock the config and is_zephyr_build
+    mock_dir_fmt_config(monkeypatch, dir_fmt)
+    mock_is_zephyr_build(monkeypatch,
+        lambda path: path in [cwd, DEFAULT_BUILD_DIR])
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd / dir_fmt
+
+
+def test_cwd_preferred_over_default_build_dir(monkeypatch, build_instance):
+    """Test: cwd is preferred over DEFAULT_BUILD_DIR (if both exist)"""
+    b = build_instance
+
+    # mock is_zephyr_build
+    mock_is_zephyr_build(monkeypatch,
+        lambda path: path in [str(cwd), DEFAULT_BUILD_DIR])
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd
+
+@pytest.mark.parametrize('dir_fmt', BUILD_DIR_FMT_INVALID)
+def test_cwd_preferred_over_non_resolvable_dir_fmt(monkeypatch, build_instance, dir_fmt):
+    """Test: cwd is preferred over dir-fmt, if dir-fmt is not resolvable"""
+    b = build_instance
+
+    # mock the config and is_zephyr_build
+    mock_dir_fmt_config(monkeypatch, dir_fmt)
+    mock_is_zephyr_build(monkeypatch,
+        lambda path: path in [str(cwd)])
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd
+
+@pytest.mark.parametrize('test_case', BUILD_DIR_FMT_VALID)
+def test_cwd_preferred_over_non_existent_dir_fmt(monkeypatch, build_instance, test_case):
+    """Test: cwd is preferred over build.dir-fmt if cwd is a build directory and dir_fmt not"""
+    dir_fmt, _ = test_case
+    b = build_instance
+
+    # mock the config and is_zephyr_build
+    mock_dir_fmt_config(monkeypatch, dir_fmt)
+    mock_is_zephyr_build(monkeypatch,
+        lambda path: path in [str(cwd)])
+
+    b._setup_build_dir()
+    assert Path(b.build_dir) == cwd