Merge pull request #63 from IRNAS/release/v0.10.1

Release v0.10.1
IRNAS · May 29, 2023 · 06870f3 · 06870f3
2 parents 5a3910b + ab1cea3
commit 06870f3
Show file tree

Hide file tree

Showing 7 changed files with 152 additions and 152 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
 
+## [0.10.1] - 2023-05-29
+
+### Changed
+
+-   Refactor preworkspace check to improve clarity.
+-   Unify helper functions in build type and release tests.
+
+### Fixed
+
+-   A bug that caused a crash when east was run from non-NCS projects.
+-   The wrong selection of release binaries when building application with only
+    TFM or SPM (#62)
+
 ## [0.10.0] - 2023-03-28
 
 ### Added
@@ -24,11 +37,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Changed
 
 -   Add back functionality to provide `cmake_args` after `--` marker for `build`
-    command. Now it is possible to provide custom `-D` define values to the
-    CMake. Commands `flash`, `bypass` already provided option for extra
-    arguments after `--`, however they did it incorrectly for very edge cases,
-    as they removed double quotes from all arguments passed after `--` marker
-    (#56).
+    command. Now it is possible to provide custom `-D` define values to the CMake.
+    Commands `flash`, `bypass` already provided option for extra arguments after
+    `--`, however they did it incorrectly for very edge cases, as they removed
+    double quotes from all arguments passed after `--` marker (#56).
 
 ### Fixed
 
@@ -38,55 +50,55 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### Added
 
--   `compile_commands.json` is now also copied to the top west directory. This 
+-   `compile_commands.json` is now also copied to the top west directory. This
     enables clangd to work as intended in `ncs` and `zephyr` folders (#53).
 -   Section in docs/configuration.md document describing `release` build type.
 
 ### Changed
 
 -   Samples (which inherently do not have a build type) have their build type
-    marked with forward slash "/" instead of "None" in the Job table that 
-    appears when running `east release` (#52).
+    marked with forward slash "/" instead of "None" in the Job table that appears
+    when running `east release` (#52).
 
 ### Fixed
 
--   Sample binaries had incorrect `-None` build type qualifier in their release 
-    name, when they shouldn't have. Incorrect build type qualifier was removed 
+-   Sample binaries had incorrect `-None` build type qualifier in their release
+    name, when they shouldn't have. Incorrect build type qualifier was removed
     (#52).
 -   Samples can now inherit from `release` build types (#47).
--   `east release` command now correctly copies build artefacts when
-    `merged.hex` is not generated. Additionally, new _Copied build artefacts_
-    section in `docs/configuration.md` now exactly defines which build artefacts
-    are copied and renamed in release procedure (#51).
+-   `east release` command now correctly copies build artefacts when `merged.hex`
+    is not generated. Additionally, new _Copied build artefacts_ section in
+    `docs/configuration.md` now exactly defines which build artefacts are copied
+    and renamed in release procedure (#51).
 
 ## [0.7.0] - 2023-02-15
 
 ### Added
 
--   New documentation files in `docs` folder: `development_guide.md`, 
+-   New documentation files in `docs` folder: `development_guide.md`,
     `getting_started.md`, `how_east_works.md`
--   `make format` command, which uses `black` and `isort`. 
-    `development_guide.md` explains the use.
--   Both commands `east util connect` and `east util rtt` now accept 
-    `--rtt-port` option, which sets the RTT Telnet port. Command 
-    `east util connect` now also accepts the `--jlink-id` option, same as 
-    `east flash`. With those new options is now easier to connect and listen to 
-    RTT messages from multiple JLink devices.
+-   `make format` command, which uses `black` and `isort`. `development_guide.md`
+    explains the use.
+-   Both commands `east util connect` and `east util rtt` now accept `--rtt-port`
+    option, which sets the RTT Telnet port. Command `east util connect` now also
+    accepts the `--jlink-id` option, same as `east flash`. With those new options
+    is now easier to connect and listen to RTT messages from multiple JLink
+    devices.
 
 ### Changed
 
 -   Updated readme so it points to the new documentation.
 
 ### Fixed
 
--   Fixed release artefacts naming issue where build type would not appear 
+-   Fixed release artefacts naming issue where build type would not appear
     correctly.
--   Create a `release_dry_run` folder instead of release folder when using 
+-   Create a `release_dry_run` folder instead of release folder when using
     \--dry-run option with _east release_ command.
--   `east release` command now runs a pre-check on the apps and samples from 
-    `east.yml`, if they exists before running the release process. That way you 
-    can catch a typo, or a mistake before you spent some time waiting through 
-    the release process.
+-   `east release` command now runs a pre-check on the apps and samples from
+    `east.yml`, if they exists before running the release process. That way you
+    can catch a typo, or a mistake before you spent some time waiting through the
+    release process.
 
 ## [0.6.2] - 2022-12-16
 
@@ -195,7 +207,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 -   Docker scripts for building and running docker containers, for development
     purposes.
 
-[Unreleased]: https://github.com/IRNAS/irnas-east-software/compare/v0.10.0...HEAD
+[Unreleased]: https://github.com/IRNAS/irnas-east-software/compare/v0.10.1...HEAD
+
+[0.10.1]: https://github.com/IRNAS/irnas-east-software/compare/v0.10.0...v0.10.1
 
 [0.10.0]: https://github.com/IRNAS/irnas-east-software/compare/v0.9.0...v0.10.0
 

diff --git a/src/east/east_context.py b/src/east/east_context.py
@@ -69,8 +69,7 @@ def __init__(self, echo: bool = False):
         os.makedirs(self.consts["east_dir"], exist_ok=True)
 
         self.console = Console(width=80, markup=RICH_CONSOLE_ENABLE_MARKUP)
-        self.ncs_version_installed = False
-        self.ncs_version_supported = False
+        self.use_toolchain_manager = False
         self.east_yml = None
 
         try:
@@ -256,7 +255,7 @@ def run_west(self, west_command: str, **kwargs) -> str:
 
         cmd = f"west {west_command}"
 
-        if self.ncs_version_installed:
+        if self.use_toolchain_manager:
             # Run west command as arbitrary command through manager
             return self._run_arbi_manager(cmd, **kwargs)
         else:
@@ -382,14 +381,20 @@ def pre_workspace_command_check(
         ignore_unsupported_ncs: bool = True,
     ):
         """
-        A list of checks that every workspace command should call before executing its
-        actual command.
+        This function contains a list of checks that every workspace (not system)
+        command should call before executing its actual command.
 
-        This command will also load the east.yml file if it is found.
+        In current implementation it does general things:
+        * Asserts that workspace command was run from the west workspace
+        * Tries to load east.yml
+        * Tries to determine the NCS version that is used in the project.
+        * Tries to find the version of the toolcahin in the nordic's toolchain manager.
 
+        This function esentially tries to answer the questions: should the underlying
+        west command be passed to the nordic's toolchain manager or directly to the
+        west.
 
         Args:
-            self.(self.ontext):             self.context for printing and exiting
             ignore_uninstalled_ncs (bool):  When true, self.does not exit if detected
                                             NCS version is not installed by the
                                             Toolchain Manager. Workspace commands such
@@ -404,7 +409,8 @@ def pre_workspace_command_check(
                                             True. Update command should set this to
                                             false.
         """
-        # Exit if we are not inside west workspace
+        # Workspace commands can only be run inside west workspace, so exit if that is
+        # not the case.
         if not self.west_dir_path:
             self.print(not_in_west_workspace_msg, highlight=False)
             self.exit()
@@ -422,34 +428,37 @@ def pre_workspace_command_check(
             self.print(no_toolchain_manager_msg, highlight=False)
             self.exit()
 
-        # Check if toolchain for detected ncs version is installed
+        # Check if ncs version was even detected, this can happen in the cases where
+        # normal zephyr repo is used
+        if self.detected_ncs_version is None:
+            self.use_toolchain_manager = False
+            return
+
+        # Early exit if toolchain for detected ncs version is installed
         result = self.run_manager("list", silent=True, return_output=True)
         if self.detected_ncs_version in result["output"]:
             # If it is installed then is also supported
-            self.ncs_version_installed = True
-            self.ncs_version_supported = True
+            self.use_toolchain_manager = True
             return
 
         # Check if toolchain for detected ncs version is supported
         result = self.run_manager("search", silent=True, return_output=True)
         if self.detected_ncs_version in result["output"]:
             # Supported but not installed, should we exit program or silently pass?
             if ignore_uninstalled_ncs:
-                self.ncs_version_supported = False
                 return
             else:
                 self.print(no_toolchain_msg(self), highlight=False)
                 self.exit()
 
-        # Not supported, should we exit program or silently pass?
+        # Not supported, should we silently pass or exit program with message?
         if ignore_unsupported_ncs:
             # Silently pass
-            self.ncs_version_supported = False
             return
-
-        # Exit program
-        # This is usually set, if we intend to install the toolchain later
-        self.print(
-            ncs_version_not_supported_msg(self, result["output"]), highlight=False
-        )
-        self.exit()
+        else:
+            # Exit program
+            # This is usually set, if we intend to install the toolchain later
+            self.print(
+                ncs_version_not_supported_msg(self, result["output"]), highlight=False
+            )
+            self.exit()
diff --git a/src/east/workspace_commands/release_commands.py b/src/east/workspace_commands/release_commands.py
@@ -46,16 +46,19 @@ def move_build_artefacts(art_name, art_dest, dry_run):
 
     build_dir = os.path.join("build", "zephyr")
 
-    # Determine, if we have basic, default build or we are using MCUBoot or TF-M
+    # Determine, if we have basic build with MCUBoot, some other build with child images, or default build
     if (
         os.path.isfile(os.path.join(build_dir, "app_update.bin"))
         or dry_run
         or RUNNING_TESTS
     ):
-        # MCUBoot or TF-M
+        # MCUBoot
         binaries = ["dfu_application.zip", "app_update.bin", "merged.hex", "zephyr.elf"]
+    elif os.path.isfile(os.path.join(build_dir, "merged.hex")):
+        # Other (TFM, SPM, ...)
+        binaries = ["merged.hex", "zephyr.elf"]
     else:
-        # Basic build
+        # Basic build (No merged.hex is generated)
         binaries = ["zephyr.bin", "zephyr.hex", "zephyr.elf"]
 
     for binary in binaries:

diff --git a/tests/helpers.py b/tests/helpers.py
@@ -2,6 +2,7 @@
 
 from click.testing import CliRunner
 
+import east
 from east.__main__ import cli
 
 west_config_content = """
@@ -279,6 +280,74 @@ def clear_rich(string):
     assert clear_rich(string1) == clear_rich(string2)
 
 
+def helper_test_against_west_run(
+    monkeypatch,
+    mocker,
+    path,
+    east_cmd,
+    expected_west_cmd=None,
+    expected_west_cmds=None,
+    should_succed=True,
+):
+    """
+    Helper function for making tests easier to read.
+
+    Args:
+        monkeypatch ():         fixture
+        mocker ():              fixture
+        path ():                To which path should we change
+        east_cmd ():            which east command should be called
+        expected_west_cmd ():   A single expected west cmd, a string.
+        expected_west_cmds ():  A List of expected west_cmds.
+        should_succed ():       If true then the command should succeded.
+
+    Only a expected_west_cmd or expected_west_cmds can be given, both not both.
+    If none is given then no run_west call should happend.
+
+    Returns:
+        Result object, which can be further checked.
+    """
+    runner = CliRunner()
+
+    # Mock output of git commmand, so tests do not have to depend on it
+    mocker.patch(
+        "east.workspace_commands.release_commands.get_git_version",
+        return_value={"tag": "v1.0.0.", "hash": ""},
+    )
+
+    monkeypatch.chdir(path)
+    mocker.patch(
+        "east.east_context.EastContext.run_west",
+        return_value={"output": "", "returncode": 0},
+    )
+
+    # Setting catch_exceptions to False enables us to see programming errors in East
+    # code
+    result = runner.invoke(cli, east_cmd.strip().split(" "), catch_exceptions=False)
+
+    run_west = east.east_context.EastContext.run_west
+
+    if expected_west_cmds:
+
+        # This conversion is needed due to assert_has_calls interface
+        calls = [
+            mocker.call(cmd, silent=True, return_output=True, exit_on_error=False)
+            for cmd in expected_west_cmds
+        ]
+        run_west.assert_has_calls(calls)
+
+    elif expected_west_cmd:
+        run_west.assert_called_once_with(expected_west_cmd)
+    else:
+
+        run_west.assert_not_called()
+
+    expected_return_code = 0 if should_succed else 1
+
+    assert result.exit_code == expected_return_code
+    return result
+
+
 def helper_test_against_west_run1(
     monkeypatch, mocker, path, east_cmd, expected_west_cmds=None, should_succed=True
 ):

diff --git a/tests/test_build_type.py b/tests/test_build_type.py
@@ -3,50 +3,10 @@
 import pytest
 from click.testing import CliRunner
 
-import east
 from east.__main__ import cli
 
 from . import helpers
-
-
-def helper_test_against_west_run(
-    monkeypatch, mocker, path, east_cmd, expected_west_cmd=None, should_succed=True
-):
-    """
-    Helper function for making tests easier to read.
-
-    Args:
-        monkeypatch ():         fixture
-        mocker ():              fixture
-        path ():                To which path should we change
-        east_cmd ():            which east command should be called
-        expected_west_cmd ():   what is expected west cmd that should be produced. If
-                                none then no run_west call should happend.
-        should_succed ():       If true then the command should succeded.
-
-    Returns:
-        Result object, which can be further checked.
-    """
-    runner = CliRunner()
-
-    monkeypatch.chdir(path)
-    mocker.patch("east.east_context.EastContext.run_west")
-
-    # Setting catch_exceptions to False enables us to see programming errors in East
-    # code
-    result = runner.invoke(cli, east_cmd.strip().split(" "), catch_exceptions=False)
-
-    run_west = east.east_context.EastContext.run_west
-
-    if expected_west_cmd:
-        run_west.assert_called_once_with(expected_west_cmd)
-    else:
-        run_west.assert_not_called()
-
-    expected_return_code = 0 if should_succed else 1
-
-    assert result.exit_code == expected_return_code
-    return result
+from .helpers import helper_test_against_west_run
 
 
 def test_build_type_with_no_east_yml(west_workplace_parametrized, monkeypatch):