support 'west init --topdir'

Added new argument for west init to specify the west topdir when
initializing a workspace from a local manifest.

Author: thorsten-klein

src/west/app/project.py

@@ -24,17 +24,18 @@
 from west import util
 from west.commands import CommandError, Verbosity, WestCommand
 from west.configuration import Configuration
-from west.manifest import MANIFEST_REV_BRANCH as MANIFEST_REV
-from west.manifest import QUAL_MANIFEST_REV_BRANCH as QUAL_MANIFEST_REV
-from west.manifest import QUAL_REFS_WEST as QUAL_REFS
 from west.manifest import (
+    _WEST_YML,
     ImportFlag,
     Manifest,
     ManifestImportFailed,
     ManifestProject,
     Submodule,
     _manifest_content_at,
 )
+from west.manifest import MANIFEST_REV_BRANCH as MANIFEST_REV
+from west.manifest import QUAL_MANIFEST_REV_BRANCH as QUAL_MANIFEST_REV
+from west.manifest import QUAL_REFS_WEST as QUAL_REFS
 from west.manifest import is_group as is_project_group
 
 #
@@ -155,19 +156,54 @@ def __init__(self):
             'init',
             'create a west workspace',
             f'''\
-Creates a west workspace.
-
-With -l, creates a workspace around an existing local repository;
-without -l, creates a workspace by cloning a manifest repository
-by URL.
-
-With -m, clones the repository at that URL and uses it as the
-manifest repository. If --mr is not given, the remote's default
-branch will be used, if it exists.
-
-With neither, -m {MANIFEST_URL_DEFAULT} is assumed.
-
-Warning: 'west init' renames and/or deletes temporary files inside the
+Initialize a west workspace.
+
+West supports two ways of creating a workspace:
+
+1. Remote Manifest Repositories
+-------------------------------
+If `-m / --manifest-url` is given, west clones the manifest repository from the
+specified URL into a new workspace. Inside that workspace, the local config
+option `manifest.path` will be set to point at this cloned directory (relative
+path from the git clone to the workspace).
+
+- If `-m / --manifest-url` is not provided, west will use Zephyr manifest
+repository by default:
+  {MANIFEST_URL_DEFAULT}.
+- With `--mr`, you can specify the revision (branch, tag, or sha) of the
+  manifest repository.
+- If `--mr / --manifest-revision` is omitted, the repository's default branch
+is used (if available).
+
+Note: The west workspace (topdir) will be the current working directory, if
+neither --topdir nor [directory] are provided.
+
+2. Local Manifest
+-----------------
+If `-l / --local` is given, west initializes a new workspace from an already
+existing local manifest. The directory containing the manifest can be provided
+in positional argument `[manifest_directory]` (defaults to current working dir)
+Inside that workspace, the local config option `manifest.path` will be set to
+point at the manifest directory (relative path from `manifest_directory` to the
+workspace)
+
+Note: The west workspace (topdir) will be the `manifest_directory`'s parent, if
+no other directory is provided in `--topdir`.
+
+Arguments
+---------
+--mf
+    The relative path to the manifest file within the manifest repository
+    (remote or local). Config option manifest.file will be set to this value.
+    Defaults to `{_WEST_YML}` if not provided.
+
+--topdir
+    Specifies the directory where west should create the workspace.
+    The `.west` folder will be created inside this directory.
+
+Known Issues
+------------
+'west init' renames and/or deletes temporary files inside the
 workspace being created. This fails on some filesystems when some
 development tool or any other program is trying to read/index these
 temporary files at the same time. For instance, it is required to stop
@@ -192,9 +228,10 @@ def do_add_parser(self, parser_adder):
         parser = self._parser(
             parser_adder,
             usage='''
-
-  %(prog)s [-m URL] [--mr REVISION] [--mf FILE] [-o=GIT_CLONE_OPTION] [directory]
-  %(prog)s -l [--mf FILE] directory
+  remote repository:
+  %(prog)s [-m URL] [--mr REVISION] [--mf FILE] [-o=GIT_CLONE_OPTION] [-t WORKSPACE_DIR | directory]
+  local manifest:
+  %(prog)s -l [-t WORKSPACE_DIR] [--mf FILE] [manifest_directory]
 ''',
         )
 
@@ -203,14 +240,16 @@ def do_add_parser(self, parser_adder):
         parser.add_argument(
             '-m',
             '--manifest-url',
-            help='''manifest repository URL to clone;
+            metavar='URL',
+            help='''remote manifest repository URL to clone;
                     cannot be combined with -l''',
         )
         parser.add_argument(
             '-o',
             '--clone-opt',
             action='append',
             default=[],
+            metavar='GIT_CLONE_OPTION',
             help='''additional option to pass to 'git clone'
                     (e.g. '-o=--depth=1'); may be given more than once;
                     cannot be combined with -l''',
@@ -219,21 +258,33 @@ def do_add_parser(self, parser_adder):
             '--mr',
             '--manifest-rev',
             dest='manifest_rev',
+            metavar='REVISION',
             help='''manifest repository branch or tag name
                     to check out first; cannot be combined with -l''',
         )
