multigit: support external modules outside of zephyr [RFC]

[nashif]

Support modules outside of the Zephyr tree. This is the first step for
supporting multigit feature.

Modules and HALs in this setup are hosted at the same level as the
zephyr tree:

zephyr/
modules/
hals/

Each module should have at least the following files:

module.yml
CMakeLists.txt
Kconfig.module

module.yml is for module meta-data such as license, url, details, etc.

Signed-off-by: Anas Nashif anas.nashif@intel.com

[carlescufi]

I don't think we should have commented out code at all in a PR, regardless of RFC or not

[carlescufi]

does this work with absolute paths? I guess so, just checking

[mbolivar]

I'm not sold on the idea of having a hal/ and module/ directory like this. Soon enough some other user will want boards/, someone will want libs/, someone will want something in Russian, and we'll go on and on.

Why not just glob for ENV_VAR_SYM_CONTAINER_DIR/*/Kconfig.module or so? (I'm not sure how the glob syntax works in our Kconfig dialect, but I basically mean find $ENV_VAR_SYM_CONTAINER_DIR -name Kconfig.module).

[SebastianBoe]

"Why not just glob for ENV_VAR_SYM_CONTAINER_DIR/*/Kconfig.module or so? (I'm not sure how the glob syntax works in our Kconfig dialect, but I basically mean find $ENV_VAR_SYM_CONTAINER_DIR -name Kconfig.module)."

You can't do that.

If a user has cloned Zephyr to $HOME that would cause CMake to run a handful of system calls on every directory recursively down from HOME.

Having one or two directories like hal/modules creates a directory namespace that prevents this from happening (on most systems).

stat("/home/sebo/.config/google-chrome", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
lstat("/home/sebo/.config/google-chrome", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
open("/home/sebo/.config/google-chrome", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
getdents(3, /* 36 entries */, 32768)    = 1528
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0

So, I would suggest it could look like this instead:

container_dir/zephyr
container_dir/modules/ext_dep_1
container_dir/modules/ext_dep_2
container_dir/modules/ext_dep_3

alternatively that we introduce a backwards-incompatible change that requires users to
move their git repo into a container directory. Then it could look like this:

container_dir/zephyr
container_dir/ext_dep_1
container_dir/ext_dep_2
container_dir/ext_dep_3

[SebastianBoe]

Another approach;

instead of having the build system search for modules one could have CMake either ask the multi-tool what directories to use, e.g.

execute_process(tool --list-external-repo-directories EXT_REPO_DIRS)

or have the tool spit out a file that CMake is able to read. E.g.

include(${ZEPHYR_BASE}/../external_repos.cmake)

or

execute_process(PYTHON parse_repos.py ${ZEPHYR_BASE}/../manifest.xml EXT_REPOS)

Then we can have the originally desired directory layout.

[mped-oticon]

@SebastianBoe Inspired by your suggestion, I'd propose something simpler (pseudo-code), where we look for a sentinel file as was mentioned earlier.

extfile=${ZEPHYR_BASE}/../zephyr_external/sentinel.cmake
if $extfile exists: include($extfile)

Then we would not need to run any external tool. And we would obtain complete flexibility of placement of modules/hals/etc.
For KConfig, I'm not sure. If nothing else, I suppose the sentinel.cmake could generate appropriate KConfig source paths for the modules it already knows about.

I think it would be wise to qualify the sentinel dir with "zephyr_external" or similar, rather than "hal", "modules", "external_repos.cmake" since those might actually collide.

Finally, note that not everybody uses Google's repo tool to administer multiple gits, thus the manifest.xml file can't be assumed to exist. Note that out of tree external modules might not even be gits.
I'd like to not confuddle the build with more python scripts and assumptions on particular git-manangement tool.

[carlescufi]

I wonder if this line should be in the root CMakeLists.txt instead

[carlescufi]

or even better in boilerplate.cmake, that's where we define ZEPHYR_BASE

[SebastianBoe]

NACKing to allow time to review.

[codecov-io]

Codecov Report

Merging #7338 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7338      +/-   ##
==========================================
+ Coverage   55.09%   55.09%   +<.01%     
==========================================
  Files         474      474              
  Lines       51786    51786              
  Branches     9937     9937              
==========================================
+ Hits        28530    28531       +1     
+ Misses      19304    19303       -1     
  Partials     3952     3952

Impacted Files	Coverage Δ
lib/posix/pthread.c	69.84% <0%> (+0.5%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af8a0b1...120256f. Read the comment docs.

[mbolivar]

Modules and HALs in this setup are hosted at the same level as the
zephyr tree:

Why are HALs not just modules that happen to contain vendor HAL code? What makes them special?

[nashif]

Why are HALs not just modules that happen to contain vendor HAL code? What makes them special?

no special reason, started this way and change lots of assumptions along the way to make this less intrusive and fast. We can put everything under modules. Having hals have its own structure cleanly separates what you want to always have (libraries) from hals where you might want to drop the things you do not need and do not support.

[mbolivar]

We can put everything under modules.

Do we need to, though? I'm saying everything underneath abspath(ZEPHYR_BASE/..) could be treated as a potential module, so e.g. all of these would get found automatically:

container_dir/
├── hal
│   ├── nordic
│   │   ├── CMakeLists.txt
│   │   ├── Kconfig.module
│   │   └── module.yml
│   ├── nxp
│   │   ├── CMakeLists.txt
│   │   ├── Kconfig.module
│   │   └── module.yml
│   └── st
│       ├── CMakeLists.txt
│       ├── Kconfig.module
│       └── module.yml
├── lib
│   ├── json
│   │   ├── CMakeLists.txt
│   │   ├── Kconfig.module
│   │   └── module.yml
│   └── mbedtls
│       ├── CMakeLists.txt
│       ├── Kconfig.module
│       └── module.yml
└── zephyr
    ├── CMakeLists.txt    <---- not sure about this one, maybe the zephyr core is not a module
    ├── Kconfig.module
    └── module.yml

[nashif]

Do we need to, though?

well, that should work and that is how I started, however, globbing the tree without a defined depth will have a performance hit. The above is possible for sure we just need to be careful about very deep trees where finding a module would take valuable seconds...

[mbolivar]

globbing the tree without a defined depth will have a performance hit

Fair enough; perhaps we should have some sort of nesting depth? The alternative is to manage the hierarchy manually in files, but I think we discussed that we don't want to do that.

[SebastianBoe]

Change requests are inline.

[SebastianBoe]

"no special reason, started this way and change lots of assumptions along the way to make this less intrusive and fast. We can put everything under modules. Having hals have its own structure cleanly separates what you want to always have (libraries) from hals where you might want to drop the things you do not need and do not support."

Libraries and hal's are not different wrt. whether they are desired or not.

You don't always want to have libraries, and you don't always want to have hal's.

I cannot either see any reason why we would need to distinguish between hal's and non-hal's here.

EDIT: Did not see discussion about why modules and hal's were introduced. See this thread for a mechanism that does require these two concepts. #7338 (comment)

[carlescufi]

@nashif @mbolivar @SebastianBoe
Regarding the above conversation and the options suggested, I want to sum the up in order for us to be able to make a choice:

1. Hardcoded paths in the root CMakeLists.txt (hal/, lib/, etc). Not very elegant since it hardcodes those paths in a build file.
2. Search everywhere under ZEPHYR_BASE/.. for Kconfig.module and module.yaml. Can work, but as @SebastianBoe mentions if the user clones the standalone zephyr repo without a properly constructed container folder it will lead to massive globbing.
3. zephyr/ + modules/. I personally don't dislike that. If modules/ is not found, error. The only hardcoded path is ZEPHYR_BASE/../modules. Simple to maintain. How about zephyr/ + ext/ instead though? that would mirror the current structure but with separate repos. EDIT: ext/ is probably not a good idea, since there will be applications (like MCUboot) and others in there, unless we consider ext/ to mean "not developed by the Zephyr core team"
4. CMake runs west to find the list of folders with modules.
5. CMake parses the manifest directly to find the list of folders with modules.

I personally quite like option 3 because:

a. It separates the core zephyr and the "rest" cleanly
b. It introduces no dependencies between the git repo management and the build system other than the simple topmost folder layout.
c. It's very easy to implement and move forward.

[carlescufi]

Do we need to, though? I'm saying everything underneath abspath(ZEPHYR_BASE/..) could be treated as a potential module,

@mbolivar if you do this and the user has mistakenly cloned zephyr into ~/ then CMake could be globbing for a long long time.

[carlescufi]

I do not like the modules + hals split contained in the root CMakeLists file at the moment

[mbolivar]

Do we need to, though? I'm saying everything underneath abspath(ZEPHYR_BASE/..) could be treated as a potential module,
@mbolivar if you do this and the user has mistakenly cloned zephyr into ~/ then CMake could be globbing for a long long time.

I suppose you could fix that by making sure there was some sentinel file in there.

Anyway, in my original thinking, I didn't like the idea of west parsing the manifest to get a list of modules, but I think I'm changing my position here and am in favor of that, at least for now :).

My main concern here is that we are careful to keep any west commands related to managing multiple git repositories really cleanly separated from everything else in the tool (except perhaps for common stuff like logging). But if other parts of our tooling want to depend on a particular multi-repo management strategy, that seems fine.

This is because I've seen the general problem of wanting something like repo that also works on Windows for years, and I think there's a chance to make a nice tool for this. Plus I don't see why from a layering perspective why the Git management commands should depend on anything else in the tool.

[carlescufi]

I didn't like the idea of west parsing the manifest to get a list of modules,

Did you mean CMake? west would need to use the manifest, no way around that me thinks. west is the repo tool after all.

I suppose you could fix that by making sure there was some sentinel file in there.

But in what repo would that file livein ? Or would it be generated by west when syncing? If so we are again adding dependencies between west and CMake.

@nashif @mbolivar can you then propose an option 6. perhaps or vote for one of the above?

[mbolivar]

I didn't like the idea of west parsing the manifest to get a list of modules,

Did you mean CMake? west would need to use the manifest, no way around that me thinks. west is the repo tool after all.

Yes, sorry, I meant something along the lines of what @SebastianBoe said: west would be able to output something palatable for CMake to get the list of current modules. So west is still parsing the manifest as you say, it's just also got some code for outputting the contents in a way CMake can handle.

I suppose you could fix that by making sure there was some sentinel file in there.

But in what repo would that file livein ? Or would it be generated by west when syncing? If so we are again adding dependencies between west and CMake.

Just thinking out loud here, if zephyr always lives alongside the manifest repository, then that could work as a marker that it's a "full" checkout and not just a mistaken checkout of Zephyr itself.

Though if we go the manifest -> west list -> CMake route, that's a moot point.

@nashif @mbolivar can you then propose an option 6. perhaps or vote for one of the above?

Thanks for making the list. I vote option 4. I don't like option 3 much because it seems to enforce a flat hierarchy in modules, which is sure to violate somebody's requirements down the line.

[carlescufi]

Just thinking out loud here, if zephyr always lives alongside the manifest repository, then that could work as a marker that it's a "full" checkout and not just a mistaken checkout of Zephyr itself.

That will however depend on how we structure the manifest in west, won't it? It might be a hidden folder (I guess we could look for that as well). In any case, yes, you are right, but it then introduces a dependency between the build system and the tool (west).

Anyway, in my original thinking, I didn't like the idea of west parsing the manifest to get a list of modules, but I think I'm changing my position here and am in favor of that, at least for now :).

Were you thinking about 2 separate tools? One for building and running, and another one for managing repos? Because otherwise, if we have a single tool, then there is no way around west reading the manifest and actually even populating it in some cases.

I'm also rethinking all of this today and I am sort of leaning towards option 2. It's the only one that allows the build system to be completely independent of the Git repo management and I think we can mitigate the potential massive globbing by adding a top-level module.yaml or zephyr.yaml on the root folder of every single one of our Git repos. Then CMake could go:

for each folder:
   if (!exists folder.module.yaml)
      continue;
   else
      parse_module()

@SebastianBoe @nashif @mbolivar would that work for you?

EDIT: Actually that might not work if west can place Git repos buried under several layers of folder hierarchy that is only present in the clone, but not in the upstream repos. In that case I guess maybe option 4. is the better one if we want arbitrary folders and names to place the repos in.

[mbolivar]

To help narrow things down, I'd like to propose that we eliminate the following options from consideration before looking at what is left.

AFAICT nobody is currently voting for them.

Option 1 (hardcoded paths in root CMakeLists.txt)

Seems like a bad workflow for multiple reasons.

• Since upstream Zephyr version-controls and edits this file, it seems to always imply annoying merge conflicts for third party developers who need to manage their own out of tree modules.
• It means that the multigit tool will have to parse -- and potentially update -- the CMakeLists.txt, which is a layering violation and sounds painful.

Option 3 (zephyr + modules)

The flat module namespace is highly restrictive. I think it makes sense to allow modules to be specified in subdirectories. Look at the way we have per-vendor directories in ext/hal today; this is useful and keeps things clean. Think about use cases like recursive grep within a certain vendor's HALs, etc.

I also think modules in subdirectories will become a requirement someday even if we can start out without it. For example, consider that MCUboot has a submodule which needs to be checked out underneath the mcuboot/sim/mcuboot-sys/ directory for the simulator to build.

At OSF, we support this in repo by adding an additional project to the manifest that puts the Git repository where git submodule expects it. In upstream Zephyr, a flat modules namespace would not permit handling this properly if mcuboot is itself a module.

(@carlescufi voted for this originally, but is now leaning away from it.)

Option 5 (CMake parses manifest directly)

I think CMake is not a suitable implementation language for managing complicated text formats, and the manifests are going to be complicated, since they'll need to manage:

• Git remotes (unless we hard-code URLs for each project)
• Git repositories and their attributes, such as ref to check out, path in the local filesystem (it's useful to be able to change this from the upstream repo name), etc.
• More as time goes on and we add features; see https://gerrit.googlesource.com/git-repo/+/master/docs/manifest-format.txt for an example of where this ended up for repo

Just because CMake is Turing complete doesn't make it the right tool for the job. Its "quirky" scoping rules, evaluation and conditional behavior, lack of proper data types (not even lists or arrays), etc. rule it out in my opinion. Whether we put it into west or something else, managing module manifests in Python makes a lot more sense to me.

[mped-oticon]

I made a more elaborate comment under the review, but perhaps it is valuable to note again that not everybody uses repo/manifest to administer their external modules.
Therefore a proper solution should make as few assumptions as possible, and preferably have some optional commonly-agreed hook-points that vendors can use.

E.g. at Oticon we have historically been using Perforce which supports arbitrary mappings of the depot tree. While we are moving away from Perforce to git, some shared firmware components still exist in Perforce. Thus the only commonality is the file system workspace tree. Thus out-of-zephyr-tree modules 1) can't be assumed to be managed by repo, 2) may have complicated mappings on the workspace.

[carlescufi]

@mped-oticon I understand your point of view, but we need to provide a way for our users to retrieve and submit changes to multiple repositories. I suggest we continue this conversation in #6770 and #6205. The need for a meta-tool and a common mutliple repository management system has been established. I understand that this will not play well with perforce or other version control tools, but we are not going to use repo, and one can always argue that west needs to support more version control tools and add support for those in it.

Dismissing stale review

[mbolivar]

@nashif should we close this? I believe at the TSC meeting we agreed to depend on west to list the out of tree modules. For now this means that we are going to get them from a west manifest, which only supports Git trees. We need to understand requirements better for including non-Git repositories in those manifests; perhaps @mped-oticon could help with that.