west: commands: build: Specify source dir without a flag

In order to simplify the usage of `west build`, take a positional
argument with the source directory instead of requiring the `-s,
--source-dir` flag. This makes it easier and quicker to invoke west when
building, as well as being consistent with CMake.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>

doc/extensions/zephyr/application.py

@@ -234,10 +234,10 @@ class ZephyrAppCommandsDirective(Directive):
         cmake_args = ' --{}'.format(cmake_args) if cmake_args != '' else ''
         # ignore zephyr_app since west needs to run within
         # the installation. Instead rely on relative path.
-        src = ' -s {}'.format(cd_to) if cd_to else ''
+        src = ' {}'.format(cd_to) if cd_to else ''
         dst = ' -d {}'.format(build_dir) if build_dir != 'build' else ''
 
-        goal_args = ' -b {}{}{}{}'.format(board, src, dst, cmake_args)
+        goal_args = ' -b {}{}{}{}'.format(board, dst, src, cmake_args)
         if 'build' in goals:
             content.append('west build{}'.format(goal_args))
             # No longer need to specify additional args, they are in the

doc/guides/west/build-flash-debug.rst

@@ -61,9 +61,9 @@ no additional parameters.
   whether ``-b`` is required, just try leaving it out. West will print an
   error if the option is required and was not given.
 
-To specify the source directory, use ``--source-dir`` (or ``-s``)::
+Specify the source directory path as the first positional argument::
 
-  west build -b <BOARD> --source-dir path/to/source/directory
+  west build -b <BOARD> path/to/source/directory
 
 Additionally you can specify the build system target using the ``--target``
 (or ``-t``) option. For example, to run the ``clean`` target::
@@ -79,10 +79,11 @@ Finally, you can add additional arguments to the CMake invocation performed by
 command. For example, to use the Unix Makefiles CMake generator instead of
 Ninja (which ``west build`` uses by default), run::
 
-  west build -- -G'Unix Makefiles'
+  west build -b reel_board samples/hello_world -- -G'Unix Makefiles'
 
-As another example, the following command adds the ``file.conf`` Kconfig
-fragment to the files which are merged into your final build configuration::
+As another example, and assuming you have already built a sample, the following
+command adds the ``file.conf`` Kconfig fragment to the files which are merged
+into your final build configuration::
 
   west build -- -DOVERLAY_CONFIG=file.conf
 

scripts/west_commands/build.py

@@ -11,6 +11,7 @@ from west.build import DEFAULT_CMAKE_GENERATOR, is_zephyr_build
 
 from zephyr_ext_common import find_build_dir, Forceable, BUILD_DIR_DESCRIPTION
 