-        parser.add_argument(
-            '--mf', '--manifest-file', dest='manifest_file', help='manifest file name to use'
-        )
         parser.add_argument(
             '-l',
             '--local',
             action='store_true',
-            help='''use "directory" as an existing local
-                    manifest repository instead of cloning one from
-                    MANIFEST_URL; .west is created next to "directory"
-                    in this case, and manifest.path points at
-                    "directory"''',
+            help='''initialize from an already existing local
+                    manifest instead of cloning a remote manifest.''',
+        )
+        parser.add_argument(
+            '--mf',
+            '--manifest-file',
+            dest='manifest_file',
+            metavar='FILE',
+            help=f'''manifest file to use. It is the relative
+                     path of the manifest file within the repository
+                     (remote or local). Defaults to {_WEST_YML}.''',
+        )
+        parser.add_argument(
+            '-t',
+            '--topdir',
+            dest='topdir',
+            metavar='WORKSPACE_DIR',
+            help='''the directory of the west workspace, where
+                    .west will be created in.''',
         )
         parser.add_argument(
             '--rename-delay',
@@ -249,9 +300,10 @@ def do_add_parser(self, parser_adder):
             'directory',
             nargs='?',
             default=None,
-            help='''with -l, the path to the local manifest repository;
-            without it, the directory to create the workspace in (defaulting
-            to the current working directory in this case)''',
+            metavar='directory',
+            help='''with --local: the path to the local manifest repository
+                    which contains a west.yml;
+                    otherwise: the directory to create the workspace in''',
         )
 
         return parser
@@ -302,15 +354,19 @@ def local(self, args) -> Path:
         #
         # https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.parent
         manifest_dir = Path(args.directory or os.getcwd()).resolve()
-        manifest_filename = args.manifest_file or 'west.yml'
+        manifest_filename = args.manifest_file or _WEST_YML
         manifest_file = manifest_dir / manifest_filename
-        topdir = manifest_dir.parent
-        rel_manifest = manifest_dir.name
-        west_dir = topdir / WEST_DIR
-
         if not manifest_file.is_file():
             self.die(f'can\'t init: no {manifest_filename} found in {manifest_dir}')
 
+        topdir = Path(args.topdir or manifest_dir.parent).resolve()
+
+        if not manifest_file.is_relative_to(topdir):
+            self.die(f'{manifest_file} must be relative to west topdir')
+
+        rel_manifest = manifest_dir.relative_to(topdir)
+        west_dir = topdir / WEST_DIR
+
         self.banner('Initializing from existing manifest repository', rel_manifest)
         self.small_banner(f'Creating {west_dir} and local configuration file')
         self.create(west_dir)
@@ -322,8 +378,11 @@ def local(self, args) -> Path:
         return topdir
 
     def bootstrap(self, args) -> Path:
-        topdir = Path(abspath(args.directory or os.getcwd()))
-        self.banner('Initializing in', topdir)
+        if args.topdir and args.directory:
+            self.die('--topdir cannot be combined with positional argument [directory]')
+
+        topdir = Path(abspath(args.topdir or args.directory or os.getcwd()))
+        self.banner(f'Initializing in {topdir}')
 
         manifest_url = args.manifest_url or MANIFEST_URL_DEFAULT
         if args.manifest_rev:
@@ -378,7 +437,7 @@ def bootstrap(self, args) -> Path:
             raise
 
         # Verify the manifest file exists.
