west library / application / bootstrap structure and API

[tejlmand]

from the user point of view west is the go-to tool intended for repository handling, building, flashing etc.

Internally, west can be seen as:

• bootstrap functionality
• west library (a library which can be reused by build scripts, test, etc)
• application (command line)

With the initial version, v.0.1.0 out and progress is made on v0.2.0, it is important to keep track of the future direction.

This includes, but is not limited to:

• Structure and name spacing
• API
• What features of west do we expect other would re-use ?

[tejlmand]

IRC discussion found in attachment.
#zephyr-meta.txt

[ulfalizer]

Thanks for opening this!

Re. the API, I would go for a plain

from west import ESP32BinaryRunner

(Much nicer than the ugly separate top-level runners thing I suggested earlier.)

I don't think West is large enough to warrant any namespacing in the API. Heavily namespaced code tends to be hard to read, IMO (west.ESP32BinaryRunner reads nicer than west.runners.ESP32BinaryRunner, and in that case there's no risk for collision).

For the structure, simplest thing would just to put everything at the top level I think. Probably too simple though. At least the tests ought to go in a separate directory.

bootstrap.py
westcmd.py
west.py (the library, imported by e.g. westcmd.py)
command_build.py
command_flash.py
...
runner_bossac.py
runner_xtensa.py
...
tests/

Adding one layer of subdirectories:

bootstrap.py
westcmd.py
west.py
commands/__init__.py
commands/build.py
commands/flash.py
...
runners/__init__.py
runners/bossac.py
runners/xtensa.py
...
tests/

west.py could populate the API with something like

from runners.bossac import BossacBinaryRunner
from runners.xtensa import XtensaBinaryRunner
...

I'm sure it could be tweaked (didn't even list a bunch of other files), but personally I prefer fairly shallow and "cd-less" hierarchies at least, unless the project is massive.

Any thoughts?

[ulfalizer]

I looked into how the Python standard library tends to organize things (can be found in /usr/lib/python3.6 or the like), and I think it's pretty nice:

• The majority of the modules are one file each (os.py, argparse.py, etc.)

• Modules that use "helper" modules tend to make stuff from them available by importing from them in __init__.py. You get this in json/__init__.py, for example:

  from .decoder import JSONDecoder, JSONDecodeError
  from .encoder import JSONEncoder

  (Those relative imports are the standard library being extra cautious, but they don't hurt at least.)

  decoder(.py) and encoder(.py) are not part of the public API, just a way to split the implementation into multiple files. To access e.g. JSONDecoder, you do

  from json import JSONDecoder

• Some modules, like collections, put nearly all their code into __init__.py.

  I suspect this is a leftover from being a single .py file in the past, before other stuff got added (there's two files in collections/: __init__.py and abc.py, where abc.py is meant to be accessed as collections.abc).

Adopting the suggestion from the last comment to that approach, you get Mark II:

#
# West-the-application
#

bootstrap.py
westcmd.py
...
commands/__init__.py
commands/build.py
commands/flash.py
...

#
# West-the-library
#

# (A name like libwest/ would be nice for the directory, but would probably
# add some renaming shenanigans)

west/__init__.py
west/build.py
west/cmake.py
west/...
west/runner_bossac.py
west/runner_arc.py
...

*Misc.*
tests/

(The runners could go in an extra directory as well, though there aren't a lot of them at the moment, and shaving a directory level is always nice. It's an implementation detail anyway.)

west/__init__.py would contain code similar to the old west.py code:

from .runner_bossac import BossacBinaryRunner
from .runner_xtensa import XtensaBinaryRunner

People would then be able to simply do

from west import BossacBinaryRunner, XtensaBinaryRunner

commands/__init__.py could do a similar thing to make it easy to use:

from .debug import Debug
from .flash import Flash
...

This is pretty similar to the old design, just a bit stripped down, and with the library clearly separated out.

@mbolivar @tejlmand @carlescufi
Thoughts?

Edit: Wonder if Mark III would be to get rid of the commands/ directory...

[mbolivar]

@mbolivar [...]
Thoughts?

I think this discussion is a great one and worth having, but that before we go and do The Big Refactor, we should at a minimum have an answer for how people are going to define and use custom, out of tree, never-going-to-be-upstreamed west commands without having to touch the main west repository.

I also think having a west/__init__.py won't work because that would make west a "regular package" in PEP420 terms, and we have already decided not to do that by having the bootstrap code separately importable as west._bootstrap.

[carlescufi]

@ulfalizer thanks for the detailed description.

To be completely honest to you and @mbolivar, I do not know about the exact details of PEP420 so I don't think I can have an opinion on using the same packaging approach as the Python standard library. I believe we took a decision on importing everything from a single namespace, meaning the from west import <module> side of things is already covered.
Currently the actual west source code is in src/west:

├── src
│   └── west
│       ├── _bootstrap
│       ├── commands
│       └── runners

I am not sure if you want to move that to the root folder?
Regarding the "West the application" vs "West the library" split you mention, the issue I have with it is that then flash and build are not part of the library? I'd personally like them to be, in the sense that it would be great to have a build and flash API set exposed by the library.

I feel we need to discuss this in the context of:

1. What functionality should actually be in the library
2. What functionality should be "app-only"
3. What functionality should be extendable by the downstream maintainer

Maybe we could start there? Defining in detail what belongs where?

[ulfalizer]

Heh, didn't notice that people had written stuff here.

I also think having a west/__init__.py won't work because that would make west a "regular package" in PEP420 terms, and we have already decided not to do that by having the bootstrap code separately importable as west._bootstrap.

Yeah, that part irks me. The bootstrapper could just be a single bootstrap.py script at the top level I think, and it'd be super simple to understand and work with, even for Python noobs.

I don't understand what we need namespace packages for, and I've never seen them come up outside distribution packaging and the like. It's just more stuff that'll confuse people I think. :)

Regarding the "West the application" vs "West the library" split you mention, the issue I have with it is that then flash and build are not part of the library? I'd personally like them to be, in the sense that it would be great to have a build and flash API set exposed by the library.

Personally, I'd just move the flashers and the like out of West and into the Zephyr repo. That has at least three advantages:

1. You don't get a bunch of unrelated subcommands in the same tool, where it might be unclear which ones deal with repo management, etc.

2. Working on the flashers won't interfere with the repo-like's self-update mechanism.

   Working on the Zephyr source code is a lot smoother than working on the West source code in general, due to the oddball self-update stuff (which might make sense for the repo parts though).

3. The flashing helpers are closely linked to the CMake files, which live in the Zephyr repository. Storing them together makes it easy to sync changes to both.

I feel we need to discuss this in the context of:

1. What functionality should actually be in the library
2. What functionality should be "app-only"
3. What functionality should be extendable by the downstream maintainer

Sounds good.

Currently, I don't think we have anything like a well-defined API. It's more of a "let's make everything importable in case someone wants to use it".

I suspect we don't even need a library though, at least not yet. It feels like we're trying to be a bit too "enterprisey".

[tejlmand]

Well, from early discussion, west is the tool to use with zephyr, but not a build-system.
As is stated here: https://docs.zephyrproject.org/latest/west/index.html

The Zephyr project is developing a swiss-army knife command line tool named west.

So west is not only a repo handling tool, but a toolbox around Zephyr.
Where the zephyr-project repo then becomes the source and build system.

In this context, it makes sense to have to flashing scripts in west.
And to use west as the goto command when working with Zephyr,.
In that context it also makes sense to have west build which then runs cmake + ninja.
As well as west flash for flashing.

About making a lib, then for automated build systems, it is often nicer / more convenient to be able to interface to an well-structured API, instead of a command line interface.

1. You don't get a bunch of unrelated subcommands in the same tool, where it might be unclear which ones deal with repo management, etc.

I assume this is because you consider west's main feature to be the repo part.
That's not how I understand the long term development of west.
But repo handling is the first goal, in order to reach the others.

2. Working on the flashers won't interfere with the repo-like's self-update mechanism.

Working on the Zephyr source code is a lot smoother than working on the West source code in general, due to the oddball self-update stuff (which might make sense for the repo parts though).

So maybe we should re-visit the self-update mechanism for west itself ?
Or make adjustments.

3. The flashing helpers are closely linked to the CMake files, which live in the Zephyr repository. Storing them together makes it easy to sync changes to both.

Why is that ?
CMake is a tool to manage the build process.
ninja / make are build systems.
Don't see why flashing is close related to build system, except that in Zephyr context is has been decided to facilitate flashing by making a target, such as ninja flash
But west flash could have been just a good a choice.

Note: Actually I think Zephyr is the first time I have experienced the possibility to use a build system to flash a target.

[ulfalizer]

So west is not only a repo handling tool, but a toolbox around Zephyr.
Where the zephyr-project repo then becomes the source and build system.

In this context, it makes sense to have to flashing scripts in west.
And to use west as the goto command when working with Zephyr,.
In that context it also makes sense to have west build which then runs cmake + ninja.
As well as west flash for flashing.

That still feels like "we decided that it makes sense to lump a bunch of things into a west command, therefore it makes sense to do that", at least to me.

I don't think we should get hung up on the name. What we're adding is a repo tool and some flashing and building helpers. Calling both of them West doesn't say anything about where it makes the most sense to implement each.

I assume this is because you consider west's main feature to be the repo part.
That's not how I understand the long term development of west.
But repo handling is the first goal, in order to reach the others.

It's because I think those are unrelated features, regardless of whether you call them West or not (and regardless of what you call any command(s)).

I think we ought to be thinking "does it make sense to put these features together?", ignoring the West part altogether. We could still it call some things West if we want to, for branding. That doesn't mean that they all have to go in the same command, or be implemented in the same place.

So maybe we should re-visit the self-update mechanism for west itself ?
Or make adjustments.

Easiest would be to implement features where the self-update mechanism makes less sense somewhere else, I think.

Why is that ?
CMake is a tool to manage the build process.
ninja / make are build systems.
Don't see why flashing is close related to build system, except that in Zephyr context is has been decided to facilitate flashing by making a target, such as ninja flash
But west flash could have been just a good a choice.

I thought they were wrappers around existing CMake commands, but I might be wrong. It feels like they make the most sense in the Zephyr repository at least.

Doing a reversal test, if the building and flashing helpers were already implemented in the Zephyr repository, would you want to move them into the tool that implements the multi-repo commands? What advantages do you see with that?

[mbolivar]

What we're adding is a repo tool and some flashing and building helpers.

I am confused about your continued and persistent insistence on this point, which I feel we have discussed at length already is not correct. It seems like you are looking at what is currently in the west source code and deciding that is all that will ever be in west.

West is a meta tool for doing Zephyr development. Its commands will implement what is required to develop embedded applications using the Zephyr RTOS. Almost by the definition of an RTOS that involves flashing and debugging binaries on supported targets. For various reasons it is also going to involve managing multiple git repositories in a coordinated way. Concluding that it stops there makes no sense.

Doing a reversal test, if the building and flashing helpers were already implemented in the Zephyr repository, would you want to move them into the tool that implements the multi-repo commands? What advantages do you see with that?

We have also discussed this repeatedly. I feel it is obvious that you disagree with the outcome, but I don't think it's worth continually revisiting this subject.

Separate orthogonal tools would be the result of applying the Unix philosophy to implementing software to manage Zephyr's user workflows. That is well understood. The Unix philosophy has advantages and disadvantages which are discussed ad nauseam in numerous places and I don't think it's worth repeating them as if this were still a debate in this context.

Can we move on?

[tejlmand]

Agreeing with @mbolivar here, so no need to repeat him on that part.

I thought they were wrappers around existing CMake commands, but I might be wrong.

Nope, they are referred to from CMake and are defined as custom targets, found in cmake/flash/CMakeLists.txt
They are added in a loop which expands to:
add_custom_target(flash .....)
add_custom_target(debug .....)
add_custom_target(debugserver .....)
add_custom_target(attach .....)
which all will execute ${ZEPHYR_BASE}/scripts/meta/west/main.py with different parameters when you type make flash/debug/attach, etc. or ninja flash/debug/attach, etc..

Note the python script folder, those are already west commands, pre-dating the repo support.

Furthermore I think it makes sense to have this in the west repo.
and it is cleaner to have a given set of commands, such as:
west flash
west debug
west attach
and then the build command does what it is supposed to do: build the project.
(imho those flash/debug commands could then potentially be removed from the build targets, but guess that's a completely different discussion)

Those independent tools can be maintained without needing to checkout zephyr repo.
(Just as OpenOCD, JLink, etc. are maintained outside Zephyr project)

[ulfalizer]

@mbolivar

I am confused about your continued and persistent insistence on this point, which I feel we have discussed at length already is not correct. It seems like you are looking at what is currently in the west source code and deciding that is all that will ever be in west.

Alright, so we're adding a repo tool, some building and flashing helpers, and some other tools in the future. Do those tools that we haven't added yet mean that it makes more sense for the repo tool to be bundled with the building and flashing helpers?

That's a legit question. I'm not trying to be difficult, but I just don't get it. We obviously are adding a repo tool and some building and flashing helpers, and it doesn't seem like those go together at all. Bundling them together leads to weird and bad behavior like the repo tool's self-update mechanism coming into play when people work on the flashing helpers.

This isn't some Unix purism thing to me. I just don't get why we decided to suddenly start putting everything new into a single command. Do we have any criteria for what goes in the west command?

I think we need to avoid confusing West-the-new-hotness with the west command. We could still call it West even if it's split into pieces. We don't have to put everything into the same command. :)

@tejlmand
Hmm, so does that mean that Zephyr actually depends on West now, rather than the other way around?

I was thinking you could just do something like ninja flash, to call the flashing helpers in the Zephyr repo (or wherever it makes the most sense to implement them, and you already know where I don't think it makes sense... ;).

[mbolivar]

Do we have any criteria for what goes in the west command

Rough and consensus-driven, but yes: it belongs in West if it's a core user workflow.

"Get the code, keep it up to date" passes this test. "Run and debug code on my target" clearly does as well.

This is like awscli for zephyr.

[tejlmand]

This isn't some Unix purism thing to me. I just don't get why we decided to suddenly start putting everything new into a single command. Do we have any criteria for what goes in the west command?

Well, to me, this seems to have been discussed very early in the phase of creating stand-alone west.
So this comes as no surprise to me.
Whereas, limiting west to repo handling apears to be a change in strategy.
Note: change in strategy can of course be done if we decide so, but for now, I think i'm more aligned with @mbolivar that west is the tool for core user workflow.

Hmm, so does that mean that Zephyr actually depends on West now, rather than the other way around?

yep it does.
and I guess the motivation for putting flashing and debugging into the build commands, was because people would already do ninja or make for building. But I guess that's an old discussion from before I started working on Zephyr. Maybe @carlescufi knows more on this.

But when having west as the core workflow tool, I think it makes a lot of sense to have west to be able to do as @mbolivar points out

"Get the code, keep it up to date" passes this test. "Run and debug code on my target" clearly does as well.

and then the build system (being cmake/ninja, cmake/make, or something else in future), is just a build system.
Today core workflow tools are placed inside build system.
Which is also unusual, when you start thinking about it.

[carlescufi]

@mbolivar @tejlmand @ulfalizer

Let me settle this once and for all. The scope of the tool has been described in an issue for a long time now, and we are not going to change it now.

zephyrproject-rtos/zephyr#6205

[mbolivar]

@carlescufi @tejlmand now that we've settled the design down a little bit, I think it might be okay to revisit this subject.

Here is what we have left in west/src after moving the runners and build/flash/debug commands out:

west/src
└── west
    ├── _bootstrap
    │   ├── __init__.py
    │   ├── main.py
    │   ├── version.py
    │   └── west-schema.yml
    ├── commands
    │   ├── command.py
    │   ├── __init__.py
    │   ├── project.py
    │   └── west-commands-schema.yml
    ├── build.py
    ├── cmake.py
    ├── config.py
    ├── log.py
    ├── main.py
    ├── manifest.py
    ├── manifest-schema.yml
    └── util.py

I would like to propose the following layout as a way to merge the bootstrapper, "real" main, and built-in commands, along with a library-style API for other features:

west/src
└── west
    ├── app
    │   ├── builtins.py
    │   ├── __init__.py
    │   ├── main.py
    │   └── version.py
    ├── commands.py
    ├── config.py
    ├── log.py
    ├── manifest.py
    ├── schemas
    │   ├── manifest-schema.yml
    │   ├── west-commands-schema.yml
    │   └── west-schema.yml
    ├── schemas.py
    ├── util.py
    └── zbuild.py

In particular:

• The application (or non-library) components are combined into a new
  west/src/west/app. This includes every non-schema part of
  west/src/west/_bootstrap, as well as west/src/main.py (which
  gets merged with west/src/west/_bootstrap/main.py into the new
  west/src/app/main.py), and west/src/west/commands/project.py
  (which gets reborn as west/src/app/builtins.py, a place where
  built-in commands are defined). Users who import this package and
  use its contents as Python modules will never have any API guarantees.

• We stop importing schema .yml files directly and put them behind a
  library interface whose Python package/module is named west.schemas, and
  whose source file is west/src/west/schemas.py. The schema data
  itself will be available as string attributes in that package:

  • west.schemas.WEST_SCHEMA
  • west.schemas.MANIFEST_SCHEMA
  • west.schemas.WEST_COMMANDS_SCHEMA

  Behind the scenes, these will exist in the source repository as .yml
  files, but that is an implementation detail in case we ever have to
  get tricky with the resource loading.

• In addition to west.schemas, we take a look at the remaining
  "library" APIs and move towards making their interfaces stable. This
  covers the creation of at least the following modules, defining their
  APIs, and including them in the Zephyr documentation (with a combo
  of autodoc directives and .rst text):

  • west.commands: in west/src/west/commands.py, this is the
    "library part" of the commands API, defining things like
    WestCommand etc.

  • west.config: in west/src/west/config.py, this is a
    non-stateful library interface for west configuration files. the
    west/src/west/app code could load a configuration file and pass
    state around for the current app however it wants, but NO state
    in here (see Global west state makes it harder to unit test  #149)

  • west.log: I'm actually not sure I want to keep this one, but
    it'd be the non-stateful bits of logging (i.e. everything except
    the verbosity state). We might want to move to Python's standard
    logging; as I said, I'm not sure. We might want to move the
    module-level functions here to class- or instance-level methods in
    WestCommand (but that would complicate the runners package a
    bit)

  • west.manifest: in west/src/west/manifest.py, this is a library
    for interacting with west manifest files (and eventually manifest
    directories if we go that route)

  • west.util: in west/src/west/util.py, this is the same old
    utility code we've found useful, maybe with a bit of polish to
    make sure we're ready to stand by the API.

  • west.zbuild: in west/src/west/zbuild.py, this is library code
    for interfacing with the zephyr build system; this combines the
    old build.py and cmake.py files in west/src/west.

[mbolivar]

I'm calling this a bug as the lack of even stable west.module APIs to import is a real problem for people who want to write their own extensions.

[marc-hb]

Let me settle this once and for all. The scope of the [meta-!]tool has been described in an issue for a long time now, and we are not going to change it now.
zephyrproject-rtos/zephyr#6205

This settles the "one single command" CLI question. I hope the internal implementation, APIs and documentation will keep preserving as much isolation as possible between individual tools and the many benefits of software modularity; not the least troubleshooting and maintenance. I also hope the number of "apparently circular" dependencies like ninja flash-> west flash -> ninja will be kept to a minimum because they make it a bit harder to understand how the whole build system is structured and designed. My 2 cents, thanks!

[mbolivar]

Fixed in #358