+_ARG_SEPARATOR = '--'
 
 BUILD_DESCRIPTION = '''\
 Convenience wrapper for building Zephyr applications.
@@ -41,7 +42,15 @@ after a '--'. For example, this sets an overlay config file:
 
 west build [...] -- -DOVERLAY_CONFIG=some.conf
 
-(Doing this forces a CMake run.)'''
+(Doing this forces a CMake run.)
+
+positional arguments:
+  source_dir            Explicitly set the source directory. If not given and
+                        rebuilding an existing Zephyr build directory, this is
+                        taken from the CMake cache. Otherwise, the current
+                        directory is assumed.
+  cmake_opt             Extra options to pass to CMake; implies -c
+'''
 
 
 class Build(Forceable):
@@ -52,7 +61,7 @@ class Build(Forceable):
             # Keep this in sync with the string in west-commands.yml.
             'compile a Zephyr application',
             BUILD_DESCRIPTION,
-            accepts_unknown_args=False)
+            accepts_unknown_args=True)
 
         self.source_dir = None
         '''Source directory for the build, or None on error.'''
@@ -78,7 +87,10 @@ class Build(Forceable):
             self.name,
             help=self.help,
             formatter_class=argparse.RawDescriptionHelpFormatter,
-            description=self.description)
+            description=self.description,
+            usage='''west build [-h] [-b BOARD] [-d BUILD_DIR]
+                  [-t TARGET] [-c] [-f] [source_dir]
+                  -- [cmake_opt [cmake_opt ...]]''')
 
         # Remember to update scripts/west-completion.bash if you add or remove
         # flags
@@ -86,12 +98,8 @@ class Build(Forceable):
         parser.add_argument('-b', '--board',
                             help='''Board to build for (must be given for the
                             first build, can be omitted later)''')
-        parser.add_argument('-s', '--source-dir',
-                            help='''Explicitly set the source directory.
-                            If not given and rebuilding an existing Zephyr
-                            build directory, this is taken from the CMake
-                            cache. Otherwise, the current directory is
-                            assumed.''')
+        # Hidden option for backwards compatibility
+        parser.add_argument('-s', '--source-dir', help=argparse.SUPPRESS)
         parser.add_argument('-d', '--build-dir',
                             help=BUILD_DIR_DESCRIPTION +
                             "The directory is created if it doesn't exist.")
@@ -101,14 +109,22 @@ class Build(Forceable):
         parser.add_argument('-c', '--cmake', action='store_true',
                             help='Force CMake to run')
         self.add_force_arg(parser)
-        parser.add_argument('cmake_opts', nargs='*', metavar='cmake_opt',
-                            help='Extra option to pass to CMake; implies -c')
-
         return parser
 
-    def do_run(self, args, ignored):
+    def do_run(self, args, remainder):
         self.args = args        # Avoid having to pass them around
-        log.dbg('args:', args, level=log.VERBOSE_EXTREME)
+        log.dbg('args: {} remainder: {}'.format(args, remainder,
+                                                level=log.VERBOSE_EXTREME))
+        # Store legacy -s option locally
+        source_dir = self.args.source_dir
+        self._parse_remainder(remainder)
+        if source_dir:
+            if self.args.source_dir:
+                log.die("source directory specified twice:({} and {})".format(
+                                            source_dir, self.args.source_dir))
+            self.args.source_dir = source_dir
+        log.dbg('source_dir: {} cmake_opts: {}'.format(self.args.source_dir,
+                                                       self.args.cmake_opts))
         self._sanity_precheck()
         self._setup_build_dir()
         if is_zephyr_build(self.build_dir):
@@ -141,6 +157,23 @@ class Build(Forceable):
         extra_args = ['--target', args.target] if args.target else []
         cmake.run_build(self.build_dir, extra_args=extra_args)
 
+    def _parse_remainder(self, remainder):
+        self.args.source_dir = None
+        self.args.cmake_opts = None
+        try:
+            # Only one source_dir is allowed, as the first positional arg
+            if remainder[0] != _ARG_SEPARATOR:
+                self.args.source_dir = remainder[0]
+                remainder = remainder[1:]
+            # Only the first argument separator is consumed, the rest are
+            # passed on to CMake
+            if remainder[0] == _ARG_SEPARATOR:
+                remainder = remainder[1:]
+            if len(remainder):
+                self.args.cmake_opts = remainder
+        except IndexError:
+            return
+
     def _sanity_precheck(self):
         app = self.args.source_dir
         if app:

scripts/west_commands/tests/test_build.py

@@ -0,0 +1,59 @@
+# Copyright (c) 2018 Nordic Semiconductor ASA
+#
+# SPDX-License-Identifier: Apache-2.0
+
+from argparse import Namespace
+from unittest.mock import patch
+
+from build import Build
+import pytest
+
+TEST_CASES = [
+    {'r': [],
+     's': None, 'c': None},
+    {'r': ['source_dir'],
+     's': 'source_dir', 'c': None},
+    {'r': ['source_dir', '--'],
+     's': 'source_dir', 'c': None},
+    {'r': ['source_dir', '--', 'cmake_opt'],
+     's': 'source_dir', 'c': ['cmake_opt']},
+    {'r': ['source_dir', '--', 'cmake_opt', 'cmake_opt2'],
+     's': 'source_dir', 'c': ['cmake_opt', 'cmake_opt2']},
+    {'r': ['thing_one', 'thing_two'],
+     's': 'thing_one', 'c': ['thing_two']},
+    {'r': ['thing_one', 'thing_two', 'thing_three'],
+     's': 'thing_one', 'c': ['thing_two', 'thing_three']},
+    {'r': ['--'],
+     's': None, 'c': None},
+    {'r': ['--', '--'],
+     's': None, 'c': ['--']},
+    {'r': ['--', 'cmake_opt'],
+     's': None, 'c': ['cmake_opt']},
+    {'r': ['--', 'cmake_opt', 'cmake_opt2'],
+     's': None, 'c': ['cmake_opt', 'cmake_opt2']},
+    {'r': ['--', 'cmake_opt', 'cmake_opt2', '--'],
+     's': None, 'c': ['cmake_opt', 'cmake_opt2', '--']},
+    {'r': ['--', 'cmake_opt', 'cmake_opt2', '--', 'tool_opt'],
+     's': None, 'c': ['cmake_opt', 'cmake_opt2', '--', 'tool_opt']},
+    {'r': ['--', 'cmake_opt', 'cmake_opt2', '--', 'tool_opt', 'tool_opt2'],
+     's': None, 'c': ['cmake_opt', 'cmake_opt2', '--', 'tool_opt',
+                      'tool_opt2']},
+    {'r': ['--', 'cmake_opt', 'cmake_opt2', '--', 'tool_opt', 'tool_opt2',
+           '--'],
+     's': None, 'c': ['cmake_opt', 'cmake_opt2', '--', 'tool_opt', 'tool_opt2',
+                      '--']},
+    ]
+
+ARGS = Namespace(board=None, build_dir=None, cmake=False, command='build',
+                 force=False, help=None, target=None, verbose=3, version=False,
+                 zephyr_base=None)
+
+@pytest.mark.parametrize('test_case', TEST_CASES)
+def test_parse_remainder(test_case):
+    b = Build()
+
+    b.args = Namespace()
+    b._parse_remainder(test_case['r'])
+    assert b.args.source_dir == test_case['s']
+    assert b.args.cmake_opts == test_case['c']
+