-        temp_manifest_filename = args.manifest_file or 'west.yml'
+        temp_manifest_filename = args.manifest_file or _WEST_YML
         temp_manifest = tempdir / temp_manifest_filename
         if not temp_manifest.is_file():
             self.die(

tests/test_project.py

@@ -1862,6 +1862,139 @@ def test_init_local_with_manifest_filename(repos_tmpdir):
     cmd('update')
 
 
+TEST_CASES_INIT_TOPDIR = [
+    # (local_dir, topdir, directory, manifest_file, expected_error)
+    ######################################
+    # REMOTE MANIFEST
+    ######################################
+    # init from remote repository (without any parameters)
+    (None, None, None, None, None),
+    # specify topdir in current directory
+    (None, Path('.'), None, None, None),
+    # specify topdir in a subfolder
+    (None, Path('subdir'), None, None, None),
+    # use deprecated [directory] to specify topdir
+    (None, None, Path('subdir'), None, None),
+    # specify topdir in a sibling
+    (None, Path('..') / 'sibling', None, None, None),
+    # error cases
+    # specify topdir and [directory]
+    (None, Path('subdir'), Path('not-used'), None, subprocess.CalledProcessError),
+    # specify a non-existent manifest file
+    (None, Path('.'), None, Path('non-existent.yml'), subprocess.CalledProcessError),
+    ######################################
+    # LOCAL MANIFEST
+    ######################################
+    # init workspace in current working directory (without --topdir)
+    (Path('workspace') / 'zephyr', None, Path('workspace') / 'zephyr', None, None),
+    # init workspace in current working directory
+    (Path('workspace') / 'zephyr', Path('.'), Path('workspace') / 'zephyr', None, None),
+    # init workspace in a subfolder of current working directory
+    (Path('workspace') / 'zephyr', Path('workspace'), Path('workspace') / 'zephyr', None, None),
+    # init workspace in current working directory by providing a manifest file
+    (Path('workspace') / 'zephyr', Path('.'), Path('workspace'), Path('zephyr') / 'west.yml', None),
+    # init workspace in itself by providing manifest file
+    (
+        Path('workspace') / 'zephyr',
+        Path('.'),
+        Path('.'),
+        Path('workspace') / 'zephyr' / 'west.yml',
+        None,
+    ),
+    # init workspace in a subfolder by providing manifest file
+    (
+        Path('workspace') / 'subdir' / 'zephyr',
+        Path('workspace'),
+        Path('workspace') / 'subdir',
+        Path('zephyr') / 'west.yml',
+        None,
+    ),
+    # init workspace in a subfolder by providing manifest file
+    (
+        Path('workspace') / 'subdir' / 'zephyr',
+        Path('workspace'),
+        Path('workspace'),
+        Path('subdir') / 'zephyr' / 'west.yml',
+        None,
+    ),
+    # error cases
+    # init workspace without a directory
+    (Path('workspace') / 'zephyr', Path('.'), None, None, subprocess.CalledProcessError),
+    # init workspace in a sibling repository path
+    (
+        Path('workspace') / 'zephyr',
+        Path('sibling'),
+        Path('workspace') / 'zephyr',
+        None,
+        subprocess.CalledProcessError,
+    ),
+    # init workspace from non-existent manifest
+    (
+        Path('workspace') / 'zephyr',
+        Path('.'),
+        Path('non-existent.yml'),
+        None,
+        subprocess.CalledProcessError,
+    ),
+    # init workspace from a manifest not inside the workspace
+    (Path('..') / 'zephyr', Path('.'), Path('..') / 'zephyr', None, subprocess.CalledProcessError),
+]
+
+
+@pytest.mark.parametrize("test_case", TEST_CASES_INIT_TOPDIR)
+def test_init(repos_tmpdir, test_case):
+    repos_tmpdir.chdir()
+    flags = []
+
+    local_dir, topdir, directory, manifest_file, expected_error = test_case
+
+    # prepare local manifest in local_dir
+    if local_dir:
+        # place the local manifest to given path
+        clone(str(repos_tmpdir / 'repos' / 'zephyr'), str(local_dir))
+        flags += ['-l']
+    else:
+        # clone from remote manifest
+        flags += ["-m", repos_tmpdir / 'repos' / 'zephyr']
+
+    # extend west init flags according to specified test case
+    if topdir:
+        flags += ['-t', topdir]
+    if manifest_file:
+        flags += ['--mf', manifest_file]
+    if directory:
+        flags += [directory]
+
+    # initialize west workspace
+    if not expected_error:
+        cmd(['init'] + flags)
+    else:
+        cmd_raises(['init'] + flags, expected_error)
+        return
+
+    # go to west workspace and check for correct config
+    if local_dir:
+        # topdir is either specified or default (directory.parent)
+        workspace = topdir or directory.parent
+    else:
+        # topdir is either specified, directory or default (cwd)
+        workspace = topdir or directory or Path.cwd()
+
+    os.chdir(workspace)
+    actual = cmd('config manifest.path')
+    if local_dir:
+        assert Path(actual.rstrip()) == directory.relative_to(workspace)
+    else:
+        assert Path(actual.rstrip()) == Path('zephyr')
+
+    manifest_file = manifest_file or Path('west.yml')
+    actual = cmd('config manifest.file')
+    assert Path(actual.rstrip()) == Path(manifest_file)
+
+    # update must run successful
+    cmd('update')
+
+
 def test_init_local_with_empty_path(repos_tmpdir):
     # Test "west init -l ." + "west update".
     # Regression test for: